fix(chain): populate direct_anchors for non-transitive anchor reasons - #2256
Closed
harry9879 wants to merge 1 commit into
Closed
fix(chain): populate direct_anchors for non-transitive anchor reasons#2256harry9879 wants to merge 1 commit into
harry9879 wants to merge 1 commit into
Conversation
CanonicalViewTask::new only queued transactions with a transitive or
assumed CanonicalReason for the direct-anchor lookup, so a
non-transitive Anchor { descendant: None, .. } reason never landed in
direct_anchors even though its anchor was already chain-verified.
This broke resolution for an assume_canonical transaction whose
confirmation comes from a directly anchored descendant (rather than
from being an ancestor of the assumed-canonical root, which is
covered by the existing transitive-Anchor propagation): finish()
walks the descendant chain looking for an entry in direct_anchors and
found nothing, so the transaction was reported ChainPosition::Unconfirmed
instead of Confirmed.
Seed direct_anchors immediately for non-transitive Anchor reasons
since their anchor is already verified, making it visible to that
descendant search without needing a redundant chain query.
Adds a regression test covering an assumed-canonical root with a
directly anchored descendant.
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.
Summary
CanonicalViewTask::newonly queues transactions with a transitive or assumedCanonicalReasonfor the direct-anchor lookup performed viaChainQuery. A non-transitiveAnchor { descendant: None, .. }reason is skipped, even though its anchor was already chain-verified when the transaction was first canonicalized — so it never ends up indirect_anchors.This breaks resolution for an
assume_canonicaltransaction whose confirmation comes from a directly anchored descendant.finish()handles this case (an assumed root/ancestor with no anchor of its own) by walking its descendants and looking for one present indirect_anchors:If that descendant only has a non-transitive
Anchorreason (the ordinary case for a genuinely confirmed descendant that isn't itself an ancestor of the assumed root), the lookup misses and the assumed transaction incorrectly resolves toUnconfirmedinstead ofConfirmed.Note this is distinct from the already-covered case where an ancestor of the assumed root has its own anchor — in that case "assumed" propagation relabels the ancestor's reason to
Assumed { descendant: Some(root) }(transitive), which is queued and does get intodirect_anchors. The gap is specifically for descendants that keep their own ordinaryAnchorreason.Fix
Seed
direct_anchorsimmediately inCanonicalViewTask::newfor non-transitiveAnchorreasons, since their anchor is already verified and doesn't need a redundant chain query — this makes it visible to the descendant search infinish().Impact
A confirmed, spendable UTXO could be misreported as unconfirmed/untrusted when using
CanonicalizationParams::assume_canonical, causing wrong balance categorization and false negatives on confirmation/maturity checks. Direction is "too conservative" (no double-spend or fund-loss risk).bdk_walletdoesn't currently passassume_canonicalin its default flow, so this isn't reachable through typical wallet usage today, but it's a reachable bug for any downstream consumer using that public API directly.