-
Notifications
You must be signed in to change notification settings - Fork 91
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
Polkadot v0.9.38 #1332
Polkadot v0.9.38 #1332
Conversation
@NunoAlexandre I can not reproduce the integration test failures locally. Could you try to? |
That's interesting, because I can indeed. What's your rust toolchain? It should be picking it up from the toolchain file but still worth checking! |
@NunoAlexandre @mustermeiszer I managed to reproduce some failures locally. Currently looking into them and will update as soon as I have more info. |
That's awesome, thanks! Since I often bump against those errors when updating our chain to a newer Polkadot version, I would very much appreciate if you could educate me on how to fix them so I can move that forward in the future, shall that happen again 👍 |
After doing some investigation into the test errors we figured out that the original error came from genesis builder, where a decoding error would happen when building the wasm, more info here - https://substrate.stackexchange.com/questions/7881/error-loading-of-original-wasm-failed Tested locally with |
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.
Looks really good! I appreciate the extra cleanup as well, such as the derive for default structs, switching from into to from impls, and superfluous var removals :).
8126e4d
to
f5505a5
Compare
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.
LGTM
@@ -2,7 +2,7 @@ | |||
|
|||
set -eux | |||
|
|||
RUST_TOOLCHAIN="${RUST_TOOLCHAIN:-nightly-2022-11-14}" | |||
RUST_TOOLCHAIN="${RUST_TOOLCHAIN:-nightly-2023-02-07}" |
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.
Do we know on which one parity is running polkadot and substrate?
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.
No, I just picked this since is the one for rust v1.69
. I'll double-check what parity is using for their v0.9.38
polkadot/substrate.
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.
I don't think parity is using any specific versions given:
https://github.com/paritytech/substrate/blob/polkadot-v0.9.38/.maintain/init.sh#L8
I also checked their CI scripts and it seems that they are using whatever is stable/nightly when they build, however, someone might know more. Maybe @wischli can help? ^^
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.
Parity doesn't use a nightly (I believe we only need it for fudge these days?), so I wouldn't be surprised if they just track stable.
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.
The releases in the Polkadot repository expose minimal compatible Rust versions For v0.9.38 that is
- Rust Stable: rustc 1.66.1 (90743e729 2023-01-10)
- Rust Nightly: rustc 1.67.0-nightly (a00f8ba7f 2022-11-15)
For the latest release v0.9.42 we are on
- Rust Stable: rustc 1.68.2 (9eb3afe9e 2023-03-27)
- Rust Nightly: rustc 1.67.0-nightly (a00f8ba7f 2022-11-15)
For their CI, they seem to follow latest stable as stated by @branan, here's an excerpt:
[2023-05-30 22:10:38] $ cargo --version
[2023-05-30 22:10:38] cargo 1.69.0 (6e9a83356 2023-04-12)
However, in their Github actions, they are using hongfuzz fuzzer with latest minimal nightly.
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.
Parity doesn't use a nightly (I believe we only need it for fudge these days?), so I wouldn't be surprised if they just track stable.
Where does this assumption come from? We could remove the necessity in fudge then.
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.
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.
All looking pretty sane to me. Seems like not too many code changes for this one (yay)
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.
Great additions since my last review. Really happy about the newer rust version and improved String formatting 🥳
Thanks for handling all recent Polkadot updates. I know how tedious that can be 🫶
"The function is registered but the type mismatches. Expected {}, found: {}", | ||
expected, found |
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.
Saving code lines with the new rust feature 🤯
Thank you all so much ❤️ auto-merge is enabled 🚀 |
The update to Polkadot 0.9.38 emcompasses two major changes in the framework:
None of those changes ended up having a substantial impact in the codebase. Regarding XCM, I took the opportunity to simplify the Xcm config around
CurrencyIdConvert
by using theOrmlAssetRegistry
for all the conversions, now that 1) we have CFG and AIR registered in Centrifuge's and Altair's registries (respectively) and 2) since thelocation
of Tranche tokens is set toNone
and can't therefore be handled through XCM.To Do
ignored
for now