Skip to content

batchcanon: VTXO lineage availability derivation (C5 core) - #796

Closed
ellemouton wants to merge 2 commits into
c3-batch-canonicality-managerfrom
c5-vtxo-availability-gate
Closed

batchcanon: VTXO lineage availability derivation (C5 core)#796
ellemouton wants to merge 2 commits into
c3-batch-canonicality-managerfrom
c5-vtxo-availability-gate

Conversation

@ellemouton

Copy link
Copy Markdown
Member

Summary

The reusable, fully-tested core of task C5 (darepo#454): derive per-VTXO lineage availability from batch canonicality State, and the store-driven gate logic the VTXO manager's admission path will call. Stacked on #795 (C3/C4).

What's here (pure, unit-tested)

  • Availability vocabulary (the arkrpc: validate ancestry tree_depth at indexer trust boundary (#370) #454 VTXO-lineage states, never persisted): available_final, available_provisional, available_unknown, limbo_reorg, limbo_conflict, invalidated.
  • AvailabilityForState maps one batch's State; CombineAvailability takes the worst across a multi-parent lineage (a VTXO is only as available as its least-available parent — the AND a multi-input OOR VTXO needs); Usable() is true only for confirmed lineage.
  • LineageAvailability / LineageBlocked load each parent batch from the Store and produce the combined availability / block decision the manager calls per candidate. Deliberately permissive: unseen / not-yet-registered lineage does not block — only limbo_reorg/limbo_conflict/invalidated does — so the gate is safe to enable before every producer registers its batches.

Why just the core (and what's next)

The actual wiring into vtxo.Manager.selectAndReserveVTXOs is an invasive change to a performance-sensitive selection path (the selection projection deliberately omits ancestry, so it needs a projection/query change + multi-parent lineage resolution) and the expiry-as-terminal rewire touches the per-VTXO FSM transitions — both are best validated with the daemon/itest harness. This PR lands the correctness-critical derivation logic with full unit tests; the manager/FSM wiring is the next step (precise plan in the branch's .context/reorg-safety-remaining-execplan.md).

🤖 Generated with Claude Code

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the Availability type and its associated logic to derive and manage VTXO-lineage spendability based on parent batch states, along with comprehensive unit tests. The review feedback suggests optimizing LineageAvailability by deduplicating input transaction IDs to avoid redundant store queries and adding a defensive check for nil records. Additionally, it recommends caching the rank of the worst availability in CombineAvailability to prevent redundant function calls during iteration.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +166 to +191
func LineageAvailability(ctx context.Context, store Store,
batchTxids ...chainhash.Hash) (Availability, error) {

if len(batchTxids) == 0 {
return AvailabilityUnknown, nil
}

avails := make([]Availability, 0, len(batchTxids))
for _, txid := range batchTxids {
record, err := store.GetBatch(ctx, txid)
switch {
case errors.Is(err, ErrBatchNotFound):
avails = append(avails, AvailabilityUnknown)

case err != nil:
return AvailabilityUnknown, err

default:
avails = append(
avails, AvailabilityForState(record.State),
)
}
}

return CombineAvailability(avails...), nil
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There are two potential issues in LineageAvailability:

  1. Redundant Store Queries: If batchTxids contains duplicate transaction IDs (e.g., if multiple inputs from the same batch are spent), the function will perform redundant store queries. Deduplicating the transaction IDs beforehand avoids unnecessary database I/O.
  2. Defensive Programming: If a store implementation returns a nil record with a nil error (e.g., in mock/test environments or due to an unexpected bug), accessing record.State will cause a panic. Adding a defensive nil check prevents this.
func LineageAvailability(ctx context.Context, store Store,
	batchTxids ...chainhash.Hash) (Availability, error) {

	if len(batchTxids) == 0 {
		return AvailabilityUnknown, nil
	}

	seen := make(map[chainhash.Hash]struct{}, len(batchTxids))
	avails := make([]Availability, 0, len(batchTxids))
	for _, txid := range batchTxids {
		if _, ok := seen[txid]; ok {
			continue
		}
		seen[txid] = struct{}{}

		record, err := store.GetBatch(ctx, txid)
		switch {
		case errors.Is(err, ErrBatchNotFound):
			avails = append(avails, AvailabilityUnknown)

		case err != nil:
			return AvailabilityUnknown, err

		case record == nil:
			return AvailabilityUnknown, fmt.Errorf("store returned nil record for txid %s", txid)

		default:
			avails = append(
				avails, AvailabilityForState(record.State),
			)
		}
	}

	return CombineAvailability(avails...), nil
}

Comment on lines +141 to +154
func CombineAvailability(parents ...Availability) Availability {
if len(parents) == 0 {
return AvailabilityUnknown
}

worst := parents[0]
for _, p := range parents[1:] {
if availabilityRank(p) > availabilityRank(worst) {
worst = p
}
}

return worst
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In CombineAvailability, availabilityRank(worst) is evaluated in every iteration of the loop, even though worst only changes occasionally. Since this function is called in performance-sensitive paths (such as VTXO selection), caching the rank of worst in a local variable avoids redundant function calls and improves efficiency.

Suggested change
func CombineAvailability(parents ...Availability) Availability {
if len(parents) == 0 {
return AvailabilityUnknown
}
worst := parents[0]
for _, p := range parents[1:] {
if availabilityRank(p) > availabilityRank(worst) {
worst = p
}
}
return worst
}
func CombineAvailability(parents ...Availability) Availability {
if len(parents) == 0 {
return AvailabilityUnknown
}
worst := parents[0]
worstRank := availabilityRank(worst)
for _, p := range parents[1:] {
pRank := availabilityRank(p)
if pRank > worstRank {
worst = p
worstRank = pRank
}
}
return worst
}

@litbot-9000

Copy link
Copy Markdown
Collaborator

@ellemouton, remember to re-request review from reviewers when ready

@levmi levmi added enhancement New feature or request P1 Priority 1 — high reorg safety Fund-safety: stuck, lost, or mis-counted funds vtxo labels Jul 6, 2026
Squashed for the btcd v2 port. The batchcanon.Manager actor: one
reorg-aware conf watch per batch + one spend watch per consumed input
via chainsource, derives State by priority, recomputes effective
expiry on reconfirm, and reconciles non-final watches on restart. No
admission (that is C5).
@ellemouton
ellemouton force-pushed the c3-batch-canonicality-manager branch from 007743b to 4f0707f Compare July 8, 2026 21:04
@ellemouton
ellemouton force-pushed the c5-vtxo-availability-gate branch from 3efe711 to a18c0a4 Compare July 8, 2026 21:07
Squashed for the btcd v2 port. batchcanon Availability vocab
(available_final/provisional/unknown, limbo_reorg/conflict,
invalidated) + CombineAvailability + store-driven LineageBlocked, and
the vtxo.Manager coin-selection/forfeit admission gate that drops
candidates whose batch lineage is limbo/invalidated. Permissive for
unseen/unregistered; no-op when the store is nil.
@ellemouton
ellemouton force-pushed the c3-batch-canonicality-manager branch from 4f0707f to 212e4d0 Compare July 8, 2026 22:42
@ellemouton
ellemouton force-pushed the c5-vtxo-availability-gate branch from a18c0a4 to b0e7182 Compare July 8, 2026 22:42
@ellemouton

Copy link
Copy Markdown
Member Author

Superseded by #896 as part of condensing the reorg-safety client stack (epic lightninglabs/darepo#454) from 12 PRs into 3. The commits are carried over unchanged; see #896. Branch retained as a backup.

@ellemouton ellemouton closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request P1 Priority 1 — high reorg safety Fund-safety: stuck, lost, or mis-counted funds vtxo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants