diff --git a/.cargo/config.toml b/.cargo/config.toml index 4796a2c26965c..042dded2fa9b3 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,4 +1,9 @@ -# +[build] +rustdocflags = [ + "-Dwarnings", + "-Arustdoc::redundant_explicit_links", # stylistic +] + # An auto defined `clippy` feature was introduced, # but it was found to clash with user defined features, # so was renamed to `cargo-clippy`. @@ -30,4 +35,5 @@ rustflags = [ "-Aclippy::derivable_impls", # false positives "-Aclippy::stable_sort_primitive", # prefer stable sort "-Aclippy::extra-unused-type-parameters", # stylistic + "-Aclippy::default_constructed_unit_structs", # stylistic ] diff --git a/.gitlab/pipeline/build.yml b/.gitlab/pipeline/build.yml index 029c0f6a3cddd..fefa3739a9ff4 100644 --- a/.gitlab/pipeline/build.yml +++ b/.gitlab/pipeline/build.yml @@ -91,6 +91,7 @@ build-rustdoc: - .run-immediately variables: SKIP_WASM_BUILD: 1 + RUSTDOCFLAGS: "" artifacts: name: "${CI_JOB_NAME}_${CI_COMMIT_REF_NAME}-doc" when: on_success @@ -99,7 +100,6 @@ build-rustdoc: - ./crate-docs/ script: # FIXME: it fails with `RUSTDOCFLAGS="-Dwarnings"` and `--all-features` - # FIXME: return to stable when https://github.com/rust-lang/rust/issues/96937 gets into stable - time cargo doc --features try-runtime,experimental --workspace --no-deps - rm -f ./target/doc/.lock - mv ./target/doc ./crate-docs diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml index e1e8b96bca5d6..12ce2140b1468 100644 --- a/.gitlab/pipeline/test.yml +++ b/.gitlab/pipeline/test.yml @@ -181,7 +181,6 @@ test-rustdoc: - .run-immediately variables: SKIP_WASM_BUILD: 1 - RUSTDOCFLAGS: "-Dwarnings" script: - time cargo doc --workspace --all-features --no-deps allow_failure: true diff --git a/Cargo.lock b/Cargo.lock index f8d20fde40d8d..c222079d09fb3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6875,9 +6875,9 @@ dependencies = [ [[package]] name = "landlock" -version = "0.2.0" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "520baa32708c4e957d2fc3a186bc5bd8d26637c33137f399ddfc202adb240068" +checksum = "1530c5b973eeed4ac216af7e24baf5737645a6272e361f1fb95710678b67d9cc" dependencies = [ "enumflags2", "libc", diff --git a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs index 3f4c08ecbb834..1e78df7115435 100644 --- a/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs +++ b/cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs @@ -346,6 +346,13 @@ impl RuntimeApiSubsystemClient for BlockChainRpcClient { Ok(self.rpc_client.parachain_host_minimum_backing_votes(at, session_index).await?) } + async fn disabled_validators( + &self, + at: Hash, + ) -> Result, ApiError> { + Ok(self.rpc_client.parachain_host_disabled_validators(at).await?) + } + async fn async_backing_params(&self, at: Hash) -> Result { Ok(self.rpc_client.parachain_host_async_backing_params(at).await?) } diff --git a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs index b1fd7d1ab7d9c..5924716adcb41 100644 --- a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs +++ b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs @@ -597,6 +597,14 @@ impl RelayChainRpcClient { .await } + pub async fn parachain_host_disabled_validators( + &self, + at: RelayHash, + ) -> Result, RelayChainError> { + self.call_remote_runtime_function("ParachainHost_disabled_validators", at, None::<()>) + .await + } + #[allow(missing_docs)] pub async fn parachain_host_async_backing_params( &self, diff --git a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs index c2e065ccadcdf..f8fe8831d3c74 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs @@ -33,7 +33,7 @@ use xcm_emulator::{ }; decl_test_relay_chains! { - #[api_version(7)] + #[api_version(8)] pub struct Westend { genesis = westend::genesis(), on_init = (), @@ -50,7 +50,7 @@ decl_test_relay_chains! { AssetRate: westend_runtime::AssetRate, } }, - #[api_version(7)] + #[api_version(8)] pub struct Rococo { genesis = rococo::genesis(), on_init = (), @@ -65,7 +65,7 @@ decl_test_relay_chains! { Balances: rococo_runtime::Balances, } }, - #[api_version(7)] + #[api_version(8)] pub struct Wococo { genesis = rococo::genesis(), on_init = (), diff --git a/polkadot/node/collation-generation/src/lib.rs b/polkadot/node/collation-generation/src/lib.rs index 4e13755deedfb..b8c9c1a36e46b 100644 --- a/polkadot/node/collation-generation/src/lib.rs +++ b/polkadot/node/collation-generation/src/lib.rs @@ -193,7 +193,7 @@ async fn handle_new_activations( metrics: Metrics, ) -> crate::error::Result<()> { // follow the procedure from the guide: - // https://paritytech.github.io/polkadot/book/node/collators/collation-generation.html + // https://paritytech.github.io/polkadot-sdk/book/node/collators/collation-generation.html if config.collator.is_none() { return Ok(()) diff --git a/polkadot/node/core/backing/src/tests/mod.rs b/polkadot/node/core/backing/src/tests/mod.rs index bdc8b3fa1af84..35d17f3d905e4 100644 --- a/polkadot/node/core/backing/src/tests/mod.rs +++ b/polkadot/node/core/backing/src/tests/mod.rs @@ -1595,8 +1595,8 @@ fn retry_works() { }, AllMessages::RuntimeApi(RuntimeApiMessage::Request( _, - RuntimeApiRequest::SessionExecutorParams(sess_idx, tx), - )) if sess_idx == 1 => { + RuntimeApiRequest::SessionExecutorParams(1, tx), + )) => { tx.send(Ok(Some(ExecutorParams::default()))).unwrap(); }, msg => { diff --git a/polkadot/node/core/pvf/common/Cargo.toml b/polkadot/node/core/pvf/common/Cargo.toml index 0f7308396d809..4bdacca72f420 100644 --- a/polkadot/node/core/pvf/common/Cargo.toml +++ b/polkadot/node/core/pvf/common/Cargo.toml @@ -29,7 +29,7 @@ sp-io = { path = "../../../../../substrate/primitives/io" } sp-tracing = { path = "../../../../../substrate/primitives/tracing" } [target.'cfg(target_os = "linux")'.dependencies] -landlock = "0.2.0" +landlock = "0.3.0" [dev-dependencies] assert_matches = "1.4.0" diff --git a/polkadot/node/core/pvf/common/src/worker/security.rs b/polkadot/node/core/pvf/common/src/worker/security.rs index b7abf028f9410..1b76141774485 100644 --- a/polkadot/node/core/pvf/common/src/worker/security.rs +++ b/polkadot/node/core/pvf/common/src/worker/security.rs @@ -223,13 +223,22 @@ pub mod landlock { /// Landlock ABI version. We use ABI V1 because: /// /// 1. It is supported by our reference kernel version. - /// 2. Later versions do not (yet) provide additional security. + /// 2. Later versions do not (yet) provide additional security that would benefit us. /// - /// # Versions (as of June 2023) + /// # Versions (as of October 2023) /// /// - Polkadot reference kernel version: 5.16+ - /// - ABI V1: 5.13 - introduces landlock, including full restrictions on file reads - /// - ABI V2: 5.19 - adds ability to configure file renaming (not used by us) + /// + /// - ABI V1: kernel 5.13 - Introduces landlock, including full restrictions on file reads. + /// + /// - ABI V2: kernel 5.19 - Adds ability to prevent file renaming. Does not help us. During + /// execution an attacker can only affect the name of a symlinked artifact and not the + /// original one. + /// + /// - ABI V3: kernel 6.2 - Adds ability to prevent file truncation. During execution, can + /// prevent attackers from affecting a symlinked artifact. We don't strictly need this as we + /// plan to check for file integrity anyway; see + /// . /// /// # Determinism /// @@ -335,7 +344,7 @@ pub mod landlock { A: Into>, { let mut ruleset = - Ruleset::new().handle_access(AccessFs::from_all(LANDLOCK_ABI))?.create()?; + Ruleset::default().handle_access(AccessFs::from_all(LANDLOCK_ABI))?.create()?; for (fs_path, access_bits) in fs_exceptions { let paths = &[fs_path.as_ref().to_owned()]; let mut rules = path_beneath_rules(paths, access_bits).peekable(); @@ -466,5 +475,38 @@ pub mod landlock { assert!(handle.join().is_ok()); } + + // Test that checks whether landlock under our ABI version is able to truncate files. + #[test] + fn restricted_thread_can_truncate_file() { + // TODO: This would be nice: . + if !check_is_fully_enabled() { + return + } + + // Restricted thread can truncate file. + let handle = + thread::spawn(|| { + // Create and write a file. This should succeed before any landlock + // restrictions are applied. + const TEXT: &str = "foo"; + let tmpfile = tempfile::NamedTempFile::new().unwrap(); + let path = tmpfile.path(); + + fs::write(path, TEXT).unwrap(); + + // Apply Landlock with all exceptions under the current ABI. + let status = try_restrict(vec![(path, AccessFs::from_all(LANDLOCK_ABI))]); + if !matches!(status, Ok(RulesetStatus::FullyEnforced)) { + panic!("Ruleset should be enforced since we checked if landlock is enabled: {:?}", status); + } + + // Try to truncate the file. + let result = tmpfile.as_file().set_len(0); + assert!(result.is_ok()); + }); + + assert!(handle.join().is_ok()); + } } } diff --git a/polkadot/node/core/pvf/src/lib.rs b/polkadot/node/core/pvf/src/lib.rs index 1b8d83773813a..e3c6da9c4c6ba 100644 --- a/polkadot/node/core/pvf/src/lib.rs +++ b/polkadot/node/core/pvf/src/lib.rs @@ -19,8 +19,10 @@ //! The PVF validation host. Responsible for coordinating preparation and execution of PVFs. //! //! For more background, refer to the Implementer's Guide: [PVF -//! Pre-checking](https://paritytech.github.io/polkadot/book/pvf-prechecking.html) and [Candidate -//! Validation](https://paritytech.github.io/polkadot/book/node/utility/candidate-validation.html#pvf-host). +//! Pre-checking](https://paritytech.github.io/polkadot-sdk/book/pvf-prechecking.html), [Candidate +//! Validation](https://paritytech.github.io/polkadot-sdk/book/node/utility/candidate-validation.html) +//! and [PVF Host and Workers](https://paritytech.github.io/polkadot-sdk/book/node/utility/pvf-host-and-workers.html). +//! //! //! # Entrypoint //! diff --git a/polkadot/node/core/runtime-api/src/cache.rs b/polkadot/node/core/runtime-api/src/cache.rs index 6cf7fa744d3f9..69eea22b23bda 100644 --- a/polkadot/node/core/runtime-api/src/cache.rs +++ b/polkadot/node/core/runtime-api/src/cache.rs @@ -64,6 +64,7 @@ pub(crate) struct RequestResultCache { unapplied_slashes: LruMap>, key_ownership_proof: LruMap<(Hash, ValidatorId), Option>, minimum_backing_votes: LruMap, + disabled_validators: LruMap>, para_backing_state: LruMap<(Hash, ParaId), Option>, async_backing_params: LruMap, } @@ -96,6 +97,7 @@ impl Default for RequestResultCache { unapplied_slashes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), key_ownership_proof: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), minimum_backing_votes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), + disabled_validators: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), para_backing_state: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), async_backing_params: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)), } @@ -444,6 +446,21 @@ impl RequestResultCache { self.minimum_backing_votes.insert(session_index, minimum_backing_votes); } + pub(crate) fn disabled_validators( + &mut self, + relay_parent: &Hash, + ) -> Option<&Vec> { + self.disabled_validators.get(relay_parent).map(|v| &*v) + } + + pub(crate) fn cache_disabled_validators( + &mut self, + relay_parent: Hash, + disabled_validators: Vec, + ) { + self.disabled_validators.insert(relay_parent, disabled_validators); + } + pub(crate) fn para_backing_state( &mut self, key: (Hash, ParaId), @@ -520,6 +537,7 @@ pub(crate) enum RequestResult { slashing::OpaqueKeyOwnershipProof, Option<()>, ), + DisabledValidators(Hash, Vec), ParaBackingState(Hash, ParaId, Option), AsyncBackingParams(Hash, async_backing::AsyncBackingParams), } diff --git a/polkadot/node/core/runtime-api/src/lib.rs b/polkadot/node/core/runtime-api/src/lib.rs index 1b18941e54645..bdcca08b10dda 100644 --- a/polkadot/node/core/runtime-api/src/lib.rs +++ b/polkadot/node/core/runtime-api/src/lib.rs @@ -166,6 +166,8 @@ where .requests_cache .cache_key_ownership_proof((relay_parent, validator_id), key_ownership_proof), SubmitReportDisputeLost(_, _, _, _) => {}, + DisabledValidators(relay_parent, disabled_validators) => + self.requests_cache.cache_disabled_validators(relay_parent, disabled_validators), ParaBackingState(relay_parent, para_id, constraints) => self .requests_cache .cache_para_backing_state((relay_parent, para_id), constraints), @@ -296,6 +298,8 @@ where Request::SubmitReportDisputeLost(dispute_proof, key_ownership_proof, sender) }, ), + Request::DisabledValidators(sender) => query!(disabled_validators(), sender) + .map(|sender| Request::DisabledValidators(sender)), Request::ParaBackingState(para, sender) => query!(para_backing_state(para), sender) .map(|sender| Request::ParaBackingState(para, sender)), Request::AsyncBackingParams(sender) => query!(async_backing_params(), sender) @@ -565,6 +569,12 @@ where ver = Request::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT, sender ), + Request::DisabledValidators(sender) => query!( + DisabledValidators, + disabled_validators(), + ver = Request::DISABLED_VALIDATORS_RUNTIME_REQUIREMENT, + sender + ), Request::ParaBackingState(para, sender) => { query!( ParaBackingState, diff --git a/polkadot/node/core/runtime-api/src/tests.rs b/polkadot/node/core/runtime-api/src/tests.rs index fb97139a80287..979b3587d2692 100644 --- a/polkadot/node/core/runtime-api/src/tests.rs +++ b/polkadot/node/core/runtime-api/src/tests.rs @@ -268,6 +268,10 @@ impl RuntimeApiSubsystemClient for MockSubsystemClient { async fn minimum_backing_votes(&self, _: Hash, _: SessionIndex) -> Result { todo!("Not required for tests") } + + async fn disabled_validators(&self, _: Hash) -> Result, ApiError> { + todo!("Not required for tests") + } } #[test] diff --git a/polkadot/node/network/approval-distribution/src/lib.rs b/polkadot/node/network/approval-distribution/src/lib.rs index f76826d7fdf42..d8949b1c1123c 100644 --- a/polkadot/node/network/approval-distribution/src/lib.rs +++ b/polkadot/node/network/approval-distribution/src/lib.rs @@ -16,7 +16,10 @@ //! [`ApprovalDistribution`] implementation. //! -//! +//! See the documentation on [approval distribution][approval-distribution-page] in the +//! implementers' guide. +//! +//! [approval-distribution-page]: https://paritytech.github.io/polkadot-sdk/book/node/approval/approval-distribution.html #![warn(missing_docs)] diff --git a/polkadot/node/overseer/examples/minimal-example.rs b/polkadot/node/overseer/examples/minimal-example.rs index e78941776d5ec..cffdfd9f8aa14 100644 --- a/polkadot/node/overseer/examples/minimal-example.rs +++ b/polkadot/node/overseer/examples/minimal-example.rs @@ -163,7 +163,6 @@ fn main() { .unwrap(); let overseer_fut = overseer.run().fuse(); - let timer_stream = timer_stream; pin_mut!(timer_stream); pin_mut!(overseer_fut); diff --git a/polkadot/node/overseer/src/lib.rs b/polkadot/node/overseer/src/lib.rs index 84d5d19c3b93c..673569ab946f0 100644 --- a/polkadot/node/overseer/src/lib.rs +++ b/polkadot/node/overseer/src/lib.rs @@ -17,7 +17,7 @@ //! # Overseer //! //! `overseer` implements the Overseer architecture described in the -//! [implementers-guide](https://w3f.github.io/parachain-implementers-guide/node/index.html). +//! [implementers' guide][overseer-page]. //! For the motivations behind implementing the overseer itself you should //! check out that guide, documentation in this crate will be mostly discussing //! technical stuff. @@ -53,6 +53,8 @@ //! . +--------------------+ +---------------------+ . //! .................................................................. //! ``` +//! +//! [overseer-page]: https://paritytech.github.io/polkadot-sdk/book/node/overseer.html // #![deny(unused_results)] // unused dependencies can not work for test and examples at the same time diff --git a/polkadot/node/service/src/relay_chain_selection.rs b/polkadot/node/service/src/relay_chain_selection.rs index 189073783f0dc..5fae6a96de4ea 100644 --- a/polkadot/node/service/src/relay_chain_selection.rs +++ b/polkadot/node/service/src/relay_chain_selection.rs @@ -31,7 +31,7 @@ //! leaf returned from the chain selection subsystem by calling into other //! subsystems which yield information about approvals and disputes. //! -//! [chain-selection-guide]: https://w3f.github.io/parachain-implementers-guide/protocol-chain-selection.html +//! [chain-selection-guide]: https://paritytech.github.io/polkadot-sdk/book/protocol-chain-selection.html #![cfg(feature = "full-node")] diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index 01ccee3add904..6bc874384594e 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -695,6 +695,8 @@ pub enum RuntimeApiRequest { ), /// Get the minimum required backing votes. MinimumBackingVotes(SessionIndex, RuntimeApiSender), + /// Returns all disabled validators at a given block height. + DisabledValidators(RuntimeApiSender>), /// Get the backing state of the given para. ParaBackingState(ParaId, RuntimeApiSender>), /// Get candidate's acceptance limitations for asynchronous backing for a relay parent. @@ -726,6 +728,9 @@ impl RuntimeApiRequest { /// Minimum version to enable asynchronous backing: `AsyncBackingParams` and `ParaBackingState`. pub const ASYNC_BACKING_STATE_RUNTIME_REQUIREMENT: u32 = 7; + + /// `DisabledValidators` + pub const DISABLED_VALIDATORS_RUNTIME_REQUIREMENT: u32 = 8; } /// A message to the Runtime API subsystem. diff --git a/polkadot/node/subsystem-types/src/runtime_client.rs b/polkadot/node/subsystem-types/src/runtime_client.rs index 3007e985b4f7b..f7adcf9862b5d 100644 --- a/polkadot/node/subsystem-types/src/runtime_client.rs +++ b/polkadot/node/subsystem-types/src/runtime_client.rs @@ -255,6 +255,10 @@ pub trait RuntimeApiSubsystemClient { at: Hash, para_id: Id, ) -> Result, ApiError>; + + // === v8 === + /// Gets the disabled validators at a specific block height + async fn disabled_validators(&self, at: Hash) -> Result, ApiError>; } /// Default implementation of [`RuntimeApiSubsystemClient`] using the client. @@ -497,11 +501,14 @@ where self.client.runtime_api().para_backing_state(at, para_id) } - /// Returns candidate's acceptance limitations for asynchronous backing for a relay parent. async fn async_backing_params( &self, at: Hash, ) -> Result { self.client.runtime_api().async_backing_params(at) } + + async fn disabled_validators(&self, at: Hash) -> Result, ApiError> { + self.client.runtime_api().disabled_validators(at) + } } diff --git a/polkadot/node/subsystem-util/src/lib.rs b/polkadot/node/subsystem-util/src/lib.rs index 57e4f9cde09af..a5f3e9d4a0c0e 100644 --- a/polkadot/node/subsystem-util/src/lib.rs +++ b/polkadot/node/subsystem-util/src/lib.rs @@ -226,6 +226,7 @@ specialize_requests! { fn request_unapplied_slashes() -> Vec<(SessionIndex, CandidateHash, slashing::PendingSlashes)>; UnappliedSlashes; fn request_key_ownership_proof(validator_id: ValidatorId) -> Option; KeyOwnershipProof; fn request_submit_report_dispute_lost(dp: slashing::DisputeProof, okop: slashing::OpaqueKeyOwnershipProof) -> Option<()>; SubmitReportDisputeLost; + fn request_disabled_validators() -> Vec; DisabledValidators; fn request_async_backing_params() -> AsyncBackingParams; AsyncBackingParams; } diff --git a/polkadot/primitives/src/runtime_api.rs b/polkadot/primitives/src/runtime_api.rs index 6cb66d40204d1..5ec897c8cbb40 100644 --- a/polkadot/primitives/src/runtime_api.rs +++ b/polkadot/primitives/src/runtime_api.rs @@ -248,6 +248,7 @@ sp_api::decl_runtime_apis! { #[api_version(6)] fn minimum_backing_votes() -> u32; + /***** Added in v7: Asynchronous backing *****/ /// Returns the state of parachain backing for a given para. @@ -257,5 +258,11 @@ sp_api::decl_runtime_apis! { /// Returns candidate's acceptance limitations for asynchronous backing for a relay parent. #[api_version(7)] fn async_backing_params() -> AsyncBackingParams; + + /***** Added in v8 *****/ + + /// Returns a list of all disabled validators at the given block. + #[api_version(8)] + fn disabled_validators() -> Vec; } } diff --git a/polkadot/roadmap/implementers-guide/README.md b/polkadot/roadmap/implementers-guide/README.md index 996041f176bb2..e03c0c45ddba0 100644 --- a/polkadot/roadmap/implementers-guide/README.md +++ b/polkadot/roadmap/implementers-guide/README.md @@ -4,7 +4,7 @@ The implementers' guide is compiled from several source files with [`mdBook`](ht ## Hosted build -This is available [here](https://paritytech.github.io/polkadot/book/). +This is available [here](https://paritytech.github.io/polkadot-sdk/book/). ## Local build diff --git a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs index d01b543630c31..24a076f3a4431 100644 --- a/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs +++ b/polkadot/runtime/parachains/src/runtime_api_impl/vstaging.rs @@ -15,3 +15,30 @@ // along with Polkadot. If not, see . //! Put implementations of functions from staging APIs here. + +use crate::shared; +use primitives::ValidatorIndex; +use sp_std::{collections::btree_map::BTreeMap, prelude::Vec}; + +/// Implementation for `DisabledValidators` +// CAVEAT: this should only be called on the node side +// as it might produce incorrect results on session boundaries +pub fn disabled_validators() -> Vec +where + T: pallet_session::Config + shared::Config, +{ + let shuffled_indices = >::active_validator_indices(); + // mapping from raw validator index to `ValidatorIndex` + // this computation is the same within a session, but should be cheap + let reverse_index = shuffled_indices + .iter() + .enumerate() + .map(|(i, v)| (v.0, ValidatorIndex(i as u32))) + .collect::>(); + + // we might have disabled validators who are not parachain validators + >::disabled_validators() + .iter() + .filter_map(|v| reverse_index.get(v).cloned()) + .collect() +} diff --git a/polkadot/runtime/parachains/src/session_info.rs b/polkadot/runtime/parachains/src/session_info.rs index 0043851be0f22..9e1b3d05842fd 100644 --- a/polkadot/runtime/parachains/src/session_info.rs +++ b/polkadot/runtime/parachains/src/session_info.rs @@ -17,8 +17,9 @@ //! The session info pallet provides information about validator sets //! from prior sessions needed for approvals and disputes. //! -//! See . - +//! See the documentation on [session info][session-info-page] in the implementers' guide. +//! +//! [session-info-page]: https://paritytech.github.io/polkadot-sdk/book/runtime/session_info.html use crate::{ configuration, paras, scheduler, shared, util::{take_active_subset, take_active_subset_and_inactive}, diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index fd3e656f69573..9933f64429745 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -49,7 +49,9 @@ use runtime_parachains::{ inclusion::{AggregateMessageOrigin, UmpQueueId}, initializer as parachains_initializer, origin as parachains_origin, paras as parachains_paras, paras_inherent as parachains_paras_inherent, - runtime_api_impl::v7 as parachains_runtime_api_impl, + runtime_api_impl::{ + v7 as parachains_runtime_api_impl, vstaging as parachains_staging_runtime_api_impl, + }, scheduler as parachains_scheduler, session_info as parachains_session_info, shared as parachains_shared, }; @@ -1585,7 +1587,7 @@ sp_api::impl_runtime_apis! { } } - #[api_version(7)] + #[api_version(8)] impl primitives::runtime_api::ParachainHost for Runtime { fn validators() -> Vec { parachains_runtime_api_impl::validators::() @@ -1728,6 +1730,11 @@ sp_api::impl_runtime_apis! { fn async_backing_params() -> primitives::AsyncBackingParams { parachains_runtime_api_impl::async_backing_params::() } + + fn disabled_validators() -> Vec { + parachains_staging_runtime_api_impl::disabled_validators::() + } + } #[api_version(3)] diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index d61acf36b4cb8..0e93b3449fec5 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -70,7 +70,9 @@ use runtime_parachains::{ inclusion::{AggregateMessageOrigin, UmpQueueId}, initializer as parachains_initializer, origin as parachains_origin, paras as parachains_paras, paras_inherent as parachains_paras_inherent, reward_points as parachains_reward_points, - runtime_api_impl::v7 as parachains_runtime_api_impl, + runtime_api_impl::{ + v7 as parachains_runtime_api_impl, vstaging as parachains_staging_runtime_api_impl, + }, scheduler as parachains_scheduler, session_info as parachains_session_info, shared as parachains_shared, }; @@ -1695,7 +1697,7 @@ sp_api::impl_runtime_apis! { } } - #[api_version(7)] + #[api_version(8)] impl primitives::runtime_api::ParachainHost for Runtime { fn validators() -> Vec { parachains_runtime_api_impl::validators::() @@ -1838,6 +1840,10 @@ sp_api::impl_runtime_apis! { fn async_backing_params() -> primitives::AsyncBackingParams { parachains_runtime_api_impl::async_backing_params::() } + + fn disabled_validators() -> Vec { + parachains_staging_runtime_api_impl::disabled_validators::() + } } impl beefy_primitives::BeefyApi for Runtime { diff --git a/polkadot/xcm/xcm-builder/src/lib.rs b/polkadot/xcm/xcm-builder/src/lib.rs index 26f2aadadf3de..9fdd55d9f9274 100644 --- a/polkadot/xcm/xcm-builder/src/lib.rs +++ b/polkadot/xcm/xcm-builder/src/lib.rs @@ -33,9 +33,9 @@ pub use location_conversion::{ Account32Hash, AccountId32Aliases, AccountKey20Aliases, AliasesIntoAccountId32, ChildParachainConvertsVia, DescribeAccountId32Terminal, DescribeAccountIdTerminal, DescribeAccountKey20Terminal, DescribeAllTerminal, DescribeBodyTerminal, DescribeFamily, - DescribeLocation, DescribePalletTerminal, DescribeTerminus, GlobalConsensusConvertsFor, - GlobalConsensusParachainConvertsFor, HashedDescription, ParentIsPreset, - SiblingParachainConvertsVia, + DescribeLocation, DescribePalletTerminal, DescribeTerminus, DescribeTreasuryVoiceTerminal, + GlobalConsensusConvertsFor, GlobalConsensusParachainConvertsFor, HashedDescription, + LocalTreasuryVoiceConvertsVia, ParentIsPreset, SiblingParachainConvertsVia, }; mod origin_conversion; diff --git a/polkadot/xcm/xcm-builder/src/location_conversion.rs b/polkadot/xcm/xcm-builder/src/location_conversion.rs index 4a5fbbb123be1..25d16f7eb8ccc 100644 --- a/polkadot/xcm/xcm-builder/src/location_conversion.rs +++ b/polkadot/xcm/xcm-builder/src/location_conversion.rs @@ -84,6 +84,20 @@ impl DescribeLocation for DescribeAccountKey20Terminal { } } +/// Create a description of the remote treasury `location` if possible. No two locations should have +/// the same descriptor. +pub struct DescribeTreasuryVoiceTerminal; + +impl DescribeLocation for DescribeTreasuryVoiceTerminal { + fn describe_location(l: &MultiLocation) -> Option> { + match (l.parents, &l.interior) { + (0, X1(Plurality { id: BodyId::Treasury, part: BodyPart::Voice })) => + Some((b"Treasury", b"Voice").encode()), + _ => None, + } + } +} + pub type DescribeAccountIdTerminal = (DescribeAccountId32Terminal, DescribeAccountKey20Terminal); pub struct DescribeBodyTerminal; @@ -101,6 +115,7 @@ pub type DescribeAllTerminal = ( DescribePalletTerminal, DescribeAccountId32Terminal, DescribeAccountKey20Terminal, + DescribeTreasuryVoiceTerminal, DescribeBodyTerminal, ); @@ -328,6 +343,25 @@ impl>, AccountId: From<[u8; 32]> + Into<[u8; 32]> } } +/// Returns specified `TreasuryAccount` as `AccountId32` if passed `location` matches Treasury +/// plurality. +pub struct LocalTreasuryVoiceConvertsVia( + PhantomData<(TreasuryAccount, AccountId)>, +); +impl, AccountId: From<[u8; 32]> + Into<[u8; 32]> + Clone> + ConvertLocation for LocalTreasuryVoiceConvertsVia +{ + fn convert_location(location: &MultiLocation) -> Option { + match *location { + MultiLocation { + parents: 0, + interior: X1(Plurality { id: BodyId::Treasury, part: BodyPart::Voice }), + } => Some((TreasuryAccount::get().into() as [u8; 32]).into()), + _ => None, + } + } +} + /// Conversion implementation which converts from a `[u8; 32]`-based `AccountId` into a /// `MultiLocation` consisting solely of a `AccountId32` junction with a fixed value for its /// network (provided by `Network`) and the `AccountId`'s `[u8; 32]` datum for the `id`. @@ -442,10 +476,13 @@ impl #[cfg(test)] mod tests { use super::*; - + use primitives::AccountId; pub type ForeignChainAliasAccount = HashedDescription; + pub type ForeignChainAliasTreasuryAccount = + HashedDescription>; + use frame_support::parameter_types; use xcm::latest::Junction; @@ -936,4 +973,70 @@ mod tests { }; assert!(ForeignChainAliasAccount::<[u8; 32]>::convert_location(&mul).is_none()); } + + #[test] + fn remote_account_convert_on_para_sending_from_remote_para_treasury() { + let relay_treasury_to_para_location = MultiLocation { + parents: 1, + interior: X1(Plurality { id: BodyId::Treasury, part: BodyPart::Voice }), + }; + let actual_description = ForeignChainAliasTreasuryAccount::<[u8; 32]>::convert_location( + &relay_treasury_to_para_location, + ) + .unwrap(); + + assert_eq!( + [ + 18, 84, 93, 74, 187, 212, 254, 71, 192, 127, 112, 51, 3, 42, 54, 24, 220, 185, 161, + 67, 205, 154, 108, 116, 108, 166, 226, 211, 29, 11, 244, 115 + ], + actual_description + ); + + let para_to_para_treasury_location = MultiLocation { + parents: 1, + interior: X2( + Parachain(1001), + Plurality { id: BodyId::Treasury, part: BodyPart::Voice }, + ), + }; + let actual_description = ForeignChainAliasTreasuryAccount::<[u8; 32]>::convert_location( + ¶_to_para_treasury_location, + ) + .unwrap(); + + assert_eq!( + [ + 202, 52, 249, 30, 7, 99, 135, 128, 153, 139, 176, 141, 138, 234, 163, 150, 7, 36, + 204, 92, 220, 137, 87, 57, 73, 91, 243, 189, 245, 200, 217, 204 + ], + actual_description + ); + } + + #[test] + fn local_account_convert_on_para_from_relay_treasury() { + let location = MultiLocation { + parents: 0, + interior: X1(Plurality { id: BodyId::Treasury, part: BodyPart::Voice }), + }; + + parameter_types! { + pub TreasuryAccountId: AccountId = AccountId::new([42u8; 32]); + } + + let actual_description = + LocalTreasuryVoiceConvertsVia::::convert_location( + &location, + ) + .unwrap(); + + assert_eq!( + [ + 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, + 42, 42, 42, 42, 42, 42, 42, 42, 42, 42, 42 + ], + actual_description + ); + } } diff --git a/substrate/client/cli/src/commands/inspect_node_key.rs b/substrate/client/cli/src/commands/inspect_node_key.rs index 19b5a31ca12c0..6cf025a2d1150 100644 --- a/substrate/client/cli/src/commands/inspect_node_key.rs +++ b/substrate/client/cli/src/commands/inspect_node_key.rs @@ -85,7 +85,7 @@ mod tests { fn inspect_node_key() { let path = tempfile::tempdir().unwrap().into_path().join("node-id").into_os_string(); let path = path.to_str().unwrap(); - let cmd = GenerateNodeKeyCmd::parse_from(&["generate-node-key", "--file", path.clone()]); + let cmd = GenerateNodeKeyCmd::parse_from(&["generate-node-key", "--file", path]); assert!(cmd.run().is_ok()); diff --git a/substrate/client/consensus/beefy/src/communication/gossip.rs b/substrate/client/consensus/beefy/src/communication/gossip.rs index 8c025ca067619..342cd0511a511 100644 --- a/substrate/client/consensus/beefy/src/communication/gossip.rs +++ b/substrate/client/consensus/beefy/src/communication/gossip.rs @@ -16,11 +16,10 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use std::{collections::BTreeMap, sync::Arc, time::Duration}; +use std::{collections::BTreeSet, sync::Arc, time::Duration}; use sc_network::{PeerId, ReputationChange}; use sc_network_gossip::{MessageIntent, ValidationResult, Validator, ValidatorContext}; -use sp_core::hashing::twox_64; use sp_runtime::traits::{Block, Hash, Header, NumberFor}; use codec::{Decode, DecodeAll, Encode}; @@ -115,9 +114,6 @@ where <::Hashing as Hash>::hash(b"beefy-justifications") } -/// A type that represents hash of the message. -pub type MessageHash = [u8; 8]; - #[derive(Clone, Debug)] pub(crate) struct GossipFilterCfg<'a, B: Block> { pub start: NumberFor, @@ -133,18 +129,21 @@ struct FilterInner { } struct Filter { + // specifies live rounds inner: Option>, - live_votes: BTreeMap, fnv::FnvHashSet>, + // cache of seen valid justifications in active rounds + rounds_with_valid_proofs: BTreeSet>, } impl Filter { pub fn new() -> Self { - Self { inner: None, live_votes: BTreeMap::new() } + Self { inner: None, rounds_with_valid_proofs: BTreeSet::new() } } /// Update filter to new `start` and `set_id`. fn update(&mut self, cfg: GossipFilterCfg) { - self.live_votes.retain(|&round, _| round >= cfg.start && round <= cfg.end); + self.rounds_with_valid_proofs + .retain(|&round| round >= cfg.start && round <= cfg.end); // only clone+overwrite big validator_set if set_id changed match self.inner.as_mut() { Some(f) if f.validator_set.id() == cfg.validator_set.id() => { @@ -203,14 +202,14 @@ impl Filter { .unwrap_or(Consider::RejectOutOfScope) } - /// Add new _known_ `hash` to the round's known votes. - fn add_known_vote(&mut self, round: NumberFor, hash: MessageHash) { - self.live_votes.entry(round).or_default().insert(hash); + /// Add new _known_ `round` to the set of seen valid justifications. + fn mark_round_as_proven(&mut self, round: NumberFor) { + self.rounds_with_valid_proofs.insert(round); } - /// Check if `hash` is already part of round's known votes. - fn is_known_vote(&self, round: NumberFor, hash: &MessageHash) -> bool { - self.live_votes.get(&round).map(|known| known.contains(hash)).unwrap_or(false) + /// Check if `round` is already part of seen valid justifications. + fn is_already_proven(&self, round: NumberFor) -> bool { + self.rounds_with_valid_proofs.contains(&round) } fn validator_set(&self) -> Option<&ValidatorSet> { @@ -273,16 +272,13 @@ where &self, vote: VoteMessage, AuthorityId, Signature>, sender: &PeerId, - data: &[u8], ) -> Action { - let msg_hash = twox_64(data); let round = vote.commitment.block_number; let set_id = vote.commitment.validator_set_id; self.known_peers.lock().note_vote_for(*sender, round); // Verify general usefulness of the message. - // We are going to discard old votes right away (without verification) - // Also we keep track of already received votes to avoid verifying duplicates. + // We are going to discard old votes right away (without verification). { let filter = self.gossip_filter.read(); @@ -293,10 +289,6 @@ where Consider::Accept => {}, } - if filter.is_known_vote(round, &msg_hash) { - return Action::Keep(self.votes_topic, benefit::KNOWN_VOTE_MESSAGE) - } - // ensure authority is part of the set. if !filter .validator_set() @@ -309,7 +301,6 @@ where } if BeefyKeystore::verify(&vote.id, &vote.signature, &vote.commitment.encode()) { - self.gossip_filter.write().add_known_vote(round, msg_hash); Action::Keep(self.votes_topic, benefit::VOTE_MESSAGE) } else { debug!( @@ -328,34 +319,46 @@ where let (round, set_id) = proof_block_num_and_set_id::(&proof); self.known_peers.lock().note_vote_for(*sender, round); - let guard = self.gossip_filter.read(); - // Verify general usefulness of the justification. - match guard.consider_finality_proof(round, set_id) { - Consider::RejectPast => return Action::Discard(cost::OUTDATED_MESSAGE), - Consider::RejectFuture => return Action::Discard(cost::FUTURE_MESSAGE), - Consider::RejectOutOfScope => return Action::Discard(cost::OUT_OF_SCOPE_MESSAGE), - Consider::Accept => {}, + let action = { + let guard = self.gossip_filter.read(); + + // Verify general usefulness of the justification. + match guard.consider_finality_proof(round, set_id) { + Consider::RejectPast => return Action::Discard(cost::OUTDATED_MESSAGE), + Consider::RejectFuture => return Action::Discard(cost::FUTURE_MESSAGE), + Consider::RejectOutOfScope => return Action::Discard(cost::OUT_OF_SCOPE_MESSAGE), + Consider::Accept => {}, + } + + if guard.is_already_proven(round) { + return Action::Discard(benefit::NOT_INTERESTED) + } + + // Verify justification signatures. + guard + .validator_set() + .map(|validator_set| { + if let Err((_, signatures_checked)) = + verify_with_validator_set::(round, validator_set, &proof) + { + debug!( + target: LOG_TARGET, + "🥩 Bad signatures on message: {:?}, from: {:?}", proof, sender + ); + let mut cost = cost::INVALID_PROOF; + cost.value += + cost::PER_SIGNATURE_CHECKED.saturating_mul(signatures_checked as i32); + Action::Discard(cost) + } else { + Action::Keep(self.justifs_topic, benefit::VALIDATED_PROOF) + } + }) + .unwrap_or(Action::Discard(cost::OUT_OF_SCOPE_MESSAGE)) + }; + if matches!(action, Action::Keep(_, _)) { + self.gossip_filter.write().mark_round_as_proven(round); } - // Verify justification signatures. - guard - .validator_set() - .map(|validator_set| { - if let Err((_, signatures_checked)) = - verify_with_validator_set::(round, validator_set, &proof) - { - debug!( - target: LOG_TARGET, - "🥩 Bad signatures on message: {:?}, from: {:?}", proof, sender - ); - let mut cost = cost::INVALID_PROOF; - cost.value += - cost::PER_SIGNATURE_CHECKED.saturating_mul(signatures_checked as i32); - Action::Discard(cost) - } else { - Action::Keep(self.justifs_topic, benefit::VALIDATED_PROOF) - } - }) - .unwrap_or(Action::Discard(cost::OUT_OF_SCOPE_MESSAGE)) + action } } @@ -375,7 +378,7 @@ where ) -> ValidationResult { let raw = data; let action = match GossipMessage::::decode_all(&mut data) { - Ok(GossipMessage::Vote(msg)) => self.validate_vote(msg, sender, raw), + Ok(GossipMessage::Vote(msg)) => self.validate_vote(msg, sender), Ok(GossipMessage::FinalityProof(proof)) => self.validate_finality_proof(proof, sender), Err(e) => { debug!(target: LOG_TARGET, "Error decoding message: {}", e); @@ -483,41 +486,6 @@ pub(crate) mod tests { }; use sp_keystore::{testing::MemoryKeystore, Keystore}; - #[test] - fn known_votes_insert_remove() { - let mut filter = Filter::::new(); - let msg_hash = twox_64(b"data"); - let keys = vec![Keyring::Alice.public()]; - let validator_set = ValidatorSet::::new(keys.clone(), 1).unwrap(); - - filter.add_known_vote(1, msg_hash); - filter.add_known_vote(1, msg_hash); - filter.add_known_vote(2, msg_hash); - assert_eq!(filter.live_votes.len(), 2); - - filter.add_known_vote(3, msg_hash); - assert!(filter.is_known_vote(3, &msg_hash)); - assert!(!filter.is_known_vote(3, &twox_64(b"other"))); - assert!(!filter.is_known_vote(4, &msg_hash)); - assert_eq!(filter.live_votes.len(), 3); - - assert!(filter.inner.is_none()); - assert_eq!(filter.consider_vote(1, 1), Consider::RejectOutOfScope); - - filter.update(GossipFilterCfg { start: 3, end: 10, validator_set: &validator_set }); - assert_eq!(filter.live_votes.len(), 1); - assert!(filter.live_votes.contains_key(&3)); - assert_eq!(filter.consider_vote(2, 1), Consider::RejectPast); - assert_eq!(filter.consider_vote(3, 1), Consider::Accept); - assert_eq!(filter.consider_vote(4, 1), Consider::Accept); - assert_eq!(filter.consider_vote(20, 1), Consider::RejectFuture); - assert_eq!(filter.consider_vote(4, 2), Consider::RejectFuture); - - let validator_set = ValidatorSet::::new(keys, 2).unwrap(); - filter.update(GossipFilterCfg { start: 5, end: 10, validator_set: &validator_set }); - assert!(filter.live_votes.is_empty()); - } - struct TestContext; impl ValidatorContext for TestContext { fn broadcast_topic(&mut self, _topic: B::Hash, _force: bool) { @@ -610,20 +578,6 @@ pub(crate) mod tests { assert!(matches!(res, ValidationResult::ProcessAndKeep(_))); expected_report.cost_benefit = benefit::VOTE_MESSAGE; assert_eq!(report_stream.try_recv().unwrap(), expected_report); - assert_eq!( - gv.gossip_filter - .read() - .live_votes - .get(&vote.commitment.block_number) - .map(|x| x.len()), - Some(1) - ); - - // second time we should hit the cache - let res = gv.validate(&mut context, &sender, &encoded); - assert!(matches!(res, ValidationResult::ProcessAndKeep(_))); - expected_report.cost_benefit = benefit::KNOWN_VOTE_MESSAGE; - assert_eq!(report_stream.try_recv().unwrap(), expected_report); // reject vote, voter not in validator set let mut bad_vote = vote.clone(); @@ -692,7 +646,7 @@ pub(crate) mod tests { // reject proof, bad signatures (Bob instead of Alice) let bad_validator_set = ValidatorSet::::new(vec![Keyring::Bob.public()], 0).unwrap(); - let proof = dummy_proof(20, &bad_validator_set); + let proof = dummy_proof(21, &bad_validator_set); let encoded_proof = GossipMessage::::FinalityProof(proof).encode(); let res = gv.validate(&mut context, &sender, &encoded_proof); assert!(matches!(res, ValidationResult::Discard)); diff --git a/substrate/client/consensus/beefy/src/communication/mod.rs b/substrate/client/consensus/beefy/src/communication/mod.rs index 7f9535bfc23f1..10a6071aae658 100644 --- a/substrate/client/consensus/beefy/src/communication/mod.rs +++ b/substrate/client/consensus/beefy/src/communication/mod.rs @@ -102,7 +102,7 @@ mod cost { mod benefit { use sc_network::ReputationChange as Rep; pub(super) const VOTE_MESSAGE: Rep = Rep::new(100, "BEEFY: Round vote message"); - pub(super) const KNOWN_VOTE_MESSAGE: Rep = Rep::new(50, "BEEFY: Known vote"); + pub(super) const NOT_INTERESTED: Rep = Rep::new(10, "BEEFY: Not interested in round"); pub(super) const VALIDATED_PROOF: Rep = Rep::new(100, "BEEFY: Justification"); } diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index 3bb65e9d57f43..90b63c9cd4462 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -1302,7 +1302,7 @@ async fn gossipped_finality_proofs() { // Only Alice and Bob are running the voter -> finality threshold not reached let peers = [BeefyKeyring::Alice, BeefyKeyring::Bob]; let validator_set = ValidatorSet::new(make_beefy_ids(&validators), 0).unwrap(); - let session_len = 30; + let session_len = 10; let min_block_delta = 1; let mut net = BeefyTestNet::new(3); @@ -1332,14 +1332,8 @@ async fn gossipped_finality_proofs() { let net = Arc::new(Mutex::new(net)); - // Pump net + Charlie gossip to see peers. - let timeout = Box::pin(tokio::time::sleep(Duration::from_millis(200))); - let gossip_engine_pump = &mut charlie_gossip_engine; - let pump_with_timeout = future::select(gossip_engine_pump, timeout); - run_until(pump_with_timeout, &net).await; - - // push 10 blocks - let hashes = net.lock().generate_blocks_and_sync(10, session_len, &validator_set, true).await; + // push 42 blocks + let hashes = net.lock().generate_blocks_and_sync(42, session_len, &validator_set, true).await; let peers = peers.into_iter().enumerate(); diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index a239e34030c27..309d8c5135bef 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -604,6 +604,11 @@ where VersionedFinalityProof::V1(ref sc) => sc.commitment.block_number, }; + if block_num <= self.persisted_state.voting_oracle.best_beefy_block { + // we've already finalized this round before, short-circuit. + return Ok(()) + } + // Finalize inner round and update voting_oracle state. self.persisted_state.voting_oracle.finalize(block_num)?; @@ -629,7 +634,7 @@ where self.backend .append_justification(hash, (BEEFY_ENGINE_ID, finality_proof.encode())) }) { - error!( + debug!( target: LOG_TARGET, "🥩 Error {:?} on appending justification: {:?}", e, finality_proof ); @@ -648,7 +653,7 @@ where } /// Handle previously buffered justifications, that now land in the voting interval. - fn try_pending_justififactions(&mut self) -> Result<(), Error> { + fn try_pending_justifications(&mut self) -> Result<(), Error> { // Interval of blocks for which we can process justifications and votes right now. let (start, end) = self.voting_oracle().accepted_interval()?; // Process pending justifications. @@ -782,7 +787,7 @@ where fn process_new_state(&mut self) { // Handle pending justifications and/or votes for now GRANDPA finalized blocks. - if let Err(err) = self.try_pending_justififactions() { + if let Err(err) = self.try_pending_justifications() { debug!(target: LOG_TARGET, "🥩 {}", err); } diff --git a/substrate/client/network/common/src/role.rs b/substrate/client/network/common/src/role.rs index cd43f6655b72c..fd02c00e2324a 100644 --- a/substrate/client/network/common/src/role.rs +++ b/substrate/client/network/common/src/role.rs @@ -16,6 +16,10 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . +// file-level lint whitelist to avoid problem with bitflags macro below +// TODO: can be dropped after an update to bitflags 2.4 +#![allow(clippy::bad_bit_mask)] + use codec::{self, Encode, EncodeLike, Input, Output}; /// Role that the peer sent to us during the handshake, with the addition of what our local node diff --git a/substrate/client/network/src/protocol/notifications/behaviour.rs b/substrate/client/network/src/protocol/notifications/behaviour.rs index 89513e004c6df..b78f15f8529c6 100644 --- a/substrate/client/network/src/protocol/notifications/behaviour.rs +++ b/substrate/client/network/src/protocol/notifications/behaviour.rs @@ -1423,7 +1423,6 @@ impl NetworkBehaviour for Notifications { let delay_id = self.next_delay_id; self.next_delay_id.0 += 1; let delay = futures_timer::Delay::new(ban_duration); - let peer_id = peer_id; self.delays.push( async move { delay.await; diff --git a/substrate/frame/balances/src/lib.rs b/substrate/frame/balances/src/lib.rs index a2cacc45369a2..c408bf3f35f3b 100644 --- a/substrate/frame/balances/src/lib.rs +++ b/substrate/frame/balances/src/lib.rs @@ -256,7 +256,7 @@ pub mod pallet { /// The overarching hold reason. #[pallet::no_default_bounds] - type RuntimeHoldReason: Parameter + Member + MaxEncodedLen + Ord + Copy; + type RuntimeHoldReason: Parameter + Member + MaxEncodedLen + Copy; /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; @@ -300,7 +300,7 @@ pub mod pallet { type ReserveIdentifier: Parameter + Member + MaxEncodedLen + Ord + Copy; /// The ID type for freezes. - type FreezeIdentifier: Parameter + Member + MaxEncodedLen + Ord + Copy; + type FreezeIdentifier: Parameter + Member + MaxEncodedLen + Copy; /// The maximum number of locks that should exist on an account. /// Not strictly enforced, but used for weight estimation. diff --git a/substrate/frame/beefy-mmr/src/mock.rs b/substrate/frame/beefy-mmr/src/mock.rs index b2d8758a04be6..b85096813deca 100644 --- a/substrate/frame/beefy-mmr/src/mock.rs +++ b/substrate/frame/beefy-mmr/src/mock.rs @@ -19,16 +19,15 @@ use std::vec; use codec::Encode; use frame_support::{ - construct_runtime, parameter_types, - traits::{ConstU16, ConstU32, ConstU64}, + construct_runtime, derive_impl, parameter_types, + traits::{ConstU32, ConstU64}, }; use sp_consensus_beefy::mmr::MmrLeafVersion; -use sp_core::H256; use sp_io::TestExternalities; use sp_runtime::{ app_crypto::ecdsa::Public, impl_opaque_keys, - traits::{BlakeTwo256, ConvertInto, IdentityLookup, Keccak256, OpaqueKeys}, + traits::{ConvertInto, Keccak256, OpaqueKeys}, BuildStorage, }; use sp_state_machine::BasicExternalities; @@ -58,30 +57,9 @@ construct_runtime!( } ); +#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] impl frame_system::Config for Test { - type BaseCallFilter = frame_support::traits::Everything; - type BlockWeights = (); - type BlockLength = (); - type DbWeight = (); - type RuntimeOrigin = RuntimeOrigin; - type Nonce = u64; - type Hash = H256; - type RuntimeCall = RuntimeCall; - type Hashing = BlakeTwo256; - type AccountId = u64; - type Lookup = IdentityLookup; type Block = Block; - type RuntimeEvent = RuntimeEvent; - type BlockHashCount = ConstU64<250>; - type Version = (); - type PalletInfo = PalletInfo; - type AccountData = (); - type OnNewAccount = (); - type OnKilledAccount = (); - type SystemWeightInfo = (); - type SS58Prefix = ConstU16<42>; - type OnSetCode = (); - type MaxConsumers = ConstU32<16>; } impl pallet_session::Config for Test { diff --git a/substrate/frame/beefy/src/mock.rs b/substrate/frame/beefy/src/mock.rs index b55a65dbd73af..9480fd0406d78 100644 --- a/substrate/frame/beefy/src/mock.rs +++ b/substrate/frame/beefy/src/mock.rs @@ -22,19 +22,15 @@ use frame_election_provider_support::{ onchain, SequentialPhragmen, }; use frame_support::{ - construct_runtime, parameter_types, - traits::{ConstU16, ConstU32, ConstU64, KeyOwnerProofSystem, OnFinalize, OnInitialize}, + construct_runtime, derive_impl, parameter_types, + traits::{ConstU32, ConstU64, KeyOwnerProofSystem, OnFinalize, OnInitialize}, }; use pallet_session::historical as pallet_session_historical; -use sp_core::{crypto::KeyTypeId, ConstU128, H256}; +use sp_core::{crypto::KeyTypeId, ConstU128}; use sp_io::TestExternalities; use sp_runtime::{ - app_crypto::ecdsa::Public, - curve::PiecewiseLinear, - impl_opaque_keys, - testing::TestXt, - traits::{BlakeTwo256, IdentityLookup, OpaqueKeys}, - BuildStorage, Perbill, + app_crypto::ecdsa::Public, curve::PiecewiseLinear, impl_opaque_keys, testing::TestXt, + traits::OpaqueKeys, BuildStorage, Perbill, }; use sp_staking::{EraIndex, SessionIndex}; use sp_state_machine::BasicExternalities; @@ -69,30 +65,10 @@ construct_runtime!( } ); +#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] impl frame_system::Config for Test { - type BaseCallFilter = frame_support::traits::Everything; - type BlockWeights = (); - type BlockLength = (); - type DbWeight = (); - type RuntimeOrigin = RuntimeOrigin; - type Nonce = u64; - type Hash = H256; - type RuntimeCall = RuntimeCall; - type Hashing = BlakeTwo256; - type AccountId = u64; - type Lookup = IdentityLookup; type Block = Block; - type RuntimeEvent = RuntimeEvent; - type BlockHashCount = ConstU64<250>; - type Version = (); - type PalletInfo = PalletInfo; type AccountData = pallet_balances::AccountData; - type OnNewAccount = (); - type OnKilledAccount = (); - type SystemWeightInfo = (); - type SS58Prefix = ConstU16<42>; - type OnSetCode = (); - type MaxConsumers = ConstU32<16>; } impl frame_system::offchain::SendTransactionTypes for Test diff --git a/substrate/frame/contracts/src/wasm/prepare.rs b/substrate/frame/contracts/src/wasm/prepare.rs index b129c17e13eca..dfe8c4f8f9b91 100644 --- a/substrate/frame/contracts/src/wasm/prepare.rs +++ b/substrate/frame/contracts/src/wasm/prepare.rs @@ -79,8 +79,7 @@ impl LoadedModule { } let engine = Engine::new(&config); - let module = - Module::new(&engine, code.clone()).map_err(|_| "Can't load the module into wasmi!")?; + let module = Module::new(&engine, code).map_err(|_| "Can't load the module into wasmi!")?; // Return a `LoadedModule` instance with // __valid__ module. diff --git a/substrate/frame/identity/src/benchmarking.rs b/substrate/frame/identity/src/benchmarking.rs index 4b51d23f6b34f..059de204bbf77 100644 --- a/substrate/frame/identity/src/benchmarking.rs +++ b/substrate/frame/identity/src/benchmarking.rs @@ -22,7 +22,9 @@ use super::*; use crate::Pallet as Identity; -use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller, BenchmarkError}; +use frame_benchmarking::{ + account, impl_benchmark_test_suite, v2::*, whitelisted_caller, BenchmarkError, +}; use frame_support::{ ensure, traits::{EnsureOrigin, Get}, @@ -118,110 +120,128 @@ fn create_identity_info(num_fields: u32) -> IdentityInfo add_registrars::(r)?; +#[benchmarks] +mod benchmarks { + use super::*; + + #[benchmark] + fn add_registrar(r: Linear<1, { T::MaxRegistrars::get() - 1 }>) -> Result<(), BenchmarkError> { + add_registrars::(r)?; ensure!(Registrars::::get().len() as u32 == r, "Registrars not set up correctly."); let origin = T::RegistrarOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let account = T::Lookup::unlookup(account("registrar", r + 1, SEED)); - }: _(origin, account) - verify { + + #[extrinsic_call] + _(origin as T::RuntimeOrigin, account); + ensure!(Registrars::::get().len() as u32 == r + 1, "Registrars not added."); + Ok(()) } - set_identity { - let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; - let x in 0 .. T::MaxAdditionalFields::get(); - let caller = { - // The target user - let caller: T::AccountId = whitelisted_caller(); - let caller_lookup = T::Lookup::unlookup(caller.clone()); - let caller_origin: ::RuntimeOrigin = RawOrigin::Signed(caller.clone()).into(); - let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - - // Add an initial identity - let initial_info = create_identity_info::(1); - Identity::::set_identity(caller_origin.clone(), Box::new(initial_info.clone()))?; - - // User requests judgement from all the registrars, and they approve - for i in 0..r { - let registrar: T::AccountId = account("registrar", i, SEED); - let registrar_lookup = T::Lookup::unlookup(registrar.clone()); - let balance_to_use = T::Currency::minimum_balance() * 10u32.into(); - let _ = T::Currency::make_free_balance_be(®istrar, balance_to_use); - - Identity::::request_judgement(caller_origin.clone(), i, 10u32.into())?; - Identity::::provide_judgement( - RawOrigin::Signed(registrar).into(), - i, - caller_lookup.clone(), - Judgement::Reasonable, - T::Hashing::hash_of(&initial_info), - )?; - } - caller - }; - }: _(RawOrigin::Signed(caller.clone()), Box::new(create_identity_info::(x))) - verify { + #[benchmark] + fn set_identity( + r: Linear<1, { T::MaxRegistrars::get() }>, + x: Linear<0, { T::MaxAdditionalFields::get() }>, + ) -> Result<(), BenchmarkError> { + add_registrars::(r)?; + + let caller: T::AccountId = whitelisted_caller(); + let caller_lookup = T::Lookup::unlookup(caller.clone()); + let caller_origin: ::RuntimeOrigin = + RawOrigin::Signed(caller.clone()).into(); + let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); + + // Add an initial identity + let initial_info = create_identity_info::(1); + Identity::::set_identity(caller_origin.clone(), Box::new(initial_info.clone()))?; + + // User requests judgement from all the registrars, and they approve + for i in 0..r { + let registrar: T::AccountId = account("registrar", i, SEED); + let _ = T::Lookup::unlookup(registrar.clone()); + let balance_to_use = T::Currency::minimum_balance() * 10u32.into(); + let _ = T::Currency::make_free_balance_be(®istrar, balance_to_use); + + Identity::::request_judgement(caller_origin.clone(), i, 10u32.into())?; + Identity::::provide_judgement( + RawOrigin::Signed(registrar).into(), + i, + caller_lookup.clone(), + Judgement::Reasonable, + T::Hashing::hash_of(&initial_info), + )?; + } + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), Box::new(create_identity_info::(x))); + assert_last_event::(Event::::IdentitySet { who: caller }.into()); + Ok(()) } // We need to split `set_subs` into two benchmarks to accurately isolate the potential // writes caused by new or old sub accounts. The actual weight should simply be // the sum of these two weights. - set_subs_new { + #[benchmark] + fn set_subs_new(s: Linear<0, { T::MaxSubAccounts::get() }>) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); - // Create a new subs vec with s sub accounts - let s in 0 .. T::MaxSubAccounts::get() => (); + + // Create a new subs vec with sub accounts let subs = create_sub_accounts::(&caller, s)?; ensure!(SubsOf::::get(&caller).1.len() == 0, "Caller already has subs"); - }: set_subs(RawOrigin::Signed(caller.clone()), subs) - verify { + + #[extrinsic_call] + set_subs(RawOrigin::Signed(caller.clone()), subs); + ensure!(SubsOf::::get(&caller).1.len() as u32 == s, "Subs not added"); + Ok(()) } - set_subs_old { + #[benchmark] + fn set_subs_old(p: Linear<0, { T::MaxSubAccounts::get() }>) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); + // Give them p many previous sub accounts. - let p in 0 .. T::MaxSubAccounts::get() => { - let _ = add_sub_accounts::(&caller, p)?; - }; + let _ = add_sub_accounts::(&caller, p)?; + // Remove all subs. let subs = create_sub_accounts::(&caller, 0)?; - ensure!( - SubsOf::::get(&caller).1.len() as u32 == p, - "Caller does have subs", - ); - }: set_subs(RawOrigin::Signed(caller.clone()), subs) - verify { + ensure!(SubsOf::::get(&caller).1.len() as u32 == p, "Caller does have subs",); + + #[extrinsic_call] + set_subs(RawOrigin::Signed(caller.clone()), subs); + ensure!(SubsOf::::get(&caller).1.len() == 0, "Subs not removed"); + Ok(()) } - clear_identity { + #[benchmark] + fn clear_identity( + r: Linear<1, { T::MaxRegistrars::get() }>, + s: Linear<0, { T::MaxSubAccounts::get() }>, + x: Linear<0, { T::MaxAdditionalFields::get() }>, + ) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); - let caller_origin = ::RuntimeOrigin::from(RawOrigin::Signed(caller.clone())); + let caller_origin = + ::RuntimeOrigin::from(RawOrigin::Signed(caller.clone())); let caller_lookup = ::unlookup(caller.clone()); let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; - let s in 0 .. T::MaxSubAccounts::get() => { - // Give them s many sub accounts - let caller: T::AccountId = whitelisted_caller(); - let _ = add_sub_accounts::(&caller, s)?; - }; - let x in 0 .. T::MaxAdditionalFields::get(); + // Register the registrars + add_registrars::(r)?; + + // Add sub accounts + let _ = add_sub_accounts::(&caller, s)?; // Create their main identity with x additional fields let info = create_identity_info::(x); - let caller: T::AccountId = whitelisted_caller(); - let caller_origin = ::RuntimeOrigin::from(RawOrigin::Signed(caller.clone())); Identity::::set_identity(caller_origin.clone(), Box::new(info.clone()))?; // User requests judgement from all the registrars, and they approve for i in 0..r { let registrar: T::AccountId = account("registrar", i, SEED); - let balance_to_use = T::Currency::minimum_balance() * 10u32.into(); + let balance_to_use = T::Currency::minimum_balance() * 10u32.into(); let _ = T::Currency::make_free_balance_be(®istrar, balance_to_use); Identity::::request_judgement(caller_origin.clone(), i, 10u32.into())?; @@ -233,111 +253,175 @@ benchmarks! { T::Hashing::hash_of(&info), )?; } + ensure!(IdentityOf::::contains_key(&caller), "Identity does not exist."); - }: _(RawOrigin::Signed(caller.clone())) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone())); + ensure!(!IdentityOf::::contains_key(&caller), "Identity not cleared."); + Ok(()) } - request_judgement { + #[benchmark] + fn request_judgement( + r: Linear<1, { T::MaxRegistrars::get() }>, + x: Linear<0, { T::MaxAdditionalFields::get() }>, + ) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; - let x in 0 .. T::MaxAdditionalFields::get() => { - // Create their main identity with x additional fields - let info = create_identity_info::(x); - let caller: T::AccountId = whitelisted_caller(); - let caller_origin = ::RuntimeOrigin::from(RawOrigin::Signed(caller)); - Identity::::set_identity(caller_origin, Box::new(info))?; - }; - }: _(RawOrigin::Signed(caller.clone()), r - 1, 10u32.into()) - verify { - assert_last_event::(Event::::JudgementRequested { who: caller, registrar_index: r-1 }.into()); + // Register the registrars + add_registrars::(r)?; + + // Create their main identity with x additional fields + let info = create_identity_info::(x); + let caller_origin = + ::RuntimeOrigin::from(RawOrigin::Signed(caller.clone())); + Identity::::set_identity(caller_origin.clone(), Box::new(info))?; + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), r - 1, 10u32.into()); + + assert_last_event::( + Event::::JudgementRequested { who: caller, registrar_index: r - 1 }.into(), + ); + + Ok(()) } - cancel_request { + #[benchmark] + fn cancel_request( + r: Linear<1, { T::MaxRegistrars::get() }>, + x: Linear<0, { T::MaxAdditionalFields::get() }>, + ) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); - let caller_origin = ::RuntimeOrigin::from(RawOrigin::Signed(caller.clone())); let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; - let x in 0 .. T::MaxAdditionalFields::get() => { - // Create their main identity with x additional fields - let info = create_identity_info::(x); - let caller: T::AccountId = whitelisted_caller(); - let caller_origin = ::RuntimeOrigin::from(RawOrigin::Signed(caller)); - Identity::::set_identity(caller_origin, Box::new(info))?; - }; - - Identity::::request_judgement(caller_origin, r - 1, 10u32.into())?; - }: _(RawOrigin::Signed(caller.clone()), r - 1) - verify { - assert_last_event::(Event::::JudgementUnrequested { who: caller, registrar_index: r-1 }.into()); + // Register the registrars + add_registrars::(r)?; + + // Create their main identity with x additional fields + let info = create_identity_info::(x); + let caller_origin = + ::RuntimeOrigin::from(RawOrigin::Signed(caller.clone())); + Identity::::set_identity(caller_origin.clone(), Box::new(info))?; + + Identity::::request_judgement(caller_origin.clone(), r - 1, 10u32.into())?; + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), r - 1); + + assert_last_event::( + Event::::JudgementUnrequested { who: caller, registrar_index: r - 1 }.into(), + ); + + Ok(()) } - set_fee { + #[benchmark] + fn set_fee(r: Linear<1, { T::MaxRegistrars::get() - 1 }>) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); let caller_lookup = T::Lookup::unlookup(caller.clone()); - let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; + add_registrars::(r)?; let registrar_origin = T::RegistrarOrigin::try_successful_origin() .expect("RegistrarOrigin has no successful origin required for the benchmark"); Identity::::add_registrar(registrar_origin, caller_lookup)?; + let registrars = Registrars::::get(); ensure!(registrars[r as usize].as_ref().unwrap().fee == 0u32.into(), "Fee already set."); - }: _(RawOrigin::Signed(caller), r, 100u32.into()) - verify { - let registrars = Registrars::::get(); - ensure!(registrars[r as usize].as_ref().unwrap().fee == 100u32.into(), "Fee not changed."); + + #[extrinsic_call] + _(RawOrigin::Signed(caller), r, 100u32.into()); + + let updated_registrars = Registrars::::get(); + ensure!( + updated_registrars[r as usize].as_ref().unwrap().fee == 100u32.into(), + "Fee not changed." + ); + + Ok(()) } - set_account_id { + #[benchmark] + fn set_account_id(r: Linear<1, { T::MaxRegistrars::get() - 1 }>) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); let caller_lookup = T::Lookup::unlookup(caller.clone()); let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; + add_registrars::(r)?; let registrar_origin = T::RegistrarOrigin::try_successful_origin() .expect("RegistrarOrigin has no successful origin required for the benchmark"); Identity::::add_registrar(registrar_origin, caller_lookup)?; + let registrars = Registrars::::get(); ensure!(registrars[r as usize].as_ref().unwrap().account == caller, "id not set."); + let new_account = T::Lookup::unlookup(account("new", 0, SEED)); - }: _(RawOrigin::Signed(caller), r, new_account) - verify { - let registrars = Registrars::::get(); - ensure!(registrars[r as usize].as_ref().unwrap().account == account("new", 0, SEED), "id not changed."); + + #[extrinsic_call] + _(RawOrigin::Signed(caller), r, new_account); + + let updated_registrars = Registrars::::get(); + ensure!( + updated_registrars[r as usize].as_ref().unwrap().account == account("new", 0, SEED), + "id not changed." + ); + + Ok(()) } - set_fields { + #[benchmark] + fn set_fields(r: Linear<1, { T::MaxRegistrars::get() - 1 }>) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); let caller_lookup = T::Lookup::unlookup(caller.clone()); let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; + add_registrars::(r)?; let registrar_origin = T::RegistrarOrigin::try_successful_origin() .expect("RegistrarOrigin has no successful origin required for the benchmark"); Identity::::add_registrar(registrar_origin, caller_lookup)?; - let fields = IdentityFields( - IdentityField::Display | IdentityField::Legal | IdentityField::Web | IdentityField::Riot - | IdentityField::Email | IdentityField::PgpFingerprint | IdentityField::Image | IdentityField::Twitter - ); - let registrars = Registrars::::get(); - ensure!(registrars[r as usize].as_ref().unwrap().fields == Default::default(), "fields already set."); - }: _(RawOrigin::Signed(caller), r, fields) - verify { + + let fields = + IdentityFields( + IdentityField::Display | + IdentityField::Legal | IdentityField::Web | + IdentityField::Riot | IdentityField::Email | + IdentityField::PgpFingerprint | + IdentityField::Image | IdentityField::Twitter, + ); + let registrars = Registrars::::get(); - ensure!(registrars[r as usize].as_ref().unwrap().fields != Default::default(), "fields not set."); + ensure!( + registrars[r as usize].as_ref().unwrap().fields == Default::default(), + "fields already set." + ); + + #[extrinsic_call] + _(RawOrigin::Signed(caller), r, fields); + + let updated_registrars = Registrars::::get(); + ensure!( + updated_registrars[r as usize].as_ref().unwrap().fields != Default::default(), + "fields not set." + ); + + Ok(()) } - provide_judgement { + #[benchmark] + fn provide_judgement( + r: Linear<1, { T::MaxRegistrars::get() - 1 }>, + x: Linear<0, { T::MaxAdditionalFields::get() }>, + ) -> Result<(), BenchmarkError> { // The user let user: T::AccountId = account("user", r, SEED); - let user_origin = ::RuntimeOrigin::from(RawOrigin::Signed(user.clone())); + let user_origin = + ::RuntimeOrigin::from(RawOrigin::Signed(user.clone())); let user_lookup = ::unlookup(user.clone()); let _ = T::Currency::make_free_balance_be(&user, BalanceOf::::max_value()); @@ -345,8 +429,7 @@ benchmarks! { let caller_lookup = T::Lookup::unlookup(caller.clone()); let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; - let x in 0 .. T::MaxAdditionalFields::get(); + add_registrars::(r)?; let info = create_identity_info::(x); let info_hash = T::Hashing::hash_of(&info); @@ -356,18 +439,28 @@ benchmarks! { .expect("RegistrarOrigin has no successful origin required for the benchmark"); Identity::::add_registrar(registrar_origin, caller_lookup)?; Identity::::request_judgement(user_origin, r, 10u32.into())?; - }: _(RawOrigin::Signed(caller), r, user_lookup, Judgement::Reasonable, info_hash) - verify { - assert_last_event::(Event::::JudgementGiven { target: user, registrar_index: r }.into()) + + #[extrinsic_call] + _(RawOrigin::Signed(caller), r, user_lookup, Judgement::Reasonable, info_hash); + + assert_last_event::( + Event::::JudgementGiven { target: user, registrar_index: r }.into(), + ); + + Ok(()) } - kill_identity { - let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; - let s in 0 .. T::MaxSubAccounts::get(); - let x in 0 .. T::MaxAdditionalFields::get(); + #[benchmark] + fn kill_identity( + r: Linear<1, { T::MaxRegistrars::get() }>, + s: Linear<0, { T::MaxSubAccounts::get() }>, + x: Linear<0, { T::MaxAdditionalFields::get() }>, + ) -> Result<(), BenchmarkError> { + add_registrars::(r)?; let target: T::AccountId = account("target", 0, SEED); - let target_origin: ::RuntimeOrigin = RawOrigin::Signed(target.clone()).into(); + let target_origin: ::RuntimeOrigin = + RawOrigin::Signed(target.clone()).into(); let target_lookup = T::Lookup::unlookup(target.clone()); let _ = T::Currency::make_free_balance_be(&target, BalanceOf::::max_value()); @@ -378,7 +471,7 @@ benchmarks! { // User requests judgement from all the registrars, and they approve for i in 0..r { let registrar: T::AccountId = account("registrar", i, SEED); - let balance_to_use = T::Currency::minimum_balance() * 10u32.into(); + let balance_to_use = T::Currency::minimum_balance() * 10u32.into(); let _ = T::Currency::make_free_balance_be(®istrar, balance_to_use); Identity::::request_judgement(target_origin.clone(), i, 10u32.into())?; @@ -390,62 +483,86 @@ benchmarks! { T::Hashing::hash_of(&info), )?; } + ensure!(IdentityOf::::contains_key(&target), "Identity not set"); + let origin = T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: _(origin, target_lookup) - verify { + + #[extrinsic_call] + _(origin as T::RuntimeOrigin, target_lookup); + ensure!(!IdentityOf::::contains_key(&target), "Identity not removed"); - } - add_sub { - let s in 0 .. T::MaxSubAccounts::get() - 1; + Ok(()) + } + #[benchmark] + fn add_sub(s: Linear<0, { T::MaxSubAccounts::get() - 1 }>) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); let _ = add_sub_accounts::(&caller, s)?; let sub = account("new_sub", 0, SEED); let data = Data::Raw(vec![0; 32].try_into().unwrap()); + ensure!(SubsOf::::get(&caller).1.len() as u32 == s, "Subs not set."); - }: _(RawOrigin::Signed(caller.clone()), T::Lookup::unlookup(sub), data) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), T::Lookup::unlookup(sub), data); + ensure!(SubsOf::::get(&caller).1.len() as u32 == s + 1, "Subs not added."); - } - rename_sub { - let s in 1 .. T::MaxSubAccounts::get(); + Ok(()) + } + #[benchmark] + fn rename_sub(s: Linear<1, { T::MaxSubAccounts::get() }>) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); let (sub, _) = add_sub_accounts::(&caller, s)?.remove(0); let data = Data::Raw(vec![1; 32].try_into().unwrap()); + ensure!(SuperOf::::get(&sub).unwrap().1 != data, "data already set"); - }: _(RawOrigin::Signed(caller), T::Lookup::unlookup(sub.clone()), data.clone()) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller), T::Lookup::unlookup(sub.clone()), data.clone()); + ensure!(SuperOf::::get(&sub).unwrap().1 == data, "data not set"); - } - remove_sub { - let s in 1 .. T::MaxSubAccounts::get(); + Ok(()) + } + #[benchmark] + fn remove_sub(s: Linear<1, { T::MaxSubAccounts::get() }>) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); let (sub, _) = add_sub_accounts::(&caller, s)?.remove(0); ensure!(SuperOf::::contains_key(&sub), "Sub doesn't exists"); - }: _(RawOrigin::Signed(caller), T::Lookup::unlookup(sub.clone())) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller), T::Lookup::unlookup(sub.clone())); + ensure!(!SuperOf::::contains_key(&sub), "Sub not removed"); - } - quit_sub { - let s in 0 .. T::MaxSubAccounts::get() - 1; + Ok(()) + } + #[benchmark] + fn quit_sub(s: Linear<0, { T::MaxSubAccounts::get() - 1 }>) -> Result<(), BenchmarkError> { let caller: T::AccountId = whitelisted_caller(); let sup = account("super", 0, SEED); let _ = add_sub_accounts::(&sup, s)?; let sup_origin = RawOrigin::Signed(sup).into(); - Identity::::add_sub(sup_origin, T::Lookup::unlookup(caller.clone()), Data::Raw(vec![0; 32].try_into().unwrap()))?; + Identity::::add_sub( + sup_origin, + T::Lookup::unlookup(caller.clone()), + Data::Raw(vec![0; 32].try_into().unwrap()), + )?; ensure!(SuperOf::::contains_key(&caller), "Sub doesn't exists"); - }: _(RawOrigin::Signed(caller.clone())) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone())); + ensure!(!SuperOf::::contains_key(&caller), "Sub not removed"); + + Ok(()) } impl_benchmark_test_suite!(Identity, crate::tests::new_test_ext(), crate::tests::Test); diff --git a/substrate/frame/merkle-mountain-range/src/mock.rs b/substrate/frame/merkle-mountain-range/src/mock.rs index ecc254278bf0f..d3cb4e5702972 100644 --- a/substrate/frame/merkle-mountain-range/src/mock.rs +++ b/substrate/frame/merkle-mountain-range/src/mock.rs @@ -19,13 +19,9 @@ use crate as pallet_mmr; use crate::*; use codec::{Decode, Encode}; -use frame_support::{ - parameter_types, - traits::{ConstU32, ConstU64}, -}; -use sp_core::H256; +use frame_support::{derive_impl, parameter_types}; use sp_mmr_primitives::{Compact, LeafDataProvider}; -use sp_runtime::traits::{BlakeTwo256, IdentityLookup, Keccak256}; +use sp_runtime::traits::Keccak256; type Block = frame_system::mocking::MockBlock; @@ -37,30 +33,9 @@ frame_support::construct_runtime!( } ); +#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] impl frame_system::Config for Test { - type BaseCallFilter = frame_support::traits::Everything; - type RuntimeOrigin = RuntimeOrigin; - type RuntimeCall = RuntimeCall; - type Nonce = u64; - type Hash = H256; - type Hashing = BlakeTwo256; - type AccountId = sp_core::sr25519::Public; - type Lookup = IdentityLookup; type Block = Block; - type RuntimeEvent = RuntimeEvent; - type BlockHashCount = ConstU64<250>; - type DbWeight = (); - type BlockWeights = (); - type BlockLength = (); - type Version = (); - type PalletInfo = PalletInfo; - type AccountData = (); - type OnNewAccount = (); - type OnKilledAccount = (); - type SystemWeightInfo = (); - type SS58Prefix = (); - type OnSetCode = (); - type MaxConsumers = ConstU32<16>; } impl Config for Test { diff --git a/substrate/frame/nfts/src/tests.rs b/substrate/frame/nfts/src/tests.rs index 6e264048f11a6..a82fcca015121 100644 --- a/substrate/frame/nfts/src/tests.rs +++ b/substrate/frame/nfts/src/tests.rs @@ -17,7 +17,7 @@ //! Tests for Nfts pallet. -use crate::{mock::*, Event, *}; +use crate::{mock::*, Event, SystemConfig, *}; use enumflags2::BitFlags; use frame_support::{ assert_noop, assert_ok, diff --git a/substrate/frame/preimage/src/migration.rs b/substrate/frame/preimage/src/migration.rs index 821cb01bbaae5..a86109f892a4f 100644 --- a/substrate/frame/preimage/src/migration.rs +++ b/substrate/frame/preimage/src/migration.rs @@ -133,7 +133,7 @@ pub mod v1 { None => OldRequestStatus::Requested { deposit: None, count: 1, len: Some(len) }, }, - v0::OldRequestStatus::Requested(count) if count == 0 => { + v0::OldRequestStatus::Requested(0) => { log::error!(target: TARGET, "preimage has counter of zero: {:?}", hash); continue }, diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/composite_helper.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/composite_helper.rs new file mode 100644 index 0000000000000..3c81d2360cb7b --- /dev/null +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/composite_helper.rs @@ -0,0 +1,70 @@ +// 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 + +use crate::construct_runtime::parse::PalletPath; +use proc_macro2::{Ident, TokenStream}; +use quote::quote; + +pub(crate) fn expand_conversion_fn( + composite_name: &str, + path: &PalletPath, + instance: Option<&Ident>, + variant_name: &Ident, +) -> TokenStream { + let composite_name = quote::format_ident!("{}", composite_name); + let runtime_composite_name = quote::format_ident!("Runtime{}", composite_name); + + if let Some(inst) = instance { + quote! { + impl From<#path::#composite_name<#path::#inst>> for #runtime_composite_name { + fn from(hr: #path::#composite_name<#path::#inst>) -> Self { + #runtime_composite_name::#variant_name(hr) + } + } + } + } else { + quote! { + impl From<#path::#composite_name> for #runtime_composite_name { + fn from(hr: #path::#composite_name) -> Self { + #runtime_composite_name::#variant_name(hr) + } + } + } + } +} + +pub(crate) fn expand_variant( + composite_name: &str, + index: u8, + path: &PalletPath, + instance: Option<&Ident>, + variant_name: &Ident, +) -> TokenStream { + let composite_name = quote::format_ident!("{}", composite_name); + + if let Some(inst) = instance { + quote! { + #[codec(index = #index)] + #variant_name(#path::#composite_name<#path::#inst>), + } + } else { + quote! { + #[codec(index = #index)] + #variant_name(#path::#composite_name), + } + } +} diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/freeze_reason.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/freeze_reason.rs index b142f8e84c92f..18790850d6b3c 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/freeze_reason.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/freeze_reason.rs @@ -15,8 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License -use crate::construct_runtime::{parse::PalletPath, Pallet}; -use proc_macro2::{Ident, TokenStream}; +use super::composite_helper; +use crate::construct_runtime::Pallet; +use proc_macro2::TokenStream; use quote::quote; pub fn expand_outer_freeze_reason(pallet_decls: &[Pallet], scrate: &TokenStream) -> TokenStream { @@ -27,17 +28,29 @@ pub fn expand_outer_freeze_reason(pallet_decls: &[Pallet], scrate: &TokenStream) let variant_name = &decl.name; let path = &decl.path; let index = decl.index; + let instance = decl.instance.as_ref(); - conversion_fns.push(expand_conversion_fn(path, variant_name)); + conversion_fns.push(composite_helper::expand_conversion_fn( + "FreezeReason", + path, + instance, + variant_name, + )); - freeze_reason_variants.push(expand_variant(index, path, variant_name)); + freeze_reason_variants.push(composite_helper::expand_variant( + "FreezeReason", + index, + path, + instance, + variant_name, + )); } } quote! { /// A reason for placing a freeze on funds. #[derive( - Copy, Clone, Eq, PartialEq, Ord, PartialOrd, + Copy, Clone, Eq, PartialEq, #scrate::__private::codec::Encode, #scrate::__private::codec::Decode, #scrate::__private::codec::MaxEncodedLen, #scrate::__private::scale_info::TypeInfo, #scrate::__private::RuntimeDebug, @@ -49,20 +62,3 @@ pub fn expand_outer_freeze_reason(pallet_decls: &[Pallet], scrate: &TokenStream) #( #conversion_fns )* } } - -fn expand_conversion_fn(path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - impl From<#path::FreezeReason> for RuntimeFreezeReason { - fn from(hr: #path::FreezeReason) -> Self { - RuntimeFreezeReason::#variant_name(hr) - } - } - } -} - -fn expand_variant(index: u8, path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - #[codec(index = #index)] - #variant_name(#path::FreezeReason), - } -} diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs index ed7183c4a150d..1bb7462133cdd 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/hold_reason.rs @@ -15,8 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License -use crate::construct_runtime::{parse::PalletPath, Pallet}; -use proc_macro2::{Ident, TokenStream}; +use super::composite_helper; +use crate::construct_runtime::Pallet; +use proc_macro2::TokenStream; use quote::quote; pub fn expand_outer_hold_reason(pallet_decls: &[Pallet], scrate: &TokenStream) -> TokenStream { @@ -27,17 +28,29 @@ pub fn expand_outer_hold_reason(pallet_decls: &[Pallet], scrate: &TokenStream) - let variant_name = &decl.name; let path = &decl.path; let index = decl.index; + let instance = decl.instance.as_ref(); - conversion_fns.push(expand_conversion_fn(path, variant_name)); + conversion_fns.push(composite_helper::expand_conversion_fn( + "HoldReason", + path, + instance, + variant_name, + )); - hold_reason_variants.push(expand_variant(index, path, variant_name)); + hold_reason_variants.push(composite_helper::expand_variant( + "HoldReason", + index, + path, + instance, + variant_name, + )); } } quote! { /// A reason for placing a hold on funds. #[derive( - Copy, Clone, Eq, PartialEq, Ord, PartialOrd, + Copy, Clone, Eq, PartialEq, #scrate::__private::codec::Encode, #scrate::__private::codec::Decode, #scrate::__private::codec::MaxEncodedLen, #scrate::__private::scale_info::TypeInfo, #scrate::__private::RuntimeDebug, @@ -49,20 +62,3 @@ pub fn expand_outer_hold_reason(pallet_decls: &[Pallet], scrate: &TokenStream) - #( #conversion_fns )* } } - -fn expand_conversion_fn(path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - impl From<#path::HoldReason> for RuntimeHoldReason { - fn from(hr: #path::HoldReason) -> Self { - RuntimeHoldReason::#variant_name(hr) - } - } - } -} - -fn expand_variant(index: u8, path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - #[codec(index = #index)] - #variant_name(#path::HoldReason), - } -} diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/lock_id.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/lock_id.rs index ba35147a051fb..e67c0da00ea15 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/lock_id.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/lock_id.rs @@ -15,8 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License -use crate::construct_runtime::{parse::PalletPath, Pallet}; -use proc_macro2::{Ident, TokenStream}; +use super::composite_helper; +use crate::construct_runtime::Pallet; +use proc_macro2::TokenStream; use quote::quote; pub fn expand_outer_lock_id(pallet_decls: &[Pallet], scrate: &TokenStream) -> TokenStream { @@ -27,17 +28,29 @@ pub fn expand_outer_lock_id(pallet_decls: &[Pallet], scrate: &TokenStream) -> To let variant_name = &decl.name; let path = &decl.path; let index = decl.index; + let instance = decl.instance.as_ref(); - conversion_fns.push(expand_conversion_fn(path, variant_name)); + conversion_fns.push(composite_helper::expand_conversion_fn( + "LockId", + path, + instance, + variant_name, + )); - lock_id_variants.push(expand_variant(index, path, variant_name)); + lock_id_variants.push(composite_helper::expand_variant( + "LockId", + index, + path, + instance, + variant_name, + )); } } quote! { /// An identifier for each lock placed on funds. #[derive( - Copy, Clone, Eq, PartialEq, Ord, PartialOrd, + Copy, Clone, Eq, PartialEq, #scrate::__private::codec::Encode, #scrate::__private::codec::Decode, #scrate::__private::codec::MaxEncodedLen, #scrate::__private::scale_info::TypeInfo, #scrate::__private::RuntimeDebug, @@ -49,20 +62,3 @@ pub fn expand_outer_lock_id(pallet_decls: &[Pallet], scrate: &TokenStream) -> To #( #conversion_fns )* } } - -fn expand_conversion_fn(path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - impl From<#path::LockId> for RuntimeLockId { - fn from(hr: #path::LockId) -> Self { - RuntimeLockId::#variant_name(hr) - } - } - } -} - -fn expand_variant(index: u8, path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - #[codec(index = #index)] - #variant_name(#path::LockId), - } -} diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/mod.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/mod.rs index 830338f9265ff..a0fc6b8130b3c 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/mod.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/mod.rs @@ -16,6 +16,7 @@ // limitations under the License mod call; +pub mod composite_helper; mod config; mod freeze_reason; mod hold_reason; diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/slash_reason.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/slash_reason.rs index 2a3283230ad84..892b842b17487 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/slash_reason.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/slash_reason.rs @@ -15,8 +15,9 @@ // See the License for the specific language governing permissions and // limitations under the License -use crate::construct_runtime::{parse::PalletPath, Pallet}; -use proc_macro2::{Ident, TokenStream}; +use super::composite_helper; +use crate::construct_runtime::Pallet; +use proc_macro2::TokenStream; use quote::quote; pub fn expand_outer_slash_reason(pallet_decls: &[Pallet], scrate: &TokenStream) -> TokenStream { @@ -27,17 +28,29 @@ pub fn expand_outer_slash_reason(pallet_decls: &[Pallet], scrate: &TokenStream) let variant_name = &decl.name; let path = &decl.path; let index = decl.index; + let instance = decl.instance.as_ref(); - conversion_fns.push(expand_conversion_fn(path, variant_name)); + conversion_fns.push(composite_helper::expand_conversion_fn( + "SlashReason", + path, + instance, + variant_name, + )); - slash_reason_variants.push(expand_variant(index, path, variant_name)); + slash_reason_variants.push(composite_helper::expand_variant( + "SlashReason", + index, + path, + instance, + variant_name, + )); } } quote! { /// A reason for slashing funds. #[derive( - Copy, Clone, Eq, PartialEq, Ord, PartialOrd, + Copy, Clone, Eq, PartialEq, #scrate::__private::codec::Encode, #scrate::__private::codec::Decode, #scrate::__private::codec::MaxEncodedLen, #scrate::__private::scale_info::TypeInfo, #scrate::__private::RuntimeDebug, @@ -49,20 +62,3 @@ pub fn expand_outer_slash_reason(pallet_decls: &[Pallet], scrate: &TokenStream) #( #conversion_fns )* } } - -fn expand_conversion_fn(path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - impl From<#path::SlashReason> for RuntimeSlashReason { - fn from(hr: #path::SlashReason) -> Self { - RuntimeSlashReason::#variant_name(hr) - } - } - } -} - -fn expand_variant(index: u8, path: &PalletPath, variant_name: &Ident) -> TokenStream { - quote! { - #[codec(index = #index)] - #variant_name(#path::SlashReason), - } -} diff --git a/substrate/frame/support/procedural/src/pallet/parse/composite.rs b/substrate/frame/support/procedural/src/pallet/parse/composite.rs index cb554a116175c..ddcc2d07f93b3 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/composite.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/composite.rs @@ -15,6 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use super::helper; use quote::ToTokens; use syn::spanned::Spanned; @@ -108,6 +109,13 @@ impl CompositeDef { return Err(syn::Error::new(item.span(), msg)) } + let has_instance = if item.generics.params.first().is_some() { + helper::check_config_def_gen(&item.generics, item.ident.span())?; + true + } else { + false + }; + let has_derive_attr = item.attrs.iter().any(|attr| { if let syn::Meta::List(syn::MetaList { path, .. }) = &attr.meta { path.get_ident().map(|ident| ident == "derive").unwrap_or(false) @@ -119,7 +127,7 @@ impl CompositeDef { if !has_derive_attr { let derive_attr: syn::Attribute = syn::parse_quote! { #[derive( - Copy, Clone, Eq, PartialEq, Ord, PartialOrd, + Copy, Clone, Eq, PartialEq, #scrate::__private::codec::Encode, #scrate::__private::codec::Decode, #scrate::__private::codec::MaxEncodedLen, #scrate::__private::scale_info::TypeInfo, #scrate::__private::RuntimeDebug, @@ -128,6 +136,20 @@ impl CompositeDef { item.attrs.push(derive_attr); } + if has_instance { + item.attrs.push(syn::parse_quote! { + #[scale_info(skip_type_params(I))] + }); + + item.variants.push(syn::parse_quote! { + #[doc(hidden)] + #[codec(skip)] + __Ignore( + #scrate::__private::sp_std::marker::PhantomData, + ) + }); + } + let composite_keyword = syn::parse2::(item.ident.to_token_stream())?; diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index 8c4b3de49b5d5..700d777a14818 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -1646,8 +1646,7 @@ pub mod pallet_prelude { /// the enum: /// /// ```ignore -/// Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, MaxEncodedLen, TypeInfo, -/// RuntimeDebug +/// Copy, Clone, Eq, PartialEq, Encode, Decode, MaxEncodedLen, TypeInfo, RuntimeDebug /// ``` /// /// The inverse is also true: if there are any #[derive] attributes present for the enum, then @@ -1815,6 +1814,15 @@ pub mod pallet_prelude { /// #[pallet::origin] /// pub struct Origin(PhantomData); /// +/// // Declare a hold reason (this is optional). +/// // +/// // Creates a hold reason for this pallet that is aggregated by `construct_runtime`. +/// // A similar enum can be defined for `FreezeReason`, `LockId` or `SlashReason`. +/// #[pallet::composite_enum] +/// pub enum HoldReason { +/// SomeHoldReason +/// } +/// /// // Declare validate_unsigned implementation (this is optional). /// #[pallet::validate_unsigned] /// impl ValidateUnsigned for Pallet { @@ -1949,6 +1957,11 @@ pub mod pallet_prelude { /// #[pallet::origin] /// pub struct Origin(PhantomData<(T, I)>); /// +/// #[pallet::composite_enum] +/// pub enum HoldReason { +/// SomeHoldReason +/// } +/// /// #[pallet::validate_unsigned] /// impl, I: 'static> ValidateUnsigned for Pallet { /// type Call = Call; diff --git a/substrate/frame/support/test/tests/construct_runtime_ui.rs b/substrate/frame/support/test/tests/construct_runtime_ui.rs index c3197c99a72bf..0cf857e2d73c0 100644 --- a/substrate/frame/support/test/tests/construct_runtime_ui.rs +++ b/substrate/frame/support/test/tests/construct_runtime_ui.rs @@ -32,4 +32,5 @@ fn ui() { let t = trybuild::TestCases::new(); t.compile_fail("tests/construct_runtime_ui/*.rs"); + t.pass("tests/construct_runtime_ui/pass/*.rs"); } diff --git a/substrate/frame/support/test/tests/construct_runtime_ui/deprecated_where_block.stderr b/substrate/frame/support/test/tests/construct_runtime_ui/deprecated_where_block.stderr index cc2c2e160095d..08954bb6ab5c5 100644 --- a/substrate/frame/support/test/tests/construct_runtime_ui/deprecated_where_block.stderr +++ b/substrate/frame/support/test/tests/construct_runtime_ui/deprecated_where_block.stderr @@ -148,7 +148,11 @@ error[E0277]: the trait bound `Runtime: Config` is not satisfied in `frame_syste | ||_- in this macro invocation ... | | - = note: required because it appears within the type `Event` +note: required because it appears within the type `Event` + --> $WORKSPACE/substrate/frame/system/src/lib.rs + | + | pub enum Event { + | ^^^^^ note: required by a bound in `From` --> $RUST/core/src/convert/mod.rs | @@ -169,7 +173,11 @@ error[E0277]: the trait bound `Runtime: Config` is not satisfied in `frame_syste | ||_- in this macro invocation ... | | - = note: required because it appears within the type `Event` +note: required because it appears within the type `Event` + --> $WORKSPACE/substrate/frame/system/src/lib.rs + | + | pub enum Event { + | ^^^^^ note: required by a bound in `TryInto` --> $RUST/core/src/convert/mod.rs | diff --git a/substrate/frame/support/test/tests/construct_runtime_ui/pass/composite_enum_instance.rs b/substrate/frame/support/test/tests/construct_runtime_ui/pass/composite_enum_instance.rs new file mode 100644 index 0000000000000..ad63708747694 --- /dev/null +++ b/substrate/frame/support/test/tests/construct_runtime_ui/pass/composite_enum_instance.rs @@ -0,0 +1,80 @@ +// 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. + +#![cfg_attr(not(feature = "std"), no_std)] + +use frame_support::derive_impl; + +pub use pallet::*; + +#[frame_support::pallet(dev_mode)] +pub mod pallet { + use frame_support::pallet_prelude::*; + + // The struct on which we build all of our Pallet logic. + #[pallet::pallet] + pub struct Pallet(PhantomData<(T, I)>); + + // Your Pallet's configuration trait, representing custom external types and interfaces. + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::composite_enum] + pub enum HoldReason { + SomeHoldReason + } + + #[pallet::composite_enum] + pub enum FreezeReason { + SomeFreezeReason + } + + #[pallet::composite_enum] + pub enum SlashReason { + SomeSlashReason + } + + #[pallet::composite_enum] + pub enum LockId { + SomeLockId + } +} + +#[derive_impl(frame_system::config_preludes::TestDefaultConfig as frame_system::DefaultConfig)] +impl frame_system::Config for Runtime { + type Block = Block; +} + +pub type Header = sp_runtime::generic::Header; +pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; +pub type Block = sp_runtime::generic::Block; + +frame_support::construct_runtime!( + pub struct Runtime + { + // Exclude part `Storage` in order not to check its metadata in tests. + System: frame_system, + Pallet1: pallet, + Pallet2: pallet::, + } +); + +impl pallet::Config for Runtime {} + +impl pallet::Config for Runtime {} + +fn main() {} diff --git a/substrate/frame/support/test/tests/derive_no_bound_ui/debug.stderr b/substrate/frame/support/test/tests/derive_no_bound_ui/debug.stderr index d86292d71b7a1..3291f658f10e1 100644 --- a/substrate/frame/support/test/tests/derive_no_bound_ui/debug.stderr +++ b/substrate/frame/support/test/tests/derive_no_bound_ui/debug.stderr @@ -5,4 +5,4 @@ error[E0277]: `::C` doesn't implement `std::fmt::Debug` | ^ `::C` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug` | = help: the trait `std::fmt::Debug` is not implemented for `::C` - = note: required for the cast from `::C` to the object type `dyn std::fmt::Debug` + = note: required for the cast from `&::C` to `&dyn std::fmt::Debug` diff --git a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr index c86930f8a64e3..08ea7c0bec3a5 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr @@ -19,7 +19,7 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` | = help: the trait `std::fmt::Debug` is not implemented for `::Bar` = note: required for `&::Bar` to implement `std::fmt::Debug` - = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` + = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` error[E0277]: the trait bound `::Bar: Clone` is not satisfied --> tests/pallet_ui/call_argument_invalid_bound.rs:38:36 diff --git a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr index 1b04f44c78feb..80316fcd24897 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr @@ -19,7 +19,7 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` | = help: the trait `std::fmt::Debug` is not implemented for `::Bar` = note: required for `&::Bar` to implement `std::fmt::Debug` - = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` + = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` error[E0277]: the trait bound `::Bar: Clone` is not satisfied --> tests/pallet_ui/call_argument_invalid_bound_2.rs:38:36 diff --git a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr index 7429bce050c28..d45b74bad8428 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr @@ -20,7 +20,7 @@ error[E0277]: `Bar` doesn't implement `std::fmt::Debug` = help: the trait `std::fmt::Debug` is not implemented for `Bar` = note: add `#[derive(Debug)]` to `Bar` or manually `impl std::fmt::Debug for Bar` = note: required for `&Bar` to implement `std::fmt::Debug` - = note: required for the cast from `&Bar` to the object type `dyn std::fmt::Debug` + = note: required for the cast from `&&Bar` to `&dyn std::fmt::Debug` help: consider annotating `Bar` with `#[derive(Debug)]` | 34 + #[derive(Debug)] diff --git a/substrate/frame/support/test/tests/pallet_ui/dev_mode_without_arg_max_encoded_len.stderr b/substrate/frame/support/test/tests/pallet_ui/dev_mode_without_arg_max_encoded_len.stderr index 74ee0e4aeba5c..531e8bdffeb0c 100644 --- a/substrate/frame/support/test/tests/pallet_ui/dev_mode_without_arg_max_encoded_len.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/dev_mode_without_arg_max_encoded_len.stderr @@ -30,13 +30,13 @@ error[E0277]: the trait bound `Vec: MaxEncodedLen` is not satisfied | ^^^^^^ the trait `MaxEncodedLen` is not implemented for `Vec` | = help: the following other types implement trait `MaxEncodedLen`: - () - (TupleElement0, TupleElement1) - (TupleElement0, TupleElement1, TupleElement2) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6, TupleElement7) + bool + i8 + i16 + i32 + i64 + i128 + u8 + u16 and $N others = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageMyStorage, Vec>` to implement `StorageInfoTrait` diff --git a/substrate/frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.stderr b/substrate/frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.stderr index cfa0d465990a2..ea1d0ed99cd39 100644 --- a/substrate/frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.stderr @@ -5,13 +5,13 @@ error[E0277]: the trait bound `MyError: PalletError` is not satisfied | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `PalletError` is not implemented for `MyError` | = help: the following other types implement trait `PalletError`: - () - (TupleElement0, TupleElement1) - (TupleElement0, TupleElement1, TupleElement2) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6, TupleElement7) + bool + i8 + i16 + i32 + i64 + i128 + u8 + u16 and $N others = note: this error originates in the derive macro `frame_support::PalletError` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/substrate/frame/support/test/tests/pallet_ui/event_field_not_member.stderr b/substrate/frame/support/test/tests/pallet_ui/event_field_not_member.stderr index 4df6deafa0df6..fc4a33b721500 100644 --- a/substrate/frame/support/test/tests/pallet_ui/event_field_not_member.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/event_field_not_member.stderr @@ -18,4 +18,4 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` | = help: the trait `std::fmt::Debug` is not implemented for `::Bar` = note: required for `&::Bar` to implement `std::fmt::Debug` - = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` + = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` diff --git a/substrate/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr b/substrate/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr index 3a26d1c049541..5ea3be470a068 100644 --- a/substrate/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr @@ -4,8 +4,8 @@ error[E0046]: not all trait items implemented, missing: `Call`, `Error`, `INHERE 36 | impl ProvideInherent for Pallet {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `Call`, `Error`, `INHERENT_IDENTIFIER`, `create_inherent`, `is_inherent` in implementation | - = help: implement the missing item: `type Call = Type;` - = help: implement the missing item: `type Error = Type;` + = help: implement the missing item: `type Call = /* Type */;` + = help: implement the missing item: `type Error = /* Type */;` = help: implement the missing item: `const INHERENT_IDENTIFIER: [u8; 8] = value;` = help: implement the missing item: `fn create_inherent(_: &InherentData) -> std::option::Option<::Call> { todo!() }` = help: implement the missing item: `fn is_inherent(_: &::Call) -> bool { todo!() }` diff --git a/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr b/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr index e290b22a0eaa5..930af1d7fcb30 100644 --- a/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr @@ -5,10 +5,10 @@ error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied | ^^^^^^^^^^^^^^^^^^^^ the trait `WrapperTypeDecode` is not implemented for `Bar` | = help: the following other types implement trait `WrapperTypeDecode`: - Arc Box - Rc frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes + Rc + Arc = note: required for `Bar` to implement `Decode` = note: required for `Bar` to implement `FullCodec` = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `PartialStorageInfoTrait` @@ -20,14 +20,14 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied | ^^^^^^^^^^^^^^^^^^^^ the trait `EncodeLike` is not implemented for `Bar` | = help: the following other types implement trait `EncodeLike`: - <&&T as EncodeLike> - <&T as EncodeLike> - <&T as EncodeLike> - <&[(K, V)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[T] as EncodeLike>> + + + + + + + + and $N others = note: required for `Bar` to implement `FullEncode` = note: required for `Bar` to implement `FullCodec` @@ -40,14 +40,14 @@ error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied | ^^^^^^^^^^^^^^^^^^^^ the trait `WrapperTypeEncode` is not implemented for `Bar` | = help: the following other types implement trait `WrapperTypeEncode`: - &T - &mut T - Arc Box + bytes::bytes::Bytes Cow<'a, T> + parity_scale_codec::Ref<'a, T, U> + frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes Rc + Arc Vec - bytes::bytes::Bytes and $N others = note: required for `Bar` to implement `Encode` = note: required for `Bar` to implement `FullEncode` @@ -61,14 +61,14 @@ error[E0277]: the trait bound `Bar: TypeInfo` is not satisfied | ^^^^^^^ the trait `TypeInfo` is not implemented for `Bar` | = help: the following other types implement trait `TypeInfo`: - &T - &mut T - () - (A, B) - (A, B, C) - (A, B, C, D) - (A, B, C, D, E) - (A, B, C, D, E, F) + bool + char + i8 + i16 + i32 + i64 + i128 + u8 and $N others = note: required for `Bar` to implement `StaticTypeInfo` = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `StorageEntryMetadataBuilder` @@ -80,10 +80,10 @@ error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied | ^^^^^^^ the trait `WrapperTypeDecode` is not implemented for `Bar` | = help: the following other types implement trait `WrapperTypeDecode`: - Arc Box - Rc frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes + Rc + Arc = note: required for `Bar` to implement `Decode` = note: required for `Bar` to implement `FullCodec` = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `StorageEntryMetadataBuilder` @@ -95,14 +95,14 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied | ^^^^^^^ the trait `EncodeLike` is not implemented for `Bar` | = help: the following other types implement trait `EncodeLike`: - <&&T as EncodeLike> - <&T as EncodeLike> - <&T as EncodeLike> - <&[(K, V)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[T] as EncodeLike>> + + + + + + + + and $N others = note: required for `Bar` to implement `FullEncode` = note: required for `Bar` to implement `FullCodec` @@ -115,14 +115,14 @@ error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied | ^^^^^^^ the trait `WrapperTypeEncode` is not implemented for `Bar` | = help: the following other types implement trait `WrapperTypeEncode`: - &T - &mut T - Arc Box + bytes::bytes::Bytes Cow<'a, T> + parity_scale_codec::Ref<'a, T, U> + frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes Rc + Arc Vec - bytes::bytes::Bytes and $N others = note: required for `Bar` to implement `Encode` = note: required for `Bar` to implement `FullEncode` diff --git a/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr b/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr index 0e3a7c9f1cbfa..79798963c8b1a 100644 --- a/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr @@ -5,10 +5,10 @@ error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied | ^^^^^^^^^^^^^^^^^^^^ the trait `WrapperTypeDecode` is not implemented for `Bar` | = help: the following other types implement trait `WrapperTypeDecode`: - Arc Box - Rc frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes + Rc + Arc = note: required for `Bar` to implement `Decode` = note: required for `Bar` to implement `FullCodec` = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `PartialStorageInfoTrait` @@ -20,14 +20,14 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied | ^^^^^^^^^^^^^^^^^^^^ the trait `EncodeLike` is not implemented for `Bar` | = help: the following other types implement trait `EncodeLike`: - <&&T as EncodeLike> - <&T as EncodeLike> - <&T as EncodeLike> - <&[(K, V)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[T] as EncodeLike>> + + + + + + + + and $N others = note: required for `Bar` to implement `FullEncode` = note: required for `Bar` to implement `FullCodec` @@ -40,14 +40,14 @@ error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied | ^^^^^^^^^^^^^^^^^^^^ the trait `WrapperTypeEncode` is not implemented for `Bar` | = help: the following other types implement trait `WrapperTypeEncode`: - &T - &mut T - Arc Box + bytes::bytes::Bytes Cow<'a, T> + parity_scale_codec::Ref<'a, T, U> + frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes Rc + Arc Vec - bytes::bytes::Bytes and $N others = note: required for `Bar` to implement `Encode` = note: required for `Bar` to implement `FullEncode` @@ -61,14 +61,14 @@ error[E0277]: the trait bound `Bar: TypeInfo` is not satisfied | ^^^^^^^ the trait `TypeInfo` is not implemented for `Bar` | = help: the following other types implement trait `TypeInfo`: - &T - &mut T - () - (A, B) - (A, B, C) - (A, B, C, D) - (A, B, C, D, E) - (A, B, C, D, E, F) + bool + char + i8 + i16 + i32 + i64 + i128 + u8 and $N others = note: required for `Bar` to implement `StaticTypeInfo` = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `StorageEntryMetadataBuilder` @@ -80,10 +80,10 @@ error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied | ^^^^^^^ the trait `WrapperTypeDecode` is not implemented for `Bar` | = help: the following other types implement trait `WrapperTypeDecode`: - Arc Box - Rc frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes + Rc + Arc = note: required for `Bar` to implement `Decode` = note: required for `Bar` to implement `FullCodec` = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `StorageEntryMetadataBuilder` @@ -95,14 +95,14 @@ error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied | ^^^^^^^ the trait `EncodeLike` is not implemented for `Bar` | = help: the following other types implement trait `EncodeLike`: - <&&T as EncodeLike> - <&T as EncodeLike> - <&T as EncodeLike> - <&[(K, V)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[(T,)] as EncodeLike>> - <&[T] as EncodeLike>> + + + + + + + + and $N others = note: required for `Bar` to implement `FullEncode` = note: required for `Bar` to implement `FullCodec` @@ -115,14 +115,14 @@ error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied | ^^^^^^^ the trait `WrapperTypeEncode` is not implemented for `Bar` | = help: the following other types implement trait `WrapperTypeEncode`: - &T - &mut T - Arc Box + bytes::bytes::Bytes Cow<'a, T> + parity_scale_codec::Ref<'a, T, U> + frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes Rc + Arc Vec - bytes::bytes::Bytes and $N others = note: required for `Bar` to implement `Encode` = note: required for `Bar` to implement `FullEncode` diff --git a/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied.stderr b/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied.stderr index 9a31e4b6bdf46..e04de98800ec2 100644 --- a/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied.stderr @@ -5,13 +5,13 @@ error[E0277]: the trait bound `Bar: MaxEncodedLen` is not satisfied | ^^^^^^ the trait `MaxEncodedLen` is not implemented for `Bar` | = help: the following other types implement trait `MaxEncodedLen`: - () - (TupleElement0, TupleElement1) - (TupleElement0, TupleElement1, TupleElement2) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6, TupleElement7) + bool + i8 + i16 + i32 + i64 + i128 + u8 + u16 and $N others = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `StorageInfoTrait` diff --git a/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr b/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr index cdcd1b401f801..31fe3b5733896 100644 --- a/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr @@ -5,14 +5,14 @@ error[E0277]: the trait bound `Bar: MaxEncodedLen` is not satisfied | ^^^^^^ the trait `MaxEncodedLen` is not implemented for `Bar` | = help: the following other types implement trait `MaxEncodedLen`: - () - (TupleElement0, TupleElement1) - (TupleElement0, TupleElement1, TupleElement2) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6, TupleElement7) + bool + i8 + i16 + i32 + i64 + i128 + u8 + u16 and $N others - = note: required for `Key` to implement `KeyGeneratorMaxEncodedLen` - = note: required for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, Key, u32>` to implement `StorageInfoTrait` + = note: required for `NMapKey` to implement `KeyGeneratorMaxEncodedLen` + = note: required for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, NMapKey, u32>` to implement `StorageInfoTrait` diff --git a/substrate/frame/treasury/src/benchmarking.rs b/substrate/frame/treasury/src/benchmarking.rs index f5f73ea8ddabd..61fe29dafcae5 100644 --- a/substrate/frame/treasury/src/benchmarking.rs +++ b/substrate/frame/treasury/src/benchmarking.rs @@ -336,5 +336,9 @@ mod benchmarks { Ok(()) } - impl_benchmark_test_suite!(Treasury, crate::tests::new_test_ext(), crate::tests::Test); + impl_benchmark_test_suite!( + Treasury, + crate::tests::ExtBuilder::default().build(), + crate::tests::Test + ); } diff --git a/substrate/frame/treasury/src/lib.rs b/substrate/frame/treasury/src/lib.rs index b2b3a8801c156..5e429d3914bd0 100644 --- a/substrate/frame/treasury/src/lib.rs +++ b/substrate/frame/treasury/src/lib.rs @@ -89,7 +89,8 @@ use sp_runtime::{ use sp_std::{collections::btree_map::BTreeMap, prelude::*}; use frame_support::{ - print, + dispatch::{DispatchResult, DispatchResultWithPostInfo}, + ensure, print, traits::{ tokens::Pay, Currency, ExistenceRequirement::KeepAlive, Get, Imbalance, OnUnbalanced, ReservableCurrency, WithdrawReasons, @@ -456,6 +457,14 @@ pub mod pallet { Weight::zero() } } + + #[cfg(feature = "try-runtime")] + fn try_state( + _: frame_system::pallet_prelude::BlockNumberFor, + ) -> Result<(), sp_runtime::TryRuntimeError> { + Self::do_try_state()?; + Ok(()) + } } #[derive(Default)] @@ -1020,6 +1029,85 @@ impl, I: 'static> Pallet { // Must never be less than 0 but better be safe. .saturating_sub(T::Currency::minimum_balance()) } + + /// Ensure the correctness of the state of this pallet. + #[cfg(any(feature = "try-runtime", test))] + fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> { + Self::try_state_proposals()?; + Self::try_state_spends()?; + + Ok(()) + } + + /// ### Invariants of proposal storage items + /// + /// 1. [`ProposalCount`] >= Number of elements in [`Proposals`]. + /// 2. Each entry in [`Proposals`] should be saved under a key stricly less than current + /// [`ProposalCount`]. + /// 3. Each [`ProposalIndex`] contained in [`Approvals`] should exist in [`Proposals`]. + /// Note, that this automatically implies [`Approvals`].count() <= [`Proposals`].count(). + #[cfg(any(feature = "try-runtime", test))] + fn try_state_proposals() -> Result<(), sp_runtime::TryRuntimeError> { + let current_proposal_count = ProposalCount::::get(); + ensure!( + current_proposal_count as usize >= Proposals::::iter().count(), + "Actual number of proposals exceeds `ProposalCount`." + ); + + Proposals::::iter_keys().try_for_each(|proposal_index| -> DispatchResult { + ensure!( + current_proposal_count as u32 > proposal_index, + "`ProposalCount` should by strictly greater than any ProposalIndex used as a key for `Proposals`." + ); + Ok(()) + })?; + + Approvals::::get() + .iter() + .try_for_each(|proposal_index| -> DispatchResult { + ensure!( + Proposals::::contains_key(proposal_index), + "Proposal indices in `Approvals` must also be contained in `Proposals`." + ); + Ok(()) + })?; + + Ok(()) + } + + /// ## Invariants of spend storage items + /// + /// 1. [`SpendCount`] >= Number of elements in [`Spends`]. + /// 2. Each entry in [`Spends`] should be saved under a key stricly less than current + /// [`SpendCount`]. + /// 3. For each spend entry contained in [`Spends`] we should have spend.expire_at + /// > spend.valid_from. + #[cfg(any(feature = "try-runtime", test))] + fn try_state_spends() -> Result<(), sp_runtime::TryRuntimeError> { + let current_spend_count = SpendCount::::get(); + ensure!( + current_spend_count as usize >= Spends::::iter().count(), + "Actual number of spends exceeds `SpendCount`." + ); + + Spends::::iter_keys().try_for_each(|spend_index| -> DispatchResult { + ensure!( + current_spend_count > spend_index, + "`SpendCount` should by strictly greater than any SpendIndex used as a key for `Spends`." + ); + Ok(()) + })?; + + Spends::::iter().try_for_each(|(_index, spend)| -> DispatchResult { + ensure!( + spend.valid_from < spend.expire_at, + "Spend cannot expire before it becomes valid." + ); + Ok(()) + })?; + + Ok(()) + } } impl, I: 'static> OnUnbalanced> for Pallet { diff --git a/substrate/frame/treasury/src/tests.rs b/substrate/frame/treasury/src/tests.rs index 4bb00547d9f28..1748189723edf 100644 --- a/substrate/frame/treasury/src/tests.rs +++ b/substrate/frame/treasury/src/tests.rs @@ -217,16 +217,28 @@ impl Config for Test { type BenchmarkHelper = (); } -pub fn new_test_ext() -> sp_io::TestExternalities { - let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); - pallet_balances::GenesisConfig:: { - // Total issuance will be 200 with treasury account initialized at ED. - balances: vec![(0, 100), (1, 98), (2, 1)], +pub struct ExtBuilder {} + +impl Default for ExtBuilder { + fn default() -> Self { + Self {} + } +} + +impl ExtBuilder { + pub fn build(self) -> sp_io::TestExternalities { + let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); + pallet_balances::GenesisConfig:: { + // Total issuance will be 200 with treasury account initialized at ED. + balances: vec![(0, 100), (1, 98), (2, 1)], + } + .assimilate_storage(&mut t) + .unwrap(); + crate::GenesisConfig::::default().assimilate_storage(&mut t).unwrap(); + let mut ext = sp_io::TestExternalities::new(t); + ext.execute_with(|| System::set_block_number(1)); + ext } - .assimilate_storage(&mut t) - .unwrap(); - crate::GenesisConfig::::default().assimilate_storage(&mut t).unwrap(); - t.into() } fn get_payment_id(i: SpendIndex) -> Option { @@ -239,7 +251,7 @@ fn get_payment_id(i: SpendIndex) -> Option { #[test] fn genesis_config_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_eq!(Treasury::pot(), 0); assert_eq!(Treasury::proposal_count(), 0); }); @@ -247,7 +259,7 @@ fn genesis_config_works() { #[test] fn spend_local_origin_permissioning_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_noop!(Treasury::spend_local(RuntimeOrigin::signed(1), 1, 1), BadOrigin); assert_noop!( Treasury::spend_local(RuntimeOrigin::signed(10), 6, 1), @@ -271,7 +283,7 @@ fn spend_local_origin_permissioning_works() { #[docify::export] #[test] fn spend_local_origin_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { // Check that accumulate works when we have Some value in Dummy already. Balances::make_free_balance_be(&Treasury::account_id(), 101); // approve spend of some amount to beneficiary `6`. @@ -295,7 +307,7 @@ fn spend_local_origin_works() { #[test] fn minting_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { // Check that accumulate works when we have Some value in Dummy already. Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_eq!(Treasury::pot(), 100); @@ -304,7 +316,7 @@ fn minting_works() { #[test] fn spend_proposal_takes_min_deposit() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_ok!({ #[allow(deprecated)] Treasury::propose_spend(RuntimeOrigin::signed(0), 1, 3) @@ -316,7 +328,7 @@ fn spend_proposal_takes_min_deposit() { #[test] fn spend_proposal_takes_proportional_deposit() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_ok!({ #[allow(deprecated)] Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3) @@ -328,7 +340,7 @@ fn spend_proposal_takes_proportional_deposit() { #[test] fn spend_proposal_fails_when_proposer_poor() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_noop!( { #[allow(deprecated)] @@ -341,7 +353,7 @@ fn spend_proposal_fails_when_proposer_poor() { #[test] fn accepted_spend_proposal_ignored_outside_spend_period() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_ok!({ @@ -361,7 +373,7 @@ fn accepted_spend_proposal_ignored_outside_spend_period() { #[test] fn unused_pot_should_diminish() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { let init_total_issuance = Balances::total_issuance(); Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_eq!(Balances::total_issuance(), init_total_issuance + 100); @@ -374,7 +386,7 @@ fn unused_pot_should_diminish() { #[test] fn rejected_spend_proposal_ignored_on_spend_period() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_ok!({ @@ -394,7 +406,7 @@ fn rejected_spend_proposal_ignored_on_spend_period() { #[test] fn reject_already_rejected_spend_proposal_fails() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_ok!({ @@ -417,7 +429,7 @@ fn reject_already_rejected_spend_proposal_fails() { #[test] fn reject_non_existent_spend_proposal_fails() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_noop!( { #[allow(deprecated)] @@ -430,7 +442,7 @@ fn reject_non_existent_spend_proposal_fails() { #[test] fn accept_non_existent_spend_proposal_fails() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_noop!( { #[allow(deprecated)] @@ -443,7 +455,7 @@ fn accept_non_existent_spend_proposal_fails() { #[test] fn accept_already_rejected_spend_proposal_fails() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_ok!({ @@ -466,7 +478,7 @@ fn accept_already_rejected_spend_proposal_fails() { #[test] fn accepted_spend_proposal_enacted_on_spend_period() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_eq!(Treasury::pot(), 100); @@ -487,7 +499,7 @@ fn accepted_spend_proposal_enacted_on_spend_period() { #[test] fn pot_underflow_should_not_diminish() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_eq!(Treasury::pot(), 100); @@ -514,7 +526,7 @@ fn pot_underflow_should_not_diminish() { // i.e. pot should not include existential deposit needed for account survival. #[test] fn treasury_account_doesnt_get_deleted() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_eq!(Treasury::pot(), 100); let treasury_balance = Balances::free_balance(&Treasury::account_id()); @@ -613,7 +625,7 @@ fn genesis_funding_works() { #[test] fn max_approvals_limited() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), u64::MAX); Balances::make_free_balance_be(&0, u64::MAX); @@ -645,7 +657,7 @@ fn max_approvals_limited() { #[test] fn remove_already_removed_approval_fails() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_ok!({ @@ -669,7 +681,7 @@ fn remove_already_removed_approval_fails() { #[test] fn spending_local_in_batch_respects_max_total() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { // Respect the `max_total` for the given origin. assert_ok!(RuntimeCall::from(UtilityCall::batch_all { calls: vec![ @@ -694,7 +706,7 @@ fn spending_local_in_batch_respects_max_total() { #[test] fn spending_in_batch_respects_max_total() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { // Respect the `max_total` for the given origin. assert_ok!(RuntimeCall::from(UtilityCall::batch_all { calls: vec![ @@ -739,7 +751,7 @@ fn spending_in_batch_respects_max_total() { #[test] fn spend_origin_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), Box::new(1), 1, Box::new(6), None)); assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), Box::new(1), 2, Box::new(6), None)); assert_noop!( @@ -764,7 +776,7 @@ fn spend_origin_works() { #[test] fn spend_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), Box::new(1), 2, Box::new(6), None)); @@ -796,7 +808,7 @@ fn spend_works() { #[test] fn spend_expires() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_eq!(::PayoutPeriod::get(), 5); // spend `0` expires in 5 blocks after the creating. @@ -816,7 +828,7 @@ fn spend_expires() { #[docify::export] #[test] fn spend_payout_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); // approve a `2` coins spend of asset `1` to beneficiary `6`, the spend valid from now. assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), Box::new(1), 2, Box::new(6), None)); @@ -838,7 +850,7 @@ fn spend_payout_works() { #[test] fn payout_retry_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), Box::new(1), 2, Box::new(6), None)); assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); @@ -863,7 +875,7 @@ fn payout_retry_works() { #[test] fn spend_valid_from_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_eq!(::PayoutPeriod::get(), 5); System::set_block_number(1); @@ -895,7 +907,7 @@ fn spend_valid_from_works() { #[test] fn void_spend_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { System::set_block_number(1); // spend cannot be voided if already attempted. assert_ok!(Treasury::spend( @@ -926,7 +938,7 @@ fn void_spend_works() { #[test] fn check_status_works() { - new_test_ext().execute_with(|| { + ExtBuilder::default().build().execute_with(|| { assert_eq!(::PayoutPeriod::get(), 5); System::set_block_number(1); @@ -984,3 +996,167 @@ fn check_status_works() { System::assert_last_event(Event::::SpendProcessed { index: 4 }.into()); }); } + +#[test] +fn try_state_proposals_invariant_1_works() { + ExtBuilder::default().build().execute_with(|| { + use frame_support::pallet_prelude::DispatchError::Other; + // Add a proposal using `propose_spend` + assert_ok!({ + #[allow(deprecated)] + Treasury::propose_spend(RuntimeOrigin::signed(0), 1, 3) + }); + assert_eq!(Proposals::::iter().count(), 1); + assert_eq!(ProposalCount::::get(), 1); + // Check invariant 1 holds + assert!(ProposalCount::::get() as usize >= Proposals::::iter().count()); + // Break invariant 1 by decreasing `ProposalCount` + ProposalCount::::put(0); + // Invariant 1 should be violated + assert_eq!( + Treasury::do_try_state(), + Err(Other("Actual number of proposals exceeds `ProposalCount`.")) + ); + }); +} + +#[test] +fn try_state_proposals_invariant_2_works() { + ExtBuilder::default().build().execute_with(|| { + use frame_support::pallet_prelude::DispatchError::Other; + // Add a proposal using `propose_spend` + assert_ok!({ + #[allow(deprecated)] + Treasury::propose_spend(RuntimeOrigin::signed(0), 1, 3) + }); + assert_eq!(Proposals::::iter().count(), 1); + let current_proposal_count = ProposalCount::::get(); + assert_eq!(current_proposal_count, 1); + // Check invariant 2 holds + assert!( + Proposals::::iter_keys() + .all(|proposal_index| { + proposal_index < current_proposal_count + }) + ); + // Break invariant 2 by inserting the proposal under key = 1 + let proposal = Proposals::::take(0).unwrap(); + Proposals::::insert(1, proposal); + // Invariant 2 should be violated + assert_eq!( + Treasury::do_try_state(), + Err(Other("`ProposalCount` should by strictly greater than any ProposalIndex used as a key for `Proposals`.")) + ); + }); +} + +#[test] +fn try_state_proposals_invariant_3_works() { + ExtBuilder::default().build().execute_with(|| { + use frame_support::pallet_prelude::DispatchError::Other; + // Add a proposal using `propose_spend` + assert_ok!({ + #[allow(deprecated)] + Treasury::propose_spend(RuntimeOrigin::signed(0), 10, 3) + }); + assert_eq!(Proposals::::iter().count(), 1); + // Approve the proposal + assert_ok!({ + #[allow(deprecated)] + Treasury::approve_proposal(RuntimeOrigin::root(), 0) + }); + assert_eq!(Approvals::::get().len(), 1); + // Check invariant 3 holds + assert!(Approvals::::get() + .iter() + .all(|proposal_index| { Proposals::::contains_key(proposal_index) })); + // Break invariant 3 by adding another key to `Approvals` + let mut approvals_modified = Approvals::::get(); + approvals_modified.try_push(2).unwrap(); + Approvals::::put(approvals_modified); + // Invariant 3 should be violated + assert_eq!( + Treasury::do_try_state(), + Err(Other("Proposal indices in `Approvals` must also be contained in `Proposals`.")) + ); + }); +} + +#[test] +fn try_state_spends_invariant_1_works() { + ExtBuilder::default().build().execute_with(|| { + use frame_support::pallet_prelude::DispatchError::Other; + // Propose and approve a spend + assert_ok!({ + Treasury::spend(RuntimeOrigin::signed(10), Box::new(1), 1, Box::new(6), None) + }); + assert_eq!(Spends::::iter().count(), 1); + assert_eq!(SpendCount::::get(), 1); + // Check invariant 1 holds + assert!(SpendCount::::get() as usize >= Spends::::iter().count()); + // Break invariant 1 by decreasing `SpendCount` + SpendCount::::put(0); + // Invariant 1 should be violated + assert_eq!( + Treasury::do_try_state(), + Err(Other("Actual number of spends exceeds `SpendCount`.")) + ); + }); +} + +#[test] +fn try_state_spends_invariant_2_works() { + ExtBuilder::default().build().execute_with(|| { + use frame_support::pallet_prelude::DispatchError::Other; + // Propose and approve a spend + assert_ok!({ + Treasury::spend(RuntimeOrigin::signed(10), Box::new(1), 1, Box::new(6), None) + }); + assert_eq!(Spends::::iter().count(), 1); + let current_spend_count = SpendCount::::get(); + assert_eq!(current_spend_count, 1); + // Check invariant 2 holds + assert!( + Spends::::iter_keys() + .all(|spend_index| { + spend_index < current_spend_count + }) + ); + // Break invariant 2 by inserting the spend under key = 1 + let spend = Spends::::take(0).unwrap(); + Spends::::insert(1, spend); + // Invariant 2 should be violated + assert_eq!( + Treasury::do_try_state(), + Err(Other("`SpendCount` should by strictly greater than any SpendIndex used as a key for `Spends`.")) + ); + }); +} + +#[test] +fn try_state_spends_invariant_3_works() { + ExtBuilder::default().build().execute_with(|| { + use frame_support::pallet_prelude::DispatchError::Other; + // Propose and approve a spend + assert_ok!({ + Treasury::spend(RuntimeOrigin::signed(10), Box::new(1), 1, Box::new(6), None) + }); + assert_eq!(Spends::::iter().count(), 1); + let current_spend_count = SpendCount::::get(); + assert_eq!(current_spend_count, 1); + // Check invariant 3 holds + assert!(Spends::::iter_values() + .all(|SpendStatus { valid_from, expire_at, .. }| { valid_from < expire_at })); + // Break invariant 3 by reversing spend.expire_at and spend.valid_from + let spend = Spends::::take(0).unwrap(); + Spends::::insert( + 0, + SpendStatus { valid_from: spend.expire_at, expire_at: spend.valid_from, ..spend }, + ); + // Invariant 3 should be violated + assert_eq!( + Treasury::do_try_state(), + Err(Other("Spend cannot expire before it becomes valid.")) + ); + }); +} diff --git a/substrate/primitives/npos-elections/src/lib.rs b/substrate/primitives/npos-elections/src/lib.rs index 0afe1ec5bb692..62ae050211482 100644 --- a/substrate/primitives/npos-elections/src/lib.rs +++ b/substrate/primitives/npos-elections/src/lib.rs @@ -19,7 +19,7 @@ //! - [`seq_phragmen`]: Implements the Phragmén Sequential Method. An un-ranked, relatively fast //! election method that ensures PJR, but does not provide a constant factor approximation of the //! maximin problem. -//! - [`phragmms`](phragmms::phragmms): Implements a hybrid approach inspired by Phragmén which is +//! - [`ghragmms`](phragmms::phragmms()): Implements a hybrid approach inspired by Phragmén which is //! executed faster but it can achieve a constant factor approximation of the maximin problem, //! similar to that of the MMS algorithm. //! - [`balance`](balancing::balance): Implements the star balancing algorithm. This iterative diff --git a/substrate/primitives/runtime-interface/tests/ui/no_feature_gated_method.stderr b/substrate/primitives/runtime-interface/tests/ui/no_feature_gated_method.stderr index 23e671f6ce3f3..10012ede793de 100644 --- a/substrate/primitives/runtime-interface/tests/ui/no_feature_gated_method.stderr +++ b/substrate/primitives/runtime-interface/tests/ui/no_feature_gated_method.stderr @@ -3,3 +3,15 @@ error[E0425]: cannot find function `bar` in module `test` | 33 | test::bar(); | ^^^ not found in `test` + | +note: found an item that was configured out + --> tests/ui/no_feature_gated_method.rs:25:5 + | +25 | fn bar() {} + | ^^^ + = note: the item is gated behind the `bar-feature` feature +note: found an item that was configured out + --> tests/ui/no_feature_gated_method.rs:25:5 + | +25 | fn bar() {} + | ^^^ diff --git a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs index 69c95d13c0985..9493a693bbed3 100644 --- a/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs +++ b/substrate/utils/frame/benchmarking-cli/src/pallet/writer.rs @@ -779,6 +779,7 @@ fn worst_case_pov( /// A simple match statement which outputs the log 16 of some value. fn easy_log_16(i: u32) -> u32 { + #[allow(clippy::redundant_guards)] match i { i if i == 0 => 0, i if i <= 16 => 1,