polkadot-sdk
polkadot-sdk copied to clipboard
Update `RuntimeVerison` type and use `system_version` to derive extrinsics root `StateVersion` instead of `V0`
This PR
- Renames
RuntimeVersion::state_versiontosystem_version - Uses
Runtime::system_versionto derive extrinsics rootStateVersioninstead of defaultStateVersion::V0
This PR should not be breaking any existing chains so long as they use same RuntimeVersion::state_version for Runtime::system_version
Using RuntimeVersion::system_version = 2 will make the extrinsics root to use StateVersion::V1 instead of V0
RFC for this change - https://github.com/polkadot-fellows/RFCs/pull/42
@bkchr I had looked through the failed tests and not sure if the failures are related to changes in this PR. Can you confirm? If not where should I look at to fix these tests ?
Hey @koute @bkchr would appreciate some thoughts on the failing issues. Also, any chance you can do a first pass/review this PR ?
Hey @bkchr @koute Would be really great to this PR in. I will fix the conflicts in sometime but please have a look so that this is landed
@bkchr @koute bumping this again
@bkchr @koute bumping this again
@bkchr @koute bumping this again :)
@vedhavyas sorry for the delay. Can you please fix the CI issues and merge master?
@bkchr yes will do. Thank you!
@bkchr seems like CI is stuck. Is there an way to restart it ?
seems like CI is stuck. Is there an way to restart it ?
Done.
@koute any idea why some tests failed. Looking at the CI logs of each failed test does not give much info. Any pointers pls ?
I think the zombienet tests are just flaky. @vedhavyas please give us merge rights to your branch, then we can also merge master for example
@vedhavyas please give us merge rights to your branch, then we can also merge master for example
I don't think that'll work because PR was created from an organization fork, not from a personal one.
I just merged master in though.
Merged master again, hoping to push this across the finish line soon :crossed_fingers:
@koute @bkchr how can I get you interested in reviewing this before cutting the next stable release?
@koute how can I get you interested in reviewing this before cutting the next stable release?
Make it backwards compatible (have it emit a deprecation warning when the old field name is used) and you'll have a :+1: from me.
What @koute is referring to is, this discussion: https://github.com/paritytech/polkadot-sdk/pull/4257#discussion_r1690053835
@bkchr @koute Should be updated now. Can you take a look if I missed anything ?
@bkchr @koute are we good to go here?
Not sure what to do with zombienet checks (not clear what is wrong from CI logs alone), but the rest of the CI should be fixed assuming I didn't miss anything in prdoc
The zombienet errors seem to be valid:
tokio-runtime-worker cumulus-pov-recovery: [Parachain] Failed to fetch relay chain runtime version. error=RpcCallError("state_getRuntimeVersion")
The bridges zombienet errors are maybe also related. Probably PJS tries to fetch the RuntimeVersion and it not having the state_version field is maybe the problem.
I have added stateVersion alias to systemVersion field, which should fix the decoding issue. If desirable customized serialization can be implemented to produce both stateVersion and systemVersion in the serialized JSON for smoother migration process.
If desirable customized serialization can be implemented to produce both
stateVersionandsystemVersionin the serialized JSON for smoother migration process.
Yeah I also thought about this.
So do you want me to do that?
So do you want me to do that?
Yeah please do it. Can you also try to merge master again? I hope that this fixes the bridges tests.
Had to introduce custom deserialization as well in order to allow "duplicated" field and make serialization/deserialization symmetrical
The CI pipeline was cancelled due to failure one of the required jobs. Job name: test-linux-stable-int Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7190974
If there are no blockers here, I'd really appreciate finally merging this and be done with it :pray:
@koute needs to approve :P
@nazar-pc we need a master merge again :see_no_evil: