vtxo: widen onchain spend tracking to onchain-kind vtxos - #1184
vtxo: widen onchain spend tracking to onchain-kind vtxos#1184bitcoin-coder-bob wants to merge 10 commits into
Conversation
Adds a vtxo_kind column (0 = offchain, 1 = onchain) so an on-chain Arkade UTXO (issue #1159) can be told apart from a batch leaf or offchain-tx output. Without it, such a vtxo misclassifies: empty commitment fields read as a note, and the sweeper would treat SpentBy as a checkpoint txid. - domain.Vtxo gains a Kind field; IsNote() keys off it so an on-chain vtxo with empty commitments is not a note. - sqlite + postgres migrations add the column (DEFAULT 0 backfills all history as offchain) and recreate vtxo_vw / intent_with_inputs_vw so it is visible. badger persists it via gob with no migration; old records decode to offchain. - UpsertVtxo and rowToVtxo carry Kind on both SQL backends. Chosen as an explicit enum over a bool so future on-chain sub-kinds need no further migration. Behaviour-preserving: nothing writes Onchain yet, so every existing row is offchain and all classifiers evaluate as before. The protective read-guards (sweeper/indexer filters) land with the on-chain write path, where they are testable against real on-chain rows. Part of #1159.
rowToVtxo on both SQL backends already carried Kind, but the round-replay converter (combinedRowToVtxo) and the marker converters built domain.Vtxo values without it, so a vtxo read through those paths came back as VtxoKindOffchain regardless of its stored kind. Any guard keyed on Kind downstream of round replay or marker preload would have been silently wrong. Note combinedRowToVtxo also drops Depth and MarkerIDs, which predates this work. Left alone here rather than widening the change, but it means round replay does not reconstruct the DAG fields either.
An on-chain Arkade UTXO has no batch expiry, so ExpiresAt is not meaningful for it and is left zero. Without this guard IsExpired would compare against the Unix epoch and report every on-chain vtxo as expired, which cascades through RequiresForfeit and the two spend-path checks in service.go and would make them permanently unspendable. Behaviour-preserving today: nothing writes VtxoKindOnchain yet, so every existing row is offchain and evaluates exactly as before.
An on-chain Arkade UTXO joins a batch as a boarding input, signed directly and never forfeited. IsNote already excludes the kind, which flipped RequiresForfeit to true for it, so guard it explicitly alongside IsNote and IsExpired.
Same up, down, re-apply sequence on both SQL backends, against a database the test owns on the shared test server.
Closes #1181. Part of #1159, follow-up to #1174 on top of #1161. #1174 tracks the onchain spends of unrolled vtxos, the only vtxos with an onchain output until now. With #1161 arkd also records vtxos held in on-chain Arkade UTXOs, Kind = Onchain, whose unilateral spends bypass arkd exactly as an unrolled vtxo's do. Every predicate the tracking keyed on unrolled now takes either shape: - Vtxo.HasOnchainOutput is the one Go predicate, unrolled or onchain kind, and IsOnchainSpent builds on it. - The five sql statements, mark, re-point, retract and the two candidate selectors, match (unrolled = true OR vtxo_kind = 1) on sqlite and postgres. - The badger selectors take both shapes and its mark guard uses the predicate. - applyOnchainSpends and the restore loop in restoreWatchingVtxos follow, the latter through the widened selectors. SelectSweepableUnrolledVtxos stays as it is: an onchain-kind UTXO has no checkpoint tx for the sweeper to resolve, and its ark_txid predicate already keeps onchain spends out. Tests, on all three backends with Kind = Onchain rows inserted directly so nothing waits for the write path: mark, re-point and retract work on an onchain-kind row, the selectors partition by kind as they do by unrolled, and an onchain-kind row never enters the sweepable set. The IsOnchainSpent table and the applyOnchainSpends unit test gain the kind cases. Removing the kind from the predicate fails the domain and application cases; the sql cases cannot pass on the old statements, which never matched a row that was not unrolled.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
arkana-ai-bot
left a comment
There was a problem hiding this comment.
Arkana review — widen onchain spend tracking to onchain-kind vtxos
This PR extends the onchain spend tracking and domain logic to cover VtxoKindOnchain vtxos (part of issue #1159), building on the VtxoKind discriminator introduced in #1161.
Domain changes are correct:
HasOnchainOutput()correctly captures both unrolled vtxos and onchain-kind vtxos. Using this helper consistently in place of rawUnrolledchecks keeps the two categories in sync.IsNote(): adding theKind != VtxoKindOnchainguard is necessary — an on-chain Arkade UTXO has no commitment txids, which would otherwise make it read as a note.IsExpired(): guarding on-chain kind is critical — a zeroExpiresAt(no batch expiry) would read as permanently expired, making the vtxo appear unspendable everywhere.RequiresForfeit(): on-chain kind vtxos join a batch as boarding inputs and are signed directly, never forfeited. The guard is correct.IsOnchainSpent(): updated to useHasOnchainOutput()— correct.
Application layer:
applyOnchainSpendsswitches toHasOnchainOutput()— correct.restoreWatchingVtxoslog message and comment updated — no logic change.
Test coverage is thorough: each modified method has a new test case for VtxoKindOnchain.
This is stacked on #1161 (VtxoKind discriminator). Flagging for human review as it touches VTXO domain logic and sweep/spend tracking. Looks correct. ✓
Closes #1181. Part of #1159, Milestone 5. Follow-up to #1174 on top of #1161.
Important
Stacked. Based on
feat/unrolled-vtxo-onchain-spend(#1174) withbob/onchain-arkade-vtxo-kind(#1161) merged in, so the diff shows #1161's commits through the merge until both land on master. The change itself is the last commit. Once #1161 and #1174 merge, this rebases onto master and the merge commit goes away.Context
#1174 tracks the onchain spends of unrolled vtxos, the only vtxos with an onchain output until now. Its candidate selectors, its apply path and its restore-on-restart loop are all scoped to
unrolled = true, andVtxo.IsOnchainSpentderives "spent onchain" from unrolled plus the absence of an in-Ark spend marker.With #1161 arkd also records vtxos held in on-chain Arkade UTXOs,
Kind = Onchain. Their unilateral spends bypass arkd exactly as an unrolled vtxo's do, and need the same detection, retraction and guards.What changed
Every predicate the tracking keyed on unrolled now takes either shape.
Vtxo.HasOnchainOutputis the one Go predicate, unrolled or onchain kind, andIsOnchainSpentbuilds on it.(unrolled = true OR vtxo_kind = 1). The literal is the value theadd_vtxo_kindmigration defines, and a comment above the first statement says so.applyOnchainSpendsfilters on the predicate, and the restore loop inrestoreWatchingVtxosfollows through the widened selectors.SelectSweepableUnrolledVtxosstays as it is: an onchain-kind UTXO has no checkpoint tx for the sweeper to resolve, and itsark_txidpredicate already keeps onchain spends out.Nothing here writes
Kind = Onchainrows. That is M3.Tests
On sqlite, postgres and badger, with onchain-kind rows inserted directly so nothing waits for the write path:
The
IsOnchainSpenttable and theapplyOnchainSpendsunit test gain the kind cases. Removing the kind from the predicate fails both; the sql cases cannot pass on the old statements, which never matched a row that was not unrolled.go build,go vetandmake lintclean.Decided on the issue
#1159 question 9 is decided (decision): cosigned outputs the client reveals are registered at cosign time as a pending on-chain kind and promoted to onchain kind at confirmation. A pending output has nothing on chain for a unilateral spend to take and its CSV cannot mature before confirmation, so
HasOnchainOutputstays on the confirmed kind and a pending row never enters the predicates here. Adding the pending enum value later is a migration-free change that does not touch this diff.