From ee6c6d05f376a4d1c364fb9e145a00f6b5a0e84e Mon Sep 17 00:00:00 2001 From: Frederik Gartenmeister Date: Tue, 5 Sep 2023 16:26:26 +0200 Subject: [PATCH 1/7] feat: account ensuring origin --- Cargo.lock | 1 + runtime/common/Cargo.toml | 1 + runtime/common/src/lib.rs | 159 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 91c0645f2d..2837d347a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11006,6 +11006,7 @@ dependencies = [ "pallet-authorship", "pallet-balances", "pallet-base-fee", + "pallet-collective", "pallet-data-collector", "pallet-ethereum", "pallet-evm", diff --git a/runtime/common/Cargo.toml b/runtime/common/Cargo.toml index 480040f0a2..d03e0834b3 100644 --- a/runtime/common/Cargo.toml +++ b/runtime/common/Cargo.toml @@ -76,6 +76,7 @@ sp-io = { git = "https://github.com/paritytech/substrate", default-features = fa cfg-mocks = { path = "../../libs/mocks", features = ["runtime-benchmarks", "std"] } hex-literal = "0.2.1" sp-io = { git = "https://github.com/paritytech/substrate", default-features = true, branch = "polkadot-v0.9.38" } +pallet-collective = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.38" } [features] default = ["std"] diff --git a/runtime/common/src/lib.rs b/runtime/common/src/lib.rs index c89d64c4aa..44070d0f61 100644 --- a/runtime/common/src/lib.rs +++ b/runtime/common/src/lib.rs @@ -419,3 +419,162 @@ pub mod xcm_transactor { } } } + +pub mod origin { + use std::marker::PhantomData; + + use cfg_primitives::AccountId; + use frame_support::{ + dispatch::RawOrigin, + traits::{EitherOfDiverse, EnsureOrigin}, + }; + use frame_system::EnsureRoot; + use sp_core::Get; + + type EnsureAccountOrRootOr = + EitherOfDiverse, EnsureRoot>, O>; + + pub struct EnsureAccount(PhantomData); + + impl EnsureOrigin for EnsureAccount + where + OuterOrigin: + Into, OuterOrigin>> + From> + Clone, + Account: Get, + { + type Success = (); + + fn try_origin(o: OuterOrigin) -> Result { + o.into().and_then(|raw| match raw { + RawOrigin::Root | RawOrigin::None => Err(OuterOrigin::from(raw)), + RawOrigin::Signed(ref signer) => { + if signer == &Account::get() { + Ok(()) + } else { + Err(OuterOrigin::from(raw)) + } + } + }) + } + + #[cfg(feature = "runtime-benchmarks")] + fn try_successful_origin() -> Result { + Ok(RawOrigin::Signed(Account::get()).into()) + } + } + + #[cfg(test)] + mod test { + use cfg_primitives::HalfOfCouncil; + use sp_core::{crypto::AccountId32, parameter_types}; + + use super::*; + + parameter_types! { + pub Admin: AccountId = AccountId::new([0u8;32]); + } + + #[derive(Clone)] + enum OuterOrigin { + Raw(RawOrigin), + Council(pallet_collective::RawOrigin), + Dummy, + } + + impl Into, OuterOrigin>> for OuterOrigin { + fn into(self) -> Result, OuterOrigin> { + match self { + Self::Raw(raw) => Ok(raw), + _ => Err(self), + } + } + } + + impl + Into< + Result< + pallet_collective::RawOrigin< + sp_runtime::AccountId32, + pallet_collective::Instance1, + >, + OuterOrigin, + >, + > for OuterOrigin + { + fn into( + self, + ) -> Result< + pallet_collective::RawOrigin, + OuterOrigin, + > { + match self { + Self::Council(raw) => Ok(raw), + _ => Err(self), + } + } + } + + impl From> for OuterOrigin { + fn from(value: RawOrigin) -> Self { + Self::Raw(value) + } + } + + impl From> for OuterOrigin { + fn from( + value: pallet_collective::RawOrigin, + ) -> Self { + Self::Council(value) + } + } + + #[test] + fn works_with_account() { + let origin = OuterOrigin::Raw(RawOrigin::Signed(Admin::get())); + + assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_ok()) + } + + #[test] + fn fails_with_non_admin_account() { + let origin = OuterOrigin::Raw(RawOrigin::Signed(AccountId::from([1u8; 32]))); + + assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_err()) + } + + #[test] + fn works_with_half_of_council() { + let origin = OuterOrigin::Council(pallet_collective::RawOrigin::Members(5, 9)); + + assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_ok()) + } + + #[test] + fn fails_with_less_than_half_of_council() { + let origin = OuterOrigin::Council(pallet_collective::RawOrigin::Members(4, 9)); + + assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_err()) + } + + #[test] + fn works_with_root() { + let origin = OuterOrigin::Raw(RawOrigin::Root); + + assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_ok()) + } + + #[test] + fn fails_with_none() { + let origin = OuterOrigin::Raw(RawOrigin::None); + + assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_err()) + } + + #[test] + fn fails_with_dummy() { + let origin = OuterOrigin::Dummy; + + assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_err()) + } + } +} From 2431d2222283baf5c337eb858e4a4c6df7af58f0 Mon Sep 17 00:00:00 2001 From: Frederik Gartenmeister Date: Tue, 5 Sep 2023 16:36:45 +0200 Subject: [PATCH 2/7] feat: bumb runtime version develpment --- runtime/development/Cargo.toml | 2 +- runtime/development/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/development/Cargo.toml b/runtime/development/Cargo.toml index 29b65c3dbf..111f6227b9 100644 --- a/runtime/development/Cargo.toml +++ b/runtime/development/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "development-runtime" -version = "0.10.20" +version = "0.10.21" authors = ["Centrifuge "] edition = "2021" build = "build.rs" diff --git a/runtime/development/src/lib.rs b/runtime/development/src/lib.rs index 1c7378d0c1..df4f017ae8 100644 --- a/runtime/development/src/lib.rs +++ b/runtime/development/src/lib.rs @@ -138,7 +138,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("centrifuge-devel"), impl_name: create_runtime_str!("centrifuge-devel"), authoring_version: 1, - spec_version: 1020, + spec_version: 1021, impl_version: 1, #[cfg(not(feature = "disable-runtime-api"))] apis: RUNTIME_API_VERSIONS, From 890333133b21d61899fcad4e60e4261364db49c6 Mon Sep 17 00:00:00 2001 From: Frederik Gartenmeister Date: Tue, 5 Sep 2023 16:56:07 +0200 Subject: [PATCH 3/7] fix: allow council to control gatway on Altair based chains --- runtime/altair/src/lib.rs | 2 +- runtime/common/src/lib.rs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/runtime/altair/src/lib.rs b/runtime/altair/src/lib.rs index b0398f15ba..ad91ea8906 100644 --- a/runtime/altair/src/lib.rs +++ b/runtime/altair/src/lib.rs @@ -1416,7 +1416,7 @@ parameter_types! { } impl pallet_liquidity_pools_gateway::Config for Runtime { - type AdminOrigin = EnsureRoot; + type AdminOrigin = EnsureRootOr; type InboundQueue = LiquidityPools; type LocalEVMOrigin = pallet_liquidity_pools_gateway::EnsureLocal; type MaxIncomingMessageSize = MaxIncomingMessageSize; diff --git a/runtime/common/src/lib.rs b/runtime/common/src/lib.rs index 44070d0f61..a967163d39 100644 --- a/runtime/common/src/lib.rs +++ b/runtime/common/src/lib.rs @@ -431,6 +431,9 @@ pub mod origin { use frame_system::EnsureRoot; use sp_core::Get; + type EnsureAccountOrRoot = + EitherOfDiverse, EnsureRoot>; + type EnsureAccountOrRootOr = EitherOfDiverse, EnsureRoot>, O>; From 3f4d05a175020f55dce3045f81938f78f62fa760 Mon Sep 17 00:00:00 2001 From: Frederik Gartenmeister Date: Tue, 5 Sep 2023 20:55:02 +0200 Subject: [PATCH 4/7] fix: use EnsureSigned, make use of type and add admin --- Cargo.lock | 51 +++------- Cargo.toml | 2 +- runtime/centrifuge/Cargo.toml | 3 +- runtime/centrifuge/src/evm.rs | 6 +- runtime/centrifuge/src/lib.rs | 21 ++++- runtime/common/Cargo.toml | 2 +- runtime/common/src/lib.rs | 169 +++++++++++++++++++++------------- 7 files changed, 142 insertions(+), 112 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2837d347a2..a617e38b6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -214,7 +214,7 @@ dependencies = [ "frame-system-benchmarking", "frame-system-rpc-runtime-api", "frame-try-runtime", - "hex-literal 0.3.4", + "hex-literal", "liquidity-pools-gateway-routers", "log", "moonbeam-relay-encoder", @@ -1078,7 +1078,7 @@ dependencies = [ "frame-system", "futures", "getrandom 0.2.10", - "hex-literal 0.2.2", + "hex-literal", "jsonrpsee", "log", "pallet-anchors", @@ -1160,7 +1160,7 @@ dependencies = [ "frame-system-benchmarking", "frame-system-rpc-runtime-api", "frame-try-runtime", - "hex-literal 0.3.4", + "hex-literal", "liquidity-pools-gateway-routers", "log", "moonbeam-relay-encoder", @@ -1357,7 +1357,7 @@ dependencies = [ "cfg-utils", "frame-support", "hex", - "hex-literal 0.3.4", + "hex-literal", "orml-asset-registry", "parity-scale-codec 3.6.4", "scale-info", @@ -2679,7 +2679,7 @@ dependencies = [ [[package]] name = "development-runtime" -version = "0.10.20" +version = "0.10.21" dependencies = [ "axelar-gateway-precompile", "cfg-primitives", @@ -2706,7 +2706,7 @@ dependencies = [ "frame-try-runtime", "getrandom 0.2.10", "hex", - "hex-literal 0.3.4", + "hex-literal", "liquidity-pools-gateway-routers", "moonbeam-relay-encoder", "orml-asset-registry", @@ -4451,31 +4451,12 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" -[[package]] -name = "hex-literal" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d70693199b3cf4552f3fa720b54163927a3ebed2aef240efaf556033ab336a11" -dependencies = [ - "hex-literal-impl", - "proc-macro-hack", -] - [[package]] name = "hex-literal" version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ebdb29d2ea9ed0083cd8cece49bbd968021bd99b0849edb4a9a7ee0fdf6a4e0" -[[package]] -name = "hex-literal-impl" -version = "0.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59448fc2f82a5fb6907f78c3d69d843e82ff5b051923313cc4438cb0c7b745a8" -dependencies = [ - "proc-macro-hack", -] - [[package]] name = "hkdf" version = "0.12.3" @@ -5064,7 +5045,7 @@ dependencies = [ "frame-system-benchmarking", "frame-system-rpc-runtime-api", "frame-try-runtime", - "hex-literal 0.3.4", + "hex-literal", "kusama-runtime-constants", "log", "pallet-authority-discovery", @@ -9668,7 +9649,7 @@ version = "0.9.38" source = "git+https://github.com/paritytech//polkadot?rev=097ffd245c42aeff28cf80f8a3568e1bee2e7da7#097ffd245c42aeff28cf80f8a3568e1bee2e7da7" dependencies = [ "bitvec 1.0.1", - "hex-literal 0.3.4", + "hex-literal", "parity-scale-codec 3.6.4", "polkadot-core-primitives", "polkadot-parachain", @@ -9734,7 +9715,7 @@ dependencies = [ "frame-system-benchmarking", "frame-system-rpc-runtime-api", "frame-try-runtime", - "hex-literal 0.3.4", + "hex-literal", "log", "pallet-authority-discovery", "pallet-authorship", @@ -9938,7 +9919,7 @@ dependencies = [ "frame-support", "frame-system-rpc-runtime-api", "futures", - "hex-literal 0.3.4", + "hex-literal", "kusama-runtime", "kvdb", "kvdb-rocksdb", @@ -10277,12 +10258,6 @@ dependencies = [ "version_check", ] -[[package]] -name = "proc-macro-hack" -version = "0.5.20+deprecated" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" - [[package]] name = "proc-macro2" version = "1.0.66" @@ -10837,7 +10812,7 @@ dependencies = [ "frame-system-benchmarking", "frame-system-rpc-runtime-api", "frame-try-runtime", - "hex-literal 0.3.4", + "hex-literal", "log", "pallet-authority-discovery", "pallet-authorship", @@ -10998,7 +10973,7 @@ dependencies = [ "fp-self-contained", "frame-support", "frame-system", - "hex-literal 0.2.2", + "hex-literal", "log", "orml-oracle", "orml-traits", @@ -15424,7 +15399,7 @@ dependencies = [ "frame-system-benchmarking", "frame-system-rpc-runtime-api", "frame-try-runtime", - "hex-literal 0.3.4", + "hex-literal", "log", "pallet-authority-discovery", "pallet-authorship", diff --git a/Cargo.toml b/Cargo.toml index abc6b81d2a..64edcefeff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,7 +76,7 @@ async-trait = "0.1" clap = { version = "4.0.9", features = ["derive"] } codec = { package = "parity-scale-codec", version = "3.0", default-features = false } futures = "0.3.25" -hex-literal = "0.2.1" +hex-literal = "0.3.4" jsonrpsee = { version = "0.16.2", features = ["server", "macros"] } log = "0.4.8" serde = { version = "1.0.119", features = ["derive"] } diff --git a/runtime/centrifuge/Cargo.toml b/runtime/centrifuge/Cargo.toml index 56171b9a09..ddc2e80f54 100644 --- a/runtime/centrifuge/Cargo.toml +++ b/runtime/centrifuge/Cargo.toml @@ -11,7 +11,7 @@ repository = "https://github.com/centrifuge/centrifuge-chain" [dependencies] # third-party dependencies codec = { package = "parity-scale-codec", version = "3.0", default-features = false, features = ["derive"] } -hex-literal = { version = "0.3.4", optional = true } +hex-literal = { version = "0.3.4", default-features = false } log = { version = "0.4.17", default-features = false } scale-info = { version = "2.3.0", default-features = false, features = ["derive"] } serde = { version = "1.0.119", optional = true } @@ -269,7 +269,6 @@ runtime-benchmarks = [ "frame-support/runtime-benchmarks", "frame-system-benchmarking/runtime-benchmarks", "frame-system/runtime-benchmarks", - "hex-literal", "liquidity-pools-gateway-routers/runtime-benchmarks", "orml-asset-registry/runtime-benchmarks", "orml-tokens/runtime-benchmarks", diff --git a/runtime/centrifuge/src/evm.rs b/runtime/centrifuge/src/evm.rs index acae21fcf9..470ce16df9 100644 --- a/runtime/centrifuge/src/evm.rs +++ b/runtime/centrifuge/src/evm.rs @@ -10,13 +10,13 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. -use cfg_primitives::{AccountId, MAXIMUM_BLOCK_WEIGHT, NORMAL_DISPATCH_RATIO}; +use cfg_primitives::{TwoThirdOfCouncil, MAXIMUM_BLOCK_WEIGHT, NORMAL_DISPATCH_RATIO}; use frame_support::{parameter_types, traits::FindAuthor, weights::Weight, ConsensusEngineId}; -use frame_system::EnsureRoot; use pallet_evm::{EnsureAddressRoot, EnsureAddressTruncated}; use runtime_common::{ account_conversion::AccountConverter, evm::{precompile::CentrifugePrecompiles, BaseFeeThreshold, WEIGHT_PER_GAS}, + origin::EnsureAccountOrRootOr, }; use sp_core::{crypto::ByteArray, H160, U256}; use sp_runtime::Permill; @@ -91,6 +91,6 @@ impl pallet_ethereum_transaction::Config for crate::Runtime { } impl axelar_gateway_precompile::Config for crate::Runtime { - type AdminOrigin = EnsureRoot; + type AdminOrigin = EnsureAccountOrRootOr; type RuntimeEvent = crate::RuntimeEvent; } diff --git a/runtime/centrifuge/src/lib.rs b/runtime/centrifuge/src/lib.rs index 5128f93ac8..8660e7daac 100644 --- a/runtime/centrifuge/src/lib.rs +++ b/runtime/centrifuge/src/lib.rs @@ -435,6 +435,8 @@ parameter_types! { impl pallet_liquidity_pools::Config for Runtime { type AccountConverter = AccountConverter; + // NOTE: No need to adapt that. The Router is an artifact and will be removed + // with FI PR type AdminOrigin = EnsureRootOr; type AssetRegistry = OrmlAssetRegistry; type Balance = Balance; @@ -490,8 +492,24 @@ parameter_types! { pub const MaxIncomingMessageSize: u32 = 1024; } +parameter_types! { + // A temporary admin account for the LP logic + // This is a multi-sig controlled pure proxy on mainnet + // - address: "4eEqmbQMbFfNUg6bQnqi9zgUvQvSpNbUgstEM64Xq9FW58Xv" (on Centrifuge) + // (pub key 0x80339e91a87b9c082705fd1a6d39b3e00b46e445ad8c80c127f6a56941c6aa57) + // + // This account is besides Root and 2/3-council able to + // - add valid relayer contracts + // - rm valid relayer contracts + // - add valid LP instance contracts + // - rm valid LP instance contracts + // - add conversions from Axelar `sourceChain` strings to `DomainAddress` + // - set the Axelar gateway contract in the Axelar gateway precompile + pub LpAdminAccount: AccountId = AccountId::new(hex_literal::hex!("80339e91a87b9c082705fd1a6d39b3e00b46e445ad8c80c127f6a56941c6aa57")); +} + impl pallet_liquidity_pools_gateway::Config for Runtime { - type AdminOrigin = EnsureRootOr; + type AdminOrigin = EnsureAccountOrRootOr; type InboundQueue = DummyInboundQueue; type LocalEVMOrigin = pallet_liquidity_pools_gateway::EnsureLocal; type MaxIncomingMessageSize = MaxIncomingMessageSize; @@ -2052,6 +2070,7 @@ mod __runtime_api_use { #[cfg(not(feature = "disable-runtime-api"))] use __runtime_api_use::*; +use runtime_common::origin::EnsureAccountOrRootOr; #[cfg(not(feature = "disable-runtime-api"))] impl_runtime_apis! { diff --git a/runtime/common/Cargo.toml b/runtime/common/Cargo.toml index d03e0834b3..2e04865880 100644 --- a/runtime/common/Cargo.toml +++ b/runtime/common/Cargo.toml @@ -74,7 +74,7 @@ sp-io = { git = "https://github.com/paritytech/substrate", default-features = fa [dev-dependencies] cfg-mocks = { path = "../../libs/mocks", features = ["runtime-benchmarks", "std"] } -hex-literal = "0.2.1" +hex-literal = "0.3.4" sp-io = { git = "https://github.com/paritytech/substrate", default-features = true, branch = "polkadot-v0.9.38" } pallet-collective = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.38" } diff --git a/runtime/common/src/lib.rs b/runtime/common/src/lib.rs index a967163d39..8b0064847e 100644 --- a/runtime/common/src/lib.rs +++ b/runtime/common/src/lib.rs @@ -421,54 +421,34 @@ pub mod xcm_transactor { } pub mod origin { - use std::marker::PhantomData; - use cfg_primitives::AccountId; - use frame_support::{ - dispatch::RawOrigin, - traits::{EitherOfDiverse, EnsureOrigin}, - }; - use frame_system::EnsureRoot; + use frame_support::traits::{EitherOfDiverse, SortedMembers}; + use frame_system::{EnsureRoot, EnsureSignedBy}; use sp_core::Get; - type EnsureAccountOrRoot = - EitherOfDiverse, EnsureRoot>; + pub type EnsureAccountOrRoot = + EitherOfDiverse, AccountId>, EnsureRoot>; - type EnsureAccountOrRootOr = - EitherOfDiverse, EnsureRoot>, O>; + pub type EnsureAccountOrRootOr = EitherOfDiverse< + EitherOfDiverse, AccountId>, EnsureRoot>, + O, + >; - pub struct EnsureAccount(PhantomData); + pub struct AdminOnly(sp_std::marker::PhantomData); - impl EnsureOrigin for EnsureAccount + impl SortedMembers for AdminOnly where - OuterOrigin: - Into, OuterOrigin>> + From> + Clone, Account: Get, { - type Success = (); - - fn try_origin(o: OuterOrigin) -> Result { - o.into().and_then(|raw| match raw { - RawOrigin::Root | RawOrigin::None => Err(OuterOrigin::from(raw)), - RawOrigin::Signed(ref signer) => { - if signer == &Account::get() { - Ok(()) - } else { - Err(OuterOrigin::from(raw)) - } - } - }) - } - - #[cfg(feature = "runtime-benchmarks")] - fn try_successful_origin() -> Result { - Ok(RawOrigin::Signed(Account::get()).into()) + fn sorted_members() -> sp_std::vec::Vec { + sp_std::vec![Account::get()] } } #[cfg(test)] mod test { use cfg_primitives::HalfOfCouncil; + use frame_support::traits::EnsureOrigin; use sp_core::{crypto::AccountId32, parameter_types}; use super::*; @@ -531,53 +511,110 @@ pub mod origin { } } - #[test] - fn works_with_account() { - let origin = OuterOrigin::Raw(RawOrigin::Signed(Admin::get())); + mod ensure_account_or_root_or { + use super::*; - assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_ok()) - } + #[test] + fn works_with_account() { + let origin = OuterOrigin::Raw(RawOrigin::Signed(Admin::get())); - #[test] - fn fails_with_non_admin_account() { - let origin = OuterOrigin::Raw(RawOrigin::Signed(AccountId::from([1u8; 32]))); + assert!( + EnsureAccountOrRootOr::::ensure_origin(origin).is_ok() + ) + } - assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_err()) - } + #[test] + fn fails_with_non_admin_account() { + let origin = OuterOrigin::Raw(RawOrigin::Signed(AccountId::from([1u8; 32]))); - #[test] - fn works_with_half_of_council() { - let origin = OuterOrigin::Council(pallet_collective::RawOrigin::Members(5, 9)); + assert!( + EnsureAccountOrRootOr::::ensure_origin(origin).is_err() + ) + } - assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_ok()) - } + #[test] + fn works_with_half_of_council() { + let origin = OuterOrigin::Council(pallet_collective::RawOrigin::Members(5, 9)); - #[test] - fn fails_with_less_than_half_of_council() { - let origin = OuterOrigin::Council(pallet_collective::RawOrigin::Members(4, 9)); + assert!( + EnsureAccountOrRootOr::::ensure_origin(origin).is_ok() + ) + } - assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_err()) - } + #[test] + fn fails_with_less_than_half_of_council() { + let origin = OuterOrigin::Council(pallet_collective::RawOrigin::Members(4, 9)); - #[test] - fn works_with_root() { - let origin = OuterOrigin::Raw(RawOrigin::Root); + assert!( + EnsureAccountOrRootOr::::ensure_origin(origin).is_err() + ) + } - assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_ok()) - } + #[test] + fn works_with_root() { + let origin = OuterOrigin::Raw(RawOrigin::Root); - #[test] - fn fails_with_none() { - let origin = OuterOrigin::Raw(RawOrigin::None); + assert!( + EnsureAccountOrRootOr::::ensure_origin(origin).is_ok() + ) + } + + #[test] + fn fails_with_none() { + let origin = OuterOrigin::Raw(RawOrigin::None); + + assert!( + EnsureAccountOrRootOr::::ensure_origin(origin).is_err() + ) + } - assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_err()) + #[test] + fn fails_with_dummy() { + let origin = OuterOrigin::Dummy; + + assert!( + EnsureAccountOrRootOr::::ensure_origin(origin).is_err() + ) + } } - #[test] - fn fails_with_dummy() { - let origin = OuterOrigin::Dummy; + mod ensure_account_or_root { + use super::*; + + #[test] + fn works_with_account() { + let origin = OuterOrigin::Raw(RawOrigin::Signed(Admin::get())); + + assert!(EnsureAccountOrRoot::::ensure_origin(origin).is_ok()) + } - assert!(EnsureAccountOrRootOr::::ensure_origin(origin).is_err()) + #[test] + fn fails_with_non_admin_account() { + let origin = OuterOrigin::Raw(RawOrigin::Signed(AccountId::from([1u8; 32]))); + + assert!(EnsureAccountOrRoot::::ensure_origin(origin).is_err()) + } + + #[test] + fn works_with_root() { + let origin = OuterOrigin::Raw(RawOrigin::Root); + + assert!(EnsureAccountOrRoot::::ensure_origin(origin).is_ok()) + } + + #[test] + fn fails_with_none() { + let origin = OuterOrigin::Raw(RawOrigin::None); + + assert!(EnsureAccountOrRoot::::ensure_origin(origin).is_err()) + } + + #[test] + fn fails_with_dummy() { + let origin = OuterOrigin::Dummy; + + assert!(EnsureAccountOrRoot::::ensure_origin(origin).is_err()) + } } } } From 0fe4cff66c235a0a0c8cca2eaf276e1da3346b99 Mon Sep 17 00:00:00 2001 From: Frederik Gartenmeister Date: Tue, 5 Sep 2023 22:16:36 +0200 Subject: [PATCH 5/7] feat: overestimated weights for lp logic --- .../axelar-gateway-precompile/src/lib.rs | 11 +++- .../axelar-gateway-precompile/src/weights.rs | 42 ++++++++++++ .../liquidity-pools-gateway/src/weights.rs | 66 +++++++++++++++++-- pallets/liquidity-pools/src/weights.rs | 62 ++++++++++++++--- runtime/altair/src/evm.rs | 1 + runtime/centrifuge/src/evm.rs | 1 + runtime/development/src/evm.rs | 1 + 7 files changed, 167 insertions(+), 17 deletions(-) create mode 100644 pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/weights.rs diff --git a/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/lib.rs b/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/lib.rs index 89b12b3093..2a272a754b 100644 --- a/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/lib.rs +++ b/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/lib.rs @@ -22,6 +22,8 @@ use sp_core::{bounded::BoundedVec, ConstU32, H160, H256, U256}; use sp_runtime::{DispatchError, DispatchResult}; use sp_std::vec::Vec; +pub use crate::weights::WeightInfo; + pub const MAX_SOURCE_CHAIN_BYTES: u32 = 128; pub const MAX_SOURCE_ADDRESS_BYTES: u32 = 32; pub const MAX_TOKEN_SYMBOL_BYTES: u32 = 32; @@ -33,6 +35,8 @@ pub type Bytes = BoundedBytes>; pub use pallet::*; +pub mod weights; + #[derive( PartialEq, Clone, @@ -90,6 +94,7 @@ pub mod pallet { use sp_core::{H160, H256}; use super::SourceConverter; + use crate::weights::WeightInfo; // Simple declaration of the `Pallet` type. It is placeholder we use to // implement traits and method. @@ -106,6 +111,8 @@ pub mod pallet { /// The origin that is allowed to set the gateway address we accept /// messageas from type AdminOrigin: EnsureOrigin<::RuntimeOrigin>; + + type WeightInfo: WeightInfo; } #[pallet::storage] @@ -162,7 +169,7 @@ pub mod pallet { #[pallet::call] impl Pallet { - #[pallet::weight(0)] + #[pallet::weight(::WeightInfo::set_gateway())] #[pallet::call_index(0)] pub fn set_gateway(origin: OriginFor, address: H160) -> DispatchResult { ::AdminOrigin::ensure_origin(origin)?; @@ -174,7 +181,7 @@ pub mod pallet { Ok(()) } - #[pallet::weight(0)] + #[pallet::weight(::WeightInfo::set_converter())] #[pallet::call_index(1)] pub fn set_converter( origin: OriginFor, diff --git a/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/weights.rs b/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/weights.rs new file mode 100644 index 0000000000..bfe9fba211 --- /dev/null +++ b/pallets/liquidity-pools-gateway/axelar-gateway-precompile/src/weights.rs @@ -0,0 +1,42 @@ +// Copyright 2023 Centrifuge Foundation (centrifuge.io). +// +// This file is part of the Centrifuge chain project. +// Centrifuge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version (see http://www.gnu.org/licenses). +// Centrifuge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +use frame_support::weights::{constants::RocksDbWeight, Weight}; + +pub trait WeightInfo { + fn set_gateway() -> Weight; + fn set_converter() -> Weight; +} + +impl WeightInfo for () { + fn set_gateway() -> Weight { + // TODO: BENCHMARK CORRECTLY + // + // NOTE: Reasonable weight taken from `PoolSystem::set_max_reserve` + // This one has one read and one write for sure and possible one + // read for `AdminOrigin` + Weight::from_parts(17_000_000, 5991) + .saturating_add(RocksDbWeight::get().reads(2)) + .saturating_add(RocksDbWeight::get().writes(1)) + } + + fn set_converter() -> Weight { + // TODO: BENCHMARK CORRECTLY + // + // NOTE: Reasonable weight taken from `PoolSystem::set_max_reserve` + // This one has one read and one write for sure and possible one + // read for `AdminOrigin` + Weight::from_parts(17_000_000, 5991) + .saturating_add(RocksDbWeight::get().reads(2)) + .saturating_add(RocksDbWeight::get().writes(1)) + } +} diff --git a/pallets/liquidity-pools-gateway/src/weights.rs b/pallets/liquidity-pools-gateway/src/weights.rs index 8d8dfa035d..c83bec47fe 100644 --- a/pallets/liquidity-pools-gateway/src/weights.rs +++ b/pallets/liquidity-pools-gateway/src/weights.rs @@ -10,7 +10,7 @@ // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. -use frame_support::weights::Weight; +use frame_support::weights::{constants::RocksDbWeight, Weight}; pub trait WeightInfo { fn set_domain_router() -> Weight; @@ -21,28 +21,80 @@ pub trait WeightInfo { fn process_msg() -> Weight; } +// NOTE: We use temporary weights here. `execute_epoch` is by far our heaviest +// extrinsic. N denotes the number of tranches. 4 is quite heavy and +// should be enough. +const N: u64 = 4; + impl WeightInfo for () { fn set_domain_router() -> Weight { - Weight::from_ref_time(10_000_000) + // TODO: BENCHMARK CORRECTLY + // + // NOTE: Reasonable weight taken from `PoolSystem::set_max_reserve` + // This one has one read and one write for sure and possible one + // read for `AdminOrigin` + Weight::from_parts(17_000_000, 5991) + .saturating_add(RocksDbWeight::get().reads(2)) + .saturating_add(RocksDbWeight::get().writes(1)) } fn add_instance() -> Weight { - Weight::from_ref_time(10_000_000) + // TODO: BENCHMARK CORRECTLY + // + // NOTE: Reasonable weight taken from `PoolSystem::set_max_reserve` + // This one has one read and one write for sure and possible one + // read for `AdminOrigin` + Weight::from_parts(17_000_000, 5991) + .saturating_add(RocksDbWeight::get().reads(2)) + .saturating_add(RocksDbWeight::get().writes(1)) } fn remove_instance() -> Weight { - Weight::from_ref_time(10_000_000) + // TODO: BENCHMARK CORRECTLY + // + // NOTE: Reasonable weight taken from `PoolSystem::set_max_reserve` + // This one has one read and one write for sure and possible one + // read for `AdminOrigin` + Weight::from_parts(17_000_000, 5991) + .saturating_add(RocksDbWeight::get().reads(2)) + .saturating_add(RocksDbWeight::get().writes(1)) } fn add_relayer() -> Weight { - Weight::from_ref_time(10_000_000) + // TODO: BENCHMARK CORRECTLY + // + // NOTE: Reasonable weight taken from `PoolSystem::set_max_reserve` + // This one has one read and one write for sure and possible one + // read for `AdminOrigin` + Weight::from_parts(17_000_000, 5991) + .saturating_add(RocksDbWeight::get().reads(2)) + .saturating_add(RocksDbWeight::get().writes(1)) } fn remove_relayer() -> Weight { - Weight::from_ref_time(10_000_000) + // TODO: BENCHMARK CORRECTLY + // + // NOTE: Reasonable weight taken from `PoolSystem::set_max_reserve` + // This one has one read and one write for sure and possible one + // read for `AdminOrigin` + Weight::from_parts(17_000_000, 5991) + .saturating_add(RocksDbWeight::get().reads(2)) + .saturating_add(RocksDbWeight::get().writes(1)) } fn process_msg() -> Weight { - Weight::from_ref_time(10_000_000) + // TODO: BENCHMARK AND USE REAL WEIGHTS + // + // NOTE: For reference this weight compared to our maximum weight + // * This weight { ref_time: 4333558693, proof_size: 91070 } + // * Maximum weight { ref_time: 500000000000, proof_size: 5242880 } + // + Weight::from_parts(78_019_565, 19974) + .saturating_add(Weight::from_ref_time(38_884_782).saturating_mul(N)) + .saturating_add(RocksDbWeight::get().reads(8)) + .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) + .saturating_add(RocksDbWeight::get().writes(8)) + .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) + .saturating_add(Weight::from_proof_size(17774).saturating_mul(N)) } } diff --git a/pallets/liquidity-pools/src/weights.rs b/pallets/liquidity-pools/src/weights.rs index 8052879699..2d5b4c30ae 100644 --- a/pallets/liquidity-pools/src/weights.rs +++ b/pallets/liquidity-pools/src/weights.rs @@ -10,8 +10,7 @@ // but WITHOUT ANY WARRANTY; without even the implied warranty of // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the // GNU General Public License for more details. - -use frame_support::weights::Weight; +use frame_support::weights::{constants::RocksDbWeight, Weight}; pub trait WeightInfo { fn add_pool() -> Weight; @@ -22,28 +21,75 @@ pub trait WeightInfo { fn set_domain_router() -> Weight; } +// NOTE: We use temporary weights here. `execute_epoch` is by far our heaviest +// extrinsic. N denotes the number of tranches. 4 is quite heavy and +// should be enough. +const N: u64 = 4; + impl WeightInfo for () { fn set_domain_router() -> Weight { - Weight::zero() + // TODO: BENCHMARK AND USE REAL WEIGHTS + Weight::from_parts(78_019_565, 19974) + .saturating_add(Weight::from_ref_time(38_884_782).saturating_mul(N)) + .saturating_add(RocksDbWeight::get().reads(8)) + .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) + .saturating_add(RocksDbWeight::get().writes(8)) + .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) + .saturating_add(Weight::from_proof_size(17774).saturating_mul(N)) } fn add_pool() -> Weight { - Weight::zero() + // TODO: BENCHMARK AND USE REAL WEIGHTS + Weight::from_parts(78_019_565, 19974) + .saturating_add(Weight::from_ref_time(38_884_782).saturating_mul(N)) + .saturating_add(RocksDbWeight::get().reads(8)) + .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) + .saturating_add(RocksDbWeight::get().writes(8)) + .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) + .saturating_add(Weight::from_proof_size(17774).saturating_mul(N)) } fn add_tranche() -> Weight { - Weight::zero() + // TODO: BENCHMARK AND USE REAL WEIGHTS + Weight::from_parts(78_019_565, 19974) + .saturating_add(Weight::from_ref_time(38_884_782).saturating_mul(N)) + .saturating_add(RocksDbWeight::get().reads(8)) + .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) + .saturating_add(RocksDbWeight::get().writes(8)) + .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) + .saturating_add(Weight::from_proof_size(17774).saturating_mul(N)) } fn update_token_price() -> Weight { - Weight::zero() + // TODO: BENCHMARK AND USE REAL WEIGHTS + Weight::from_parts(78_019_565, 19974) + .saturating_add(Weight::from_ref_time(38_884_782).saturating_mul(N)) + .saturating_add(RocksDbWeight::get().reads(8)) + .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) + .saturating_add(RocksDbWeight::get().writes(8)) + .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) + .saturating_add(Weight::from_proof_size(17774).saturating_mul(N)) } fn update_member() -> Weight { - Weight::zero() + // TODO: BENCHMARK AND USE REAL WEIGHTS + Weight::from_parts(78_019_565, 19974) + .saturating_add(Weight::from_ref_time(38_884_782).saturating_mul(N)) + .saturating_add(RocksDbWeight::get().reads(8)) + .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) + .saturating_add(RocksDbWeight::get().writes(8)) + .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) + .saturating_add(Weight::from_proof_size(17774).saturating_mul(N)) } fn transfer() -> Weight { - Weight::zero() + // TODO: BENCHMARK AND USE REAL WEIGHTS + Weight::from_parts(78_019_565, 19974) + .saturating_add(Weight::from_ref_time(38_884_782).saturating_mul(N)) + .saturating_add(RocksDbWeight::get().reads(8)) + .saturating_add(RocksDbWeight::get().reads((7_u64).saturating_mul(N))) + .saturating_add(RocksDbWeight::get().writes(8)) + .saturating_add(RocksDbWeight::get().writes((6_u64).saturating_mul(N))) + .saturating_add(Weight::from_proof_size(17774).saturating_mul(N)) } } diff --git a/runtime/altair/src/evm.rs b/runtime/altair/src/evm.rs index 3276d60d25..148c37f95e 100644 --- a/runtime/altair/src/evm.rs +++ b/runtime/altair/src/evm.rs @@ -92,4 +92,5 @@ impl pallet_ethereum_transaction::Config for Runtime { impl axelar_gateway_precompile::Config for Runtime { type AdminOrigin = EnsureRootOr; type RuntimeEvent = RuntimeEvent; + type WeightInfo = (); } diff --git a/runtime/centrifuge/src/evm.rs b/runtime/centrifuge/src/evm.rs index 470ce16df9..733a319f06 100644 --- a/runtime/centrifuge/src/evm.rs +++ b/runtime/centrifuge/src/evm.rs @@ -93,4 +93,5 @@ impl pallet_ethereum_transaction::Config for crate::Runtime { impl axelar_gateway_precompile::Config for crate::Runtime { type AdminOrigin = EnsureAccountOrRootOr; type RuntimeEvent = crate::RuntimeEvent; + type WeightInfo = (); } diff --git a/runtime/development/src/evm.rs b/runtime/development/src/evm.rs index 5546d1803f..27d300e4f0 100644 --- a/runtime/development/src/evm.rs +++ b/runtime/development/src/evm.rs @@ -93,4 +93,5 @@ impl pallet_ethereum_transaction::Config for Runtime { impl axelar_gateway_precompile::Config for Runtime { type AdminOrigin = EnsureRoot; type RuntimeEvent = RuntimeEvent; + type WeightInfo = (); } From c1e5d34ddfc1988a9620829b66fb17eaf6bcad5c Mon Sep 17 00:00:00 2001 From: Frederik Gartenmeister Date: Tue, 5 Sep 2023 22:27:54 +0200 Subject: [PATCH 6/7] fix: taplooo --- runtime/common/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/common/Cargo.toml b/runtime/common/Cargo.toml index e9b79e71d1..8e64162621 100644 --- a/runtime/common/Cargo.toml +++ b/runtime/common/Cargo.toml @@ -77,8 +77,8 @@ sp-io = { git = "https://github.com/paritytech/substrate", default-features = fa [dev-dependencies] cfg-mocks = { path = "../../libs/mocks", features = ["runtime-benchmarks", "std"] } hex-literal = "0.3.4" -sp-io = { git = "https://github.com/paritytech/substrate", default-features = true, branch = "polkadot-v0.9.38" } pallet-collective = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.38" } +sp-io = { git = "https://github.com/paritytech/substrate", default-features = true, branch = "polkadot-v0.9.38" } [features] default = ["std"] From fbb35fe3631d8cbf747d32e052cf0538e20ab8d5 Mon Sep 17 00:00:00 2001 From: Frederik Gartenmeister Date: Wed, 6 Sep 2023 09:33:14 +0200 Subject: [PATCH 7/7] fix: tests and comment --- runtime/common/src/lib.rs | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/runtime/common/src/lib.rs b/runtime/common/src/lib.rs index c8c8d64b35..bd831925cd 100644 --- a/runtime/common/src/lib.rs +++ b/runtime/common/src/lib.rs @@ -430,10 +430,7 @@ pub mod origin { pub type EnsureAccountOrRoot = EitherOfDiverse, AccountId>, EnsureRoot>; - pub type EnsureAccountOrRootOr = EitherOfDiverse< - EitherOfDiverse, AccountId>, EnsureRoot>, - O, - >; + pub type EnsureAccountOrRootOr = EitherOfDiverse, O>; pub struct AdminOnly(sp_std::marker::PhantomData); @@ -460,13 +457,13 @@ pub mod origin { #[derive(Clone)] enum OuterOrigin { - Raw(RawOrigin), + Raw(frame_system::RawOrigin), Council(pallet_collective::RawOrigin), Dummy, } - impl Into, OuterOrigin>> for OuterOrigin { - fn into(self) -> Result, OuterOrigin> { + impl Into, OuterOrigin>> for OuterOrigin { + fn into(self) -> Result, OuterOrigin> { match self { Self::Raw(raw) => Ok(raw), _ => Err(self), @@ -498,8 +495,8 @@ pub mod origin { } } - impl From> for OuterOrigin { - fn from(value: RawOrigin) -> Self { + impl From> for OuterOrigin { + fn from(value: frame_system::RawOrigin) -> Self { Self::Raw(value) } } @@ -517,7 +514,7 @@ pub mod origin { #[test] fn works_with_account() { - let origin = OuterOrigin::Raw(RawOrigin::Signed(Admin::get())); + let origin = OuterOrigin::Raw(frame_system::RawOrigin::Signed(Admin::get())); assert!( EnsureAccountOrRootOr::::ensure_origin(origin).is_ok() @@ -526,7 +523,8 @@ pub mod origin { #[test] fn fails_with_non_admin_account() { - let origin = OuterOrigin::Raw(RawOrigin::Signed(AccountId::from([1u8; 32]))); + let origin = + OuterOrigin::Raw(frame_system::RawOrigin::Signed(AccountId::from([1u8; 32]))); assert!( EnsureAccountOrRootOr::::ensure_origin(origin).is_err() @@ -553,7 +551,7 @@ pub mod origin { #[test] fn works_with_root() { - let origin = OuterOrigin::Raw(RawOrigin::Root); + let origin = OuterOrigin::Raw(frame_system::RawOrigin::Root); assert!( EnsureAccountOrRootOr::::ensure_origin(origin).is_ok() @@ -562,7 +560,7 @@ pub mod origin { #[test] fn fails_with_none() { - let origin = OuterOrigin::Raw(RawOrigin::None); + let origin = OuterOrigin::Raw(frame_system::RawOrigin::None); assert!( EnsureAccountOrRootOr::::ensure_origin(origin).is_err() @@ -584,28 +582,29 @@ pub mod origin { #[test] fn works_with_account() { - let origin = OuterOrigin::Raw(RawOrigin::Signed(Admin::get())); + let origin = OuterOrigin::Raw(frame_system::RawOrigin::Signed(Admin::get())); assert!(EnsureAccountOrRoot::::ensure_origin(origin).is_ok()) } #[test] fn fails_with_non_admin_account() { - let origin = OuterOrigin::Raw(RawOrigin::Signed(AccountId::from([1u8; 32]))); + let origin = + OuterOrigin::Raw(frame_system::RawOrigin::Signed(AccountId::from([1u8; 32]))); assert!(EnsureAccountOrRoot::::ensure_origin(origin).is_err()) } #[test] fn works_with_root() { - let origin = OuterOrigin::Raw(RawOrigin::Root); + let origin = OuterOrigin::Raw(frame_system::RawOrigin::Root); assert!(EnsureAccountOrRoot::::ensure_origin(origin).is_ok()) } #[test] fn fails_with_none() { - let origin = OuterOrigin::Raw(RawOrigin::None); + let origin = OuterOrigin::Raw(frame_system::RawOrigin::None); assert!(EnsureAccountOrRoot::::ensure_origin(origin).is_err()) }