Repository navigation
mempool: don't record a txid as seen until it has been fetched - #609
Draft
skazistp-cpu wants to merge 1 commit into
Draft
skazistp-cpu wants to merge 1 commit into
skazistp-cpu wants to merge 1 commit into
Conversation
Author
|
For context: f4ef75b fixed the same problem for |
g_txidSeen and g_txList are shared by every GetMempoolStream client, but refreshMempoolTxns runs with the context of whichever client's goroutine happens to do the refresh. It marked each txid as seen before fetching it, and treated any getrawtransaction error as "the transaction left the mempool". Since zcash#564 made RawRequest honor that context, a client that disconnects during a refresh makes every remaining getrawtransaction fail at once. Those txids are then recorded as seen and never fetched, so no other client receives them until the next block. Before zcash#564 the same happened to any transaction whose fetch hit a transient RPC error. Mark a txid as seen only once it has been fetched. If the refreshing client's context is done, stop and return its error. Any other fetch error leaves the txid unseen, so it is retried if it is still listed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
skazistp-cpu
force-pushed
the
fix/mempool-refresh-cancel-poisons-seen
branch
from
October 4, 2026 11:14
b5d93fe to
fa8972c
Compare
skazistp-cpu
marked this pull request as draft
October 4, 2026 11:17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #615.
When one
GetMempoolStreamclient disconnects during a mempool refresh, the transactions that refresh hadn't reached yet are never delivered to any client until the next block.Problem
g_txidSeenandg_txListare shared by every mempool stream.refreshMempoolTxnsruns on whichever client's goroutine gets there first, using that client'sctx. For each new txid, it:g_txidSeen, before fetching itgetrawtransaction, and on any error doescontinue(the comment reads "mempool transactions can disappear")Since #564 (
98f38efb, "make RawRequest context-aware"), a cancelledctxaborts the HTTP request immediately. If the refreshing client goes away partway through, every remaininggetrawtransactionfails at once. Each of those txids has already been recorded as seen, so later refreshes, whichever client triggers them, skip them. The other connected wallets never get those pending transactions (incoming payments, for example) until the block that mines them resets the set.Mobile wallets connect and drop mempool streams constantly. A refresh makes one sequential RPC per new txid while holding
g_lock, so there's a real window for this.Before #564, the same permanent skip already happened to any tx whose fetch hit a transient error, such as a timeout or a busy node.
Fix
A txid is recorded in
g_txidSeenonly aftergetrawtransactionsucceeds. If the fetch fails because the refreshing client'sctxis done, the refresh stops and returns that error without recording anything. Any other failure leaves the txid unseen:getrawmempool, so nothing more happens.A fetched transaction that fails to parse is still marked seen, as before, so a bad transaction isn't retried every 2 seconds.
GetMempool's loop and tip handling are unchanged.Tests
New file
common/mempool_refresh_test.go:masterreturned <nil>, want context.Canceledandmempool list has 1 transactions, want 3getrawtransactionfails transiently, then the next refresh runssecond refresh has 2 transactions, want 3go test ./...andgo test -race ./common/pass, andgofmtis clean.go test -count=2 ./common/fails onmasteras well as on this branch, because the existing tests share state across runs, so I ran every check with-count=1.Prepared with AI assistance.
— Miroslav, Sebrona