perf(tx-pool): avoid copying tx cost (#12629)

This commit is contained in:
Hai | RISE
2024-11-20 16:56:12 +07:00
committed by GitHub
parent 11847b4f1e
commit 7b13a22698
6 changed files with 63 additions and 39 deletions

View File

@ -657,7 +657,7 @@ impl<T: TransactionOrdering> TxPool<T> {
InsertErr::Overdraft { transaction } => Err(PoolError::new(
*transaction.hash(),
PoolErrorKind::InvalidTransaction(InvalidPoolTransactionError::Overdraft {
cost: transaction.cost(),
cost: *transaction.cost(),
balance: on_chain_balance,
}),
)),
@ -1229,7 +1229,7 @@ impl<T: PoolTransaction> AllTransactions<T> {
tx.state.insert(TxState::NO_NONCE_GAPS);
tx.state.insert(TxState::NO_PARKED_ANCESTORS);
tx.cumulative_cost = U256::ZERO;
if tx.transaction.cost() > info.balance {
if tx.transaction.cost() > &info.balance {
// sender lacks sufficient funds to pay for this transaction
tx.state.remove(TxState::ENOUGH_BALANCE);
} else {
@ -1542,7 +1542,7 @@ impl<T: PoolTransaction> AllTransactions<T> {
}
}
}
} else if new_blob_tx.cost() > on_chain_balance {
} else if new_blob_tx.cost() > &on_chain_balance {
// the transaction would go into overdraft
return Err(InsertErr::Overdraft { transaction: Arc::new(new_blob_tx) })
}

View File

@ -59,6 +59,8 @@ macro_rules! set_value {
*$field = new_value;
}
}
// Ensure the tx cost is always correct after each mutation.
$this.update_cost();
};
}
@ -123,6 +125,8 @@ pub enum MockTransaction {
input: Bytes,
/// The size of the transaction, returned in the implementation of [`PoolTransaction`].
size: usize,
/// The cost of the transaction, returned in the implementation of [`PoolTransaction`].
cost: U256,
},
/// EIP-2930 transaction type.
Eip2930 {
@ -148,6 +152,8 @@ pub enum MockTransaction {
access_list: AccessList,
/// The size of the transaction, returned in the implementation of [`PoolTransaction`].
size: usize,
/// The cost of the transaction, returned in the implementation of [`PoolTransaction`].
cost: U256,
},
/// EIP-1559 transaction type.
Eip1559 {
@ -175,6 +181,8 @@ pub enum MockTransaction {
input: Bytes,
/// The size of the transaction, returned in the implementation of [`PoolTransaction`].
size: usize,
/// The cost of the transaction, returned in the implementation of [`PoolTransaction`].
cost: U256,
},
/// EIP-4844 transaction type.
Eip4844 {
@ -206,6 +214,8 @@ pub enum MockTransaction {
sidecar: BlobTransactionSidecar,
/// The size of the transaction, returned in the implementation of [`PoolTransaction`].
size: usize,
/// The cost of the transaction, returned in the implementation of [`PoolTransaction`].
cost: U256,
},
}
@ -235,6 +245,7 @@ impl MockTransaction {
value: Default::default(),
input: Default::default(),
size: Default::default(),
cost: U256::ZERO,
}
}
@ -252,6 +263,7 @@ impl MockTransaction {
gas_price: 0,
access_list: Default::default(),
size: Default::default(),
cost: U256::ZERO,
}
}
@ -270,6 +282,7 @@ impl MockTransaction {
input: Bytes::new(),
access_list: Default::default(),
size: Default::default(),
cost: U256::ZERO,
}
}
@ -290,6 +303,7 @@ impl MockTransaction {
access_list: Default::default(),
sidecar: Default::default(),
size: Default::default(),
cost: U256::ZERO,
}
}
@ -560,6 +574,19 @@ impl MockTransaction {
pub const fn is_eip2930(&self) -> bool {
matches!(self, Self::Eip2930 { .. })
}
fn update_cost(&mut self) {
match self {
Self::Legacy { cost, gas_limit, gas_price, value, .. } |
Self::Eip2930 { cost, gas_limit, gas_price, value, .. } => {
*cost = U256::from(*gas_limit) * U256::from(*gas_price) + *value
}
Self::Eip1559 { cost, gas_limit, max_fee_per_gas, value, .. } |
Self::Eip4844 { cost, gas_limit, max_fee_per_gas, value, .. } => {
*cost = U256::from(*gas_limit) * U256::from(*max_fee_per_gas) + *value
}
};
}
}
impl PoolTransaction for MockTransaction {
@ -593,16 +620,16 @@ impl PoolTransaction for MockTransaction {
*self.get_nonce()
}
fn cost(&self) -> U256 {
// Having `get_cost` from `make_setters_getters` would be cleaner but we didn't
// want to also generate the error-prone cost setters. For now cost should be
// correct at construction and auto-updated per field update via `update_cost`,
// not to be manually set.
fn cost(&self) -> &U256 {
match self {
Self::Legacy { gas_price, value, gas_limit, .. } |
Self::Eip2930 { gas_limit, gas_price, value, .. } => {
U256::from(*gas_limit) * U256::from(*gas_price) + *value
}
Self::Eip1559 { max_fee_per_gas, value, gas_limit, .. } |
Self::Eip4844 { max_fee_per_gas, value, gas_limit, .. } => {
U256::from(*gas_limit) * U256::from(*max_fee_per_gas) + *value
}
Self::Legacy { cost, .. } |
Self::Eip2930 { cost, .. } |
Self::Eip1559 { cost, .. } |
Self::Eip4844 { cost, .. } => cost,
}
}
@ -783,6 +810,7 @@ impl TryFrom<TransactionSignedEcRecovered> for MockTransaction {
value,
input,
size,
cost: U256::from(gas_limit) * U256::from(gas_price) + value,
}),
Transaction::Eip2930(TxEip2930 {
chain_id,
@ -805,6 +833,7 @@ impl TryFrom<TransactionSignedEcRecovered> for MockTransaction {
input,
access_list,
size,
cost: U256::from(gas_limit) * U256::from(gas_price) + value,
}),
Transaction::Eip1559(TxEip1559 {
chain_id,
@ -829,6 +858,7 @@ impl TryFrom<TransactionSignedEcRecovered> for MockTransaction {
input,
access_list,
size,
cost: U256::from(gas_limit) * U256::from(max_fee_per_gas) + value,
}),
Transaction::Eip4844(TxEip4844 {
chain_id,
@ -857,6 +887,7 @@ impl TryFrom<TransactionSignedEcRecovered> for MockTransaction {
access_list,
sidecar: BlobTransactionSidecar::default(),
size,
cost: U256::from(gas_limit) * U256::from(max_fee_per_gas) + value,
}),
_ => unreachable!("Invalid transaction type"),
}
@ -888,28 +919,24 @@ impl From<MockTransaction> for Transaction {
match mock {
MockTransaction::Legacy {
chain_id,
hash: _,
sender: _,
nonce,
gas_price,
gas_limit,
to,
value,
input,
size: _,
..
} => Self::Legacy(TxLegacy { chain_id, nonce, gas_price, gas_limit, to, value, input }),
MockTransaction::Eip2930 {
chain_id,
hash: _,
sender: _,
nonce,
to,
gas_limit,
input,
value,
gas_price,
gas_limit,
to,
value,
access_list,
size: _,
input,
..
} => Self::Eip2930(TxEip2930 {
chain_id,
nonce,
@ -922,17 +949,15 @@ impl From<MockTransaction> for Transaction {
}),
MockTransaction::Eip1559 {
chain_id,
hash: _,
sender: _,
nonce,
gas_limit,
max_fee_per_gas,
max_priority_fee_per_gas,
gas_limit,
to,
value,
access_list,
input,
size: _,
..
} => Self::Eip1559(TxEip1559 {
chain_id,
nonce,
@ -946,19 +971,17 @@ impl From<MockTransaction> for Transaction {
}),
MockTransaction::Eip4844 {
chain_id,
hash: _,
sender: _,
nonce,
gas_limit,
max_fee_per_gas,
max_priority_fee_per_gas,
max_fee_per_blob_gas,
gas_limit,
to,
value,
access_list,
input,
sidecar,
size: _,
max_fee_per_blob_gas,
input,
..
} => Self::Eip4844(TxEip4844 {
chain_id,
nonce,

View File

@ -961,7 +961,7 @@ pub trait PoolTransaction: fmt::Debug + Send + Sync + Clone {
/// For legacy transactions: `gas_price * gas_limit + tx_value`.
/// For EIP-4844 blob transactions: `max_fee_per_gas * gas_limit + tx_value +
/// max_blob_fee_per_gas * blob_gas_used`.
fn cost(&self) -> U256;
fn cost(&self) -> &U256;
/// Amount of gas that should be used in executing this transaction. This is paid up-front.
fn gas_limit(&self) -> u64;
@ -1228,8 +1228,8 @@ impl PoolTransaction for EthPooledTransaction {
/// For legacy transactions: `gas_price * gas_limit + tx_value`.
/// For EIP-4844 blob transactions: `max_fee_per_gas * gas_limit + tx_value +
/// max_blob_fee_per_gas * blob_gas_used`.
fn cost(&self) -> U256 {
self.cost
fn cost(&self) -> &U256 {
&self.cost
}
/// Amount of gas that should be used in executing this transaction. This is paid up-front.

View File

@ -384,11 +384,12 @@ where
let cost = transaction.cost();
// Checks for max cost
if cost > account.balance {
if cost > &account.balance {
let expected = *cost;
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::InsufficientFunds(
GotExpected { got: account.balance, expected: cost }.into(),
GotExpected { got: account.balance, expected }.into(),
)
.into(),
)

View File

@ -312,7 +312,7 @@ impl<T: PoolTransaction> ValidPoolTransaction<T> {
///
/// For EIP-1559 transactions: `max_fee_per_gas * gas_limit + tx_value`.
/// For legacy transactions: `gas_price * gas_limit + tx_value`.
pub fn cost(&self) -> U256 {
pub fn cost(&self) -> &U256 {
self.transaction.cost()
}

View File

@ -82,7 +82,7 @@ impl TransactionValidator for OkValidator {
) -> TransactionValidationOutcome<Self::Transaction> {
// Always return valid
TransactionValidationOutcome::Valid {
balance: transaction.cost(),
balance: *transaction.cost(),
state_nonce: transaction.nonce(),
transaction: ValidTransaction::Valid(transaction),
propagate: false,