Skip to content

fix(kotlin-sdk): act on swept transactions in the Room store - #4590

Open
romchornyi wants to merge 2 commits into
split/4406-5-swiftfrom
split/4406-6-kotlin
Open

fix(kotlin-sdk): act on swept transactions in the Room store#4590
romchornyi wants to merge 2 commits into
split/4406-5-swiftfrom
split/4406-6-kotlin

Conversation

@romchornyi

Copy link
Copy Markdown
Contributor

Stacked on #4589. Review only this PR's own diff; its base is split/4406-5-swift.
Last of the five PRs #4406 was split into: seam → storage → producer → Swift → Kotlin.

Issue being fixed or feature implemented

The Android half of the same contract. Until it lands, the Kotlin host never attests CORE_SWEEP_REMOVAL, so Rust fail-closes and an Android wallet freezes its sync watermark on the first sweep it meets — funds-safe, but a user-visible stall.

What was done?

The store

Kotlin deletes rather than marks: Room rows here are wallet-scoped, so there is no shared row to keep inert the way SwiftData needs. The order is load-bearing — hold before delete, because the foreign key nulls the very column that finds a released coin's rows.

Held inputs become pending-input tombstones carrying the winner and, when it was mined, its height; a co-swept child's claim on its parent's output goes with the batch; the drain guards isSpent against a tombstone; and every restore path excludes what a sweep removed.

Capability

NativePersistenceBridge gains the CORE_SWEEP_REMOVAL constant on the class whose default implementation refuses the round: a subclass that declares the bit without overriding the callback fails its round rather than silently dropping the removal.

Kotlin deliberately does not declare DASHPAY_PAYMENTS — this store has no payments overlay, and saying so is what keeps Rust's flip from being staged onto a round that would drop it.

JNI — why it ships in the same PR

rs-unified-sdk-jni adds the sweeps trampoline with a with_local_frame per batch, its descriptor in the bridge method table's smoke check, and reorders transactions ahead of utxos_added because the swept-row guard reads a state the transaction pass writes.

Kotlin alone is safe (the declared ∩ structural intersection withholds the bit until the slot is wired), but JNI alone is a hard init failure by that same smoke check — and the SDK ships both from one revision, so they travel together.

Schema

Room goes to schema 13 with migrations 10→11→12→13 and their generated JSON kept as reviewed.

How Has This Been Tested?

ANDROID_HOME=$HOME/Library/Android/sdk ./gradlew :sdk:testDebugUnitTest398 unit tests pass, and cargo check -p rs-unified-sdk-jni is clean.

Robolectric coverage (no native needed) for: the capability default refusing a hand-declared round, a release naming more outpoints than SQLite can bind in one statement, co-swept twins, detached tombstones with a swept winner, cross-round reinstatement, the releaseByOutpoint spender guard, and asset-lock Consumed(4) terminal guards — plus migration tests validating against the schema JSONs.

The test file is the union of this branch's cases and those #4356 and the marketplace work added to the same regions while this PR was open; all 143 test and helper functions from both sides are present, verified function by function.

Breaking Changes

None. NativePersistenceBridge gains an open fun with a fail-closed default, so an out-of-tree subclass that declares the bit without implementing it fails its round rather than silently losing the removal — deliberate, and the safe direction.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: bc7903d5-49c0-45e5-86fb-f199bb0d9c63

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

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

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

@thepastaclaw

thepastaclaw commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

🕓 Ready for review — 50 ahead in queue (commit c055ac3)
Queue position: 51/51 · 2 reviews active
ETA: start ~15:44 UTC · complete ~16:39 UTC (median 54m across 30 recent reviews; 2 slots)
Queued 1m ago · Last checked: 2026-09-03 16:50 UTC

The Room mirror of the storage contract, plus the JNI trampoline that
delivers a round's sweeps.

Kotlin deletes rather than marks — Room rows here are wallet-scoped, so
there is no shared row to keep inert the way SwiftData needs — but the
order is load-bearing: hold before delete, because the foreign key nulls
the very column that finds a released coin's rows. Held inputs become
pending-input tombstones carrying the winner and, when it was mined, its
height; a co-swept child's claim on its parent's output goes with the
batch; the drain guards `isSpent` against a tombstone; and every restore
path excludes what a sweep removed.

`NativePersistenceBridge` gains the `CORE_SWEEP_REMOVAL` constant on the
class whose default implementation refuses the round: a subclass that
declares the bit without overriding the callback fails its round rather
than silently dropping the removal. Kotlin deliberately does not declare
`DASHPAY_PAYMENTS` — this store has no payments overlay, and saying so
is what keeps Rust's flip from being staged onto a round that would drop
it.

The JNI half must ship with it. `rs-unified-sdk-jni` adds the sweeps
trampoline with a `with_local_frame` per batch, its descriptor in the
bridge method table's smoke check, and reorders `transactions` ahead of
`utxos_added` because the swept-row guard reads a state the transaction
pass writes. Kotlin alone is safe (the declared ∩ structural
intersection withholds the bit until the slot is wired) but JNI alone is
a hard init failure by that same smoke check, and the SDK ships both
from one revision.

Room goes to schema 13 with migrations 10→11→12→13 and their generated
JSON kept as reviewed.

Tests: ~2,000 lines of Robolectric coverage — the capability default
refusing a hand-declared round, a release naming more outpoints than
SQLite can bind in one statement, co-swept twins, detached tombstones
with a swept winner, cross-round reinstatement, the `releaseByOutpoint`
spender guard, and asset-lock Consumed(4) terminal guards — plus
migration tests validating against the schema JSONs. 398 unit tests pass
(`./gradlew :sdk:testDebugUnitTest`).

The test file is the union of this branch's cases and those `#4356` and
the marketplace work added to the same regions while this PR was open;
all 143 test and helper functions from both sides are present.
…e left

`TransactionEntity` and `TxoEntity` were each imported twice, which
Kotlin rejects as an ambiguous import — the Android CI job failed to
compile the test source. Both came from folding this branch's test cases
together with the ones `#4356` and the marketplace work added to the
same import block while this PR was open; the union kept every line from
both sides, identical ones included.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants