From b34a32c4330b2549d129cdf63e7872378167e054 Mon Sep 17 00:00:00 2001 From: jwinterm Date: Sun, 19 Apr 2026 16:09:09 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20nginx=20=E2=80=94=20re-resolve=20upstrea?= =?UTF-8?q?m=20hostname=20via=20Docker=20DNS?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror of wowcheckout e0c349a. Same bug would bite xmrcheckout after any api container rebuild — nginx caches IP at startup and never re-resolves, so a rebuild that gives the api container a new IP leaves nginx 502'ing until manually restarted. Use 'resolver 127.0.0.11 valid=10s' + variable-based proxy_pass so nginx re-resolves every ~10s via Docker's internal DNS. --- TODO.md | 85 +++++++++++++++++++++++++++++++++++++++ nginx/conf.d/default.conf | 17 ++++++-- 2 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 TODO.md diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..d76cffb --- /dev/null +++ b/TODO.md @@ -0,0 +1,85 @@ +# xmrcheckout — Local TODO / Followups + +Notes for ourselves. Not committed (see .gitignore or just left untracked). +Kept here so context survives a terminal restart. + +## Reconciler wallet handling + +### Understanding (not a bug — design) + +- Each merchant has their own Monero view-only wallet (`user--.keys`). +- We run 3 `monero-wallet-rpc` containers as a pool. monero-wallet-rpc can hold + one wallet open at a time. We hash-shard wallets across the 3 backends + (consistent hashing on wallet_name). +- At any idle moment, 2 out of 3 backends are expected to hold no wallet. + That's fine. Wallets open lazily when the reconciler needs to query them + (via `_ensure_wallet_open`). +- `open_wallet` / `close_wallet` on the monero-wallet-rpc process is cheap + when the wallet is already open in the wallet-rpc process (the wallet-rpc + itself short-circuits), but our Python-side `backend.current_wallet` cache + is reset every reconciler loop, so we send redundant RPC calls each loop. + +### Medium-priority followups + +1. **Make `MoneroWalletService` long-lived across reconciler loops.** + Currently `reconciler.main()` constructs `MoneroWalletService()` fresh every + iteration, which resets `backend.current_wallet = None`. The actual + monero-wallet-rpc process usually still has the wallet open, but our Python + code doesn't know that, so it resends `close_wallet` + `open_wallet` on + every loop. Harmless but wasteful — ~2 extra RPCs per active invoice per + tick. Fix: hoist the service out of the loop, keep it across iterations. + +2. **Verify wallet-rpc recovery after wallet-rpc container restart.** + When wallet-rpc containers restart, the `_ensure_wallet_open` path should + reopen wallets on first use. It appears to work in practice but we haven't + written a targeted test. Add a test that simulates wallet-rpc restart + between two invoice reconciliation passes for the same tenant. + +3. **Dedicated wallet-rpc backend per busy tenant.** + The consistent-hash sharding works fine at low tenant count. When one + tenant becomes hot (lots of invoices), one backend is hit 100% and the + other two sit idle. If we hit scale, think about either pinning hot + tenants to dedicated backends, or sharding by invoice rather than tenant. + +## Webhook payload: amount propagation + +- We now include `amount` (fiat from quote), `currency`, and `amount_crypto` + in the BTCPay-compat webhook payload. This was added because Medusa's + capture workflow needs the amount to flip `captured_at` — without it, it + captures 0 and the payment stays stuck "authorized" even after we fire + `InvoiceSettled`/`InvoicePaymentSettled`. +- Downstream followups: + - Consider including `amount_atomic` (integer atomic units) for any + processor that prefers integer arithmetic over Decimal strings. + - Consider including `exchange_rate` at webhook time so downstream can + reproduce the fiat figure if their Decimal precision differs. + +## Logging + +- Silenced `monero.backends.jsonrpc.wallet` library logger (set to CRITICAL) + in `reconciler.py` so the `-13 No wallet file` errors from routine + `close_wallet` calls on idle backends stop polluting logs. +- If we need to debug wallet-rpc issues in the future, temporarily set it + back to WARNING and rebuild. + +## Upstream PRs + +Open PRs we submitted upstream (xmrcheckout/xmrcheckout): +- Advisory locks, restore_height lookback, transfer-deletion guard, + decimal formatting, webhook retries, reconciler error logging. + +Followups for upstream: +- Propose the webhook-amount addition as a PR once we've run it in prod for + a week. That's a breaking-ish change for anyone downstream relying on a + specific payload shape, so flag it carefully. +- Propose the `get_status` fix (drop the set_daemon call). Clear win — quiets + log noise with no functional change. Low-risk PR. + +## Known-unfixed + +- We do not yet have a way for a tenant to manually trigger re-send of a + webhook delivery (e.g. if Medusa was down when the settle fired). Would + be a nice admin UI action. +- If wallet-rpc runs out of memory handling many simultaneous wallet opens, + we have no backpressure — the reconciler will just keep trying. Should + add a circuit-breaker / per-backend queue depth limit at some point. diff --git a/nginx/conf.d/default.conf b/nginx/conf.d/default.conf index 85aeb8b..d70afe4 100644 --- a/nginx/conf.d/default.conf +++ b/nginx/conf.d/default.conf @@ -1,5 +1,13 @@ limit_req_zone $binary_remote_addr zone=global_rate_limit:10m rate=50r/s; +# Docker's embedded DNS (127.0.0.11) lets nginx re-resolve upstream service +# hostnames. Without `resolver` + a variable-based proxy_pass, nginx resolves +# `api` / `ui` to an IP once at startup and caches it forever — if the api +# container is rebuilt and Docker gives it a new IP, nginx keeps hitting the +# old one and every request returns 502 Bad Gateway until nginx is manually +# restarted. `valid=10s` bounds the stale window to ~10 seconds. +resolver 127.0.0.11 valid=10s ipv6=off; + server { listen 80; server_name xmrcheckout.such.software localhost; @@ -11,7 +19,8 @@ server { } location /api/ { - proxy_pass http://api:8000/api/; + set $api_upstream "api"; + proxy_pass http://$api_upstream:8000/api/; proxy_http_version 1.1; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; @@ -23,7 +32,8 @@ server { } location = /openapi.json { - proxy_pass http://api:8000/openapi.json; + set $api_upstream "api"; + proxy_pass http://$api_upstream:8000/openapi.json; proxy_http_version 1.1; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; @@ -37,7 +47,8 @@ server { } location / { - proxy_pass http://ui:3000/; + set $ui_upstream "ui"; + proxy_pass http://$ui_upstream:3000/; proxy_http_version 1.1; proxy_set_header Host $host; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;