MempoolTxsDetector is only fed with mempool txs.

[ZanCorDX]

📝 Summary

MempoolTxsDetector used to consider as mempool any MempoolTx but those coming from eth_sendRawTransaction should not be considered as mempool.
MempoolTxsDetector was changed, now it's a long lived object.
We insert txs into MempoolTxsDetector on the read mempool tasks and clean it on the OrderPool to avoid leaks.

✅ I have completed the following steps:

• Run make lint
• Run make test
• Added tests (if applicable)

[claude]

Claude finished @ZanCorDX's task in 1m 15s —— View job

---

PR Review: MempoolTxsDetector refactor

• Read diff and changed files
• Analyze code for bugs, edge cases, and concurrency issues
• Post review findings

Summary

The refactoring direction is sound — making MempoolTxsDetector a long-lived shared object and feeding it only from actual mempool sources (not eth_sendRawTransaction) is the right approach. The removal of ReplaceableOrderStreamSniffer simplifies the pipeline nicely.

Issues found

1. Bug — leak in try_send_to_orderpool (mod.rs:555): add_tx is called before the orderpool send, so if the send fails the hash leaks in the DashSet forever (never cleaned by head_updated). The txpool_fetcher.rs path correctly adds only on success — this path should match.

2. block_building_helper_stats_logger.rs still treats all txs as mempool (lines 98-101): A fresh local MempoolTxsDetector is created and all Order::Tx are added to it — including private txs. This defeats the purpose of the PR for stats logging. Should likely use the shared detector from BlockBuildingContext.

See inline comments for details.

[claude]

This creates a fresh MempoolTxsDetector and feeds it all Order::Tx orders — including private txs from eth_sendRawTransaction. This contradicts the PR's goal of distinguishing mempool txs from private txs.

Should this use the shared MempoolTxsDetector from BlockBuildingContext (i.e. block_ctx.mempool_tx_detector) instead of constructing a local one?

[claude]

Nit: unrelated whitespace change (blank line removed). Consider reverting to keep the diff focused.

[claude]

Claude finished @ZanCorDX's task in 1m 54s —— View job

---

PR Review: MempoolTxsDetector refactor (re-review after sync)

• Read updated diff and changed files
• Check if previous review issues were addressed
• Post review findings

Summary

The second commit (4c5b074) attempted to address the try_send_to_orderpool bug from the first review, but introduced a new issue:

Issues

1. Bug — duplicated code in try_send_to_orderpool (mod.rs:555-566): The new code with add_tx/remove_tx was added but the old code block was not removed, resulting in duplicated logic. This will fail to compile since tx is moved into MempoolTx::new(tx) on the first block and then reused on the second block. See inline comment.

2. block_building_helper_stats_logger.rs still creates a local detector (lines 98-104): A fresh MempoolTxsDetector is created and all Order::Tx hashes are added — including private txs. This still misclassifies private txs as mempool txs for stats purposes. See inline comment.

[claude]

Still creates a fresh local MempoolTxsDetector and adds all Order::Tx — including private txs from eth_sendRawTransaction. This means stats logging will misclassify private txs as mempool txs, defeating the purpose of this PR for the stats path.

Should use the shared detector from BlockBuildingContext (accessible via self.block_building_helper) instead of constructing a local one.

[claude]

Bug: The new code block (lines 555-561) was added but the old code block (lines 562-566) was not removed. This duplicates the logic and will fail to compile — tx is moved into MempoolTx::new(tx) on line 556, making lines 559 (tx.hash()) and 562 (MempoolTx::new(tx)) invalid.

Fix: remove lines 562-566 (the old code) and save the hash before moving tx:

let hash = tx.hash();
mempool_detector.add_tx(hash);
let order = Order::Tx(MempoolTx::new(tx));
let command = ReplaceableOrderPoolCommand::Order(Arc::new(order));
if let Err(e) = orderpool_sender.send(command).await {
    mempool_detector.remove_tx(hash);
    error!("Error sending order to orderpool: {:#}", e);
}

[claude]

Claude finished @ZanCorDX's task in 1m 20s —— View job

---

PR Review: MempoolTxsDetector refactor (re-review after latest sync)

• Read updated diff and changed files
• Check if previous review issues were addressed
• Post review findings

Summary

The latest commit (23e866a) addressed both issues from previous reviews:

1. try_send_to_orderpool bug — fixed. The duplicated code block is gone. The hash is now captured before moving tx, add_tx is called before send, and remove_tx is called on failure. Clean pattern.

2. block_building_helper_stats_logger.rs — fixed. Now uses the shared MempoolTxsDetector from BlockBuildingContext instead of creating a local one that misclassified private txs.

Remaining issue

Backtest path still adds all Order::Tx as mempool (backtest/execute.rs:104-108): The @Pending comment acknowledges this, but backtest_prepare_orders_from_building_context still treats every Order::Tx as a mempool tx — including private txs. This means backtest stats will misclassify private txs as mempool txs. Not a blocker if backtest data doesn't currently include private txs, but worth tracking.

Nit

• txpool_fetcher.rs:73: Unrelated whitespace removal (blank line between }; and let tx = ...). Minor, but keeping diffs focused helps reviewers.

Overall the core changes look correct. The refactoring direction is sound — the shared long-lived MempoolTxsDetector fed only from mempool sources, with cleanup in OrderPool::head_updated, is a clean approach.