Repository navigation
Conversation
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.
Description
The Electrum and Esplora clients record an eviction for every expected txid that is missing from its script's history in a single response. They can't tell "the transaction left the mempool" apart from "the source did not list it". Once
last_evicted >= last_seen, the transaction leaves the canonical view and drops out of balances, and the outputs it spent appear unspent again. That includes the wallet's own broadcast transactions. None of this was documented, so callers deciding whether to pass expected txids, or how to treatis_evicted() == true, had to infer the trust placed in the chain source from the implementation.This PR documents it. Documentation only: no code, signature or behaviour changes.
Fixes #2303
What changed:
SyncRequestBuilder::expected_spk_txids: new "Eviction inference" section covering how evictions are inferred, the trust placed in the source, and the trade-off of not passing expected txids.TxUpdate::evicted_ats: an entry means "not observed", not a verified eviction. Notes thatbdk_bitcoind_rpcinfers evictions differently (it comparesgetrawmempoolresults).TxNode::is_evicted: trust in the chain source, effect on the canonical view, balances and spendable outputs, and how a transaction comes back.TxGraph::insert_evicted_at,batch_insert_relevant_evicted_at, theirIndexedTxGraphcounterparts,list_expected_spk_txidsand thetx_graphmodule docs: cross-references and consistent wording.Notes to the reviewers
Two statements come from reading the implementation, not from the issue text, so please check them:
last_evictedandlast_seenstill counts as evicted (>=inTxNode::is_evicted), so a transaction returns only when it is recorded as seen strictly later.mark_canonicalmarks ancestors transitively).Whether the inference should be more conservative (for example requiring an omission across more than one sync) is a behaviour change and out of scope here. I'm happy to follow up separately.
Verified locally:
cargo +nightly fmt --all -- --checkcargo check --workspace --all-featurescargo clippy --all-features --all-targets -- -D warningsRUSTDOCFLAGS='-D warnings' cargo doc --workspace --no-depsChangelog notice
None. Documentation only.
Checklists
All Submissions:
Bugfixes: