vtxo: event and persistence projection for cosigned on-chain spends (M3 slice e)

[bitcoin-coder-bob]

M3 slice e of #1159. Stacked on #1186, which is stacked on #1161.

Merge order. #1161, then #1186, then this. It also must not merge before
#1162, since slice c will sequence the claim from that PR around the write this
adds. Nothing here depends on #1162's code, only on it landing first.

What this is

Design 7 gives the on-chain read model two feeders. A chain watcher for
unilateral spends, which is #1174 and #1184, and cosigned spends recorded
directly at cosign time, which is this. arkd signs those itself, so no
chain-watching is needed to know they happened.

The question 9 decision
settled what gets written. The revealed and verified outputs are registered at
cosign time as a pending on-chain kind, in the same durable write as the input
marks, and the per-transaction lifecycle in slice g later promotes or deletes
them.

First commit, the pending kind

VtxoKindOnchainPending is the third enum value. No migration, because the
column is an integer and Design 7 left the discriminator open for exactly this.

What it exposed is the interesting part. All four rules keyed on the kind
compared against VtxoKindOnchain directly, so a pending output would have:

• read as a note, since it has no commitment txids
• required a forfeit
• been treated as expired at the epoch, from its zero ExpiresAt
• had that zero taken as a real deadline by the raw readers Keep a vtxo with no batch expiry out of expiry comparisons #1186 guards

They now ask IsOnchainKind(), which covers both on-chain values. A pending
output is as far from a batch leaf as a confirmed one is.

Second commit, the durable write

RecordCosignedTx(ctx, txid, inputs, outputs) on domain.VtxoRepository, with
sqlite, postgres and badger implementations.

Both halves land together or not at all. A partial write is worse than a
failed one here, because the caller holds a claim on the inputs across it and
releases it afterwards, so half a write frees an input whose spend was never
recorded.

The inputs get no ark txid, deliberately. An in-Ark spend always sets either
ArkTxid or SettledBy, so their absence on a spent vtxo is the entire
discriminator for an onchain spend. The code says so at the assignment.

The outputs are forced to pending whatever kind the caller passed, since only
the lifecycle in slice g may decide something confirmed. A caller cannot register
an output as spendable by mistake.

Two judgement calls worth a second opinion

addVtxosTx is extracted in both SQL backends so AddVtxos and this path
share one row builder. The alternative was a second copy of a twenty-field
literal, where a new column reaches one path and misses the other. It touches
existing code, which is normally discouraged, but the duplication risk looked
worse than the churn.

Badger uses a real transaction through the helpers' existing
transaction-in-context support, rather than the unsynchronised loop SpendVtxos
uses, because atomicity is this slice's whole point. That needs one
//nolint:staticcheck, since those helpers read the context key as a bare
string.

Tests

TestRecordCosignedTx runs every case against both sqlite and badger.
Postgres is covered by the same interface but needs a live server, so CI is its
first real run.

Mutation checked rather than asserted:

Mutation	Result
Stop forcing the pending kind	10 subtests fail
Let an ark txid through on the input mark	8 subtests fail
Collapse IsOnchainKind to a single kind	8 subtests fail

Build, race detector on ./internal/core/..., and make lint at zero issues.

Note on CI

No checks will run here. The base branch contains a slash, which the workflow
filter does not match, and CodeRabbit skips drafts. Everything above was verified
locally.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[arkana-ai-bot]

Arkana review — arkade-os/arkd #1189

vtxo: event and persistence projection for cosigned on-chain spends (M3 slice e)

⚠️ PROTOCOL-CRITICAL — touches VTXO lifecycle, kind routing, and atomic DB writes. Flagged for human sign-off before merge.

Summary

Solid implementation. This adds the state for outputs of co-signed but unconfirmed transactions, wires up the atomic write across all three backends (Badger, Postgres, SQLite), and updates every kind-routing predicate to use the new helper instead of a bare comparison.

What's correct

• Atomicity: marks inputs spent and inserts pending outputs in a single transaction on all backends. The comment correctly notes that a partial write would free a claim whose spend was never recorded — the implementation avoids this.
• ** predicate**: The four rules that previously compared directly to (, , , ) now route through it. Without this, a pending output would be treated as a note, required a forfeit, expired at the epoch, and had its zero treated as a real deadline.
• Test coverage: exercises the atomic write, the DB tests cover all backends, and pins the new predicate.
• Open enum: Adding as a new iota value with no schema migration needed is consistent with the enum's documented design.

Minor note

The Badger impl uses with a bare string key (suppressed by ). This follows the existing repo pattern, so it's not a blocker — but worth noting as tech debt to address when the Badger layer is refactored.

Verdict

Looks correct and ready. The promotion path ( → on confirmation, delete on tx death) presumably lands in a follow-up slice — please confirm that path is covered before this merges, or confirm it's already in scope here.

Human review required due to protocol-critical nature (VTXO lifecycle + signing boundary).

[arkana-ai-bot]

Arkana review — arkade-os/arkd #1189

vtxo: event and persistence projection for cosigned on-chain spends (M3 slice e)

⚠️ PROTOCOL-CRITICAL — touches VTXO lifecycle, kind routing, and atomic DB writes. Flagged for human sign-off before merge.

Summary

Solid implementation. This adds the VtxoKindOnchainPending state for outputs of co-signed but unconfirmed transactions, wires up the RecordCosignedTx atomic write across all three backends (Badger, Postgres, SQLite), and updates every kind-routing predicate to use the new IsOnchainKind() helper instead of a bare == VtxoKindOnchain comparison.

What's correct

• Atomicity: RecordCosignedTx marks inputs spent and inserts pending outputs in a single transaction on all backends. The comment correctly notes that a partial write would free a claim whose spend was never recorded — the implementation avoids this.
• IsOnchainKind() predicate: The four rules that previously compared directly to VtxoKindOnchain (IsNote, RequiresForfeit, HasBatchExpiry, IsExpired) now route through it. Without this, a pending output would be treated as a note, required a forfeit, expired at the epoch, and had its zero ExpiresAt treated as a real deadline.
• Test coverage: cosigned_tx_repo_test.go exercises the atomic write, the DB tests cover all backends, and TestVtxo_IsOnchainKind pins the new predicate.
• Open enum: Adding VtxoKindOnchainPending as a new iota value with no schema migration needed is consistent with the enum's documented design.

Minor note

The Badger impl uses context.WithValue(ctx, "tx", tx) with a bare string key (suppressed by //nolint:staticcheck). This follows the existing repo pattern, so it's not a blocker — but worth noting as tech debt to address when the Badger layer is refactored.

Verdict

Looks correct and ready. The promotion path (VtxoKindOnchainPending → VtxoKindOnchain on confirmation, delete on tx death) presumably lands in a follow-up slice — please confirm that path is covered before this merges, or confirm it's already in scope here.

Human review required due to protocol-critical nature (VTXO lifecycle + signing boundary).

[arkana-ai-bot]

Arkana review — VtxoRepository.RecordCosignedTx

Verdict: looks correct; protocol-critical atomicity requirement — please have a human reviewer confirm the Badger and Postgres implementations satisfy it before merging.

What this does

Adds RecordCosignedTx to VtxoRepository: marks cosigned inputs as spent (no ark txid, distinguishing an onchain spend from an in-Ark one) and inserts the outputs as VtxoKindOnchainPending in a single atomic write. The comment on the interface correctly explains why a partial write would be worse than no write: the input claim is released after this call, so a half-written state would free an input whose spend was never recorded.

Assessment

• Badger implementation: uses a single Badger transaction committed last. The context.WithValue(ctx, "tx", tx) approach relies on the sub-helpers reading a bare string key — fragile but consistent with the existing pattern in the codebase.
• Atomicity contract: the interface doc makes the requirement explicit. Reviewers should verify the Postgres implementation uses a single SQL transaction and the in-memory implementation holds a single mutex lock across both operations.
• The forced VtxoKindOnchainPending override on every output (regardless of what the caller sets) is the right defensive posture: nothing arkd has only cosigned is spendable yet.
• Mock implementations (indexer_test, sweeper_test) stub RecordCosignedTx with no-ops, which is correct for those test contexts.

Please verify all three backend implementations (Badger, Postgres, in-memory/event) satisfy the atomicity requirement.

[arkana-ai-bot]

Review — vtxo: event and persistence projection for cosigned on-chain

Verdict: looks correct. Protocol-critical — flag for human review before landing.

What changed

Introduces VtxoKindOnchainPending as a third VtxoKind, plus RecordCosignedTx on the repository interface.

VtxoKindOnchainPending

A vtxo output in a transaction arkd cosigned but that hasn't confirmed yet. The new IsOnchainKind() helper centralises the == VtxoKindOnchain checks that previously scattered across IsNote, RequiresForfeit, HasBatchExpiry, and IsExpired. Every call site that touched the single-kind comparison is now routed through IsOnchainKind(), which covers both values. The commit message spells out exactly what would break if any check kept the old single-kind comparison (note read, forfeit required, expired at epoch).

RecordCosignedTx

An atomic write that marks inputs spent (with no ArkTxid — the discriminator for onchain vs in-Ark) and inserts the outputs as VtxoKindOnchainPending, regardless of what kind the caller set. Both halves commit together or not at all. The interface doc calls out why atomicity matters: the caller holds a claim on the inputs across this write and releases it after, so a partial write would free an input whose spend was never recorded.

The Badger implementation uses a context-injected transaction to achieve the atomicity. The bare-string context key (context.WithValue(ctx, "tx", tx)) is a pre-existing pattern in this codebase; the //nolint:staticcheck comment is accurate.

Assessment

• The state machine reasoning is sound: a pending output is correctly excluded from expiry, forfeit, and note classification, exactly like a confirmed onchain vtxo.
• RecordCosignedTx forcing VtxoKindOnchainPending unconditionally is correct — only the per-transaction lifecycle (on confirmation) should promote to VtxoKindOnchain.
• Tests: TestVtxo_IsOnchainKind, TestRecordCosignedTx, and the mock updates in indexer_test / sweeper_test are all present and thorough.
• The "unknown input does not block the outputs" test case is worth noting: if a vtxo is missing from the repo when marking it spent, the error is swallowed and the outputs still land. This is likely intentional (the write is idempotent enough for redelivery) but reviewers should confirm this is the desired semantics for the application layer.

🤖 Reviewed by Arkana (pr-lifecycle)

[arkana-ai-bot]

Protocol-critical — requires human review before merge regardless of the analysis below.

Correctness

The VtxoKindOnchainPending state machine is well-reasoned. Introducing IsOnchainKind() as a single gate for all kind-keyed rules (IsNote, RequiresForfeit, HasBatchExpiry, IsExpired) is the right approach and avoids the class of bugs the PR description cites. The three-way kind enum (Offchain / Onchain / OnchainPending) is small and the open-enum comment correctly flags the schema migration boundary.

RecordCosignedTx atomicity (Badger)

The retry loop uses badger.ErrConflict with a hard 100ms sleep. That is inherited from the existing Badger patterns in this repo, so it is consistent, but the fixed sleep means under contention the retry window is bounded only by maxRetries * 100ms. Worth a note on whether that bound has been validated against the cosign timeout budget.

The //nolint:staticcheck context key comment is accurate but worth tracking: if spendVtxo / addVtxos are ever refactored to stop reading that key, the atomicity guarantee silently breaks.

Badger spendVtxo behaviour on unknown inputs

The test asserts "an unknown input does not block the outputs" — meaning spendVtxo returns nil for a missing outpoint. That is correct only if the caller's claim logic already guarantees the input exists. The comment in vtxo_repo.go ("caller holds a claim") relies on that invariant but it is enforced outside this diff. Worth an explicit cross-reference to the claim-acquisition site.

Postgres RecordCosignedTx

Uses UpdateVtxoSpent with an explicit NULL ArkTxid. The comment "its absence marks the spend as onchain" is the discriminator used elsewhere — good that it is documented at the write site too.

Tests

Domain unit tests cover all four kind-keyed predicates for the new kind. The integration test (TestRecordCosignedTx) runs against both SQLite and Badger and covers forced-kind overwrite and unknown-input non-blocking. Coverage looks solid for the diff's surface area. No test for Postgres (consistent with the rest of the test suite, which relies on CI for Postgres).

Merge dependency note

The PR description states this must not merge before #1162 because that PR sequences the claim around this write. That ordering dependency is not enforced by any code guard in this diff — it relies entirely on merge discipline. Consider a link or TODO at the RecordCosignedTx call site once #1162 lands.

Verdict: Looks structurally correct; flag for human review (protocol-critical VTXO persistence change, stacked PR with external ordering dependency on #1162).

[arkana-ai-bot]

Review — #1189 vtxo: event and persistence projection for cosigned on-chain spends

⚠️ PROTOCOL-CRITICAL (VTXO kinds) — flag for human review before merge.

The VtxoKindOnchainPending addition is well-structured. Key observations:

Domain model change. Adding VtxoKindOnchainPending as a third kind is the right granularity: it lets the indexer surface the output during the confirmation window without claiming it is spendable. The open-enum comment that no migration is needed is accurate provided the database stores the raw uint8.

IsOnchainKind() helper. Every rule that previously compared v.Kind == VtxoKindOnchain directly (IsNote, RequiresForfeit, HasBatchExpiry, IsExpired) now goes through IsOnchainKind(). The four callers are all correctly updated. The test vectors cover all three kinds for each predicate.

RecordCosignedTx interface method. The doc comment is clear: inputs marked spent-by-txid with no ark txid (distinguishing onchain from in-Ark spend), outputs inserted as VtxoKindOnchainPending regardless of caller-supplied kind, and atomicity required. The mock stubs for indexer_test.go and sweeper_test.go are correct no-ops.

One question for reviewers: the promotion from VtxoKindOnchainPending → VtxoKindOnchain on confirmation, and the deletion on transaction death, are described in the comment but the implementation of those transitions is presumably in a companion PR (the stacked series). Worth confirming that PR is either already merged or immediately follows this one so there is no window where pending outputs linger indefinitely.