fix(pool): add back already known check after refactor

crates/pool/src/mempool/pool.rs

@@ -97,7 +97,16 @@ impl PoolInner {
         }
     }
 
-    pub(crate) fn check_replacement_fees(&self, op: &UserOperation) -> MempoolResult<()> {
+    /// Returns hash of operation to replace if operation is a replacement
+    pub(crate) fn check_replacement(&self, op: &UserOperation) -> MempoolResult<Option<H256>> {
+        // 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());
@@ -106,12 +115,19 @@ impl PoolInner {
                 || op.max_fee_per_gas < replacement_fee
             {
                 return Err(MempoolError::ReplacementUnderpriced(
-                    replacement_priority_fee,
-                    replacement_fee,
+                    pool_op.uo().max_priority_fee_per_gas,
+                    pool_op.uo().max_fee_per_gas,
                 ));
             }
+
+            Ok(Some(
+                pool_op
+                    .uo()
+                    .op_hash(self.config.entry_point, self.config.chain_id),
+            ))
+        } else {
+            Ok(None)
         }
-        Ok(())
     }
 
     pub(crate) fn add_operation(&mut self, op: PoolOperation) -> MempoolResult<H256> {
@@ -226,34 +242,10 @@ 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
+        // if replacing, remove the existing operation
+        if let Some(hash) = self.check_replacement(&op.uo)? {
+            self.remove_operation_by_hash(hash);
         }
 
         // Check sender count in mempool. If sender has too many operations, must be staked

crates/pool/src/mempool/uo_pool.rs

@@ -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?;
@@ -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,

crates/pool/src/server/remote/client.rs

@@ -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>,