From 36c7b29746ee5bbe4375bf69f91138c32d1cc04c Mon Sep 17 00:00:00 2001 From: bear Date: Tue, 6 Aug 2024 16:22:08 +0800 Subject: [PATCH] Fix the proof_size over limit case --- Cargo.lock | 1 + frame/ethereum/src/lib.rs | 21 ++++++++------ frame/evm/src/runner/stack.rs | 52 ++++++++++++++++++----------------- primitives/evm/Cargo.toml | 2 ++ primitives/evm/src/lib.rs | 21 ++++++++++++++ template/runtime/src/lib.rs | 39 ++++++-------------------- 6 files changed, 72 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4f8253f0fb..3a74b837bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3478,6 +3478,7 @@ dependencies = [ name = "fp-evm" version = "3.0.0-dev" dependencies = [ + "cumulus-primitives-storage-weight-reclaim", "evm", "frame-support", "num_enum", diff --git a/frame/ethereum/src/lib.rs b/frame/ethereum/src/lib.rs index 09c727e5c5..d75822bd5f 100644 --- a/frame/ethereum/src/lib.rs +++ b/frame/ethereum/src/lib.rs @@ -378,7 +378,11 @@ impl Pallet { true, ); let proof_size_pre_execution = cumulus_primitives_storage_weight_reclaim::get_proof_size(); - TransactionPov::new(weight_limit, extrinsics_len, proof_size_pre_execution) + Some(TransactionPov::new( + weight_limit, + extrinsics_len, + proof_size_pre_execution, + )) } fn recover_signer(transaction: &Transaction) -> Option { @@ -513,8 +517,7 @@ impl Pallet { is_transactional: true, }, transaction_data.clone().into(), - weight_limit, - proof_size_base_cost, + transaction_pov, ) .validate_in_pool_for(&who) .and_then(|v| v.with_chain_id()) @@ -808,8 +811,7 @@ impl Pallet { access_list, is_transactional, validate, - weight_limit, - proof_size_base_cost, + transaction_pov, config.as_ref().unwrap_or_else(|| T::config()), ) { Ok(res) => res, @@ -838,8 +840,7 @@ impl Pallet { access_list, is_transactional, validate, - weight_limit, - proof_size_base_cost, + transaction_pov, config.as_ref().unwrap_or_else(|| T::config()), ) { Ok(res) => res, @@ -881,8 +882,7 @@ impl Pallet { is_transactional: true, }, transaction_data.into(), - weight_limit, - proof_size_base_cost, + transaction_pov, ) .validate_in_block_for(&who) .and_then(|v| v.with_chain_id()) @@ -996,6 +996,9 @@ pub struct InvalidTransactionWrapper(InvalidTransaction); impl From for InvalidTransactionWrapper { fn from(validation_error: TransactionValidationError) -> Self { match validation_error { + TransactionValidationError::ProofLimitTooLow => InvalidTransactionWrapper( + InvalidTransaction::Custom(TransactionValidationError::ProofLimitTooLow as u8), + ), TransactionValidationError::GasLimitTooLow => InvalidTransactionWrapper( InvalidTransaction::Custom(TransactionValidationError::GasLimitTooLow as u8), ), diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs index d2af678da7..18ed7b8b94 100644 --- a/frame/evm/src/runner/stack.rs +++ b/frame/evm/src/runner/stack.rs @@ -226,33 +226,17 @@ where origin: source, }; let metadata = StackSubstateMetadata::new(gas_limit, config); - let state = SubstrateStackState::new(&vicinity, metadata); + let state = SubstrateStackState::new(&vicinity, metadata, transaction_pov); let mut executor = StackExecutor::new_with_precompiles(state, config, precompiles); let (reason, retv) = f(&mut executor); // Post execution. let opcode_used_gas = executor.used_gas(); - - let proof_size_used_gas = if let Some(mut pov) = transaction_pov { - let proof_size_post_execution = - cumulus_primitives_storage_weight_reclaim::get_proof_size().unwrap_or_default(); - let proof_size_used = proof_size_post_execution - .saturating_sub(pov.proof_size_pre_execution.unwrap_or_default()) - .saturating_add(pov.extrinsics_len); - - // If the proof size used is greater than the weight limit, we skip the storage apply and return early. - if proof_size_used > pov.weight_limit.proof_size() { - return Err(RunnerError { - error: Error::::ProofLimitTooLow, - weight, - }); - } - - pov.proof_size_used = proof_size_used; - proof_size_used.saturating_mul(T::GasLimitPovSizeRatio::get()) - } else { - 0 - }; + let proof_size_used_gas = transaction_pov.map_or(0, |mut pov| { + pov.proof_size_used = pov.proof_size_used(); + pov.proof_size_used + .saturating_mul(T::GasLimitPovSizeRatio::get()) + }); let used_gas = U256::from(core::cmp::max(opcode_used_gas, proof_size_used_gas)); let fee = used_gas.saturating_mul(total_fee_per_gas); @@ -642,12 +626,17 @@ pub struct SubstrateStackState<'vicinity, 'config, T> { vicinity: &'vicinity Vicinity, substate: SubstrateStackSubstate<'config>, original_storage: BTreeMap<(H160, H256), H256>, + transaction_pov: Option, _marker: PhantomData, } impl<'vicinity, 'config, T: Config> SubstrateStackState<'vicinity, 'config, T> { /// Create a new backend with given vicinity. - pub fn new(vicinity: &'vicinity Vicinity, metadata: StackSubstateMetadata<'config>) -> Self { + pub fn new( + vicinity: &'vicinity Vicinity, + metadata: StackSubstateMetadata<'config>, + transaction_pov: Option, + ) -> Self { Self { vicinity, substate: SubstrateStackSubstate { @@ -656,8 +645,9 @@ impl<'vicinity, 'config, T: Config> SubstrateStackState<'vicinity, 'config, T> { logs: Vec::new(), parent: None, }, - _marker: PhantomData, original_storage: BTreeMap::new(), + transaction_pov, + _marker: PhantomData, } } } @@ -765,7 +755,19 @@ where } fn exit_commit(&mut self) -> Result<(), ExitError> { - self.substate.exit_commit() + match self.transaction_pov { + Some(pov) if pov.proof_size_used > pov.weight_limit.proof_size() => { + log::debug!( + target: "evm", + "Exceeded proof size limit. Proof size used: {}, proof size limit: {}", + pov.proof_size_used, + pov.weight_limit.proof_size() + ); + self.substate.exit_discard()?; + Err(ExitError::OutOfGas) + } + _ => self.substate.exit_commit(), + } } fn exit_revert(&mut self) -> Result<(), ExitError> { diff --git a/primitives/evm/Cargo.toml b/primitives/evm/Cargo.toml index 9cacc4e27d..43285d6dbf 100644 --- a/primitives/evm/Cargo.toml +++ b/primitives/evm/Cargo.toml @@ -16,6 +16,7 @@ num_enum = { workspace = true, default-features = false } scale-codec = { package = "parity-scale-codec", workspace = true } scale-info = { workspace = true } serde = { workspace = true, optional = true } +cumulus-primitives-storage-weight-reclaim = { workspace = true } # Substrate frame-support = { workspace = true } sp-core = { workspace = true } @@ -32,6 +33,7 @@ std = [ # Substrate "frame-support/std", "sp-core/std", + "cumulus-primitives-storage-weight-reclaim/std", "sp-runtime/std", ] serde = [ diff --git a/primitives/evm/src/lib.rs b/primitives/evm/src/lib.rs index c0ef435311..0d2a08dc56 100644 --- a/primitives/evm/src/lib.rs +++ b/primitives/evm/src/lib.rs @@ -97,6 +97,27 @@ impl TransactionPov { proof_size_used: extrinsics_len, } } + + pub fn proof_size_used(&self) -> u64 { + // if we don't have proof_size_pre_execution, that means that we don't care about the tx proof size + let Some(proof_size_pre_execution) = self.proof_size_pre_execution else { + return 0; + }; + + // If proof_size_pre_execution is enable, but the proof_size_post_execution is disable, maybe the proof_size host function + // doesn't work. + let Some(proof_size_post_execution) = + cumulus_primitives_storage_weight_reclaim::get_proof_size() + else { + return self.proof_size_used; + }; + + let proof_size_used = proof_size_post_execution + .saturating_sub(proof_size_pre_execution) + .saturating_add(self.extrinsics_len); + + proof_size_used + } } #[derive(Clone, Copy, Eq, PartialEq, Debug, Encode, Decode, TypeInfo)] diff --git a/template/runtime/src/lib.rs b/template/runtime/src/lib.rs index c3daf32a35..077b976d2e 100644 --- a/template/runtime/src/lib.rs +++ b/template/runtime/src/lib.rs @@ -48,7 +48,7 @@ use pallet_transaction_payment::{ConstFeeMultiplier, FungibleAdapter}; use sp_genesis_builder::PresetId; // Frontier use fp_account::EthereumSignature; -use fp_evm::weight_per_gas; +use fp_evm::{weight_per_gas, TransactionPov}; use fp_rpc::TransactionStatus; use pallet_ethereum::{Call::transact, PostLogContent, Transaction as EthereumTransaction}; use pallet_evm::{ @@ -833,19 +833,10 @@ impl_runtime_apis! { } else { gas_limit.low_u64() }; - let without_base_extrinsic_weight = true; - - let (weight_limit, proof_size_base_cost) = - match ::GasWeightMapping::gas_to_weight( - gas_limit, - without_base_extrinsic_weight - ) { - weight_limit if weight_limit.proof_size() > 0 => { - (Some(weight_limit), Some(estimated_transaction_len as u64)) - } - _ => (None, None), - }; + let weight_limit = ::GasWeightMapping::gas_to_weight(gas_limit, true); + let proof_size_pre_execution = cumulus_primitives_storage_weight_reclaim::get_proof_size(); + let transaction_pov = TransactionPov::new(weight_limit, estimated_transaction_len as u64, proof_size_pre_execution); ::Runner::call( from, to, @@ -858,8 +849,7 @@ impl_runtime_apis! { access_list.unwrap_or_default(), false, true, - weight_limit, - proof_size_base_cost, + Some(transaction_pov), config.as_ref().unwrap_or(::config()), ).map_err(|err| err.error.into()) } @@ -912,19 +902,9 @@ impl_runtime_apis! { } else { gas_limit.low_u64() }; - let without_base_extrinsic_weight = true; - - let (weight_limit, proof_size_base_cost) = - match ::GasWeightMapping::gas_to_weight( - gas_limit, - without_base_extrinsic_weight - ) { - weight_limit if weight_limit.proof_size() > 0 => { - (Some(weight_limit), Some(estimated_transaction_len as u64)) - } - _ => (None, None), - }; - + let weight_limit = ::GasWeightMapping::gas_to_weight(gas_limit, true); + let proof_size_pre_execution = cumulus_primitives_storage_weight_reclaim::get_proof_size(); + let transaction_pov = TransactionPov::new(weight_limit, estimated_transaction_len as u64, proof_size_pre_execution); ::Runner::create( from, data, @@ -936,8 +916,7 @@ impl_runtime_apis! { access_list.unwrap_or_default(), false, true, - weight_limit, - proof_size_base_cost, + Some(transaction_pov), config.as_ref().unwrap_or(::config()), ).map_err(|err| err.error.into()) }