diff --git a/crates/consensus/src/verification.rs b/crates/consensus/src/verification.rs index f02e555bbb..297233be11 100644 --- a/crates/consensus/src/verification.rs +++ b/crates/consensus/src/verification.rs @@ -2,11 +2,14 @@ use crate::{config, Config}; use reth_interfaces::{consensus::Error, Result as RethResult}; use reth_primitives::{ - Account, BlockLocked, BlockNumber, SealedHeader, Transaction, TxEip1559, TxEip2930, TxLegacy, - EMPTY_OMMER_ROOT, H256, U256, + BlockLocked, BlockNumber, Header, SealedHeader, Transaction, TransactionSignedEcRecovered, + TxEip1559, TxEip2930, TxLegacy, EMPTY_OMMER_ROOT, H256, U256, }; use reth_provider::{AccountProvider, HeaderProvider}; -use std::time::SystemTime; +use std::{ + collections::{hash_map::Entry, HashMap}, + time::SystemTime, +}; /// Validate header standalone pub fn validate_header_standalone( @@ -122,41 +125,52 @@ pub fn validate_transaction_regarding_header( Ok(()) } -/// Validate transaction in regards of State -pub fn validate_transaction_regarding_account( - transaction: &Transaction, - account: &Account, -) -> reth_interfaces::Result<()> { - // Signer account shoudn't have bytecode. Presence of bytecode means this is a smartcontract. - if account.has_bytecode() { - return Err(Error::SignerAccountHasBytecode.into()) - } +/// Iterate over all transactions, verify them agains each other and against the block. +/// There is no gas check done as [REVM](https://github.com/bluealloy/revm/blob/fd0108381799662098b7ab2c429ea719d6dfbf28/crates/revm/src/evm_impl.rs#L113-L131) already checks that. +pub fn validate_all_transaction_regarding_block_and_nonces< + 'a, + Provider: HeaderProvider + AccountProvider, +>( + transactions: impl Iterator, + header: &Header, + provider: Provider, + config: &Config, +) -> RethResult<()> { + let mut account_nonces = HashMap::new(); - let (nonce, gas_price, gas_limit, value) = match transaction { - Transaction::Legacy(TxLegacy { nonce, gas_price, gas_limit, value, .. }) => { - (nonce, gas_price, gas_limit, value) + for transaction in transactions { + validate_transaction_regarding_header( + transaction, + config, + header.number, + header.base_fee_per_gas, + )?; + + // Get nonce, if there is previous transaction from same sender we need + // to take that nonce. + let nonce = match account_nonces.entry(transaction.signer()) { + Entry::Occupied(mut entry) => { + let nonce = *entry.get(); + *entry.get_mut() += 1; + nonce + } + Entry::Vacant(entry) => { + let account = provider.basic_account(transaction.signer())?.unwrap_or_default(); + // Signer account shoudn't have bytecode. Presence of bytecode means this is a + // smartcontract. + if account.has_bytecode() { + return Err(Error::SignerAccountHasBytecode.into()) + } + let nonce = account.nonce; + entry.insert(account.nonce + 1); + nonce + } + }; + + // check nonce + if transaction.nonce() != nonce { + return Err(Error::TransactionNonceNotConsistent.into()) } - Transaction::Eip2930(TxEip2930 { nonce, gas_price, gas_limit, value, .. }) => { - (nonce, gas_price, gas_limit, value) - } - Transaction::Eip1559(TxEip1559 { nonce, gas_limit, max_fee_per_gas, value, .. }) => { - (nonce, max_fee_per_gas, gas_limit, value) - } - }; - - // check nonce - if account.nonce + 1 != *nonce { - return Err(Error::TransactionNonceNotConsistent.into()) - } - - // max fee that transaction can potentially spend - let max_fee = (*gas_limit as u128).saturating_mul(*gas_price).saturating_add(*value); - - // check if account balance has enought to cover worst case - if max_fee > account.balance.as_u128() { - return Err( - Error::InsufficientFunds { max_fee, available_funds: account.balance.as_u128() }.into() - ) } Ok(()) @@ -329,30 +343,29 @@ pub fn full_validation( let parent = validate_block_regarding_chain(block, &provider)?; validate_header_regarding_parent(&parent, &block.header, config)?; - for transaction in block.body.iter() { - validate_transaction_regarding_header( - transaction, - config, - block.header.number, - block.header.base_fee_per_gas, - )?; + // NOTE: depending on the need of the stages, recovery could be done in different place. + let transactions = block + .body + .iter() + .map(|tx| tx.try_ecrecovered().ok_or(Error::TransactionSignerRecoveryError)) + .collect::, _>>()?; - // NOTE: depending on the need of the stages, recovery could be done in different place. - let recovered = - transaction.try_ecrecovered().ok_or(Error::TransactionSignerRecoveryError)?; - - let account = - provider.basic_account(recovered.signer())?.ok_or(Error::SignerAccountNotExisting)?; - - validate_transaction_regarding_account(transaction, &account)?; - } + validate_all_transaction_regarding_block_and_nonces( + transactions.iter(), + &block.header, + provider, + config, + )?; Ok(()) } #[cfg(test)] mod tests { use reth_interfaces::Result; - use reth_primitives::{hex_literal::hex, Address, BlockHash, Header}; + use reth_primitives::{ + hex_literal::hex, Account, Address, BlockHash, Bytes, Header, Signature, TransactionKind, + TransactionSigned, + }; use super::*; @@ -419,6 +432,25 @@ mod tests { Ok(self.parent.clone()) } } + + fn mock_tx(nonce: u64) -> TransactionSignedEcRecovered { + let request = Transaction::Eip2930(TxEip2930 { + chain_id: 1u64, + nonce, + gas_price: 0x28f000fff, + gas_limit: 10, + to: TransactionKind::Call(Address::default()), + value: 3, + input: Bytes::from(vec![1, 2]), + access_list: Default::default(), + }); + + let signature = Signature { odd_y_parity: true, r: U256::default(), s: U256::default() }; + + let tx = TransactionSigned::from_transaction_and_signature(request, signature); + let signer = Address::zero(); + TransactionSignedEcRecovered::from_signed_transaction(tx, signer) + } /// got test block fn mock_block() -> (BlockLocked, Header) { // https://etherscan.io/block/15867168 where transaction root and receipts root are cleared @@ -477,4 +509,61 @@ mod tests { "Should fail with error" ); } + + #[test] + fn sanity_tx_nonce_check() { + let (block, _) = mock_block(); + let tx1 = mock_tx(0); + let tx2 = mock_tx(1); + let provider = Provider::new_known(); + let config = Config::default(); + + let txs = vec![tx1, tx2]; + validate_all_transaction_regarding_block_and_nonces( + txs.iter(), + &block.header, + provider, + &config, + ) + .expect("To Pass"); + } + + #[test] + fn nonce_gap_in_first_transaction() { + let (block, _) = mock_block(); + let tx1 = mock_tx(1); + let provider = Provider::new_known(); + let config = Config::default(); + + let txs = vec![tx1]; + assert_eq!( + validate_all_transaction_regarding_block_and_nonces( + txs.iter(), + &block.header, + provider, + &config, + ), + Err(Error::TransactionNonceNotConsistent.into()) + ) + } + + #[test] + fn nonce_gap_on_second_tx_from_same_signer() { + let (block, _) = mock_block(); + let tx1 = mock_tx(0); + let tx2 = mock_tx(3); + let provider = Provider::new_known(); + let config = Config::default(); + + let txs = vec![tx1, tx2]; + assert_eq!( + validate_all_transaction_regarding_block_and_nonces( + txs.iter(), + &block.header, + provider, + &config, + ), + Err(Error::TransactionNonceNotConsistent.into()) + ); + } } diff --git a/crates/primitives/src/transaction/mod.rs b/crates/primitives/src/transaction/mod.rs index b220c2e10c..65ab049c26 100644 --- a/crates/primitives/src/transaction/mod.rs +++ b/crates/primitives/src/transaction/mod.rs @@ -788,20 +788,13 @@ impl TransactionSignedEcRecovered { self.signer } - /// Create [TransactionSignedEcRecovered] from [TransactionSigned] and author of transaction - pub fn from_signed_transactions_and_signer( - signed_transaction: TransactionSigned, - signer: Address, - ) -> Self { - Self { signed_transaction, signer } - } - /// Transform back to [`TransactionSigned`] pub fn into_signed(self) -> TransactionSigned { self.signed_transaction } - /// Create [`TransactionSignedEcRecovered`] from [`TransactionSigned`] and [`Address`]. + /// Create [`TransactionSignedEcRecovered`] from [`TransactionSigned`] and [`Address`] of the + /// signer. pub fn from_signed_transaction(signed_transaction: TransactionSigned, signer: Address) -> Self { Self { signed_transaction, signer } } diff --git a/crates/stages/src/stages/execution.rs b/crates/stages/src/stages/execution.rs index eae4a8dbe7..67c5f466ff 100644 --- a/crates/stages/src/stages/execution.rs +++ b/crates/stages/src/stages/execution.rs @@ -209,7 +209,7 @@ impl Stage for ExecutionStage { .into_iter() .zip(signers.into_iter()) .map(|(tx, signer)| { - TransactionSignedEcRecovered::from_signed_transactions_and_signer(tx, signer) + TransactionSignedEcRecovered::from_signed_transaction(tx, signer) }) .collect();