From ece488c99b9a1eb3b6ed7034361bc381bc45bbce Mon Sep 17 00:00:00 2001 From: Thomas Niederberger <781000+Niederb@users.noreply.github.com> Date: Thu, 4 Jul 2024 12:52:15 +0200 Subject: [PATCH] Check metadata hash (#776) * Activate metadata-hash feature in kitchensink-runtime to see if we can reproduce the signing issues * Test with a newer docker image * Test a docker image that should still work * Implement empty hash and deactivate metadata hash check * Fix failing build. - Deactivate two tests for now * Cleanup/Remove old changes * Introduce feature to disable the metadata-hash-check * Fix conditional compilation * - Disable tests when the metadata-hash-check feature is active - Add feature to the main Cargo.toml * Test with newer docker image * Add feature to testing code * Make taplo fmt happy * Activate the metadata-hash feature per default in testing code * Use H256 instead of byte array * Make clippy happy by adding more clutter to the code :-( --------- Co-authored-by: masapr <40862722+masapr@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- Cargo.toml | 3 ++ examples/async/Cargo.toml | 2 +- examples/sync/Cargo.toml | 2 +- primitives/Cargo.toml | 1 + primitives/src/extrinsics/extrinsic_params.rs | 53 +++++++++++++++---- primitives/src/extrinsics/mod.rs | 4 ++ testing/async/Cargo.toml | 2 +- testing/async/examples/state_tests.rs | 2 +- testing/sync/Cargo.toml | 2 +- 10 files changed, 56 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 55d4f55b6..cdfaab229 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -225,7 +225,7 @@ jobs: - name: Run latest node run: | - docker run -p 9944:9944 -p 9933:9933 -p 30333:30333 paritypr/substrate:master-1680977e --dev --rpc-external & + docker run -p 9944:9944 -p 9933:9933 -p 30333:30333 paritypr/substrate:master-18228a9b --dev --rpc-external & - name: Wait until node has started run: sleep 20s diff --git a/Cargo.toml b/Cargo.toml index 6078ee20c..15d95b380 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -105,6 +105,9 @@ staking-xt = ["std", "ac-primitives/staking-xt"] # of the functionality this feature provides. contracts-xt = ["std", "ac-primitives/contracts-xt"] +# Provides compatibility to RFC-0078: "Merkelized Metadata" but disables the check of the metadata hash +disable-metadata-hash-check = ["ac-primitives/disable-metadata-hash-check"] + # Enables all std features of dependencies in case of std build. std = [ # crates.io no_std diff --git a/examples/async/Cargo.toml b/examples/async/Cargo.toml index 04626bdd3..f0aa010f0 100644 --- a/examples/async/Cargo.toml +++ b/examples/async/Cargo.toml @@ -26,4 +26,4 @@ sp-runtime = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = sp-weights = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "master" } # local deps -substrate-api-client = { path = "../..", version = "0.17", features = ["staking-xt", "contracts-xt"] } +substrate-api-client = { path = "../..", version = "0.17", features = ["staking-xt", "contracts-xt", "disable-metadata-hash-check"] } diff --git a/examples/sync/Cargo.toml b/examples/sync/Cargo.toml index 7e6b0adf0..e2afed9cb 100644 --- a/examples/sync/Cargo.toml +++ b/examples/sync/Cargo.toml @@ -15,4 +15,4 @@ sp-runtime = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = sp-weights = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "master" } # local deps -substrate-api-client = { path = "../..", version = "0.17", default-features = false, features = ["tungstenite-client", "ws-client"] } +substrate-api-client = { path = "../..", version = "0.17", default-features = false, features = ["tungstenite-client", "ws-client", "disable-metadata-hash-check"] } diff --git a/primitives/Cargo.toml b/primitives/Cargo.toml index d602fa548..d59d9c2df 100644 --- a/primitives/Cargo.toml +++ b/primitives/Cargo.toml @@ -47,6 +47,7 @@ default = ["std"] disable_target_static_assertions = [ "sp-runtime-interface/disable_target_static_assertions", ] +disable-metadata-hash-check = [] std = [ "codec/std", "primitive-types/std", diff --git a/primitives/src/extrinsics/extrinsic_params.rs b/primitives/src/extrinsics/extrinsic_params.rs index b135f2745..6dc23e1b7 100644 --- a/primitives/src/extrinsics/extrinsic_params.rs +++ b/primitives/src/extrinsics/extrinsic_params.rs @@ -17,6 +17,8 @@ use crate::config::Config; use codec::{Decode, Encode}; +#[cfg(feature = "disable-metadata-hash-check")] +use primitive_types::H256; use sp_runtime::{ generic::Era, traits::{BlakeTwo256, Hash}, @@ -38,11 +40,20 @@ pub struct GenericSignedExtra { #[codec(compact)] pub nonce: Index, pub tip: Tip, + #[cfg(feature = "disable-metadata-hash-check")] + pub check_hash: u8, } impl GenericSignedExtra { pub fn new(era: Era, nonce: Index, tip: Tip) -> Self { - Self { era, nonce, tip } + #[cfg(feature = "disable-metadata-hash-check")] + { + Self { era, nonce, tip, check_hash: 0 } + } + #[cfg(not(feature = "disable-metadata-hash-check"))] + { + Self { era, nonce, tip } + } } } @@ -55,6 +66,9 @@ impl GenericSignedExtra { // defines what is returned upon the `additional_signed` call. The AdditionalSigned defined here // must mirror these return values. // Example: https://github.com/paritytech/substrate/blob/23bb5a6255bbcd7ce2999044710428bc4a7a924f/frame/system/src/extensions/check_non_zero_sender.rs#L64-L66 +#[cfg(feature = "disable-metadata-hash-check")] +pub type GenericAdditionalSigned = ((), u32, u32, Hash, Hash, (), (), (), Option); +#[cfg(not(feature = "disable-metadata-hash-check"))] pub type GenericAdditionalSigned = ((), u32, u32, Hash, Hash, (), (), ()); /// This trait allows you to configure the "signed extra" and @@ -179,16 +193,33 @@ where } fn additional_signed(&self) -> Self::AdditionalSigned { - ( - (), - self.spec_version, - self.transaction_version, - self.genesis_hash, - self.mortality_checkpoint, - (), - (), - (), - ) + #[cfg(feature = "disable-metadata-hash-check")] + { + ( + (), + self.spec_version, + self.transaction_version, + self.genesis_hash, + self.mortality_checkpoint, + (), + (), + (), + None, + ) + } + #[cfg(not(feature = "disable-metadata-hash-check"))] + { + ( + (), + self.spec_version, + self.transaction_version, + self.genesis_hash, + self.mortality_checkpoint, + (), + (), + (), + ) + } } } diff --git a/primitives/src/extrinsics/mod.rs b/primitives/src/extrinsics/mod.rs index 2015aafa2..897bbc7ec 100644 --- a/primitives/src/extrinsics/mod.rs +++ b/primitives/src/extrinsics/mod.rs @@ -325,6 +325,8 @@ mod tests { assert_eq!(call, call1); } + // Currently does not with available version of substrate extrinsic + #[cfg(not(feature = "disable-metadata-hash-check"))] #[test] fn xt_hash_matches_substrate_impl() { // Define extrinsic params. @@ -384,6 +386,8 @@ mod tests { ) } + // Currently does not work with stored bytes. Need to create a new version + #[cfg(not(feature = "disable-metadata-hash-check"))] #[test] fn xt_hash_matches_substrate_impl_large_xt() { // Define xt parameters, diff --git a/testing/async/Cargo.toml b/testing/async/Cargo.toml index 304dd2a75..2e3f52fec 100644 --- a/testing/async/Cargo.toml +++ b/testing/async/Cargo.toml @@ -23,4 +23,4 @@ pallet-balances = { git = "https://github.com/paritytech/polkadot-sdk.git", bran pallet-staking = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "master" } # local deps -substrate-api-client = { path = "../..", version = "0.17", features = ["staking-xt", "contracts-xt"] } +substrate-api-client = { path = "../..", version = "0.17", features = ["staking-xt", "contracts-xt", "disable-metadata-hash-check"] } diff --git a/testing/async/examples/state_tests.rs b/testing/async/examples/state_tests.rs index 98d287941..ef5fa8de0 100644 --- a/testing/async/examples/state_tests.rs +++ b/testing/async/examples/state_tests.rs @@ -127,7 +127,7 @@ async fn main() { .get_storage_keys_paged(Some(storage_key_prefix), max_keys, None, None) .await .unwrap(); - assert_eq!(storage_keys.len() as u32, 13); + assert_eq!(storage_keys.len() as u32, 14); let max_keys = 20; let storage_keys = api diff --git a/testing/sync/Cargo.toml b/testing/sync/Cargo.toml index 34c40cd8d..97b994ae4 100644 --- a/testing/sync/Cargo.toml +++ b/testing/sync/Cargo.toml @@ -12,5 +12,5 @@ sp-core = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "ma sp-runtime = { git = "https://github.com/paritytech/polkadot-sdk.git", branch = "master" } # local deps -substrate-api-client = { path = "../..", version = "0.17", default-features = false, features = ["tungstenite-client", "ws-client"] } +substrate-api-client = { path = "../..", version = "0.17", default-features = false, features = ["tungstenite-client", "ws-client", "disable-metadata-hash-check"] } ac-keystore = { path = "../../keystore" }