From 6a601755c9270dbb0390f3fb603aaffe397cd7a8 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Thu, 21 Sep 2023 16:38:21 +0200 Subject: [PATCH] fix: make into_transaction fallible (#4710) --- .../rpc-types-compat/src/transaction/mod.rs | 2 +- crates/rpc/rpc-types/src/eth/call.rs | 2 +- .../rpc/rpc-types/src/eth/transaction/mod.rs | 6 +-- .../rpc-types/src/eth/transaction/request.rs | 18 +++---- .../rpc-types/src/eth/transaction/typed.rs | 49 ++++++++++--------- crates/rpc/rpc/src/eth/api/sign.rs | 3 +- crates/rpc/rpc/src/eth/api/transactions.rs | 3 +- crates/rpc/rpc/src/eth/error.rs | 7 ++- crates/rpc/rpc/src/eth/signer.rs | 7 ++- 9 files changed, 54 insertions(+), 43 deletions(-) diff --git a/crates/rpc/rpc-types-compat/src/transaction/mod.rs b/crates/rpc/rpc-types-compat/src/transaction/mod.rs index eb9cc88e9a..1d5e532057 100644 --- a/crates/rpc/rpc-types-compat/src/transaction/mod.rs +++ b/crates/rpc/rpc-types-compat/src/transaction/mod.rs @@ -108,7 +108,7 @@ fn fill( Transaction { hash: signed_tx.hash(), - nonce: U256::from(signed_tx.nonce()), + nonce: U64::from(signed_tx.nonce()), from: signer, to, value: U256::from(signed_tx.value()), diff --git a/crates/rpc/rpc-types/src/eth/call.rs b/crates/rpc/rpc-types/src/eth/call.rs index e065cdb396..7dd74e3c2a 100644 --- a/crates/rpc/rpc-types/src/eth/call.rs +++ b/crates/rpc/rpc-types/src/eth/call.rs @@ -110,7 +110,7 @@ pub struct CallRequest { #[serde(default, flatten)] pub input: CallInput, /// Nonce - pub nonce: Option, + pub nonce: Option, /// chain id pub chain_id: Option, /// AccessList diff --git a/crates/rpc/rpc-types/src/eth/transaction/mod.rs b/crates/rpc/rpc-types/src/eth/transaction/mod.rs index b2effca69e..64a05a65f4 100644 --- a/crates/rpc/rpc-types/src/eth/transaction/mod.rs +++ b/crates/rpc/rpc-types/src/eth/transaction/mod.rs @@ -19,7 +19,7 @@ pub struct Transaction { /// Hash pub hash: H256, /// Nonce - pub nonce: U256, + pub nonce: U64, /// Block hash pub block_hash: Option, /// Block number @@ -80,7 +80,7 @@ mod tests { fn serde_transaction() { let transaction = Transaction { hash: H256::from_low_u64_be(1), - nonce: U256::from(2), + nonce: U64::from(2), block_hash: Some(H256::from_low_u64_be(3)), block_number: Some(U256::from(4)), transaction_index: Some(U256::from(5)), @@ -117,7 +117,7 @@ mod tests { fn serde_transaction_with_parity_bit() { let transaction = Transaction { hash: H256::from_low_u64_be(1), - nonce: U256::from(2), + nonce: U64::from(2), block_hash: Some(H256::from_low_u64_be(3)), block_number: Some(U256::from(4)), transaction_index: Some(U256::from(5)), diff --git a/crates/rpc/rpc-types/src/eth/transaction/request.rs b/crates/rpc/rpc-types/src/eth/transaction/request.rs index 132deacef6..214c7d3bd2 100644 --- a/crates/rpc/rpc-types/src/eth/transaction/request.rs +++ b/crates/rpc/rpc-types/src/eth/transaction/request.rs @@ -2,7 +2,7 @@ use crate::eth::transaction::typed::{ EIP1559TransactionRequest, EIP2930TransactionRequest, LegacyTransactionRequest, TransactionKind, TypedTransactionRequest, }; -use reth_primitives::{AccessList, Address, Bytes, U128, U256, U8}; +use reth_primitives::{AccessList, Address, Bytes, U128, U256, U64, U8}; use serde::{Deserialize, Serialize}; /// Represents _all_ transaction requests received from RPC @@ -31,7 +31,7 @@ pub struct TransactionRequest { #[serde(alias = "input")] pub data: Option, /// Transaction nonce - pub nonce: Option, + pub nonce: Option, /// warm storage access pre-payment #[serde(default)] pub access_list: Option, @@ -64,10 +64,10 @@ impl TransactionRequest { // legacy transaction (Some(_), None, None) => { Some(TypedTransactionRequest::Legacy(LegacyTransactionRequest { - nonce: nonce.unwrap_or(U256::ZERO), + nonce: nonce.unwrap_or_default(), gas_price: gas_price.unwrap_or_default(), gas_limit: gas.unwrap_or_default(), - value: value.unwrap_or(U256::ZERO), + value: value.unwrap_or_default(), input: data.unwrap_or_default(), kind: match to { Some(to) => TransactionKind::Call(to), @@ -79,10 +79,10 @@ impl TransactionRequest { // EIP2930 (_, None, Some(access_list)) => { Some(TypedTransactionRequest::EIP2930(EIP2930TransactionRequest { - nonce: nonce.unwrap_or(U256::ZERO), + nonce: nonce.unwrap_or_default(), gas_price: gas_price.unwrap_or_default(), gas_limit: gas.unwrap_or_default(), - value: value.unwrap_or(U256::ZERO), + value: value.unwrap_or_default(), input: data.unwrap_or_default(), kind: match to { Some(to) => TransactionKind::Call(to), @@ -96,11 +96,11 @@ impl TransactionRequest { (None, Some(_), access_list) | (None, None, access_list @ None) => { // Empty fields fall back to the canonical transaction schema. Some(TypedTransactionRequest::EIP1559(EIP1559TransactionRequest { - nonce: nonce.unwrap_or(U256::ZERO), + nonce: nonce.unwrap_or_default(), max_fee_per_gas: max_fee_per_gas.unwrap_or_default(), - max_priority_fee_per_gas: max_priority_fee_per_gas.unwrap_or(U128::ZERO), + max_priority_fee_per_gas: max_priority_fee_per_gas.unwrap_or_default(), gas_limit: gas.unwrap_or_default(), - value: value.unwrap_or(U256::ZERO), + value: value.unwrap_or_default(), input: data.unwrap_or_default(), kind: match to { Some(to) => TransactionKind::Call(to), diff --git a/crates/rpc/rpc-types/src/eth/transaction/typed.rs b/crates/rpc/rpc-types/src/eth/transaction/typed.rs index a87e1f4345..b92a889f35 100644 --- a/crates/rpc/rpc-types/src/eth/transaction/typed.rs +++ b/crates/rpc/rpc-types/src/eth/transaction/typed.rs @@ -4,7 +4,7 @@ //! it can be converted into the container type [`TypedTransactionRequest`]. use reth_primitives::{ - AccessList, Address, Bytes, Transaction, TxEip1559, TxEip2930, TxLegacy, U128, U256, + AccessList, Address, Bytes, Transaction, TxEip1559, TxEip2930, TxLegacy, U128, U256, U64, }; use reth_rlp::{BufMut, Decodable, DecodeError, Encodable, RlpDecodable, RlpEncodable}; use serde::{Deserialize, Serialize}; @@ -23,49 +23,52 @@ pub enum TypedTransactionRequest { } impl TypedTransactionRequest { - /// coverts a typed transaction request into a primitive transaction - pub fn into_transaction(self) -> Transaction { - match self { + /// Converts a typed transaction request into a primitive transaction. + /// + /// Returns `None` if any of the following are true: + /// - `nonce` is greater than [`u64::MAX`] + /// - `gas_limit` is greater than [`u64::MAX`] + /// - `value` is greater than [`u128::MAX`] + pub fn into_transaction(self) -> Option { + Some(match self { TypedTransactionRequest::Legacy(tx) => Transaction::Legacy(TxLegacy { chain_id: tx.chain_id, - nonce: u64::from_be_bytes(tx.nonce.to_be_bytes()), - gas_price: u128::from_be_bytes(tx.gas_price.to_be_bytes()), - gas_limit: u64::from_be_bytes(tx.gas_limit.to_be_bytes()), + nonce: tx.nonce.as_u64(), + gas_price: tx.gas_price.to(), + gas_limit: tx.gas_limit.try_into().ok()?, to: tx.kind.into(), - value: u128::from_be_bytes(tx.value.to_be_bytes()), + value: tx.value.try_into().ok()?, input: tx.input, }), TypedTransactionRequest::EIP2930(tx) => Transaction::Eip2930(TxEip2930 { chain_id: tx.chain_id, - nonce: u64::from_be_bytes(tx.nonce.to_be_bytes()), - gas_price: u128::from_be_bytes(tx.gas_price.to_be_bytes()), - gas_limit: u64::from_be_bytes(tx.gas_limit.to_be_bytes()), + nonce: tx.nonce.as_u64(), + gas_price: tx.gas_price.to(), + gas_limit: tx.gas_limit.try_into().ok()?, to: tx.kind.into(), - value: u128::from_be_bytes(tx.value.to_be_bytes()), + value: tx.value.try_into().ok()?, input: tx.input, access_list: tx.access_list, }), TypedTransactionRequest::EIP1559(tx) => Transaction::Eip1559(TxEip1559 { chain_id: tx.chain_id, - nonce: u64::from_be_bytes(tx.nonce.to_be_bytes()), - max_fee_per_gas: u128::from_be_bytes(tx.max_fee_per_gas.to_be_bytes()), - gas_limit: u64::from_be_bytes(tx.gas_limit.to_be_bytes()), + nonce: tx.nonce.as_u64(), + max_fee_per_gas: tx.max_fee_per_gas.to(), + gas_limit: tx.gas_limit.try_into().ok()?, to: tx.kind.into(), - value: u128::from_be_bytes(tx.value.to_be_bytes()), + value: tx.value.try_into().ok()?, input: tx.input, access_list: tx.access_list, - max_priority_fee_per_gas: u128::from_be_bytes( - tx.max_priority_fee_per_gas.to_be_bytes(), - ), + max_priority_fee_per_gas: tx.max_priority_fee_per_gas.to(), }), - } + }) } } /// Represents a legacy transaction request #[derive(Debug, Clone, PartialEq, Eq)] pub struct LegacyTransactionRequest { - pub nonce: U256, + pub nonce: U64, pub gas_price: U128, pub gas_limit: U256, pub kind: TransactionKind, @@ -78,7 +81,7 @@ pub struct LegacyTransactionRequest { #[derive(Debug, Clone, PartialEq, Eq, RlpEncodable, RlpDecodable)] pub struct EIP2930TransactionRequest { pub chain_id: u64, - pub nonce: U256, + pub nonce: U64, pub gas_price: U128, pub gas_limit: U256, pub kind: TransactionKind, @@ -91,7 +94,7 @@ pub struct EIP2930TransactionRequest { #[derive(Debug, Clone, PartialEq, Eq, RlpEncodable, RlpDecodable)] pub struct EIP1559TransactionRequest { pub chain_id: u64, - pub nonce: U256, + pub nonce: U64, pub max_priority_fee_per_gas: U128, pub max_fee_per_gas: U128, pub gas_limit: U256, diff --git a/crates/rpc/rpc/src/eth/api/sign.rs b/crates/rpc/rpc/src/eth/api/sign.rs index 48164a6007..c12d7b3011 100644 --- a/crates/rpc/rpc/src/eth/api/sign.rs +++ b/crates/rpc/rpc/src/eth/api/sign.rs @@ -21,7 +21,8 @@ impl EthApi { pub(crate) async fn sign_typed_data(&self, data: Value, account: Address) -> EthResult { let signer = self.find_signer(&account)?; - let data = serde_json::from_value::(data).map_err(|_| SignError::TypedData)?; + let data = + serde_json::from_value::(data).map_err(|_| SignError::InvalidTypedData)?; let signature = signer.sign_typed_data(account, &data)?; let bytes = hex::encode(signature.to_bytes()).as_bytes().into(); Ok(bytes) diff --git a/crates/rpc/rpc/src/eth/api/transactions.rs b/crates/rpc/rpc/src/eth/api/transactions.rs index d3b4e16770..3479ce59bc 100644 --- a/crates/rpc/rpc/src/eth/api/transactions.rs +++ b/crates/rpc/rpc/src/eth/api/transactions.rs @@ -446,7 +446,8 @@ where if request.nonce.is_none() { let nonce = self.get_transaction_count(from, Some(BlockId::Number(BlockNumberOrTag::Pending)))?; - request.nonce = Some(nonce); + // note: `.to()` can't panic because the nonce is constructed from a `u64` + request.nonce = Some(U64::from(nonce.to::())); } let chain_id = self.chain_id(); diff --git a/crates/rpc/rpc/src/eth/error.rs b/crates/rpc/rpc/src/eth/error.rs index 1aecb13227..58728b0ee6 100644 --- a/crates/rpc/rpc/src/eth/error.rs +++ b/crates/rpc/rpc/src/eth/error.rs @@ -600,8 +600,11 @@ pub enum SignError { NoAccount, /// TypedData has invalid format. #[error("Given typed data is not valid")] - TypedData, - /// No chainid + InvalidTypedData, + /// Invalid transaction request in `sign_transaction`. + #[error("Invalid transaction request")] + InvalidTransactionRequest, + /// No chain ID was given. #[error("No chainid")] NoChainId, } diff --git a/crates/rpc/rpc/src/eth/signer.rs b/crates/rpc/rpc/src/eth/signer.rs index 72232b7162..bf7fe6b335 100644 --- a/crates/rpc/rpc/src/eth/signer.rs +++ b/crates/rpc/rpc/src/eth/signer.rs @@ -48,12 +48,14 @@ impl DevSigner { fn get_key(&self, account: Address) -> Result<&SecretKey> { self.accounts.get(&account).ok_or(SignError::NoAccount) } + fn sign_hash(&self, hash: H256, account: Address) -> Result { let secret = self.get_key(account)?; let signature = sign_message(H256::from_slice(secret.as_ref()), hash); signature.map_err(|_| SignError::CouldNotSign) } } + #[async_trait::async_trait] impl EthSigner for DevSigner { fn accounts(&self) -> Vec
{ @@ -77,7 +79,7 @@ impl EthSigner for DevSigner { address: &Address, ) -> Result { // convert to primitive transaction - let transaction = request.into_transaction(); + let transaction = request.into_transaction().ok_or(SignError::InvalidTransactionRequest)?; let tx_signature_hash = transaction.signature_hash(); let signature = self.sign_hash(tx_signature_hash, *address)?; @@ -85,7 +87,8 @@ impl EthSigner for DevSigner { } fn sign_typed_data(&self, address: Address, payload: &TypedData) -> Result { - let encoded: H256 = payload.encode_eip712().map_err(|_| SignError::TypedData)?.into(); + let encoded: H256 = + payload.encode_eip712().map_err(|_| SignError::InvalidTypedData)?.into(); self.sign_hash(encoded, address) } }