From 2d20be0d5cb6eac064a5a8bbf89e4ca8802cf266 Mon Sep 17 00:00:00 2001 From: Martin Paulucci Date: Tue, 24 Jan 2023 21:05:14 +0100 Subject: [PATCH] refactor: Make Transaction fee and price field types consistent (#1005) Co-authored-by: lambdaclass-user --- crates/transaction-pool/src/error.rs | 4 +- crates/transaction-pool/src/pool/txpool.rs | 18 ++++---- .../transaction-pool/src/test_utils/mock.rs | 46 +++++++++---------- .../transaction-pool/src/test_utils/pool.rs | 8 ++-- crates/transaction-pool/src/traits.rs | 26 +++++------ crates/transaction-pool/src/validate.rs | 2 +- 6 files changed, 51 insertions(+), 53 deletions(-) diff --git a/crates/transaction-pool/src/error.rs b/crates/transaction-pool/src/error.rs index 19d16511ec..fda5007fd7 100644 --- a/crates/transaction-pool/src/error.rs +++ b/crates/transaction-pool/src/error.rs @@ -1,6 +1,6 @@ //! Transaction pool errors -use reth_primitives::{Address, TxHash, U256}; +use reth_primitives::{Address, TxHash}; /// Transaction pool result type. pub type PoolResult = Result; @@ -13,7 +13,7 @@ pub enum PoolError { ReplacementUnderpriced(TxHash), /// Encountered a transaction that was already added into the poll #[error("[{0:?}] Transaction feeCap {1} below chain minimum.")] - ProtocolFeeCapTooLow(TxHash, U256), + ProtocolFeeCapTooLow(TxHash, u128), /// Thrown when the number of unique transactions of a sender exceeded the slot capacity. #[error("{0:?} identified as spammer. Transaction {1:?} rejected.")] SpammerExceededCapacity(Address, TxHash), diff --git a/crates/transaction-pool/src/pool/txpool.rs b/crates/transaction-pool/src/pool/txpool.rs index 2da8c71863..5bfe5993cb 100644 --- a/crates/transaction-pool/src/pool/txpool.rs +++ b/crates/transaction-pool/src/pool/txpool.rs @@ -18,7 +18,6 @@ use crate::{ }; use fnv::FnvHashMap; use reth_primitives::{TxHash, H256}; -use ruint::uint; use std::{ cmp::Ordering, collections::{btree_map::Entry, hash_map, BTreeMap, HashMap}, @@ -30,7 +29,7 @@ use std::{ /// The minimal value the basefee can decrease to /// /// The `BASE_FEE_MAX_CHANGE_DENOMINATOR` (https://eips.ethereum.org/EIPS/eip-1559) is `8`, or 12.5%, once the base fee has dropped to `7` WEI it cannot decrease further because 12.5% of 7 is less than 1. -pub(crate) const MIN_PROTOCOL_BASE_FEE: U256 = uint!(0x7_U256); +pub(crate) const MIN_PROTOCOL_BASE_FEE: u128 = 7; /// A pool that manages transactions. /// @@ -485,11 +484,11 @@ impl fmt::Debug for TxPool { /// derived from this set. Updates returned from this type must be applied to the sub-pools. pub(crate) struct AllTransactions { /// Expected base fee for the pending block. - pending_basefee: U256, + pending_basefee: u128, /// Minimum base fee required by the protocol. /// /// Transactions with a lower base fee will never be included by the chain - minimal_protocol_basefee: U256, + minimal_protocol_basefee: u128, /// The max gas limit of the block block_gas_limit: u64, /// Max number of executable transaction slots guaranteed per account @@ -559,7 +558,7 @@ impl AllTransactions { /// that got transaction included in the block. pub(crate) fn update( &mut self, - pending_block_base_fee: U256, + pending_block_base_fee: u128, _state_diffs: &StateDiff, ) -> Vec { // update new basefee @@ -661,7 +660,7 @@ impl AllTransactions { } /// Rechecks the transaction's dynamic fee condition. - fn update_base_fee(pending_block_base_fee: &U256, tx: &mut PoolInternalTransaction) { + fn update_base_fee(pending_block_base_fee: &u128, tx: &mut PoolInternalTransaction) { // Recheck dynamic fee condition. if let Some(fee_cap) = tx.transaction.max_fee_per_gas() { match fee_cap.cmp(pending_block_base_fee) { @@ -1018,7 +1017,7 @@ pub(crate) enum InsertErr { /// The transactions feeCap is lower than the chain's minimum fee requirement. /// /// See also [`MIN_PROTOCOL_BASE_FEE`] - ProtocolFeeCapTooLow { transaction: Arc>, fee_cap: U256 }, + ProtocolFeeCapTooLow { transaction: Arc>, fee_cap: u128 }, /// Sender currently exceeds the configured limit for max account slots. /// /// The sender can be considered a spammer at this point. @@ -1250,8 +1249,7 @@ mod tests { let on_chain_nonce = 0; let mut f = MockTransactionFactory::default(); let mut pool = AllTransactions::default(); - let tx = - MockTransaction::eip1559().inc_nonce().set_gas_price(U256::from(100u64)).inc_limit(); + let tx = MockTransaction::eip1559().inc_nonce().set_gas_price(100).inc_limit(); let first = f.validated(tx.clone()); let _res = pool.insert_tx(first.clone(), on_chain_balance, on_chain_nonce).unwrap(); @@ -1282,7 +1280,7 @@ mod tests { let on_chain_nonce = 0; let mut f = MockTransactionFactory::default(); let mut pool = AllTransactions::default(); - pool.pending_basefee = pool.minimal_protocol_basefee.checked_add(U256::from(1)).unwrap(); + pool.pending_basefee = pool.minimal_protocol_basefee.checked_add(1).unwrap(); let tx = MockTransaction::eip1559().inc_nonce().inc_limit(); let first = f.validated(tx.clone()); diff --git a/crates/transaction-pool/src/test_utils/mock.rs b/crates/transaction-pool/src/test_utils/mock.rs index 11b1ceec83..051433084c 100644 --- a/crates/transaction-pool/src/test_utils/mock.rs +++ b/crates/transaction-pool/src/test_utils/mock.rs @@ -13,7 +13,7 @@ use rand::{ }; use reth_primitives::{ Address, FromRecoveredTransaction, IntoRecoveredTransaction, Transaction, - TransactionSignedEcRecovered, TxEip1559, TxHash, TxLegacy, H256, U256, + TransactionSignedEcRecovered, TxEip1559, TxHash, TxLegacy, H256, U128, U256, }; use std::{ops::Range, sync::Arc, time::Instant}; @@ -83,7 +83,7 @@ pub enum MockTransaction { hash: H256, sender: Address, nonce: u64, - gas_price: U256, + gas_price: u128, gas_limit: u64, value: U256, }, @@ -91,8 +91,8 @@ pub enum MockTransaction { hash: H256, sender: Address, nonce: u64, - max_fee_per_gas: U256, - max_priority_fee_per_gas: U256, + max_fee_per_gas: u128, + max_priority_fee_per_gas: u128, gas_limit: u64, value: U256, }, @@ -115,7 +115,7 @@ impl MockTransaction { hash: H256::random(), sender: Address::random(), nonce: 0, - gas_price: U256::ZERO, + gas_price: 0, gas_limit: 0, value: Default::default(), } @@ -134,21 +134,21 @@ impl MockTransaction { } } - pub fn set_priority_fee(&mut self, val: U256) -> &mut Self { + pub fn set_priority_fee(&mut self, val: u128) -> &mut Self { if let MockTransaction::Eip1559 { max_priority_fee_per_gas, .. } = self { *max_priority_fee_per_gas = val; } self } - pub fn with_priority_fee(mut self, val: U256) -> Self { + pub fn with_priority_fee(mut self, val: u128) -> Self { if let MockTransaction::Eip1559 { ref mut max_priority_fee_per_gas, .. } = self { *max_priority_fee_per_gas = val; } self } - pub fn get_priority_fee(&self) -> Option { + pub fn get_priority_fee(&self) -> Option { if let MockTransaction::Eip1559 { max_priority_fee_per_gas, .. } = self { Some(*max_priority_fee_per_gas) } else { @@ -156,21 +156,21 @@ impl MockTransaction { } } - pub fn set_max_fee(&mut self, val: U256) -> &mut Self { + pub fn set_max_fee(&mut self, val: u128) -> &mut Self { if let MockTransaction::Eip1559 { max_fee_per_gas, .. } = self { *max_fee_per_gas = val; } self } - pub fn with_max_fee(mut self, val: U256) -> Self { + pub fn with_max_fee(mut self, val: u128) -> Self { if let MockTransaction::Eip1559 { ref mut max_fee_per_gas, .. } = self { *max_fee_per_gas = val; } self } - pub fn get_max_fee(&self) -> Option { + pub fn get_max_fee(&self) -> Option { if let MockTransaction::Eip1559 { max_fee_per_gas, .. } = self { Some(*max_fee_per_gas) } else { @@ -178,7 +178,7 @@ impl MockTransaction { } } - pub fn set_gas_price(&mut self, val: U256) -> &mut Self { + pub fn set_gas_price(&mut self, val: u128) -> &mut Self { match self { MockTransaction::Legacy { gas_price, .. } => { *gas_price = val; @@ -191,7 +191,7 @@ impl MockTransaction { self } - pub fn with_gas_price(mut self, val: U256) -> Self { + pub fn with_gas_price(mut self, val: u128) -> Self { match self { MockTransaction::Legacy { ref mut gas_price, .. } => { *gas_price = val; @@ -208,7 +208,7 @@ impl MockTransaction { self } - pub fn get_gas_price(&self) -> U256 { + pub fn get_gas_price(&self) -> u128 { match self { MockTransaction::Legacy { gas_price, .. } => *gas_price, MockTransaction::Eip1559 { max_fee_per_gas, .. } => *max_fee_per_gas, @@ -247,7 +247,7 @@ impl MockTransaction { /// Returns a new transaction with a higher gas price +1 pub fn inc_price(&self) -> Self { let mut next = self.clone(); - let gas = self.get_gas_price().checked_add(U256::from(1)).unwrap(); + let gas = self.get_gas_price().checked_add(1).unwrap(); next.with_gas_price(gas) } @@ -299,15 +299,15 @@ impl PoolTransaction for MockTransaction { fn cost(&self) -> U256 { match self { MockTransaction::Legacy { gas_price, value, gas_limit, .. } => { - U256::from(*gas_limit) * *gas_price + *value + U256::from(*gas_limit) * U256::from(*gas_price) + *value } MockTransaction::Eip1559 { max_fee_per_gas, value, gas_limit, .. } => { - U256::from(*gas_limit) * *max_fee_per_gas + *value + U256::from(*gas_limit) * U256::from(*max_fee_per_gas) + *value } } } - fn effective_gas_price(&self) -> U256 { + fn effective_gas_price(&self) -> u128 { self.get_gas_price() } @@ -315,14 +315,14 @@ impl PoolTransaction for MockTransaction { self.get_gas_limit() } - fn max_fee_per_gas(&self) -> Option { + fn max_fee_per_gas(&self) -> Option { match self { MockTransaction::Legacy { .. } => None, MockTransaction::Eip1559 { max_fee_per_gas, .. } => Some(*max_fee_per_gas), } } - fn max_priority_fee_per_gas(&self) -> Option { + fn max_priority_fee_per_gas(&self) -> Option { match self { MockTransaction::Legacy { .. } => None, MockTransaction::Eip1559 { max_priority_fee_per_gas, .. } => { @@ -354,7 +354,7 @@ impl FromRecoveredTransaction for MockTransaction { hash, sender, nonce, - gas_price: U256::from(gas_price), + gas_price, gas_limit, value: U256::from(value), }, @@ -372,8 +372,8 @@ impl FromRecoveredTransaction for MockTransaction { hash, sender, nonce, - max_fee_per_gas: U256::from(max_fee_per_gas), - max_priority_fee_per_gas: U256::from(max_priority_fee_per_gas), + max_fee_per_gas, + max_priority_fee_per_gas, gas_limit, value: U256::from(value), }, diff --git a/crates/transaction-pool/src/test_utils/pool.rs b/crates/transaction-pool/src/test_utils/pool.rs index 9e6f3cdb95..30217a8672 100644 --- a/crates/transaction-pool/src/test_utils/pool.rs +++ b/crates/transaction-pool/src/test_utils/pool.rs @@ -9,7 +9,7 @@ use crate::{ TransactionOrdering, }; use rand::Rng; -use reth_primitives::{Address, U256}; +use reth_primitives::{Address, U128, U256}; use serde::{Deserialize, Serialize}; use std::{ collections::HashMap, @@ -62,7 +62,7 @@ impl DerefMut for MockPool { /// Simulates transaction execution. pub struct MockTransactionSimulator { /// The pending base fee - base_fee: U256, + base_fee: u128, /// Generator for transactions tx_generator: MockTransactionDistribution, /// represents the on chain balance of a sender. @@ -160,7 +160,7 @@ pub struct MockSimulatorConfig { /// Scenarios to test pub scenarios: Vec, /// The start base fee - pub base_fee: U256, + pub base_fee: u128, /// generator for transactions pub tx_generator: MockTransactionDistribution, } @@ -220,7 +220,7 @@ fn test_on_chain_nonce_scenario() { num_senders: 10, balance: U256::from(200_000u64), scenarios: vec![ScenarioType::OnchainNonce], - base_fee: U256::from(10u64), + base_fee: 10, tx_generator: MockTransactionDistribution::new(30, 10..100), }; let mut simulator = MockTransactionSimulator::new(rand::thread_rng(), config); diff --git a/crates/transaction-pool/src/traits.rs b/crates/transaction-pool/src/traits.rs index c0b296734f..f1a3bfe783 100644 --- a/crates/transaction-pool/src/traits.rs +++ b/crates/transaction-pool/src/traits.rs @@ -206,7 +206,7 @@ pub struct OnNewBlockEvent { /// EIP-1559 Base fee of the _next_ (pending) block /// /// The base fee of a block depends on the utilization of the last block and its base fee. - pub pending_block_base_fee: U256, + pub pending_block_base_fee: u128, /// Provides a set of state changes that affected the accounts. pub state_changes: StateDiff, /// All mined transactions in the block @@ -256,7 +256,7 @@ pub trait PoolTransaction: fmt::Debug + Send + Sync + FromRecoveredTransaction { /// Returns the effective gas price for this transaction. /// /// This is `priority + basefee`for EIP-1559 and `gasPrice` for legacy transactions. - fn effective_gas_price(&self) -> U256; + fn effective_gas_price(&self) -> u128; /// Amount of gas that should be used in executing this transaction. This is paid up-front. fn gas_limit(&self) -> u64; @@ -264,12 +264,12 @@ pub trait PoolTransaction: fmt::Debug + Send + Sync + FromRecoveredTransaction { /// Returns the EIP-1559 Max base fee the caller is willing to pay. /// /// This will return `None` for non-EIP1559 transactions - fn max_fee_per_gas(&self) -> Option; + fn max_fee_per_gas(&self) -> Option; /// Returns the EIP-1559 Priority fee the caller is paying to the block author. /// /// This will return `None` for non-EIP1559 transactions - fn max_priority_fee_per_gas(&self) -> Option; + fn max_priority_fee_per_gas(&self) -> Option; /// Returns a measurement of the heap usage of this type and all its internals. fn size(&self) -> usize; @@ -288,7 +288,7 @@ pub(crate) struct PooledTransaction { pub(crate) cost: U256, /// This is `priority + basefee`for EIP-1559 and `gasPrice` for legacy transactions. - pub(crate) effective_gas_price: U256, + pub(crate) effective_gas_price: u128, } impl PoolTransaction for PooledTransaction { @@ -317,7 +317,7 @@ impl PoolTransaction for PooledTransaction { /// Returns the effective gas price for this transaction. /// /// This is `priority + basefee`for EIP-1559 and `gasPrice` for legacy transactions. - fn effective_gas_price(&self) -> U256 { + fn effective_gas_price(&self) -> u128 { self.effective_gas_price } @@ -329,22 +329,22 @@ impl PoolTransaction for PooledTransaction { /// Returns the EIP-1559 Max base fee the caller is willing to pay. /// /// This will return `None` for non-EIP1559 transactions - fn max_fee_per_gas(&self) -> Option { + fn max_fee_per_gas(&self) -> Option { match &self.transaction.transaction { Transaction::Legacy(_) => None, Transaction::Eip2930(_) => None, - Transaction::Eip1559(tx) => Some(U256::from(tx.max_fee_per_gas)), + Transaction::Eip1559(tx) => Some(tx.max_fee_per_gas), } } /// Returns the EIP-1559 Priority fee the caller is paying to the block author. /// /// This will return `None` for non-EIP1559 transactions - fn max_priority_fee_per_gas(&self) -> Option { + fn max_priority_fee_per_gas(&self) -> Option { match &self.transaction.transaction { Transaction::Legacy(_) => None, Transaction::Eip2930(_) => None, - Transaction::Eip1559(tx) => Some(U256::from(tx.max_priority_fee_per_gas)), + Transaction::Eip1559(tx) => Some(tx.max_priority_fee_per_gas), } } @@ -359,18 +359,18 @@ impl FromRecoveredTransaction for PooledTransaction { let (cost, effective_gas_price) = match &tx.transaction { Transaction::Legacy(t) => { let cost = U256::from(t.gas_price) * U256::from(t.gas_limit) + U256::from(t.value); - let effective_gas_price = U256::from(t.gas_price); + let effective_gas_price = t.gas_price; (cost, effective_gas_price) } Transaction::Eip2930(t) => { let cost = U256::from(t.gas_price) * U256::from(t.gas_limit) + U256::from(t.value); - let effective_gas_price = U256::from(t.gas_price); + let effective_gas_price = t.gas_price; (cost, effective_gas_price) } Transaction::Eip1559(t) => { let cost = U256::from(t.max_fee_per_gas) * U256::from(t.gas_limit) + U256::from(t.value); - let effective_gas_price = U256::from(t.max_priority_fee_per_gas); + let effective_gas_price = t.max_priority_fee_per_gas; (cost, effective_gas_price) } }; diff --git a/crates/transaction-pool/src/validate.rs b/crates/transaction-pool/src/validate.rs index d985ef0ae5..1b80a7cceb 100644 --- a/crates/transaction-pool/src/validate.rs +++ b/crates/transaction-pool/src/validate.rs @@ -101,7 +101,7 @@ impl ValidPoolTransaction { } /// Returns the EIP-1559 Max base fee the caller is willing to pay. - pub fn max_fee_per_gas(&self) -> Option { + pub fn max_fee_per_gas(&self) -> Option { self.transaction.max_fee_per_gas() }