diff --git a/prdoc/pr_10476.prdoc b/prdoc/pr_10476.prdoc new file mode 100644 index 0000000000000..12397a3dda7e2 --- /dev/null +++ b/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 != ::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 diff --git a/substrate/frame/revive/rpc/src/cli.rs b/substrate/frame/revive/rpc/src/cli.rs index 4607b6f14e1e2..aed59c6acd660 100644 --- a/substrate/frame/revive/rpc/src/cli.rs +++ b/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; @@ -222,7 +229,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, )?; @@ -250,7 +257,11 @@ pub fn run(cmd: CliCommand) -> anyhow::Result<()> { } /// Create the JSON-RPC module. -fn rpc_module(is_dev: bool, client: Client) -> Result, sc_service::Error> { +fn rpc_module( + is_dev: bool, + client: Client, + allow_unprotected_txs: bool, +) -> Result, sc_service::Error> { let eth_api = EthRpcServerImpl::new(client.clone()) .with_accounts(if is_dev { vec![ @@ -263,6 +274,7 @@ fn rpc_module(is_dev: bool, client: Client) -> Result, sc_service: } else { vec![] }) + .with_allow_unprotected_txs(allow_unprotected_txs) .into_rpc(); let health_api = SystemHealthRpcServerImpl::new(client.clone()).into_rpc(); diff --git a/substrate/frame/revive/rpc/src/lib.rs b/substrate/frame/revive/rpc/src/lib.rs index b7f2dc3f2bec2..d6e86783ce23b 100644 --- a/substrate/frame/revive/rpc/src/lib.rs +++ b/substrate/frame/revive/rpc/src/lib.rs @@ -59,12 +59,15 @@ pub struct EthRpcServerImpl { /// The accounts managed by the server. accounts: Vec, + + /// 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. @@ -72,6 +75,12 @@ impl EthRpcServerImpl { 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 { 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. diff --git a/substrate/frame/revive/src/evm/call.rs b/substrate/frame/revive/src/evm/call.rs index 6bdb294b579b1..b18e1c3e8eb43 100644 --- a/substrate/frame/revive/src/evm/call.rs +++ b/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 = >::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 != ::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 != ::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, diff --git a/substrate/frame/revive/src/evm/runtime.rs b/substrate/frame/revive/src/evm/runtime.rs index 4da3b4c99d4cd..a5a962588eb87 100644 --- a/substrate/frame/revive/src/evm/runtime.rs +++ b/substrate/frame/revive/src/evm/runtime.rs @@ -138,7 +138,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) @@ -709,4 +709,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::::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" + ); + } }