multi: Register authenticated provisional lineage end to end - #990
multi: Register authenticated provisional lineage end to end#990ellemouton wants to merge 17 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
@claude review this |
|
Claude finished @ellemouton's task in 4m 34s —— View job Review: PR #990 — Register authenticated provisional lineage end to endI reviewed the full diff against Below are observations, ordered roughly by significance. None are blockers. 1.
|
e428c4d to
b1f9ab8
Compare
f44d3e6 to
1e732f1
Compare
|
Review disposition for the current head: the CSV delay is intentionally operator-authored policy input because it is not derivable from the commitment transaction; all transaction-bound fields are authenticated, and the client still requires a positive delay. The output-index error path intentionally fails closed instead of copying the legacy confirmation-watch fallback. The duplicated receive-path coverage checks and failed-then-retried metric sequence are non-blocking cleanup notes; no correctness finding was left open in this PR. |
b1f9ab8 to
9655bde
Compare
8dbd209 to
c566338
Compare
9655bde to
659e6aa
Compare
c566338 to
899c934
Compare
659e6aa to
b2a5d3d
Compare
899c934 to
9d3b6b0
Compare
b2a5d3d to
c121b30
Compare
9d3b6b0 to
095039e
Compare
49e2003 to
9d19e5d
Compare
|
@claude review this — the current head adds canonicality-aware spendable and temporarily-unavailable balance reporting plus a real reorg/reconfirmation systest. |
|
Claude finished @ellemouton's task in 2m 56s —— View job Review: canonicality-aware balance reporting + reorg systestReviewed the head commits What's correct
Observations (all non-blocking)1. Proto docs omit the
Neither mentions unseen/unregistered lineage. Since a client rendering this figure will see freshly-created-but-unconfirmed live VTXOs land here, it's worth a word in the doc so the wire contract matches the behavior the test locks in. Fix this → 2. UX consideration for the unseen window. Following from (1): a just-received round/OOR VTXO whose commitment isn't confirmed yet is 3. Minor nil-guard asymmetry. The non-nil path guards 4. Per-RPC store fan-out. Nice, tight change — the classification predicate reusing |
9d19e5d to
97bd459
Compare
095039e to
7f636e7
Compare
|
@claude review this — final head is now rebased onto current main and includes canonicality-aware temporarily-unavailable balance reporting plus the real reorg systest. |
|
Claude finished @ellemouton's task in 2m 8s —— View job Review: PR #990 — final head (rebased onto main)
Re-reviewed the full stack (7 commits) against Rebase / correctness confirmations
Non-blocking observations (status of prior notes)1. Proto docs still omit the "unseen" bucket. 2. 3. Minor nil-guard asymmetry. The non-nil path guards Note on validationI could not run The authentication layer and the reuse of |
|
@claude review this — addressed the two actionable non-blocking notes: temporary-balance docs now include unseen/unregistered lineage, and all batch registrations now enforce positive CSV expiry with regression coverage. |
|
Review disposition: temporary-balance wire docs and positive CSV registration validation are fixed in the current head. The nil-reader descriptor note is unreachable because ListLiveVTXOs does not return nil descriptors; no correctness finding is left open. |
|
Claude finished @ellemouton's task in 2m 55s —— View job Review: follow-up fixes (CSV positivity + temporary-balance docs)
Reviewed the two new commits on top of the previously-signed-off stack. Both notes are addressed correctly. One small formatting issue that will fail CI. ✅ CSV-expiry positivity (
|
37ab56c to
6c63040
Compare
9731c80 to
e3bcacb
Compare
b541977 to
58d2862
Compare
e3bcacb to
f31cccf
Compare
58d2862 to
2d07c20
Compare
f31cccf to
dbadb1b
Compare
Adapts #990's reorg lineage/OOR-gating code to the current lint gates: drops a stray duplicated godoc line left by a merge on LineageCommitmentTxIDs, and adds justified funlen/nestif directives where main's growth plus the canonicality gate pushed the incoming-VTXO dispatch and custom-input path past the thresholds. No functional changes.
2d07c20 to
811794b
Compare
Adapts #990's reorg lineage/OOR-gating code to the current lint gates: drops a stray duplicated godoc line left by a merge on LineageCommitmentTxIDs, and adds justified funlen/nestif directives where main's growth plus the canonicality gate pushed the incoming-VTXO dispatch and custom-input path past the thresholds. No functional changes.
811794b to
3cd0109
Compare
📚 Per-package doc driftThis PR's Go changes left the per-package docs stale for
Proposed changes — diff of the 11
|
dbadb1b to
6b3483a
Compare
Build complete batch evidence from the retained commitment PSBT when a round confirms. Await durable canonicality registration before saving any new VTXO so the fail-closed admission gate never observes unregistered liquidity. Refresh registrations bind each consumed VTXO to its next lifecycle revision and complete creator lineage, allowing a terminally replaced round to restore only the exact forfeiture it owns.
Carry the commitment transaction and every input prevout in ancestry responses. Validate the transaction, output, and input bindings before receive paths register canonicality watches.
Persist authenticated evidence in the durable receive event and await canonicality registration before staging incoming VTXOs. Legacy indexers may omit the full additive extension, while partial or conflicting evidence fails closed.
Carry authenticated evidence through the round-receive ancestry lookup. Await canonicality registration before saving or notifying the VTXO manager. Missing legacy evidence remains blocked by the gate.
Custom OOR inputs bypass wallet coin selection and its fail-closed lineage gate. Reuse the full-lineage reader before maturity checks so reorged or invalidated inputs never reach checkpoint signing.
Expose reversible lineage limbo separately from spendable and pending VTXO balances so clients can explain why value is temporarily unusable.
Keep reorged, conflicted, unseen, and reconciling lineage out of the spendable figure while preserving it in a temporary-unavailability bucket. Exercise the classification across a real reorg and reconfirmation.
Reject malformed local or imported registration evidence before it can produce a height-derived expiry at the confirmation block itself.
Include unseen and unregistered lineage in the balance field contract so clients can explain the same fail-closed bucket the daemon reports.
Pass the local round start height and indexed commitment height into batch canonicality registration. Persist the indexer hint through OOR durable messages so delayed and replayed receive paths scan across already-mined confirmations.
The wired VTXO admission gate reads lineage availability straight from the durable store via LineageBlocked. When a reorg/conflict observation could not be persisted (a transient ApplyObservation failure), the in-memory watch was marked not-ready but the stale durable row stayed Ready(), so the gate kept admitting a VTXO whose commitment had just left the best chain. The overlay the code comments promised only ever protected the unwired actor QueryLineage path. Make the manager itself a batchcanon.Reader whose GetBatch overlays the in-memory watch: a not-ready watch forces the returned record not-ready, even when the durable row still reads Ready. Thread the manager (not the raw store) into the gate. The overlay only downgrades ready->not-ready, so it is strictly fail-closed. Add a wired-path test proving admission closes after a failed reorg persist.
A consumer batch can reach a terminal state (ConflictFinalized) before its ForfeitedBy(consumer) business-revision marker is durably persisted: the round FSM registers the business revision before the outbox delivers ForfeitConfirmedToVTXO, which is what drives MarkForfeited. When the terminal transition wins that race, restoreProvisionalConsumers runs the restore compare-and-swap against a VTXO not yet at the expected revision, so it defers. No further batchcanon event is then guaranteed on the consumer, leaving the consumed VTXO stranded until the next restart redrive. Have the VTXO actor send ConsumerForfeitPersistedMsg the moment MarkForfeited succeeds, carrying the consuming batch. The manager redrives that consumer's terminal lifecycle, gated on it being Ready and terminal, so the marker's arrival becomes the evidence change that completes the deferred restore without a restart. The redrive is wired as an optional VTXO-manager callback, so harness paths and deployments without batch canonicality are unaffected.
The receive paths authenticated the commitment transaction (self-hash)
and its batch output, but never proved the received VTXO actually
descends from it. Nothing tied the pre-signed tree to that authenticated
output, so a malicious or buggy indexer could name a decoy commitment
(real, deeply buried, never reorging) and have the client arm its reorg
watch there while the VTXO's true lineage silently left the chain.
Running the tree signature check alone does not close this: signatures
verify against a per-node aggregate key recomputed from the tree's own
(indexer-supplied) cosigners, so a fabricated tree signed with the
attacker's own keys passes. The load-bearing check is that each node's
aggregate key must equal the taproot key of the output it spends. At the
root that output is the authenticated commitment output, so the root key
is pinned to the real round key (unforgeable); taproot's SIGHASH_DEFAULT
commits to each tx's outputs, so the pin cascades to every leaf
(verifyAuthenticatedTree). It is key-identity-agnostic, so it never
demands the operator identity key and never false-rejects a genuine
tree.
Two receive paths consume it:
- In-round (waved/incoming_ancestry_fetcher):
VerifyReceivedVTXOBinding
runs verifyAuthenticatedTree then requires a leaf to pay the
received
VTXO script (an in-round VTXO IS a commitment-tree leaf).
- OOR (oor/incoming_batch_registration): an OOR output is an ark-tx
output, not a tree leaf, so VerifyOORAncestryLineage binds the
coin's
real base inputs instead. It re-anchors each tree's batch output on
the validated evidence (OOR metadata can be a durable replay), runs
verifyAuthenticatedTree, and requires every base coin input to be an
authenticated ancestry leaf. BaseCoinInputs derives those by walking
the coin's authenticated checkpoint/ark chain (root
FinalCheckpointPSBTs + AncestorPackages) down to the checkpoints
whose
prevouts are the committed leaves -- uniform for single- and
multi-hop receives. The coin inputs are trustworthy: each checkpoint
txid is committed by the ark txid, which equals the coin's own
outpoint/sessionID (ValidateFinalizePackage), so an attacker cannot
substitute them. This closes the genuine-but-unrelated (decoy)
commitment variant. The bind never keys off the indexer-supplied
chain depth (inflatable to skip). Injected via
IncomingLineageVerifier
so registration-logic tests keep mock trees; also wired defensively
into the recovery handler so a future registrar cannot bypass it.
On failure both paths fail closed (drop ancestry / fail registration),
so no watch is armed on an unverified commitment; the VTXO still
persists
for cooperative use and its exit material is restored by a later
backfill. OOR skips the check when evidence is absent (older indexer).
When a tracked tx's confirmation is reorged out and later reconfirms, the terminal-notification idempotency key must change so a durable subscriber does not drop the re-confirmation as a duplicate of the pre-reorg one. terminalNotifyKey was stable across reorg epochs, so a reorg-aware consumer that rolled its state back on TxReorged -- e.g. an in-progress unilateral exit waiting to re-drive -- never received the re-confirmation and stalled indefinitely (funds safe on-chain but the exit never swept). Fold a per-entry reorg epoch (incremented in handleConfirmationReorged) and the last confirmation height into terminalNotifyKey. Same-epoch, same-height retries still dedup; a reorg-and-reconfirmation now carries a fresh key and is delivered. Adds a regression test that fails without the epoch increment.
Reflow lines flagged by the ll linter across the reorg lineage-producer files. Formatting only, no behavior change.
TestGetBatchReadsUnderLock swapped mgr.cfg.Store on the test goroutine while the actor was still processing the spend observation, which reads cfg.Store on its own goroutine -- a data race the -race unit job caught. Add a synchronizing Ask so the FIFO mailbox drains before the swap.
Adapts #990's reorg lineage/OOR-gating code to the current lint gates: drops a stray duplicated godoc line left by a merge on LineageCommitmentTxIDs, and adds justified funlen/nestif directives where main's growth plus the canonicality gate pushed the incoming-VTXO dispatch and custom-input path past the thresholds. No functional changes.
3cd0109 to
928eadb
Compare
|
@ellemouton, remember to re-request review from reviewers when ready |
What this PR does
Connects the real VTXO creation and receipt paths to the
batchcanonauthorityintroduced by #980. Without this PR the authority exists but production paths do
not consistently feed it.
It handles three production sources:
The indexer ancestry response supplies the serialized commitment transaction,
ordered commitment prevouts, CSV-delay evidence, and the parent lineage required
to validate a received VTXO. The recipient treats that response as untrusted
and authenticates: the commitment txid, the referenced output, every commitment
input, the input prevouts and their ordering, and the relationship between the
received VTXO and the supplied batch.
Registration happens before the VTXO is saved or exposed:
sequenceDiagram participant R as OOR receiver participant I as Indexer participant B as batchcanon participant V as VTXO store R->>I: Request ancestry I-->>R: Commitment, prevouts, and lineage R->>R: Authenticate evidence R->>B: Register complete lineage B-->>R: Registration durable R->>V: Materialize and save VTXOOnce registered, the client responds to later reorgs using its local chain
watches; it does not requery the indexer.
Wallet reporting uses the same authority: a lifecycle-live VTXO whose lineage is
reorged, conflicted, unseen, or still reconciling is excluded from spendable
balance and included in
temporarily_unavailable_sat. Terminally invalidatedvalue is excluded from both buckets.
Review focus
Full design & diagrams:
REORG_SAFETY_PR_REVIEW_GUIDE.md§4.3. Overallarchitecture and PR stack: lumos#454.