Skip to content

Commit

Permalink
fix(pool): add back already known check after refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
dancoombs committed Oct 14, 2023
1 parent d6b825c commit b8fafb2
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 33 deletions.
41 changes: 11 additions & 30 deletions crates/pool/src/mempool/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,15 @@ impl PoolInner {
}
}

pub(crate) fn check_replacement_fees(&self, op: &UserOperation) -> MempoolResult<()> {
pub(crate) fn check_replacement(&self, op: &UserOperation) -> MempoolResult<()> {
// Check if operation already known
if self
.by_hash
.contains_key(&op.op_hash(self.config.entry_point, self.config.chain_id))
{
return Err(MempoolError::OperationAlreadyKnown);
}

if let Some(pool_op) = self.by_id.get(&op.id()) {
let (replacement_priority_fee, replacement_fee) =
self.get_min_replacement_fees(pool_op.uo());
Expand Down Expand Up @@ -226,35 +234,8 @@ impl PoolInner {
op: Arc<PoolOperation>,
submission_id: Option<u64>,
) -> MempoolResult<H256> {
// Check if operation already known
if self
.by_hash
.contains_key(&op.uo.op_hash(self.config.entry_point, self.config.chain_id))
{
return Err(MempoolError::OperationAlreadyKnown);
}

// Check for replacement by ID
if let Some(pool_op) = self.by_id.get(&op.uo.id()) {
let (replacement_priority_fee, replacement_fee) =
self.get_min_replacement_fees(pool_op.uo());

// replace only if higher gas
if op.uo.max_priority_fee_per_gas >= replacement_priority_fee
&& op.uo.max_fee_per_gas >= replacement_fee
{
self.remove_operation_by_hash(
pool_op
.uo()
.op_hash(self.config.entry_point, self.config.chain_id),
);
} else {
return Err(MempoolError::ReplacementUnderpriced(
pool_op.uo().max_priority_fee_per_gas,
pool_op.uo().max_fee_per_gas,
));
}
}
// Check if operation already known or replacing an existing operation
self.check_replacement(&op.uo)?;

// Check sender count in mempool. If sender has too many operations, must be staked
if *self.count_by_address.get(&op.uo.sender).unwrap_or(&0)
Expand Down
68 changes: 66 additions & 2 deletions crates/pool/src/mempool/uo_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,9 @@ where
);
}

// Check if op is replacing another op, and if so, ensure its fees are high enough
self.state.read().pool.check_replacement_fees(&op)?;
// Check if op is already known or replacing another, and if so, ensure its fees are high enough
// do this before simulation to save resources
self.state.read().pool.check_replacement(&op)?;

// Prechecks
self.prechecker.check(&op).await?;
Expand Down Expand Up @@ -722,6 +723,69 @@ mod tests {
assert_eq!(pool.best_operations(1, 0).unwrap(), vec![]);
}

#[tokio::test]
async fn test_already_known() {
let op = create_op(Address::random(), 0, 0);
let pool = create_pool(vec![op.clone()]);

let _ = pool
.add_operation(OperationOrigin::Local, op.op.clone())
.await
.unwrap();

let err = pool
.add_operation(OperationOrigin::Local, op.op.clone())
.await
.unwrap_err();
assert!(matches!(err, MempoolError::OperationAlreadyKnown));

check_ops(pool.best_operations(1, 0).unwrap(), vec![op.op]);
}

#[tokio::test]
async fn test_replacement_underpriced() {
let op = create_op(Address::random(), 0, 100);
let pool = create_pool(vec![op.clone()]);

let _ = pool
.add_operation(OperationOrigin::Local, op.op.clone())
.await
.unwrap();

let mut replacement = op.op.clone();
replacement.max_fee_per_gas = replacement.max_fee_per_gas + 1;

let err = pool
.add_operation(OperationOrigin::Local, replacement)
.await
.unwrap_err();

assert!(matches!(err, MempoolError::ReplacementUnderpriced(_, _)));

check_ops(pool.best_operations(1, 0).unwrap(), vec![op.op]);
}

#[tokio::test]
async fn test_replacement() {
let op = create_op(Address::random(), 0, 5);
let pool = create_pool(vec![op.clone()]);

let _ = pool
.add_operation(OperationOrigin::Local, op.op.clone())
.await
.unwrap();

let mut replacement = op.op.clone();
replacement.max_fee_per_gas = replacement.max_fee_per_gas + 1;

let _ = pool
.add_operation(OperationOrigin::Local, replacement.clone())
.await
.unwrap();

check_ops(pool.best_operations(1, 0).unwrap(), vec![replacement]);
}

#[derive(Clone, Debug)]
struct OpWithErrors {
op: UserOperation,
Expand Down
2 changes: 1 addition & 1 deletion crates/pool/src/server/remote/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl RemotePoolClient {
}

// Handler for the new block subscription. This will attempt to resubscribe if the gRPC
// connection disconnects using expenential backoff.
// connection disconnects using exponential backoff.
async fn new_heads_subscription_handler(
client: OpPoolClient<Channel>,
tx: mpsc::UnboundedSender<NewHead>,
Expand Down

0 comments on commit b8fafb2

Please sign in to comment.