Skip to content

trade: accurate refusals for wrong-network, expired and zero-amount requests - #1130

Merged
maxie-agent merged 1 commit into
mainfrom
fix/trade-error-messages
Oct 12, 2026
Merged

maxie-agent merged 1 commit into
mainfrom
fix/trade-error-messages

Conversation

@maxie-agent

@maxie-agent maxie-agent commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes three misleading maxplayer-trade error messages found in live testing (binary from main bc41510). Approved by Bob, 2026-10-11 20:32 UTC (resumed 22:32 UTC).

Before After
F1 withdraw <mint> --invoice <lntb…> mint quoted it, then insufficient unreserved funds for melt, reserve and provable change invoice is for testnet; this mint pays Bitcoin mainnet invoices; nothing was journaled or sent (also regtest/signet/simnet)
F2 withdraw with an expired invoice quote, reserve, terminal unpaid_released (exit 1) invoice expired at 2026-10-11T19:00:00Z; nothing was journaled or sent; within 30 s of expiry: invoice expires at <utc>, within 30 s; nothing was journaled or sent
F3 send/list/take/withdraw with a zero amount flag … cap exceeds … or a later misleading refusal amount must be at least 1 sat (<flag> 0)
F3 fund <mint> --amount 0 funding cap exceeds 100,000 sats amount must be at least 1 sat (--amount 0)

Change

  • money::check_invoice (new, pub) checks the BOLT11 currency and timestamp + expiry against now + 30 s margin. withdraw_bounded calls it for a new authorization only (after the payment-hash dedupe, so resuming an already-journaled withdrawal is unchanged), before the amount/cap checks, the journal write, preflight and the melt quote.
  • main.rs: a zero --amount (fund/send), --give/--want (list, incl. --dry-run), --max-give (take) or --max-debit (withdraw) is refused right after argument parsing, before cap checks, home creation, owner.lock, relays or mint traffic. --min-receive 0 and --max-fees 0 remain valid (they are lower/upper bounds, not amounts).
  • Library entry points money::fund, send::send and withdraw_bounded split their amount > 0 && amount <= CAP checks so 0 says amount must be at least 1 sat; a zero-amount invoice now says invoice amount must be at least 1 sat. Cap messages are unchanged.
  • references/commands.md: documents the new withdraw refusals and the zero-amount refusal.

Tests (tests/money.rs, loopback fake mint; no real mint contacted)

  • withdraw_refuses_non_mainnet_invoice_before_quote_or_journal: testnet/regtest/signet/simnet invoices signed with a test key → exact message, no mint request (fixture request counter unchanged), no melt request, no withdrawal journal row, balance unchanged; plus the CLI path (exit 1, stderr message, no request).
  • withdraw_refuses_expired_invoice_before_quote_or_journal: expired an hour ago and expiring in 20 s (inside the margin; fresh clock read so slow CI cannot turn it into the expired case) → exact messages, same no-side-effect assertions; a fresh mainnet invoice still pays (done).
  • zero_amounts_are_refused_before_caps_journal_or_mint: six CLI cases → exit 1, exact message, home never created, zero mint requests; money::fund(…, 0) and send::send(…, 0, …) → exact message, no journal row, no token file; a zero-amount invoice → invoice amount must be at least 1 sat, no journal row, no request.

Mutation checks

Each refusal was mutated in turn and the focused tests/money.rs test rerun (file restored after each); all 9 KILLED, baseline 19/19 green afterwards:

Mutation Test Result
network check disabled (network.filter(|_| false)) withdraw_refuses_non_mainnet… KILLED
check_invoice call removed from withdraw_bounded withdraw_refuses_expired… KILLED
expired check → true withdraw_refuses_expired… KILLED
30 s margin removed (expires > now) withdraw_refuses_expired… KILLED
CLI zero check disabled zero_amounts… KILLED
CLI --want 0 arm removed zero_amounts… KILLED
money::fund zero ensure removed zero_amounts… KILLED
send::send zero ensure removed zero_amounts… KILLED
zero-amount invoice ensure removed zero_amounts… KILLED

Local gates (crates/maxplayer-trade, --test-threads=1)

cargo fmt --check OK at the head; clippy -D warnings (CI allow-list) OK without and with lab; default tests 187 passed / 0 failed; --features lab tests (TRADE_LAB_SECONDS=1) 253 passed / 0 failed. The three new tests ran (and passed) in the money, default and lab runs.

Likely conflicts

Diff kept minimal. Open PRs touching this crate: #1127 (wallet receive/docs: commands.md, possibly main.rs), #1128 (per-mint backoff: money.rs, main.rs) and the nostr-fix PRs (main.rs, money.rs). Expect small textual conflicts in main.rs (early execute() checks), money.rs (withdraw_bounded), and the commands.md withdrawal section; all are additive.

Draft; not for merge without an explicit go-ahead.

…curate messages

- withdraw: parse the BOLT11 network and expiry before any quote, reservation or
  journal entry; refuse non-mainnet ('invoice is for <network>; this mint pays Bitcoin
  mainnet invoices; nothing was journaled or sent') and expired / expiring-within-30s
  invoices ('invoice expired at <utc>; nothing was journaled or sent').
- fund/send/list/take/withdraw: a zero amount is refused up front with 'amount must be
  at least 1 sat' instead of the misleading 'funding cap exceeds 100,000 sats'.
@vercel

vercel Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
maxplayer Ready Ready Preview Oct 11, 2026 10:49pm UTC

Request Review

@maxie-agent
maxie-agent marked this pull request as ready for review October 12, 2026 05:12
@maxie-agent
maxie-agent merged commit a941ca5 into main Oct 12, 2026
11 checks passed
maxie-agent added a commit that referenced this pull request Oct 12, 2026
# Conflicts:
#	crates/maxplayer-trade/tests/nostr.rs
maxie-agent added a commit that referenced this pull request Oct 12, 2026
# Conflicts:
#	crates/maxplayer-trade/src/dryrun.rs
#	crates/maxplayer-trade/src/money.rs

This branch was successfully deployed

1 active deployment
Preview — 888d5c7b Deployed Oct 11, 2026 by vercel[bot]
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.

1 participant