filecoin-project / specs-actors

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

Feat/proof runtime types #1547

Closed ZenGround0 closed 2 years ago

ZenGround0 commented 2 years ago

Needed for snap deals integration

ZenGround0 commented 2 years ago

i'm surprised that the butterflynet was able to handle a PRU message if that's the case (unless it's just in the API or some such)

I am also surprised, I was assuming this would break butterfly. Then again if it was launched with the feat/snap-deals branch of lotus then this change was getting pulled in. feat/snap-deals updated actors v7 to point to here.

ZenGround0 commented 2 years ago

Where is this actually getting used? I didn't see any of the FFI calls changing around WinningPoSt...

Yes no changes to the ffi call. The problem is the layer above. It needs to be able to select between sealed copy and updated replica when reading on disk files to start satisfying post challenges for generating both winning and window post. This new sector info data type provides the info needed to select the correct file.

ZenGround0 commented 2 years ago
  1. Having trouble finding this with confidence. I'll try changing it back and seeing where things fail again tomorrow.
  2. Activation will tell us when the upgrade kicked in on chain. We need this because winning post needs to look at state a finality back and we want to make sure we only start generating winning posts off of the upgraded replica when the upgrade happened finality epochs ago.
ZenGround0 commented 2 years ago

Can you link me to where the assumption about pointerness lives in Lotus? I think butterfly was updated before this commit existed, so we have some figuring out to do.

Here it is: https://github.com/filecoin-project/lotus/blob/master/chain/stmgr/utils.go#L37. I think the butterfly mystery can be resolved by either 1) we built butterfly off of the feat/snap-deals branch which linked to a pointer return value, or 2) this message never succeeded on chain in butterfly.

codecov-commenter commented 2 years ago

Codecov Report

Merging #1547 (167118a) into master (2d5d491) will not change coverage. The diff coverage is 0.0%.

:exclamation: Current head 167118a differs from pull request most recent head 6148c0e. Consider uploading reports for the commit 6148c0e to get more accurate results

@@          Coverage Diff           @@
##           master   #1547   +/-   ##
======================================
Coverage    69.6%   69.6%           
======================================
Files          72      72           
Lines        8700    8700           
======================================
Hits         6060    6060           
Misses       1780    1780           
Partials      860     860