feat: enforce replacements dont conflict (#4539)

This commit is contained in:
Matthias Seitz
2023-09-11 17:00:01 +02:00
committed by GitHub
parent 28f5118048
commit 624d9d581b
4 changed files with 65 additions and 6 deletions

View File

@@ -475,6 +475,12 @@ pub enum RpcPoolError {
/// Eip-4844 related error
#[error(transparent)]
Eip4844(#[from] Eip4844PoolTransactionError),
/// Thrown if a conflicting transaction type is already in the pool
///
/// In other words, thrown if a transaction with the same sender that violates the exclusivity
/// constraint (blob vs normal tx)
#[error("address already reserved")]
AddressAlreadyReserved,
#[error(transparent)]
Other(Box<dyn std::error::Error + Send + Sync>),
}
@@ -498,6 +504,9 @@ impl From<PoolError> for RpcPoolError {
PoolError::InvalidTransaction(_, err) => err.into(),
PoolError::Other(_, err) => RpcPoolError::Other(err),
PoolError::AlreadyImported(_) => RpcPoolError::AlreadyKnown,
PoolError::ExistingConflictingTransactionType(_, _, _) => {
RpcPoolError::AddressAlreadyReserved
}
}
}
}

View File

@@ -39,6 +39,9 @@ pub enum PoolError {
/// Thrown when the transaction is considered invalid.
#[error("[{0:?}] {1:?}")]
InvalidTransaction(TxHash, InvalidPoolTransactionError),
/// Thrown if the mutual exclusivity constraint (blob vs normal transaction) is violated.
#[error("[{1:?}] Transaction type {2} conflicts with existing transaction for {0:?}")]
ExistingConflictingTransactionType(Address, TxHash, u8),
/// Any other error that occurred while inserting/validating a transaction. e.g. IO database
/// error
#[error("[{0:?}] {1:?}")]
@@ -58,6 +61,7 @@ impl PoolError {
PoolError::DiscardedOnInsert(hash) => hash,
PoolError::InvalidTransaction(hash, _) => hash,
PoolError::Other(hash, _) => hash,
PoolError::ExistingConflictingTransactionType(_, hash, _) => hash,
}
}
@@ -110,6 +114,11 @@ impl PoolError {
// internal error unrelated to the transaction
false
}
PoolError::ExistingConflictingTransactionType(_, _, _) => {
// this is not a protocol error but an implementation error since the pool enforces
// exclusivity (blob vs normal tx) for all senders
false
}
}
}
}

View File

@@ -430,6 +430,13 @@ impl<T: TransactionOrdering> TxPool<T> {
*transaction.hash(),
InvalidPoolTransactionError::Overdraft,
)),
InsertErr::TxTypeConflict { transaction } => {
Err(PoolError::ExistingConflictingTransactionType(
transaction.sender(),
*transaction.hash(),
transaction.tx_type(),
))
}
}
}
}
@@ -1169,6 +1176,23 @@ impl<T: PoolTransaction> AllTransactions<T> {
/// Note: For EIP-4844 blob transactions additional constraints are enforced:
/// - new blob transactions must not have any nonce gaps
/// - blob transactions cannot go into overdraft
///
/// ## Transaction type Exclusivity
///
/// The pool enforces exclusivity of eip-4844 blob vs non-blob transactions on a per sender
/// basis:
/// - If the pool already includes a blob transaction from the `transaction`'s sender, then
/// the `transaction` must also be a blob transaction
/// - If the pool already includes a non-blob transaction from the `transaction`'s sender, then
/// the `transaction` must _not_ be a blob transaction.
///
/// In other words, the presence of blob transactions exclude non-blob transactions and vice
/// versa:
///
/// ## Replacements
///
/// The replacement candidate must satisfy given price bump constraints: replacement candidate
/// must not be underpriced
pub(crate) fn insert_tx(
&mut self,
transaction: ValidPoolTransaction<T>,
@@ -1225,7 +1249,7 @@ impl<T: PoolTransaction> AllTransactions<T> {
let mut replaced_tx = None;
let pool_tx = PoolInternalTransaction {
transaction: transaction.clone(),
transaction: Arc::clone(&transaction),
subpool: state.into(),
state,
cumulative_cost,
@@ -1239,13 +1263,18 @@ impl<T: PoolTransaction> AllTransactions<T> {
entry.insert(pool_tx);
}
Entry::Occupied(mut entry) => {
// Transaction with the same nonce already exists: replacement candidate
let existing_transaction = entry.get().transaction.as_ref();
let maybe_replacement = transaction.as_ref();
if existing_transaction.tx_type_conflicts_with(maybe_replacement) {
// blob vs non blob replacement
return Err(InsertErr::TxTypeConflict { transaction: pool_tx.transaction })
}
// Transaction already exists
// Ensure the new transaction is not underpriced
if Self::is_underpriced(
entry.get().transaction.as_ref(),
transaction.as_ref(),
&self.price_bumps,
) {
if Self::is_underpriced(existing_transaction, maybe_replacement, &self.price_bumps)
{
return Err(InsertErr::Underpriced {
transaction: pool_tx.transaction,
existing: *entry.get().transaction.hash(),
@@ -1418,6 +1447,8 @@ pub(crate) enum InsertErr<T: PoolTransaction> {
block_gas_limit: u64,
tx_gas_limit: u64,
},
/// Thrown if the mutual exclusivity constraint (blob vs normal transaction) is violated.
TxTypeConflict { transaction: Arc<ValidPoolTransaction<T>> },
}
/// Transaction was successfully inserted into the pool

View File

@@ -295,6 +295,16 @@ impl<T: PoolTransaction> ValidPoolTransaction<T> {
pub(crate) fn size(&self) -> usize {
self.transaction.size()
}
/// EIP-4844 blob transactions and normal transactions are treated as mutually exclusive per
/// account.
///
/// Returns true if the transaction is an EIP-4844 blob transaction and the other is not, or
/// vice versa.
#[inline]
pub(crate) fn tx_type_conflicts_with(&self, other: &Self) -> bool {
self.is_eip4844() != other.is_eip4844()
}
}
impl<T: PoolTransaction> IntoRecoveredTransaction for ValidPoolTransaction<T> {