feat(repositories): NArk persistence parity — IntentRepository + VirtualTxRepository (storage layer)

[Kukks]

Summary

NArk (dotnet-sdk) persistence parity for ts-sdk: two new sibling repository
interfaces — IntentRepository and VirtualTxRepository — across
all four storage backends, the pure IntentStateReducer, lifecycle
wiring that persists settlement intents and excludes intent-locked VTXOs from
spendable balance, repository-backed unilateral exit (NArk Lite/Full),
and automated prune-on-spend so the repository stays bounded.

Design + plan are local artifacts (docs/ is gitignored):
docs/superpowers/specs/2026-05-18-nark-persistence-parity-design.md,
docs/superpowers/plans/2026-05-18-nark-persistence-parity.md.

What's included

Storage layer

• Domain types: ArkIntent/ArkIntentState, VirtualTx/ChainedTxType
  (pinned from NArk ChainedTxType.cs), VtxoBranch, VirtualTxMode,
  IntentFilter. Single-wallet (ts-sdk idiom).
• Two interfaces (siblings of WalletRepository/ContractRepository),
  implemented across in-memory / SQLite / IndexedDB / Realm: SQLite
  batch writes wrapped in BEGIN IMMEDIATE/COMMIT (review Repositories layer: atomicity, silent failures, and cross-backend divergence (review findings) #493 package.json: version 0.0.1 #1);
  IndexedDB DB_VERSION 3→4, reads reject not swallow (review Repositories layer: atomicity, silent failures, and cross-backend divergence (review findings) #493 Improve Release tooling #2);
  Realm ARK_REALM_SCHEMA_VERSION 2→3.
• Shared cross-backend conformance suite; one shared matches() for
  intent filtering → behaviourally identical by construction.
• IntentStateReducer — pure, monotonic; terminal states sticky.

Lifecycle wiring

• Opt-in injection via StorageConfig.{intentRepository,virtualTxRepository, virtualTxMode} assigned at create() — no constructor/call-site churn;
  undefined ⇒ every new path is a no-op.
• getBalance() excludes VTXOs locked by non-terminal intents.
• settle() persists the intent through its lifecycle, best-effort —
  never throws into the money path.

Virtual-tx + unilateral exit

• chainToBranchAndTxs/chainTxTypeToChained — normalise an indexer VTXO
  chain into the persisted shapes (commitment-first positions).
• VirtualTxMode ("lite" default — mirrors NArk's enum default; "full"
  opt-in).
• Unroll.Session is repository-aware: each exit step resolves its raw
  tx from the repo first (instant in Full mode), falls back to the indexer
  on a miss, and caches the result (NArk Lite "materialise on demand").
  Omitted repo ⇒ byte-for-byte previous behaviour; chain traversal
  untouched.
• Automated prune-on-spend (shouldPruneSpentVtxo +
  ContractManager.pruneSpentVirtualTxs): every delta sync (and the
  vtxo_spent event path, which funnels through it) drops the persisted
  exit branch + orphaned virtual txs for VTXOs that are offchain-spent and
  not unrolled — keeping VirtualTxRepository bounded. Conservative,
  fund-safe predicate (never prunes isUnrolled outputs whose unilateral
  exit may be in flight); best-effort (logged, never breaks sync); no-op
  when no virtualTxRepository is configured.
• Existing transaction-history persistence is untouched (app concern;
  separate future deprecation spec).

Test plan

• pnpm test:unit — all green; husky pre-commit ran the full unit suite
  green on every one of the 19 commits.
• pnpm exec tsc --noEmit -p tsconfig.json — 0 errors.
• pnpm lint (prettier) — clean.
• Prior full CI run green incl. integration; CI for the latest commits
  validated on this PR.
• New tests include per-backend conformance wirings + shared suites, reducer
  units, schema-version tests, locked-balance, settle intent-wiring, chain
  mapping, Unroll repo-first resolution, and prune-on-spend
  predicate/glue (eligible-only, no-repo no-op, error-swallowing).

Remaining work (follow-up — deliberately not in this PR)

• Eager in-syncContracts VirtualTx pre-population — an optimisation
  that would let Full-mode exits skip even the first indexer call. The
  user-facing capability (repo-backed exit + Lite on-demand caching +
  automated prune) is delivered; eager pre-fill only changes when the repo
  is populated and hooks the core money-sync loop, so it's best done in a
  focused session.
• T17 e2e (crash-resume / locked-balance / branch / repo-backed exit /
  prune) — needs the nigiri regtest stack.

Kept draft until the above land.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: de155d92-bcdf-4157-987a-241e5fbe9ff8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/nark-persistence-parity

---

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 and usage tips.}

[arkanaai]

Arkana Code Review — ts-sdk#494

Scope: Storage layer + reducer for NArk persistence parity. 2088 additions across 4 backends × 2 new repo interfaces, plus IntentStateReducer. No existing code paths modified (besides extracting mock SQL executor to shared helper and fixing the IndexedDB schema version literal).

Overall assessment: Clean, well-structured PR. The conformance suite approach is excellent — same behavioural contract enforced across all four backends. Types match NArk ChainedTxType values exactly. The matches() single-source-of-truth pattern avoids cross-backend filter divergence. However, I found one real bug in the Realm backend and a few issues worth addressing before merge.

---

🔴 BUG — Realm pruneForSpentVtxo reads deleted objects (will throw in production)

src/repositories/realm/virtualTxRepository.ts:97-124 (diff lines 897-924)

async pruneForSpentVtxo(vtxo: Outpoint): Promise<void> {
    this.realm.write(() => {
        const removed = [
            ...this.realm
                .objects("ArkVtxoBranch")
                .filtered("vtxoKey == $0", vtxoKey(vtxo)),
        ] as any[];                          // ← live Realm proxies spread into array
        this.realm.delete(
            this.realm
                .objects("ArkVtxoBranch")
                .filtered("vtxoKey == $0", vtxoKey(vtxo))  // ← deletes same underlying rows
        );
        for (const e of removed) {
            const stillRef = [
                ...this.realm
                    .objects("ArkVtxoBranch")
                    .filtered("virtualTxid == $0", e.virtualTxid),  // 💥 e is invalidated
            ];

Spreading a Realm Results into an array gives you an array of live Realm object proxies. After this.realm.delete(...) removes the same underlying rows, those proxies are invalidated. Accessing e.virtualTxid on line 914 will throw "Accessing object which has been invalidated or deleted" in a real Realm environment.

The mock test passes because createMockRealm's delete() works by identity matching against stored Map values — the JS objects remain valid plain objects after removal from the Map. This bug is invisible to the test suite.

Fix: Extract the needed values before deleting:

this.realm.write(() => {
    const removed = [
        ...this.realm.objects("ArkVtxoBranch")
            .filtered("vtxoKey == $0", vtxoKey(vtxo)),
    ].map(e => ({ virtualTxid: (e as any).virtualTxid })); // snapshot values
    this.realm.delete(
        this.realm.objects("ArkVtxoBranch")
            .filtered("vtxoKey == $0", vtxoKey(vtxo))
    );
    for (const e of removed) { /* now safe */ }
});

---

🟡 TOCTOU — IndexedDB upsertVirtualTxs and pruneForSpentVtxo use separate transactions for read-then-write

src/repositories/indexedDB/virtualTxRepository.ts:52-76 (upsertVirtualTxs)

The read happens in a readonly transaction (line 56), then the merged write happens in a separate readwrite transaction (line 64). Between the two, a concurrent upsertVirtualTxs call could commit, and the second call's merge would be based on stale prevs — potentially overwriting a newer hex with null.

src/repositories/indexedDB/virtualTxRepository.ts:131-163 (pruneForSpentVtxo)

Three separate transactions: read removed branches → delete branches → check orphans + delete orphaned txs. A concurrent setBranch between delete and orphan-check could cause a VirtualTx to be incorrectly deleted (the newly-set branch's reference won't be counted).

Mitigation: For upsertVirtualTxs, you can do the get + put in a single readwrite transaction. IDB readwrite transactions on the same store are serialized:

const wt = db.transaction([STORE_VIRTUAL_TXS], "readwrite");
const ws = wt.objectStore(STORE_VIRTUAL_TXS);
for (const t of txs) {
    const prev = await req(ws.get(t.txid)) as VirtualTx | undefined;
    ws.put({ txid: t.txid, hex: t.hex ?? prev?.hex ?? null, ... });
}
await done(wt);

For pruneForSpentVtxo, the read + delete of branches should be in one readwrite transaction on STORE_VTXO_BRANCHES, and the orphan check + delete should be in a single transaction spanning both stores.

---

🟡 Protocol-adjacent: getLockedVtxoOutpoints() correctness analysis

This method is money-path — it will determine which VTXOs are excluded from spendable balance. The logic is correct:

State	Locked?	Correct?
waiting_to_submit	✅ locked	✓
waiting_for_batch	✅ locked	✓
batch_in_progress	✅ locked	✓
batch_failed	❌ unlocked	✓ (batch failed, VTXOs free)
batch_succeeded	❌ unlocked	✓ (VTXOs consumed)
cancelled	❌ unlocked	✓ (intent cancelled)

However, the SQLite backend (src/repositories/sqlite/intentRepository.ts:85-95) loads all rows then filters in JS. When the wiring lands (T15), push the WHERE state NOT IN ('batch_failed','batch_succeeded','cancelled') down to SQL — no point pulling terminal intents off disk just to discard them.

---

🟡 Minor: applySettlementEventToIntent — unnecessary runtime check

src/wallet/intentStateReducer.ts:39

if ("id" in event && typeof event.id === "string") {
    updated.batchId = event.id;
}

Every variant of SettlementEvent has id: string. This check always passes. Either trust the types and remove the guard, or add a comment explaining why it's there (future-proofing?). As-is, it reads like the author wasn't sure whether id exists, which could confuse future reviewers.

---

🟢 Nits (non-blocking)

1. src/repositories/sqlite/intentRepository.ts:38-42 — saveIntent wraps a single INSERT OR REPLACE in BEGIN IMMEDIATE / COMMIT. A single statement is already atomic in SQLite. The transaction wrapper adds overhead for no correctness gain here (unlike pruneForSpentVtxo where it's correctly used).

2. src/repositories/virtualTxRepository.ts:38 — VirtualTxMode is defined but unused in this PR. Presumably for T16. Consider adding a // used by T16 wiring comment or deferring the type to that PR.

3. Reducer doesn't handle BatchFinalization (SettlementEventType.BatchFinalization) — falls through to no-op. This is likely intentional (it's an intermediate event before BatchFinalized), but a comment in the switch would help.

4. No waiting_to_submit → waiting_for_batch transition in the reducer. Presumably that transition is imperative (caller sets it when submitting to server) rather than event-driven. Worth a doc comment on the reducer to clarify which transitions it owns vs. which are imperative.

---

🟢 Cross-repo impact

• 14 downstream repos import from @arkade-os/sdk. This PR adds new exports only — no breaking changes. Existing WalletRepository, ContractRepository, and barrel exports are untouched.
• ChainedTxType enum values (0–4) are verified identical to NArk dotnet-sdk/NArk.Abstractions/VirtualTxs/ChainedTxType.cs.
• No open PRs in consumer repos reference these new types yet.

---

🟢 Test coverage

Excellent. Shared conformance suites for both interfaces × all 4 backends. Reducer has lifecycle, monotonicity, and no-op tests. Schema version bump tests for IndexedDB v4 and Realm v3. The mock SQL executor extraction from sqlite-wallet-repository.test.ts into test/helpers/mockSqlExecutor.ts is a clean refactor.

Gap: The mock Realm doesn't reproduce real Realm's invalidated-object-after-delete semantics (see bug #1). Consider adding a createStrictMockRealm() variant that throws on property access after delete, or add an integration test note.

---

Verdict

Request changes — the Realm pruneForSpentVtxo bug (#1) will crash in production. The IndexedDB TOCTOU (#2) is lower priority but should be fixed before the wiring PR lands.

⚠️ Protocol-critical flag: While this PR is storage-only, getLockedVtxoOutpoints() is directly money-path (double-spend prevention). The interface and state model look correct, but human sign-off required before merge, especially before the T14–T16 wiring PR that connects this to settle() / getBalance().