Market: Publish Storage Deals needs hardening

[arajasek]

Thanks to @wadealexc, @Stebalien, etc. for the reports!

This is a summary of some concerns raised about the Market Actor's PublishStorageDeals method, as well as a proposed solution plan:

C1: An AuthenticateMessage impl can call back into PublishStorageDeals, potentially corrupting the market actor's state.
C2: An AuthenticateMessage impl can waste gas or fail in some unpredictable way that causes issues for PublishStorageDeals calls.
C3: If MarketNotifyDeals calls fail, the client contract is unaware. This could lead to attacks on DAOs (eg. a SP makes 100s of deals with a DAO that is unaware of the existence of these deals).

In order to solve these, the proposal is to adopt the following solutions / conventions:

• S1: Calls to AuthenticateMessage occur before market state is read. This happens in a separate loop before performing stateful validations like balance checks. This fully solves C1.
• S2: AuthenticateMessage implementations must be read-only, and are invoked as read-only by the market actor (and paych and verifreg actor?). This is not necessary to solve C1 if we adopt S1, but is likely a good restriction that we can loosen later if needed.
• S3: (convention) Client contracts are "trusted", that is, we assume the SP has vetted the code. This fully solves C2, and is relevant to mitigating C3.
• S4: (requires S3) A single failed call to MarketNotifyDeal fails the entire PSD message (all deals in the batch).
  • This was not considered in the original design of MarketNotifyDeal because we were concerned about the account actor's ability to accept these methods, but that is no longer a concern with feat: allow all unrestricted method numbers on accounts #1086.
  • This is risky if we are concerned about malicious clients wasting SP time / attacking their storage pipeline, but we are proposing adopting S3.
  • If we do S4, we should consider a client-side improvement where we don't batch deals if a client is a contract.

These proposals likely need a minor amendment to FIP-0050, but are otherwise easy to implement.

[jennijuju]

Good findings and agree with the approaches taken here. We can always lose/harden further according user feedbacks post launch!