Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate specs-actors v3 #5363

Merged
merged 58 commits into from
Jan 22, 2021
Merged

Integrate specs-actors v3 #5363

merged 58 commits into from
Jan 22, 2021

Conversation

arajasek
Copy link
Contributor

@arajasek arajasek commented Jan 16, 2021

As in the title. Needs:

  • some appeasing of tests
  • upgrade + state tree migration logic (can go into next in a separate PR)
  • careful review
  • testing

Fixes #5363

mid, err := address.IDFromAddress(maddr)
if err != nil {
return nil, xerrors.Errorf("getting miner ID: %w", err)
}

ids, err := pv.GenerateWinningPoStSectorChallenge(ctx, wpt, abi.ActorID(mid), rand, numProvSect)
// TODO: move this to somewhere in specs-actors or the state types.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @anorth. Will the winning proof always be paired 1:1 with the window post proof? If so, we should have a helper function to convert one to the other (well, ideally, we'd just get rid of the difference...).

The tricky part is that we're using the same type for winning and window proofs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know they will be paired 1:1, but I don't know the scope of possible futures. You need @porcuquine.

Winning PoSt has nothing to do with actors. The WinningPoSt proof types are declared there only for proximity to the other types, but arguably should be removed. It's unfortunate they use the same type, yes. We could probably make a change in Lotus in the near future to stop referencing the WinningPoSt constants altogether (by duplicating locally), opening up a path to removing them from there.

I think a mapping function in Lotus is the best immediate solution.

@arajasek
Copy link
Contributor Author

Devnet now runs past the upgrade epoch

@arajasek arajasek added the impact/consensus Impact: Consensus label Jan 19, 2021
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass, obviously some todos in the code, but generally looks good

@@ -704,6 +701,73 @@ func UpgradeCalico(ctx context.Context, sm *StateManager, cb ExecCallback, root
return newRoot, nil
}

func UpgradeActorsV3(ctx context.Context, sm *StateManager, cb ExecCallback, root cid.Cid, epoch abi.ChainEpoch, ts *types.TipSet) (cid.Cid, error) {
buf := bufbstore.NewTieredBstore(sm.cs.Blockstore(), bstore.NewTemporarySync())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to check how much memory this uses on mainnet

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the best way to test this? compute-state on the live network? Or just set a node to run the upgrade early?

chain/types/state.go Outdated Show resolved Hide resolved
@Stebalien Stebalien changed the base branch from next to master January 21, 2021 23:23
@Stebalien Stebalien changed the base branch from master to next January 21, 2021 23:23
@Stebalien Stebalien force-pushed the asr/specs-update branch 2 times, most recently from d0413f7 to 032ebdc Compare January 22, 2021 01:45
It's causing the tests to flake.
.circleci/config.yml Outdated Show resolved Hide resolved
api/test/window_post.go Show resolved Hide resolved
chain/actors/policy/policy.go Outdated Show resolved Hide resolved
node/modules/core.go Outdated Show resolved Hide resolved
@Stebalien Stebalien marked this pull request as ready for review January 22, 2021 19:18
@Stebalien Stebalien merged commit 735d30a into next Jan 22, 2021
@Stebalien Stebalien deleted the asr/specs-update branch January 22, 2021 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/consensus Impact: Consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants