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

feat: CheckMetadataHash extension #1865

Merged
merged 87 commits into from
Jun 27, 2024
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 May 9, 2024
07bb334
upgrade libs/*
lemunozm May 10, 2024
446439a
upgrade pallet-anchors
lemunozm May 10, 2024
52d1b85
upgrade pallet-fees
lemunozm May 10, 2024
7183815
upgrade pallet-bridge
lemunozm May 13, 2024
3374bac
migrate simple pallets
lemunozm May 13, 2024
b161119
migrate pallet-order-book
lemunozm May 13, 2024
9fc71f5
migrated collator-allowlist & swaps
lemunozm May 13, 2024
f302388
upgrade rewards
lemunozm May 13, 2024
a38bfb6
upgraded pallet-mux
lemunozm May 13, 2024
5ccb3d0
upgrade transfer-allowlist
lemunozm May 13, 2024
056d942
Merge remote-tracking branch 'origin/main' into polkadot-v1.7.2
lemunozm May 13, 2024
5940897
fix hold reason in restricted tokens
lemunozm May 13, 2024
7d3335f
simplify set_balance removing the holding part
lemunozm May 13, 2024
3c2bb34
upgrade restricted-xtokens
lemunozm May 13, 2024
da56ef2
upgrade some pallets
lemunozm May 13, 2024
8ad0451
upgrade pallet-ethereum-transaction
lemunozm May 13, 2024
8461f41
upgrade pallet-loans
lemunozm May 13, 2024
873507d
upgrade pool-system
lemunozm May 14, 2024
3ae8b03
upgrade pool-fees
lemunozm May 14, 2024
4a2b98b
upgrade pool-registry
lemunozm May 14, 2024
3f94ea2
upgrade liquidity-pools stuffs
lemunozm May 14, 2024
45ca1c4
avoid duplicated polkadot-sdk repos
lemunozm May 16, 2024
45b3f11
minor fixes
lemunozm May 16, 2024
a783c9f
upgraded runtime-common
lemunozm May 17, 2024
1f70efe
CfgLocation to RestrictedTransferLocation
lemunozm May 17, 2024
f713417
restricted tokens with NativeIndex for native fungibles
lemunozm May 20, 2024
9f12a65
rename dependency
lemunozm May 20, 2024
f84ac93
upgraded development-runtime
lemunozm May 21, 2024
769afc5
fix partially benchmarks
lemunozm May 21, 2024
5f5f7b6
fix benchmarks
lemunozm May 22, 2024
aaceaf9
overpass xcmp-queue integrity tests
lemunozm May 22, 2024
1a23394
minor coments
lemunozm May 22, 2024
3a0e1cd
upgrade altair & centrifuge
lemunozm May 23, 2024
a259be5
remove some benchmarking pallets that are not needed
lemunozm May 23, 2024
c9729ef
fix runtime upgrades
lemunozm May 23, 2024
65b32a1
upgrade integration-test proc
lemunozm May 24, 2024
ed5b888
upgrade integration-tests framework
lemunozm May 27, 2024
493a602
upgraded all tests except liquidity pools
lemunozm May 27, 2024
55d28ee
99% upgraded liquidity-pools tests
lemunozm May 27, 2024
f172b1d
fix lookup
lemunozm May 28, 2024
b886c46
Merge remote-tracking branch 'origin/main' into polkadot-v1.7.2
lemunozm May 28, 2024
9c27823
cargo fmt
lemunozm May 28, 2024
55bfab5
taplo fmt
lemunozm May 28, 2024
75ae4a4
using nightly cargo in CI
lemunozm May 28, 2024
38377a9
restore set_balance as it was
lemunozm May 28, 2024
8071ffe
allow nightly support in CI
lemunozm May 28, 2024
338a573
use restricted_tokens again to fetch from investement portfolio
lemunozm May 28, 2024
8c92688
Install rust-src for docs
lemunozm May 28, 2024
625f9d2
fix tests
lemunozm May 28, 2024
1bd859b
remove unused restricted tokens
lemunozm May 28, 2024
a304d7f
fix block rewards
lemunozm May 28, 2024
99b4abc
fix WrongWitness for some tests in IT
lemunozm May 29, 2024
17a180c
fix liquidity-pools
lemunozm May 29, 2024
aa40ed0
minor fixes
lemunozm May 29, 2024
7b1b7e2
fix clippy
lemunozm May 30, 2024
a5dbd56
remove unneeded tests
lemunozm May 30, 2024
bc17d18
feat: Update client to Polkadot v1.7.2 (#1844)
wischli May 30, 2024
a95459b
cargo fmt
lemunozm May 30, 2024
3e44ef9
fix clippy
lemunozm May 30, 2024
f83d95f
feat: Polkadot v1.7.2 migrations (#1849)
wischli May 31, 2024
fb52bc4
last William iteration review
lemunozm Jun 3, 2024
988fcbb
increase passed blocks
lemunozm Jun 5, 2024
8410d16
use rococo instead of polkadot-test-runtime
lemunozm Jun 5, 2024
db844a5
fix tests
lemunozm Jun 5, 2024
beb13f0
remove line
lemunozm Jun 5, 2024
7e69e68
dirty fix to fix Hrmp test issue
lemunozm Jun 6, 2024
1c96ae1
use default weights for treasury
lemunozm Jun 6, 2024
7edbc3a
remove getrandom unused dep
lemunozm Jun 6, 2024
e1bad5f
upgrade to last polkadot-sdk version
lemunozm Jun 6, 2024
05efc33
feat: `CheckMetadataHash` extension
wischli Jun 6, 2024
4be5b03
fix it (#1866)
lemunozm Jun 7, 2024
c2b8ece
fmt: taplo
wischli Jun 7, 2024
22d0a38
refactor: signed ext order
wischli Jun 7, 2024
4f7d77e
fix: signed ext order for ITs
wischli Jun 7, 2024
b0a7d0f
Merge remote-tracking branch 'origin/main' into wf/polkadot-v1.7.2-me…
wischli Jun 25, 2024
1cc426c
IT: more support for router tests (#1885)
lemunozm Jun 25, 2024
9774844
Merge remote-tracking branch 'origin/main' into wf/polkadot-v1.7.2-me…
wischli Jun 25, 2024
c037d29
Merge remote-tracking branch 'origin/release-v0.11.1' into wf/polkado…
wischli Jun 25, 2024
265e509
v0.11.2 rc
wischli Jun 25, 2024
cbf8691
panic if event is not found in the expected blocks (#1880)
lemunozm Jun 25, 2024
2e8e44c
fix: ci
wischli Jun 25, 2024
e5c8926
remove generic module (#1887)
lemunozm Jun 26, 2024
7445232
Merge remote-tracking branch 'origin/main' into wf/polkadot-v1.7.2-me…
wischli Jun 26, 2024
8de5ae7
revert check metadata hash disable
wischli Jun 26, 2024
705c950
Merge remote-tracking branch 'origin/release-v0.11.1' into wf/polkado…
wischli Jun 27, 2024
37d6e90
fix: disable metadata hash check for integration tests
wischli Jun 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion runtime/integration-tests/src/generic/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ pub mod utils {
frame_system::CheckNonce::<T>::from(nonce),
frame_system::CheckWeight::<T>::new(),
pallet_transaction_payment::ChargeTransactionPayment::<T>::from(0),
frame_metadata_hash_extension::CheckMetadataHash::<T>::new(true),
frame_metadata_hash_extension::CheckMetadataHash::<T>::new(false),
Copy link
Contributor Author

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/26661355826

I am not fully sure why. Based on the build.rs, it is only enabled for --features=std,metadata-hash:

#[cfg(all(feature = "std", feature = "metadata-hash"))]
fn main() {
substrate_wasm_builder::WasmBuilder::init_with_defaults()
.enable_metadata_hash("AIR", 18)
.build();
}

Moreover, it is enabled for on-chain-release-build:

# A feature that should be enabled when the runtime should be build for on-chain
# deployment. This will disable stuff that shouldn't be part of the on-chain wasm
# to make it smaller like logging for example.
on-chain-release-build = [
"sp-api/disable-logging",
"runtime-common/on-chain-release-build",
"metadata-hash",
]

I tried both running integration tests with -F metadata-hash as well as enabling it for the runtimes in the integration-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).

Copy link
Contributor

Choose a reason for hiding this comment

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

Checking this...

Copy link
Contributor

Choose a reason for hiding this comment

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

tried both running integration tests with -F metadata-hash as well as enabling it for the runtimes in the integration-tests/Cargo.toml but ran into polkadot-sdk compiler issues

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

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 also think it is some missing feature for a polkadot crate. Tried out a lot and still haven't surpassed the error

error[E0433]: failed to resolve: could not find `metadata_ir` in `__private`
   --> /Users/william/.cargo/git/checkouts/polkadot-sdk-cff69157b985ed76/c00ca35/substrate/primitives/api/src/lib.rs:797:1
    |
797 | / decl_runtime_apis! {
798 | |     /// The `Core` runtime api that every Substrate runtime needs to implement.
799 | |     #[core_trait]
800 | |     #[api_version(4)]
...   |
827 | |     }
828 | | }
    | |_^ could not find `metadata_ir` in `__private`

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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() or submit_later(), because those calls sign the transaction. Other calls just mutate the state

Copy link
Contributor

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 for sp-api. But even if you try adding it, the error still happens 🤔. It seems like who add sp-api first in the cargo tree should add the frame-metadata to it.

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 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.

Copy link
Contributor

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. Include metadata_ir always. The only reason to add the feature is to avoid the double wasm compilation. But here, it could be avoided.

runtime_common::transfer_filter::PreBalanceTransferExtension::<T>::new(),
);

Expand Down