filecoin-project / builtin-actors

The Filecoin built-in actors
Other
81 stars 76 forks source link

Miner testing cleanup after FIP-0084 #1545

Open ZenGround0 opened 4 months ago

ZenGround0 commented 4 months ago

Cleanup configuration of commit_and_prove_sectors

commit_and_prove_sectors and commit_and_prove_sectors_with_cfgs are helper functions that add sectors to a miner actor in tests. Adding a sector is an important intermediate step for most of the miner actor's testing. There are > 40 calls to commit_and_prove_sectors or commit_and_prove_sectors_with_cfgs throughout miner actor tests.

1540 changes these helper functions to use the new ProveCommitSectors3 and PreCommitSectors2 methods as part of deprecating ProveCommitSector.

The configuration for commit_and_prove_sectors had grown into a bad state when deals are concerned. It is essential for callers to provide two separate configuration states: a vector of deal_ids and a ProveCommitCfg. Before #1540 this was not as obvious because the deal_ids would be passed to a precommit which would then statefully hold configuration information. In the new flow it is an error for deal_ids to be passed to pre commit so now both configurations must be passed not just to commit_and_prove_sectors_with_cfgs but also deprecated_sector_commit internally.

In order to keep things working consistently both pre commiting in commit_and_prove_sectors_with_cfgs and prove committing in deprecated_sector_commit use the make_piece_specs_from_configs function to deterministically generate an assignment between deal ids and deals. But this feels not quite right.

One way to clean this up is to modify the ProveCommitCfg so that users must associate deal ids with deal states.

Another is to rely on more prove_commit_sectors3 native configuration info in the form of manifests. The recently added onboard_sectors and onboard_empty_sectors helper methods have not been significantly integrated into existing tests. Adapting these new methods and integrating them into other tests is another approach that would result in a cleaner factoring.

Remove deprecated_sector_commit

Both commit_and_prove_sectors and a few stray external callers still call the deprecated_sector_commit method. As of #1540 this is now a wrapper around prove_commit_sectors3. We should look into removing this method and either call prove_commit_sectors3 directly or with a better wrapper function. We can then easily remove ProveCommitSector params types and associated testing functions for constructing them.

ZenGround0 commented 4 months ago

Naming cleanup

Rename actor/miner/tests files prove_commit2_failures_test and prove_commit2_test to be prove_commit3

anorth commented 4 months ago

See also #698 while investigating this issue.