Skip to content

Commit

Permalink
Check metadata hash (#776)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
Niederb and masapr authored Jul 4, 2024
1 parent 2a4209e commit ece488c
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 17 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion examples/async/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
2 changes: 1 addition & 1 deletion examples/sync/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
1 change: 1 addition & 0 deletions primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
53 changes: 42 additions & 11 deletions primitives/src/extrinsics/extrinsic_params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -38,11 +40,20 @@ pub struct GenericSignedExtra<Tip, Index> {
#[codec(compact)]
pub nonce: Index,
pub tip: Tip,
#[cfg(feature = "disable-metadata-hash-check")]
pub check_hash: u8,
}

impl<Tip, Index> GenericSignedExtra<Tip, Index> {
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 }
}
}
}

Expand All @@ -55,6 +66,9 @@ impl<Tip, Index> GenericSignedExtra<Tip, Index> {
// 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<Hash> = ((), u32, u32, Hash, Hash, (), (), (), Option<H256>);
#[cfg(not(feature = "disable-metadata-hash-check"))]
pub type GenericAdditionalSigned<Hash> = ((), u32, u32, Hash, Hash, (), (), ());

/// This trait allows you to configure the "signed extra" and
Expand Down Expand Up @@ -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,
(),
(),
(),
)
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions primitives/src/extrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion testing/async/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
2 changes: 1 addition & 1 deletion testing/async/examples/state_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion testing/sync/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }

0 comments on commit ece488c

Please sign in to comment.