filecoin-project / specs-actors

DEPRECATED Specification of builtin actors, in the form of executable code.
Other
86 stars 102 forks source link

Feat/aggregate porep less harsh error #1412

Closed ZenGround0 closed 3 years ago

ZenGround0 commented 3 years ago
  1. AggregateProveCommit does not fail if a subset of precommits exist on chain but are out of date (prove commit epoch greater than epoch due). Instead all inputs are provided to the proof, but only the sectors not out of date are added to the state through confirmSectorsProofsValid.
  2. PreCommitExpiries are now scheduled an additional 8 hours after the corresponding prove commit epoch is due. This provides much more time (8 hours vs 0-30 minutes) for aggregates including expired precommits to make it on chain. This is a somewhat arbitrary choice, feedback on this would be great

TODO:

ZenGround0 commented 3 years ago

Out of curiosity, why did you elect to write a scenario test rather than a unit test for testing the ProveCommitAggregate with a expired sector?

With the upcoming test vectors project all scenario tests will now generate conformance tests so I now have a slight bias towards testing with scenario tests if it is not too hard to do. In this case I was already warmed up writing similar tests from measuring aggregate porep gas so it seemed worth the extra effort.

codecov-commenter commented 3 years ago

Codecov Report

Merging #1412 (fa31738) into spike/aggregate-porep (418e07d) will increase coverage by 0.0%. The diff coverage is 81.2%.

@@                  Coverage Diff                  @@
##           spike/aggregate-porep   #1412   +/-   ##
=====================================================
  Coverage                   69.8%   69.8%           
=====================================================
  Files                         72      72           
  Lines                       7700    7792   +92     
=====================================================
+ Hits                        5378    5446   +68     
- Misses                      1438    1451   +13     
- Partials                     884     895   +11