From 584f5a2422cb64b07ca9506db8f9fd06fab0f28a Mon Sep 17 00:00:00 2001 From: bernardo Date: Thu, 9 Mar 2023 11:51:06 -0300 Subject: [PATCH] Precompiles uses StorageMap; add struct PrecompileLabel; add_precompile + remove_precompile dispatchables --- Cargo.lock | 2 + frame/evm/Cargo.toml | 2 + frame/evm/precompile/dispatch/src/mock.rs | 1 + frame/evm/src/lib.rs | 68 ++++++++++++--- frame/evm/src/mock.rs | 1 + frame/evm/src/tests.rs | 25 ++++++ template/node/Cargo.toml | 1 + template/node/src/chain_spec.rs | 51 +++++++++-- template/runtime/src/lib.rs | 1 + template/runtime/src/precompiles.rs | 100 ++++++++++++++++++---- 10 files changed, 218 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eb9aa5af2f..582bde184e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2472,6 +2472,7 @@ dependencies = [ "futures", "jsonrpsee", "log", + "pallet-evm", "pallet-transaction-payment", "pallet-transaction-payment-rpc", "pallet-transaction-payment-rpc-runtime-api", @@ -4882,6 +4883,7 @@ dependencies = [ "parity-scale-codec", "rlp", "scale-info", + "serde", "sp-core", "sp-io", "sp-runtime", diff --git a/frame/evm/Cargo.toml b/frame/evm/Cargo.toml index d5bd7084cf..e5dd3d87ac 100644 --- a/frame/evm/Cargo.toml +++ b/frame/evm/Cargo.toml @@ -12,6 +12,7 @@ repository = { workspace = true } targets = ["x86_64-unknown-linux-gnu"] [dependencies] +serde = { workspace = true, default-features = false } environmental = { workspace = true, optional = true } evm = { workspace = true, features = ["with-codec"] } hex = { version = "0.4.3", default-features = false, features = ["alloc"] } @@ -40,6 +41,7 @@ pallet-evm-precompile-simple = { workspace = true } [features] default = ["std"] std = [ + "serde/std", "environmental?/std", "evm/std", "evm/with-serde", diff --git a/frame/evm/precompile/dispatch/src/mock.rs b/frame/evm/precompile/dispatch/src/mock.rs index 173091f4f6..979f1c2ff3 100644 --- a/frame/evm/precompile/dispatch/src/mock.rs +++ b/frame/evm/precompile/dispatch/src/mock.rs @@ -159,6 +159,7 @@ impl pallet_evm::Config for Test { type OnChargeTransaction = (); type OnCreate = (); type FindAuthor = FindAuthorTruncated; + type PrecompileModifierOrigin = frame_system::EnsureRoot; } pub(crate) struct MockHandle { diff --git a/frame/evm/src/lib.rs b/frame/evm/src/lib.rs index 6213c9cd2a..09cefdbca0 100644 --- a/frame/evm/src/lib.rs +++ b/frame/evm/src/lib.rs @@ -74,6 +74,8 @@ use frame_support::{ }; use frame_system::RawOrigin; use impl_trait_for_tuples::impl_for_tuples; +use scale_codec::{Decode, Encode}; +use scale_info::TypeInfo; use sp_core::{Hasher, H160, H256, U256}; use sp_runtime::{ traits::{BadOrigin, Saturating, UniqueSaturatedInto, Zero}, @@ -155,6 +157,9 @@ pub mod pallet { /// Find author for the current block. type FindAuthor: FindAuthor; + /// Origin allowed to modify Precompiles + type PrecompileModifierOrigin: EnsureOrigin; + /// EVM config used in the module. fn config() -> &'static EvmConfig { &LONDON_CONFIG @@ -404,6 +409,37 @@ pub mod pallet { pays_fee: Pays::No, }) } + + /// Add a precompile to storage + #[pallet::call_index(4)] + #[pallet::weight(T::DbWeight::get().reads_writes(0, 1))] + pub fn add_precompile( + origin: OriginFor, + address: H160, + label: PrecompileLabel, + ) -> DispatchResult { + T::PrecompileModifierOrigin::ensure_origin(origin)?; + + Self::do_add_precompile(&address, label); + + Ok(()) + } + + /// Remove a precompile from storage + #[pallet::call_index(5)] + #[pallet::weight(T::DbWeight::get().reads_writes(0, 1))] + pub fn remove_precompile( + origin: OriginFor, + address: H160, + ) -> DispatchResult { + T::PrecompileModifierOrigin::ensure_origin(origin)?; + + ensure!(Precompiles::::contains_key(address), Error::::PrecompileDoesNotExist); + + Self::do_remove_precompile(&address); + + Ok(()) + } } #[pallet::event] @@ -445,6 +481,8 @@ pub mod pallet { Reentrancy, /// EIP-3607, TransactionMustComeFromEOA, + /// Precompile does not exist in storage + PrecompileDoesNotExist, } impl From for Error { @@ -467,7 +505,7 @@ pub mod pallet { #[cfg_attr(feature = "std", derive(Default))] pub struct GenesisConfig { pub accounts: std::collections::BTreeMap, - pub precompiles: Vec<(Vec, H160)>, + pub precompiles: Vec<(H160, PrecompileLabel)>, } #[pallet::genesis_build] @@ -499,8 +537,8 @@ pub mod pallet { } } - for (label, address) in &self.precompiles { - Pallet::::add_precompile(label, address); + for (address, label) in &self.precompiles { + Pallet::::do_add_precompile(address, label.clone()); } } } @@ -515,15 +553,21 @@ pub mod pallet { StorageDoubleMap<_, Blake2_128Concat, H160, Blake2_128Concat, H256, H256, ValueQuery>; /// Allows for precompiles to have arbitrary addresses, potentially more than one. - /// `k1`: precompile label, e.g.: `b"Sha3FIPS256".to_vec()` - /// `k2`: precompile address - /// + /// `key`: precompile `H160` address + /// `value`: precompile label, e.g.: `PrecompileLabel { label: b"Sha3FIPS256".to_vec() }` + /// Please note that adding a new precompile label here is not enough to guarantee its execution - /// It is also required to list the new label on the implementation of `PrecompileSet::execute()` + /// It is also required to list new labels on the implementation of `PrecompileSet::execute()` #[pallet::storage] #[pallet::getter(fn precompiles)] pub type Precompiles = - StorageDoubleMap<_, Blake2_128Concat, Vec, Blake2_128Concat, H160, (), OptionQuery>; + StorageMap<_, Blake2_128Concat, H160, PrecompileLabel, ValueQuery>; +} + +#[derive(Decode, Encode, Default, TypeInfo, Clone, PartialEq, Debug)] +#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] +pub struct PrecompileLabel { + pub label: Vec, } /// Type alias for currency balance. @@ -688,13 +732,13 @@ static LONDON_CONFIG: EvmConfig = EvmConfig::london(); impl Pallet { /// Add a precompile to storage - pub fn add_precompile(label: &Vec, address: &H160) { - Precompiles::::set(label, address, Some(())); + pub fn do_add_precompile(address: &H160, label: PrecompileLabel) { + Precompiles::::set(address, label); } /// Remove a precompile from storage - pub fn remove_precompile(label: &Vec, address: &H160) { - Precompiles::::remove(label, address); + pub fn do_remove_precompile(address: &H160) { + Precompiles::::remove(address); } /// Check whether an account is empty. diff --git a/frame/evm/src/mock.rs b/frame/evm/src/mock.rs index a82721fe5e..d92721cfd6 100644 --- a/frame/evm/src/mock.rs +++ b/frame/evm/src/mock.rs @@ -152,6 +152,7 @@ impl crate::Config for Test { type OnChargeTransaction = (); type OnCreate = (); type FindAuthor = FindAuthorTruncated; + type PrecompileModifierOrigin = frame_system::EnsureRoot; } /// Exemple PrecompileSet with only Identity precompile. diff --git a/frame/evm/src/tests.rs b/frame/evm/src/tests.rs index 52592e86a5..74b15a09c9 100644 --- a/frame/evm/src/tests.rs +++ b/frame/evm/src/tests.rs @@ -620,3 +620,28 @@ fn eip3607_transaction_from_precompile_should_fail() { } }); } + +#[test] +fn precompile_storage_works() { + new_test_ext().execute_with(|| { + let origin = RuntimeOrigin::root(); + let address = H160::from_low_u64_be(1); + + let mut read_precompile = EVM::precompiles(address); + assert_eq!(read_precompile, PrecompileLabel::default()); + + let label = PrecompileLabel { + label: b"ECRecover".to_vec(), + }; + + assert_ok!(EVM::add_precompile(origin.clone(), address, label.clone())); + + read_precompile = EVM::precompiles(address); + assert_eq!(read_precompile, label); + + assert_ok!(EVM::remove_precompile(origin, address)); + + read_precompile = EVM::precompiles(address); + assert_eq!(read_precompile, PrecompileLabel::default()); + }); +} diff --git a/template/node/Cargo.toml b/template/node/Cargo.toml index a429a8b08a..9f93e186ef 100644 --- a/template/node/Cargo.toml +++ b/template/node/Cargo.toml @@ -66,6 +66,7 @@ frame-system = { workspace = true } pallet-transaction-payment = { workspace = true } # Frontier +pallet-evm = { workspace = true } fc-cli = { workspace = true } fc-consensus = { workspace = true } fc-db = { workspace = true } diff --git a/template/node/src/chain_spec.rs b/template/node/src/chain_spec.rs index fb938e8ce3..557732394e 100644 --- a/template/node/src/chain_spec.rs +++ b/template/node/src/chain_spec.rs @@ -13,6 +13,8 @@ use frontier_template_runtime::{ AccountId, EnableManualSeal, GenesisConfig, Signature, WASM_BINARY, }; +use pallet_evm::PrecompileLabel; + // The URL for the telemetry server. // const STAGING_TELEMETRY_URL: &str = "wss://telemetry.polkadot.io/submit/"; @@ -252,14 +254,49 @@ fn testnet_genesis( }, precompiles: vec![ // Ethereum precompiles : - (b"ECRecover".to_vec(), H160::from_low_u64_be(1)), - (b"Sha256".to_vec(), H160::from_low_u64_be(2)), - (b"Ripemd160".to_vec(), H160::from_low_u64_be(3)), - (b"Identity".to_vec(), H160::from_low_u64_be(4)), - (b"Modexp".to_vec(), H160::from_low_u64_be(5)), + ( + H160::from_low_u64_be(1), + PrecompileLabel { + label: b"ECRecover".to_vec(), + }, + ), + ( + H160::from_low_u64_be(2), + PrecompileLabel { + label: b"Sha256".to_vec(), + }, + ), + ( + H160::from_low_u64_be(3), + PrecompileLabel { + label: b"Ripemd160".to_vec(), + }, + ), + ( + H160::from_low_u64_be(4), + PrecompileLabel { + label: b"Identity".to_vec(), + }, + ), + ( + H160::from_low_u64_be(5), + PrecompileLabel { + label: b"Modexp".to_vec(), + }, + ), // Non-Frontier specific nor Ethereum precompiles : - (b"Sha3FIPS256".to_vec(), H160::from_low_u64_be(1024)), - (b"Sha3FIPS256".to_vec(), H160::from_low_u64_be(1025)), + ( + H160::from_low_u64_be(1024), + PrecompileLabel { + label: b"Sha3FIPS256".to_vec(), + }, + ), + ( + H160::from_low_u64_be(1025), + PrecompileLabel { + label: b"Sha3FIPS256".to_vec(), + }, + ), ], }, ethereum: Default::default(), diff --git a/template/runtime/src/lib.rs b/template/runtime/src/lib.rs index 4b0aec6759..3d4a424fef 100644 --- a/template/runtime/src/lib.rs +++ b/template/runtime/src/lib.rs @@ -343,6 +343,7 @@ impl pallet_evm::Config for Runtime { type OnChargeTransaction = (); type OnCreate = (); type FindAuthor = FindAuthorTruncated; + type PrecompileModifierOrigin = frame_system::EnsureRoot; } parameter_types! { diff --git a/template/runtime/src/precompiles.rs b/template/runtime/src/precompiles.rs index 49fd9bc1b0..c20e19c715 100644 --- a/template/runtime/src/precompiles.rs +++ b/template/runtime/src/precompiles.rs @@ -2,7 +2,7 @@ use pallet_evm::{Precompile, PrecompileHandle, PrecompileResult, PrecompileSet}; use sp_core::H160; use sp_std::marker::PhantomData; -use pallet_evm::Precompiles; +use pallet_evm::{PrecompileLabel, Precompiles}; use pallet_evm_precompile_modexp::Modexp; use pallet_evm_precompile_sha3fips::Sha3FIPS256; use pallet_evm_precompile_simple::{ECRecover, ECRecoverPublicKey, Identity, Ripemd160, Sha256}; @@ -24,26 +24,54 @@ where fn execute(&self, handle: &mut impl PrecompileHandle) -> Option { match handle.code_address() { // Ethereum precompiles : - a if Precompiles::::contains_key(b"ECRecover".to_vec(), a) => { + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"ECRecover".to_vec(), + } => + { Some(ECRecover::execute(handle)) } - a if Precompiles::::contains_key(b"Sha256".to_vec(), a) => { + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"Sha256".to_vec(), + } => + { Some(Sha256::execute(handle)) } - a if Precompiles::::contains_key(b"Ripemd160".to_vec(), a) => { + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"Ripemd160".to_vec(), + } => + { Some(Ripemd160::execute(handle)) } - a if Precompiles::::contains_key(b"Identity".to_vec(), a) => { + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"Identity".to_vec(), + } => + { Some(Identity::execute(handle)) } - a if Precompiles::::contains_key(b"Modexp".to_vec(), a) => { + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"Modexp".to_vec(), + } => + { Some(Modexp::execute(handle)) } // Non-Frontier specific nor Ethereum precompiles : - a if Precompiles::::contains_key(b"Sha3FIPS256".to_vec(), a) => { + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"Sha3FIPS256".to_vec(), + } => + { Some(Sha3FIPS256::execute(handle)) } - a if Precompiles::::contains_key(b"ECRecoverPublicKey".to_vec(), a) => { + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"ECRecoverPublicKey".to_vec(), + } => + { Some(ECRecoverPublicKey::execute(handle)) } _ => None, @@ -52,13 +80,55 @@ where fn is_precompile(&self, address: H160) -> bool { match address { - a if Precompiles::::contains_key(b"ECRecover".to_vec(), a) => true, - a if Precompiles::::contains_key(b"Sha256".to_vec(), a) => true, - a if Precompiles::::contains_key(b"Ripemd160".to_vec(), a) => true, - a if Precompiles::::contains_key(b"Identity".to_vec(), a) => true, - a if Precompiles::::contains_key(b"Modexp".to_vec(), a) => true, - a if Precompiles::::contains_key(b"Sha3FIPS256".to_vec(), a) => true, - a if Precompiles::::contains_key(b"ECRecoverPublicKey".to_vec(), a) => true, + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"ECRecover".to_vec(), + } => + { + true + } + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"Sha256".to_vec(), + } => + { + true + } + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"Ripemd160".to_vec(), + } => + { + true + } + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"Identity".to_vec(), + } => + { + true + } + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"Modexp".to_vec(), + } => + { + true + } + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"Sha3FIPS256".to_vec(), + } => + { + true + } + a if Precompiles::::get(a) + == PrecompileLabel { + label: b"ECRecoverPublicKey".to_vec(), + } => + { + true + } _ => false, } }