feat(cashu): seller-funded fee token (Option 2) — Track A TA-1f - #832
feat(cashu): seller-funded fee token (Option 2) — Track A TA-1f#832grunch wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49361481c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // The NUT-11 tag block is optional for a bare 1-of-1 (absent ⇒ | ||
| // SIG_INPUTS, n_sigs = 1, no extra keys). When present, enforce it. | ||
| if let Some(conditions) = conditions { |
There was a problem hiding this comment.
Reject refundable Cashu fee tokens
When a seller includes NUT-11 locktime/refund tags on the fee token, this branch still accepts it because it only checks sig_flag, n_sigs, and extra signing pubkeys. The existing escrow validation treats refund_keys as an alternate post-locktime spend path, so a malicious seller can submit a fee token that is initially valid but later spendable by the refund key instead of being exclusively controlled by Mostro, especially while live redemption is deferred. Reject refund/locktime tags here, or enforce a safe fee-token recovery policy.
Useful? React with 👍 / 👎.
| // is validated against the mint alongside the escrow token — both must | ||
| // be valid before any state changes. A fee-free node (`mostro.fee == 0`) | ||
| // accepts a `None` fee token. | ||
| let charges_fee = Settings::get_mostro().fee > 0.0; |
There was a problem hiding this comment.
Skip fee tokens when the stored fee is zero
This gates fee-token enforcement on the configured percentage rather than the actual per-order fee. For small orders where get_fee rounds the stored order.fee to 0 even though mostro.fee > 0 (for example a 100-sat order at a 0.3% fee), the handler requires a fee_token and then expects a 0-sat token, so otherwise valid zero-fee orders cannot be locked. Use the calculated order fee (order.fee > 0) to decide whether a fee token is required.
Useful? React with 👍 / 👎.
Add Cashu fee collection: the seller funds the WHOLE Mostro fee (`2 * order.fee`) as a separate P2PK-1-of-1 token locked to Mostro's arbitrator key, submitted alongside the 2-of-3 escrow in the same `AddCashuEscrow` (docs/cashu/02-track-a-lock.md §4A). This keeps total revenue and dev-fee accounting identical to Lightning; only the buyer/seller split moves (the seller bears the buyer's half too, since the buyer redeems the escrow token freely). - Migration: `cashu_fee_token` + `cashu_fee_redeemed_at` on `orders`, and a `cashu_fee_proofs (y PRIMARY KEY, order_id, created_at)` anti-reuse table. - `CashuClient::verify_fee_token` + `verify_fee_condition` (P2PK 1-of-1 to `P_M`, amount, sat unit, DLEQ, unspent) and `proof_ys_hex`. Offline unit tests for the condition + Y derivation. - `db::update_order_cashu_escrow_with_fee`: transactional CAS that persists the escrow, the fee token, and every fee-proof `Y` in ONE transaction — crash can't leave a locked order whose fee is unrecorded, and the `y` PRIMARY KEY makes two concurrent submissions of the same fee token roll back the second (`CashuLockOutcome::FeeReuse`), closing the TOCTOU that the unspent check (sequential-only) leaves open. Plus `find_pending_cashu_fee_redeems` / `mark_cashu_fee_redeemed`. - Handler: when `mostro.fee > 0`, require + validate the fee token after the escrow token (missing ⇒ `CashuSignatureMissing`), persist it in the atomic CAS, and reject cross-order reuse (`InvalidCashuToken`). Fee-free nodes accept a `None` fee token unchanged. - Scheduler: cashu-only `job_retry_cashu_fee_redeem` surfaces the pending redeem backlog from the DB. Tests: the transactional CAS (Locked / StatusMismatch no-op / FeeReuse rollback), pending-redeem lifecycle, and the 1-of-1 fee condition + Y derivation. The migrated test schema gains the fee columns + table. Scoped follow-up: the LIVE redeem (sign each fee proof with P_M, swap at the mint, stamp `cashu_fee_redeemed_at`) needs an ecash revenue store (Cashu mode has no LND to melt to) and mint-backed tests — the retry job body is the seam. Until then fee tokens are validated, persisted, and anti-reuse-guarded, so no revenue is lost, only deferred. Depends on TA-1 (+ mostro-core 0.14.1 fee_token). Base: feat/cashu-ta3-restore-monitor Refs: docs/cashu/02-track-a-lock.md (TA-1f, §4A)
d39115c to
456c3f4
Compare
4936148 to
4a8c6b8
Compare
ToRyVand
left a comment
There was a problem hiding this comment.
Reviewed the TA-1f diff. The transactional CAS is the right shape and the TOCTOU reasoning behind cashu_fee_proofs is sound. One finding that I think can strand an order, plus a risk in the deferred-redeem framing worth naming.
Verified: 1033 passed / 0 failed (matches the checklist exactly), cargo clippy --all-targets -- -D warnings clean, cargo fmt --check clean.
The fee arithmetic checks out
Worth confirming since it's the business claim the PR rests on: get_fee() (src/util.rs:81-86) is (mostro.fee * amount) / 2, so order.fee stores the per-side half and Lightning collects it from both sides. 2 * order.fee is therefore exactly the Lightning total, and "total revenue and dev-fee accounting stay identical to Lightning; only the buyer/seller split moves" holds precisely, including the double-rounding case (both paths compute 2 × round(pct·amount/2), so they agree by construction).
Also confirmed CashuLockProof.fee_token is present in the 0.14.5 mostro-core this repo pins (message.rs:609), so the stated 0.14.1 dependency is satisfied.
1. The fee gate and the fee amount read from different sources — an order can become unlockable
In add_cashu_escrow.rs:
let charges_fee = Settings::get_mostro().fee > 0.0; // live setting
let fee_ctx = if charges_fee {
let fee_token = proof.fee_token.clone()
.ok_or(MostroCantDo(CantDoReason::CashuSignatureMissing))?;
let expected_fee = u64::try_from(order.fee.saturating_mul(2)) // stored at creation
…The requirement is decided by the live [mostro] fee, but the amount is derived from the stored order.fee, frozen when the order was created. verify_fee_token then demands exact equality (src/cashu/mod.rs:419, if value != expected_fee).
So if an operator raises the fee while orders are in flight:
- Order created while
fee = 0→order.fee = 0. - Operator sets
fee = 0.006, restarts. - Seller submits
AddCashuEscrow.charges_feeis now true, so a fee token is required — butexpected_fee = 2 * 0 = 0. - No token can satisfy an exact-zero amount, so the lock is refused on every attempt and the order sits in
WaitingPaymentindefinitely.
The reverse direction is milder but also inconsistent: an order created with fee > 0 that is locked after the operator sets fee = 0 skips the requirement entirely and ignores a supplied token, so Mostro forgoes revenue the parties had already agreed to.
Both fall out of the same mismatch, and the fix is small: gate on order.fee > 0 rather than the live setting, so the requirement and the amount come from the same frozen value the parties agreed to. Changing [mostro] fee mid-flight is ordinary operator behaviour and WaitingPayment can persist for a while, so I don't think this is only theoretical.
2. "No revenue is lost — only deferred" depends on a third party's keyset lifetime
The scoped follow-up is the right call and the seam (job_retry_cashu_fee_redeem) is in the right place. But the deferral is currently unbounded: the fee token is persisted as a string, cashu_fee_redeemed_at stays NULL forever, and nothing in this PR can ever collect it.
That's fine as long as the mint keeps honouring the keyset those proofs were minted under. Keyset rotation (NUT-02) is mint policy and outside Mostro's control — most mints keep accepting inactive keysets for swaps, but it isn't guaranteed, and a node accumulating fee tokens over months against a third-party mint is carrying a receivable it cannot age-check. Worth naming in the follow-up: either a deadline/alert on the oldest pending redeem, or a line in the spec stating the assumption explicitly, rather than leaving "only deferred" resting on an unstated mint property.
3. Notes
- The stack is 37 commits behind
main, and this sits on top of two PRs that haven't been reviewed yet (#830 TA-2, #831 TA-3), so it can't land before them. Reviewing the stack bottom-up would help — I've left comments on #830; #831 is still untouched. - A fee token containing the same
Ytwice would hit the primary key on the second insert inside the loop and surface asFeeReuse— arguably the right answer for a malformed token, but the operator-visible reason would say "reuse" for what is really a duplicate-proof token. Only worth a comment ifverify_fee_tokendoesn't already reject duplicates upstream. cashu_fee_proofshas no index onorder_id, but nothing in production queries it that way (INSERT only,db.rs:927) — so unlike thebondsmigration'sidx_bonds_order_id, none is needed here. Noting it so a reviewer doesn't flag it as an omission.
Called out as done well: the rollback-by-Drop comments on both early returns make the transaction semantics obvious at the call site instead of relying on the reader knowing sqlx's Transaction behaviour, and the migration's explanation of why the escrow token's per-order 2-of-3 makes it self-limiting while the node-wide P2PK fee token needs an explicit ledger is exactly the reasoning that stops someone deleting the table later as redundant.
Contributor, not a maintainer — technical review only, no merge signal implied.
What & why
Cashu fee collection (
docs/cashu/02-track-a-lock.md§4A, Option 2 — funder-pays-at-lock). Mostro is deliberately out of the money path in Cashu mode (the buyer redeems the 2-of-3 in full), so it cannot skim a fee output from the escrow token. Instead the seller funds the whole Mostro fee (2 * order.fee) as a separate P2PK-1-of-1 token locked toP_M, submitted in the sameAddCashuEscrow.Total revenue and dev-fee accounting stay identical to Lightning; only the buyer/seller split moves (the seller bears the buyer's half too, since nothing can deduct from a token the buyer redeems freely).
Changes
cashu_fee_token+cashu_fee_redeemed_atonorders; newcashu_fee_proofs (y PRIMARY KEY, order_id, created_at)anti-reuse table.CashuClient::verify_fee_token+verify_fee_condition(P2PK 1-of-1 toP_M, amount ==2*order.fee, sat unit, DLEQ, unspent) +proof_ys_hex. Offline unit tests for the condition and Y derivation.db::update_order_cashu_escrow_with_fee: a transactional CAS that persists the escrow, the fee token, and every fee-proofYin one transaction:yPRIMARY KEY makes two concurrent submissions of the same fee token roll back the second (CashuLockOutcome::FeeReuse). The NUT-07 unspent check only stops sequential reuse; this closes the concurrent gap.find_pending_cashu_fee_redeems/mark_cashu_fee_redeemed.mostro.fee > 0, require + validate the fee token after the escrow token (missing ⇒CashuSignatureMissing), persist it in the atomic CAS, reject cross-order reuse (InvalidCashuToken). Fee-free nodes accept aNonefee token unchanged.job_retry_cashu_fee_redeemsurfaces the pending-redeem backlog.Tests
Locked(fee persisted, not yet redeemed) /StatusMismatchno-op /FeeReuserolls back the escrow write too.find_pending…→mark…redeemed).Scoped follow-up — the live redeem
The live redeem (sign each fee proof with
P_M, swap at the mint, stampcashu_fee_redeemed_at) is deliberately not in this PR. It needs (a) an ecash revenue store — Cashu mode has no LND to melt the fee to, so the swapped proofs must be persisted — and (b) mint-backed tests.job_retry_cashu_fee_redeemis the seam where it lands. Until then fee tokens are validated, persisted, and anti-reuse-guarded, so no revenue is lost — only deferred. This split matches the spec: TA-1f "ships the crash-safe fee persistence andcashu_fee_proofsanti-reuse guard".Checklist
cargo fmt --checkcargo clippy --all-targets --all-features -- -D warningscargo test— 1033 passedDepends on #831/#830/#829/#828 and
mostro-core 0.14.1(CashuLockProof.fee_token). Refs:docs/cashu/02-track-a-lock.md(TA-1f, §4A).🧪 Manual testing — step by step
Prereqs: the Cashu-mode
mostrodsetup from CF-5 (#828) — mint up (docker compose -f docker-compose.cashu.yml up -d),[cashu] enabled = true. This branch is the top of the Track A stack (contains CF-5 + TA-1 + TA-2 + TA-3 + TA-1f). Fee collection only engages when the node charges a fee:Run with
RUST_LOG=mostro=info.1. Fee token is required when the node charges a fee
[mostro] fee > 0, drive an order toWaitingPayment(take flow) and submitAddCashuEscrowwithout afee_token.CantDo(CashuSignatureMissing)— the order staysWaitingPayment, nothing is locked. ✔️fee = 0) accepts aNonefee token unchanged (no fee required). ✔️2. Valid fee token locks + persists atomically
AddCashuEscrowwith the 2-of-3 escrow token and a P2PK-1-of-1 fee token locked toP_Mworth2 * order.fee, both minted on the test mint.Active, and the fee is persisted crash-safely in the same write:3. Cross-order fee reuse is rejected (anti-TOCTOU)
AddCashuEscrowreusing the same fee token.CantDo(InvalidCashuToken), and the second order's escrow write is rolled back (stillWaitingPayment, nocashu_escrow_token) — thecashu_fee_proofs.yPRIMARY KEY blocks the reuse inside the transaction. ✔️4. Pending-redeem backlog is surfaced
5. Automated tests
Expected: all green —
Locked/StatusMismatchno-op /FeeReuserollback, the fee condition (acceptP_M; reject wrong/extra key or 2-of-N), and deterministic Y derivation. ✔️