feat(cashu): C2 — embedded Cashu wallet over cdk (native), typed stub on web - #235
feat(cashu): C2 — embedded Cashu wallet over cdk (native), typed stub on web#235grunch wants to merge 5 commits into
Conversation
… on web Phase C2 of docs/cashu/README.md. The seller needs ecash at the node's mint before an escrow can be locked and the buyer needs somewhere for redeemed ecash to land, so this is a prerequisite for every later phase rather than a feature on its own. No UI yet. Inert on Lightning: every entry point in `api/cashu.rs` returns `CashuNotEnabled` unless `escrow_mode::is_cashu_mode()` is true, which needs the active node to have advertised Cashu *and* a usable mint. Nothing connects at startup — a Lightning user never opens a proof store or contacts a mint. Wallet (`rust/src/cashu/`) - `CashuWallet::connect` binds to one mint (the node pins it; a second mint would have no counterparty) and verifies NUT-07, NUT-11, NUT-12 and a `sat` keyset up front. Checking at connect rather than at first use matters: a seller who funds a wallet at a mint that cannot do NUT-11 would otherwise find out when the escrow lock fails, with the sats already there. - `balance`, `receive_token` (swap-in, DLEQ-verified), `create_token`, `check_proofs_state` (NUT-07 reconciliation of interrupted sends). - Seed is the identity's BIP-39 seed, so the ecash is recoverable from the words the user already backed up — one secret to protect, not two. - Proofs live in `cashu.sqlite`, a sibling of the app DB derived from the path Dart already passes to `init_db`. cdk owns that file's schema; mixing it into the app's would put two migration systems on one file. - Web gets a typed stub (`CashuUnsupportedOnWeb`), mirroring `nwc::client`. The gap is storage only: cdk itself compiles to wasm, cdk-sqlite does not. Bridge (`rust/src/api/cashu.rs`) - `cashu_connect`, `cashu_status`, `cashu_get_balance`, `cashu_receive_token`, `cashu_create_token`, `cashu_check_proofs_state`, `cashu_disconnect` and an `on_cashu_wallet_changed` stream. Errors are stable markers; Dart localizes. - `cashu_status` is deliberately ungated — the UI asks before it knows what the node runs, and "not connected" is truthful on every node. Dependencies pinned `=0.17.3` with default features off. docs/cashu/cdk-spike.md records what was verified against that version: NUT-11 with custom tags is fully exposed (so C4 needs no hand-built secrets), the wallet database trait is wasm-shaped but has 50 methods and no IndexedDB backend (so C9 stays a phase), and cdk compiles to wasm32 today. Tests: capability gating and its markers, every bridge entry point shut on a Lightning node, idempotent disconnect, proof-store path refusal without a DB. Integration tests against a local nutshell are `#[ignore]` behind MOSTRO_TEST_MINT_URL — a mock would have to fake blind signatures and DLEQ, and a wallet that passes against faked cryptography has proven nothing.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (7)
WalkthroughAdds a native Cashu wallet with capability checks, token operations, proof reconciliation, secure seed handling, persistent proof-store paths, wasm stubs, status APIs, subscriptions, documentation, tests, and Flutter-Rust bridge bindings. ChangesCashu wallet integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CashuClient
participant cashu_connect
participant IdentityState
participant CashuWallet
participant Mint
CashuClient->>cashu_connect: request wallet connection
cashu_connect->>IdentityState: derive current BIP-39 seed
IdentityState-->>cashu_connect: zeroized seed
cashu_connect->>CashuWallet: connect with mint URL and proof-store path
CashuWallet->>Mint: probe mint capabilities and keysets
Mint-->>CashuWallet: return mint information
CashuWallet-->>cashu_connect: return connected wallet
cashu_connect-->>CashuClient: publish wallet status
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Review — C2 (strict pass)
Scope reviewed: 🔴 Major1. A failed balance read is reported as balance_sats: wallet.balance().await.unwrap_or_else(|e| {
log::warn!("[cashu] balance read failed: {e}");
0
}),The comment argues the next event will carry the real figure. That is true and still wrong for this data type: ecash is bearer money, and a user who opens the wallet during a transient store error is shown "0 Satoshis" with no indication anything failed. The rational reaction to that screen is panic, and the second-most rational is to re-import a seed somewhere else.
2. A failed
Today the only recovery is the user finding the "check for unredeemed tokens" button in C3. That is a recovery mechanism, not an error path. At minimum, 3. A scanned token with a
🟡 Minor4. It derives the store from 5. Seed material is copied around unprotected —
6. Concurrent 🔵 Nit / Info7. Dependency weight. 8. ✅ What is right
Test gaps
|
…hu: URIs Addresses the strict review on #235. Major - `CashuWalletStatus.balance_sats` is now `Option<u64>`. It used to report a failed read as `0`, which for bearer money is the worst possible lie: a user opening the wallet during a transient store error was shown "0 Satoshis" and no indication anything had failed. `None` and `Some(0)` are different facts and only one of them is alarming. - A failed `confirm` left the prepared send's proofs reserved with no owner — `confirm` consumes the handle, so nothing could cancel them — and the balance silently dropped until the user happened to run a proof-state check. The failure path now reclaims and reports how much came back. - `receive_token` rejected `cashu:` and `cashu://` URIs, which is what QR payloads normally carry. The user got "may be from another mint, or already spent" for a token that was neither. Minor - `proof_store_path` joined onto `Path::parent()`, which is `Some("")` for the IndexedDB *database name* the web build passes to `init_db` — silently producing a relative file next to the process cwd. The two cases are now separated explicitly. - The BIP-39 seed is wrapped in `Zeroizing` from derivation through to the cdk boundary, so the copy made to seed the wallet is wiped on drop. - Concurrent `cashu_connect` calls opened the proof store twice and made two mint round trips, discarding one wallet. Serialised behind a connect mutex. - The wasm stub's `capabilities()` panicked via `unreachable!()` while its neighbours returned empty values. It now answers, so "this cannot happen" has one meaning in that file. Tests: `a_scanned_token_uri_is_accepted` over the five shapes a scanner produces, and `normalisation_leaves_a_token_body_alone` so the strip cannot eat a payload that merely contains the scheme.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Running the `#[ignore]`d suite against a real nutshell for the first time showed it could not pass at all, and then that it could not pass twice. - `a_token_round_trips_between_two_wallets` asserted "fund the sender wallet first" and returned. Nothing a reviewer could do satisfied that, so the test was documentation rather than verification. `mint_for_test` mints from the mint itself, which settles instantly against a FakeWallet backend. - A fixed seed plus a fresh DB replays the same NUT-13 blinding secrets, and the mint answers "Blinded Message is already signed" on the second run. Seeds are now unique per process and per call. - The round trip pinned the received amount at 8. nutshell's default keyset charges a swap fee, so an 8 sat token redeems for 7. The face value is what the sender parts with and what a validator checks; the assertion bounds the redeemed amount instead of pinning it. Verified 4/4 against nutshell 0.20.3, twice in a row. The mint rate limits by default — running back to back needs MINT_RATE_LIMIT=FALSE.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/src/api/cashu.rs`:
- Line 41: In rust/src/api/cashu.rs at lines 41-41 and 263-263, update the Cashu
error returns to use marker-only stable codes: return CashuStoreUnavailable
without English prose and replace the wallet stream closure message with a code
such as CashuWalletStreamClosed. Keep diagnostic logging separate from the
errors returned to Dart.
- Around line 149-157: Update CashuWallet::connect so its final wallet
installation cannot revive a wallet after cashu_disconnect wins: serialize the
disconnect path with the same lifecycle mutex used by connect, then re-check
ensure_enabled() after acquiring that mutex before storing the pending wallet.
Preserve the existing authoritative live-wallet behavior, and add a targeted
regression test covering disconnect during an in-flight connect.
In `@rust/src/cashu/wallet.rs`:
- Around line 205-215: Update the error branch handling confirm failure in the
send flow to preserve whether self.check_proofs_state() succeeded. Report the
reclaimed satoshis only when recovery succeeds; when recovery fails, include an
explicit recovery-failed/unknown status and the recovery error instead of
treating it as zero. Keep the original send error in the CashuSendFailed
message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e941161c-696c-4dc9-af7e-6eaf1a347d4e
⛔ Files ignored due to path filters (1)
rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
docs/cashu/README.mddocs/cashu/cdk-spike.mdrust/Cargo.tomlrust/src/api/cashu.rsrust/src/api/identity.rsrust/src/api/mod.rsrust/src/api/types.rsrust/src/cashu/mod.rsrust/src/cashu/wallet.rsrust/src/crypto/keys.rsrust/src/db/app_db.rsrust/src/frb_generated.rsrust/src/lib.rs
… wallet Three findings, all valid. Major - A `cashu_disconnect` running while `CashuWallet::connect` awaited the mint cleared the slot, and the connect then filled it back in because it saw `None` — rebinding the wallet the caller had just dropped, and (on a node switch, which is where disconnect is called from) leaving it pointing at the *previous* node's mint. Connect and disconnect now share one lifecycle lock, and connect re-checks that the active node still resolves to the mint it connected to before installing. The old comment even said "treat a live wallet as authoritative", which is exactly backwards for this case. Minor - `CashuStoreUnavailable: database not initialised` and `CashuWalletStream closed: channel sender dropped` carried English prose past the bridge. Rust returns markers and Dart localizes them (repo rule), so both are now bare markers; the second was not even a marker Dart could match, so it always fell through to the generic message. - A failed `check_proofs_state` during send recovery was reported as "reclaimed 0 sat", making an unknown outcome look like a confirmed zero — the same trap this PR fixed for `balance_sats`. A reclaim that fails now says so, and points at the proof-state check. Test: `a_wallet_is_only_installed_while_its_mint_is_still_the_active_one` pins the install guard, including that a trailing slash is a formatting difference rather than a different mint. The race itself is not reproducible in a unit test, so the decision it turns on is extracted and tested directly.
codaMW
left a comment
There was a problem hiding this comment.
Reviewed C2 independently full pass over cashu/wallet.rs, api/cashu.rs,
crypto/keys.rs, identity.rs, app_db.rs, the Cargo pins, and the spike doc,
on top of your strict round and the CodeRabbit round. Ran the integration suite
against a live mint. Approving.
All three Majors verified fixed:
- Balance-on-error,
snapshot()now returnsbalance_sats: Option<u64>,
Noneon a failed read (never0). The comment nails the reason a false
zero on bearer money reads as "your money is gone." - Stranded proofs
create_token'sconfirm-failure arm now runs
check_proofs_state()to reclaim, and bails with an honest error
(reclaimed N sat, or reports the reclaim failure too). No silent balance drop,
and it avoids the "reclaimed 0 = false reassurance" trap. cashu:URInormalize_token()stripscashu:///cashu:before
decoding, so scanner payloads round-trip.
Minors also addressed: the proof-store path separates the web name-case from the native path explicitly; the BIP-39 seed is now Zeroizing<[u8; 64]> through derive_bip39_seed / current_bip39_seed; and lifecycle_lock serializes connect/disconnect so a second connect can't open the store twice.
Money-path verification (live):
- Integration suite against a real nutshell mint (0.20.3,
MINT_RATE_LIMIT=FALSE):
4/4 passed, twice in a row token round-trips between two wallets with real
blind signatures + DLEQ, unreachable mint ->CashuMintUnreachable, zero-amount
send refused before touching the mint. The second pass confirms the NUT-13
seed-replay fix in1d39d71. - Lightning regression holds
cashu_connectcallsensure_enabled()before
touching the store or mint; every bridge entry point bailsCashuNotEnabledon
a Lightning node; after the full test run there is nocashu.sqliteanywhere
under the app data dir. The gate doesn't leak a store.
cargo test 142 passed / clippy -D warnings clean / cargo check --target wasm32-unknown-unknown clean / flutter analyze clean.
The refusal to mock the mint is the right call the #[ignore] + env-var harness
against a real mint is what makes this verifiable. Reusing the identity seed (one
secret, recoverable from words already backed up) and probing capabilities at
connect are both the correct trade-offs.
On the swap-fee note (8 sat token redeems for 7), agree that's correct behaviour
and a C3/C10 UI question, not one this PR should answer.
AndreaDiazCorreia
left a comment
There was a problem hiding this comment.
Ran the suite against a local nutshell 0.20.3 plus two extra probes. The gate holds up — every
door shut on a Lightning node, one test each — and C2's "done when" (connect, paste a token, see
balance, export one) is demonstrably met. Three things came out of the probes, checked against
the phase plan so the scope question is asked the right way round.
Requesting changes on 1 and 2 only — both are things this PR states and the code doesn't do,
and C3/C4 stack directly on them. 3 and everything under "Smaller" are questions, not
blockers; happy for any of them to become a follow-up issue instead.
1. DLEQ on receive — C2 asks for it, and it's currently best-effort
Same token with its DLEQ proofs stripped, then received:
proofs_with_dleq=1 verify_token_dleq=Err("Could not verify DLEQ proof") receive_without_dleq=Ok(7)
cdk verifies only if proof.dleq.is_some() (wallet/receive/saga/mod.rs:133-139), so a token
carrying none is accepted silently. Not a theft vector — the mint still validates the swap — but
the C2 checklist says receive_token(encoded) (swap-in, DLEQ-verified), and NUT-12 is a hard
connect requirement precisely for this.
- Add
verify_token_dleq(&token)before the receive (it does reject a missing proof, as the
probe shows), or is the softer guarantee intended and the checklist line the thing to relax?
2. check_proofs_state promises more than C10 asks of it
Fund 64, create_token(8), never redeem it, then run the state check:
funded=64 after_send=56 reported=0 after_check=56
pending_sends=1 check_send_status=Ok(false) revoke_send=Ok(7) after_revoke=63
The proofs don't come back and the call reports 0. That's consistent with the plan — full
reconciliation is C10, release-blocking, and risk #6 says the same — so I'm not asking for
revoke_send here. What doesn't line up is what this PR claims for it:
create_token's failure branch can only ever report(reclaimed 0 sat)— the same misleading
zero the comment above it says it wants to avoid;cashu_check_proofs_statereturns that number to Dart as an amount reclaimed, and
if reclaimed > 0 { notify() }never fires;- the test-plan item "kill the app mid-send → reserved proofs come back" doesn't pass.
Cause: check_all_pending_proofs() filters used_by_operation.is_none() (wallet/proofs.rs:135)
while prepare_send reserves with an operation id (send/saga/mod.rs:627), so it skips by
construction every proof this wallet reserves; its return is "how much is still stuck", not "how
much came back".
- Rather than implementing C10 early, should C2 just stop promising it — rename the value to what
it is, drop the reclaim wording in the docstrings and the test plan, and leave a pointer to C10?
3. Wallet stays bound to the previous node's mint (no phase owns this)
should_install guards the invariant at install time, but nothing calls cashu_disconnect() on a
node change (no Rust caller outside the module, none in lib/), the already-connected early
return (api/cashu.rs:144) doesn't re-check the mint, and the other entry points check
is_cashu_mode() but never which mint. Cashu node A → B keeps operating against A's mint.
I couldn't find this in any phase — C3 is UI + providers, C10 is backup/reconciliation.
- Since you already treat the invariant as C2's in
connect, does completing it belong here too —
e.g. awith_wallet()helper validatingwallet.mint_url()againstescrow_mode::get_resolved()
so every entry point is covered — or is it a C3 concern once there's a UI to drive the switch?
Smaller
- The C2 checklist lists the bridge as
cashu_connect_status; this shipscashu_connect,
cashu_status,cashu_disconnectandcashu_check_proofs_state. Worth syncing that line while
you're in the doc (the pin note already landed there). cashu_receive_token/cashu_create_tokenhold theread()guard across the mint round trip;
tokio'sRwLockfavours writers, so a pending disconnect stalls everycashu_statusbehind it —
which C3's status stream would feel as a frozen screen. Hold anArc<CashuWallet>like
nwc::client?zeroizepullsfeatures = ["zeroize_derive"]but nothing derivesZeroize.the_proof_store_needs_an_initialised_databaserelies on no other test in the binary calling
init_db— true today, silently broken by the first one that does.- On your swap-fee question:
create_tokencollapses prepare+confirm and dropsprepared.fee(),
which is exactly the number C3 needs to show "you send 8, they receive 7". Return it alongside
the token so C3 doesn't have to reopen this signature?
Conflicts: - rust/src/api/types.rs: CashuWalletStatus (C2) and EscrowModeInfo (C1b) were added at the same spot; both kept. - rust/Cargo.lock, rust/src/frb_generated.rs: generated — regenerated from the merged Cargo.toml and rust/src/api/ (frb-generate.sh).
Phase C2 of
docs/cashu/README.md, plus the cdk spike the plan called forbefore writing it. Independent of C1 (#234) — the doc lists them as parallel.
Inert on Lightning. Every entry point in
api/cashu.rsreturnsCashuNotEnabledunlessescrow_mode::is_cashu_mode()is true, which requiresthe active node to have advertised Cashu and a usable mint. Nothing connects
at startup: a Lightning user never opens a proof store or contacts a mint. No UI
in this phase.
The wallet (
rust/src/cashu/)CashuWallet::connectbinds to one mint — the node pins it for every escrow,so ecash anywhere else has no counterparty — and verifies NUT-07, NUT-11, NUT-12
and a
satkeyset before returning. Checking at connect rather than at first useis the point: a seller who funds a wallet at a mint that cannot do NUT-11 would
otherwise discover it when the escrow lock fails, with their sats already there.
Operations:
balance,receive_token(swap-in, DLEQ-verified by cdk),create_token,check_proofs_state(NUT-07 reconciliation, so proofs reservedfor a token nobody redeemed come back instead of silently understating the
balance).
Two decisions worth flagging:
words the user already backed up — one secret to protect, not two.
cashu.sqlite, a sibling of the app DB, derived from thepath Dart already passes to
init_db(no new Dart call, and device paths stayDart's job). cdk owns that file's schema; mixing it into the app's would put
two migration systems on one file.
Web gets a typed stub returning
CashuUnsupportedOnWeb, mirroringcrate::nwc::client. The gap is storage alone — cdk compiles to wasm,cdk-sqlitedoes not.The bridge (
rust/src/api/cashu.rs)cashu_connect,cashu_status,cashu_get_balance,cashu_receive_token,cashu_create_token,cashu_check_proofs_state,cashu_disconnect, and anon_cashu_wallet_changedstream. Errors are stable markers; Dart localizes them.cashu_statusis deliberately ungated: the UI asks before it knows what the noderuns, and "not connected" is truthful on every node.
The spike (
docs/cashu/cdk-spike.md)Ran against cdk 0.17.3, everything verified rather than read off docs:
Conditions::newtakeslocktime, pubkeys, refund keys,
n_sigs,sigflagandn_sigs_refund;secrets round-trip back to conditions (so the buyer can verify a lock);
Proof::sign_p2pkattaches a per-proof witness;SendOptionscarries bothconditions and signing keys. C4 needs no hand-built secrets — this was the
question that could have doubled its size.
WalletDatabaseover IndexedDB is possible but is its own phase — thetrait is
async_trait(?Send)on wasm by design, but it has 50 methods and noIndexedDB backend exists at 0.17.3. C9 stays a phase, and it compounds with
Web: IndexedDB storage backend is a stub — nothing persists across a reload #233 (the app's own IndexedDB backend is a stub too).
wasm32-unknown-unknownwith--no-default-features --features wallet. Confirmed, not assumed.=0.17.3, default features off. Exact rather than caret: cdk ispre-1.0 and its wallet API moves between minor releases, with no test to catch
a silent bump until a trade fails. The doc carries an upgrade procedure.
Tests
Unit: capability gating and its markers (dropping any single NUT closes the door,
and the marker says which), the default capability set failing closed, every
bridge entry point shut on a Lightning node, idempotent disconnect that still
notifies, proof-store path refusing to guess without an initialised DB.
Integration against a local nutshell are
#[ignore]behindMOSTRO_TEST_MINT_URL, so CI stays green without a mint:A mock is not an option here — it would have to fake blind signatures and DLEQ
proofs, and a wallet that passes against faked cryptography has proven nothing.
Verification
./scripts/frb-generate.shwas run after therust/src/api/change.Test plan
cashu_statusreports not-connected; every other callreturns
CashuNotEnabled; nocashu.sqliteis ever created.token, see the balance move, export a token, redeem it in another wallet.
come back.
CashuMintUnusable: nut11ratherthan half-working.
Next: C3 (wallet UI) and C4 (escrow lock), which stack on this.
Summary by CodeRabbit
Manual verification
The wallet holds real money, so the checks below are in two halves: the
integration suite, which now actually runs (it did not before — see the last
commit), and the Lightning regression, which is the one that must not move.
Already run on this branch
Plus the integration suite against a real mint — 4/4, twice in a row
(nutshell 0.20.3). Reproduce it:
A · Integration suite against a live mint
Expect 4 passed. Run it a second time — it must still be 4 passed. (That
was broken until the last commit: fixed seeds replayed NUT-13 blinding secrets
and the mint refused them.
MINT_RATE_LIMIT=FALSEmatters for the same reason —nutshell rate limits by default and the second run gets 429s.)
What those four prove:
satkeyset;CashuMintUnreachablerather than hanging;signatures and DLEQ;
B · Lightning regression — nothing may happen on a Lightning node
Run the app against the default (Lightning) node.
this PR is reachable from any of it.
directory: there must be no
cashu.sqlitenext tomostro.db. If oneappears, the lazy-connect gate leaked.
~/.local/share/<app id>/(same directory asmostro.db)With the container running you can confirm by watching it:
docker logs -f nutshell— using the app must produce no requests.C · The gate, from the outside
There is no public Cashu daemon yet, so the wallet cannot be exercised through
the UI in this PR (there is no UI — that is C3). What you can check:
CashuNotEnabled.Covered by
api::cashu::tests::every_entry_point_is_shut_on_a_lightning_node,which asserts each door individually rather than trusting one of them.
Known limitation worth a reviewer's opinion
nutshell's default keyset charges a swap fee, so an 8 sat token redeems for 7.
The face value is what a validator checks, so this is correct behaviour — but it
means a user's balance drops slightly on every send. Whether the UI should
surface that is a C3/C10 question, not one this PR answers.