test(merchant): centralize Cashu token fixtures - de-couple tests from the wallet (T16)

[felixfelix-bot]

Why (T16)

To let the router choose its Cashu wallet backend (gonuts / CDK / nucula), the tests must not hard-wire the concrete library, so every backend can run the same suite. The production code already depends only on WalletPort; the residual coupling was in tests.

What

merchant_token_flow_test.go was the only merchant test importing gonuts-tollgate — purely to build token fixtures (cashu.Proofs, NewTokenV3/V4, cashu.Sat). This moves that construction into a single wallet-agnostic fixture helper (tokenfixture_test.go, build tag testenv && !cdk_wallet), so:

• the test file no longer imports gonuts,
• a future testenv && cdk_wallet sibling can build the same fixtures via CDK and the tests stay identical,
• the wallet is now swappable in tests by changing one file, not every test.

No behavior change — the helper produces the same serialized V3/V4 tokens and all characterization assertions are unchanged.

Intentionally not changed

quotes_wireformat_test.go and lightning_state_test.go pin gonuts nut04.State behavior on purpose — they are the compatibility contract a replacement wallet must keep (see the wallet-migration research), so they stay gonuts-specific.

Verification

• go build ./... green.
• go test -tags testenv -count=1 ./... in src/merchant green (174s).
• go vet -tags testenv ./... clean; gofmt clean.

Follow-up: the tollwallet adapter tests remain (some are gonuts-adapter-specific by design); the shared conformance suite is the next step.

For reviewers

• Test-only: no production code, go.mod, or dependency changes.
• src/merchant is a nested Go module, so the documented go test ./... from src/ does not cover it. Verified inside src/merchant: gofmt clean, go vet ./..., go build ./..., go test -race -count=1 -tags testenv ./... green.

[Amperstrand]

Campaign evidence supporting this direction (+ suggested vectors)

The 0.6.0 QEMU-venue validation campaign hit a live instance of exactly the wire-format coupling this PR centralizes — from the tester-tooling side:

@cashu/cashu-ts (JS) serializes proof amount as a string; the daemon's Go decoder requires uint64. A structurally-valid v3 token minted via cashu-ts is rejected:

Invalid cashu token: invalid token: error unmarshaling token: json: cannot unmarshal string
into Go struct field Proof.token.proofs.amount of type uint64

(kind-21023 payment-error-invalid-token; our minter now normalizes amount to Number before building the v3 payload — physical-router-test-automation/scripts/0.6.0-validation/mint-tokens.mjs.)

Suggestion for tokenfixture_test.go: alongside the positive fixtures, pin two cross-backend contract vectors —

1. Known-good v3 token (numeric amounts) that every wallet backend must accept;
2. Negative vector: string-amount token asserting the exact decode failure above.

That locks the wire contract for gonuts/CDK/nucula siblings and documents it for external tooling (any JS/Python mint-er hits this). Full error transcript + reproduction in the PRTA campaign evidence (#108 Phase A comment).

This also matters for the release channel: testers minting from cashu-ts against v0.6.0 mints will hit this silently-valid-looking rejection.

[felixfelix-bot]

Reviewed head: 948ee6efad — base 040dd7fa (main has moved on; the branch carries a CHANGELOG.md conflict).

What this does. Moves the Cashu token constructions out of src/merchant/merchant_token_flow_test.go into shared fixture helpers (src/merchant/tokenfixture_test.go) so the merchant suite no longer imports the wallet library directly (T16).

CLEAN — verified, not assumed: the assertions are not weakened. The refactor is byte-for-byte behaviour-preserving. All five call sites now go through mustV4Token/mustV3Token with the same (amount, secret) pair they had at the merge base (1/"test-secret", 1/"stub-secret-spent", 4/"stub-secret-v4", 2/"stub-secret-v3", 5/"fork63-five-sat"), the shared invariants are unchanged (Id: "00ad", C: "ab", mint hoisted to fixtureMintURL, unit cashu.Sat, includeFees: false), and no if/t.Error/t.Fatal/expected literal differs between revisions. The helper bodies are the deleted lines, not a rewrite. Corroborated by execution: the two tests owning those fixtures pass at the head — 9 PASS sub-assertions, 0 failures — and the Fork63 post-swap-fee test (which pins an exact credit amount) would have failed on any sloppy transliteration.

SHOULD-FIX — the promised "second backend" is a silent skip, not a guarantee. Both files carry //go:build testenv && !cdk_wallet. Measured:

go list -tags testenv                -f '{{join .TestGoFiles "\n"}}' .  → 20 files, both present
go list -tags 'testenv cdk_wallet'   -f '{{join .TestGoFiles "\n"}}' .  → 18 files, both ABSENT

Under -tags 'testenv cdk_wallet' the evidence files vanish from the build and the run still exits 0 — an absence, not a failure, which is the worst mode for the T16 goal and the opposite of what the new file's own doc comment promises (tokenfixture_test.go:9-10). If the point is "the same suite runs against two backends", either add a lane that actually runs both tags or say in the comment that the exclusion is deliberate and the parity claim is unenforced.

Note (not a defect): the coupling is relocated rather than removed from the package — the fixtures are still gonuts-shaped; that is fine for a test-only slice, just worth knowing before the parity claim is repeated.

CHANGELOG conflict (trivial): this branch and #395 both edit the same [Unreleased] list; rebase before pushing.

RC RECOMMENDATION: LAND AFTER THE ALPHA RC. File-for-file it is safe — the fixtures are byte-exact and the suite is green at the head — but it ships zero bytes to a router, so there is no release reason to put it inside the RC's pinned commit. Its purpose is only realised once the wallet swap actually happens (post-RC work, currently gated behind #395's blocking finding). Land it as an ordinary low-risk refactor after the RC, ideally with the C2 item folded in so the parity claim becomes enforceable rather than aspirational.

[Amperstrand]

Heads-up: this branch is currently CONFLICTING with main (GitHub reports DIRTY) — likely against the #365-era test.yml/merchant test churn or the #392/#400-era merchant.go changes. Content-wise the centralize-fixtures refactor looks like exactly what the token-flow tests need; it'll need a rebase before review can land cleanly. Happy to review once rebased.

[Amperstrand]

Review follow-ups landed — branch rebased onto current main (125f6a0) and rewritten to 5d0d890; CHANGELOG conflict resolved, MERGEABLE:

Finding	Disposition	Evidence
SHOULD-FIX: the "second backend" is a silent skip, not a guarantee — the fixture doc comment promises what nothing enforces	Comment rewritten to state exactly what the review measured: the !cdk_wallet exclusion is deliberate, parity is not enforced — go list -tags 'testenv cdk_wallet' shows this file and every test using it absent from the build (0 fixture files), and the tagged run still exits 0, an absence not a failure. It also names what enforcing parity takes (a lane running both tag combinations, failing when the cdk_wallet side is empty) and when that lands (post-RC, with #395)	src/merchant/tokenfixture_test.go doc comment; reproduced the review's own probe: go list -tags 'testenv cdk_wallet' … | grep -cE "token_flow|tokenfixture" → 0
CHANGELOG conflict (this branch and #395 edit the same [Unreleased] list)	Rebased onto main; both #410's ngit-pipeline entry and this PR's entry coexist	mergeable=MERGEABLE vs main @ 125f6a0

Suite on the rebased head: go test -race -count=1 -tags testenv ./... in src/merchant → ok, 221.070s (the two fixture-owning tests included). gofmt -l clean, go vet -tags testenv clean.

RC disposition per the review stands: land after the cut, ideally with the enforcing lane folded in.

cc @felixfelix-bot — the parity claim is now honest in-tree; ready for another pass.

[Amperstrand]

Conflict resolution verified + branch validated post-rebase.

• The GitHub conflict is CHANGELOG.md only (the fixture de-coupling entry vs. newer [Unreleased] additions on main). Rebased pr/396 onto current main locally: clean replay (2 commits), no code conflicts — merchant_token_flow_test.go / tokenfixture_test.go apply untouched.
• Post-rebase validation: gofmt/go vet clean, full src/merchant suite go test -count=1 ./... → ok (216.9s) with the centralized fixtures in place.

From our side the branch is merge-ready once rebased (or merge via rebase); happy to see it land — the wallet-decoupling keeps merchant tests hermetic, which this session's #413 finding (test expectations depending on main's evolving merchant behavior) argues for.

[felixfelix-bot]

UPDATE at head 5d0d890363 (live, re-derived immediately before posting) — supersedes my review at 948ee6efad (id 5260608482, artifact post-396.md). The branch was rebased onto the current main (52b8481a3b, so base and merge-base agree now) and gained a third own commit, 5d0d890363 test(merchant): review follow-up — the fixture comment states the parity claim is unenforced. Diff at this head: 3 files, +70/−58 — CHANGELOG.md, src/merchant/merchant_token_flow_test.go, and the new src/merchant/tokenfixture_test.go.

What it does now. Every token construction in the merchant token-flow tests goes through two helpers, mustV3Token/mustV4Token, and the only gonuts-tollgate/cashu import left in the test package lives in tokenfixture_test.go. Swapping the wallet becomes an edit to that one file plus a build-tagged sibling, which is the point of T16.

The follow-up commit is accurate — verified, not taken on trust. Its new comment says that running go test -tags 'testenv cdk_wallet' ./merchant/ would silently drop the fixture file and every test using it and still exit 0. I checked the tags at this head: both tokenfixture_test.go and merchant_token_flow_test.go carry //go:build testenv && !cdk_wallet, and the only cdk_wallet-tagged files in the tree are src/tollwallet/cdk_wallet.go and src/tollwallet/cdk_wallet_test.go — no merchant sibling exists. So the tag combination really is an absence rather than a failure, exactly as stated, and the "enforce parity in a lane that runs both tag combinations" ask is the right one to defer to the wallet swap (#395, itself a LAND-AFTER). This supersedes the "parity claim is aspirational" note in my earlier review: the claim is now correctly scoped and falsifiable.

One finding (doc wording, minor). The CHANGELOG entry says "The merchant token-flow tests no longer import the concrete wallet package" — true of merchant_token_flow_test.go, misleading about the package: tokenfixture_test.go:15-19 still imports github.com/OpenTollGate/gonuts-tollgate/cashu, which is the whole design (one place, not zero places). As written, a reader auditing the post-RC wallet swap would look for a dependency that is still there. Suggest aligning it with the fixture file's own phrasing ("the single place in the merchant tests that touches a concrete Cashu library").

Acquittals. No production code: src/merchant/*_test.go and CHANGELOG.md only — nothing under packaging/, nothing compiled into the shipped binary, no workflow invokes it. The helpers keep the exact fixture values the inline code used (amount, Id: "00ad", C: "ab", per-test secret, and one shared fixtureMintURL = "https://testmint.example.com"), so the characterisation tests assert what they asserted before.

CI: no GitHub CI evidence — gh pr checks 396 --repo OpenTollGate/tollgate-module-basic-go → no checks reported (exit 1) for the pr/t16-decouple branch; actions/runs?head_sha=5d0d890363 → total_count: 0 (Actions queued-dead org-wide since 2026-08-27; newest run anywhere is a 2026-09-17 "Graph Update" still queued). The ngit lane builds main only → no ngit result for this branch, asserted rather than implied green. I did not run the merchant suite at this head; the earlier review's read of the suite is unchanged by a test-only refactor, and I am stating that rather than dressing it up as evidence.

RC RECOMMENDATION: LAND AFTER THE ALPHA RC (unchanged). It ships zero bytes to a router — packaging/, src/ production code and packaging/files/ are untouched — so it cannot change what testers install and must not gate the cut. Its value only materialises when the wallet backend actually changes (post-RC, behind #395), and landing it inside the freeze would add test churn to a release commit for no release benefit. The single requested edit is the CHANGELOG sentence; fix it whenever it next lands.

[Amperstrand]

Verified on the branch: vet/gofmt clean, full merchant suite green (226s). The fixture is the single gonuts touchpoint in merchant tests; the follow-up commit's build-tag comment honestly documents that cdk_wallet parity is unenforced until the tagged sibling lands with #395. Changelog entry correct (Changed/Internal).