Repository navigation
fix: credits-first buyer mint choice and private settlement parity - #1100
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
maxie-agent
marked this pull request as ready for review
October 6, 2026 03:50
This was referenced Oct 6, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Status
Advisor R2 finding 1 is fixed at
4bb84c81beab40d37c7ff61b9e97019b816ce27f; local verification and all eight CI jobs are complete. Free advisor R3 verdict: Approve, no critical/high/medium findings. This PR remains a draft; no merge is authorized. Bob's credits-first decisions are final; no merge is authorized.Root cause and change
The source selector followed seller order even when the buyer held enough credits. The hop planner could also choose a credit mint, which cannot issue a Lightning mint quote, so some jobs failed only after delivery. Award filtering planned from the default mint rather than the balance-aware source. Private invoice validation additionally rejected an entire claim when any listed mint was not configured by the checker.
nostr://balances, then covering HTTPS balances, preserving seller order within each pass and retaining the real-mint fence.money_lock; selection uses net-available balances (excluding this job’s own hold), while reservation enforcement retains raw balances. Balance-read failure is surfaced before mint filtering.realized_mintis the funding source, so checking that field for membership would incorrectly reject hops.Cases
C = seller-listed credits, L = seller-listed Lightning mint, X = buyer-default Lightning mint not listed by seller. Enough means net available after other jobs’ live reservations at one configured mint; payments never split.
https://in seller orderPays-once verification
Before changing production code, a valid signed inline payment was run through unchanged
authorize_pay_asyncat9ba25cdagainst a real local credit sidecar. It failed at target mint quote: unsupported, before source quote/melt, budget charge, payment journal, or crossmint journal: 1 passed, 0 failed. The permanent regression also exercises the upgraded HTTPS replan. Same-result re-accept is tested against a local relay with newly arrived credits and unchanged attempt identity.Review fixes (authorized by bob, 2026-10-05 23:47 / 23:50 UTC)
money_lock; attempt resolution stays outside the lock, the pinned-attempt re-check remains, and filter/ceiling/reservation share one snapshot under the guard.reserve_at.CdkHopEffects::openrefusesnostr://targets before opening either wallet, mirroring source refusal.Fix-specific regressions
manual_award_relay_fetch_does_not_hold_money_lockexercises the real manual handler against a blocked relay handshake. Moving the lock back above fetch: 0 passed / 1 failed / 0 ignored, at the intended mutex assertion. Restored: 1/0/0.reserved_credits_choose_sats_for_filter_ceiling_accept_and_award,reserved_credits_filter_uses_available_sats_on_manual_and_auto_paths, and extendedaward_ceiling_mint_matches_the_accept_selection. Restoring raw-balance selection: 0/2/0, both reserved-credit tests fail at the intended assertions. Restored reserved-credit tests: 2/0/0; parity: 1/0/0.a_nostr_target_cannot_open_a_hop: 1/0/0. No mutation requested/performed.balance_read_failure_diagnostic_is_not_a_credit_hop_refusal: 1/0/0. Shared manual/auto gate and park-reason test; no mutation requested/performed.private_invoice_rejects_normalized_equal_mint_aliasesandseller_creq_deduplicates_normalized_mints: each 1/0/0. Seller test puts the:443alias first; no mutation requested/performed.Deferred
Review-fix verification (previous head
25c2d25)All suites use
--locked; counts aggregate all 33 emitted targets, including doc-tests.cargo test -p maxplayer-core --release --no-default-features --features gateway,git-delivery,wallet,live-mints --lockedcargo test -p maxplayer-core --release --features acp,gateway,git-delivery,wallet --lockedcargo fmt --checkwas run and exits 1 on existing repository drift; changed files introduce no new rustfmt drift compared with4520cc2, andgit diff --checkpasses. No whole-file formatting rewrite. CI has no fmt/clippy gate.cargo clippy -p maxplayer-core --features acp,gateway,git-delivery,wallet --all-targets --locked --no-depsinitially exits 101 solely for pre-existingunused_io_amountat untouchedgit_transport.rs:2129. Rerun with-- -A clippy::unused_io_amountpasses (exit 0); no added-line diagnostics. CLI fixtures were not touched; CLI tests were not rerun in this fix round. Ignored tests remain unverified.Previous-head verification (
4520cc2)All test commands below used
--locked; counts aggregate all targets emitted by each command. Ignored tests are not claimed as verified.9ba25cd, real credit sidecarcargo test --manifest-path crates/maxplayer-mint/Cargo.toml --locked--features acp,gateway,git-delivery,wallet--no-default-features --features gateway,git-delivery,wallet,live-mints--no-default-features --features wallet,acp--features walletmaxplayer-private-protocol--no-default-features --features gateway,git-delivery,wallet --lib credits_first(before/after mutations)The sidecar's ignored child helper is explicitly invoked by the passing HTTPS fixture parent; mint lib/bin/doc targets each had zero tests. The shipped build passed:
cargo build -p maxplayer --release --no-default-features --features wallet,acp --locked.All four spec mutations produced their intended named test failure (each 0 passed / 1 failed, exit 101; not a compiler failure):
credits_first_hop_skips_credit_targets_in_every_positioncredits_first_source_selection_matrixcredits_first_extra_credit_is_awardable_on_manual_and_auto_pathscredits_first_realized_mint_must_be_in_signed_creqThe isolated mutation checkout was restored byte-for-byte; restored baseline 8/0/0. Hash verification confirmed mutations never altered the primary worktree.
Formatting: changed-line
rustfmt --edition 2024check andgit diff --checkpass, preserving pre-existing unrelated formatting. CI itself has no fmt/clippy job. Core/CLI all-targetclippy --no-depsand sidecar clippy were run; no diagnostics on changed lines. Core's initial all-target run was blocked by pre-existingclippy::unused_io_amountin untouchedgit_transport.rs:2129(confirmed on the base); rerunning with only-A clippy::unused_io_amountpassed. CLI and sidecar clippy passed without that exception.Spec refinements and limitations
money_lock; pinned-attempt resolution remains outside the lock to avoid reentrant deadlock.request.realized_mintis actually the funding source and may legitimately be outside that list.Scope and rollout
pay_from, protocol/relay change, new bind field, or store migration.extra_mintskeeps its existing meaning. No relay deployment is required.Fixes #1039
Refs #1069 — the private mechanism is fixed; the public repro remains unexplained.
Refs #1092
Spec: docs/specs/buyer-mint-choice.md.
Previous-head CI and advisor R2 re-review (
25c2d25)Head:
25c2d25d510c9a6a12eb12974200adf603be29f2. CI run 37397883092 — success.Read-only advisor re-review requested at this head, targeted job
7f3064dea54b42310a5daa08f57f7d3efa54a9e08cdf01704ff74329a125b1ce, paymentnone, 0 sats. Collected successfully for 0 sats (payment statenone). Advisor verdict: all five scoped fixes resolved, but one new high finding; no medium findings. No project tests were independently rerun by the advisor.Author validation of advisor R2 finding 1: CONFIRMED routing regression. At H,
gateway.rs:1460-1470strips HTTPS:443when emitting seller CREQs, whilewallet_ops.rs:720-728andcrossmint.rs:201-209retain that port in buyer balance identity. Inspected immutablegit show H:<path>. An offline probe calling the compiled productionbuild_seller_creq,parse_creq,normalize_mint_url, andplan_paymentconfirms:https://mint.example:443; seller configured identically.https://mint.example; wallet identity remainshttps://mint.example:443.No network/payment was executed; the report's possible payment-loss outcome is unverified, not established by this reproduction. A repair must preserve existing wallet/reservation identities; a normalization change alone must not bypass existing reservations. That R2-reviewed head is historical; the R3 fix below addresses the confirmed routing regression. PR remains draft; no merge authorized.
Advisor R2 finding 1 fix / R3 verification
Head:
4bb84c81beab40d37c7ff61b9e97019b816ce27f. Authorized by bob (option 1), 2026-10-06 01:58 UTC. Narrow delta from25c2d25; base9ba25cd.gateway.rs: seller creq canonicalization/deduplication usesMintUrl::from_stroutput, preserving explicit:443, seller order and first occurrence. A separate URL-structure validation discards the parsed URL; it cannot rewrite wallet identity.private_content/invoice.rs: duplicate keys use the sameMintUrlidentity; existing well-formedness checks remain. Corrected the misleading default-port comment. No wallet, reservation, lock, or pays-once logic changed.seller_creq_deduplicates_normalized_mintsandprivate_invoice_rejects_normalized_equal_mint_aliases: slash/case aliases collapse; explicit:443and no-port entries remain distinct.crossmint::tests::seller_creq_explicit_default_port_uses_held_mint_and_pays_directfollows realbuild_seller_creq → parse_creq → select_source_mint → plan_payment, with a full-price configured:443balance. AssertsDirect,holds_at_least, and selection with both same-mint and different-default fallbacks.Mutation evidence
Reintroducing the
Url::parsecanonicalization pre-pass in the builder produces 0 passed / 1 failed / 0 ignored at the intendedDirectassertion: actualHop { source: https://mint.example:443, target: https://mint.example }. Restored code is byte-identical (SHA-25690eb459a1bda1b7bffd16a77804fcd2692ce2ef5d6afd2528afa18be1c68a9c7), and restored regression passes 1/0/0.Exact local results at this head
cargo test -p maxplayer-core --release --no-default-features --features gateway,git-delivery,wallet,live-mints --lockedcargo test -p maxplayer-core --release --features acp,gateway,git-delivery,wallet --locked--lib seller_creq_):443regressionFull-suite totals each aggregate 33 emitted targets including doc-tests. Both completed commands exited 0. Clippy passed:
cargo clippy -p maxplayer-core --release --features acp,gateway,git-delivery,wallet --locked --all-targets --no-deps -- -A clippy::unused_io_amount; no added-line diagnostics. CI has no clippy/rustfmt step.git diff --checkpasses; no whole-file formatting.CI: run 37408045671, all eight jobs success on H; single watcher exited 0. Free advisor R3 job
814f55663406e2f6b91cd5b9e76e06e59b257ada9ebff26b861c9e99c99e9e3awas collected successfully with paymentnone, amount 0 sats, payment statenone. R3 verdict: Approve; no critical/high/medium findings. Ignored tests, a live:443mint payment, and production rollout remain unverified. F2/L7 and #1101 remain deliberately deferred as above. No merge, main push, Discord post, or GitHub comment.Completed advisor R3 review and author validation
Advisor verdict: Approve. R2 finding 1 resolved; no critical/high/medium findings and no blocking low findings. There are no high/medium findings to classify as confirmed/refuted/unclear. Author validation confirms the repair at immutable H: gateway.rs:1463-1472 emits/dedupes MintUrl spelling, invoice.rs:117-124 retains well-formedness and the same identity, crossmint.rs:312-343 exercises Direct from the configured :443 balance.
Confirmed nonblocking coverage limitation: private_content/tests.rs:1734-1739 constructs a PaymentRequest through serde before encoding. cashu 0.17.2 MintUrl deserialization already folds case/slashes; therefore the private test does not independently prove folding of raw unequal CBOR aliases. It does prove the explicit-port distinction, and the builder test covers raw aliases. Optional raw-CBOR strengthening is not required for this narrow fix. No product defect claimed; no further code changed.
Advisor independently inspected the pinned source/dependencies and verified all eight CI job conclusions on H. It did not rerun local suites, mutation, or clippy; those are author-run evidence. Ignored tests, a live :443 payment and production rollout remain unverified. F2/L7 and #1101 remain deliberately out of scope.
CI job conclusions at H:
Review job
814f55663406e2f6b91cd5b9e76e06e59b257ada9ebff26b861c9e99c99e9e3a, delivered commitffd2c60a3daa28f1272cb90832ef3e5a521cfd6c; sats spent in this run: 0. No code changes after advisor review.