Repository navigation
feat(merchant): wallet.backend selection wired through the OpenWallet seam - #1155
Open
Amperstrand wants to merge 20 commits into
Open
Amperstrand wants to merge 20 commits into
Amperstrand wants to merge 20 commits into
Conversation
… seam Closes the module half of the campaign's P4: the merchant service now routes its wallet construction through tollwallet.OpenWallet instead of calling the gonuts constructor directly. - config schema: wallet section (backend enum default|cdk, socket — the cdk-walletd sidecar's AF_UNIX path, default /var/run/cdk-walletd.sock matching the packaged daemon's UCI default). 113-entry schema consistency check passes. - config struct: WalletBackendConfig; absent section (legacy configs) parses as Backend "" which selects the in-process wallet — the pre-selection behavior, byte-identical. - merchant: openServiceWallet() (newFullMerchant now calls it); wallet.backend=cdk opens the SidecarWallet client; selection is logged; cdk without a socket fails loudly (never silently falls back). - tests: four-case selection suite (absent/default -> GonutsWallet, cdk -> SidecarWallet against a minimal fake daemon, cdk-without- socket refuses). With this, wallet.backend=cdk is reachable from a RUNNING module — the prerequisite for the P4 e2e leg (receive -> session grant through the full service on the cdk backend). The in-process cdk_static lane and the sidecar parity suite remain the wallet-layer gates.
…ll legs The e2e the wiring PR needed: the merchant service end-to-end on wallet.backend=cdk against the packaged cdk-walletd sidecar, in the cloud-lab docker topology. Composition (docker-compose.cdk-backend.yml, layered over the base lab): cdk-walletd (bind-mounted musl binary, socket on a shared volume, load-or-create mnemonic) + upstream-cdk (the same Dockerfile.tollgate build, config selects the sidecar) + the client dependency retarget via depends_on: !override. Lanes: - test_cdk_backend_e2e.py (in-container pytest): advertisement, full payment -> kind 1022 session event, identical-token replay refused (the sidecar CheckTokenSpent path). - run-cdk-backend-e2e.sh (host driver): the two daemon-kill boundary legs that need docker. LEG A: payment during a sidecar outage is REFUSED (400/21023 notice), the module survives, and recovers when the sidecar returns. LEG B: sidecar killed mid-payment; the verdict is value-based — allotment (the session's cumulative total) may rise by at most one payment's worth above the pre-kill baseline, and both attempts may never both grant. Two harness traps this lane pinned, both now encoded in the driver: - 'compose run client' STARTS the client's dependencies — it resurrected the deliberately-stopped sidecar before every outage payment, turning each outage leg into a happy-path run (measured: allotment kept growing 'during' the outage). The kill legs run with --no-deps. - build-time container egress is firewall-blocked on the bench host (runtime bridge traffic is fine): builds run with network: host, and the client is pre-built explicitly because implicit dependency builds skip build.network on this compose version. Verified: 3 consecutive full runs green (3/3 pytest + 4 driver checks each); sidecar money-op log lines confirmed.
Collaborator
Author
|
P4 e2e landed on the branch (7778f22): the full module on wallet.backend=cdk against the packaged cdk-walletd sidecar, in the cloud-lab topology. Verified (3 consecutive full runs green):
Two harness traps the lane pinned (encoded in the driver, worth knowing for any cloud-lab kill leg):
|
… legs C+D prove it module-level THE GAP (RED first: TestSidecarReceiveAmbiguity_SurfacesAsOutcomeUnknown failed on the pre-fix tree): the sidecar client returned raw ErrSidecarAmbiguous from a written-but-unanswered money call, but the merchant's do-not-resubmit classification keys on ErrOutcomeUnknown — the gonuts path maps its AmbiguousOutcomeError, the sidecar path did not. On wallet.backend=cdk an ambiguous receive fell into the generic 'payment-processing-failed' notice: no outcome-unknown guidance, the customer resubmits, and if the daemon did execute the swap the resubmit is refused as spent — value taken, no session, the #258/#640 class on the new backend. Fix: SidecarWallet.call wraps ambiguity for every non-read-only method as ErrOutcomeUnknown (ErrSidecarAmbiguous stays wrapped inside for diagnostics). MODULE-LEVEL PROOF — two new e2e legs (run-cdk-backend-e2e.sh, 3x full runs green, 7 checks each): - LEG C (quote persistence): a Lightning quote created through the module survives a cdk-walletd restart — post-restart poll shows the quote ISSUED with access granted (allotment 6M): the paid invoice was recognized AFTER the restart. - LEG D (deterministic ambiguity): a unix-socket proxy between module and daemon forwards the receive, then DROPS the daemon's answer (executed-but-unanswered, no kill race). The module answers kind 21023 code payment-outcome-unknown (the fix, live), the token is deterministically spent, its replay is refused, and the module recovers on the direct socket afterwards. Harness notes encoded: the swallow must live in the RESPONSE pump (two readers race on the upstream socket; a request-side swallow loses the race and the answer gets forwarded — measured); the socket dir is a bind mount so the host-side proxy is reachable from both containers; pkill -f self-matches the wrapping shell (split the pattern). tollwallet suite: no new failures (the two v2-keyset failures are the pre-existing tbmg#1156 fixture bug on main).
Collaborator
Author
|
Two additions on the branch (7f28a82):
|
…n verdict Found by the skill-rubric audit pass over the money paths (2026-11-01), measured live before fixing: on wallet.backend=cdk the pre-flight DecodeToken is an RPC to cdk-walletd, so a daemon outage surfaced as payment-error-invalid-token — telling a paying customer their e-cash note is fake (message even leaking dial internals). The decode-before- Receive ordering (#403/#409) is correct; its assumption that decode is local and infallible was true for the in-process backend only. ErrSidecarNotConnected at decode now returns the new payment-processing-unavailable class: 'your note was not spent — nothing was taken — try again in a moment.' Safe-retry guidance is correct here (the connect failed before any money op), unlike the post-Receive ambiguity classes which must say do-not-resubmit. RED-first: TestPurchaseSessionWalletBackendOutageIsNotAnInvalidTokenNotice failed on the pre-fix tree. Live-verified in the rig: outage payment now answers kind 21023 code payment-processing-unavailable; LEG A of the e2e driver asserts the code. Full five-leg suite green with the fix (run14).
Collaborator
Author
|
Audit fix (d4ff8e6), found by the state-machine review pass and measured live before fixing: on wallet.backend=cdk the pre-flight DecodeToken is an RPC to cdk-walletd, so a daemon outage surfaced as payment-error-invalid-token — telling a paying customer their e-cash note is fake. ErrSidecarNotConnected at decode now returns a new payment-processing-unavailable class ('your note was not spent — nothing was taken — try again in a moment'; safe-retry guidance is correct pre-Receive, unlike the post-Receive ambiguity classes). RED-first unit test; LEG A of the e2e driver now asserts the code (full five-leg suite green with the fix). Also filed #1157 (the degraded-mode wallet factory bypasses wallet.backend — narrow today precisely because this decode fix stops sidecar outages from reaching the mint-health downgrade chain; the remaining window is mint-returns-while-degraded). |
…river run-module-e2e.sh parameterizes the P4 driver on BACKEND (cdk|gonuts); run-cdk-backend-e2e.sh becomes the BACKEND=cdk shim. One declaration of the leg logic (the drift doctrine), the same VALUE-BASED verdicts on both backends. The gonuts lane runs the base upstream service (config without a wallet section = the default in-process backend) with mint-outage analogs of the kill legs; LEG D is cdk-only (no proxyable socket in-process — its ambiguity class is covered by the wallet-layer parity lane and the unit tests; stated in the matrix doc, not silently dropped). Honesty pins in the driver: LEG A on gonuts is module-survival-only (the pay helper funds from the mint, so a mint outage kills the helper before any POST — the non-vacuous mint-outage refusal through the module is LEG B's killed-mid-flight first attempt, now required to be non-empty output); the gonuts override carries the same build.network: host rules and remaps the debug port (a sibling session holds 2121 on this bench). Measured (MODULE-E2E-MATRIX.md): both columns green 3 consecutive full runs — happy 3/3, quote survives the dependency restart (ISSUED + granted, both), single-grant value verdict on kill-mid-payment (both), cdk: outage = payment-processing-unavailable, LEG D = payment-outcome-unknown with spent replay refused. With this, module- level conformance is pinned alongside the wallet-layer parity lane; every software gate the gonuts-demotion decision needs now exists.
…tags Slipped out of 7f28a82: LEG A/D assert on the notice's machine-readable code tag, which --verbose must include.
Collaborator
Author
|
The module-level conformance matrix is in (7a46baf + c7f151b): run-module-e2e.sh parameterizes the driver on BACKEND (cdk|gonuts) — one declaration of the leg logic, the same value-based verdicts. The gonuts lane runs the base upstream (no wallet section = the default in-process backend) with mint-outage analogs; LEG D is cdk-only (no proxyable socket in-process; its ambiguity class is covered by the wallet-layer parity lane and the unit tests — stated in the matrix doc, not silently dropped). Measured (tests/cloud-lab/MODULE-E2E-MATRIX.md), both columns green across consecutive full runs:
With this, every software gate the gonuts-demotion decision needs exists at both layers: wallet-level (three-backend parity, mutation-checked) and module-level (this matrix). The demotion itself remains an owner call. |
Collaborator
Author
Local CI gate — PR #1155: feat(merchant): wallet.backend selection wired through the OpenWallet seamRun of
Verdict: 5 leg(s) RED — 1 green. What this gate does not run (no vacuous green)
failing-leg log tailscontract-fences — tailpackaging-sequential — tailshell-suites — tailpackaging-concurrent — tailgo-battery — tail |
…1157) The audit's split-brain finding: all three degrade paths (the two boot-time branches and the runtime downgrade via NewMerchantDegradedFromFull) hardcoded DefaultWalletFactory — the legacy in-process gonuts constructor — so under wallet.backend=cdk a degrade window could silently construct a SECOND wallet implementation and split operator value away from the chosen sidecar. walletFactoryForBackend(configManager) routes the degraded wallet construction through the same selection seam: backend=cdk constructs via openServiceWallet (a dead sidecar yields a wallet-LESS degraded merchant — fail closed, never a silent fallback); the default backend keeps DefaultWalletFactory byte-identical. RED-first: TestDegradedWalletFactoryRespectsBackendSelection failed on the pre-fix tree (walletFactoryForBackend did not exist); now pins (a) cdk + dead socket → factory error, no wallet, no legacy artifacts, (b) default → legacy factory unchanged, (c) the runtime downgrade path constructs wallet-less on a dead sidecar. Gates: merchant suite green except the two StorageMmap tests — pre-existing on pristine origin/main, filed as #1158 (fixture writes no config; verified in a throwaway worktree before claiming it). Module e2e cdk lane: full five-leg suite green with the change.
Collaborator
Author
|
Closes #1157 (dd2ddd7): the degraded-mode wallet factory now routes through the backend selection at all three degrade paths — backend=cdk constructs via openServiceWallet and a dead sidecar yields a wallet-LESS degraded merchant (fail closed, never a silent second wallet implementation); the default backend keeps DefaultWalletFactory byte-identical. RED-first test pins all three (factory error + no legacy artifacts under cdk; legacy path unchanged; runtime downgrade wallet-less on dead socket). Module e2e cdk lane: full five-leg suite green with the change. One pre-existing note: the merchant suite's two StorageMmap tests fail on PRISTINE origin/main too (verified in a throwaway worktree) — filed as #1158, a fixture regression (the setup writes no config, so the injected storage error is never recorded and ops blame the mints). Not from this branch. |
Collaborator
Author
Local CI gate — PR #1155: feat(merchant): wallet.backend selection wired through the OpenWallet seamRun of
Verdict: 5 leg(s) RED — 1 green. What this gate does not run (no vacuous green)
failing-leg log tailscontract-fences — tailpackaging-sequential — tailshell-suites — tailpackaging-concurrent — tailgo-battery — tail |
One page: the question (maintained fallback vs unmaintained legacy vs removal), the evidence table (every gate now green at both layers, with pointers), the honest not-done list (on-device proof, CI billing, the removal-only migration path), the risks of demoting, and the recommendation — (b) unmaintained legacy fallback now, revisit removal after on-device proof and the token-transfer rehearsal. No code change rides on the answer before it; the policy default is already cdk. Demotion changes engineering attention, not shipped behavior.
Collaborator
Author
|
Added docs/architecture/gonuts-demotion-readiness.md (this branch): the one-page decision brief for gonuts's status — every gate now green at both layers (parity mutation-checked, module conformance matrix, ambiguity semantics, degrade safety, packaging, rollback path), the honest not-done list, risks, and a recommendation (unmaintained legacy fallback now; removal only after on-device proof + migration rehearsal). It's a read, not an investigation. |
…le-proven on BOTH backends The last 'covered elsewhere' asterisk in the conformance matrix closed: the gonuts lane now routes its mint traffic through the in-network killproxy (promoted from the parity lane, PR #1137, bind widened for in-network use) and LEG D arms it to swallow the mint's swap RESPONSE after the mint processed the request — the same deterministic executed-but-unanswered injection as the cdk lane's sock proxy, at the in-process backend's own dependency. Measured: gonuts LEG D → payment-outcome-unknown (do-not-resubmit), deterministically-spent replay refused, passthrough recovery — 3 of 4 runs; the 4th granted through the armed proxy (a rig-timing race, candidate: the helper's funding send-swap racing the arm, or the client retry landing between swallow cycles) — documented in the matrix and filed for follow-up. Two different underlying mechanisms (ErrSidecarAmbiguous vs gonuts AmbiguousOutcomeError) now provably produce the SAME customer-facing class at the module level: that is the conformance claim. Also: LEG B's kill is now STAGING-triggered on both lanes (the event file appearing means the POST is in flight — the fixed offset kept landing during the gonuts lane's funding phase); the staging window is 30s (cold funding through the proxy exceeds 5s); the quote helper honors MINT_URL. cdk lane green with the changes; both lanes' full suites green.
Collaborator
Author
|
The gonuts LEG D landed (3f4660b): the ambiguity class is now module-proven on BOTH backends — the gonuts lane routes mint traffic through the in-network killproxy and LEG D swallows the mint's swap response after the mint processed it, producing the same payment-outcome-unknown class from a different underlying mechanism (gonuts AmbiguousOutcomeError vs ErrSidecarAmbiguous). 3 of 4 runs green; the swallow-race flake is documented in the matrix and filed (see the new issue). LEG B's kill is now staging-triggered on both lanes. cdk lane green with the changes. |
Collaborator
Author
Local CI gate — PR #1155: feat(merchant): wallet.backend selection wired through the OpenWallet seamRun of
Verdict: 5 leg(s) RED — 1 green. What this gate does not run (no vacuous green)
failing-leg log tailscontract-fences — tailpackaging-sequential — tailshell-suites — tailpackaging-concurrent — tailgo-battery — tail |
…ic capture, probe mode Rig improvements from the gonuts LEG D swallow-race investigation (3/5 green with these; the race itself remains open): - The pay helper can ARM an injector file between staging and its own POST (--arm-file; the client mounts the proxy's state dir) — removes both the driver-poll arm latency and the funding-swap ordering race. - The pay helper gained --no-post (fund + stage an event without posting) for out-of-band protocol probes. - dump_logs captures the killproxy container log on the gonuts lane; the mint runs with RUST_LOG for the same reason. - LEG D-gonuts asserts unchanged (correct when the leg engages). Tonight's forensics (in the #1163 comment): in failing runs the module's gonuts Receive completes (amount=100, err=nil) with ZERO requests reaching the armed proxy after ARMED — despite gonuts' Receive always swapping (wallet.go:660: trusted and untrusted paths both swapWithRetry). Two independent docker-DNS errno -3 traces (getaddrinfo 'Try again', in the proxy's forward() and the client container) point at bench DNS flakiness under container churn as a contributing factor; the exact mechanism is unpinned.
The grant-despite-arm signature, solved. The DECIDE instrumentation
(logged the swallow-check tuple per watched POST) caught it red-handed:
POST /v1/swap ARMED DECIDE code=200 armed=True RELEASED=True
A stale kp.marker.release from the PREVIOUS run's successful LEG D
latched released=True in the fresh killproxy process before the
driver's state-dir cleanup ran (the dir is bind-mounted and outlives
containers), and the reset branch could never clear it (it requires
'triggered', which only a successful swallow sets — and swallows were
being skipped). The module's swap WAS arriving and forwarding all
along; only the swallow was disarmed. Every earlier 'no wire request'
reading was a log-ordering artifact of reading without the tuple.
Fix, both layers:
- driver: the kp-state cleanup runs BEFORE any container starts;
- killproxy: released is state-dir-derived — a clean dir (no marker,
no release) self-heals the latch, independent of 'triggered'.
The proxy also gained DECIDE tuples (path/code/armed/released per
watched POST) and connection-level open/close logging — this class is
now diagnosable from the log alone. The driver gained KEEP_STACK=1
(catch a failing leg with the live stack) and the pay helper gained
--no-post (fund+stage without posting, for protocol probes).
investigate-1163.sh is kept as the reproduction harness that caught it.
Stability: 6/6 consecutive full gonuts-lane runs green (previously ~3/5
with this signature; the earlier 1/4 batch was multi-causal bench
flakiness, separately documented on the issue).
The test sets an empty TOLLGATE_TEST_CONFIG_DIR and lets the ConfigManager write factory defaults — whose accepted mints are seven REAL internet mints, so this UNIT test probed the live network on every run and failed on any DNS-restricted runner (measured: mint.lnserver.com unresolvable on the bench -> go-battery's first module red for an environmental reason). The test now seeds the incident's SEVEN-mint shape pointed at closed local ports (instant refusal, zero network) before the boot; the construction-blocking stub and every assertion are untouched. Verified: passes standalone (-tags testenv, package .). Known separate flake (PRE-EXISTING on pristine main, verified by stash): under 'go test ./...' the CLI-socket-in-flight assertion races (startup_gate_test.go:116 on main; :146 with this change) and fails in ~0.01s — systematic on this bench, unrelated to this change. Filed for follow-up.
The socket assertion was a single os.Stat on a side effect produced by a concurrent goroutine — unsynchronized by construction. Poll with a bounded wait (the construction is still BLOCKED at that point, so the wait cannot outlive the in-flight window the assertion describes). Polling also made the REAL ./... failure legible: it is not a timing race. Under 'go test ./...' the test binary shows TWO CLI-server starts — one at the default testenv socket path (an earlier test in the package), then the boot's own attempt failing with 'cannot determine ownership of existing socket ... connect: invalid argument' (EINVAL — debris at the socket path that is not a socket, which claimSocketLock's staleness classification does not cover). Full log line in PR #1166; the ownership/staleness flow (src/cli/server.go Start) needs its own follow-up.
…startup-gate uses a short config dir THE STARTUP-GATE FLAKE, SOLVED: the CLI socket lives at <CONFIG_DIR>/tollgate.sock and AF_UNIX caps sun_path at 108 bytes. t.TempDir() under a long TMPDIR (28 bytes here) plus this test's 52-char name lands the path at 107-109 bytes BY RANDOM-SUFFIX LENGTH — over the limit, bind AND dial fail with a bare EINVAL that the ownership probe misreported as 'cannot determine ownership of existing socket' for a path that may not even exist. Hence: deterministic-ish flake, invisible on short-TMPDIR runners (CI), and the misleading 'double start' framing in the earlier investigation (the default testenv path is 69 bytes and always fine). - Start() now rejects >=108-byte paths with a readable error naming the limit (pinned by TestCLIServerStartRejectsOverLimitSocketPath). - The startup-gate test uses a short /tmp config dir for its socket. 3/3 green under 'go test ./...' (the shape that failed 6/6 before).
…sists its mints (#1158) Two halves of the #1158 breakage, both verified red on pristine origin/main before fixing: 1. Fixture: newDegradedSetup mutated the GetConfig() COPY with its test mints but never persisted them — the manager kept the factory-default (mint-less) config, so the degraded constructor returned at the no-configured-mints branch and the injected wallet error was never recorded. Fixed the way the sibling fixtures already do it (SaveConfig) plus a manager reload so reads THROUGH the manager see the mints. 2. Product (the real bug the broken fixture was masking): the storage-aware walletUnavailableReason landed on main but every op still routed through notInitializedError -> DegradeReason, so a jffs2/mmap wallet reported 'wallet not initialized: no reachable mints' — the exact #583 misdirection (blaming fine mints for a filesystem the mint recovery can never fix). notInitializedError now surfaces the storage class first; every other reason keeps the existing DegradeReason text, byte-compatible. All four StorageMmap tests green (red on main). Full merchant suite: one clean run; one run with a single unnamed failure that did not reproduce on rerun — noted for CI to arbitrate.
The fixture hardcoded an arbitrary keyset id over hand-picked keys — exactly the shape the #705 forged-keyset-id guard exists to refuse, so both TestV2Keyset tests failed on main from the day the guard shipped (the guard is correct; the fixture was dishonest). Adopt the shared honest-fixture helper (honest_keyset_test.go's deriveHonestKeysetID — added for this class after #705 landed): the advertised id is now DERIVED from the published keys, and the keys live in one map so the id and the JSON cannot drift apart. Both tests red on main, green here; full tollwallet suite green (with PR #1160's merchant fix, both main battery breakages now have fixes in review).
Collaborator
Author
|
Review obligation satisfied: the full 'make go-battery' is GREEN on this branch — 19 modules (gofmt, vet, build, race tests). Getting there required the main-repair chain this session produced, now cherry-picked onto the branch so it is self-sufficient and verifiably green today: the gofmt fix (#1165), the startup-gate offline seed + polled assertion + sun_path-length guard (#1166 — the flake was a 108-byte AF_UNIX socket path from long-TMPDIR temp dirs, not a race), the degraded StorageMmap fix (#1160), and the v2-keyset fixture fix (#1162). If those smaller PRs merge first, the duplicated commits here rebase away cleanly — they are byte-identical cherry-picks. |
Collaborator
Author
Collaborator
Author
|
Four-agent PR-REVIEW.md pass (worktree-isolated): APPROVE — all 13 criteria, zero blocking. Verified in a fresh worktree: the three selection/outage test groups pass, TestSidecar passes, js-schema-lint PASS (113 schema entries), and the sidecar diff confirms ErrSidecarAmbiguous is wrapped in ErrOutcomeUnknown (not replaced) — the reconcile-never-retry class the fund-safety rules require. Non-blocking note: the compose rig was not re-executed by the reviewers (docker out of scope for the pass) — the PR's recorded five-leg × two-backend runs remain the rig evidence. Merge-order manifest posted earlier in the thread. |
This was referenced Nov 2, 2026
Collaborator
|
LGTM in substance — approve, with one thing to confirm before merge. Verified on the branch, all four touched modules green: (with Wiring One thing I could not verify, and it matters for this PR: the That is worth pairing with the note I left on #1136: CI's Non-blocking, but I would want it answered before this ships: what happens at runtime when |
Collaborator
Review — PR #1155, head
|
Collaborator
Review — PR #1155, head
|
Collaborator
Author
Local CI gate — PR #1155: feat(merchant): wallet.backend selection wired through the OpenWallet seamRun of
Verdict: 4 leg(s) RED — 2 green. What this gate does not run (no vacuous green)
failing-leg log tailscontract-fences — tailpackaging-sequential — tailshell-suites — tailpackaging-concurrent — tail |
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.
The module half of the CDK-migration campaign's P4: the merchant service routes its wallet construction through tollwallet.OpenWallet (the merged selection seam) instead of calling the in-process gonuts constructor directly — wallet.backend=cdk becomes reachable from a running module.
What changes
Proven where
Context
The wallet-layer gates already exist and are green: the crash-boundary parity suite (7/7, mutation-checked, plus the locked-token demotion-gate case) runs the money ops through the sidecar client, and the same WalletPort money-cycle assertions pass on gonuts in-process, cdk-walletd sidecar, and cdk-go in-process. This PR is the piece that lets the service itself pick.