-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: add Network Version 27 skeleton #13125
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
Conversation
@TippyFlitsUK : I marked this as ready for review so we can get Copilot review comments. Let me know if it's not ready to be looked at. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces the Network Version 27 skeleton by bumping dependencies from v16 to v17, updating upgrade schedules, and modifying API documentation and tests accordingly.
- Updated various import paths and constant values to use v17 versions.
- Added new upgrade parameters (e.g. UpgradeXxHeight) and updated openrpc example numbers to 27.
Reviewed Changes
Copilot reviewed 65 out of 69 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
storage/pipeline/mocks/mocks.go | Updated miner import from v16 to v17. |
node/impl/full/state.go | Added UpgradeXxHeight to the network parameters. |
itests/kit/ensemble_opts_nv.go | Updated network version and migration parameters to v27. |
go.mod | Bumped filecoin-ffi and go-state-types dependency versions. |
(Many chain/actors files) | Updated actor versions and module references to v17 where needed. |
API documentation files | Replaced number examples 25 with 27. |
build/buildconstants/*.go | Adjusted upgrade height constants, including new UpgradeXxHeight. |
cmd/lotus-shed/migrations.go | Added new migration methods for v17. |
Did a manual test run in a local devnet: Devnet starts from network version 26:
Seeing a pre-migration:
Seeing the actual migratioN:
And that the chain is progressing after the upgrade:
Network version also updated:
|
I have been spending quite a bit of time to trying to figure out why the lotus/itests/migration_test.go Lines 1155 to 1156 in e9c3238
But I could not get to the bottom of it - I think it is now failing because when creating the skeleton we ran @rvagg Do you have some insights here? I think maybe an option here is to rework the TestMigrationTeepTockFix-test, rather than changing the pack.sh script? |
This came from following these docs for nv27 in #13125
Well, the diagnosis of this is the check in lotus/chain/consensus/filcns/upgrades.go Lines 3075 to 3079 in e9c3238
But, your "new" v17 bundle has the same CIDs as the v16 bundle, so when we do a lotus/chain/actors/manifest.go Lines 42 to 53 in e9c3238
So any lookup that says "tell me what actors version this CID is" for a v16 CID will tell it that it's version 17. I'm having a think about what to do about this. This check is unique to TockFix and TockFix was only run on calibnet and it's passed already so it might be safe to just remove that check entirely and YOLO through the test. I'll figure it out and push something to this branch. |
As per comment inline, this is getting in the way of landing a skeleton for nv27 with an identical actors bundle so version resolution for v16 CIDs was showing 17.
e9c3238
to
f2d05a5
Compare
OK, I commented that block out and left a big inline doc about it. I also rebased and squashed this down since it's a bit old and has got a bit messy. @rjan90 you should do another review to sanity check that it's all in place but afaik this is good to go. |
Related Issues
Closes: #13073
Checklist
Before you mark the PR ready for review, please make sure that: