Repository navigation
fix(config): per-mint payment method for custom-unit mints, and make the cdk tag compile - #1185
felixfelix-bot wants to merge 3 commits into
Conversation
…gain
Two defects found while working out how a Pecan-settled mint could attach.
1. The custom-unit payment method was derived from the UNIT and defaulted to
"paypal" via TOLLGATE_CDK_PURCHASE_METHOD:
if m := os.Getenv("..."); m != "" { return m }
return "paypal"
No real processor answers "paypal" — it was a bench-harness stub promoted to
a shipped default. Against a Pecan-backed mint, which registers the method
"branch", every issuance quote went to a method the mint does not have, so
issuance could never work. It also put a per-MINT fact in a per-PROCESS env
var, so one router could not front a counter-settled operator mint and a
bolt11 sat mint at the same time.
Now accepted_mints[].payment_method (empty = the unit's default), plumbed
through MintConfig -> merchant -> cdk/gonuts/select. A custom-unit mint with
no configured method is REFUSED with an actionable error rather than sent to
a guessed one.
2. The cdk_wallet tag had never compiled. go vet -tags cdk_wallet failed on
w.ActiveKeyset: this module pins a patched cdk-go whose accessor is
FetchActiveKeyset returning KeySetInfo. So the backend that makes custom
units work was never build-verified by any test. Fixed the probe to the
pinned API; the four NewWalletPort call sites that my new argument broke are
updated too. is clean for the first time.
Also adds the schema entry for payment_method (the config/struct drift fence
caught its absence) and docs/pecan-teller-integration.md, which records what C
still needs and why the method-name mismatch was the blocker.
Verification: scripts/go-test.sh RC=0, 20 modules, 0 failures; gofmt clean;
go vet -tags cdk_wallet clean; TestCustomUnitPaymentMethod_* pass under the tag.
… by gonuts Ran a real cdk-mintd 0.18.3 with unit="mb" and custom_payment_methods=["branch"] — the shape a Pecan-fronted mint has — and drove IssueOnRequest against it. No funds (fakewallet); keysets, quotes and proofs real. Works on the cdk backend: quote -> PAID -> ISSUED, and the returned token decodes to mb with no sat in it. The mint's own log confirms method=branch unit=mb at every state transition. Refused on the DEFAULT (shipping) gonuts backend with 'could not find an active keyset for the unit'. Cause: gonuts-tollgate hardcodes unit: cashu.Sat at wallet construction (wallet/wallet.go:128) and AddMint looks up keysets with w.unit, so an mb-only mint has no match. That is the real blocker for the Pecan ticket, and it is not the method name: custom-unit redemption is reachable only via the cdk_wallet build tag, which the router build cannot use in-process because cdk-go is CGO/FFI and the module must stay CGO_ENABLED=0. The sanctioned route is the sidecar. Also gitignores mbtest/, the harness that holds the mint and wallet dbs.
|
LGTM — approve. The This is the first PR in this sweep where the CDK path was actually executed — worth noting because #1136 and #1155 could only be verified in their default (gonuts) configuration. The bug is a good one, and the fix matches itThe test header states the failure precisely: the payment method was derived from the unit and defaulted to Making the method a per-mint config fact rather than a unit-derived guess is the correct model, and the reason given is the decisive one: "one router can front a counter-settled operator mint and a bolt11 sat mint at the same time, which a unit-derived guess cannot express." That is a genuine architectural constraint, not a preference. The second test is the one I would have asked for: One thing to check before merge
Also: this PR carries No changelog fragment concern — one is included ( |
|
Follow-up on my own review — the changelog fragment on this branch is wrong, and it is empty. I flagged this as "worth confirming" and then confirmed it: So two defects in one file, both worth fixing before merge:
Suggested replacement: Everything else in my review standsThe code is right and the tests pass for real ( The two sequencing notes also stand: Minor process note for this PR's author: an empty fragment file is caught by |
The fragment cited OpenTollGate#1185, which is a different PR (per-mint payment method for custom-unit mints). A fragment pointing at the wrong PR links the changelog entry to unrelated work at fold time.
Review — PR #1185, head
|
Local CI gate — PR #1185: fix(config): per-mint payment method for custom-unit mints, and make the cdk tag compileRun of
Verdict: 3 leg(s) RED — 3 green. What this gate does not run (no vacuous green)
failing-leg log tailscontract-fences — tailpackaging-sequential — tailpackaging-concurrent — tail |
Amperstrand
left a comment
Amperstrand
left a comment
There was a problem hiding this comment.
The code is verified good; the changelog fragment points at the wrong PR and must be fixed before merge.
Fragment (blocking, mechanical): changelog.d/1150-custom-unit-payment-method.changed.md is named after — and both bullets link — #1150, which is a different, already-merged PR (Cobrador peering / SSID names / portal pin). Rename to 1183-style convention, i.e. 1185-custom-unit-payment-method.changed.md, and point both link tails at this PR. Related shape nit: the fold expects one bullet per fragment ("the file body is the bullet"); the second bullet (cdk tag compile fix) deserves its own 1185-cdk-wallet-tag-compiles.fixed.md.
Code verified at head 152c29a27a28caad7d06e88aa07e71e1a793412f:
- Plumbing is coherent end to end:
MintConfig.PaymentMethod→ schema entry →newFullMerchant's per-mint map →NewWalletPort/OpenWallet→ the cdk backend's quote method; the gonuts backend takes it for signature parity and ignores it, documented as such. - The refusal-instead-of-guess in
RequestMintQuoteis the right call, with an actionable error naming the config key. - Gates re-run:
src/tollwalletdefault tests ok (59s suite);go vet -tags cdk_wallet ./...clean — confirming the tag-compiles claim;src/config_managerand targetedsrc/merchanttests ok;js-schema-lint.mjsPASS (113 entries — the struct/schema fence agrees);check-changelog-duplicates.pyclean. - Sat-mint neutrality holds: empty
unit/payment_methodkeeps the previous path.
Dependency flag (as the body honestly states, worth keeping visible): custom-unit support on the default backend depends on gonuts-tollgate #51 (unit-parametric wallet), which is OPEN with changes requested — wallet/wallet.go still hardcodes cashu.Sat. Until #51 lands and is tagged/bumped, accepted_mints[].payment_method is only effective behind the cdk_wallet tag, which the router build cannot use in-process (CGO constraint). That is a declared limitation, not a defect in this PR, but reviewers/operators reading the changelog bullet should not conclude Pecan-style mints work on stock hardware from this merge alone. The NUT-20 quote-locking gap (RequestMintQuote passes nil) is likewise declared open.
Disposition: request-changes for the fragment rename/links only — with that fixed this lands, with the gonuts #51 dependency noted in the merge train.
Follow-up to #1040, which merged as
8e084b9d. That PR made custom units priceable; these are two defects found while working out how an operator-settled mint (Pecan) would actually attach.1. The payment method was a hidden env var with a wrong default
No real processor answers
paypal— it was a bench-harness stub promoted to a shipped default. Against a mint whose processor registers any other name, every issuance quote addresses a method the mint does not have, so issuance can never succeed. It also put a per-mint fact in a per-process environment variable, so one router could not front a counter-settled operator mint and a bolt11 sat mint simultaneously.Now
accepted_mints[].payment_method(empty = the unit's default), plumbed throughMintConfig->merchant->cdk/gonuts/select, plus its schema entry (the struct/schema drift fence caught the omission). A custom-unit mint declaring no method is refused with an actionable error rather than sent to a guess.Measured, not assumed, against a real
cdk-mintd 0.18.3withunit = "mb"andcustom_payment_methods = ["branch"]:The returned token decodes to
mband contains nosat. Full detail indocs/evidence/custom-unit-issuance-2026-11-03.md.2. The
cdk_wallettag had never compiledgo vet -tags cdk_walletfailed onw.ActiveKeyset(). This module pins a patched cdk-go whose accessor isFetchActiveKeyset()returningKeySetInfo; the probe was written against a different version. Because the tag is not part of the default battery, the backend that makes custom units work had zero build verification. Fixed, plus the fourNewWalletPortcall sites my new argument touched.go vet -tags cdk_wallet ./...is clean for the first time.The limit this exposed — read before assuming custom units work on a router
On the default gonuts backend the same config is refused:
gonuts-tollgatehardcodesunit: cashu.Satat wallet construction (wallet/wallet.go:128) andAddMintlooks up keysets withw.unit. Custom-unit redemption is therefore reachable only through thecdk_wallettag — which the router build cannot use in-process, because cdk-go is CGO/FFI and the module must stayCGO_ENABLED=0(AGENTS.md). The sanctioned route is the sidecar, which is separate work.This PR fixes the method name; it does not make a Pecan mint spendable on stock hardware, and it does not close NUT-20 quote locking (
RequestMintQuotepassesnil; Pecan rejects unlocked quotes).Verification
scripts/go-test.sh-> RC=0, 20 modules, 0 failuresgo vet -tags cdk_wallet ./...-> cleangofmt -lover changed files -> 0TestCustomUnitPaymentMethod_*pass under the tagcheck-changelog-duplicates.pypasses; fragment includedNo router-visible behaviour change for a sat mint: empty
unit/payment_methodkeeps the exact previous path.