Allow for "Nick's Method" Style Transactions & Contract Deployments in Revive

.github/workflows/tests-evm.yml

@@ -51,7 +51,7 @@ jobs:
         uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
         with:
           repository: paritytech/revive-differential-tests
-          ref: a6e4932a08b1ca231e4a02ca6e54e08a53f0e786
+          ref: 66feb36b4ef2c79415ca8ea765d8235d48dfa8f8
           path: revive-differential-tests
           submodules: recursive
       - name: Installing Retester
@@ -60,7 +60,7 @@ jobs:
         run: mkdir workdir
       - name: Downloading & Initializing the compilation caches
         run: |
-          curl -fL --retry 3 --retry-all-errors --connect-timeout 10 -o cache.tar.gz "https://github.com/paritytech/revive-differential-tests/releases/download/compilation-caches-v1.0/cache.tar.gz"
+          curl -fL --retry 3 --retry-all-errors --connect-timeout 10 -o cache.tar.gz "https://github.com/paritytech/revive-differential-tests/releases/download/compilation-caches-v1.1/cache.tar.gz"
           tar -zxf cache.tar.gz -C ./workdir > /dev/null 2>&1
       - name: Running the Differential Tests
         run: |
@@ -111,8 +111,7 @@ jobs:
       RUST_BACKTRACE: 1
     strategy:
       matrix:
-        platform:
-          ["test:pvm", "test:evm"]
+        platform: ["test:pvm", "test:evm"]
     steps:
       - name: Checkout
         uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0

prdoc/pr_10397.prdoc

@@ -0,0 +1,8 @@
+title: Update the commit hash of the revive-differential-tests
+doc:
+- audience: Runtime Dev
+  description: |-
+    # Description
+
+    This is a PR that updates the commit hash of the revive-differential-tests framework and the compilation caches to a version that includes fixes to certain tests that used hard-coded gas values. The compilation caches required an update since this was a change to the contract's code.
+crates: []

prdoc/pr_10476.prdoc

@@ -0,0 +1,56 @@
+title: Allow for "Nick's Method" style deployments
+doc:
+- audience: Runtime Dev
+  description: "# Description\n\nThis pull request allows for \u201C[Nick\u2019s Method](https://weka.medium.com/how-to-send-ether-to-11-440-people-187e332566b7)\u201D\
+    \ style of deploying contracts which was requested in https://github.com/paritytech/contract-issues/issues/225\
+    \ and was attempted to be solved in https://github.com/paritytech/contract-issues/issues/99.\n\
+    \nThe \u201CNick Method\u201D style of contract deployment is a very useful concept\
+    \ to use when we wish to deploy a contract on multiple chains **with the same\
+    \ address**. It allows us to do that by constructing a deployment transaction\
+    \ that can be executed on any chain, thus allowing us to get the same address\
+    \ for the contract\u2019s deployment on any chain.\n\nAdditionally, this method\
+    \ allows for the contracts to be deployed trustlessly, meaning that if any actor\
+    \ (regardless of whether they\u2019re honest or not) follow the same method for\
+    \ deploying the contract then they\u2019d get the same address across all chains.\
+    \ This allows anyone in the Polkadot ecosystem to take existing contracts that\
+    \ use this method of deployment (e.g., `Multicall3` and the ERC-1820 registry)\
+    \ and deploy them on Polkadot and there\u2019s already trust that the code is\
+    \ the same.\n\nIn order to be able to use the same transaction across multiple\
+    \ chains transaction authors need to:\n\n- Not include a chain id.\n- Include\
+    \ a high gas limit so that the transaction works on (almost) all chains.\n\nAs\
+    \ mentioned in https://github.com/paritytech/contract-issues/issues/225, this\
+    \ pattern is already being used in a number of contracts. Most notably, `Multicall3`\
+    \ and the ERC-1820 Registry.\n\nBefore this PR this method didn\u2019t work in\
+    \ revive since the chain id was an absolute must and any transaction that didn\u2019\
+    t include a chain id would fail with an `Invalid Transaction` error as seen below:\n\
+    \nhttps://github.com/paritytech/polkadot-sdk/blob/c688963f51c55b3c2a16a00a33c4a086792a1544/substrate/frame/revive/src/evm/call.rs#L71-L76\n\
+    \nThe above implementation misses an important detail: legacy transactions are\
+    \ permitted to not have the chain id set while all other non-legacy transactions\
+    \ do not permit that. Therefore, this part of the code was changed to the following:\n\
+    \n```rust\nmatch (tx.chain_id, tx.r#type.as_ref()) {\n\t(None, Some(super::Byte(TYPE_LEGACY)))\
+    \ => {},\n\t(Some(chain_id), ..) =>\n\t\tif chain_id != <T as Config>::ChainId::get().into()\
+    \ {\n\t\t\tlog::debug!(target: LOG_TARGET, \"Invalid chain_id {chain_id:?}\");\n\
+    \t\t\treturn Err(InvalidTransaction::Call);\n\t\t},\n\t(None, ..) => {\n\t\tlog::debug!(target:\
+    \ LOG_TARGET, \"Invalid chain_id None\");\n\t\treturn Err(InvalidTransaction::Call);\n\
+    \t},\n}\n```\n\nThe above code skips the chain id check if the transaction is\
+    \ of the legacy type. Otherwise, the chain id is checked. If no chain id is provided\
+    \ and the transaction is not of the legacy type then we error out.\n\n## Integration\n\
+    \nBe aware that we now allow for legacy transactions to not have the chain ID\
+    \ set in order to allow for \u201C[Nick\u2019s Method](https://weka.medium.com/how-to-send-ether-to-11-440-people-187e332566b7)\u201D\
+    \ style of contract deployment. Non-legacy transaction continue to require the\
+    \ chain id to be provided.\n\n## Review Notes\n\n- The main change that this PR\
+    \ makes can be found in the `substrate/frame/revive/src/evm/call.rs` file allowing\
+    \ for legacy contracts to not have the chain id set.\n- Two new tests were added\
+    \ with this PR:\n  - `dry_run_contract_deployment_with_nick_method_works`\n  -\
+    \ `contract_deployment_with_nick_method_works`\n- Both of the above tests test\
+    \ that \u201C[Nick\u2019s Method](https://weka.medium.com/how-to-send-ether-to-11-440-people-187e332566b7)\u201D\
+    \ can be used to deploy the singleton factory contract provided in [ERC-2470](https://eips.ethereum.org/EIPS/eip-2470)\
+    \ in a dry run and in an `eth_transact` call.\n- Note that the above tests needed\
+    \ to modify the transaction provided in the ERC to update its gas limit (since\
+    \ the provided gas limit was too low for our platform), **which breaks the whole\
+    \ idea of Nick\u2019s Method where the same transaction can be submitted to the\
+    \ same chain**. I suspect that https://github.com/paritytech/polkadot-sdk/pull/10393\
+    \ should fix this issue with the new gas scaling."
+crates:
+- name: pallet-revive
+  bump: patch

substrate/client/cli/src/params/rpc_params.rs

@@ -188,6 +188,12 @@ pub struct RpcParams {
 	/// `--dev` mode the default is to allow all origins.
 	#[arg(long, value_name = "ORIGINS")]
 	pub rpc_cors: Option<Cors>,
+
+	/// By default, the node rejects any transaction that's unprotected (i.e., that doesn't have a
+	/// chain-id). If the user wishes the submit such a transaction then they can use this flag to
+	/// instruct the RPC to ignore this check.
+	#[arg(long)]
+	pub allow_unprotected_txs: bool,
 }
 
 impl RpcParams {

substrate/frame/revive/rpc/src/cli.rs

@@ -222,7 +222,7 @@ pub fn run(cmd: CliCommand) -> anyhow::Result<()> {
 		&rpc_config,
 		prometheus_registry,
 		tokio_handle,
-		|| rpc_module(is_dev, client.clone()),
+		|| rpc_module(is_dev, client.clone(), rpc_params.allow_unprotected_txs),
 		None,
 	)?;
 
@@ -250,7 +250,11 @@ pub fn run(cmd: CliCommand) -> anyhow::Result<()> {
 }
 
 /// Create the JSON-RPC module.
-fn rpc_module(is_dev: bool, client: Client) -> Result<RpcModule<()>, sc_service::Error> {
+fn rpc_module(
+	is_dev: bool,
+	client: Client,
+	allow_unprotected_txs: bool,
+) -> Result<RpcModule<()>, sc_service::Error> {
 	let eth_api = EthRpcServerImpl::new(client.clone())
 		.with_accounts(if is_dev {
 			vec![
@@ -263,6 +267,7 @@ fn rpc_module(is_dev: bool, client: Client) -> Result<RpcModule<()>, sc_service:
 		} else {
 			vec![]
 		})
+		.with_allow_unprotected_txs(allow_unprotected_txs)
 		.into_rpc();
 
 	let health_api = SystemHealthRpcServerImpl::new(client.clone()).into_rpc();

substrate/frame/revive/rpc/src/lib.rs

@@ -59,19 +59,28 @@ pub struct EthRpcServerImpl {
 
 	/// The accounts managed by the server.
 	accounts: Vec<Account>,
+
+	/// Controls if unprotected txs are allowed or not.
+	allow_unprotected_txs: bool,
 }
 
 impl EthRpcServerImpl {
 	/// Creates a new [`EthRpcServerImpl`].
 	pub fn new(client: client::Client) -> Self {
-		Self { client, accounts: vec![] }
+		Self { client, accounts: vec![], allow_unprotected_txs: false }
 	}
 
 	/// Sets the accounts managed by the server.
 	pub fn with_accounts(mut self, accounts: Vec<Account>) -> Self {
 		self.accounts = accounts;
 		self
 	}
+
+	/// Sets whether unprotected transactions are allowed or not.
+	pub fn with_allow_unprotected_txs(mut self, allow_unprotected_txs: bool) -> Self {
+		self.allow_unprotected_txs = allow_unprotected_txs;
+		self
+	}
 }
 
 /// The error type for the EVM RPC server.
@@ -168,6 +177,32 @@ impl EthRpcServer for EthRpcServerImpl {
 	async fn send_raw_transaction(&self, transaction: Bytes) -> RpcResult<H256> {
 		let hash = H256(keccak_256(&transaction.0));
 		log::trace!(target: LOG_TARGET, "send_raw_transaction transaction: {transaction:?} ethereum_hash: {hash:?}");
+
+		if !self.allow_unprotected_txs {
+			let signed_transaction = TransactionSigned::decode(transaction.0.as_slice())
+				.inspect_err(|err| {
+					log::trace!(target: LOG_TARGET, "Transaction decoding failed. ethereum_hash: {hash:?}, error: {err:?}");
+				})?;
+
+			let is_chain_id_provided = match signed_transaction {
+				TransactionSigned::Transaction7702Signed(tx) =>
+					tx.transaction_7702_unsigned.chain_id != U256::zero(),
+				TransactionSigned::Transaction4844Signed(tx) =>
+					tx.transaction_4844_unsigned.chain_id != U256::zero(),
+				TransactionSigned::Transaction1559Signed(tx) =>
+					tx.transaction_1559_unsigned.chain_id != U256::zero(),
+				TransactionSigned::Transaction2930Signed(tx) =>
+					tx.transaction_2930_unsigned.chain_id != U256::zero(),
+				TransactionSigned::TransactionLegacySigned(tx) =>
+					tx.transaction_legacy_unsigned.chain_id.is_some(),
+			};
+
+			if !is_chain_id_provided {
+				log::trace!(target: LOG_TARGET, "Invalid Transaction: transaction doesn't include a chain-id. ethereum_hash: {hash:?}");
+				return Err(EthRpcError::InvalidTransaction)
+			}
+		}
+
 		let call = subxt_client::tx().revive().eth_transact(transaction.0);
 
 		// Subscribe to new block only when automine is enabled.

substrate/frame/revive/src/evm/call.rs

@@ -18,7 +18,7 @@
 //! Functionality to decode an eth transaction into an dispatchable call.
 
 use crate::{
-	evm::{fees::InfoT, runtime::SetWeightLimit},
+	evm::{fees::InfoT, runtime::SetWeightLimit, TYPE_LEGACY},
 	extract_code_and_data, BalanceOf, CallOf, Config, GenericTransaction, Pallet, Weight, Zero,
 	LOG_TARGET, RUNTIME_PALLETS_ADDR, U256,
 };
@@ -68,11 +68,25 @@ where
 		return Err(InvalidTransaction::Payment);
 	};
 
-	let chain_id = tx.chain_id.unwrap_or_default();
-
-	if chain_id != <T as Config>::ChainId::get().into() {
-		log::debug!(target: LOG_TARGET, "Invalid chain_id {chain_id:?}");
-		return Err(InvalidTransaction::Call);
+	// We would like to allow for transactions without a chain id to be executed through pallet
+	// revive. These are called unprotected transactions and they are transactions that predate
+	// EIP-155 which do not include a Chain ID. These transactions are still useful today in certain
+	// patterns in Ethereum such as "Nick's Method" for contract deployment which allows a contract
+	// to be deployed on all chains with the same address. This is only allowed for legacy
+	// transactions and isn't allowed for any other transaction type.
+	// * Here's a relevant EIP: https://eips.ethereum.org/EIPS/eip-2470
+	// * Here's Nick's article: https://weka.medium.com/how-to-send-ether-to-11-440-people-187e332566b7
+	match (tx.chain_id, tx.r#type.as_ref()) {
+		(None, Some(super::Byte(TYPE_LEGACY))) => {},
+		(Some(chain_id), ..) =>
+			if chain_id != <T as Config>::ChainId::get().into() {
+				log::debug!(target: LOG_TARGET, "Invalid chain_id {chain_id:?}");
+				return Err(InvalidTransaction::Call);
+			},
+		(None, ..) => {
+			log::debug!(target: LOG_TARGET, "Invalid chain_id None");
+			return Err(InvalidTransaction::Call);
+		},
 	}
 
 	if effective_gas_price < base_fee {

substrate/frame/revive/src/evm/runtime.rs

@@ -705,4 +705,92 @@ mod test {
 			_ => panic!("Expected the RuntimeCall::Contracts variant, got: {:?}", call),
 		}
 	}
+
+	#[test]
+	fn dry_run_contract_deployment_with_nick_method_works() {
+		// Arrange
+		let raw_transaction_bytes = alloy_core::hex!("0xf9016c8085174876e8008303c4d88080b90154608060405234801561001057600080fd5b50610134806100206000396000f3fe6080604052348015600f57600080fd5b506004361060285760003560e01c80634af63f0214602d575b600080fd5b60cf60048036036040811015604157600080fd5b810190602081018135640100000000811115605b57600080fd5b820183602082011115606c57600080fd5b80359060200191846001830284011164010000000083111715608d57600080fd5b91908080601f016020809104026020016040519081016040528093929190818152602001838380828437600092019190915250929550509135925060eb915050565b604080516001600160a01b039092168252519081900360200190f35b6000818351602085016000f5939250505056fea26469706673582212206b44f8a82cb6b156bfcc3dc6aadd6df4eefd204bc928a4397fd15dacf6d5320564736f6c634300060200331b83247000822470");
+
+		ExtBuilder::default().build().execute_with(|| {
+			let mut signed_transaction =
+				TransactionSigned::decode(raw_transaction_bytes.as_slice())
+					.expect("Invalid raw transaction bytes");
+			if let TransactionSigned::TransactionLegacySigned(ref mut legacy_transaction) =
+				signed_transaction
+			{
+				legacy_transaction.transaction_legacy_unsigned.gas =
+					U256::from_dec_str("3750815700000").unwrap();
+			}
+			let generic_transaction = GenericTransaction::from_signed(
+				signed_transaction.clone(),
+				Pallet::<Test>::evm_base_fee(),
+				None,
+			);
+
+			let deployer = signed_transaction
+				.recover_eth_address()
+				.expect("Failed to recover the deployer account");
+			Pallet::<Test>::set_evm_balance(
+				&deployer,
+				U256::from_dec_str("10000000000000000000").unwrap(),
+			)
+			.expect("Setting balance failed");
+
+			// Act
+			let dry_run_result = Pallet::<Test>::dry_run_eth_transact(
+				generic_transaction.clone(),
+				Default::default(),
+			);
+
+			// Assert
+			assert!(
+				generic_transaction.chain_id.is_none(),
+				"Chain Id in the generic transaction is not None"
+			);
+			let _dry_run_result = dry_run_result.expect("Dry run failed");
+		})
+	}
+
+	#[test]
+	fn contract_deployment_with_nick_method_works() {
+		// Arrange
+		let raw_transaction_bytes = alloy_core::hex!("0xf9016c8085174876e8008303c4d88080b90154608060405234801561001057600080fd5b50610134806100206000396000f3fe6080604052348015600f57600080fd5b506004361060285760003560e01c80634af63f0214602d575b600080fd5b60cf60048036036040811015604157600080fd5b810190602081018135640100000000811115605b57600080fd5b820183602082011115606c57600080fd5b80359060200191846001830284011164010000000083111715608d57600080fd5b91908080601f016020809104026020016040519081016040528093929190818152602001838380828437600092019190915250929550509135925060eb915050565b604080516001600160a01b039092168252519081900360200190f35b6000818351602085016000f5939250505056fea26469706673582212206b44f8a82cb6b156bfcc3dc6aadd6df4eefd204bc928a4397fd15dacf6d5320564736f6c634300060200331b83247000822470");
+
+		let mut signed_transaction = TransactionSigned::decode(raw_transaction_bytes.as_slice())
+			.expect("Invalid raw transaction bytes");
+		if let TransactionSigned::TransactionLegacySigned(ref mut legacy_transaction) =
+			signed_transaction
+		{
+			legacy_transaction.transaction_legacy_unsigned.gas =
+				U256::from_dec_str("3750815700000").unwrap();
+		}
+		let generic_transaction = GenericTransaction::from_signed(
+			signed_transaction.clone(),
+			ExtBuilder::default().build().execute_with(|| Pallet::<Test>::evm_base_fee()),
+			None,
+		);
+
+		let unchecked_extrinsic_builder = UncheckedExtrinsicBuilder {
+			tx: generic_transaction,
+			before_validate: None,
+			dry_run: None,
+		};
+
+		// Act
+		let eth_transact_result = unchecked_extrinsic_builder.check();
+
+		// Assert
+		let (
+			_encoded_len,
+			_function,
+			_extra,
+			generic_transaction,
+			_gas_required,
+			_signed_transaction,
+		) = eth_transact_result.expect("eth_transact failed");
+		assert!(
+			generic_transaction.chain_id.is_none(),
+			"Chain Id in the generic transaction is not None"
+		);
+	}
 }