[pallet-revive] eth-prc fix geth diff (#6608)

* Add a bunch of differential tests to ensure that responses from eth-rpc matches the one from `geth` - These [tests](https://github.com/paritytech/polkadot-sdk/blob/pg/fix-geth-diff/substrate/frame/revive/rpc/examples/js/src/geth-diff.test.ts) are not run in CI for now but can be run locally with ```bash cd revive/rpc/examples/js bun test ``` * EVM RPC server will not fail gas_estimation if no gas is specified, I updated pallet-revive to add an extra `skip_transfer` boolean check to replicate this behavior in our pallet * `eth_transact` and `bare_eth_transact` api have been updated to use `GenericTransaction` directly as this is what is used by `eth_estimateGas` and `eth_call` ## TODO - [ ] Add tests the new `skip_transfer` flag --------- Co-authored-by: GitHub Action <[email protected]> Co-authored-by: Alexander Theißen <[email protected]>
paritytech · Dec 1, 2024 · d1fafa8 · d1fafa8
1 parent 5e0bcb0
commit d1fafa8
Show file tree

Hide file tree

Showing 55 changed files with 1,553 additions and 1,248 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs
@@ -124,7 +124,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: alloc::borrow::Cow::Borrowed("westmint"),
 	impl_name: alloc::borrow::Cow::Borrowed("westmint"),
 	authoring_version: 1,
-	spec_version: 1_016_006,
+	spec_version: 1_016_008,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 16,
@@ -2081,18 +2081,10 @@ impl_runtime_apis! {
 			let account = <Runtime as pallet_revive::Config>::AddressMapper::to_account_id(&address);
 			System::account_nonce(account)
 		}
-		fn eth_transact(
-			from: H160,
-			dest: Option<H160>,
-			value: U256,
-			input: Vec<u8>,
-			gas_limit: Option<Weight>,
-			storage_deposit_limit: Option<Balance>,
-		) -> pallet_revive::EthContractResult<Balance>
+
+		fn eth_transact(tx: pallet_revive::evm::GenericTransaction) -> Result<pallet_revive::EthTransactInfo<Balance>, pallet_revive::EthTransactError>
 		{
-			use pallet_revive::AddressMapper;
-			let blockweights = <Runtime as frame_system::Config>::BlockWeights::get();
-			let origin = <Runtime as pallet_revive::Config>::AddressMapper::to_account_id(&from);
+			let blockweights: BlockWeights = <Runtime as frame_system::Config>::BlockWeights::get();
 
 			let encoded_size = |pallet_call| {
 				let call = RuntimeCall::Revive(pallet_call);
@@ -2101,15 +2093,9 @@ impl_runtime_apis! {
 			};
 
 			Revive::bare_eth_transact(
-				origin,
-				dest,
-				value,
-				input,
-				gas_limit.unwrap_or(blockweights.max_block),
-				storage_deposit_limit.unwrap_or(u128::MAX),
+				tx,
+				blockweights.max_block,
 				encoded_size,
-				pallet_revive::DebugInfo::UnsafeDebug,
-				pallet_revive::CollectEvents::UnsafeCollect,
 			)
 		}
 
@@ -2127,7 +2113,7 @@ impl_runtime_apis! {
 				dest,
 				value,
 				gas_limit.unwrap_or(blockweights.max_block),
-				storage_deposit_limit.unwrap_or(u128::MAX),
+				pallet_revive::DepositLimit::Balance(storage_deposit_limit.unwrap_or(u128::MAX)),
 				input_data,
 				pallet_revive::DebugInfo::UnsafeDebug,
 				pallet_revive::CollectEvents::UnsafeCollect,
@@ -2149,7 +2135,7 @@ impl_runtime_apis! {
 				RuntimeOrigin::signed(origin),
 				value,
 				gas_limit.unwrap_or(blockweights.max_block),
-				storage_deposit_limit.unwrap_or(u128::MAX),
+				pallet_revive::DepositLimit::Balance(storage_deposit_limit.unwrap_or(u128::MAX)),
 				code,
 				data,
 				salt,

diff --git a/prdoc/pr_6608.prdoc b/prdoc/pr_6608.prdoc
@@ -0,0 +1,14 @@
+title: '[pallet-revive] eth-prc fix geth diff'
+doc:
+- audience: Runtime Dev
+  description: |-
+    * Add a bunch of differential tests to ensure that responses from eth-rpc matches the one from `geth`
+    * EVM RPC server will not fail gas_estimation if no gas is specified, I updated pallet-revive to add an extra `skip_transfer` boolean check to replicate this behavior in our pallet
+    * `eth_transact` and `bare_eth_transact` api have been updated to use `GenericTransaction` directly as this is what is used by `eth_estimateGas` and `eth_call`
+crates:
+- name: pallet-revive-eth-rpc
+  bump: minor
+- name: pallet-revive
+  bump: minor
+- name: asset-hub-westend-runtime
+  bump: minor
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
@@ -3218,18 +3218,9 @@ impl_runtime_apis! {
 			System::account_nonce(account)
 		}
 
-		fn eth_transact(
-			from: H160,
-			dest: Option<H160>,
-			value: U256,
-			input: Vec<u8>,
-			gas_limit: Option<Weight>,
-			storage_deposit_limit: Option<Balance>,
-		) -> pallet_revive::EthContractResult<Balance>
+		fn eth_transact(tx: pallet_revive::evm::GenericTransaction) -> Result<pallet_revive::EthTransactInfo<Balance>, pallet_revive::EthTransactError>
 		{
-			use pallet_revive::AddressMapper;
 			let blockweights: BlockWeights = <Runtime as frame_system::Config>::BlockWeights::get();
-			let origin = <Runtime as pallet_revive::Config>::AddressMapper::to_account_id(&from);
 
 			let encoded_size = |pallet_call| {
 				let call = RuntimeCall::Revive(pallet_call);
@@ -3238,15 +3229,9 @@ impl_runtime_apis! {
 			};
 
 			Revive::bare_eth_transact(
-				origin,
-				dest,
-				value,
-				input,
-				gas_limit.unwrap_or(blockweights.max_block),
-				storage_deposit_limit.unwrap_or(u128::MAX),
+				tx,
+				blockweights.max_block,
 				encoded_size,
-				pallet_revive::DebugInfo::UnsafeDebug,
-				pallet_revive::CollectEvents::UnsafeCollect,
 			)
 		}
 
@@ -3263,7 +3248,7 @@ impl_runtime_apis! {
 				dest,
 				value,
 				gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block),
-				storage_deposit_limit.unwrap_or(u128::MAX),
+				pallet_revive::DepositLimit::Balance(storage_deposit_limit.unwrap_or(u128::MAX)),
 				input_data,
 				pallet_revive::DebugInfo::UnsafeDebug,
 				pallet_revive::CollectEvents::UnsafeCollect,
@@ -3284,7 +3269,7 @@ impl_runtime_apis! {
 				RuntimeOrigin::signed(origin),
 				value,
 				gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block),
-				storage_deposit_limit.unwrap_or(u128::MAX),
+				pallet_revive::DepositLimit::Balance(storage_deposit_limit.unwrap_or(u128::MAX)),
 				code,
 				data,
 				salt,

diff --git a/substrate/frame/revive/Cargo.toml b/substrate/frame/revive/Cargo.toml
@@ -65,6 +65,7 @@ pallet-revive-fixtures = { workspace = true, default-features = true }
 secp256k1 = { workspace = true, features = ["recovery"] }
 serde_json = { workspace = true }
 hex-literal = { workspace = true }
+env_logger = { workspace = true }
 
 # Polkadot SDK Dependencies
 pallet-balances = { workspace = true, default-features = true }

diff --git a/substrate/frame/revive/mock-network/src/tests.rs b/substrate/frame/revive/mock-network/src/tests.rs
@@ -24,7 +24,7 @@ use frame_support::traits::{fungibles::Mutate, Currency};
 use frame_system::RawOrigin;
 use pallet_revive::{
 	test_utils::{self, builder::*},
-	Code,
+	Code, DepositLimit,
 };
 use pallet_revive_fixtures::compile_module;
 use pallet_revive_uapi::ReturnErrorCode;
@@ -52,7 +52,7 @@ fn instantiate_test_contract(name: &str) -> Contract<parachain::Runtime> {
 			RawOrigin::Signed(ALICE).into(),
 			Code::Upload(wasm),
 		)
-		.storage_deposit_limit(1_000_000_000_000)
+		.storage_deposit_limit(DepositLimit::Balance(1_000_000_000_000))
 		.build_and_unwrap_contract()
 	});
 

diff --git a/substrate/frame/revive/rpc/Cargo.toml b/substrate/frame/revive/rpc/Cargo.toml
@@ -67,13 +67,13 @@ hex = { workspace = true }
 hex-literal = { workspace = true, optional = true }
 scale-info = { workspace = true }
 secp256k1 = { workspace = true, optional = true, features = ["recovery"] }
-env_logger = { workspace = true }
 ethabi = { version = "18.0.0" }
 
 [features]
 example = ["hex-literal", "rlp", "secp256k1", "subxt-signer"]
 
 [dev-dependencies]
+env_logger = { workspace = true }
 static_init = { workspace = true }
 hex-literal = { workspace = true }
 pallet-revive-fixtures = { workspace = true }

diff --git a/substrate/frame/revive/rpc/examples/js/abi/errorTester.ts b/substrate/frame/revive/rpc/examples/js/abi/errorTester.ts
@@ -0,0 +1,106 @@
+export const abi = [
+	{
+		inputs: [
+			{
+				internalType: 'string',
+				name: 'message',
+				type: 'string',
+			},
+		],
+		name: 'CustomError',
+		type: 'error',
+	},
+	{
+		inputs: [
+			{
+				internalType: 'bool',
+				name: 'newState',
+				type: 'bool',
+			},
+		],
+		name: 'setState',
+		outputs: [],
+		stateMutability: 'nonpayable',
+		type: 'function',
+	},
+	{
+		inputs: [],
+		name: 'state',
+		outputs: [
+			{
+				internalType: 'bool',
+				name: '',
+				type: 'bool',
+			},
+		],
+		stateMutability: 'view',
+		type: 'function',
+	},
+	{
+		inputs: [],
+		name: 'triggerAssertError',
+		outputs: [],
+		stateMutability: 'pure',
+		type: 'function',
+	},
+	{
+		inputs: [],
+		name: 'triggerCustomError',
+		outputs: [],
+		stateMutability: 'pure',
+		type: 'function',
+	},
+	{
+		inputs: [],
+		name: 'triggerDivisionByZero',
+		outputs: [
+			{
+				internalType: 'uint256',
+				name: '',
+				type: 'uint256',
+			},
+		],
+		stateMutability: 'pure',
+		type: 'function',
+	},
+	{
+		inputs: [],
+		name: 'triggerOutOfBoundsError',
+		outputs: [
+			{
+				internalType: 'uint256',
+				name: '',
+				type: 'uint256',
+			},
+		],
+		stateMutability: 'pure',
+		type: 'function',
+	},
+	{
+		inputs: [],
+		name: 'triggerRequireError',
+		outputs: [],
+		stateMutability: 'pure',
+		type: 'function',
+	},
+	{
+		inputs: [],
+		name: 'triggerRevertError',
+		outputs: [],
+		stateMutability: 'pure',
+		type: 'function',
+	},
+	{
+		inputs: [
+			{
+				internalType: 'uint256',
+				name: 'value',
+				type: 'uint256',
+			},
+		],
+		name: 'valueMatch',
+		outputs: [],
+		stateMutability: 'payable',
+		type: 'function',
+	},
+] as const
diff --git a/substrate/frame/revive/rpc/examples/js/abi/event.json b/substrate/frame/revive/rpc/examples/js/abi/event.json
diff --git a/substrate/frame/revive/rpc/examples/js/abi/event.ts b/substrate/frame/revive/rpc/examples/js/abi/event.ts
@@ -0,0 +1,34 @@
+export const abi = [
+	{
+		anonymous: false,
+		inputs: [
+			{
+				indexed: true,
+				internalType: 'address',
+				name: 'sender',
+				type: 'address',
+			},
+			{
+				indexed: false,
+				internalType: 'uint256',
+				name: 'value',
+				type: 'uint256',
+			},
+			{
+				indexed: false,
+				internalType: 'string',
+				name: 'message',
+				type: 'string',
+			},
+		],
+		name: 'ExampleEvent',
+		type: 'event',
+	},
+	{
+		inputs: [],
+		name: 'triggerEvent',
+		outputs: [],
+		stateMutability: 'nonpayable',
+		type: 'function',
+	},
+] as const