From 12130a76ac2b955f8ca68a53299cfddf3ff9dbae Mon Sep 17 00:00:00 2001 From: eskimor Date: Tue, 24 Oct 2023 17:51:01 +0200 Subject: [PATCH 01/13] Remove obsolete comment. (#2008) it is indeed correct. Co-authored-by: eskimor --- polkadot/runtime/parachains/src/scheduler.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/polkadot/runtime/parachains/src/scheduler.rs b/polkadot/runtime/parachains/src/scheduler.rs index f3333d5cf857..b81b68b5745e 100644 --- a/polkadot/runtime/parachains/src/scheduler.rs +++ b/polkadot/runtime/parachains/src/scheduler.rs @@ -352,7 +352,6 @@ impl Pallet { None }, Ok((pos_in_claimqueue, pe)) => { - // is this correct? availability_cores[core_idx.0 as usize] = CoreOccupied::Paras(pe); Some((*core_idx, pos_in_claimqueue)) From 4a44356773d27221c4ac9e48786d07dc72104a48 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 24 Oct 2023 17:59:38 +0200 Subject: [PATCH 02/13] Improve features dev-ex (#1831) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a config file that allows to run `zepter` without any arguments in the workspace to address all issues. A secondary workflow for the CI is provided as `zepter run check`. Both the formatting and linting are now in one check for efficiancy. The latest version also detects some more things that `featalign` was already showing. Error message [in the CI](https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3916205) now looks like this: ```pre ... crate 'test-parachains' (/Users/vados/Documents/work/polkadot-sdk/polkadot/parachain/test-parachains/Cargo.toml) feature 'std' must propagate to: parity-scale-codec Found 55 issues (run with --fix to fix). Error: Command 'lint propagate-feature' failed with exit code 1 Polkadot-SDK uses the Zepter CLI to detect abnormalities in the feature configuration. It looks like one more more checks failed; please check the console output. You can try to automatically address them by running `zepter`. Otherwise please ask directly in the Merge Request, GitHub Discussions or on Matrix Chat, thank you. For more information, see: - https://github.com/paritytech/polkadot-sdk/issues/1831 - https://github.com/ggwpez/zepter ``` TODO: - [x] Check that CI fails correctly --------- Signed-off-by: Oliver Tale-Yazdi --- .config/zepter.yaml | 45 +++++++++++++++++++ .gitlab/pipeline/check.yml | 19 ++------ .../chain-polkadot-bulletin/Cargo.toml | 1 + bridges/primitives/relayers/Cargo.toml | 2 + cumulus/pallets/parachain-system/Cargo.toml | 1 + cumulus/pallets/xcmp-queue/Cargo.toml | 1 + cumulus/parachains/common/Cargo.toml | 3 ++ .../assets/asset-hub-kusama/Cargo.toml | 1 + .../assets/asset-hub-rococo/Cargo.toml | 1 + .../assets/asset-hub-westend/Cargo.toml | 1 + .../runtimes/assets/common/Cargo.toml | 1 + .../runtimes/assets/test-utils/Cargo.toml | 1 + .../parachains/runtimes/test-utils/Cargo.toml | 1 + cumulus/primitives/timestamp/Cargo.toml | 1 + cumulus/primitives/utility/Cargo.toml | 1 + docs/STYLE_GUIDE.md | 2 +- polkadot/parachain/test-parachains/Cargo.toml | 2 +- .../test-parachains/adder/Cargo.toml | 2 +- .../test-parachains/undying/Cargo.toml | 8 +++- polkadot/runtime/parachains/Cargo.toml | 2 + polkadot/xcm/Cargo.toml | 1 + polkadot/xcm/pallet-xcm-benchmarks/Cargo.toml | 2 + polkadot/xcm/pallet-xcm/Cargo.toml | 1 + polkadot/xcm/xcm-executor/Cargo.toml | 1 + substrate/bin/node/runtime/Cargo.toml | 1 + substrate/frame/asset-conversion/Cargo.toml | 1 + substrate/frame/democracy/Cargo.toml | 1 + substrate/frame/glutton/Cargo.toml | 1 + substrate/frame/identity/Cargo.toml | 1 + substrate/frame/message-queue/Cargo.toml | 1 + substrate/frame/multisig/Cargo.toml | 1 + substrate/frame/nfts/Cargo.toml | 1 + .../nomination-pools/benchmarking/Cargo.toml | 2 + substrate/frame/referenda/Cargo.toml | 1 + substrate/frame/society/Cargo.toml | 1 + substrate/frame/statement/Cargo.toml | 1 + substrate/frame/support/Cargo.toml | 1 + .../primitives/api/proc-macro/Cargo.toml | 2 +- .../primitives/consensus/beefy/Cargo.toml | 1 + substrate/primitives/core/Cargo.toml | 2 + substrate/primitives/io/Cargo.toml | 3 +- .../merkle-mountain-range/Cargo.toml | 1 + .../runtime-interface/test-wasm/Cargo.toml | 1 + .../primitives/statement-store/Cargo.toml | 2 + .../primitives/wasm-interface/Cargo.toml | 2 +- substrate/test-utils/runtime/Cargo.toml | 2 + 46 files changed, 108 insertions(+), 23 deletions(-) create mode 100644 .config/zepter.yaml diff --git a/.config/zepter.yaml b/.config/zepter.yaml new file mode 100644 index 000000000000..2bdc106b2da9 --- /dev/null +++ b/.config/zepter.yaml @@ -0,0 +1,45 @@ +version: + format: 1 + # Minimum version of the binary that is expected to work. This is just for printing a nice error + # message when someone tries to use an older version. + binary: 0.13.2 + +# The examples in this file assume crate `A` to have a dependency on crate `B`. +workflows: + check: + - [ + 'lint', + # Check that `A` activates the features of `B`. + 'propagate-feature', + # These are the features to check: + '--features=try-runtime,runtime-benchmarks,std', + # Do not try to add a new section into `[features]` of `A` only because `B` expose that feature. There are edge-cases where this is still needed, but we can add them manually. + '--left-side-feature-missing=ignore', + # Enabling this feature somehow pulls in two versions of `sp-runtime-interface` and makes it impossible to build that crate with `cargo b -p sp-runtime-interface`. We therefore disable it for now. + '--ignore-missing-propagate=sp-core/std:bandersnatch_vrfs/std', + # Ignore the case that `A` it outside of the workspace. Otherwise it will report errors in external dependencies that we have no influence on. + '--left-side-outside-workspace=ignore', + # Some features imply that they activate a specific dependency as non-optional. Otherwise the default behaviour with a `?` is used. + '--feature-enables-dep=try-runtime:frame-try-runtime,runtime-benchmarks:frame-benchmarking', + # Actually modify the files and not just report the issues: + '--offline', + '--locked', + '--show-path', + '--quiet', + ] + # Format the features into canonical format: + - ['format', 'features', '--offline', '--locked', '--quiet'] + # Same as `check`, but with the `--fix` flag. + default: + - [ $check.0, '--fix' ] + - [ $check.1, '--fix' ] + +# Will be displayed when any workflow fails: +help: + text: | + Polkadot-SDK uses the Zepter CLI to detect abnormalities in the feature configuration. + It looks like one more more checks failed; please check the console output. You can try to automatically address them by running `zepter`. + Otherwise please ask directly in the Merge Request, GitHub Discussions or on Matrix Chat, thank you. + links: + - "https://github.com/paritytech/polkadot-sdk/issues/1831" + - "https://github.com/ggwpez/zepter" diff --git a/.gitlab/pipeline/check.yml b/.gitlab/pipeline/check.yml index 5a304d8bc4e2..bf2e7341b8bb 100644 --- a/.gitlab/pipeline/check.yml +++ b/.gitlab/pipeline/check.yml @@ -21,16 +21,6 @@ check-try-runtime: # experimental code may rely on try-runtime and vice-versa - time cargo check --locked --all --features try-runtime,experimental -cargo-fmt-manifest: - stage: check - extends: - - .docker-env - - .common-refs - script: - - cargo install zepter --locked --version 0.11.0 -q -f --no-default-features && zepter --version - - echo "👉 Hello developer! If you see this CI check failing then it means that one of the your changes in a Cargo.toml file introduced ill-formatted or unsorted features. Please take a look at 'docs/STYLE_GUIDE.md#manifest-formatting' to find out more." - - zepter format features --check - # FIXME .cargo-deny-licenses: stage: check @@ -91,17 +81,14 @@ job-starter: script: - echo ok -test-rust-feature-propagation: +check-rust-feature-propagation: stage: check extends: - .kubernetes-env - .test-pr-refs script: - - cargo install --locked --version 0.11.1 -q -f zepter && zepter --version - - echo "👉 Hello developer! If you see this CI check failing then it means that one of the crates is missing a feature for one of its dependencies. The output below tells you which feature needs to be added for which dependency to which crate. You can do this by modifying the Cargo.toml file. For more context see the MR where this check was introduced https://github.com/paritytech/substrate/pull/14660" - - zepter lint propagate-feature --feature try-runtime --left-side-feature-missing=ignore --workspace --feature-enables-dep="try-runtime:frame-try-runtime" --locked - - zepter lint propagate-feature --feature runtime-benchmarks --left-side-feature-missing=ignore --workspace --feature-enables-dep="runtime-benchmarks:frame-benchmarking" --locked - - zepter lint propagate-feature --feature std --left-side-feature-missing=ignore --workspace --locked + - cargo install --locked --version 0.13.2 -q -f zepter && zepter --version + - zepter run check # More info can be found here: https://github.com/paritytech/polkadot/pull/5865 .check-runtime-migration: diff --git a/bridges/primitives/chain-polkadot-bulletin/Cargo.toml b/bridges/primitives/chain-polkadot-bulletin/Cargo.toml index d748f5aa933c..9cc79bef1a6f 100644 --- a/bridges/primitives/chain-polkadot-bulletin/Cargo.toml +++ b/bridges/primitives/chain-polkadot-bulletin/Cargo.toml @@ -35,6 +35,7 @@ std = [ "codec/std", "frame-support/std", "frame-system/std", + "scale-info/std", "sp-api/std", "sp-runtime/std", "sp-std/std", diff --git a/bridges/primitives/relayers/Cargo.toml b/bridges/primitives/relayers/Cargo.toml index 93ad07892635..ffed2debbe68 100644 --- a/bridges/primitives/relayers/Cargo.toml +++ b/bridges/primitives/relayers/Cargo.toml @@ -30,7 +30,9 @@ default = [ "std" ] std = [ "bp-messages/std", "bp-runtime/std", + "codec/std", "frame-support/std", + "scale-info/std", "sp-runtime/std", "sp-std/std", ] diff --git a/cumulus/pallets/parachain-system/Cargo.toml b/cumulus/pallets/parachain-system/Cargo.toml index 1b367f94e330..d48604d50258 100644 --- a/cumulus/pallets/parachain-system/Cargo.toml +++ b/cumulus/pallets/parachain-system/Cargo.toml @@ -55,6 +55,7 @@ cumulus-test-relay-sproof-builder = { path = "../../test/relay-sproof-builder" } [features] default = [ "std" ] std = [ + "bytes/std", "codec/std", "cumulus-pallet-parachain-system-proc-macro/std", "cumulus-primitives-core/std", diff --git a/cumulus/pallets/xcmp-queue/Cargo.toml b/cumulus/pallets/xcmp-queue/Cargo.toml index 358a0bec8bb0..b923c16cb1bf 100644 --- a/cumulus/pallets/xcmp-queue/Cargo.toml +++ b/cumulus/pallets/xcmp-queue/Cargo.toml @@ -57,6 +57,7 @@ std = [ "log/std", "polkadot-runtime-common/std", "polkadot-runtime-parachains/std", + "rand_chacha/std", "scale-info/std", "sp-core/std", "sp-io/std", diff --git a/cumulus/parachains/common/Cargo.toml b/cumulus/parachains/common/Cargo.toml index 3393a7a46c1d..92c7cb6ef121 100644 --- a/cumulus/parachains/common/Cargo.toml +++ b/cumulus/parachains/common/Cargo.toml @@ -52,11 +52,13 @@ substrate-wasm-builder = { path = "../../../substrate/utils/wasm-builder" } [features] default = [ "std" ] std = [ + "codec/std", "cumulus-primitives-core/std", "cumulus-primitives-utility/std", "frame-support/std", "frame-system/std", "log/std", + "num-traits/std", "pallet-asset-tx-payment/std", "pallet-assets/std", "pallet-authorship/std", @@ -66,6 +68,7 @@ std = [ "polkadot-core-primitives/std", "polkadot-primitives/std", "rococo-runtime-constants/std", + "scale-info/std", "sp-consensus-aura/std", "sp-core/std", "sp-io/std", diff --git a/cumulus/parachains/runtimes/assets/asset-hub-kusama/Cargo.toml b/cumulus/parachains/runtimes/assets/asset-hub-kusama/Cargo.toml index dc401e3d8cd3..ede9c6af35a0 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-kusama/Cargo.toml +++ b/cumulus/parachains/runtimes/assets/asset-hub-kusama/Cargo.toml @@ -203,6 +203,7 @@ std = [ "polkadot-core-primitives/std", "polkadot-parachain-primitives/std", "polkadot-runtime-common/std", + "primitive-types/std", "scale-info/std", "sp-api/std", "sp-block-builder/std", diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/Cargo.toml b/cumulus/parachains/runtimes/assets/asset-hub-rococo/Cargo.toml index b1ec66f40bff..ea5b5fa05540 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/Cargo.toml +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/Cargo.toml @@ -217,6 +217,7 @@ std = [ "polkadot-core-primitives/std", "polkadot-parachain-primitives/std", "polkadot-runtime-common/std", + "primitive-types/std", "rococo-runtime-constants/std", "scale-info/std", "sp-api/std", diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/Cargo.toml b/cumulus/parachains/runtimes/assets/asset-hub-westend/Cargo.toml index 7a523f8bfb66..5e3807f27858 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/Cargo.toml +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/Cargo.toml @@ -194,6 +194,7 @@ std = [ "polkadot-core-primitives/std", "polkadot-parachain-primitives/std", "polkadot-runtime-common/std", + "primitive-types/std", "scale-info/std", "sp-api/std", "sp-block-builder/std", diff --git a/cumulus/parachains/runtimes/assets/common/Cargo.toml b/cumulus/parachains/runtimes/assets/common/Cargo.toml index 56171c7212e6..770acc93c71d 100644 --- a/cumulus/parachains/runtimes/assets/common/Cargo.toml +++ b/cumulus/parachains/runtimes/assets/common/Cargo.toml @@ -43,6 +43,7 @@ std = [ "pallet-asset-tx-payment/std", "pallet-xcm/std", "parachains-common/std", + "scale-info/std", "sp-api/std", "sp-runtime/std", "sp-std/std", diff --git a/cumulus/parachains/runtimes/assets/test-utils/Cargo.toml b/cumulus/parachains/runtimes/assets/test-utils/Cargo.toml index d8b5ca5c8e59..86cc72e2dd3c 100644 --- a/cumulus/parachains/runtimes/assets/test-utils/Cargo.toml +++ b/cumulus/parachains/runtimes/assets/test-utils/Cargo.toml @@ -53,6 +53,7 @@ substrate-wasm-builder = { path = "../../../../../substrate/utils/wasm-builder" default = [ "std" ] std = [ "assets-common/std", + "codec/std", "cumulus-pallet-dmp-queue/std", "cumulus-pallet-parachain-system/std", "cumulus-pallet-xcmp-queue/std", diff --git a/cumulus/parachains/runtimes/test-utils/Cargo.toml b/cumulus/parachains/runtimes/test-utils/Cargo.toml index 1ecbb212a0e6..c455807fd8f8 100644 --- a/cumulus/parachains/runtimes/test-utils/Cargo.toml +++ b/cumulus/parachains/runtimes/test-utils/Cargo.toml @@ -49,6 +49,7 @@ substrate-wasm-builder = { path = "../../../../substrate/utils/wasm-builder" } default = [ "std" ] std = [ "assets-common/std", + "codec/std", "cumulus-pallet-dmp-queue/std", "cumulus-pallet-parachain-system/std", "cumulus-pallet-xcmp-queue/std", diff --git a/cumulus/primitives/timestamp/Cargo.toml b/cumulus/primitives/timestamp/Cargo.toml index 6b0d3d4a4dc6..aed51a449127 100644 --- a/cumulus/primitives/timestamp/Cargo.toml +++ b/cumulus/primitives/timestamp/Cargo.toml @@ -20,6 +20,7 @@ cumulus-primitives-core = { path = "../core", default-features = false } [features] default = [ "std" ] std = [ + "codec/std", "cumulus-primitives-core/std", "sp-inherents/std", "sp-std/std", diff --git a/cumulus/primitives/utility/Cargo.toml b/cumulus/primitives/utility/Cargo.toml index 691a4599b2c4..d50f93d89b76 100644 --- a/cumulus/primitives/utility/Cargo.toml +++ b/cumulus/primitives/utility/Cargo.toml @@ -31,6 +31,7 @@ std = [ "codec/std", "cumulus-primitives-core/std", "frame-support/std", + "log/std", "pallet-xcm-benchmarks/std", "polkadot-runtime-common/std", "polkadot-runtime-parachains/std", diff --git a/docs/STYLE_GUIDE.md b/docs/STYLE_GUIDE.md index a9ac4a5910b3..1ae9bc5003f6 100644 --- a/docs/STYLE_GUIDE.md +++ b/docs/STYLE_GUIDE.md @@ -152,7 +152,7 @@ let mut target_path = > **TLDR** > You can use the CLI tool [Zepter](https://crates.io/crates/zepter) to -> format the files: `zepter format features` +> format the files: `zepter format features --fix` (or `zepter f f -f`). Rust `Cargo.toml` files need to respect certain formatting rules. All entries need to be alphabetically sorted. This makes it easier to read them and insert diff --git a/polkadot/parachain/test-parachains/Cargo.toml b/polkadot/parachain/test-parachains/Cargo.toml index a30be9c678af..3252d1f83cd3 100644 --- a/polkadot/parachain/test-parachains/Cargo.toml +++ b/polkadot/parachain/test-parachains/Cargo.toml @@ -19,4 +19,4 @@ sp-core = { path = "../../../substrate/primitives/core" } [features] default = [ "std" ] -std = [ "adder/std", "halt/std" ] +std = [ "adder/std", "halt/std", "parity-scale-codec/std" ] diff --git a/polkadot/parachain/test-parachains/adder/Cargo.toml b/polkadot/parachain/test-parachains/adder/Cargo.toml index f22b5ccfdcc9..1a47328b28e9 100644 --- a/polkadot/parachain/test-parachains/adder/Cargo.toml +++ b/polkadot/parachain/test-parachains/adder/Cargo.toml @@ -23,4 +23,4 @@ substrate-wasm-builder = { path = "../../../../substrate/utils/wasm-builder" } [features] default = [ "std" ] -std = [ "parachain/std", "sp-io/std", "sp-std/std" ] +std = [ "parachain/std", "parity-scale-codec/std", "sp-io/std", "sp-std/std" ] diff --git a/polkadot/parachain/test-parachains/undying/Cargo.toml b/polkadot/parachain/test-parachains/undying/Cargo.toml index 8690da5a4bdd..273eef4b63a0 100644 --- a/polkadot/parachain/test-parachains/undying/Cargo.toml +++ b/polkadot/parachain/test-parachains/undying/Cargo.toml @@ -24,4 +24,10 @@ substrate-wasm-builder = { path = "../../../../substrate/utils/wasm-builder" } [features] default = [ "std" ] -std = [ "parachain/std", "sp-io/std", "sp-std/std" ] +std = [ + "log/std", + "parachain/std", + "parity-scale-codec/std", + "sp-io/std", + "sp-std/std", +] diff --git a/polkadot/runtime/parachains/Cargo.toml b/polkadot/runtime/parachains/Cargo.toml index a39500833732..e49c00459dec 100644 --- a/polkadot/runtime/parachains/Cargo.toml +++ b/polkadot/runtime/parachains/Cargo.toml @@ -88,6 +88,8 @@ std = [ "polkadot-parachain-primitives/std", "polkadot-runtime-metrics/std", "primitives/std", + "rand/std", + "rand_chacha/std", "rustc-hex/std", "scale-info/std", "serde/std", diff --git a/polkadot/xcm/Cargo.toml b/polkadot/xcm/Cargo.toml index 92024e69c1b6..60c27f7fcfc3 100644 --- a/polkadot/xcm/Cargo.toml +++ b/polkadot/xcm/Cargo.toml @@ -29,6 +29,7 @@ wasm-api = [] std = [ "bounded-collections/std", "environmental/std", + "log/std", "parity-scale-codec/std", "scale-info/std", "serde/std", diff --git a/polkadot/xcm/pallet-xcm-benchmarks/Cargo.toml b/polkadot/xcm/pallet-xcm-benchmarks/Cargo.toml index fb4f389b212c..88df81a3dad4 100644 --- a/polkadot/xcm/pallet-xcm-benchmarks/Cargo.toml +++ b/polkadot/xcm/pallet-xcm-benchmarks/Cargo.toml @@ -41,6 +41,8 @@ std = [ "frame-benchmarking/std", "frame-support/std", "frame-system/std", + "log/std", + "scale-info/std", "sp-io/std", "sp-runtime/std", "sp-std/std", diff --git a/polkadot/xcm/pallet-xcm/Cargo.toml b/polkadot/xcm/pallet-xcm/Cargo.toml index e471e8cf2695..da472fbe6db5 100644 --- a/polkadot/xcm/pallet-xcm/Cargo.toml +++ b/polkadot/xcm/pallet-xcm/Cargo.toml @@ -38,6 +38,7 @@ std = [ "frame-benchmarking?/std", "frame-support/std", "frame-system/std", + "log/std", "scale-info/std", "serde", "sp-core/std", diff --git a/polkadot/xcm/xcm-executor/Cargo.toml b/polkadot/xcm/xcm-executor/Cargo.toml index 902f55901d6c..9f0caa80617c 100644 --- a/polkadot/xcm/xcm-executor/Cargo.toml +++ b/polkadot/xcm/xcm-executor/Cargo.toml @@ -29,6 +29,7 @@ runtime-benchmarks = [ "sp-runtime/runtime-benchmarks", ] std = [ + "environmental/std", "frame-benchmarking/std", "frame-support/std", "log/std", diff --git a/substrate/bin/node/runtime/Cargo.toml b/substrate/bin/node/runtime/Cargo.toml index bf6c540774c1..a9d685523f6c 100644 --- a/substrate/bin/node/runtime/Cargo.toml +++ b/substrate/bin/node/runtime/Cargo.toml @@ -228,6 +228,7 @@ std = [ "pallet-utility/std", "pallet-vesting/std", "pallet-whitelist/std", + "primitive-types/std", "scale-info/std", "sp-api/std", "sp-authority-discovery/std", diff --git a/substrate/frame/asset-conversion/Cargo.toml b/substrate/frame/asset-conversion/Cargo.toml index 0fd2d6c19f89..de898d4ccde1 100644 --- a/substrate/frame/asset-conversion/Cargo.toml +++ b/substrate/frame/asset-conversion/Cargo.toml @@ -39,6 +39,7 @@ std = [ "frame-system/std", "pallet-assets/std", "pallet-balances/std", + "primitive-types/std", "scale-info/std", "sp-api/std", "sp-arithmetic/std", diff --git a/substrate/frame/democracy/Cargo.toml b/substrate/frame/democracy/Cargo.toml index c1477745848d..870bfaa9b892 100644 --- a/substrate/frame/democracy/Cargo.toml +++ b/substrate/frame/democracy/Cargo.toml @@ -39,6 +39,7 @@ std = [ "frame-benchmarking?/std", "frame-support/std", "frame-system/std", + "log/std", "pallet-balances/std", "pallet-preimage/std", "pallet-scheduler/std", diff --git a/substrate/frame/glutton/Cargo.toml b/substrate/frame/glutton/Cargo.toml index 3f47191cf0a8..368fcab65cc2 100644 --- a/substrate/frame/glutton/Cargo.toml +++ b/substrate/frame/glutton/Cargo.toml @@ -36,6 +36,7 @@ std = [ "frame-benchmarking?/std", "frame-support/std", "frame-system/std", + "log/std", "pallet-balances/std", "scale-info/std", "sp-core/std", diff --git a/substrate/frame/identity/Cargo.toml b/substrate/frame/identity/Cargo.toml index cc2b50cdbd3d..309c0aab5500 100644 --- a/substrate/frame/identity/Cargo.toml +++ b/substrate/frame/identity/Cargo.toml @@ -31,6 +31,7 @@ sp-core = { path = "../../primitives/core" } default = [ "std" ] std = [ "codec/std", + "enumflags2/std", "frame-benchmarking?/std", "frame-support/std", "frame-system/std", diff --git a/substrate/frame/message-queue/Cargo.toml b/substrate/frame/message-queue/Cargo.toml index 831259597ea2..48304cd882c1 100644 --- a/substrate/frame/message-queue/Cargo.toml +++ b/substrate/frame/message-queue/Cargo.toml @@ -37,6 +37,7 @@ std = [ "frame-benchmarking?/std", "frame-support/std", "frame-system/std", + "log/std", "scale-info/std", "sp-arithmetic/std", "sp-core/std", diff --git a/substrate/frame/multisig/Cargo.toml b/substrate/frame/multisig/Cargo.toml index b0fc189974ba..a2ee608c33cd 100644 --- a/substrate/frame/multisig/Cargo.toml +++ b/substrate/frame/multisig/Cargo.toml @@ -35,6 +35,7 @@ std = [ "frame-benchmarking?/std", "frame-support/std", "frame-system/std", + "log/std", "pallet-balances/std", "scale-info/std", "sp-io/std", diff --git a/substrate/frame/nfts/Cargo.toml b/substrate/frame/nfts/Cargo.toml index 3ad8707b9a37..2a3b2921c75f 100644 --- a/substrate/frame/nfts/Cargo.toml +++ b/substrate/frame/nfts/Cargo.toml @@ -33,6 +33,7 @@ sp-keystore = { path = "../../primitives/keystore" } default = [ "std" ] std = [ "codec/std", + "enumflags2/std", "frame-benchmarking?/std", "frame-support/std", "frame-system/std", diff --git a/substrate/frame/nomination-pools/benchmarking/Cargo.toml b/substrate/frame/nomination-pools/benchmarking/Cargo.toml index e4720f25fcdd..e8b18666815e 100644 --- a/substrate/frame/nomination-pools/benchmarking/Cargo.toml +++ b/substrate/frame/nomination-pools/benchmarking/Cargo.toml @@ -43,6 +43,7 @@ sp-io = { path = "../../../primitives/io" } default = [ "std" ] std = [ + "codec/std", "frame-benchmarking/std", "frame-election-provider-support/std", "frame-support/std", @@ -52,6 +53,7 @@ std = [ "pallet-nomination-pools/std", "pallet-staking/std", "pallet-timestamp/std", + "scale-info/std", "sp-core/std", "sp-io/std", "sp-runtime-interface/std", diff --git a/substrate/frame/referenda/Cargo.toml b/substrate/frame/referenda/Cargo.toml index 9146bef79ce9..4f53e2bc002a 100644 --- a/substrate/frame/referenda/Cargo.toml +++ b/substrate/frame/referenda/Cargo.toml @@ -42,6 +42,7 @@ std = [ "frame-benchmarking?/std", "frame-support/std", "frame-system/std", + "log/std", "pallet-balances/std", "pallet-preimage/std", "pallet-scheduler/std", diff --git a/substrate/frame/society/Cargo.toml b/substrate/frame/society/Cargo.toml index fa13bc3bc8d2..654447e6893b 100644 --- a/substrate/frame/society/Cargo.toml +++ b/substrate/frame/society/Cargo.toml @@ -40,6 +40,7 @@ std = [ "frame-support-test/std", "frame-support/std", "frame-system/std", + "log/std", "pallet-balances/std", "rand_chacha/std", "scale-info/std", diff --git a/substrate/frame/statement/Cargo.toml b/substrate/frame/statement/Cargo.toml index ffb469051d1b..a5c8cf5b8de7 100644 --- a/substrate/frame/statement/Cargo.toml +++ b/substrate/frame/statement/Cargo.toml @@ -33,6 +33,7 @@ std = [ "codec/std", "frame-support/std", "frame-system/std", + "log/std", "pallet-balances/std", "scale-info/std", "sp-api/std", diff --git a/substrate/frame/support/Cargo.toml b/substrate/frame/support/Cargo.toml index 3eb453d9b2ed..5caf993bb35a 100644 --- a/substrate/frame/support/Cargo.toml +++ b/substrate/frame/support/Cargo.toml @@ -66,6 +66,7 @@ std = [ "log/std", "scale-info/std", "serde/std", + "serde_json/std", "sp-api/std", "sp-arithmetic/std", "sp-core/std", diff --git a/substrate/primitives/api/proc-macro/Cargo.toml b/substrate/primitives/api/proc-macro/Cargo.toml index 25c87b5d0a4d..d60b9f1bb4ea 100644 --- a/substrate/primitives/api/proc-macro/Cargo.toml +++ b/substrate/primitives/api/proc-macro/Cargo.toml @@ -30,6 +30,6 @@ assert_matches = "1.3.0" [features] # Required for the doc tests default = [ "std" ] -std = [] +std = [ "blake2/std" ] no-metadata-docs = [] frame-metadata = [] diff --git a/substrate/primitives/consensus/beefy/Cargo.toml b/substrate/primitives/consensus/beefy/Cargo.toml index cfc98f19bccf..5ff0a2ebc70f 100644 --- a/substrate/primitives/consensus/beefy/Cargo.toml +++ b/substrate/primitives/consensus/beefy/Cargo.toml @@ -42,6 +42,7 @@ std = [ "sp-mmr-primitives/std", "sp-runtime/std", "sp-std/std", + "strum/std", ] # Serde support without relying on std features. diff --git a/substrate/primitives/core/Cargo.toml b/substrate/primitives/core/Cargo.toml index 7929833a2e29..1b6b10eeeedf 100644 --- a/substrate/primitives/core/Cargo.toml +++ b/substrate/primitives/core/Cargo.toml @@ -116,7 +116,9 @@ std = [ "thiserror", "tiny-bip39", "tracing", + "w3f-bls?/std", "zeroize/alloc", + "zeroize/std", ] # Serde support without relying on std features. diff --git a/substrate/primitives/io/Cargo.toml b/substrate/primitives/io/Cargo.toml index ab9d26ec26f4..445104b736e0 100644 --- a/substrate/primitives/io/Cargo.toml +++ b/substrate/primitives/io/Cargo.toml @@ -44,8 +44,9 @@ std = [ "bytes/std", "codec/std", "ed25519-dalek", + "ed25519-dalek?/std", "libsecp256k1", - "log", + "log/std", "secp256k1", "sp-core/std", "sp-externalities/std", diff --git a/substrate/primitives/merkle-mountain-range/Cargo.toml b/substrate/primitives/merkle-mountain-range/Cargo.toml index 166c18954458..5216765825ff 100644 --- a/substrate/primitives/merkle-mountain-range/Cargo.toml +++ b/substrate/primitives/merkle-mountain-range/Cargo.toml @@ -33,6 +33,7 @@ std = [ "codec/std", "log/std", "mmr-lib/std", + "scale-info/std", "serde/std", "sp-api/std", "sp-core/std", diff --git a/substrate/primitives/runtime-interface/test-wasm/Cargo.toml b/substrate/primitives/runtime-interface/test-wasm/Cargo.toml index de0d6f9a13e2..7729f89fa39a 100644 --- a/substrate/primitives/runtime-interface/test-wasm/Cargo.toml +++ b/substrate/primitives/runtime-interface/test-wasm/Cargo.toml @@ -25,6 +25,7 @@ substrate-wasm-builder = { path = "../../../utils/wasm-builder", optional = true [features] default = [ "std" ] std = [ + "bytes/std", "sp-core/std", "sp-io/std", "sp-runtime-interface/std", diff --git a/substrate/primitives/statement-store/Cargo.toml b/substrate/primitives/statement-store/Cargo.toml index 75bbf421adac..658229cef220 100644 --- a/substrate/primitives/statement-store/Cargo.toml +++ b/substrate/primitives/statement-store/Cargo.toml @@ -37,10 +37,12 @@ rand = { version = "0.8.5", features = ["small_rng"], optional = true } default = [ "std" ] std = [ "aes-gcm", + "aes-gcm?/std", "codec/std", "curve25519-dalek", "ed25519-dalek", "hkdf", + "hkdf?/std", "rand", "scale-info/std", "sha2", diff --git a/substrate/primitives/wasm-interface/Cargo.toml b/substrate/primitives/wasm-interface/Cargo.toml index 92f884e3fd27..c7413fec43c4 100644 --- a/substrate/primitives/wasm-interface/Cargo.toml +++ b/substrate/primitives/wasm-interface/Cargo.toml @@ -23,5 +23,5 @@ sp-std = { path = "../std", default-features = false} [features] default = [ "std" ] -std = [ "codec/std", "log", "sp-std/std", "wasmtime" ] +std = [ "codec/std", "log/std", "sp-std/std", "wasmtime" ] wasmtime = [ "anyhow", "dep:wasmtime" ] diff --git a/substrate/test-utils/runtime/Cargo.toml b/substrate/test-utils/runtime/Cargo.toml index dc0a6076a291..84b214a0b9cb 100644 --- a/substrate/test-utils/runtime/Cargo.toml +++ b/substrate/test-utils/runtime/Cargo.toml @@ -81,6 +81,8 @@ std = [ "sc-executor/std", "sc-service", "scale-info/std", + "serde/std", + "serde_json/std", "sp-api/std", "sp-application-crypto/std", "sp-block-builder/std", From 8a79fb22dbe8796579d28d06077341cec4ae67d0 Mon Sep 17 00:00:00 2001 From: Alexander Samusev <41779041+alvicsam@users.noreply.github.com> Date: Tue, 24 Oct 2023 18:44:48 +0200 Subject: [PATCH 03/13] [ci] Run check-rust-feature-propagation in pr and master (#2012) Need to run this job in master to make it `Required` --- .gitlab/pipeline/check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab/pipeline/check.yml b/.gitlab/pipeline/check.yml index bf2e7341b8bb..cd26003d88c1 100644 --- a/.gitlab/pipeline/check.yml +++ b/.gitlab/pipeline/check.yml @@ -85,7 +85,7 @@ check-rust-feature-propagation: stage: check extends: - .kubernetes-env - - .test-pr-refs + - .common-refs script: - cargo install --locked --version 0.13.2 -q -f zepter && zepter --version - zepter run check From fbd57771185ffe1f928d420f8942b0d39febd7bc Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Tue, 24 Oct 2023 18:37:34 +0000 Subject: [PATCH 04/13] Application Crypto and BEEFY Support for paired (ECDSA,BLS) crypto (#1815) Next step in process of making BEEFY being able to generate both ECDSA and BLS signature after #1705. It allows BEEFY to use a pair of ECDSA and BLS key as a AuthorityId. --------- Co-authored-by: Davide Galassi Co-authored-by: Robert Hambrock --- substrate/client/keystore/src/local.rs | 27 +++++++- .../application-crypto/src/ecdsa_bls377.rs | 57 ++++++++++++++++ .../primitives/application-crypto/src/lib.rs | 2 + .../primitives/consensus/beefy/src/lib.rs | 66 +++++++++++++++++-- substrate/primitives/core/src/bls.rs | 2 +- substrate/primitives/core/src/lib.rs | 2 + .../primitives/core/src/paired_crypto.rs | 20 +++--- substrate/primitives/core/src/testing.rs | 2 + substrate/primitives/io/src/lib.rs | 21 +++++- substrate/primitives/keystore/src/lib.rs | 65 +++++++++++++++++- substrate/primitives/keystore/src/testing.rs | 26 +++++++- 11 files changed, 273 insertions(+), 17 deletions(-) create mode 100644 substrate/primitives/application-crypto/src/ecdsa_bls377.rs diff --git a/substrate/client/keystore/src/local.rs b/substrate/client/keystore/src/local.rs index c77f023e0f0a..8089dbba0352 100644 --- a/substrate/client/keystore/src/local.rs +++ b/substrate/client/keystore/src/local.rs @@ -37,7 +37,7 @@ use sp_core::bandersnatch; } sp_keystore::bls_experimental_enabled! { -use sp_core::{bls377, bls381}; +use sp_core::{bls377, bls381, ecdsa_bls377}; } use crate::{Error, Result}; @@ -366,6 +366,31 @@ impl Keystore for LocalKeystore { ) -> std::result::Result, TraitError> { self.sign::(key_type, public, msg) } + + fn ecdsa_bls377_public_keys(&self, key_type: KeyTypeId) -> Vec { + self.public_keys::(key_type) + } + + /// Generate a new pair of paired-keys compatible with the '(ecdsa,bls377)' signature scheme. + /// + /// If `[seed]` is `Some` then the key will be ephemeral and stored in memory. + fn ecdsa_bls377_generate_new( + &self, + key_type: KeyTypeId, + seed: Option<&str>, + ) -> std::result::Result { + self.generate_new::(key_type, seed) + } + + fn ecdsa_bls377_sign( + &self, + key_type: KeyTypeId, + public: &ecdsa_bls377::Public, + msg: &[u8], + ) -> std::result::Result, TraitError> { + self.sign::(key_type, public, msg) + } + } } diff --git a/substrate/primitives/application-crypto/src/ecdsa_bls377.rs b/substrate/primitives/application-crypto/src/ecdsa_bls377.rs new file mode 100644 index 000000000000..70940587ceda --- /dev/null +++ b/substrate/primitives/application-crypto/src/ecdsa_bls377.rs @@ -0,0 +1,57 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! ECDSA and BLS12-377 paired crypto applications. + +use crate::{KeyTypeId, RuntimePublic}; + +pub use sp_core::paired_crypto::ecdsa_bls377::*; + +mod app { + crate::app_crypto!(super, sp_core::testing::ECDSA_BLS377); +} + +#[cfg(feature = "full_crypto")] +pub use app::Pair as AppPair; +pub use app::{Public as AppPublic, Signature as AppSignature}; + +impl RuntimePublic for Public { + type Signature = Signature; + + /// Dummy implementation. Returns an empty vector. + fn all(_key_type: KeyTypeId) -> Vec { + Vec::new() + } + + fn generate_pair(key_type: KeyTypeId, seed: Option>) -> Self { + sp_io::crypto::ecdsa_bls377_generate(key_type, seed) + } + + /// Dummy implementation. Returns `None`. + fn sign>(&self, _key_type: KeyTypeId, _msg: &M) -> Option { + None + } + + /// Dummy implementation. Returns `false`. + fn verify>(&self, _msg: &M, _signature: &Self::Signature) -> bool { + false + } + + fn to_raw_vec(&self) -> Vec { + sp_core::crypto::ByteArray::to_raw_vec(self) + } +} diff --git a/substrate/primitives/application-crypto/src/lib.rs b/substrate/primitives/application-crypto/src/lib.rs index 5384220bc9ca..686b486f3353 100644 --- a/substrate/primitives/application-crypto/src/lib.rs +++ b/substrate/primitives/application-crypto/src/lib.rs @@ -50,6 +50,8 @@ pub mod bls377; #[cfg(feature = "bls-experimental")] pub mod bls381; pub mod ecdsa; +#[cfg(feature = "bls-experimental")] +pub mod ecdsa_bls377; pub mod ed25519; pub mod sr25519; mod traits; diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index c69e26bf574d..5bdf8ce010a1 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -74,10 +74,11 @@ pub trait BeefyAuthorityId: RuntimeAppPublic { /// Your code should use the above types as concrete types for all crypto related /// functionality. pub mod ecdsa_crypto { - use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE as BEEFY_KEY_TYPE}; + use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; use sp_application_crypto::{app_crypto, ecdsa}; use sp_core::crypto::Wraps; - app_crypto!(ecdsa, BEEFY_KEY_TYPE); + + app_crypto!(ecdsa, KEY_TYPE); /// Identity of a BEEFY authority using ECDSA as its crypto. pub type AuthorityId = Public; @@ -115,10 +116,11 @@ pub mod ecdsa_crypto { #[cfg(feature = "bls-experimental")] pub mod bls_crypto { - use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE as BEEFY_KEY_TYPE}; + use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; use sp_application_crypto::{app_crypto, bls377}; use sp_core::{bls377::Pair as BlsPair, crypto::Wraps, Pair as _}; - app_crypto!(bls377, BEEFY_KEY_TYPE); + + app_crypto!(bls377, KEY_TYPE); /// Identity of a BEEFY authority using BLS as its crypto. pub type AuthorityId = Public; @@ -140,6 +142,46 @@ pub mod bls_crypto { } } } + +/// BEEFY cryptographic types for (ECDSA,BLS) crypto pair +/// +/// This module basically introduces four crypto types: +/// - `ecdsa_bls_crypto::Pair` +/// - `ecdsa_bls_crypto::Public` +/// - `ecdsa_bls_crypto::Signature` +/// - `ecdsa_bls_crypto::AuthorityId` +/// +/// Your code should use the above types as concrete types for all crypto related +/// functionality. +#[cfg(feature = "bls-experimental")] +pub mod ecdsa_bls_crypto { + use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; + use sp_application_crypto::{app_crypto, ecdsa_bls377}; + use sp_core::{crypto::Wraps, ecdsa_bls377::Pair as EcdsaBlsPair, Pair as _}; + + app_crypto!(ecdsa_bls377, KEY_TYPE); + + /// Identity of a BEEFY authority using (ECDSA,BLS) as its crypto. + pub type AuthorityId = Public; + + /// Signature for a BEEFY authority using (ECDSA,BLS) as its crypto. + pub type AuthoritySignature = Signature; + + impl BeefyAuthorityId for AuthorityId + where + ::Output: Into<[u8; 32]>, + { + fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { + // `w3f-bls` library uses IETF hashing standard and as such does not exposes + // a choice of hash to field function. + // We are directly calling into the library to avoid introducing new host call. + // and because BeefyAuthorityId::verify is being called in the runtime so we don't have + + EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref()) + } + } +} + /// The `ConsensusEngineId` of BEEFY. pub const BEEFY_ENGINE_ID: sp_runtime::ConsensusEngineId = *b"BEEF"; @@ -458,4 +500,20 @@ mod tests { let (other_pair, _) = bls_crypto::Pair::generate(); assert!(!BeefyAuthorityId::::verify(&other_pair.public(), &signature, msg,)); } + + #[test] + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls_beefy_verify_works() { + let msg = &b"test-message"[..]; + let (pair, _) = ecdsa_bls_crypto::Pair::generate(); + + let signature: ecdsa_bls_crypto::Signature = pair.as_inner_ref().sign(&msg).into(); + + // Verification works if same hashing function is used when signing and verifying. + assert!(BeefyAuthorityId::::verify(&pair.public(), &signature, msg)); + + // Other public key doesn't work + let (other_pair, _) = ecdsa_bls_crypto::Pair::generate(); + assert!(!BeefyAuthorityId::::verify(&other_pair.public(), &signature, msg,)); + } } diff --git a/substrate/primitives/core/src/bls.rs b/substrate/primitives/core/src/bls.rs index 8ce6eb166f86..e519ba1806c4 100644 --- a/substrate/primitives/core/src/bls.rs +++ b/substrate/primitives/core/src/bls.rs @@ -134,7 +134,7 @@ impl Eq for Public {} impl PartialOrd for Public { fn partial_cmp(&self, other: &Self) -> Option { - self.inner.partial_cmp(&other.inner) + Some(self.cmp(other)) } } diff --git a/substrate/primitives/core/src/lib.rs b/substrate/primitives/core/src/lib.rs index 75b84c89ae68..ec0641c54668 100644 --- a/substrate/primitives/core/src/lib.rs +++ b/substrate/primitives/core/src/lib.rs @@ -75,6 +75,8 @@ pub mod uint; #[cfg(feature = "bls-experimental")] pub use bls::{bls377, bls381}; +#[cfg(feature = "bls-experimental")] +pub use paired_crypto::ecdsa_bls377; pub use self::{ hash::{convert_hash, H160, H256, H512}, diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index 355fc690779f..a97b657e7578 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -33,12 +33,12 @@ use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; #[cfg(all(not(feature = "std"), feature = "serde"))] use sp_std::alloc::{format, string::String}; -use sp_runtime_interface::pass_by::PassByInner; +use sp_runtime_interface::pass_by::{self, PassBy, PassByInner}; use sp_std::convert::TryFrom; /// ECDSA and BLS12-377 paired crypto scheme #[cfg(feature = "bls-experimental")] -pub mod ecdsa_n_bls377 { +pub mod ecdsa_bls377 { use crate::{bls377, crypto::CryptoTypeId, ecdsa}; /// An identifier used to match public keys against BLS12-377 keys @@ -49,12 +49,12 @@ pub mod ecdsa_n_bls377 { const SIGNATURE_LEN: usize = ecdsa::SIGNATURE_SERIALIZED_SIZE + bls377::SIGNATURE_SERIALIZED_SIZE; - /// (ECDSA, BLS12-377) key-pair pair. + /// (ECDSA,BLS12-377) key-pair pair. #[cfg(feature = "full_crypto")] pub type Pair = super::Pair; - /// (ECDSA, BLS12-377) public key pair. + /// (ECDSA,BLS12-377) public key pair. pub type Public = super::Public; - /// (ECDSA, BLS12-377) signature pair. + /// (ECDSA,BLS12-377) signature pair. pub type Signature = super::Signature; impl super::CryptoType for Public { @@ -151,6 +151,10 @@ impl PassByInner for Public PassBy for Public { + type PassBy = pass_by::Inner; +} + #[cfg(feature = "full_crypto")] impl< LeftPair: PairT, @@ -244,7 +248,7 @@ pub trait SignatureBound: ByteArray {} impl SignatureBound for T {} /// A pair of signatures of different types -#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, PartialEq, Eq)] +#[derive(Clone, Encode, Decode, MaxEncodedLen, TypeInfo, PartialEq, Eq)] pub struct Signature([u8; LEFT_PLUS_RIGHT_LEN]); #[cfg(feature = "full_crypto")] @@ -447,12 +451,12 @@ where } } -// Test set exercising the (ECDSA, BLS12-377) implementation +// Test set exercising the (ECDSA,BLS12-377) implementation #[cfg(all(test, feature = "bls-experimental"))] mod test { use super::*; use crate::crypto::DEV_PHRASE; - use ecdsa_n_bls377::{Pair, Signature}; + use ecdsa_bls377::{Pair, Signature}; use crate::{bls377, ecdsa}; #[test] diff --git a/substrate/primitives/core/src/testing.rs b/substrate/primitives/core/src/testing.rs index 25f5f9012c99..947dcc387fc7 100644 --- a/substrate/primitives/core/src/testing.rs +++ b/substrate/primitives/core/src/testing.rs @@ -31,6 +31,8 @@ pub const BANDERSNATCH: KeyTypeId = KeyTypeId(*b"band"); pub const BLS377: KeyTypeId = KeyTypeId(*b"bls7"); /// Key type for generic BLS12-381 key. pub const BLS381: KeyTypeId = KeyTypeId(*b"bls8"); +/// Key type for (ECDSA,BLS12-377) key pair +pub const ECDSA_BLS377: KeyTypeId = KeyTypeId(*b"ecb7"); /// Macro for exporting functions from wasm in with the expected signature for using it with the /// wasm executor. This is useful for tests where you need to call a function in wasm. diff --git a/substrate/primitives/io/src/lib.rs b/substrate/primitives/io/src/lib.rs index ec098a155c9c..c4182d6ab3a0 100644 --- a/substrate/primitives/io/src/lib.rs +++ b/substrate/primitives/io/src/lib.rs @@ -106,7 +106,7 @@ use sp_core::{ }; #[cfg(feature = "bls-experimental")] -use sp_core::bls377; +use sp_core::{bls377, ecdsa_bls377}; #[cfg(feature = "std")] use sp_trie::{LayoutV0, LayoutV1, TrieConfiguration}; @@ -1207,6 +1207,25 @@ pub trait Crypto { .expect("`bls377_generate` failed") } + /// Generate an `(ecdsa,bls12-377)` key for the given key type using an optional `seed` and + /// store it in the keystore. + /// + /// The `seed` needs to be a valid utf8. + /// + /// Returns the public key. + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_generate( + &mut self, + id: KeyTypeId, + seed: Option>, + ) -> ecdsa_bls377::Public { + let seed = seed.as_ref().map(|s| std::str::from_utf8(s).expect("Seed is valid utf8!")); + self.extension::() + .expect("No `keystore` associated for the current context!") + .ecdsa_bls377_generate_new(id, seed) + .expect("`ecdsa_bls377_generate` failed") + } + /// Generate a `bandersnatch` key pair for the given key type using an optional /// `seed` and store it in the keystore. /// diff --git a/substrate/primitives/keystore/src/lib.rs b/substrate/primitives/keystore/src/lib.rs index 035af7099a6f..e415080779cf 100644 --- a/substrate/primitives/keystore/src/lib.rs +++ b/substrate/primitives/keystore/src/lib.rs @@ -23,7 +23,7 @@ pub mod testing; #[cfg(feature = "bandersnatch-experimental")] use sp_core::bandersnatch; #[cfg(feature = "bls-experimental")] -use sp_core::{bls377, bls381}; +use sp_core::{bls377, bls381, ecdsa_bls377}; use sp_core::{ crypto::{ByteArray, CryptoTypeId, KeyTypeId}, ecdsa, ed25519, sr25519, @@ -270,6 +270,10 @@ pub trait Keystore: Send + Sync { #[cfg(feature = "bls-experimental")] fn bls377_public_keys(&self, id: KeyTypeId) -> Vec; + /// Returns all (ecdsa,bls12-377) paired public keys for the given key type. + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_public_keys(&self, id: KeyTypeId) -> Vec; + /// Generate a new bls381 key pair for the given key type and an optional seed. /// /// Returns an `bls381::Public` key of the generated key pair or an `Err` if @@ -292,6 +296,17 @@ pub trait Keystore: Send + Sync { seed: Option<&str>, ) -> Result; + /// Generate a new (ecdsa,bls377) key pair for the given key type and an optional seed. + /// + /// Returns an `ecdsa_bls377::Public` key of the generated key pair or an `Err` if + /// something failed during key generation. + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_generate_new( + &self, + key_type: KeyTypeId, + seed: Option<&str>, + ) -> Result; + /// Generate a bls381 signature for a given message. /// /// Receives [`KeyTypeId`] and a [`bls381::Public`] key to be able to map @@ -324,6 +339,22 @@ pub trait Keystore: Send + Sync { msg: &[u8], ) -> Result, Error>; + /// Generate a (ecdsa,bls377) signature pair for a given message. + /// + /// Receives [`KeyTypeId`] and a [`ecdsa_bls377::Public`] key to be able to map + /// them to a private key that exists in the keystore. + /// + /// Returns an [`ecdsa_bls377::Signature`] or `None` in case the given `key_type` + /// and `public` combination doesn't exist in the keystore. + /// An `Err` will be returned if generating the signature itself failed. + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_sign( + &self, + key_type: KeyTypeId, + public: &ecdsa_bls377::Public, + msg: &[u8], + ) -> Result, Error>; + /// Insert a new secret key. fn insert(&self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()>; @@ -349,6 +380,7 @@ pub trait Keystore: Send + Sync { /// - bandersnatch /// - bls381 /// - bls377 + /// - (ecdsa,bls377) paired keys /// /// To support more schemes you can overwrite this method. /// @@ -398,6 +430,13 @@ pub trait Keystore: Send + Sync { .map_err(|_| Error::ValidationError("Invalid public key format".into()))?; self.bls377_sign(id, &public, msg)?.map(|s| s.encode()) }, + #[cfg(feature = "bls-experimental")] + ecdsa_bls377::CRYPTO_ID => { + let public = ecdsa_bls377::Public::from_slice(public) + .map_err(|_| Error::ValidationError("Invalid public key format".into()))?; + self.ecdsa_bls377_sign(id, &public, msg)?.map(|s| s.encode()) + }, + _ => return Err(Error::KeyNotSupported(id)), }; Ok(signature) @@ -560,6 +599,11 @@ impl Keystore for Arc { (**self).bls377_public_keys(id) } + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_public_keys(&self, id: KeyTypeId) -> Vec { + (**self).ecdsa_bls377_public_keys(id) + } + #[cfg(feature = "bls-experimental")] fn bls381_generate_new( &self, @@ -578,6 +622,15 @@ impl Keystore for Arc { (**self).bls377_generate_new(key_type, seed) } + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_generate_new( + &self, + key_type: KeyTypeId, + seed: Option<&str>, + ) -> Result { + (**self).ecdsa_bls377_generate_new(key_type, seed) + } + #[cfg(feature = "bls-experimental")] fn bls381_sign( &self, @@ -598,6 +651,16 @@ impl Keystore for Arc { (**self).bls377_sign(key_type, public, msg) } + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_sign( + &self, + key_type: KeyTypeId, + public: &ecdsa_bls377::Public, + msg: &[u8], + ) -> Result, Error> { + (**self).ecdsa_bls377_sign(key_type, public, msg) + } + fn insert(&self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()> { (**self).insert(key_type, suri, public) } diff --git a/substrate/primitives/keystore/src/testing.rs b/substrate/primitives/keystore/src/testing.rs index efa35fd24bf4..7f5dfd9faa71 100644 --- a/substrate/primitives/keystore/src/testing.rs +++ b/substrate/primitives/keystore/src/testing.rs @@ -22,7 +22,7 @@ use crate::{Error, Keystore, KeystorePtr}; #[cfg(feature = "bandersnatch-experimental")] use sp_core::bandersnatch; #[cfg(feature = "bls-experimental")] -use sp_core::{bls377, bls381}; +use sp_core::{bls377, bls381, ecdsa_bls377}; use sp_core::{ crypto::{ByteArray, KeyTypeId, Pair, VrfSecret}, ecdsa, ed25519, sr25519, @@ -322,6 +322,30 @@ impl Keystore for MemoryKeystore { self.sign::(key_type, public, msg) } + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_public_keys(&self, key_type: KeyTypeId) -> Vec { + self.public_keys::(key_type) + } + + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_generate_new( + &self, + key_type: KeyTypeId, + seed: Option<&str>, + ) -> Result { + self.generate_new::(key_type, seed) + } + + #[cfg(feature = "bls-experimental")] + fn ecdsa_bls377_sign( + &self, + key_type: KeyTypeId, + public: &ecdsa_bls377::Public, + msg: &[u8], + ) -> Result, Error> { + self.sign::(key_type, public, msg) + } + fn insert(&self, key_type: KeyTypeId, suri: &str, public: &[u8]) -> Result<(), ()> { self.keys .write() From 2e11ae89dc13e55c3162b9e7d7af6a1fe531df62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 25 Oct 2023 00:30:48 +0200 Subject: [PATCH 05/13] basic-authorship: Improve time recording and logging (#2010) --- .../basic-authorship/src/basic_authorship.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/substrate/client/basic-authorship/src/basic_authorship.rs b/substrate/client/basic-authorship/src/basic_authorship.rs index 57c2996ab406..4db2de61cb4c 100644 --- a/substrate/client/basic-authorship/src/basic_authorship.rs +++ b/substrate/client/basic-authorship/src/basic_authorship.rs @@ -334,7 +334,7 @@ where deadline: time::Instant, block_size_limit: Option, ) -> Result, sp_blockchain::Error> { - let propose_with_timer = time::Instant::now(); + let block_timer = time::Instant::now(); let mut block_builder = self.client.new_block_at(self.parent_hash, inherent_digests, PR::ENABLED)?; @@ -343,7 +343,6 @@ where // TODO call `after_inherents` and check if we should apply extrinsincs here // - let block_timer = time::Instant::now(); let end_reason = self.apply_extrinsics(&mut block_builder, deadline, block_size_limit).await?; let (block, storage_changes, proof) = block_builder.build()?.into_inner(); @@ -352,7 +351,7 @@ where let proof = PR::into_proof(proof).map_err(|e| sp_blockchain::Error::Application(Box::new(e)))?; - self.print_summary(&block, end_reason, block_took, propose_with_timer.elapsed()); + self.print_summary(&block, end_reason, block_took, block_timer.elapsed()); Ok(Proposal { block, proof, storage_changes }) } @@ -443,6 +442,11 @@ where let pending_tx = if let Some(pending_tx) = pending_iterator.next() { pending_tx } else { + debug!( + target: LOG_TARGET, + "No more transactions, proceeding with proposing." + ); + break EndProposingReason::NoMoreTransactions }; @@ -539,19 +543,24 @@ where } /// Prints a summary and does telemetry + metrics. + /// + /// - `block`: The block that was build. + /// - `end_reason`: Why did we stop producing the block? + /// - `block_took`: How long did it took to produce the actual block? + /// - `propose_took`: How long did the entire proposing took? fn print_summary( &self, block: &Block, end_reason: EndProposingReason, block_took: time::Duration, - propose_with_took: time::Duration, + propose_took: time::Duration, ) { let extrinsics = block.extrinsics(); self.metrics.report(|metrics| { metrics.number_of_transactions.set(extrinsics.len() as u64); metrics.block_constructed.observe(block_took.as_secs_f64()); metrics.report_end_proposing_reason(end_reason); - metrics.create_block_proposal_time.observe(propose_with_took.as_secs_f64()); + metrics.create_block_proposal_time.observe(propose_took.as_secs_f64()); }); let extrinsics_summary = if extrinsics.is_empty() { From ff3a3bca449a3cae910d2dc8c7ed3658938c6478 Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Wed, 25 Oct 2023 11:02:13 +1100 Subject: [PATCH 06/13] Small optimisation to `--profile dev` wasm builds (#1851) `wasm-builder` was adjusted to default to building wasm blobs in `release` mode even when cargo is in `debug` because `debug` wasm is too slow. A side effect of this was `.compact` and `.compact.compressed` getting built when the dev is running build in `debug`, adding ~5s to the build time of every wasm runtime. I think it's reasonable to assume if the dev is running `debug` build they want to optimise speed and do not care about the size of the wasm binary. Compacting a blob has negligible impact on its actual performance. In this PR, I adjusted the behavior of the wasm builder so it does not produce `.compact` or `.compact.compressed` wasm when the user is running in `debug`. The builder will continue to produce the bloaty wasm in release mode unless it is overriden with an env var. As suggested by @koute in review, also refactored the `maybe_compact_wasm_and_copy_blobs` into multiple funuctions, and renamed things to better support RISC-V in the future. --- There is no `T-runtime` label so @KiChjang told me to put `T1-FRAME` :) --------- Co-authored-by: Koute --- substrate/utils/wasm-builder/src/builder.rs | 4 +- .../utils/wasm-builder/src/wasm_project.rs | 305 +++++++++++------- 2 files changed, 190 insertions(+), 119 deletions(-) diff --git a/substrate/utils/wasm-builder/src/builder.rs b/substrate/utils/wasm-builder/src/builder.rs index e0a30644b231..9c1655d85623 100644 --- a/substrate/utils/wasm-builder/src/builder.rs +++ b/substrate/utils/wasm-builder/src/builder.rs @@ -273,9 +273,9 @@ fn build_project( ); let (wasm_binary, wasm_binary_bloaty) = if let Some(wasm_binary) = wasm_binary { - (wasm_binary.wasm_binary_path_escaped(), bloaty.wasm_binary_bloaty_path_escaped()) + (wasm_binary.wasm_binary_path_escaped(), bloaty.bloaty_path_escaped()) } else { - (bloaty.wasm_binary_bloaty_path_escaped(), bloaty.wasm_binary_bloaty_path_escaped()) + (bloaty.bloaty_path_escaped(), bloaty.bloaty_path_escaped()) }; crate::write_file_if_changed( diff --git a/substrate/utils/wasm-builder/src/wasm_project.rs b/substrate/utils/wasm-builder/src/wasm_project.rs index da6fab863df4..c41e0935d750 100644 --- a/substrate/utils/wasm-builder/src/wasm_project.rs +++ b/substrate/utils/wasm-builder/src/wasm_project.rs @@ -48,13 +48,13 @@ fn colorize_info_message(message: &str) -> String { pub struct WasmBinaryBloaty(PathBuf); impl WasmBinaryBloaty { - /// Returns the escaped path to the bloaty wasm binary. - pub fn wasm_binary_bloaty_path_escaped(&self) -> String { + /// Returns the escaped path to the bloaty binary. + pub fn bloaty_path_escaped(&self) -> String { self.0.display().to_string().escape_default().to_string() } - /// Returns the path to the wasm binary. - pub fn wasm_binary_bloaty_path(&self) -> &Path { + /// Returns the path to the binary. + pub fn bloaty_path(&self) -> &Path { &self.0 } } @@ -110,78 +110,112 @@ fn crate_metadata(cargo_manifest: &Path) -> Metadata { /// /// # Returns /// -/// The path to the compact WASM binary and the bloaty WASM binary. +/// The path to the compact runtime binary and the bloaty runtime binary. pub(crate) fn create_and_compile( project_cargo_toml: &Path, default_rustflags: &str, cargo_cmd: CargoCommandVersioned, features_to_enable: Vec, - wasm_binary_name: Option, + bloaty_blob_out_name_override: Option, check_for_runtime_version_section: bool, ) -> (Option, WasmBinaryBloaty) { - let wasm_workspace_root = get_wasm_workspace_root(); - let wasm_workspace = wasm_workspace_root.join("wbuild"); + let runtime_workspace_root = get_wasm_workspace_root(); + let runtime_workspace = runtime_workspace_root.join("wbuild"); let crate_metadata = crate_metadata(project_cargo_toml); let project = create_project( project_cargo_toml, - &wasm_workspace, + &runtime_workspace, &crate_metadata, crate_metadata.workspace_root.as_ref(), features_to_enable, ); - let profile = build_project(&project, default_rustflags, cargo_cmd); - let (wasm_binary, wasm_binary_compressed, bloaty) = - compact_wasm_file(&project, profile, project_cargo_toml, wasm_binary_name); + let build_config = BuildConfiguration::detect(&project); + + // Build the bloaty runtime blob + build_bloaty_blob(&build_config.blob_build_profile, &project, default_rustflags, cargo_cmd); + + // Get the name of the bloaty runtime blob. + let bloaty_blob_default_name = get_blob_name(project_cargo_toml); + let bloaty_blob_out_name = + bloaty_blob_out_name_override.unwrap_or_else(|| bloaty_blob_default_name.clone()); + + let bloaty_blob_binary = copy_bloaty_blob( + &project, + &build_config.blob_build_profile, + &bloaty_blob_default_name, + &bloaty_blob_out_name, + ); + + // Try to compact and compress the bloaty blob, if the *outer* profile wants it. + // + // This is because, by default the inner profile will be set to `Release` even when the outer + // profile is `Debug`, because the blob built in `Debug` profile is too slow for normal + // development activities. + let (compact_blob_path, compact_compressed_blob_path) = + if build_config.outer_build_profile.wants_compact() { + let compact_blob_path = compact_wasm( + &project, + &build_config.blob_build_profile, + project_cargo_toml, + &bloaty_blob_out_name, + ); + let compact_compressed_blob_path = compact_blob_path + .as_ref() + .and_then(|p| try_compress_blob(&p.0, &bloaty_blob_out_name)); + (compact_blob_path, compact_compressed_blob_path) + } else { + (None, None) + }; if check_for_runtime_version_section { - ensure_runtime_version_wasm_section_exists(bloaty.wasm_binary_bloaty_path()); + ensure_runtime_version_wasm_section_exists(bloaty_blob_binary.bloaty_path()); } - wasm_binary + compact_blob_path .as_ref() - .map(|wasm_binary| copy_wasm_to_target_directory(project_cargo_toml, wasm_binary)); + .map(|wasm_binary| copy_blob_to_target_directory(project_cargo_toml, wasm_binary)); - wasm_binary_compressed.as_ref().map(|wasm_binary_compressed| { - copy_wasm_to_target_directory(project_cargo_toml, wasm_binary_compressed) + compact_compressed_blob_path.as_ref().map(|wasm_binary_compressed| { + copy_blob_to_target_directory(project_cargo_toml, wasm_binary_compressed) }); - let final_wasm_binary = wasm_binary_compressed.or(wasm_binary); + let final_blob_binary = compact_compressed_blob_path.or(compact_blob_path); generate_rerun_if_changed_instructions( project_cargo_toml, &project, - &wasm_workspace, - final_wasm_binary.as_ref(), - &bloaty, + &runtime_workspace, + final_blob_binary.as_ref(), + &bloaty_blob_binary, ); - if let Err(err) = adjust_mtime(&bloaty, final_wasm_binary.as_ref()) { - build_helper::warning!("Error while adjusting the mtime of the wasm binaries: {}", err) + if let Err(err) = adjust_mtime(&bloaty_blob_binary, final_blob_binary.as_ref()) { + build_helper::warning!("Error while adjusting the mtime of the blob binaries: {}", err) } - (final_wasm_binary, bloaty) + (final_blob_binary, bloaty_blob_binary) } -/// Ensures that the `runtime_version` wasm section exists in the given wasm file. +/// Ensures that the `runtime_version` section exists in the given blob. /// /// If the section can not be found, it will print an error and exit the builder. -fn ensure_runtime_version_wasm_section_exists(wasm: &Path) { - let wasm_blob = fs::read(wasm).expect("`{wasm}` was just written and should exist; qed"); +fn ensure_runtime_version_wasm_section_exists(blob_path: &Path) { + let blob = fs::read(blob_path).expect("`{blob_path}` was just written and should exist; qed"); - let module: Module = match deserialize_buffer(&wasm_blob) { + let module: Module = match deserialize_buffer(&blob) { Ok(m) => m, Err(e) => { - println!("Failed to deserialize `{}`: {e:?}", wasm.display()); + println!("Failed to deserialize `{}`: {e:?}", blob_path.display()); process::exit(1); }, }; if !module.custom_sections().any(|cs| cs.name() == "runtime_version") { println!( - "Couldn't find the `runtime_version` wasm section. \ + "Couldn't find the `runtime_version` section. \ Please ensure that you are using the `sp_version::runtime_version` attribute macro!" ); process::exit(1); @@ -208,7 +242,7 @@ fn adjust_mtime( let metadata = fs::metadata(invoked_timestamp)?; let mtime = filetime::FileTime::from_last_modification_time(&metadata); - filetime::set_file_mtime(bloaty_wasm.wasm_binary_bloaty_path(), mtime)?; + filetime::set_file_mtime(bloaty_wasm.bloaty_path(), mtime)?; if let Some(binary) = compressed_or_compact_wasm.as_ref() { filetime::set_file_mtime(binary.wasm_binary_path(), mtime)?; } @@ -279,8 +313,8 @@ fn get_crate_name(cargo_manifest: &Path) -> String { .expect("Package name exists; qed") } -/// Returns the name for the wasm binary. -fn get_wasm_binary_name(cargo_manifest: &Path) -> String { +/// Returns the name for the blob binary. +fn get_blob_name(cargo_manifest: &Path) -> String { get_crate_name(cargo_manifest).replace('-', "_") } @@ -501,7 +535,7 @@ fn create_project( ) -> PathBuf { let crate_name = get_crate_name(project_cargo_toml); let crate_path = project_cargo_toml.parent().expect("Parent path exists; qed"); - let wasm_binary = get_wasm_binary_name(project_cargo_toml); + let wasm_binary = get_blob_name(project_cargo_toml); let wasm_project_folder = wasm_workspace.join(&crate_name); fs::create_dir_all(wasm_project_folder.join("src")) @@ -539,8 +573,8 @@ fn create_project( wasm_project_folder } -/// The cargo profile that is used to build the wasm project. -#[derive(Debug, EnumIter)] +/// A rustc profile. +#[derive(Clone, Debug, EnumIter)] enum Profile { /// The `--profile dev` profile. Debug, @@ -551,17 +585,63 @@ enum Profile { } impl Profile { - /// Create a profile by detecting which profile is used for the main build. + /// The name of the profile as supplied to the cargo `--profile` cli option. + fn name(&self) -> &'static str { + match self { + Self::Debug => "dev", + Self::Release => "release", + Self::Production => "production", + } + } + + /// The sub directory within `target` where cargo places the build output. + /// + /// # Note + /// + /// Usually this is the same as [`Self::name`] with the exception of the debug + /// profile which is called `dev`. + fn directory(&self) -> &'static str { + match self { + Self::Debug => "debug", + _ => self.name(), + } + } + + /// Whether the resulting binary should be compacted and compressed. + fn wants_compact(&self) -> bool { + !matches!(self, Self::Debug) + } +} + +/// The build configuration for this build. +#[derive(Debug)] +struct BuildConfiguration { + /// The profile that is used to build the outer project. + pub outer_build_profile: Profile, + /// The profile to use to build the runtime blob. + pub blob_build_profile: Profile, +} + +impl BuildConfiguration { + /// Create a [`BuildConfiguration`] by detecting which profile is used for the main build and + /// checking any env var overrides. /// /// We cannot easily determine the profile that is used by the main cargo invocation /// because the `PROFILE` environment variable won't contain any custom profiles like /// "production". It would only contain the builtin profile where the custom profile /// inherits from. This is why we inspect the build path to learn which profile is used. /// + /// When not overriden by a env variable we always default to building wasm with the `Release` + /// profile even when the main build uses the debug build. This is because wasm built with the + /// `Debug` profile is too slow for normal development activities and almost never intended. + /// + /// When cargo is building in `--profile dev`, user likely intends to compile fast, so we don't + /// bother producing compact or compressed blobs. + /// /// # Note /// /// Can be overriden by setting [`crate::WASM_BUILD_TYPE_ENV`]. - fn detect(wasm_project: &Path) -> Profile { + fn detect(wasm_project: &Path) -> Self { let (name, overriden) = if let Ok(name) = env::var(crate::WASM_BUILD_TYPE_ENV) { (name, true) } else { @@ -585,7 +665,8 @@ impl Profile { .to_string(); (name, false) }; - match (Profile::iter().find(|p| p.directory() == name), overriden) { + let outer_build_profile = Profile::iter().find(|p| p.directory() == name); + let blob_build_profile = match (outer_build_profile.clone(), overriden) { // When not overriden by a env variable we default to using the `Release` profile // for the wasm build even when the main build uses the debug build. This // is because the `Debug` profile is too slow for normal development activities. @@ -615,35 +696,12 @@ impl Profile { ); process::exit(1); }, + }; + BuildConfiguration { + outer_build_profile: outer_build_profile.unwrap_or(Profile::Release), + blob_build_profile, } } - - /// The name of the profile as supplied to the cargo `--profile` cli option. - fn name(&self) -> &'static str { - match self { - Self::Debug => "dev", - Self::Release => "release", - Self::Production => "production", - } - } - - /// The sub directory within `target` where cargo places the build output. - /// - /// # Note - /// - /// Usually this is the same as [`Self::name`] with the exception of the debug - /// profile which is called `dev`. - fn directory(&self) -> &'static str { - match self { - Self::Debug => "debug", - _ => self.name(), - } - } - - /// Whether the resulting binary should be compacted and compressed. - fn wants_compact(&self) -> bool { - !matches!(self, Self::Debug) - } } /// Check environment whether we should build without network @@ -651,12 +709,13 @@ fn offline_build() -> bool { env::var(OFFLINE).map_or(false, |v| v == "true") } -/// Build the project to create the WASM binary. -fn build_project( +/// Build the project and create the bloaty runtime blob. +fn build_bloaty_blob( + blob_build_profile: &Profile, project: &Path, default_rustflags: &str, cargo_cmd: CargoCommandVersioned, -) -> Profile { +) { let manifest_path = project.join("Cargo.toml"); let mut build_cmd = cargo_cmd.command(); @@ -685,9 +744,8 @@ fn build_project( build_cmd.arg("--color=always"); } - let profile = Profile::detect(project); build_cmd.arg("--profile"); - build_cmd.arg(profile.name()); + build_cmd.arg(blob_build_profile.name()); if offline_build() { build_cmd.arg("--offline"); @@ -697,69 +755,82 @@ fn build_project( println!("{} {:?}", colorize_info_message("Executing build command:"), build_cmd); println!("{} {}", colorize_info_message("Using rustc version:"), cargo_cmd.rustc_version()); - match build_cmd.status().map(|s| s.success()) { - Ok(true) => profile, - // Use `process.exit(1)` to have a clean error output. - _ => process::exit(1), + // Use `process::exit(1)` to have a clean error output. + if build_cmd.status().map(|s| s.success()).is_err() { + process::exit(1); } } -/// Compact the WASM binary using `wasm-gc` and compress it using zstd. -fn compact_wasm_file( +fn compact_wasm( project: &Path, - profile: Profile, + inner_profile: &Profile, cargo_manifest: &Path, - out_name: Option, -) -> (Option, Option, WasmBinaryBloaty) { - let default_out_name = get_wasm_binary_name(cargo_manifest); - let out_name = out_name.unwrap_or_else(|| default_out_name.clone()); + out_name: &str, +) -> Option { + let default_out_name = get_blob_name(cargo_manifest); let in_path = project .join("target/wasm32-unknown-unknown") - .join(profile.directory()) + .join(inner_profile.directory()) .join(format!("{}.wasm", default_out_name)); - let (wasm_compact_path, wasm_compact_compressed_path) = if profile.wants_compact() { - let wasm_compact_path = project.join(format!("{}.compact.wasm", out_name,)); - wasm_opt::OptimizationOptions::new_opt_level_0() - .mvp_features_only() - .debug_info(true) - .add_pass(wasm_opt::Pass::StripDwarf) - .run(&in_path, &wasm_compact_path) - .expect("Failed to compact generated WASM binary."); - - let wasm_compact_compressed_path = - project.join(format!("{}.compact.compressed.wasm", out_name)); - if compress_wasm(&wasm_compact_path, &wasm_compact_compressed_path) { - (Some(WasmBinary(wasm_compact_path)), Some(WasmBinary(wasm_compact_compressed_path))) - } else { - (Some(WasmBinary(wasm_compact_path)), None) - } - } else { - (None, None) - }; + let wasm_compact_path = project.join(format!("{}.compact.wasm", out_name)); + let start = std::time::Instant::now(); + wasm_opt::OptimizationOptions::new_opt_level_0() + .mvp_features_only() + .debug_info(true) + .add_pass(wasm_opt::Pass::StripDwarf) + .run(&in_path, &wasm_compact_path) + .expect("Failed to compact generated WASM binary."); + println!( + "{} {}", + colorize_info_message("Compacted wasm in"), + colorize_info_message(format!("{:?}", start.elapsed()).as_str()) + ); + Some(WasmBinary(wasm_compact_path)) +} + +fn copy_bloaty_blob( + project: &Path, + inner_profile: &Profile, + in_name: &str, + out_name: &str, +) -> WasmBinaryBloaty { + let in_path = project + .join("target/wasm32-unknown-unknown") + .join(inner_profile.directory()) + .join(format!("{}.wasm", in_name)); let bloaty_path = project.join(format!("{}.wasm", out_name)); fs::copy(in_path, &bloaty_path).expect("Copying the bloaty file to the project dir."); - - (wasm_compact_path, wasm_compact_compressed_path, WasmBinaryBloaty(bloaty_path)) + WasmBinaryBloaty(bloaty_path) } -fn compress_wasm(wasm_binary_path: &Path, compressed_binary_out_path: &Path) -> bool { +fn try_compress_blob(compact_blob_path: &Path, out_name: &str) -> Option { use sp_maybe_compressed_blob::CODE_BLOB_BOMB_LIMIT; - let data = fs::read(wasm_binary_path).expect("Failed to read WASM binary"); + let project = compact_blob_path.parent().expect("blob path should have a parent directory"); + let compact_compressed_blob_path = + project.join(format!("{}.compact.compressed.wasm", out_name)); + + let start = std::time::Instant::now(); + let data = fs::read(compact_blob_path).expect("Failed to read WASM binary"); if let Some(compressed) = sp_maybe_compressed_blob::compress(&data, CODE_BLOB_BOMB_LIMIT) { - fs::write(compressed_binary_out_path, &compressed[..]) + fs::write(&compact_compressed_blob_path, &compressed[..]) .expect("Failed to write WASM binary"); - true + println!( + "{} {}", + colorize_info_message("Compressed blob in"), + colorize_info_message(format!("{:?}", start.elapsed()).as_str()) + ); + Some(WasmBinary(compact_compressed_blob_path)) } else { build_helper::warning!( - "Writing uncompressed wasm. Exceeded maximum size {}", + "Writing uncompressed blob. Exceeded maximum size {}", CODE_BLOB_BOMB_LIMIT, ); - - false + println!("{}", colorize_info_message("Skipping blob compression")); + None } } @@ -869,7 +940,7 @@ fn generate_rerun_if_changed_instructions( packages.iter().for_each(package_rerun_if_changed); compressed_or_compact_wasm.map(|w| rerun_if_changed(w.wasm_binary_path())); - rerun_if_changed(bloaty_wasm.wasm_binary_bloaty_path()); + rerun_if_changed(bloaty_wasm.bloaty_path()); // Register our env variables println!("cargo:rerun-if-env-changed={}", crate::SKIP_BUILD_ENV); @@ -902,9 +973,9 @@ fn package_rerun_if_changed(package: &DeduplicatePackage) { .for_each(rerun_if_changed); } -/// Copy the WASM binary to the target directory set in `WASM_TARGET_DIRECTORY` environment +/// Copy the blob binary to the target directory set in `WASM_TARGET_DIRECTORY` environment /// variable. If the variable is not set, this is a no-op. -fn copy_wasm_to_target_directory(cargo_manifest: &Path, wasm_binary: &WasmBinary) { +fn copy_blob_to_target_directory(cargo_manifest: &Path, blob_binary: &WasmBinary) { let target_dir = match env::var(crate::WASM_TARGET_DIRECTORY) { Ok(path) => PathBuf::from(path), Err(_) => return, @@ -923,8 +994,8 @@ fn copy_wasm_to_target_directory(cargo_manifest: &Path, wasm_binary: &WasmBinary fs::create_dir_all(&target_dir).expect("Creates `WASM_TARGET_DIRECTORY`."); fs::copy( - wasm_binary.wasm_binary_path(), - target_dir.join(format!("{}.wasm", get_wasm_binary_name(cargo_manifest))), + blob_binary.wasm_binary_path(), + target_dir.join(format!("{}.wasm", get_blob_name(cargo_manifest))), ) - .expect("Copies WASM binary to `WASM_TARGET_DIRECTORY`."); + .expect("Copies blob binary to `WASM_TARGET_DIRECTORY`."); } From 3148063a6f7389331e3ff9e3d7b7281950b0f790 Mon Sep 17 00:00:00 2001 From: PG Herveou Date: Wed, 25 Oct 2023 09:47:54 +0200 Subject: [PATCH 07/13] Contracts: Add benchmarks to include files (#2022) Project that includes pallet-contracts via crates.io will fail to run ```bash cargo check --features=runtime-benchmarks ``` without the currently not included benchmarks files --- substrate/frame/contracts/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/contracts/Cargo.toml b/substrate/frame/contracts/Cargo.toml index 1b0d9529295c..d80577f9d441 100644 --- a/substrate/frame/contracts/Cargo.toml +++ b/substrate/frame/contracts/Cargo.toml @@ -9,7 +9,7 @@ homepage = "https://substrate.io" repository.workspace = true description = "FRAME pallet for WASM contracts" readme = "README.md" -include = ["src/**/*", "build.rs", "README.md", "CHANGELOG.md"] +include = ["src/**/*", "benchmarks/**", "build.rs", "README.md", "CHANGELOG.md"] [package.metadata.docs.rs] targets = ["x86_64-unknown-linux-gnu"] From b7a8532db14ef665d69c05019340896fbb42b88d Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Wed, 25 Oct 2023 21:59:40 +1300 Subject: [PATCH 08/13] publish pallet-root-testing (#2017) so we can use it in our tests --- substrate/frame/root-testing/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/substrate/frame/root-testing/Cargo.toml b/substrate/frame/root-testing/Cargo.toml index bb19d90466e4..7837289cec59 100644 --- a/substrate/frame/root-testing/Cargo.toml +++ b/substrate/frame/root-testing/Cargo.toml @@ -8,7 +8,6 @@ homepage = "https://substrate.io" repository.workspace = true description = "FRAME root testing pallet" readme = "README.md" -publish = false [package.metadata.docs.rs] targets = ["x86_64-unknown-linux-gnu"] From c86b633695299ed27053940d5ea5c5a2392964b3 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 25 Oct 2023 14:46:49 +0200 Subject: [PATCH 09/13] Sort the benchmarks before listing them (#2026) Signed-off-by: Oliver Tale-Yazdi --- .../utils/frame/benchmarking-cli/src/pallet/command.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs index 84da3aaa02c0..99f77866f8d0 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/command.rs @@ -748,13 +748,17 @@ impl CliConfiguration for PalletCmd { /// List the benchmarks available in the runtime, in a CSV friendly format. fn list_benchmark( - benchmarks_to_run: Vec<( + mut benchmarks_to_run: Vec<( Vec, Vec, Vec<(BenchmarkParameter, u32, u32)>, Vec<(String, String)>, )>, ) { + // Sort and de-dub by pallet and function name. + benchmarks_to_run.sort_by(|(pa, sa, _, _), (pb, sb, _, _)| (pa, sa).cmp(&(pb, sb))); + benchmarks_to_run.dedup_by(|(pa, sa, _, _), (pb, sb, _, _)| (pa, sa) == (pb, sb)); + println!("pallet, benchmark"); for (pallet, extrinsic, _, _) in benchmarks_to_run { println!("{}, {}", String::from_utf8_lossy(&pallet), String::from_utf8_lossy(&extrinsic)); From f6560c2b7226ea756ade18df42018c3eaf3be2e0 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Wed, 25 Oct 2023 17:35:15 +0200 Subject: [PATCH 10/13] [testnet] Align testnet system parachain runtimes using `RelayTreasuryLocation` and `SystemParachains` in the same way (#2023) This PR addresses several issues: - simplify referencing `RelayTreasuryLocation` without needing additional `RelayTreasury` struct - fix for referencing `SystemParachains` from parachain with `parents: 1` instead of `parents: 0` - removed hard-coded constants and fix tests for `asset-hub-rococo` which was merged to master after https://github.com/paritytech/polkadot-sdk/pull/1726 --------- Co-authored-by: command-bot <> --- .../src/tests/reserve_transfer.rs | 10 +- .../assets/asset-hub-rococo/src/tests/send.rs | 5 +- .../asset-hub-rococo/src/tests/teleport.rs | 8 +- .../assets/asset-hub-rococo/src/lib.rs | 8 +- .../asset-hub-rococo/src/weights/xcm/mod.rs | 18 +-- .../xcm/pallet_xcm_benchmarks_fungible.rs | 131 ++++++++++-------- .../assets/asset-hub-rococo/src/xcm_config.rs | 33 +++-- .../asset-hub-westend/src/xcm_config.rs | 34 ++--- .../assets/test-utils/src/test_cases.rs | 5 +- .../test-utils/src/test_cases_over_bridge.rs | 17 ++- .../bridge-hub-rococo/src/xcm_config.rs | 38 +++-- .../contracts-rococo/src/xcm_config.rs | 40 ++++-- 12 files changed, 195 insertions(+), 152 deletions(-) diff --git a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/reserve_transfer.rs b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/reserve_transfer.rs index fb25607c635e..ee5f0727a1e9 100644 --- a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/reserve_transfer.rs +++ b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/reserve_transfer.rs @@ -39,7 +39,7 @@ fn relay_origin_assertions(t: RelayToSystemParaTest) { fn system_para_dest_assertions_incomplete(_t: RelayToSystemParaTest) { AssetHubRococo::assert_dmp_queue_incomplete( - Some(Weight::from_parts(1_000_000_000, 0)), + Some(Weight::from_parts(57_185_000, 3504)), Some(Error::UntrustedReserveLocation), ); } @@ -52,8 +52,8 @@ fn system_para_to_para_assertions(t: SystemParaToParaTest) { type RuntimeEvent = ::RuntimeEvent; AssetHubRococo::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts( - 630_092_000, - 6_196, + 864_610_000, + 8_799, ))); assert_expected_events!( @@ -77,8 +77,8 @@ fn system_para_to_para_assets_assertions(t: SystemParaToParaTest) { type RuntimeEvent = ::RuntimeEvent; AssetHubRococo::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts( - 676_119_000, - 6196, + 864_610_000, + 8799, ))); assert_expected_events!( diff --git a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/send.rs b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/send.rs index b8ec370e3f8d..195afcd34c70 100644 --- a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/send.rs +++ b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/send.rs @@ -83,7 +83,10 @@ fn send_xcm_from_para_to_system_para_paying_fee_with_assets_works() { AssetHubRococo::execute_with(|| { type RuntimeEvent = ::RuntimeEvent; - AssetHubRococo::assert_xcmp_queue_success(Some(Weight::from_parts(2_176_414_000, 203_593))); + AssetHubRococo::assert_xcmp_queue_success(Some(Weight::from_parts( + 15_594_564_000, + 562_893, + ))); assert_expected_events!( AssetHubRococo, diff --git a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/teleport.rs b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/teleport.rs index 4b2ea0e160cb..0d2ca6852470 100644 --- a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/teleport.rs +++ b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/teleport.rs @@ -68,7 +68,7 @@ fn relay_dest_assertions_fail(_t: SystemParaToRelayTest) { Rococo::assert_ump_queue_processed( false, Some(AssetHubRococo::para_id()), - Some(Weight::from_parts(148_433_000, 3_593)), + Some(Weight::from_parts(157_718_000, 3_593)), ); } @@ -76,8 +76,8 @@ fn para_origin_assertions(t: SystemParaToRelayTest) { type RuntimeEvent = ::RuntimeEvent; AssetHubRococo::assert_xcm_pallet_attempted_complete(Some(Weight::from_parts( - 534_872_000, - 7_133, + 720_053_000, + 7_203, ))); AssetHubRococo::assert_parachain_system_ump_sent(); @@ -97,7 +97,7 @@ fn para_origin_assertions(t: SystemParaToRelayTest) { fn para_dest_assertions(t: RelayToSystemParaTest) { type RuntimeEvent = ::RuntimeEvent; - AssetHubRococo::assert_dmp_queue_complete(Some(Weight::from_parts(165_592_000, 0))); + AssetHubRococo::assert_dmp_queue_complete(Some(Weight::from_parts(157_718_000, 3593))); assert_expected_events!( AssetHubRococo, diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index f57cfda1428d..1ce504d6704f 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -1381,7 +1381,13 @@ impl_runtime_apis! { MultiAsset { fun: Fungible(UNITS), id: Concrete(TokenLocation::get()) }, )); pub const CheckedAccount: Option<(AccountId, xcm_builder::MintLocation)> = None; - pub const TrustedReserve: Option<(MultiLocation, MultiAsset)> = None; + // AssetHubRococo trusts AssetHubWococo as reserve for WOCs + pub TrustedReserve: Option<(MultiLocation, MultiAsset)> = Some( + ( + xcm_config::bridging::to_wococo::AssetHubWococo::get(), + MultiAsset::from((xcm_config::bridging::to_wococo::WocLocation::get(), 1000000000000 as u128)) + ) + ); } impl pallet_xcm_benchmarks::fungible::Config for Runtime { diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/xcm/mod.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/xcm/mod.rs index 22c2f2450b29..2fbbd61654be 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/xcm/mod.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/xcm/mod.rs @@ -61,16 +61,8 @@ impl XcmWeightInfo for AssetHubRococoXcmWeight { fn withdraw_asset(assets: &MultiAssets) -> Weight { assets.weigh_multi_assets(XcmFungibleWeight::::withdraw_asset()) } - // Currently there is no trusted reserve (`IsReserve = ()`), - // but we need this hack for `pallet_xcm::reserve_transfer_assets` - // (TODO) fix https://github.com/paritytech/polkadot/pull/7424 - // (TODO) fix https://github.com/paritytech/polkadot/pull/7546 - fn reserve_asset_deposited(_assets: &MultiAssets) -> Weight { - // TODO: if we change `IsReserve = ...` then use this line... - // TODO: or if remote weight estimation is fixed, then remove - // TODO: hardcoded - fix https://github.com/paritytech/cumulus/issues/1974 - let hardcoded_weight = Weight::from_parts(1_000_000_000_u64, 0); - hardcoded_weight.min(XcmFungibleWeight::::reserve_asset_deposited()) + fn reserve_asset_deposited(assets: &MultiAssets) -> Weight { + assets.weigh_multi_assets(XcmFungibleWeight::::reserve_asset_deposited()) } fn receive_teleported_asset(assets: &MultiAssets) -> Weight { assets.weigh_multi_assets(XcmFungibleWeight::::receive_teleported_asset()) @@ -125,12 +117,8 @@ impl XcmWeightInfo for AssetHubRococoXcmWeight { fn report_error(_query_response_info: &QueryResponseInfo) -> Weight { XcmGeneric::::report_error() } - fn deposit_asset(assets: &MultiAssetFilter, _dest: &MultiLocation) -> Weight { - // Hardcoded till the XCM pallet is fixed - let hardcoded_weight = Weight::from_parts(1_000_000_000_u64, 0); - let weight = assets.weigh_multi_assets(XcmFungibleWeight::::deposit_asset()); - hardcoded_weight.min(weight) + assets.weigh_multi_assets(XcmFungibleWeight::::deposit_asset()) } fn deposit_reserve_asset( assets: &MultiAssetFilter, diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/xcm/pallet_xcm_benchmarks_fungible.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/xcm/pallet_xcm_benchmarks_fungible.rs index 9d95048c0ac2..926d7aeb9e15 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/xcm/pallet_xcm_benchmarks_fungible.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/xcm/pallet_xcm_benchmarks_fungible.rs @@ -17,28 +17,26 @@ //! Autogenerated weights for `pallet_xcm_benchmarks::fungible` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2023-07-31, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2023-10-25, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `runner-ynta1nyy-project-238-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` -//! EXECUTION: , WASM-EXECUTION: Compiled, CHAIN: Some("asset-hub-rococo-dev"), DB CACHE: 1024 +//! HOSTNAME: `runner-ayothjw6-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` +//! WASM-EXECUTION: Compiled, CHAIN: Some("asset-hub-rococo-dev"), DB CACHE: 1024 // Executed Command: -// ./target/production/polkadot-parachain +// target/production/polkadot-parachain // benchmark // pallet -// --template=./templates/xcm-bench-template.hbs -// --chain=asset-hub-rococo-dev -// --wasm-execution=compiled -// --pallet=pallet_xcm_benchmarks::fungible -// --no-storage-info -// --no-median-slopes -// --no-min-squares -// --extrinsic=* // --steps=50 // --repeat=20 -// --json -// --header=./file_header.txt -// --output=./parachains/runtimes/assets/asset-hub-rococo/src/weights/xcm/pallet_xcm_benchmarks_fungible.rs +// --extrinsic=* +// --wasm-execution=compiled +// --heap-pages=4096 +// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json +// --pallet=pallet_xcm_benchmarks::fungible +// --chain=asset-hub-rococo-dev +// --header=./cumulus/file_header.txt +// --template=./cumulus/templates/xcm-bench-template.hbs +// --output=./cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/xcm/ #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -56,8 +54,8 @@ impl WeightInfo { // Proof Size summary in bytes: // Measured: `101` // Estimated: `3593` - // Minimum execution time: 26_104_000 picoseconds. - Weight::from_parts(26_722_000, 3593) + // Minimum execution time: 20_408_000 picoseconds. + Weight::from_parts(21_066_000, 3593) .saturating_add(T::DbWeight::get().reads(1)) .saturating_add(T::DbWeight::get().writes(1)) } @@ -67,15 +65,19 @@ impl WeightInfo { // Proof Size summary in bytes: // Measured: `101` // Estimated: `6196` - // Minimum execution time: 52_259_000 picoseconds. - Weight::from_parts(53_854_000, 6196) + // Minimum execution time: 42_449_000 picoseconds. + Weight::from_parts(43_065_000, 6196) .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(2)) } - // Storage: `System::Account` (r:2 w:2) + // Storage: `System::Account` (r:3 w:3) // Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) + // Storage: UNKNOWN KEY `0x48297505634037ef48c848c99c0b1f1b` (r:1 w:0) + // Proof: UNKNOWN KEY `0x48297505634037ef48c848c99c0b1f1b` (r:1 w:0) // Storage: `ParachainInfo::ParachainId` (r:1 w:0) // Proof: `ParachainInfo::ParachainId` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + // Storage: `ParachainSystem::UpwardDeliveryFeeFactor` (r:1 w:0) + // Proof: `ParachainSystem::UpwardDeliveryFeeFactor` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) // Storage: `PolkadotXcm::SupportedVersion` (r:1 w:0) // Proof: `PolkadotXcm::SupportedVersion` (`max_values`: None, `max_size`: None, mode: `Measured`) // Storage: `PolkadotXcm::VersionDiscoveryQueue` (r:1 w:1) @@ -88,49 +90,58 @@ impl WeightInfo { // Proof: `ParachainSystem::PendingUpwardMessages` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) pub fn transfer_reserve_asset() -> Weight { // Proof Size summary in bytes: - // Measured: `210` - // Estimated: `6196` - // Minimum execution time: 77_248_000 picoseconds. - Weight::from_parts(80_354_000, 6196) - .saturating_add(T::DbWeight::get().reads(8)) - .saturating_add(T::DbWeight::get().writes(4)) + // Measured: `246` + // Estimated: `8799` + // Minimum execution time: 85_637_000 picoseconds. + Weight::from_parts(86_550_000, 8799) + .saturating_add(T::DbWeight::get().reads(11)) + .saturating_add(T::DbWeight::get().writes(5)) } - // Storage: `Benchmark::Override` (r:0 w:0) - // Proof: `Benchmark::Override` (`max_values`: None, `max_size`: None, mode: `Measured`) + // Storage: UNKNOWN KEY `0x48297505634037ef48c848c99c0b1f1b` (r:1 w:0) + // Proof: UNKNOWN KEY `0x48297505634037ef48c848c99c0b1f1b` (r:1 w:0) + // Storage: `ParachainInfo::ParachainId` (r:1 w:0) + // Proof: `ParachainInfo::ParachainId` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) pub fn reserve_asset_deposited() -> Weight { // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 500_000_000_000 picoseconds. - Weight::from_parts(500_000_000_000, 0) + // Measured: `39` + // Estimated: `3504` + // Minimum execution time: 7_006_000 picoseconds. + Weight::from_parts(7_185_000, 3504) + .saturating_add(T::DbWeight::get().reads(2)) } + // Storage: UNKNOWN KEY `0x48297505634037ef48c848c99c0b1f1b` (r:1 w:0) + // Proof: UNKNOWN KEY `0x48297505634037ef48c848c99c0b1f1b` (r:1 w:0) // Storage: `ParachainInfo::ParachainId` (r:1 w:0) // Proof: `ParachainInfo::ParachainId` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + // Storage: `ParachainSystem::UpwardDeliveryFeeFactor` (r:1 w:0) + // Proof: `ParachainSystem::UpwardDeliveryFeeFactor` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) // Storage: `PolkadotXcm::SupportedVersion` (r:1 w:0) // Proof: `PolkadotXcm::SupportedVersion` (`max_values`: None, `max_size`: None, mode: `Measured`) // Storage: `PolkadotXcm::VersionDiscoveryQueue` (r:1 w:1) // Proof: `PolkadotXcm::VersionDiscoveryQueue` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) // Storage: `PolkadotXcm::SafeXcmVersion` (r:1 w:0) // Proof: `PolkadotXcm::SafeXcmVersion` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + // Storage: `System::Account` (r:2 w:2) + // Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) // Storage: `ParachainSystem::HostConfiguration` (r:1 w:0) // Proof: `ParachainSystem::HostConfiguration` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) // Storage: `ParachainSystem::PendingUpwardMessages` (r:1 w:1) // Proof: `ParachainSystem::PendingUpwardMessages` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) pub fn initiate_reserve_withdraw() -> Weight { // Proof Size summary in bytes: - // Measured: `109` - // Estimated: `3574` - // Minimum execution time: 482_070_000 picoseconds. - Weight::from_parts(490_269_000, 3574) - .saturating_add(T::DbWeight::get().reads(6)) - .saturating_add(T::DbWeight::get().writes(2)) + // Measured: `246` + // Estimated: `6196` + // Minimum execution time: 185_307_000 picoseconds. + Weight::from_parts(189_716_000, 6196) + .saturating_add(T::DbWeight::get().reads(10)) + .saturating_add(T::DbWeight::get().writes(4)) } pub fn receive_teleported_asset() -> Weight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 3_970_000 picoseconds. - Weight::from_parts(4_056_000, 0) + // Minimum execution time: 2_934_000 picoseconds. + Weight::from_parts(3_078_000, 0) } // Storage: `System::Account` (r:1 w:1) // Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) @@ -138,15 +149,19 @@ impl WeightInfo { // Proof Size summary in bytes: // Measured: `0` // Estimated: `3593` - // Minimum execution time: 26_324_000 picoseconds. - Weight::from_parts(26_985_000, 3593) + // Minimum execution time: 18_701_000 picoseconds. + Weight::from_parts(19_221_000, 3593) .saturating_add(T::DbWeight::get().reads(1)) .saturating_add(T::DbWeight::get().writes(1)) } - // Storage: `System::Account` (r:1 w:1) + // Storage: `System::Account` (r:2 w:2) // Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) + // Storage: UNKNOWN KEY `0x48297505634037ef48c848c99c0b1f1b` (r:1 w:0) + // Proof: UNKNOWN KEY `0x48297505634037ef48c848c99c0b1f1b` (r:1 w:0) // Storage: `ParachainInfo::ParachainId` (r:1 w:0) // Proof: `ParachainInfo::ParachainId` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + // Storage: `ParachainSystem::UpwardDeliveryFeeFactor` (r:1 w:0) + // Proof: `ParachainSystem::UpwardDeliveryFeeFactor` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) // Storage: `PolkadotXcm::SupportedVersion` (r:1 w:0) // Proof: `PolkadotXcm::SupportedVersion` (`max_values`: None, `max_size`: None, mode: `Measured`) // Storage: `PolkadotXcm::VersionDiscoveryQueue` (r:1 w:1) @@ -159,32 +174,38 @@ impl WeightInfo { // Proof: `ParachainSystem::PendingUpwardMessages` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) pub fn deposit_reserve_asset() -> Weight { // Proof Size summary in bytes: - // Measured: `109` - // Estimated: `3593` - // Minimum execution time: 52_814_000 picoseconds. - Weight::from_parts(54_666_000, 3593) - .saturating_add(T::DbWeight::get().reads(7)) - .saturating_add(T::DbWeight::get().writes(3)) + // Measured: `145` + // Estimated: `6196` + // Minimum execution time: 57_182_000 picoseconds. + Weight::from_parts(58_877_000, 6196) + .saturating_add(T::DbWeight::get().reads(10)) + .saturating_add(T::DbWeight::get().writes(4)) } + // Storage: UNKNOWN KEY `0x48297505634037ef48c848c99c0b1f1b` (r:1 w:0) + // Proof: UNKNOWN KEY `0x48297505634037ef48c848c99c0b1f1b` (r:1 w:0) // Storage: `ParachainInfo::ParachainId` (r:1 w:0) // Proof: `ParachainInfo::ParachainId` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + // Storage: `ParachainSystem::UpwardDeliveryFeeFactor` (r:1 w:0) + // Proof: `ParachainSystem::UpwardDeliveryFeeFactor` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) // Storage: `PolkadotXcm::SupportedVersion` (r:1 w:0) // Proof: `PolkadotXcm::SupportedVersion` (`max_values`: None, `max_size`: None, mode: `Measured`) // Storage: `PolkadotXcm::VersionDiscoveryQueue` (r:1 w:1) // Proof: `PolkadotXcm::VersionDiscoveryQueue` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) // Storage: `PolkadotXcm::SafeXcmVersion` (r:1 w:0) // Proof: `PolkadotXcm::SafeXcmVersion` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) + // Storage: `System::Account` (r:1 w:1) + // Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) // Storage: `ParachainSystem::HostConfiguration` (r:1 w:0) // Proof: `ParachainSystem::HostConfiguration` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) // Storage: `ParachainSystem::PendingUpwardMessages` (r:1 w:1) // Proof: `ParachainSystem::PendingUpwardMessages` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) pub fn initiate_teleport() -> Weight { // Proof Size summary in bytes: - // Measured: `109` - // Estimated: `3574` - // Minimum execution time: 33_044_000 picoseconds. - Weight::from_parts(33_849_000, 3574) - .saturating_add(T::DbWeight::get().reads(6)) - .saturating_add(T::DbWeight::get().writes(2)) + // Measured: `145` + // Estimated: `3610` + // Minimum execution time: 45_073_000 picoseconds. + Weight::from_parts(45_927_000, 3610) + .saturating_add(T::DbWeight::get().reads(9)) + .saturating_add(T::DbWeight::get().writes(3)) } } diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs index ae4a275d43ac..222e96d59d17 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs @@ -40,7 +40,7 @@ use parachains_common::{ }; use polkadot_parachain_primitives::primitives::Sibling; use polkadot_runtime_common::xcm_sender::ExponentialPrice; -use rococo_runtime_constants::system_parachain::SystemParachains; +use rococo_runtime_constants::system_parachain; use sp_runtime::traits::{AccountIdConversion, ConvertInto}; use xcm::latest::prelude::*; use xcm_builder::{ @@ -76,6 +76,7 @@ parameter_types! { pub CheckingAccount: AccountId = PolkadotXcm::check_account(); pub const GovernanceLocation: MultiLocation = MultiLocation::parent(); pub TreasuryAccount: Option = Some(TREASURY_PALLET_ID.into_account_truncating()); + pub RelayTreasuryLocation: MultiLocation = (Parent, PalletInstance(rococo_runtime_constants::TREASURY_PALLET_ID)).into(); } /// Adapter for resolving `NetworkId` based on `pub storage Flavor: RuntimeFlavor`. @@ -497,10 +498,11 @@ pub type Barrier = TrailingSetTopicAsId< // If the message is one that immediately attempts to pay for execution, then // allow it. AllowTopLevelPaidExecutionFrom, - // Parent, its pluralities (i.e. governance bodies) and BridgeHub get free - // execution. + // Parent, its pluralities (i.e. governance bodies), relay treasury pallet and + // BridgeHub get free execution. AllowExplicitUnpaidExecutionFrom<( ParentOrParentsPlurality, + Equals, Equals, )>, // Subscriptions for version tracking are OK. @@ -530,22 +532,25 @@ pub type ForeignAssetFeeAsExistentialDepositMultiplierFeeCharger = ForeignAssetsInstance, >; -parameter_types! { - pub RelayTreasuryLocation: MultiLocation = (Parent, PalletInstance(rococo_runtime_constants::TREASURY_PALLET_ID)).into(); -} - -pub struct RelayTreasury; -impl Contains for RelayTreasury { - fn contains(location: &MultiLocation) -> bool { - let relay_treasury_location = RelayTreasuryLocation::get(); - *location == relay_treasury_location - } +match_types! { + pub type SystemParachains: impl Contains = { + MultiLocation { + parents: 1, + interior: X1(Parachain( + system_parachain::ASSET_HUB_ID | + system_parachain::BRIDGE_HUB_ID | + system_parachain::CONTRACTS_ID | + system_parachain::ENCOINTER_ID + )), + } + }; } /// Locations that will not be charged fees in the executor, /// either execution or delivery. /// We only waive fees for system functions, which these locations represent. -pub type WaivedLocations = (RelayOrOtherSystemParachains, RelayTreasury); +pub type WaivedLocations = + (RelayOrOtherSystemParachains, Equals); /// Cases where a remote origin is accepted as trusted Teleporter for a given asset: /// diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs index f14b410327c9..fe0fd613d220 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs @@ -26,7 +26,7 @@ use assets_common::{ }; use frame_support::{ match_types, parameter_types, - traits::{ConstU32, Contains, Everything, Nothing, PalletInfoAccess}, + traits::{ConstU32, Contains, Equals, Everything, Nothing, PalletInfoAccess}, }; use frame_system::EnsureRoot; use pallet_xcm::XcmPassthrough; @@ -74,6 +74,7 @@ parameter_types! { PalletInstance(::index() as u8).into(); pub CheckingAccount: AccountId = PolkadotXcm::check_account(); pub TreasuryAccount: Option = Some(TREASURY_PALLET_ID.into_account_truncating()); + pub RelayTreasuryLocation: MultiLocation = (Parent, PalletInstance(westend_runtime_constants::TREASURY_PALLET_ID)).into(); } /// Type for specifying how a `MultiLocation` can be converted into an `AccountId`. This is used @@ -157,7 +158,7 @@ pub type ForeignFungiblesTransactor = FungiblesAdapter< CheckingAccount, >; -/// `AssetId/Balance` converter for `PoolAssets` +/// `AssetId`/`Balance` converter for `PoolAssets`. pub type PoolAssetsConvertedConcreteId = assets_common::PoolAssetsConvertedConcreteId; @@ -236,9 +237,6 @@ match_types! { MultiLocation { parents: 1, interior: Here } | MultiLocation { parents: 1, interior: X1(Plurality { .. }) } }; - pub type TreasuryPallet: impl Contains = { - MultiLocation { parents: 1, interior: X1(PalletInstance(37)) } - }; } /// A call filter for the XCM Transact instruction. This is a temporary measure until we properly @@ -466,9 +464,12 @@ pub type Barrier = TrailingSetTopicAsId< // If the message is one that immediately attempts to pay for execution, then // allow it. AllowTopLevelPaidExecutionFrom, - // Parent, its pluralities (i.e. governance bodies) and treasury pallet get - // free execution. - AllowExplicitUnpaidExecutionFrom<(ParentOrParentsPlurality, TreasuryPallet)>, + // Parent, its pluralities (i.e. governance bodies), relay treasury pallet and + // BridgeHub get free execution. + AllowExplicitUnpaidExecutionFrom<( + ParentOrParentsPlurality, + Equals, + )>, // Subscriptions for version tracking are OK. AllowSubscriptionsFrom, ), @@ -504,22 +505,11 @@ match_types! { }; } -parameter_types! { - pub RelayTreasuryLocation: MultiLocation = (Parent, PalletInstance(westend_runtime_constants::TREASURY_PALLET_ID)).into(); -} - -pub struct RelayTreasury; -impl Contains for RelayTreasury { - fn contains(location: &MultiLocation) -> bool { - let relay_treasury_location = RelayTreasuryLocation::get(); - *location == relay_treasury_location - } -} - /// Locations that will not be charged fees in the executor, /// either execution or delivery. /// We only waive fees for system functions, which these locations represent. -pub type WaivedLocations = (RelayOrOtherSystemParachains, RelayTreasury); +pub type WaivedLocations = + (RelayOrOtherSystemParachains, Equals); /// Cases where a remote origin is accepted as trusted Teleporter for a given asset: /// @@ -550,6 +540,8 @@ impl xcm_executor::Config for XcmConfig { >; type Trader = ( UsingComponents>, + // This trader allows to pay with `is_sufficient=true` "Trust Backed" assets from dedicated + // `pallet_assets` instance - `Assets`. cumulus_primitives_utility::TakeFirstAssetTrader< AccountId, AssetFeeAsExistentialDepositMultiplierFeeCharger, diff --git a/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs b/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs index b0616acb1a4d..5fb34e7a571f 100644 --- a/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs +++ b/cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs @@ -93,6 +93,7 @@ pub fn teleports_for_native_asset_works< .with_session_keys(collator_session_keys.session_keys()) .with_safe_xcm_version(XCM_VERSION) .with_para_id(runtime_para_id.into()) + .with_tracing() .build() .execute_with(|| { let mut alice = [0u8; 32]; @@ -111,7 +112,7 @@ pub fn teleports_for_native_asset_works< let native_asset_id = MultiLocation::parent(); let buy_execution_fee_amount_eta = - WeightToFee::weight_to_fee(&Weight::from_parts(90_000_000_000, 0)); + WeightToFee::weight_to_fee(&Weight::from_parts(90_000_000_000, 1024)); let native_asset_amount_unit = existential_deposit; let native_asset_amount_received = native_asset_amount_unit * 10.into() + buy_execution_fee_amount_eta.into(); @@ -128,7 +129,7 @@ pub fn teleports_for_native_asset_works< id: Concrete(native_asset_id), fun: Fungible(buy_execution_fee_amount_eta), }, - weight_limit: Limited(Weight::from_parts(303531000, 65536)), + weight_limit: Limited(Weight::from_parts(3035310000, 65536)), }, DepositAsset { assets: Wild(AllCounted(1)), diff --git a/cumulus/parachains/runtimes/assets/test-utils/src/test_cases_over_bridge.rs b/cumulus/parachains/runtimes/assets/test-utils/src/test_cases_over_bridge.rs index 9852453d283b..8cc5f81f4972 100644 --- a/cumulus/parachains/runtimes/assets/test-utils/src/test_cases_over_bridge.rs +++ b/cumulus/parachains/runtimes/assets/test-utils/src/test_cases_over_bridge.rs @@ -182,6 +182,14 @@ pub fn limited_reserve_transfer_assets_for_native_asset_works< weight_limit, )); + // check pallet_xcm attempted + RuntimeHelper::::assert_pallet_xcm_event_outcome( + &unwrap_pallet_xcm_event, + |outcome| { + assert_ok!(outcome.ensure_complete()); + }, + ); + // check alice account decreased by balance_to_transfer // TODO:check-parameter: change and assert in tests when (https://github.com/paritytech/polkadot-sdk/pull/1234) merged assert_eq!( @@ -196,15 +204,6 @@ pub fn limited_reserve_transfer_assets_for_native_asset_works< existential_deposit + balance_to_transfer.into() ); - // check events - // check pallet_xcm attempted - RuntimeHelper::::assert_pallet_xcm_event_outcome( - &unwrap_pallet_xcm_event, - |outcome| { - assert_ok!(outcome.ensure_complete()); - }, - ); - // check that xcm was sent let xcm_sent_message_hash = >::events() .into_iter() diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs index 703acfa44174..2456a7ee63a8 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs @@ -29,7 +29,7 @@ use crate::{ }; use frame_support::{ match_types, parameter_types, - traits::{ConstU32, Contains, Everything, Nothing}, + traits::{ConstU32, Contains, Equals, Everything, Nothing}, }; use frame_system::EnsureRoot; use pallet_xcm::XcmPassthrough; @@ -40,7 +40,7 @@ use parachains_common::{ }; use polkadot_parachain_primitives::primitives::Sibling; use polkadot_runtime_common::xcm_sender::ExponentialPrice; -use rococo_runtime_constants::system_parachain::SystemParachains; +use rococo_runtime_constants::system_parachain; use sp_core::Get; use sp_runtime::traits::AccountIdConversion; use xcm::latest::prelude::*; @@ -67,6 +67,7 @@ parameter_types! { pub const MaxInstructions: u32 = 100; pub const MaxAssetsIntoHolding: u32 = 64; pub TreasuryAccount: Option = Some(TREASURY_PALLET_ID.into_account_truncating()); + pub RelayTreasuryLocation: MultiLocation = (Parent, PalletInstance(rococo_runtime_constants::TREASURY_PALLET_ID)).into(); } /// Adapter for resolving `NetworkId` based on `pub storage Flavor: RuntimeFlavor`. @@ -222,8 +223,12 @@ pub type Barrier = TrailingSetTopicAsId< // If the message is one that immediately attempts to pay for execution, then // allow it. AllowTopLevelPaidExecutionFrom, - // Parent and its pluralities (i.e. governance bodies) get free execution. - AllowExplicitUnpaidExecutionFrom, + // Parent, its pluralities (i.e. governance bodies) and relay treasury pallet + // get free execution. + AllowExplicitUnpaidExecutionFrom<( + ParentOrParentsPlurality, + Equals, + )>, // Subscriptions for version tracking are OK. AllowSubscriptionsFrom, ), @@ -234,22 +239,25 @@ pub type Barrier = TrailingSetTopicAsId< >, >; -parameter_types! { - pub RelayTreasuryLocation: MultiLocation = (Parent, PalletInstance(rococo_runtime_constants::TREASURY_PALLET_ID)).into(); -} - -pub struct RelayTreasury; -impl Contains for RelayTreasury { - fn contains(location: &MultiLocation) -> bool { - let relay_treasury_location = RelayTreasuryLocation::get(); - *location == relay_treasury_location - } +match_types! { + pub type SystemParachains: impl Contains = { + MultiLocation { + parents: 1, + interior: X1(Parachain( + system_parachain::ASSET_HUB_ID | + system_parachain::BRIDGE_HUB_ID | + system_parachain::CONTRACTS_ID | + system_parachain::ENCOINTER_ID + )), + } + }; } /// Locations that will not be charged fees in the executor, /// either execution or delivery. /// We only waive fees for system functions, which these locations represent. -pub type WaivedLocations = (RelayOrOtherSystemParachains, RelayTreasury); +pub type WaivedLocations = + (RelayOrOtherSystemParachains, Equals); /// Cases where a remote origin is accepted as trusted Teleporter for a given asset: /// - NativeToken with the parent Relay Chain and sibling parachains. diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs index 71a789e3e25a..6ff60b958fed 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs @@ -20,7 +20,7 @@ use super::{ use crate::common::rococo::currency::CENTS; use frame_support::{ match_types, parameter_types, - traits::{ConstU32, EitherOfDiverse, Everything, Nothing}, + traits::{ConstU32, EitherOfDiverse, Equals, Everything, Nothing}, weights::Weight, }; use frame_system::EnsureRoot; @@ -31,7 +31,7 @@ use parachains_common::{ }; use polkadot_parachain_primitives::primitives::Sibling; use polkadot_runtime_common::xcm_sender::ExponentialPrice; -use rococo_runtime_constants::system_parachain::SystemParachains; +use rococo_runtime_constants::system_parachain; use sp_runtime::traits::AccountIdConversion; use xcm::latest::prelude::*; use xcm_builder::{ @@ -52,6 +52,7 @@ parameter_types! { pub UniversalLocation: InteriorMultiLocation = Parachain(ParachainInfo::parachain_id().into()).into(); pub const ExecutiveBody: BodyId = BodyId::Executive; pub TreasuryAccount: Option = Some(TREASURY_PALLET_ID.into_account_truncating()); + pub RelayTreasuryLocation: MultiLocation = (Parent, PalletInstance(rococo_runtime_constants::TREASURY_PALLET_ID)).into(); } /// We allow root and the Relay Chain council to execute privileged collator selection operations. @@ -140,8 +141,12 @@ pub type Barrier = TrailingSetTopicAsId< // If the message is one that immediately attempts to pay for execution, then // allow it. AllowTopLevelPaidExecutionFrom, - // Parent and its pluralities (i.e. governance bodies) get free execution. - AllowExplicitUnpaidExecutionFrom, + // Parent, its pluralities (i.e. governance bodies) and relay treasury pallet + // get free execution. + AllowExplicitUnpaidExecutionFrom<( + ParentOrParentsPlurality, + Equals, + )>, // Subscriptions for version tracking are OK. AllowSubscriptionsFrom, ), @@ -152,6 +157,26 @@ pub type Barrier = TrailingSetTopicAsId< >, >; +match_types! { + pub type SystemParachains: impl Contains = { + MultiLocation { + parents: 1, + interior: X1(Parachain( + system_parachain::ASSET_HUB_ID | + system_parachain::BRIDGE_HUB_ID | + system_parachain::CONTRACTS_ID | + system_parachain::ENCOINTER_ID + )), + } + }; +} + +/// Locations that will not be charged fees in the executor, +/// either execution or delivery. +/// We only waive fees for system functions, which these locations represent. +pub type WaivedLocations = + (RelayOrOtherSystemParachains, Equals); + pub type TrustedTeleporter = ConcreteAssetFromSystem; pub struct XcmConfig; @@ -174,12 +199,7 @@ impl xcm_executor::Config for XcmConfig { type MaxAssetsIntoHolding = ConstU32<8>; type AssetLocker = (); type AssetExchanger = (); - type FeeManager = XcmFeesToAccount< - Self, - RelayOrOtherSystemParachains, - AccountId, - TreasuryAccount, - >; + type FeeManager = XcmFeesToAccount; type MessageExporter = (); type UniversalAliases = Nothing; type CallDispatcher = RuntimeCall; From bdf186870dc4a7d74d59cad338baf8478d0715b4 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Wed, 25 Oct 2023 16:32:18 -0400 Subject: [PATCH 11/13] `polkadot-parachain-primitives` should not depend on `frame-support`. (#1897) This PR does not make any functional changes to the code. Rather, it restructures the dependency graph. Before this PR, the crate `polkadot-parachain-primitives` depended directly on the crate `frame-support`. This is wrong in principal because a parachain does not necessarily have anything to do with frame. This dependency was only for the `Weight` type which was just a re-export from `sp-weights` anyway. So this PR changes the dependency to be directly on the much lighter `sp-weights`. --------- Co-authored-by: Joshy Orndorff Co-authored-by: command-bot <> --- Cargo.lock | 10 +++++----- polkadot/parachain/Cargo.toml | 9 +++------ polkadot/parachain/src/primitives.rs | 2 +- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d1a1b1877a92..7208b77e1098 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12625,7 +12625,6 @@ version = "1.0.0" dependencies = [ "bounded-collections", "derive_more", - "frame-support", "parity-scale-codec", "polkadot-core-primitives", "scale-info", @@ -12633,6 +12632,7 @@ dependencies = [ "sp-core", "sp-runtime", "sp-std", + "sp-weights", ] [[package]] @@ -13440,9 +13440,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.68" +version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b1106fec09662ec6dd98ccac0f81cef56984d0b49f75c92d8cbad76e20c005c" +checksum = "134c189feb4956b20f6f547d2cf727d4c0fe06722b20a0eec87ed445a97f92da" dependencies = [ "unicode-ident", ] @@ -13639,9 +13639,9 @@ dependencies = [ [[package]] name = "quinn-proto" -version = "0.9.4" +version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f31999cfc7927c4e212e60fd50934ab40e8e8bfd2d493d6095d2d306bc0764d9" +checksum = "c956be1b23f4261676aed05a0046e204e8a6836e50203902683a718af0797989" dependencies = [ "bytes", "rand 0.8.5", diff --git a/polkadot/parachain/Cargo.toml b/polkadot/parachain/Cargo.toml index 681bee3c3273..27aa117a87f0 100644 --- a/polkadot/parachain/Cargo.toml +++ b/polkadot/parachain/Cargo.toml @@ -15,7 +15,7 @@ scale-info = { version = "2.10.0", default-features = false, features = ["derive sp-std = { path = "../../substrate/primitives/std", default-features = false } sp-runtime = { path = "../../substrate/primitives/runtime", default-features = false, features = ["serde"] } sp-core = { path = "../../substrate/primitives/core", default-features = false, features = ["serde"] } -frame-support = { path = "../../substrate/frame/support", default-features = false } +sp-weights = { path = "../../substrate/primitives/weights", default-features = false } polkadot-core-primitives = { path = "../core-primitives", default-features = false } derive_more = "0.99.11" bounded-collections = { version = "0.1.8", default-features = false, features = ["serde"] } @@ -28,7 +28,6 @@ default = [ "std" ] wasm-api = [] std = [ "bounded-collections/std", - "frame-support/std", "parity-scale-codec/std", "polkadot-core-primitives/std", "scale-info/std", @@ -36,8 +35,6 @@ std = [ "sp-core/std", "sp-runtime/std", "sp-std/std", + "sp-weights/std", ] -runtime-benchmarks = [ - "frame-support/runtime-benchmarks", - "sp-runtime/runtime-benchmarks", -] +runtime-benchmarks = [ "sp-runtime/runtime-benchmarks" ] diff --git a/polkadot/parachain/src/primitives.rs b/polkadot/parachain/src/primitives.rs index 3247e841422e..dc89ae7868e1 100644 --- a/polkadot/parachain/src/primitives.rs +++ b/polkadot/parachain/src/primitives.rs @@ -20,12 +20,12 @@ use sp_std::vec::Vec; use bounded_collections::{BoundedVec, ConstU32}; -use frame_support::weights::Weight; use parity_scale_codec::{CompactAs, Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; use serde::{Deserialize, Serialize}; use sp_core::{bytes, RuntimeDebug, TypeId}; use sp_runtime::traits::Hash as _; +use sp_weights::Weight; use polkadot_core_primitives::{Hash, OutboundHrmpMessage}; From 0bcebac4fc69f6bef5da7d94615916bbde294967 Mon Sep 17 00:00:00 2001 From: Dastan <88332432+dastansam@users.noreply.github.com> Date: Thu, 26 Oct 2023 15:52:12 +0600 Subject: [PATCH 12/13] Expose collection attributes from `Inspect` trait (#1914) # Description - What does this PR do? While working with `pallet_nfts` through `nonfungibles_v2` traits `Inspect, Mutate`, I found out that once you have set the collection attribute with `::set_collection_attribute()`, it's not possible to read it with `::collection_attribute()` since they use different `namespace` values. When setting the attribute, `AttributeNamespace::Pallet` is used, while `AttributeNamespace::CollectionOwner` is used when reading. more context: https://github.com/freeverseio/laos/issues/7#issuecomment-1766137370 This PR makes `item` an optional parameter in `Inspect::system_attribute()`, to be able to read collection attributes. - Why are these changes needed? To be able to read collection level attributes when reading attributes of the collection. It will be possible to read collection attributes by passing `None` for `item` - How were these changes implemented and what do they affect? `NftsApi` is also affected and `NftsApi::system_attribute()` now accepts optional `item` parameter. ## Breaking change Because of the change in the `NftsApi::system_attribute()` method's `item` param, parachains who integrated the `NftsApi` need to update their API code and frontend integrations accordingly. AssetHubs are unaffected since the NftsApi wasn't released on those parachains yet. --- .../assets/asset-hub-westend/src/lib.rs | 4 +- substrate/bin/node/runtime/src/lib.rs | 4 +- substrate/frame/nfts/runtime-api/src/lib.rs | 2 +- substrate/frame/nfts/src/impl_nonfungibles.rs | 8 +- substrate/frame/nfts/src/tests.rs | 82 ++++++++++++++++++- .../src/traits/tokens/nonfungible_v2.rs | 4 +- .../src/traits/tokens/nonfungibles_v2.rs | 13 +-- 7 files changed, 101 insertions(+), 16 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index c090536b3da6..30d384222422 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -1081,10 +1081,10 @@ impl_runtime_apis! { fn system_attribute( collection: u32, - item: u32, + item: Option, key: Vec, ) -> Option> { - >::system_attribute(&collection, &item, &key) + >::system_attribute(&collection, item.as_ref(), &key) } fn collection_attribute(collection: u32, key: Vec) -> Option> { diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index f4c8a5940a3c..a2d100e1f8b5 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2630,10 +2630,10 @@ impl_runtime_apis! { fn system_attribute( collection: u32, - item: u32, + item: Option, key: Vec, ) -> Option> { - >::system_attribute(&collection, &item, &key) + >::system_attribute(&collection, item.as_ref(), &key) } fn collection_attribute(collection: u32, key: Vec) -> Option> { diff --git a/substrate/frame/nfts/runtime-api/src/lib.rs b/substrate/frame/nfts/runtime-api/src/lib.rs index cf2d444b42f8..77535c64069c 100644 --- a/substrate/frame/nfts/runtime-api/src/lib.rs +++ b/substrate/frame/nfts/runtime-api/src/lib.rs @@ -48,7 +48,7 @@ sp_api::decl_runtime_apis! { fn system_attribute( collection: CollectionId, - item: ItemId, + item: Option, key: Vec, ) -> Option>; diff --git a/substrate/frame/nfts/src/impl_nonfungibles.rs b/substrate/frame/nfts/src/impl_nonfungibles.rs index 4a6b70eb9973..ee7f42cfc689 100644 --- a/substrate/frame/nfts/src/impl_nonfungibles.rs +++ b/substrate/frame/nfts/src/impl_nonfungibles.rs @@ -79,17 +79,19 @@ impl, I: 'static> Inspect<::AccountId> for Palle Attribute::::get((collection, Some(item), namespace, key)).map(|a| a.0.into()) } - /// Returns the system attribute value of `item` of `collection` corresponding to `key`. + /// Returns the system attribute value of `item` of `collection` corresponding to `key` if + /// `item` is `Some`. Otherwise, returns the system attribute value of `collection` + /// corresponding to `key`. /// /// By default this is `None`; no attributes are defined. fn system_attribute( collection: &Self::CollectionId, - item: &Self::ItemId, + item: Option<&Self::ItemId>, key: &[u8], ) -> Option> { let namespace = AttributeNamespace::Pallet; let key = BoundedSlice::<_, _>::try_from(key).ok()?; - Attribute::::get((collection, Some(item), namespace, key)).map(|a| a.0.into()) + Attribute::::get((collection, item, namespace, key)).map(|a| a.0.into()) } /// Returns the attribute value of `item` of `collection` corresponding to `key`. diff --git a/substrate/frame/nfts/src/tests.rs b/substrate/frame/nfts/src/tests.rs index a82fcca01512..aeebf51b7c78 100644 --- a/substrate/frame/nfts/src/tests.rs +++ b/substrate/frame/nfts/src/tests.rs @@ -22,7 +22,7 @@ use enumflags2::BitFlags; use frame_support::{ assert_noop, assert_ok, traits::{ - tokens::nonfungibles_v2::{Create, Destroy, Mutate}, + tokens::nonfungibles_v2::{Create, Destroy, Inspect, Mutate}, Currency, Get, }, }; @@ -982,6 +982,86 @@ fn set_collection_owner_attributes_should_work() { }); } +#[test] +fn set_collection_system_attributes_should_work() { + new_test_ext().execute_with(|| { + Balances::make_free_balance_be(&account(1), 100); + + assert_ok!(Nfts::force_create( + RuntimeOrigin::root(), + account(1), + collection_config_with_all_settings_enabled() + )); + assert_ok!(Nfts::mint(RuntimeOrigin::signed(account(1)), 0, 0, account(1), None)); + + let collection_id = 0; + let attribute_key = [0u8]; + let attribute_value = [0u8]; + + assert_ok!(, ItemConfig>>::set_collection_attribute( + &collection_id, + &attribute_key, + &attribute_value + )); + + assert_eq!(attributes(0), vec![(None, AttributeNamespace::Pallet, bvec![0], bvec![0])]); + + assert_eq!( + >>::system_attribute( + &collection_id, + None, + &attribute_key + ), + Some(attribute_value.to_vec()) + ); + + // test typed system attribute + let typed_attribute_key = [0u8; 32]; + #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] + struct TypedAttributeValue(u32); + let typed_attribute_value = TypedAttributeValue(42); + + assert_ok!( + , ItemConfig>>::set_typed_collection_attribute( + &collection_id, + &typed_attribute_key, + &typed_attribute_value + ) + ); + + assert_eq!( + >>::typed_system_attribute( + &collection_id, + None, + &typed_attribute_key + ), + Some(typed_attribute_value) + ); + + // check storage + assert_eq!( + attributes(collection_id), + [ + (None, AttributeNamespace::Pallet, bvec![0], bvec![0]), + ( + None, + AttributeNamespace::Pallet, + bvec![ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0 + ], + bvec![42, 0, 0, 0] + ) + ] + ); + + assert_ok!(Nfts::burn(RuntimeOrigin::root(), collection_id, 0)); + let w = Nfts::get_destroy_witness(&0).unwrap(); + assert_ok!(Nfts::destroy(RuntimeOrigin::signed(account(1)), collection_id, w)); + assert_eq!(attributes(collection_id), vec![]); + }) +} + #[test] fn set_item_owner_attributes_should_work() { new_test_ext().execute_with(|| { diff --git a/substrate/frame/support/src/traits/tokens/nonfungible_v2.rs b/substrate/frame/support/src/traits/tokens/nonfungible_v2.rs index 788a4d25e810..05f76e2859d2 100644 --- a/substrate/frame/support/src/traits/tokens/nonfungible_v2.rs +++ b/substrate/frame/support/src/traits/tokens/nonfungible_v2.rs @@ -226,7 +226,7 @@ impl< >::custom_attribute(account, &A::get(), item, key) } fn system_attribute(item: &Self::ItemId, key: &[u8]) -> Option> { - >::system_attribute(&A::get(), item, key) + >::system_attribute(&A::get(), Some(item), key) } fn typed_attribute(item: &Self::ItemId, key: &K) -> Option { >::typed_attribute(&A::get(), item, key) @@ -244,7 +244,7 @@ impl< ) } fn typed_system_attribute(item: &Self::ItemId, key: &K) -> Option { - >::typed_system_attribute(&A::get(), item, key) + >::typed_system_attribute(&A::get(), Some(item), key) } fn can_transfer(item: &Self::ItemId) -> bool { >::can_transfer(&A::get(), item) diff --git a/substrate/frame/support/src/traits/tokens/nonfungibles_v2.rs b/substrate/frame/support/src/traits/tokens/nonfungibles_v2.rs index 868afbdf7eee..c0209b6d5123 100644 --- a/substrate/frame/support/src/traits/tokens/nonfungibles_v2.rs +++ b/substrate/frame/support/src/traits/tokens/nonfungibles_v2.rs @@ -75,12 +75,14 @@ pub trait Inspect { None } - /// Returns the system attribute value of `item` of `collection` corresponding to `key`. + /// Returns the system attribute value of `item` of `collection` corresponding to `key` if + /// `item` is `Some`. Otherwise, returns the system attribute value of `collection` + /// corresponding to `key`. /// /// By default this is `None`; no attributes are defined. fn system_attribute( _collection: &Self::CollectionId, - _item: &Self::ItemId, + _item: Option<&Self::ItemId>, _key: &[u8], ) -> Option> { None @@ -113,13 +115,14 @@ pub trait Inspect { .and_then(|v| V::decode(&mut &v[..]).ok()) } - /// Returns the strongly-typed system attribute value of `item` of `collection` corresponding to - /// `key`. + /// Returns the strongly-typed system attribute value of `item` corresponding to `key` if + /// `item` is `Some`. Otherwise, returns the strongly-typed system attribute value of + /// `collection` corresponding to `key`. /// /// By default this just attempts to use `system_attribute`. fn typed_system_attribute( collection: &Self::CollectionId, - item: &Self::ItemId, + item: Option<&Self::ItemId>, key: &K, ) -> Option { key.using_encoded(|d| Self::system_attribute(collection, item, d)) From 21d36b7b4229c4d5225944f197918cde23fda4ea Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Thu, 26 Oct 2023 15:36:05 +0200 Subject: [PATCH 13/13] Removed TODO from test-case for hard-coded delivery fee estimation (#2042) Co-authored-by: Adrian Catangiu --- .../assets/asset-hub-rococo/tests/tests.rs | 38 +-- .../runtimes/assets/test-utils/src/lib.rs | 44 ++++ .../test-utils/src/test_cases_over_bridge.rs | 222 ++++++++++-------- 3 files changed, 164 insertions(+), 140 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs index a763382f9054..b93315cc39d8 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/tests/tests.rs @@ -683,6 +683,7 @@ mod asset_hub_rococo_tests { bridging_to_asset_hub_wococo, WeightLimit::Unlimited, Some(xcm_config::bridging::XcmBridgeHubRouterFeeAssetId::get()), + Some(xcm_config::TreasuryAccount::get().unwrap()), ) } @@ -717,29 +718,11 @@ mod asset_hub_rococo_tests { Runtime, AllPalletsWithoutSystem, XcmConfig, - ParachainSystem, - XcmpQueue, LocationToAccountId, ToWococoXcmRouterInstance, >( collator_session_keys(), - ExistentialDeposit::get(), - AccountId::from(ALICE), - Box::new(|runtime_event_encoded: Vec| { - match RuntimeEvent::decode(&mut &runtime_event_encoded[..]) { - Ok(RuntimeEvent::PolkadotXcm(event)) => Some(event), - _ => None, - } - }), - Box::new(|runtime_event_encoded: Vec| { - match RuntimeEvent::decode(&mut &runtime_event_encoded[..]) { - Ok(RuntimeEvent::XcmpQueue(event)) => Some(event), - _ => None, - } - }), bridging_to_asset_hub_wococo, - WeightLimit::Unlimited, - Some(xcm_config::bridging::XcmBridgeHubRouterFeeAssetId::get()), || { sp_std::vec![ UnpaidExecution { weight_limit: Unlimited, check_origin: None }, @@ -888,6 +871,7 @@ mod asset_hub_wococo_tests { with_wococo_flavor_bridging_to_asset_hub_rococo, WeightLimit::Unlimited, Some(xcm_config::bridging::XcmBridgeHubRouterFeeAssetId::get()), + Some(xcm_config::TreasuryAccount::get().unwrap()), ) } @@ -922,29 +906,11 @@ mod asset_hub_wococo_tests { Runtime, AllPalletsWithoutSystem, XcmConfig, - ParachainSystem, - XcmpQueue, LocationToAccountId, ToRococoXcmRouterInstance, >( collator_session_keys(), - ExistentialDeposit::get(), - AccountId::from(ALICE), - Box::new(|runtime_event_encoded: Vec| { - match RuntimeEvent::decode(&mut &runtime_event_encoded[..]) { - Ok(RuntimeEvent::PolkadotXcm(event)) => Some(event), - _ => None, - } - }), - Box::new(|runtime_event_encoded: Vec| { - match RuntimeEvent::decode(&mut &runtime_event_encoded[..]) { - Ok(RuntimeEvent::XcmpQueue(event)) => Some(event), - _ => None, - } - }), with_wococo_flavor_bridging_to_asset_hub_rococo, - WeightLimit::Unlimited, - Some(xcm_config::bridging::XcmBridgeHubRouterFeeAssetId::get()), || { sp_std::vec![ UnpaidExecution { weight_limit: Unlimited, check_origin: None }, diff --git a/cumulus/parachains/runtimes/assets/test-utils/src/lib.rs b/cumulus/parachains/runtimes/assets/test-utils/src/lib.rs index e0f05fa7b0a4..471b1f09b567 100644 --- a/cumulus/parachains/runtimes/assets/test-utils/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/test-utils/src/lib.rs @@ -19,4 +19,48 @@ pub mod test_cases; pub mod test_cases_over_bridge; pub mod xcm_helpers; + +use frame_support::traits::ProcessMessageError; pub use parachains_runtimes_test_utils::*; +use std::fmt::Debug; + +use xcm::latest::prelude::*; +use xcm_builder::{CreateMatcher, MatchXcm}; + +/// Helper function to verify `xcm` contains all relevant instructions expected on destination +/// chain as part of a reserve-asset-transfer. +pub(crate) fn assert_matches_reserve_asset_deposited_instructions( + xcm: &mut Xcm, + expected_reserve_assets_deposited: &MultiAssets, + expected_beneficiary: &MultiLocation, +) { + let _ = xcm + .0 + .matcher() + .skip_inst_while(|inst| !matches!(inst, ReserveAssetDeposited(..))) + .expect("no instruction ReserveAssetDeposited?") + .match_next_inst(|instr| match instr { + ReserveAssetDeposited(reserve_assets) => { + assert_eq!(reserve_assets, expected_reserve_assets_deposited); + Ok(()) + }, + _ => Err(ProcessMessageError::BadFormat), + }) + .expect("expected instruction ReserveAssetDeposited") + .match_next_inst(|instr| match instr { + ClearOrigin => Ok(()), + _ => Err(ProcessMessageError::BadFormat), + }) + .expect("expected instruction ClearOrigin") + .match_next_inst(|instr| match instr { + BuyExecution { .. } => Ok(()), + _ => Err(ProcessMessageError::BadFormat), + }) + .expect("expected instruction BuyExecution") + .match_next_inst(|instr| match instr { + DepositAsset { assets: _, beneficiary } if beneficiary == expected_beneficiary => + Ok(()), + _ => Err(ProcessMessageError::BadFormat), + }) + .expect("expected instruction DepositAsset"); +} diff --git a/cumulus/parachains/runtimes/assets/test-utils/src/test_cases_over_bridge.rs b/cumulus/parachains/runtimes/assets/test-utils/src/test_cases_over_bridge.rs index 8cc5f81f4972..6c8ac8c6452b 100644 --- a/cumulus/parachains/runtimes/assets/test-utils/src/test_cases_over_bridge.rs +++ b/cumulus/parachains/runtimes/assets/test-utils/src/test_cases_over_bridge.rs @@ -16,13 +16,12 @@ //! Module contains predefined test-case scenarios for `Runtime` with various assets transferred //! over a bridge. +use crate::assert_matches_reserve_asset_deposited_instructions; use codec::Encode; use cumulus_primitives_core::XcmpMessageSource; use frame_support::{ assert_ok, - traits::{ - fungible::Mutate, Currency, OnFinalize, OnInitialize, OriginTrait, ProcessMessageError, - }, + traits::{Currency, Get, OnFinalize, OnInitialize, OriginTrait, ProcessMessageError}, }; use frame_system::pallet_prelude::BlockNumberFor; use parachains_common::{AccountId, Balance}; @@ -30,10 +29,13 @@ use parachains_runtimes_test_utils::{ mock_open_hrmp_channel, AccountIdOf, BalanceOf, CollatorSessionKeys, ExtBuilder, RuntimeHelper, ValidatorIdOf, XcmReceivedFrom, }; -use sp_runtime::traits::StaticLookup; +use sp_runtime::{traits::StaticLookup, Saturating}; use xcm::{latest::prelude::*, VersionedMultiAssets}; use xcm_builder::{CreateMatcher, MatchXcm}; -use xcm_executor::{traits::ConvertLocation, XcmExecutor}; +use xcm_executor::{ + traits::{ConvertLocation, TransactAsset}, + XcmExecutor, +}; pub struct TestBridgingConfig { pub bridged_network: NetworkId, @@ -61,6 +63,7 @@ pub fn limited_reserve_transfer_assets_for_native_asset_works< prepare_configuration: fn() -> TestBridgingConfig, weight_limit: WeightLimit, maybe_paid_export_message: Option, + delivery_fees_account: Option>, ) where Runtime: frame_system::Config + pallet_balances::Config @@ -151,6 +154,11 @@ pub fn limited_reserve_transfer_assets_for_native_asset_works< existential_deposit ); + let delivery_fees_account_balance_before = delivery_fees_account + .as_ref() + .map(|dfa| >::free_balance(dfa)) + .unwrap_or(0.into()); + // local native asset (pallet_balances) let asset_to_transfer = MultiAsset { fun: Fungible(balance_to_transfer.into()), @@ -166,22 +174,76 @@ pub fn limited_reserve_transfer_assets_for_native_asset_works< }), }; - // Make sure sender has enough funds for paying delivery fees - // TODO: Get this fee via weighing the corresponding message - let delivery_fees = 1324039894; - >::mint_into(&alice_account, delivery_fees.into()) + let assets_to_transfer = MultiAssets::from(asset_to_transfer); + let mut expected_assets = assets_to_transfer.clone(); + let context = XcmConfig::UniversalLocation::get(); + expected_assets + .reanchor(&target_location_from_different_consensus, context) .unwrap(); + let expected_beneficiary = target_destination_account; + + // Make sure sender has enough funds for paying delivery fees + let handling_delivery_fees = { + // Probable XCM with `ReserveAssetDeposited`. + let mut expected_reserve_asset_deposited_message = Xcm(vec![ + ReserveAssetDeposited(MultiAssets::from(expected_assets.clone())), + ClearOrigin, + BuyExecution { + fees: MultiAsset { + id: Concrete(Default::default()), + fun: Fungible(balance_to_transfer), + }, + weight_limit: Unlimited, + }, + DepositAsset { assets: Wild(AllCounted(1)), beneficiary: expected_beneficiary }, + SetTopic([ + 220, 188, 144, 32, 213, 83, 111, 175, 44, 210, 111, 19, 90, 165, 191, 112, + 140, 247, 192, 124, 42, 17, 153, 141, 114, 34, 189, 20, 83, 69, 237, 173, + ]), + ]); + assert_matches_reserve_asset_deposited_instructions( + &mut expected_reserve_asset_deposited_message, + &expected_assets, + &expected_beneficiary, + ); + + // Call `SendXcm::validate` to get delivery fees. + let (_, delivery_fees): (_, MultiAssets) = XcmConfig::XcmSender::validate( + &mut Some(target_location_from_different_consensus), + &mut Some(expected_reserve_asset_deposited_message), + ) + .expect("validate passes"); + // Drip delivery fee to Alice account. + let mut delivery_fees_added = false; + for delivery_fee in delivery_fees.inner() { + assert_ok!(::deposit_asset( + &delivery_fee, + &MultiLocation { + parents: 0, + interior: X1(AccountId32 { + network: None, + id: alice_account.clone().into(), + }), + }, + None, + )); + delivery_fees_added = true; + } + delivery_fees_added + }; + // do pallet_xcm call reserve transfer assert_ok!(>::limited_reserve_transfer_assets( RuntimeHelper::::origin_of(alice_account.clone()), Box::new(target_location_from_different_consensus.into_versioned()), Box::new(target_destination_account.into_versioned()), - Box::new(VersionedMultiAssets::from(MultiAssets::from(asset_to_transfer))), + Box::new(VersionedMultiAssets::from(assets_to_transfer)), 0, weight_limit, )); + // check events // check pallet_xcm attempted RuntimeHelper::::assert_pallet_xcm_event_outcome( &unwrap_pallet_xcm_event, @@ -190,20 +252,6 @@ pub fn limited_reserve_transfer_assets_for_native_asset_works< }, ); - // check alice account decreased by balance_to_transfer - // TODO:check-parameter: change and assert in tests when (https://github.com/paritytech/polkadot-sdk/pull/1234) merged - assert_eq!( - >::free_balance(&alice_account), - alice_account_init_balance - balance_to_transfer.into() - ); - - // check reserve account - // check reserve account increased by balance_to_transfer - assert_eq!( - >::free_balance(&reserve_account), - existential_deposit + balance_to_transfer.into() - ); - // check that xcm was sent let xcm_sent_message_hash = >::events() .into_iter() @@ -219,7 +267,6 @@ pub fn limited_reserve_transfer_assets_for_native_asset_works< local_bridge_hub_para_id.into(), ) .unwrap(); - assert_eq!( xcm_sent_message_hash, Some(xcm_sent.using_encoded(sp_io::hashing::blake2_256)) @@ -268,12 +315,41 @@ pub fn limited_reserve_transfer_assets_for_native_asset_works< .split_global() .expect("split works"); assert_eq!(destination, &target_location_junctions_without_global_consensus); - assert_matches_pallet_xcm_reserve_transfer_assets_instructions(inner_xcm); + assert_matches_reserve_asset_deposited_instructions( + inner_xcm, + &expected_assets, + &expected_beneficiary, + ); Ok(()) }, _ => Err(ProcessMessageError::BadFormat), }) .expect("contains ExportMessage"); + + // check alice account decreased by balance_to_transfer + assert_eq!( + >::free_balance(&alice_account), + alice_account_init_balance + .saturating_sub(existential_deposit) + .saturating_sub(balance_to_transfer.into()) + ); + + // check reserve account increased by balance_to_transfer + assert_eq!( + >::free_balance(&reserve_account), + existential_deposit + balance_to_transfer.into() + ); + + // check dedicated account increased by delivery fees (if configured) + if handling_delivery_fees { + if let Some(delivery_fees_account) = delivery_fees_account { + let delivery_fees_account_balance_after = + >::free_balance(&delivery_fees_account); + assert!( + delivery_fees_account_balance_after > delivery_fees_account_balance_before + ); + } + } }) } @@ -405,15 +481,21 @@ pub fn receive_reserve_asset_deposited_from_different_consensus_works< 0.into() ); + let expected_assets = MultiAssets::from(vec![MultiAsset { + id: Concrete(foreign_asset_id_multilocation), + fun: Fungible(transfered_foreign_asset_id_amount), + }]); + let expected_beneficiary = MultiLocation { + parents: 0, + interior: X1(AccountId32 { network: None, id: target_account.clone().into() }), + }; + // Call received XCM execution let xcm = Xcm(vec![ DescendOrigin(bridge_instance), UniversalOrigin(universal_origin), DescendOrigin(descend_origin), - ReserveAssetDeposited(MultiAssets::from(vec![MultiAsset { - id: Concrete(foreign_asset_id_multilocation), - fun: Fungible(transfered_foreign_asset_id_amount), - }])), + ReserveAssetDeposited(expected_assets.clone()), ClearOrigin, BuyExecution { fees: MultiAsset { @@ -422,22 +504,17 @@ pub fn receive_reserve_asset_deposited_from_different_consensus_works< }, weight_limit: Unlimited, }, - DepositAsset { - assets: Wild(AllCounted(1)), - beneficiary: MultiLocation { - parents: 0, - interior: X1(AccountId32 { - network: None, - id: target_account.clone().into(), - }), - }, - }, + DepositAsset { assets: Wild(AllCounted(1)), beneficiary: expected_beneficiary }, SetTopic([ 220, 188, 144, 32, 213, 83, 111, 175, 44, 210, 111, 19, 90, 165, 191, 112, 140, 247, 192, 124, 42, 17, 153, 141, 114, 34, 189, 20, 83, 69, 237, 173, ]), ]); - assert_matches_pallet_xcm_reserve_transfer_assets_instructions(&mut xcm.clone()); + assert_matches_reserve_asset_deposited_instructions( + &mut xcm.clone(), + &expected_assets, + &expected_beneficiary, + ); let hash = xcm.using_encoded(sp_io::hashing::blake2_256); @@ -498,55 +575,15 @@ pub fn receive_reserve_asset_deposited_from_different_consensus_works< }) } -fn assert_matches_pallet_xcm_reserve_transfer_assets_instructions( - xcm: &mut Xcm, -) { - let _ = xcm - .0 - .matcher() - .skip_inst_while(|inst| !matches!(inst, ReserveAssetDeposited(..))) - .expect("no instruction ReserveAssetDeposited?") - .match_next_inst(|instr| match instr { - ReserveAssetDeposited(..) => Ok(()), - _ => Err(ProcessMessageError::BadFormat), - }) - .expect("expected instruction ReserveAssetDeposited") - .match_next_inst(|instr| match instr { - ClearOrigin => Ok(()), - _ => Err(ProcessMessageError::BadFormat), - }) - .expect("expected instruction ClearOrigin") - .match_next_inst(|instr| match instr { - BuyExecution { .. } => Ok(()), - _ => Err(ProcessMessageError::BadFormat), - }) - .expect("expected instruction BuyExecution") - .match_next_inst(|instr| match instr { - DepositAsset { .. } => Ok(()), - _ => Err(ProcessMessageError::BadFormat), - }) - .expect("expected instruction DepositAsset"); -} - pub fn report_bridge_status_from_xcm_bridge_router_works< Runtime, AllPalletsWithoutSystem, XcmConfig, - HrmpChannelOpener, - HrmpChannelSource, LocationToAccountId, XcmBridgeHubRouterInstance, >( collator_session_keys: CollatorSessionKeys, - existential_deposit: BalanceOf, - alice_account: AccountIdOf, - unwrap_pallet_xcm_event: Box) -> Option>>, - unwrap_xcmp_queue_event: Box< - dyn Fn(Vec) -> Option>, - >, prepare_configuration: fn() -> TestBridgingConfig, - weight_limit: WeightLimit, - maybe_paid_export_message: Option, congested_message: fn() -> Xcm, uncongested_message: fn() -> Xcm, ) where @@ -572,10 +609,6 @@ pub fn report_bridge_status_from_xcm_bridge_router_works< <::Lookup as StaticLookup>::Source: From<::AccountId>, ::AccountId: From, - HrmpChannelOpener: frame_support::inherent::ProvideInherent< - Call = cumulus_pallet_parachain_system::Call, - >, - HrmpChannelSource: XcmpMessageSource, XcmBridgeHubRouterInstance: 'static, { ExtBuilder::::default() @@ -584,25 +617,6 @@ pub fn report_bridge_status_from_xcm_bridge_router_works< .with_tracing() .build() .execute_with(|| { - // check transfer works - limited_reserve_transfer_assets_for_native_asset_works::< - Runtime, - AllPalletsWithoutSystem, - XcmConfig, - HrmpChannelOpener, - HrmpChannelSource, - LocationToAccountId, - >( - collator_session_keys, - existential_deposit, - alice_account, - unwrap_pallet_xcm_event, - unwrap_xcmp_queue_event, - prepare_configuration, - weight_limit, - maybe_paid_export_message, - ); - let report_bridge_status = |is_congested: bool| { // prepare bridge config let TestBridgingConfig { local_bridge_hub_location, .. } = prepare_configuration();