[pallet-revive] Support all eth tx types (paritytech#6461)

Add support for all eth tx types Note that js libs will continue to use the Legacy type since we don't include base_fee_per_gas yet in the block. We can think about setting these values after we revisit how we encode the gas into weight & deposit_limit in a follow up PR --------- Co-authored-by: Alexander Theißen <[email protected]> Co-authored-by: GitHub Action <[email protected]>
ndkazu · Nov 21, 2024 · d8ce550 · d8ce550
1 parent 6d59c3b
commit d8ce550
Show file tree

Hide file tree

Showing 13 changed files with 824 additions and 190 deletions.
diff --git a/prdoc/pr_6461.prdoc b/prdoc/pr_6461.prdoc
@@ -0,0 +1,12 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+title: '[pallet-revive] add support for all eth tx types'
+doc:
+- audience: Runtime Dev
+  description: Add support for 1559, 4844, and 2930 transaction types
+crates:
+- name: pallet-revive-eth-rpc
+  bump: minor
+- name: pallet-revive
+  bump: minor
+
diff --git a/substrate/frame/revive/rpc/src/client.rs b/substrate/frame/revive/rpc/src/client.rs
@@ -17,13 +17,12 @@
 //! The client connects to the source substrate chain
 //! and is used by the rpc server to query and send transactions to the substrate chain.
 use crate::{
-	rlp,
 	runtime::GAS_PRICE,
 	subxt_client::{
 		revive::{calls::types::EthTransact, events::ContractEmitted},
 		runtime_types::pallet_revive::storage::ContractInfo,
 	},
-	TransactionLegacySigned, LOG_TARGET,
+	LOG_TARGET,
 };
 use futures::{stream, StreamExt};
 use jsonrpsee::types::{error::CALL_EXECUTION_FAILED_CODE, ErrorObjectOwned};
@@ -269,25 +268,26 @@ impl ClientInner {
 		let extrinsics = extrinsics.iter().flat_map(|ext| {
 			let call = ext.as_extrinsic::<EthTransact>().ok()??;
 			let transaction_hash = H256(keccak_256(&call.payload));
-			let tx = rlp::decode::<TransactionLegacySigned>(&call.payload).ok()?;
-			let from = tx.recover_eth_address().ok()?;
-			let contract_address = if tx.transaction_legacy_unsigned.to.is_none() {
-				Some(create1(&from, tx.transaction_legacy_unsigned.nonce.try_into().ok()?))
+			let signed_tx = TransactionSigned::decode(&call.payload).ok()?;
+			let from = signed_tx.recover_eth_address().ok()?;
+			let tx_info = GenericTransaction::from_signed(signed_tx.clone(), Some(from));
+			let contract_address = if tx_info.to.is_none() {
+				Some(create1(&from, tx_info.nonce.unwrap_or_default().try_into().ok()?))
 			} else {
 				None
 			};
 
-			Some((from, tx, transaction_hash, contract_address, ext))
+			Some((from, signed_tx, tx_info, transaction_hash, contract_address, ext))
 		});
 
 		// Map each extrinsic to a receipt
 		stream::iter(extrinsics)
-			.map(|(from, tx, transaction_hash, contract_address, ext)| async move {
+			.map(|(from, signed_tx, tx_info, transaction_hash, contract_address, ext)| async move {
 				let events = ext.events().await?;
 				let tx_fees =
 					events.find_first::<TransactionFeePaid>()?.ok_or(ClientError::TxFeeNotFound)?;
 
-				let gas_price = tx.transaction_legacy_unsigned.gas_price;
+				let gas_price = tx_info.gas_price.unwrap_or_default();
 				let gas_used = (tx_fees.tip.saturating_add(tx_fees.actual_fee))
 					.checked_div(gas_price.as_u128())
 					.unwrap_or_default();
@@ -324,16 +324,16 @@ impl ClientInner {
 					contract_address,
 					from,
 					logs,
-					tx.transaction_legacy_unsigned.to,
+					tx_info.to,
 					gas_price,
 					gas_used.into(),
 					success,
 					transaction_hash,
 					transaction_index.into(),
-					tx.transaction_legacy_unsigned.r#type.as_byte()
+					tx_info.r#type.unwrap_or_default()
 				);
 
-				Ok::<_, ClientError>((receipt.transaction_hash, (tx.into(), receipt)))
+				Ok::<_, ClientError>((receipt.transaction_hash, (signed_tx, receipt)))
 			})
 			.buffer_unordered(10)
 			.collect::<Vec<Result<_, _>>>()

diff --git a/substrate/frame/revive/rpc/src/example.rs b/substrate/frame/revive/rpc/src/example.rs
@@ -20,8 +20,7 @@
 use crate::{EthRpcClient, ReceiptInfo};
 use anyhow::Context;
 use pallet_revive::evm::{
-	rlp::*, Account, BlockTag, Bytes, GenericTransaction, TransactionLegacyUnsigned, H160, H256,
-	U256,
+	Account, BlockTag, Bytes, GenericTransaction, TransactionLegacyUnsigned, H160, H256, U256,
 };
 
 /// Wait for a transaction receipt.
@@ -169,11 +168,11 @@ impl TransactionBuilder {
 
 		mutate(&mut unsigned_tx);
 
-		let tx = signer.sign_transaction(unsigned_tx.clone());
-		let bytes = tx.rlp_bytes().to_vec();
+		let tx = signer.sign_transaction(unsigned_tx.into());
+		let bytes = tx.signed_payload();
 
 		let hash = client
-			.send_raw_transaction(bytes.clone().into())
+			.send_raw_transaction(bytes.into())
 			.await
 			.with_context(|| "transaction failed")?;
 

diff --git a/substrate/frame/revive/rpc/src/lib.rs b/substrate/frame/revive/rpc/src/lib.rs
@@ -137,7 +137,7 @@ impl EthRpcServer for EthRpcServerImpl {
 	async fn send_raw_transaction(&self, transaction: Bytes) -> RpcResult<H256> {
 		let hash = H256(keccak_256(&transaction.0));
 
-		let tx = rlp::decode::<TransactionLegacySigned>(&transaction.0).map_err(|err| {
+		let tx = TransactionSigned::decode(&transaction.0).map_err(|err| {
 			log::debug!(target: LOG_TARGET, "Failed to decode transaction: {err:?}");
 			EthRpcError::from(err)
 		})?;
@@ -147,21 +147,10 @@ impl EthRpcServer for EthRpcServerImpl {
 			EthRpcError::InvalidSignature
 		})?;
 
+		let tx = GenericTransaction::from_signed(tx, Some(eth_addr));
+
 		// Dry run the transaction to get the weight limit and storage deposit limit
-		let TransactionLegacyUnsigned { to, input, value, .. } = tx.transaction_legacy_unsigned;
-		let dry_run = self
-			.client
-			.dry_run(
-				&GenericTransaction {
-					from: Some(eth_addr),
-					input: Some(input.clone()),
-					to,
-					value: Some(value),
-					..Default::default()
-				},
-				BlockTag::Latest.into(),
-			)
-			.await?;
+		let dry_run = self.client.dry_run(&tx, BlockTag::Latest.into()).await?;
 
 		let EthContractResult { gas_required, storage_deposit, .. } = dry_run;
 		let call = subxt_client::tx().revive().eth_transact(
@@ -174,11 +163,10 @@ impl EthRpcServer for EthRpcServerImpl {
 		Ok(hash)
 	}
 
-	async fn send_transaction(&self, transaction: GenericTransaction) -> RpcResult<H256> {
+	async fn send_transaction(&self, mut transaction: GenericTransaction) -> RpcResult<H256> {
 		log::debug!(target: LOG_TARGET, "{transaction:#?}");
-		let GenericTransaction { from, gas, gas_price, input, to, value, r#type, .. } = transaction;
 
-		let Some(from) = from else {
+		let Some(from) = transaction.from else {
 			log::debug!(target: LOG_TARGET, "Transaction must have a sender");
 			return Err(EthRpcError::InvalidTransaction.into());
 		};
@@ -189,27 +177,26 @@ impl EthRpcServer for EthRpcServerImpl {
 			.find(|account| account.address() == from)
 			.ok_or(EthRpcError::AccountNotFound(from))?;
 
-		let gas_price = gas_price.unwrap_or_else(|| U256::from(GAS_PRICE));
-		let chain_id = Some(self.client.chain_id().into());
-		let input = input.unwrap_or_default();
-		let value = value.unwrap_or_default();
-		let r#type = r#type.unwrap_or_default();
+		if transaction.gas.is_none() {
+			transaction.gas = Some(self.estimate_gas(transaction.clone(), None).await?);
+		}
 
-		let Some(gas) = gas else {
-			log::debug!(target: LOG_TARGET, "Transaction must have a gas limit");
-			return Err(EthRpcError::InvalidTransaction.into());
-		};
+		if transaction.gas_price.is_none() {
+			transaction.gas_price = Some(self.gas_price().await?);
+		}
 
-		let r#type = Type0::try_from_byte(r#type.clone())
-			.map_err(|_| EthRpcError::TransactionTypeNotSupported(r#type))?;
+		if transaction.nonce.is_none() {
+			transaction.nonce =
+				Some(self.get_transaction_count(from, BlockTag::Latest.into()).await?);
+		}
 
-		let nonce = self.get_transaction_count(from, BlockTag::Latest.into()).await?;
+		if transaction.chain_id.is_none() {
+			transaction.chain_id = Some(self.chain_id().await?);
+		}
 
-		let tx =
-			TransactionLegacyUnsigned { chain_id, gas, gas_price, input, nonce, to, value, r#type };
-		let tx = account.sign_transaction(tx);
-		let rlp_bytes = rlp::encode(&tx).to_vec();
-		self.send_raw_transaction(Bytes(rlp_bytes)).await
+		let tx = transaction.try_into_unsigned().map_err(|_| EthRpcError::InvalidTransaction)?;
+		let payload = account.sign_transaction(tx).signed_payload();
+		self.send_raw_transaction(Bytes(payload)).await
 	}
 
 	async fn get_block_by_hash(

diff --git a/substrate/frame/revive/src/evm/api.rs b/substrate/frame/revive/src/evm/api.rs
@@ -25,9 +25,7 @@ pub use rlp;
 mod type_id;
 pub use type_id::*;
 
-#[cfg(feature = "std")]
 mod rpc_types;
-
 mod rpc_types_gen;
 pub use rpc_types_gen::*;
 

diff --git a/substrate/frame/revive/src/evm/api/account.rs b/substrate/frame/revive/src/evm/api/account.rs
@@ -16,10 +16,9 @@
 // limitations under the License.
 //! Utilities for working with Ethereum accounts.
 use crate::{
-	evm::{TransactionLegacySigned, TransactionLegacyUnsigned},
+	evm::{TransactionSigned, TransactionUnsigned},
 	H160,
 };
-use rlp::Encodable;
 use sp_runtime::AccountId32;
 
 /// A simple account that can sign transactions
@@ -38,6 +37,11 @@ impl From<subxt_signer::eth::Keypair> for Account {
 }
 
 impl Account {
+	/// Create a new account from a secret
+	pub fn from_secret_key(secret_key: [u8; 32]) -> Self {
+		subxt_signer::eth::Keypair::from_secret_key(secret_key).unwrap().into()
+	}
+
 	/// Get the [`H160`] address of the account.
 	pub fn address(&self) -> H160 {
 		H160::from_slice(&self.0.public_key().to_account_id().as_ref())
@@ -52,9 +56,21 @@ impl Account {
 	}
 
 	/// Sign a transaction.
-	pub fn sign_transaction(&self, tx: TransactionLegacyUnsigned) -> TransactionLegacySigned {
-		let rlp_encoded = tx.rlp_bytes();
-		let signature = self.0.sign(&rlp_encoded);
-		TransactionLegacySigned::from(tx, signature.as_ref())
+	pub fn sign_transaction(&self, tx: TransactionUnsigned) -> TransactionSigned {
+		let payload = tx.unsigned_payload();
+		let signature = self.0.sign(&payload).0;
+		tx.with_signature(signature)
 	}
 }
+
+#[test]
+fn from_secret_key_works() {
+	let account = Account::from_secret_key(hex_literal::hex!(
+		"a872f6cbd25a0e04a08b1e21098017a9e6194d101d75e13111f71410c59cd57f"
+	));
+
+	assert_eq!(
+		account.address(),
+		H160::from(hex_literal::hex!("75e480db528101a381ce68544611c169ad7eb342"))
+	)
+}