Add extrinsic weight and lenght check (polkadot-evm#637)

* Try fix * Format code * Add tests * Update evm BlockGasLimit * Format * Fix ts tests * Add weight check to ethereum * Format * Handle review issue and fix unit tests * Fix clippy * Make CI Happy
web3labs · May 12, 2022 · adcd7e4 · adcd7e4
1 parent 3931e60
commit adcd7e4
Show file tree

Hide file tree

Showing 18 changed files with 199 additions and 53 deletions.
diff --git a/client/rpc/src/eth/cache/mod.rs b/client/rpc/src/eth/cache/mod.rs
@@ -467,7 +467,7 @@ where
 			let base_fee = if let Some(base_fee) = handler.base_fee(&id) {
 				base_fee
 			} else {
-				client.runtime_api().gas_price(&id).unwrap_or(U256::zero())
+				client.runtime_api().gas_price(&id).unwrap_or_else(|_|U256::zero())
 			};
 			let receipts = handler.current_receipts(&id);
 			let mut result = FeeHistoryCacheItem {

diff --git a/frame/base-fee/src/lib.rs b/frame/base-fee/src/lib.rs
@@ -178,7 +178,7 @@ pub mod pallet {
 							// Normalize to GWEI.
 							let increase = scaled_basefee
 								.checked_div(U256::from(1_000_000))
-								.unwrap_or(U256::zero());
+								.unwrap_or_else(U256::zero);
 							*bf = bf.saturating_add(increase);
 						} else {
 							Self::deposit_event(Event::BaseFeeOverflow);
@@ -196,7 +196,7 @@ pub mod pallet {
 							// Normalize to GWEI.
 							let decrease = scaled_basefee
 								.checked_div(U256::from(1_000_000))
-								.unwrap_or(U256::zero());
+								.unwrap_or_else(U256::zero);
 							*bf = bf.saturating_sub(decrease);
 						} else {
 							Self::deposit_event(Event::BaseFeeOverflow);

diff --git a/frame/ethereum/src/lib.rs b/frame/ethereum/src/lib.rs
@@ -41,14 +41,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,
 	},
@@ -108,9 +108,11 @@ impl<O: Into<Result<RawOrigin, O>> + From<RawOrigin>> EnsureOrigin<O>
 	}
 }
 
-impl<T: Config> Call<T>
+impl<T> 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 {
 		matches!(self, Call::transact { .. })
@@ -146,8 +148,17 @@ where
 		}
 	}
 
-	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,

diff --git a/frame/ethereum/src/mock.rs b/frame/ethereum/src/mock.rs
@@ -192,9 +192,14 @@ 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,
 		}
 	}

diff --git a/frame/ethereum/src/tests/eip1559.rs b/frame/ethereum/src/tests/eip1559.rs
@@ -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 = 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
 		);
 	});
@@ -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 = 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)
@@ -97,17 +108,22 @@ fn transaction_with_to_low_nonce_should_not_work() {
 
 		// nonce is 1
 		assert_ok!(Ethereum::execute(alice.address, &t, None,));
-
 		transaction.nonce = U256::from(0);
 
 		let signed2 = transaction.sign(&alice.private_key, None);
 		let call2 = crate::Call::<Test>::transact {
 			transaction: signed2,
 		};
 		let source2 = call2.check_self_contained().unwrap().unwrap();
+		let extrinsic2 = 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
 		);
 	});
@@ -127,11 +143,10 @@ fn transaction_with_to_hight_nonce_should_fail_in_block() {
 			transaction: signed,
 		};
 		let source = call.check_self_contained().unwrap().unwrap();
-		let extrinsic = fp_self_contained::CheckedExtrinsic::<_, _, SignedExtra, _> {
+		let extrinsic = CheckedExtrinsic::<_, _, SignedExtra, _> {
 			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),
@@ -151,11 +166,10 @@ fn transaction_with_invalid_chain_id_should_fail_in_block() {
 
 		let call = crate::Call::<Test>::transact { transaction };
 		let source = call.check_self_contained().unwrap().unwrap();
-		let extrinsic = fp_self_contained::CheckedExtrinsic::<_, _, SignedExtra, _> {
+		let extrinsic = CheckedExtrinsic::<_, _, SignedExtra, _> {
 			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),

diff --git a/frame/ethereum/src/tests/eip2930.rs b/frame/ethereum/src/tests/eip2930.rs
@@ -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 = 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
 		);
 	});
@@ -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 = 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)
@@ -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 = 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
 		);
 	});

diff --git a/frame/ethereum/src/tests/legacy.rs b/frame/ethereum/src/tests/legacy.rs
@@ -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 = 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
 		);
 	});
@@ -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 = 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)
@@ -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 = 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
 		);
 	});

diff --git a/frame/ethereum/src/tests/mod.rs b/frame/ethereum/src/tests/mod.rs
@@ -15,7 +15,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-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,
@@ -26,6 +28,7 @@ use std::str::FromStr;
 use crate::{
 	mock::*, CallOrCreateInfo, RawOrigin, Transaction, TransactionAction, H160, H256, U256,
 };
+use fp_self_contained::CheckedExtrinsic;
 
 mod eip1559;
 mod eip2930;

diff --git a/primitives/self-contained/src/checked_extrinsic.rs b/primitives/self-contained/src/checked_extrinsic.rs
@@ -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,
+				))?,
 		}
 	}
 

diff --git a/primitives/self-contained/src/lib.rs b/primitives/self-contained/src/lib.rs
@@ -26,7 +26,7 @@ pub use crate::{
 };
 
 use sp_runtime::{
-	traits::{Dispatchable, PostDispatchInfoOf},
+	traits::{DispatchInfoOf, Dispatchable, PostDispatchInfoOf},
 	transaction_validity::{TransactionValidity, TransactionValidityError},
 };
 
@@ -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

diff --git a/template/examples/contract-erc20/create-erc20-rpc.ts b/template/examples/contract-erc20/create-erc20-rpc.ts
@@ -1,7 +1,7 @@
 import Web3 from "web3";
 import * as web3Utils from 'web3-utils';
 
-const web3 = new Web3("http://localhost:9933");
+const web3 = new Web3("http://127.0.0.1:9933");
 const ERC20_BYTECODE = require("./truffle/contracts/MyToken.json").bytecode;
 const STORAGE_SLOT = "0";
 

diff --git a/template/examples/contract-erc20/create-erc20.ts b/template/examples/contract-erc20/create-erc20.ts
@@ -4,8 +4,8 @@ import { U8aFixed } from '@polkadot/types/codec';
 import * as web3Utils from 'web3-utils';
 import * as crypto from '@polkadot/util-crypto';
 
-// Provider is set to localhost for development
-const wsProvider = new WsProvider("ws://localhost:9944");
+// Provider is set to 127.0.0.1 for development
+const wsProvider = new WsProvider("ws://127.0.0.1:9944");
 
 // Keyring needed to sign using Alice account
 const keyring = new Keyring({ type: 'sr25519' });

diff --git a/template/examples/contract-hello/index.js b/template/examples/contract-hello/index.js
@@ -1,5 +1,5 @@
 const Web3 = require('web3');
-const web3 = new Web3('http://localhost:8545');
+const web3 = new Web3('http://127.0.0.1:8545');
 
 const ABI = [{"inputs":[],"payable":false,"stateMutability":"nonpayable","type":"constructor"},{"anonymous":false,"inputs":[{"indexed":false,"internalType":"string","name":"message","type":"string"}],"name":"Said","type":"event"},{"constant":true,"inputs":[],"name":"owner","outputs":[{"internalType":"address","name":"","type":"address"}],"payable":false,"stateMutability":"view","type":"function"}];
 const BYTECODE = "0x608060405234801561001057600080fd5b50336000806101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff1602179055507fc68045c3c562488255b55aa2c4c7849de001859ff0d8a36a75c2d5ed80100fb660405180806020018281038252600d8152602001807f48656c6c6f2c20776f726c64210000000000000000000000000000000000000081525060200191505060405180910390a160cf806100c76000396000f3fe6080604052348015600f57600080fd5b506004361060285760003560e01c80638da5cb5b14602d575b600080fd5b60336075565b604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390f35b6000809054906101000a900473ffffffffffffffffffffffffffffffffffffffff168156fea265627a7a72315820fae816ad954005c42bea7bc7cb5b19f7fd5d3a250715ca2023275c9ca7ce644064736f6c634300050f0032";