Skip to content

Commit

Permalink
Add CheckWeight for evm txs, copy from
Browse files Browse the repository at this point in the history
(1) Add extrinsic weight and lenght check (polkadot-evm#637)
(2) fix(frame): CheckWeight controls should be applied on pre_dispatch (polkadot-evm#749)
  • Loading branch information
boundless-forest authored and icodezjb committed Dec 27, 2023
1 parent b36e8b9 commit cfef5ca
Show file tree
Hide file tree
Showing 10 changed files with 255 additions and 150 deletions.
216 changes: 108 additions & 108 deletions Cargo.lock

Large diffs are not rendered by default.

27 changes: 22 additions & 5 deletions frame/ethereum/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ use frame_support::{
dispatch::DispatchResultWithPostInfo,
scale_info::TypeInfo,
traits::{EnsureOrigin, Get, PalletInfoAccess},
weights::{Pays, PostDispatchInfo, Weight},
weights::{DispatchInfo, Pays, PostDispatchInfo, Weight},
};
use frame_system::{pallet_prelude::OriginFor, WeightInfo};
use frame_system::{pallet_prelude::OriginFor, CheckWeight, WeightInfo};
use pallet_evm::{BlockHashMapping, FeeCalculator, GasWeightMapping, Runner};
use sha3::{Digest, Keccak256};
use sp_runtime::{
generic::DigestItem,
traits::{One, Saturating, UniqueSaturatedInto, Zero},
traits::{DispatchInfoOf, Dispatchable, One, Saturating, UniqueSaturatedInto, Zero},
transaction_validity::{
InvalidTransaction, TransactionValidity, TransactionValidityError, ValidTransactionBuilder,
},
Expand Down Expand Up @@ -110,6 +110,8 @@ impl<O: Into<Result<RawOrigin, O>> + From<RawOrigin>> EnsureOrigin<O>
impl<T: Config> Call<T>
where
OriginFor<T>: Into<Result<RawOrigin, OriginFor<T>>>,
T: Send + Sync + Config,
T::Call: Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
{
pub fn is_self_contained(&self) -> bool {
match self {
Expand Down Expand Up @@ -137,19 +139,34 @@ where
pub fn pre_dispatch_self_contained(
&self,
origin: &H160,
dispatch_info: &DispatchInfoOf<T::Call>,
len: usize,
) -> Option<Result<(), TransactionValidityError>> {
if let Call::transact { transaction } = self {
if let Err(e) = CheckWeight::<T>::do_pre_dispatch(dispatch_info, len) {
return Some(Err(e));
}

Some(Pallet::<T>::validate_transaction_in_block(
*origin,
&transaction,
transaction,
))
} else {
None
}
}

pub fn validate_self_contained(&self, origin: &H160) -> Option<TransactionValidity> {
pub fn validate_self_contained(
&self,
origin: &H160,
dispatch_info: &DispatchInfoOf<T::Call>,
len: usize,
) -> Option<TransactionValidity> {
if let Call::transact { transaction } = self {
if let Err(e) = CheckWeight::<T>::do_validate(dispatch_info, len) {
return Some(Err(e));
}

Some(Pallet::<T>::validate_transaction_in_pool(
*origin,
transaction,
Expand Down
13 changes: 10 additions & 3 deletions frame/ethereum/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,19 +193,26 @@ impl fp_self_contained::SelfContainedCall for Call {
}
}

fn validate_self_contained(&self, info: &Self::SignedInfo) -> Option<TransactionValidity> {
fn validate_self_contained(
&self,
info: &Self::SignedInfo,
dispatch_info: &DispatchInfoOf<Call>,
len: usize,
) -> Option<TransactionValidity> {
match self {
Call::Ethereum(call) => call.validate_self_contained(info),
Call::Ethereum(call) => call.validate_self_contained(info, dispatch_info, len),
_ => None,
}
}

fn pre_dispatch_self_contained(
&self,
info: &Self::SignedInfo,
dispatch_info: &DispatchInfoOf<Call>,
len: usize,
) -> Option<Result<(), TransactionValidityError>> {
match self {
Call::Ethereum(call) => call.pre_dispatch_self_contained(info),
Call::Ethereum(call) => call.pre_dispatch_self_contained(info, dispatch_info, len),
_ => None,
}
}
Expand Down
29 changes: 22 additions & 7 deletions frame/ethereum/src/tests/eip1559.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,14 @@ fn transaction_without_enough_gas_should_not_work() {

let call = crate::Call::<Test>::transact { transaction };
let source = call.check_self_contained().unwrap().unwrap();

let extrinsic = fp_self_contained::CheckedExtrinsic::<u64, crate::mock::Call, SignedExtra, _> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call.clone()),
};
let dispatch_info = extrinsic.get_dispatch_info();
assert_err!(
call.validate_self_contained(&source).unwrap(),
call.validate_self_contained(&source, &dispatch_info, 0)
.unwrap(),
InvalidTransaction::Payment
);
});
Expand All @@ -83,9 +88,15 @@ fn transaction_with_to_low_nonce_should_not_work() {
transaction: signed,
};
let source = call.check_self_contained().unwrap().unwrap();
let extrinsic = fp_self_contained::CheckedExtrinsic::<u64, crate::mock::Call, SignedExtra, H160> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call.clone()),
};
let dispatch_info = extrinsic.get_dispatch_info();

assert_eq!(
call.validate_self_contained(&source).unwrap(),
call.validate_self_contained(&source, &dispatch_info, 0)
.unwrap(),
ValidTransactionBuilder::default()
.and_provides((alice.address, U256::from(1)))
.priority(0u64)
Expand All @@ -105,9 +116,15 @@ fn transaction_with_to_low_nonce_should_not_work() {
transaction: signed2,
};
let source2 = call2.check_self_contained().unwrap().unwrap();
let extrinsic2 = fp_self_contained::CheckedExtrinsic::<u64, crate::mock::Call, SignedExtra, _> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call2.clone()),
};

assert_err!(
call2.validate_self_contained(&source2).unwrap(),
call2
.validate_self_contained(&source2, &extrinsic2.get_dispatch_info(), 0)
.unwrap(),
InvalidTransaction::Stale
);
});
Expand All @@ -131,7 +148,6 @@ fn transaction_with_to_hight_nonce_should_fail_in_block() {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call),
};
use frame_support::weights::GetDispatchInfo as _;
let dispatch_info = extrinsic.get_dispatch_info();
assert_err!(
extrinsic.apply::<Test>(&dispatch_info, 0),
Expand All @@ -155,7 +171,6 @@ fn transaction_with_invalid_chain_id_should_fail_in_block() {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call),
};
use frame_support::weights::GetDispatchInfo as _;
let dispatch_info = extrinsic.get_dispatch_info();
assert_err!(
extrinsic.apply::<Test>(&dispatch_info, 0),
Expand Down Expand Up @@ -337,7 +352,7 @@ fn self_contained_transaction_with_extra_gas_should_adjust_weight_with_post_disp
transaction: signed,
};
let source = call.check_self_contained().unwrap().unwrap();
let extrinsic = CheckedExtrinsic::<_, _, frame_system::CheckWeight<Test>, _> {
let extrinsic = fp_self_contained::CheckedExtrinsic::<_, _, frame_system::CheckWeight<Test>, _> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call),
};
Expand Down
26 changes: 22 additions & 4 deletions frame/ethereum/src/tests/eip2930.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,15 @@ fn transaction_without_enough_gas_should_not_work() {

let call = crate::Call::<Test>::transact { transaction };
let source = call.check_self_contained().unwrap().unwrap();
let extrinsic = fp_self_contained::CheckedExtrinsic::<u64, crate::mock::Call, SignedExtra, _> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call.clone()),
};
let dispatch_info = extrinsic.get_dispatch_info();

assert_err!(
call.validate_self_contained(&source).unwrap(),
call.validate_self_contained(&source, &dispatch_info, 0)
.unwrap(),
InvalidTransaction::Payment
);
});
Expand All @@ -83,9 +89,15 @@ fn transaction_with_to_low_nonce_should_not_work() {
transaction: signed,
};
let source = call.check_self_contained().unwrap().unwrap();
let extrinsic = fp_self_contained::CheckedExtrinsic::<u64, crate::mock::Call, SignedExtra, H160> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call.clone()),
};
let dispatch_info = extrinsic.get_dispatch_info();

assert_eq!(
call.validate_self_contained(&source).unwrap(),
call.validate_self_contained(&source, &dispatch_info, 0)
.unwrap(),
ValidTransactionBuilder::default()
.and_provides((alice.address, U256::from(1)))
.priority(0u64)
Expand All @@ -105,9 +117,15 @@ fn transaction_with_to_low_nonce_should_not_work() {
transaction: signed2,
};
let source2 = call2.check_self_contained().unwrap().unwrap();
let extrinsic2 = fp_self_contained::CheckedExtrinsic::<u64, crate::mock::Call, SignedExtra, _> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call2.clone()),
};

assert_err!(
call2.validate_self_contained(&source2).unwrap(),
call2
.validate_self_contained(&source2, &extrinsic2.get_dispatch_info(), 0)
.unwrap(),
InvalidTransaction::Stale
);
});
Expand Down Expand Up @@ -334,7 +352,7 @@ fn self_contained_transaction_with_extra_gas_should_adjust_weight_with_post_disp
transaction: signed,
};
let source = call.check_self_contained().unwrap().unwrap();
let extrinsic = CheckedExtrinsic::<_, _, frame_system::CheckWeight<Test>, _> {
let extrinsic = fp_self_contained::CheckedExtrinsic::<_, _, frame_system::CheckWeight<Test>, _> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call),
};
Expand Down
26 changes: 22 additions & 4 deletions frame/ethereum/src/tests/legacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,15 @@ fn transaction_without_enough_gas_should_not_work() {

let call = crate::Call::<Test>::transact { transaction };
let source = call.check_self_contained().unwrap().unwrap();
let extrinsic = fp_self_contained::CheckedExtrinsic::<u64, crate::mock::Call, SignedExtra, _> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call.clone()),
};
let dispatch_info = extrinsic.get_dispatch_info();

assert_err!(
call.validate_self_contained(&source).unwrap(),
call.validate_self_contained(&source, &dispatch_info, 0)
.unwrap(),
InvalidTransaction::Payment
);
});
Expand All @@ -83,9 +89,15 @@ fn transaction_with_to_low_nonce_should_not_work() {
transaction: signed,
};
let source = call.check_self_contained().unwrap().unwrap();
let extrinsic = fp_self_contained::CheckedExtrinsic::<u64, crate::mock::Call, SignedExtra, H160> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call.clone()),
};
let dispatch_info = extrinsic.get_dispatch_info();

assert_eq!(
call.validate_self_contained(&source).unwrap(),
call.validate_self_contained(&source, &dispatch_info, 0)
.unwrap(),
ValidTransactionBuilder::default()
.and_provides((alice.address, U256::from(1)))
.priority(0u64)
Expand All @@ -105,9 +117,15 @@ fn transaction_with_to_low_nonce_should_not_work() {
transaction: signed2,
};
let source2 = call2.check_self_contained().unwrap().unwrap();
let extrinsic2 = fp_self_contained::CheckedExtrinsic::<u64, crate::mock::Call, SignedExtra, _> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call2.clone()),
};

assert_err!(
call2.validate_self_contained(&source2).unwrap(),
call2
.validate_self_contained(&source2, &extrinsic2.get_dispatch_info(), 0)
.unwrap(),
InvalidTransaction::Stale
);
});
Expand Down Expand Up @@ -334,7 +352,7 @@ fn self_contained_transaction_with_extra_gas_should_adjust_weight_with_post_disp
transaction: signed,
};
let source = call.check_self_contained().unwrap().unwrap();
let extrinsic = CheckedExtrinsic::<_, _, frame_system::CheckWeight<Test>, _> {
let extrinsic = fp_self_contained::CheckedExtrinsic::<_, _, frame_system::CheckWeight<Test>, _> {
signed: fp_self_contained::CheckedSignature::SelfContained(source),
function: Call::Ethereum(call),
};
Expand Down
2 changes: 1 addition & 1 deletion frame/ethereum/src/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use frame_support::{assert_err, assert_ok, unsigned::TransactionValidityError};
use frame_support::{assert_err, assert_ok, unsigned::TransactionValidityError, weights::GetDispatchInfo};
use rustc_hex::{FromHex, ToHex};
use sp_runtime::{
traits::Applyable,
Expand Down
13 changes: 7 additions & 6 deletions primitives/self-contained/src/checked_extrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,12 @@ where
let unsigned_validation = U::validate_unsigned(source, &self.function)?;
Ok(valid.combine_with(unsigned_validation))
}
CheckedSignature::SelfContained(signed_info) => {
self.function.validate_self_contained(&signed_info).ok_or(
TransactionValidityError::Invalid(InvalidTransaction::BadProof),
)?
}
CheckedSignature::SelfContained(signed_info) => self
.function
.validate_self_contained(signed_info, info, len)
.ok_or(TransactionValidityError::Invalid(
InvalidTransaction::BadProof,
))?
}
}

Expand Down Expand Up @@ -139,7 +140,7 @@ where
CheckedSignature::SelfContained(signed_info) => {
// If pre-dispatch fail, the block must be considered invalid
self.function
.pre_dispatch_self_contained(&signed_info)
.pre_dispatch_self_contained(&signed_info, info, len)
.ok_or(TransactionValidityError::Invalid(
InvalidTransaction::BadProof,
))??;
Expand Down
16 changes: 13 additions & 3 deletions primitives/self-contained/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub use crate::{
};

use sp_runtime::{
traits::{Dispatchable, PostDispatchInfoOf},
traits::{DispatchInfoOf, Dispatchable, PostDispatchInfoOf},
transaction_validity::{TransactionValidity, TransactionValidityError},
};

Expand All @@ -43,7 +43,12 @@ pub trait SelfContainedCall: Dispatchable {
fn check_self_contained(&self) -> Option<Result<Self::SignedInfo, TransactionValidityError>>;
/// Validate a self-contained function. Returns `None` if the
/// function is not a self-contained.
fn validate_self_contained(&self, info: &Self::SignedInfo) -> Option<TransactionValidity>;
fn validate_self_contained(
&self,
info: &Self::SignedInfo,
dispatch_info: &DispatchInfoOf<Self>,
len: usize,
) -> Option<TransactionValidity>;
/// Do any pre-flight stuff for a self-contained call.
///
/// Note this function by default delegates to `validate_self_contained`, so that
Expand All @@ -57,7 +62,12 @@ pub trait SelfContainedCall: Dispatchable {
fn pre_dispatch_self_contained(
&self,
info: &Self::SignedInfo,
) -> Option<Result<(), TransactionValidityError>>;
dispatch_info: &DispatchInfoOf<Self>,
len: usize,
) -> Option<Result<(), TransactionValidityError>> {
self.validate_self_contained(info, dispatch_info, len)
.map(|res| res.map(|_| ()))
}
/// Apply a self-contained function. Returns `None` if the
/// function is not a self-contained.
fn apply_self_contained(
Expand Down
Loading

0 comments on commit cfef5ca

Please sign in to comment.