-
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
feat: CheckMetadataHash
extension
#1865
Merged
Merged
Changes from 1 commit
Commits
Show all changes
87 commits
Select commit
Hold shift + click to select a range
d6e0bf4
set dependency versions
lemunozm 07bb334
upgrade libs/*
lemunozm 446439a
upgrade pallet-anchors
lemunozm 52d1b85
upgrade pallet-fees
lemunozm 7183815
upgrade pallet-bridge
lemunozm 3374bac
migrate simple pallets
lemunozm b161119
migrate pallet-order-book
lemunozm 9fc71f5
migrated collator-allowlist & swaps
lemunozm f302388
upgrade rewards
lemunozm a38bfb6
upgraded pallet-mux
lemunozm 5ccb3d0
upgrade transfer-allowlist
lemunozm 056d942
Merge remote-tracking branch 'origin/main' into polkadot-v1.7.2
lemunozm 5940897
fix hold reason in restricted tokens
lemunozm 7d3335f
simplify set_balance removing the holding part
lemunozm 3c2bb34
upgrade restricted-xtokens
lemunozm da56ef2
upgrade some pallets
lemunozm 8ad0451
upgrade pallet-ethereum-transaction
lemunozm 8461f41
upgrade pallet-loans
lemunozm 873507d
upgrade pool-system
lemunozm 3ae8b03
upgrade pool-fees
lemunozm 4a2b98b
upgrade pool-registry
lemunozm 3f94ea2
upgrade liquidity-pools stuffs
lemunozm 45ca1c4
avoid duplicated polkadot-sdk repos
lemunozm 45b3f11
minor fixes
lemunozm a783c9f
upgraded runtime-common
lemunozm 1f70efe
CfgLocation to RestrictedTransferLocation
lemunozm f713417
restricted tokens with NativeIndex for native fungibles
lemunozm 9f12a65
rename dependency
lemunozm f84ac93
upgraded development-runtime
lemunozm 769afc5
fix partially benchmarks
lemunozm 5f5f7b6
fix benchmarks
lemunozm aaceaf9
overpass xcmp-queue integrity tests
lemunozm 1a23394
minor coments
lemunozm 3a0e1cd
upgrade altair & centrifuge
lemunozm a259be5
remove some benchmarking pallets that are not needed
lemunozm c9729ef
fix runtime upgrades
lemunozm 65b32a1
upgrade integration-test proc
lemunozm ed5b888
upgrade integration-tests framework
lemunozm 493a602
upgraded all tests except liquidity pools
lemunozm 55d28ee
99% upgraded liquidity-pools tests
lemunozm f172b1d
fix lookup
lemunozm b886c46
Merge remote-tracking branch 'origin/main' into polkadot-v1.7.2
lemunozm 9c27823
cargo fmt
lemunozm 55bfab5
taplo fmt
lemunozm 75ae4a4
using nightly cargo in CI
lemunozm 38377a9
restore set_balance as it was
lemunozm 8071ffe
allow nightly support in CI
lemunozm 338a573
use restricted_tokens again to fetch from investement portfolio
lemunozm 8c92688
Install rust-src for docs
lemunozm 625f9d2
fix tests
lemunozm 1bd859b
remove unused restricted tokens
lemunozm a304d7f
fix block rewards
lemunozm 99b4abc
fix WrongWitness for some tests in IT
lemunozm 17a180c
fix liquidity-pools
lemunozm aa40ed0
minor fixes
lemunozm 7b1b7e2
fix clippy
lemunozm a5dbd56
remove unneeded tests
lemunozm bc17d18
feat: Update client to Polkadot v1.7.2 (#1844)
wischli a95459b
cargo fmt
lemunozm 3e44ef9
fix clippy
lemunozm f83d95f
feat: Polkadot v1.7.2 migrations (#1849)
wischli fb52bc4
last William iteration review
lemunozm 988fcbb
increase passed blocks
lemunozm 8410d16
use rococo instead of polkadot-test-runtime
lemunozm db844a5
fix tests
lemunozm beb13f0
remove line
lemunozm 7e69e68
dirty fix to fix Hrmp test issue
lemunozm 1c96ae1
use default weights for treasury
lemunozm 7edbc3a
remove getrandom unused dep
lemunozm e1bad5f
upgrade to last polkadot-sdk version
lemunozm 05efc33
feat: `CheckMetadataHash` extension
wischli 4be5b03
fix it (#1866)
lemunozm c2b8ece
fmt: taplo
wischli 22d0a38
refactor: signed ext order
wischli 4f7d77e
fix: signed ext order for ITs
wischli b0a7d0f
Merge remote-tracking branch 'origin/main' into wf/polkadot-v1.7.2-me…
wischli 1cc426c
IT: more support for router tests (#1885)
lemunozm 9774844
Merge remote-tracking branch 'origin/main' into wf/polkadot-v1.7.2-me…
wischli c037d29
Merge remote-tracking branch 'origin/release-v0.11.1' into wf/polkado…
wischli 265e509
v0.11.2 rc
wischli cbf8691
panic if event is not found in the expected blocks (#1880)
lemunozm 2e8e44c
fix: ci
wischli e5c8926
remove generic module (#1887)
lemunozm 7445232
Merge remote-tracking branch 'origin/main' into wf/polkadot-v1.7.2-me…
wischli 8de5ae7
revert check metadata hash disable
wischli 705c950
Merge remote-tracking branch 'origin/release-v0.11.1' into wf/polkado…
wischli 37d6e90
fix: disable metadata hash check for integration tests
wischli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@lemunozm Unfortunately, multiple of our integration tests fail with
CheckMetadataHash
enabled: https://github.com/centrifuge/centrifuge-chain/actions/runs/9665090255/job/26661355826I am not fully sure why. Based on the
build.rs
, it is only enabled for--features=std,metadata-hash
:centrifuge-chain/runtime/altair/build.rs
Lines 1 to 6 in 2e8e44c
Moreover, it is enabled for
on-chain-release-build
:centrifuge-chain/runtime/development/Cargo.toml
Lines 450 to 457 in 2e8e44c
I tried both running integration tests with
-F metadata-hash
as well as enabling it for the runtimes in theintegration-tests/Cargo.toml
but ran into polkadot-sdk compiler issues. So it appeared to be the incorrect approach. Upon further investigating the SDK repo, I saw all their integration tests disable the metadata hash. However, this might be due to performance reasons. Apparently, the WASM needs to be built twice if the feature is enabled (see guide).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.
Checking this...
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 think this is the correct one. Nevertheless, when adding the feature, could be another missing crate from Polkadot that also requires that feature, then you get those polkadot errors. I'm going to try to overpass this
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.
Yeah I also think it is some missing feature for a polkadot crate. Tried out a lot and still haven't surpassed the error
It can be quickly checked by trying to build any runtime with
on-chain-release
feature. In the Polkadot SDK this works for the template runtime. For us, it doesn't for any runtime.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 also don't understand why only ~20% of the integration tests fail. I couldn't detect a pattern yet as the errors occur for all runtimes as well as fudge and non-fudge envs.
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.
Fails only the test who makes a call to
submit_now()
orsubmit_later()
, because those calls sign the transaction. Other calls just mutate the stateThere 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'm also stuck in the above error. Quite weird. It seems like
frame-metadata
feature is missing forsp-api
. But even if you try adding it, the error still happens 🤔. It seems like who addsp-api
first in thecargo tree
should add theframe-metadata
to it.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.
Yeah I have been there as well. Good to know I havent missed anything obvious and sad this doesnt seem to work right now for us. Will probably open an issue in the SDK 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.
Yeah, Although I think this is more a "rust" problem. It seems unmaintenable that each time you add a feature to a crate, you "need" to propagate that feature to the crates that use the featured gated crate. Ideally, you need to add it even to those crates that don't require that feature. Just for the case that a possible user want to enable it.
Maybe the solution here from polkadot would be to just remove the
frame-metadata
feature. Includemetadata_ir
always. The only reason to add the feature is to avoid the double wasm compilation. But here, it could be avoided.