Skip to content

Commit 4cef7da

Browse files
pragmaximclaude
andcommitted
fix(eth): clear the wrapped mempool on alt-cache read-path eviction
GetTransaction's read-path staleness eviction called RemoveTransaction, which clears only the alternative-provider cache, whereas every reconcile/RBF eviction routes through removeMempoolTx and its removeTransactionFromMempool delegate that also clears the wrapped Blockbook mempool's per-address index. The gap was masked because the caller (EthereumRPC.GetTransaction) then queries the primary RPC and, on a null result, cleans the mempool itself - but when that primary eth_getTransactionByHash call errors instead of returning null, it returns early without cleanup, leaving the expired private tx listed as pending for the address until the 10-minute mempool sweep. Route the read-path eviction through removeMempoolTx like the others. Safe from re-entrancy: GetTransaction holds no mempool lock when calling the provider, and MempoolEthereumType.RemoveTransactionFromMempool takes only its own mutex. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a427eef commit 4cef7da

2 files changed

Lines changed: 39 additions & 5 deletions

File tree

bchain/coins/eth/alternativesendtx.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -427,11 +427,13 @@ func (p *AlternativeSendTxProvider) GetTransaction(txid string) (*bchain.RpcTran
427427
if found {
428428
if time.Unix(int64(storedTx.time), 0).Before(time.Now().Add(-p.mempoolTxsTimeout)) {
429429
// the same staleness timeout the reconcile loop applies, just reached on the read path
430-
// first; record it so the timeout counter and residence histogram do not undercount
431-
// entries read after expiry but before the next reconcile cycle evicts them - but only
432-
// if this read is the one that removed the entry, so a concurrent reconcile eviction of
433-
// the same expired tx does not also count it.
434-
if p.RemoveTransaction(txid) {
430+
// first; route it through removeMempoolTx (not RemoveTransaction) so the wrapped
431+
// Blockbook mempool's per-address index is cleared too - otherwise, when the caller's
432+
// own primary-RPC lookup errors instead of returning null, the expired private tx keeps
433+
// being listed as pending for the address until the 10-minute mempool sweep. Record the
434+
// exit only if this read is the one that removed the entry, so a concurrent reconcile
435+
// eviction of the same expired tx does not also count it.
436+
if p.removeMempoolTx(txid) {
435437
p.observeMempoolReconciliation("timeout")
436438
p.observeMempoolTxResidence("timeout", storedTx.time)
437439
}

bchain/coins/eth/alternativesendtx_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1401,3 +1401,35 @@ func TestAlternativeSendTxProviderHandleMempoolTransactionSkipsStaleFetchBack(t
14011401
t.Fatal("newer replacement was evicted by a stale older-generation fetch-back")
14021402
}
14031403
}
1404+
1405+
// TestAlternativeSendTxProviderGetTransactionTimeoutCleansWrappedMempool verifies the read-path
1406+
// staleness eviction routes through the removeTransactionFromMempool delegate (which clears the
1407+
// wrapped Blockbook mempool's address index), not the cache-only RemoveTransaction. Before the fix
1408+
// an expired private tx lingered in the wrapped mempool whenever the caller's own primary-RPC
1409+
// lookup errored instead of returning null (finding #3).
1410+
func TestAlternativeSendTxProviderGetTransactionTimeoutCleansWrappedMempool(t *testing.T) {
1411+
var removed string
1412+
provider := &AlternativeSendTxProvider{
1413+
fetchMempoolTx: true,
1414+
mempoolTxsTimeout: time.Minute,
1415+
mempoolTxs: map[string]storedTx{
1416+
testAlternativeTxID: {
1417+
tx: &bchain.RpcTransaction{Hash: testAlternativeTxID, From: "0x2222222222222222222222222222222222222222", AccountNonce: "0x1"},
1418+
time: uint32(time.Now().Add(-2 * time.Minute).Unix()), // already past mempoolTxsTimeout
1419+
},
1420+
},
1421+
metrics: newReconcileTestMetrics(),
1422+
}
1423+
// stands in for EthereumRPC.removeTransactionFromMempool, which clears b.Mempool too
1424+
provider.removeTransactionFromMempool = func(txid string) { removed = txid; provider.RemoveTransaction(txid) }
1425+
1426+
if _, found := provider.GetTransaction(testAlternativeTxID); found {
1427+
t.Fatal("expired tx returned as found")
1428+
}
1429+
if removed != testAlternativeTxID {
1430+
t.Fatalf("removeTransactionFromMempool delegate not invoked on read-path timeout; removed=%q, want %q", removed, testAlternativeTxID)
1431+
}
1432+
if _, found := provider.mempoolTxs[testAlternativeTxID]; found {
1433+
t.Fatal("expired tx remained in provider cache")
1434+
}
1435+
}

0 commit comments

Comments
 (0)