Skip to content

Cleanup Deal IDs and code from precommit #1695

@ZenGround0

Description

@ZenGround0

Problem

With the merging of #1691 there is now no way to prove commit a precommitment that includes deal ids. This means there is now an extra big footgun in builtin actors where an SP can

  1. Seal a sector with deals
  2. Precommit including the deal ids by mistake
  3. Wait more than a day + old 900 epoch finality (max precommit lookback)
  4. Realize there's no way to provecommit this precommitment with deal ids on chain prove_commit_sectors3 will just fail because it doesn't deal with precommits with deal ids.
  5. Never be able to pre commit this again because we are past precommit lookback
  6. All sealing cost is wasted

In practice people probably don't run into this since lotus-miner and curio are updated to handle this properly. But its a bad footgun to carry around with us. Before the latest change you could always gather together 3 extra sectors and then prove commit aggregate them (which unlike prove_commit_sectors3 did not require no deal ids on chain).

Solution

We should

  1. Assert deal_ids are empty and immediately fail on precomit. It would be even better to change the message format but thats too much of a pain requiring rolling a whole new method etc.
  2. Remove all deal checking /weight calculation code from precommit and therefore all of miner actor. Here are some things that can go

This will be a substantial cleanup to the precommit flow and the mental model of what is going on with commd declaration vs computation during pre and prove commit.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status

    📌 Triage

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions