[unstable2507] Backport #10476

prdoc/pr_10476.prdoc

@@ -0,0 +1,58 @@
+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
+- name: pallet-revive-eth-rpc
+  bump: major

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

@@ -76,6 +76,12 @@ pub struct CliCommand {
 	#[allow(missing_docs)]
 	#[clap(flatten)]
 	pub prometheus_params: PrometheusParams,
+
+	/// 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,
 }
 
 /// Initialize the logger
@@ -164,6 +170,7 @@ pub fn run(cmd: CliCommand) -> anyhow::Result<()> {
 		earliest_receipt_block,
 		index_last_n_blocks,
 		shared_params,
+		allow_unprotected_txs,
 		..
 	} = cmd;
 
@@ -221,7 +228,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(), allow_unprotected_txs),
 		None,
 	)?;
 
@@ -249,7 +256,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![
@@ -262,6 +273,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,33 @@ 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())
+				.map_err(|err| {
+					log::trace!(target: LOG_TARGET, "Transaction decoding failed. ethereum_hash: {hash:?}, error: {err:?}");
+					EthRpcError::InvalidTransaction
+				})?;
+
+			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:?}");
+				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

@@ -21,6 +21,7 @@ use crate::{
 	evm::{
 		fees::{compute_max_integer_quotient, InfoT},
 		runtime::SetWeightLimit,
+		TYPE_LEGACY,
 	},
 	extract_code_and_data, BalanceOf, CallOf, Config, GenericTransaction, Pallet, Weight, Zero,
 	LOG_TARGET, RUNTIME_PALLETS_ADDR,
@@ -69,6 +70,27 @@ impl GenericTransaction {
 		let is_dry_run = matches!(mode, CreateCallMode::DryRun);
 		let base_fee = <Pallet<T>>::evm_base_fee();
 
+		// 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 (self.chain_id, self.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);
+			},
+		}
+
 		let Some(gas) = self.gas else {
 			log::debug!(target: LOG_TARGET, "No gas provided");
 			return Err(InvalidTransaction::Call);
@@ -82,13 +104,6 @@ impl GenericTransaction {
 			return Err(InvalidTransaction::Payment);
 		};
 
-		let chain_id = self.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);
-		}
-
 		if effective_gas_price < base_fee {
 			log::debug!(
 				target: LOG_TARGET,

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

@@ -131,7 +131,7 @@ where
 		if !self.0.is_signed() {
 			if let Some(crate::Call::eth_transact { payload }) = self.0.function.is_sub_type() {
 				let checked = E::try_into_checked_extrinsic(payload, self.encoded_size())?;
-				return Ok(checked)
+				return Ok(checked);
 			};
 		}
 		self.0.check(lookup)
@@ -696,4 +696,56 @@ mod test {
 			_ => panic!("Expected the RuntimeCall::Contracts variant, got: {:?}", call),
 		}
 	}
+
+	/// The raw bytes seen in this test is of a deployment transaction from [eip-2470] which publish
+	/// a contract at a predicable address on any chain that it's run on. We use these bytes to test
+	/// that if we were to run this transaction on pallet-revive that it would run and also produce
+	/// a contract at the address described in the EIP.
+	///
+	/// Note: the linked EIP is not an EIP for Nick's method, it's just an EIP that makes use of
+	/// Nick's method.
+	///
+	/// [eip-2470]: https://eips.ethereum.org/EIPS/eip-2470
+	#[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"
+		);
+	}
 }