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

Introduce v5 actors #6195

Merged
merged 2 commits into from
May 11, 2021
Merged

Introduce v5 actors #6195

merged 2 commits into from
May 11, 2021

Conversation

arajasek
Copy link
Contributor

@arajasek arajasek commented May 6, 2021

Actor wrappers generated by codegen make actors-gen.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

The code-gen makes this kind of thing a lot less stressful.

@@ -39,6 +39,8 @@ var UpgradeNorwegianHeight = abi.ChainEpoch(40)

var UpgradeActorsV4Height = abi.ChainEpoch(45)

var UpgradeHyperdriveHeight = abi.ChainEpoch(50)
Copy link
Member

Choose a reason for hiding this comment

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

ActorsV5? I mean, I know hyperdrive is nicer... but it's kind of inconsistent.

That or we can go back and name all the other heights. I'd actually prefer to do it that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll rename the old ones -- the inconsistency was bothering me too. I think we want to go with Hyperdrive for now (and names from this point on).

Copy link
Member

Choose a reason for hiding this comment

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

UpgradeHyperspaceBypassHeight? We can totally print some bad poetry at the time of the upgrade.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yikes is this upgrade supposed to destroy the earth?!? Idea: AlcubierreOverdrive. Maybe we save that one for the upgrade instantiating aggregator nodes.

@arajasek
Copy link
Contributor Author

arajasek commented May 7, 2021

Test failures will be resolved by filecoin-project/specs-actors#1401

Copy link
Contributor

@ZenGround0 ZenGround0 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -1053,7 +1057,7 @@ func upgradeActorsV3Common(
// Perform the migration
newHamtRoot, err := nv10.MigrateStateTree(ctx, store, stateRoot.Actors, epoch, config, migrationLogger{}, cache)
if err != nil {
return cid.Undef, xerrors.Errorf("upgrading to actors v2: %w", err)
return cid.Undef, xerrors.Errorf("upgrading to actors v3: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice find

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha, thanks!

@arajasek
Copy link
Contributor Author

I think the failures are flaky at this point. I'm going to merge it into the staging branch for now, but we will need to gain confidence in this.

@arajasek arajasek closed this May 11, 2021
@arajasek arajasek reopened this May 11, 2021
@arajasek arajasek merged commit b19d6cd into feat/nv13 May 11, 2021
@arajasek arajasek deleted the asr/v5-skeleton branch May 11, 2021 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants