From 8342b6698ac0632855b373e08b580167b79dd432 Mon Sep 17 00:00:00 2001 From: Edgars Date: Mon, 10 Aug 2026 14:01:27 +0100 Subject: [PATCH 1/2] fix(antseed): narrow `attempted` to what provably never broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The keeper's first armed top-up burnt $5 of a $10 daily cap on a deposit that failed with an RPC 403. `_outcome_for` only reaches `failed` (which consumes neither cap nor cooldown) on `attempted: false`, and control.js marked every non-zero CLI exit `attempted: true` — correct, but blunt. Narrow it in the SIDECAR, not the keeper. The keeper is handed `(stderr || stdout)[:600]`: one stream, truncated, and @antseed/cli prints its transaction hash with console.log while ora writes the failure line to stderr — so on the failure path the keeper gets the stream that cannot carry the hash and never sees the one that can. Any judgement made there would be made on strictly less evidence than the sidecar already had. New antseed/broadcast.js (dependency-free, like amount.js / ids.js) answers `attempted: false` only for a RECOGNISED pre-RPC failure: a process that never spawned (execFile ENOENT/EACCES/...), a module graph that would not load, the CLI's own argument guard — and only with an empty stdout, since `buyer deposit` prints its Wallet:/Amount: preamble before the deposits client exists. Everything else stays `attempted: true`, including silence and every unrecognised shape, so a future CLI degrades toward the safe answer. The 403 that prompted this deliberately still classifies `unknown`, and the module documents why at length. `buyer deposit` broadcasts TWICE (unconditional approve, then the deposit) and polls for a receipt after each; the CLI drops the TransactionResponse before rethrowing, so a receipt poll that 403s prints exactly what a 403 before signing prints — no hash, same message. The suggested "no tx hash + transport-level error" heuristic would call that `failed` and move real USDC with the ledger recording nothing. Resolving that class needs evidence from outside the CLI's stdio (nonce around the run, or the escrow delta a cycle later). Also closes a hole the incident exposed: every `unknown` consumes the cap, so at the shipped knobs (cap 10, amount 5) only two deposits fit in 24h and the error breaker's third strike could never be written. The cap silently absorbed the failure and the keeper went quiet for a day with no halt and nothing above WARNING. It now halts on two strikes once the cap can no longer admit an attempt — forfeiting nothing it could still have done. Every comment asserting the old "did the CLI run" contract is updated; `failed` now means "no transaction could have reached Base mainnet". --- antseed/broadcast.js | 187 ++++++++++++++++++++++++++++++++++++ antseed/broadcast.test.js | 175 +++++++++++++++++++++++++++++++++ antseed/control.js | 51 +++++++--- docs/PROVIDERS.md | 33 ++++++- host_store.py | 9 +- tests/test_antseed_node.py | 12 +++ tests/test_wallet_keeper.py | 134 +++++++++++++++++++++++++- wallet_keeper.py | 107 ++++++++++++++++----- 8 files changed, 662 insertions(+), 46 deletions(-) create mode 100644 antseed/broadcast.js create mode 100644 antseed/broadcast.test.js diff --git a/antseed/broadcast.js b/antseed/broadcast.js new file mode 100644 index 0000000..35455e6 --- /dev/null +++ b/antseed/broadcast.js @@ -0,0 +1,187 @@ +// Did a FAILED buyer-CLI run put a transaction on Base mainnet? +// +// Extracted from control.js for the same reason amount.js and ids.js were: +// control.js pulls in `pg`, which only exists inside the sidecar image, so +// anything left in that file cannot be unit-tested from the repo. This module +// has NO dependencies, so broadcast.test.js runs wherever the suite runs. +// +// WHAT IT DECIDES. control.js publishes `attempted` on every response, and that +// one boolean is the difference between "nothing happened, retry freely" and "a +// transaction may be on Base mainnet right now": the router's wallet keeper +// records the first as `failed` (consumes neither the daily cap nor the +// cooldown) and the second as `unknown` (consumes both). Until now every +// non-zero CLI exit was `attempted: true`, because the exit code alone cannot +// tell the two apart. This module narrows that — but only where the answer is +// PROVABLE, which is a much smaller set than it looks. +// +// WHY IT LIVES HERE AND NOT IN THE KEEPER. The keeper never sees the evidence. +// control.js hands it `(stderr || stdout).slice(0, 600)`: one stream, not both, +// truncated. `@antseed/cli` prints its transaction hash with `console.log`, i.e. +// on STDOUT, while ora writes the spinner and the failure line to STDERR — so on +// the failure path the keeper is handed the stream that cannot contain the hash +// and never sees the one that can. (Prod reclaim rows are already truncated at +// exactly 600 chars, mid-token.) A classifier reading that view would be reading +// a lossy projection of the thing it has to be sure about. +// +// --------------------------------------------------------------------------- +// WHAT CANNOT BE PROVED FROM CLI OUTPUT — read this before widening the rule. +// +// `@antseed/cli@0.1.128`'s `buyer deposit` runs SIX RPC calls inside one ora +// spinner, and TWO of them are broadcasts (an unconditional ERC-20 `approve`, +// then the deposit itself), each followed by a `wait()` receipt poll: +// +// getTransactionCount -> estimateGas -> sendTransaction(approve) +// -> approveTx.wait() -> estimateGas -> sendTransaction(deposit) +// -> tx.wait() +// +// Every one of those can throw ethers' `SERVER_ERROR`. Critically, the CLI's +// deposit path is: +// +// const tx = await this._sendBuffered(...); // tx.hash exists here +// const receipt = await tx.wait(); // <-- throws +// return receipt.hash; // never reached +// +// `tx` is a local and the hash is never attached to the thrown error, and +// `deposit.js` prints only `err.message`. So a run that BROADCAST and then hit a +// 403 while polling for the receipt prints exactly what a run that failed before +// signing prints — no hash, same `Deposit failed: server response 403 ...` line. +// The two are byte-indistinguishable. The prod incident this module was written +// for is that shape, and it therefore STAYS `unknown`. Deciding it `failed` on +// the "no tx hash + transport-level error" heuristic would be precisely the +// optimistic mistake the `attempted` contract exists to prevent: it moves real +// USDC with the ledger recording nothing. +// +// Resolving that class needs evidence from OUTSIDE the CLI's stdio — the wallet +// nonce read before and after the run, or the escrow delta one status cycle +// later. Neither is this module's job. +// --------------------------------------------------------------------------- +// +// SO THE RULE IS AFFIRMATIVE, NOT RESIDUAL. `attempted: false` requires a +// RECOGNISED pre-RPC failure shape. An unrecognised failure stays `attempted: +// true`, which means a future @antseed/cli that changes its wording degrades +// toward the safe answer rather than the expensive one. +"use strict"; + +// A transaction-hash-shaped token. Deliberately GENEROUS — the `0x` is optional +// and a longer hex run still matches its 64-char prefix — because every extra +// match lands on `attempted: true`, the safe side. A missed hash is the only +// error direction that costs money. +const TX_HASH_RE = /(?:0x)?[0-9a-fA-F]{64}/; + +// The CLI's own markers for the step that does the signing and broadcasting. +// Their PRESENCE proves the on-chain step started, which is all we need: from +// there on nothing in the output can rule a broadcast out. Matched case- +// insensitively against both streams combined. +const ONCHAIN_STEP_MARKERS = [ + "depositing usdc", // ora start text, written to stderr + "deposit failed:", // spinner.fail() — the catch around deposit() + "deposited ", // spinner.succeed() + "withdrawing", // the /withdraw twin + "withdraw failed:", + "withdrew ", +]; + +// Phrases that only exist once a transaction exists. Belt and braces behind the +// hash and the step markers: any one of them forces `attempted: true`. +const BROADCAST_PHRASES = [ + "nonce", + "already known", + "replacement transaction", + "underpriced", + "dropped or replaced", // the CLI's own post-broadcast throw + "execution reverted", + "transactionhash", + "transaction hash", + "receipt", + "confirmations", + "eth_sendrawtransaction", + "broadcast", +]; + +// execFile could not START the process. Node reports these on the error object +// itself rather than in any stream, so this is structural evidence, not prose: +// a process that never spawned cannot have signed anything. +const SPAWN_ERRNOS = new Set([ + "ENOENT", "EACCES", "EPERM", "ENOTDIR", "ENOEXEC", "E2BIG", +]); + +// Failures that happen before the CLI can reach an RPC at all: its own argument +// guard, and a module graph that would not load. Recognised affirmatively and +// only when STDOUT IS EMPTY — `buyer deposit` prints its `Wallet:` / `Amount:` +// preamble to stdout before the deposits client is even constructed, so any +// stdout at all means the run got past the point these signatures describe, and +// a signature appearing anyway is a contradiction we decline to resolve. +const PRE_RPC_SIGNATURES = [ + "error: amount must be a positive number", + "cannot find module", + "err_module_not_found", + "module_not_found", +]; + +// r is control.js's `run()` result: { code, killed, stdout, stderr }. +// Returns { attempted, why } — `why` is a short phrase for the ledger, so an +// operator reading wallet_ops can see WHICH branch decided their row. +function classifyCliFailure(r) { + const stdout = String((r && r.stdout) || ""); + const stderr = String((r && r.stderr) || ""); + const code = r && r.code; + const combined = stdout + "\n" + stderr; + const hay = combined.toLowerCase(); + + // 1. We SIGTERMed it on the timeout. Says nothing at all about whether it had + // already broadcast — that is the entire reason control.js answers 504 here. + if (r && r.killed) { + return { attempted: true, why: "the CLI was killed on the timeout" }; + } + + // 2. The on-chain step had started. Unconditional: past this marker the output + // cannot rule a broadcast out (see the header — the hash is discarded on + // the post-broadcast failure path). + const marker = ONCHAIN_STEP_MARKERS.find((m) => hay.includes(m)); + if (marker) { + return { attempted: true, why: "the on-chain step had started (" + marker.trim() + ")" }; + } + + // 3. A transaction-hash-shaped token anywhere. Unconditional, whatever else + // the output says. + if (TX_HASH_RE.test(combined)) { + return { attempted: true, why: "the output carries a transaction-hash-shaped token" }; + } + + // 4. Wording that only exists once a transaction exists. + const phrase = BROADCAST_PHRASES.find((p) => hay.includes(p)); + if (phrase) { + return { attempted: true, why: "the output mentions " + JSON.stringify(phrase) }; + } + + // 5. Anything on stdout means the run got past its pre-flight preamble and + // into territory this module does not claim to understand. + if (stdout.trim() !== "") { + return { attempted: true, why: "the CLI produced stdout we do not recognise" }; + } + + // 6. The process never started. + if (SPAWN_ERRNOS.has(String(code)) && stderr.trim() === "") { + return { attempted: false, why: "execFile could not start the CLI (" + code + ")" }; + } + + // 7. A recognised pre-RPC failure, with an empty stdout confirming the run + // never reached its preamble. + const sig = PRE_RPC_SIGNATURES.find((s) => hay.includes(s)); + if (sig) { + return { attempted: false, why: "the CLI failed before any RPC call (" + JSON.stringify(sig) + ")" }; + } + + // 8. Unrecognised. Silence included: a CLI that exits non-zero saying nothing + // is not evidence that it did nothing. The default must never be inverted. + return { attempted: true, why: "the failure is unrecognised, so a broadcast cannot be ruled out" }; +} + +module.exports = { + classifyCliFailure, + TX_HASH_RE, + ONCHAIN_STEP_MARKERS, + BROADCAST_PHRASES, + PRE_RPC_SIGNATURES, + SPAWN_ERRNOS, +}; diff --git a/antseed/broadcast.test.js b/antseed/broadcast.test.js new file mode 100644 index 0000000..f0c94da --- /dev/null +++ b/antseed/broadcast.test.js @@ -0,0 +1,175 @@ +// The pre-broadcast classifier. This is the piece that decides whether a failed +// `antseed buyer deposit` cost the router's daily cap a slot or nothing at all — +// and, much more importantly, the piece that must NEVER call a run `failed` when +// that run could have put a transaction on Base mainnet. +// +// The fixtures below are the REAL prod output, byte for byte, from the +// wallet_ops row written the first time the keeper was armed. Keep them that +// way: the whole point of this file is that the classifier is pinned against +// what @antseed/cli actually prints, not against what it might reasonably print. +// +// Run: node --test antseed/broadcast.test.js +"use strict"; +const { test } = require("node:test"); +const assert = require("node:assert/strict"); +const { + classifyCliFailure, ONCHAIN_STEP_MARKERS, PRE_RPC_SIGNATURES, SPAWN_ERRNOS, +} = require("./broadcast.js"); + +// wallet_ops id=4 and id=6, pid=antseed, op=topup, amount 5, outcome `unknown`. +// ora writes to stderr, so this is the whole of it. +const PROD_403_STDERR = String.raw`- Depositing USDC into deposits contract... +✖ Deposit failed: server response 403 Forbidden (request={ }, response={ }, error=null, info={ "requestUrl": "https://base.publicnode.com", "responseBody": "{\"jsonrpc\":\"2.0\",\"error\":{\"code\":-32602,\"message\":\"Archive requests require a personal token. Get one at: https://www.allnodes.com/publicnode\"},\"id\":8}\n", "responseStatus": "403 Forbidden" }, code=SERVER_ERROR, version=6.16.0) +`; + +// console.log(chalk.dim(...)) — printed before the deposits client is built. +const PROD_403_STDOUT = `Wallet: 0x32485F22a04C1054a4292b152f02363e3849f93F +Amount: 5 USDC (5000000 base units) +`; + +const HASH = "0x" + "ab12cd34".repeat(8); // 0x + 64 hex + +function fail(over) { + return Object.assign({ code: 1, killed: false, stdout: "", stderr: "" }, over); +} + +// ---- the incident, and why it is NOT narrowable ------------------------- + +test("the exact prod 403 stays ATTEMPTED — the on-chain step had started", () => { + // The deposit step runs two broadcasts (approve, then deposit) and polls for a + // receipt after each. @antseed/cli throws away the TransactionResponse on the + // post-broadcast path, so a 403 while polling prints exactly this. There is no + // hash to look for and no way to tell the two apart, so `unknown` is the only + // answer that cannot lose money. + const v = classifyCliFailure(fail({ stdout: PROD_403_STDOUT, stderr: PROD_403_STDERR })); + assert.equal(v.attempted, true); + assert.match(v.why, /on-chain step/); +}); + +test("the same 403 carrying a transaction hash is ATTEMPTED too", () => { + const v = classifyCliFailure(fail({ + stdout: PROD_403_STDOUT + `Transaction: ${HASH}\n`, + stderr: PROD_403_STDERR, + })); + assert.equal(v.attempted, true); +}); + +test("a transaction hash alone forces ATTEMPTED, whatever else the output says", () => { + // Even stripped of every step marker and dressed up as a pre-RPC failure. + for (const sig of PRE_RPC_SIGNATURES) { + const v = classifyCliFailure(fail({ stderr: `${sig}\nsaw ${HASH}` })); + assert.equal(v.attempted, true, `${sig} + a tx hash must stay attempted`); + } +}); + +test("a bare 64-hex run counts as a hash — the regex over-matches on purpose", () => { + const bare = "ab12cd34".repeat(8); + assert.equal(classifyCliFailure(fail({ stderr: `oops ${bare}` })).attempted, true); + // ...and so does a longer hex blob that merely contains one. + assert.equal(classifyCliFailure(fail({ stderr: "0x" + "f".repeat(130) })).attempted, true); +}); + +test("every on-chain step marker forces ATTEMPTED on its own", () => { + for (const m of ONCHAIN_STEP_MARKERS) { + const v = classifyCliFailure(fail({ stderr: `✖ ${m} something went wrong` })); + assert.equal(v.attempted, true, `${m} must force attempted`); + } +}); + +test("wording that only exists once a transaction exists forces ATTEMPTED", () => { + for (const s of ["nonce too low", "already known", "replacement transaction underpriced", + "Transaction was dropped or replaced", "execution reverted", + "eth_sendRawTransaction failed"]) { + assert.equal(classifyCliFailure(fail({ stderr: s })).attempted, true, s); + } +}); + +// ---- the default must not be inverted ----------------------------------- + +test("a killed CLI is ATTEMPTED even with an otherwise pre-RPC-looking output", () => { + const v = classifyCliFailure(fail({ killed: true, stderr: "Cannot find module './x.js'" })); + assert.equal(v.attempted, true); + assert.match(v.why, /killed/); +}); + +test("an UNRECOGNISED non-zero exit stays ATTEMPTED", () => { + // The load-bearing default. A failure shape this module has never seen is not + // evidence that nothing happened — including the shapes that look reassuringly + // transport-ish. + for (const stderr of ["something exploded", "ECONNRESET", "socket hang up", + "Error: connect ETIMEDOUT 1.2.3.4:443", "panic: nope", + "TypeError: undefined is not a function"]) { + const v = classifyCliFailure(fail({ stderr })); + assert.equal(v.attempted, true, `${stderr} must stay attempted`); + assert.match(v.why, /unrecognised/); + } +}); + +test("a SILENT non-zero exit stays ATTEMPTED — silence is not evidence", () => { + assert.equal(classifyCliFailure(fail({ code: 1 })).attempted, true); + assert.equal(classifyCliFailure(fail({ code: 127 })).attempted, true); +}); + +test("stdout the module does not recognise keeps a run ATTEMPTED", () => { + // `buyer deposit` prints its Wallet:/Amount: preamble before the deposits + // client is constructed, so ANY stdout means the run got past the point every + // pre-RPC signature describes. A signature appearing anyway is a contradiction + // we decline to resolve in the optimistic direction. + for (const sig of PRE_RPC_SIGNATURES) { + const v = classifyCliFailure(fail({ stdout: PROD_403_STDOUT, stderr: sig })); + assert.equal(v.attempted, true, `${sig} alongside a preamble must stay attempted`); + } +}); + +// ---- what CAN be proved ------------------------------------------------- + +test("a CLI that never spawned is NOT attempted", () => { + // execFile reports these on the error object, not in any stream: structural + // evidence, not prose. A process that never started cannot have signed. + for (const code of SPAWN_ERRNOS) { + const v = classifyCliFailure(fail({ code })); + assert.equal(v.attempted, false, `${code} must be provably pre-broadcast`); + assert.match(v.why, /could not start/); + } +}); + +test("a spawn errno with output on stderr is NOT trusted", () => { + // If something wrote to stderr the process did run, whatever the errno says. + assert.equal( + classifyCliFailure(fail({ code: "ENOENT", stderr: "Deposit failed: boom" })).attempted, + true); +}); + +test("the CLI's own pre-RPC guards are NOT attempted", () => { + // control.js's validAmount rejects this upstream so it should be unreachable, + // but it is the CLI's own proof that it exited before touching an RPC. + const v = classifyCliFailure(fail({ stderr: "Error: Amount must be a positive number.\n" })); + assert.equal(v.attempted, false); + assert.match(v.why, /before any RPC/); +}); + +test("a module graph that would not load is NOT attempted", () => { + // The realistic one: a sidecar image missing a file, which is exactly the + // class the keeper's halt message calls "a configuration fault, not a chain one". + for (const stderr of ["Error: Cannot find module './ids.js'", + "code: 'ERR_MODULE_NOT_FOUND'", + "Error [ERR_MODULE_NOT_FOUND]: Cannot find package"]) { + assert.equal(classifyCliFailure(fail({ stderr })).attempted, false, stderr); + } +}); + +test("every verdict carries a why, so wallet_ops can explain itself", () => { + for (const r of [fail({ stdout: PROD_403_STDOUT, stderr: PROD_403_STDERR }), + fail({ killed: true }), fail({ code: "ENOENT" }), + fail({ stderr: "Cannot find module 'x'" }), fail({})]) { + const v = classifyCliFailure(r); + assert.equal(typeof v.attempted, "boolean"); + assert.ok(typeof v.why === "string" && v.why.length > 0); + } +}); + +test("a missing/garbage result object is ATTEMPTED", () => { + for (const r of [undefined, null, {}, { stdout: null, stderr: undefined }]) { + assert.equal(classifyCliFailure(r).attempted, true); + } +}); diff --git a/antseed/control.js b/antseed/control.js index ae06e3c..1feaae8 100644 --- a/antseed/control.js +++ b/antseed/control.js @@ -20,6 +20,11 @@ const { UPSERT_BUYER_STATUS, buyerStatusRow } = require('./store.js'); const { validAmount, MAX_AMOUNT_USDC } = require('./amount.js'); const { encodeIds } = require('./ids.js'); const { createQueue } = require('./queue.js'); +// Whether a FAILED CLI run could have broadcast a transaction. Its own +// dependency-free module for the same reason as the two above, and because the +// reasoning about what is and is not provable from CLI stdio is long enough to +// deserve a file. See antseed/broadcast.js. +const { classifyCliFailure } = require('./broadcast.js'); const path = require('path'); @@ -150,16 +155,25 @@ function readBody(req) { }); } -// `attempted` tells the CALLER whether the buyer CLI ran. It is the difference -// between "nothing happened, retry freely" and "a transaction may be on Base -// mainnet right now" — the router's keeper records the first as `failed` (costs -// nothing) and the second as `unknown` (counts as spent). Getting it wrong in -// the optimistic direction moves real USDC with the ledger recording nothing, so -// every branch below states it explicitly rather than defaulting. -function refuse(res, status, error) { // provably nothing was attempted +// `attempted` tells the CALLER whether a transaction could have reached Base +// mainnet. It is the difference between "nothing happened, retry freely" and "a +// transaction may be on Base mainnet right now" — the router's keeper records +// the first as `failed` (costs nothing) and the second as `unknown` (counts as +// spent). Getting it wrong in the optimistic direction moves real USDC with the +// ledger recording nothing, so every branch below states it explicitly rather +// than defaulting. +// +// NOTE the wording: NOT "did the CLI run". A CLI that ran and exited non-zero +// can still be `attempted: false`, but only where the failure is PROVABLY before +// any RPC call — a process that never spawned, a module graph that would not +// load. That judgement lives in broadcast.js, which also documents at length the +// much larger class it refuses to narrow: once the CLI's on-chain step has +// started, its output cannot rule a broadcast out, because @antseed/cli discards +// the transaction hash on the post-broadcast failure path. +function refuse(res, status, error) { // provably nothing could have broadcast return send(res, status, { ok: false, error, attempted: false }); } -function inconclusive(res, status, error) { // the CLI ran; outcome unknowable +function inconclusive(res, status, error) { // a broadcast cannot be ruled out return send(res, status, { ok: false, error, attempted: true }); } @@ -187,12 +201,23 @@ const server = http.createServer(async (req, res) => { if (r.code !== 0) { const why = (r.stderr || r.stdout || 'cli failed').slice(0, 600); // A CLI we KILLED on the timeout may already have broadcast the - // transaction; only a CLI that exited on its own proves it did not. - return r.killed - ? inconclusive(res, 504, 'buyer ' + verb + ' timed out after ' + + // transaction; only a CLI that exited on its own can ever prove it did not. + if (r.killed) { + return inconclusive(res, 504, 'buyer ' + verb + ' timed out after ' + DEPOSIT_TIMEOUT_MS + 'ms and was killed — the transaction may have ' + - 'been broadcast: ' + why) - : inconclusive(res, 502, why); + 'been broadcast: ' + why); + } + // ...and "can prove" is not "does prove". classifyCliFailure sees BOTH + // streams untruncated, which the caller does not, and answers + // `attempted: false` only for a recognised pre-RPC failure shape. The + // status stays 502 either way: the CLI really did fail, and an older + // keeper that falls back to the status code reads 502 as attempted — + // the safe direction. It is the FIELD that carries the money claim. + const verdict = classifyCliFailure(r); + return verdict.attempted + ? inconclusive(res, 502, why) + : refuse(res, 502, 'buyer ' + verb + ' failed before it could broadcast (' + + verdict.why + '): ' + why); } const status = await refreshStatus(); return send(res, 200, { ok: true, attempted: true, action: verb, amount, stdout: r.stdout.slice(0, 600), status }); diff --git a/docs/PROVIDERS.md b/docs/PROVIDERS.md index c62c982..0062fb0 100644 --- a/docs/PROVIDERS.md +++ b/docs/PROVIDERS.md @@ -138,13 +138,36 @@ docker compose --profile antseed up -d --build antseed - three in a row that could not be *completed*. A deposit whose HTTP call timed out, reset, or came back `502`/`504` is recorded `unknown` and **counts against the daily cap and the cooldown** — it may have put a - transaction on Base mainnet. A response that proves the buyer CLI never ran - (a `400` from the amount validator, a `401` after a token rotation, a `429` - from the sidecar's queue gate) is recorded `failed`: it consumes no cap and - no cooldown, but it is **not free** — it still counts toward this breaker - and still backs the retry interval off exponentially, or a misconfigured + transaction on Base mainnet. A response that proves **no transaction could + have been broadcast** is recorded `failed`: it consumes no cap and no + cooldown, but it is **not free** — it still counts toward this breaker and + still backs the retry interval off exponentially, or a misconfigured endpoint would be retried every 60s forever. + That proof is the sidecar's to give, never the keeper's: the keeper only + ever sees `(stderr || stdout)[:600]` of the run, one stream and truncated, + while the buyer CLI prints its transaction hash on the other one. So + `attempted: false` comes either from a request that never reached the CLI (a + `400` from the amount validator, a `401` after a token rotation, a `429` + from the sidecar's queue gate) or from `antseed/broadcast.js` recognising a + CLI failure as provably pre-RPC — a process that never spawned, a module + graph that would not load. + + It is a **narrow** set on purpose. `@antseed/cli` discards the transaction + hash when a deposit fails *after* broadcasting, so a receipt poll that 403s + is byte-identical to a 403 before signing; every failure from the moment the + CLI's on-chain step starts therefore stays `unknown`. Deciding otherwise + would move real USDC with the ledger recording nothing. Resolving that class + needs evidence from outside the CLI's stdio (the wallet nonce around the + run, or the escrow delta a status cycle later) and is not implemented. + + Because every `unknown` consumes the cap, three strikes are not always + reachable — at the shipped knobs (cap 10, amount 5) only two deposits fit in + a 24h window. So the breaker also halts on **two** strikes once the cap can + no longer admit another attempt: at that point the keeper is not done + spending for the day, it is wedged, and the halt costs nothing it could + still have done. + Both halts are cleared by an operator via `POST /x/wallet/clear-halt` (`{"kind": "topup"}` or `{"kind": "reclaim"}`); `GET /x/wallet/halts` shows what is set and why. Nothing self-clears — a breaker that re-arms itself is diff --git a/host_store.py b/host_store.py index 98c1bba..0c12d5a 100644 --- a/host_store.py +++ b/host_store.py @@ -1523,8 +1523,13 @@ def buyer_status(pid: str) -> "dict[str, Any] | None": # HTTP call to the sidecar timed out, reset, or came back 502 from a killed CLI — # must ASSUME the transaction landed rather than re-fire on top of it. The one # outcome that is NOT here is `failed`, and it is reserved for responses that PROVE -# nothing was attempted (no control URL, a 400 from the amount validator); anything -# that reached the wire is `unknown`. See wallet_keeper._control_post. +# no transaction could have reached Base mainnet: nothing was sent at all (no +# control URL, a 400 from the amount validator), or the sidecar ran the buyer CLI +# and classified its failure as provably pre-RPC (antseed/broadcast.js — a process +# that never spawned, a module graph that would not load). Anything the sidecar +# cannot place before the first RPC call is `unknown`, which notably includes +# every failure once the CLI's on-chain step has started. +# See wallet_keeper._control_post and antseed/broadcast.js. WALLET_OP_SPENT_OUTCOMES = ("pending", "fired", "effective", "ineffective", "unknown") # Outcomes of a deposit whose effect on `deposits_available` has been measured. WALLET_OP_SETTLED_OUTCOMES = ("effective", "ineffective") diff --git a/tests/test_antseed_node.py b/tests/test_antseed_node.py index 28aea01..a72676b 100644 --- a/tests/test_antseed_node.py +++ b/tests/test_antseed_node.py @@ -61,6 +61,18 @@ def test_antseed_reclaim_channel_selection(): _run_node_test("antseed/ids.test.js") +def test_antseed_pre_broadcast_classification(): + """The pre-broadcast classifier (antseed/broadcast.js) — the thing that + decides whether a failed `antseed buyer deposit` is recorded as `failed` + (costs the router nothing) or `unknown` (costs a slot of the daily cap). + + It lives in the sidecar rather than the keeper because only the sidecar has + the evidence: the keeper is handed `(stderr || stdout)[:600]`, one stream and + truncated, and the buyer CLI prints its transaction hash on the other one. + The fixtures are the real prod output byte for byte.""" + _run_node_test("antseed/broadcast.test.js") + + _LOCAL_IMPORT = re.compile( r"""(?:require\(\s*|from\s+)['"](\./[^'"]+)['"]""") diff --git a/tests/test_wallet_keeper.py b/tests/test_wallet_keeper.py index 356c44b..c0e1db2 100644 --- a/tests/test_wallet_keeper.py +++ b/tests/test_wallet_keeper.py @@ -426,10 +426,10 @@ def test_topup_refuses_to_fire_when_the_intent_row_cannot_be_persisted(monkeypat def test_a_deposit_that_never_reached_the_cli_is_failed_and_costs_nothing(): """INVERTED, and narrowed. This test used to accept ANY unsuccessful response as `failed` — an outcome that consumes neither the daily cap nor the - cooldown — which is only sound when the buyer CLI provably never ran. Now the - sidecar says so explicitly (`attempted`), and only that answer keeps the - cheap outcome. Here: a 400 from the amount validator, rejected before the CLI - is invoked.""" + cooldown — which is only sound when no transaction could have reached Base + mainnet. Now the sidecar says so explicitly (`attempted`), and only that + answer keeps the cheap outcome. Here: a 400 from the amount validator, + rejected before the CLI is invoked.""" k = _FakeControl(responses={"deposit": { "ok": False, "attempted": False, "error": "amount must be positive"}}) seed_buyer_status(PID, deposits_available="0.5", deposits_reserved="0.0") @@ -783,6 +783,132 @@ def test_one_success_breaks_the_error_run(): assert host_store.wallet_halted(PID, "topup") is False +def test_the_daily_cap_can_no_longer_starve_the_error_breaker(caplog): + """The prod shape, and the hole it exposed. Every `unknown` consumes the cap, + so at the SHIPPED knobs (cap 10, amount 5) exactly two deposits fit in a 24h + window — and the breaker above waits for three. The third row could never be + written, so the cap silently absorbed the failure and the keeper went quiet + for a day: no halt, nothing above WARNING, nobody told. + + Once the cap can no longer admit an attempt the keeper is not "done spending + today", it is wedged, so the shorter run is enough. The halt forfeits nothing + the cap was still going to allow.""" + knobs = _knobs(topup_cooldown_s=0) # the defaults: cap 10, amount 5 + assert knobs.topup_daily_cap_usdc / knobs.topup_amount_usdc \ + < wk.TOPUP_ERROR_STRIKES_TO_HALT, "the premise: 3 strikes are unreachable" + k = _failing_keeper() + seed_buyer_status(PID, deposits_available="0.5", deposits_reserved="0.0") + for _ in range(2): + _age_all_topups(-1) + assert _run(k._maybe_topup(PID, knobs, 0.5)) == "topup_unknown" + assert host_store.wallet_op_spend_since(PID, "topup", 0)["spent_usdc"] == 10.0 + + _age_all_topups(-1) + with caplog.at_level("ERROR"): + assert _run(k._maybe_topup(PID, knobs, 0.5)) == "error_halt" + assert "HARD HALT" in caplog.text + assert host_store.wallet_halted(PID, "topup") is True + assert len(k.calls) == 2, "the halt must not have fired a third deposit" + # ...and it is durable, so a restart does not resume the silent stall. + assert _run(_failing_keeper()._maybe_topup(PID, knobs, 0.5)) == "halted" + + +def test_a_cap_reached_by_deposits_that_WORKED_is_just_the_cap(): + """The other side of it: the halt above keys off the error run, not off the + cap alone. A keeper that funded itself twice today has done its job, and + halting there would need an operator to clear a breaker nothing tripped.""" + knobs = _knobs(topup_cooldown_s=0) + k = _FakeControl() + seed_buyer_status(PID, deposits_available="0.5", deposits_reserved="0.0") + for _ in range(2): + _age_all_topups(-1) + assert _run(k._maybe_topup(PID, knobs, 0.5)) == "topup_fired" + _age_all_topups(-1) + assert _run(k._maybe_topup(PID, knobs, 0.5)) == "daily_cap" + assert host_store.wallet_halted(PID, "topup") is False + + +def test_repeated_PRE_BROADCAST_failures_halt_without_spending_the_cap(caplog): + """The outcome antseed/broadcast.js newly produces, driven end to end. A + deposit that provably could not have broadcast consumes neither the cap nor + the cooldown — which is the whole point — so it must still be stopped by + something. It is: the backoff throttles each retry and the error breaker + halts the run, at the FULL three strikes, because the cap never binds.""" + knobs = _knobs(topup_cooldown_s=0) + k = _FakeControl(responses={"deposit": { + "ok": False, "attempted": False, + "error": "buyer deposit failed before it could broadcast " + "(execFile could not start the CLI (ENOENT)): cli failed"}}) + seed_buyer_status(PID, deposits_available="0.5", deposits_reserved="0.0") + + assert _run(k._maybe_topup(PID, knobs, 0.5)) == "topup_failed" + assert _run(k._maybe_topup(PID, knobs, 0.5)) == "backoff", \ + "zero cost is not a licence to hammer at the 60s cycle rate" + for _ in range(wk.TOPUP_ERROR_STRIKES_TO_HALT - 1): + _age_all_topups(-1) + assert _run(k._maybe_topup(PID, knobs, 0.5)) == "topup_failed" + + spend = host_store.wallet_op_spend_since(PID, "topup", 0) + assert spend["spent_usdc"] == 0.0 and spend["last_ts"] is None, \ + "a pre-broadcast failure must cost the cap nothing at all" + with caplog.at_level("ERROR"): + assert k._settle_topups(PID, 0.5, 0.0) == "error_halt" + assert "HARD HALT" in caplog.text + # The reason must not claim these never reached the CLI — since + # broadcast.js, `failed` also covers a CLI that ran and died pre-RPC. The + # claim it may still make is about BROADCAST. + reason = str(host_store.wallet_ops_recent(PID, limit=50)) + assert "reached the buyer CLI" not in reason + assert _run(k._maybe_topup(PID, knobs, 0.5)) == "halted" + + +def test_outcome_for_reads_the_attempted_flag_and_nothing_else(): + """C1's whole contract in one table. `failed` is reachable ONLY through an + explicit `attempted: false`; every other shape — including a missing flag — + is `unknown` and counts as spent.""" + f = wk.WalletKeeper._outcome_for + assert f({"ok": True}) == "fired" + assert f({"ok": True, "attempted": False}) == "fired" + assert f({"ok": False, "attempted": False}) == "failed" + assert f({"ok": False, "attempted": True}) == "unknown" + assert f({"ok": False}) == "unknown", "a missing flag is not a proof" + for truthy in (0, "", None, "false", "no"): + assert f({"ok": False, "attempted": truthy}) == "unknown", \ + f"only a real False may reach `failed`, not {truthy!r}" + + +def test_the_sidecar_classifies_the_failure_the_keeper_only_names(monkeypatch): + """The keeper must NOT grow its own CLI-output parser. It is handed + `(stderr || stdout)[:600]` — one stream, truncated — while @antseed/cli + prints its transaction hash on the other one, so anything decided here would + be decided on strictly less evidence than the sidecar already had. This pins + the split: control.js routes its non-zero-exit branch through broadcast.js, + and the keeper only reads the resulting flag.""" + control_js = (ROOT / "antseed" / "control.js").read_text() + assert "require('./broadcast.js')" in control_js, \ + "control.js must classify the CLI failure, not hand it on unclassified" + assert re.search(r"classifyCliFailure\(r\)", control_js), \ + "the deposit/withdraw branch must call the classifier" + # And the keeper stays out of it: no CLI prose anywhere in wallet_keeper. + keeper = (ROOT / "wallet_keeper.py").read_text() + for prose in ("Deposit failed", "SERVER_ERROR", "0x[0-9a-fA-F]{64}", + "Depositing"): + assert prose not in keeper, \ + f"wallet_keeper.py must not parse CLI output ({prose!r})" + + # The one thing it does with the flag, end to end. + k = wk.WalletKeeper([PID]) + _post_returning(monkeypatch, _Resp(502, { + "error": "buyer deposit failed before it could broadcast " + "(execFile could not start the CLI (ENOENT)): cli failed", + "attempted": False})) + resp = _run(k._control_post("deposit", None, 1.0)) + assert resp["attempted"] is False + assert k._outcome_for(resp) == "failed", \ + "a 502 whose body proves no broadcast must beat the status-code fallback" + assert 502 not in wk.NOT_ATTEMPTED_STATUSES, "...and the fallback stays pessimistic" + + def _age_all_topups(ago_s): """Backdate every topup row so cooldowns/backoffs have elapsed.""" with host_store._get_pool().connection() as conn: diff --git a/wallet_keeper.py b/wallet_keeper.py index 0e01c90..85f7239 100644 --- a/wallet_keeper.py +++ b/wallet_keeper.py @@ -50,6 +50,21 @@ guessed, and every outcome the keeper cannot rule out is recorded as `unknown` and COUNTED AS SPENT. +WHAT `failed` MEANS, AND WHY THE KEEPER DOES NOT DECIDE IT. `failed` is the +outcome that consumes NEITHER the daily cap nor the cooldown, so it may only be +reached where a broadcast is provably impossible. The sidecar decides that, not +this module: `antseed/control.js` publishes `attempted` per branch and +`antseed/broadcast.js` classifies a failed CLI run behind it. The keeper cannot +do this itself — it is handed `(stderr || stdout)[:600]`, one stream and +truncated, while the buyer CLI prints its transaction hash on the OTHER one. It +would be classifying a lossy projection of the evidence. + +That classification is narrow ON PURPOSE. `@antseed/cli` discards the hash when +a deposit fails AFTER broadcasting (a receipt poll that 403s looks exactly like +a pre-signing 403), so the whole "the RPC refused us mid-deposit" class — the +prod incident that motivated this — stays `unknown`. Resolving it needs evidence +from outside the CLI's stdio. See antseed/broadcast.js for the full argument. + SAFETY DIRECTION. The offer tourniquet in `sources/antseed.py` fails OPEN (a read blip must not kill routing); this keeper fails CLOSED (a read blip must not move money). Every guardrail below is written to that asymmetry. @@ -125,6 +140,16 @@ # so a permanently failing deposit re-fired every 60s forever. Weaker evidence # than a measured miss, hence one more strike before the same hard halt. TOPUP_ERROR_STRIKES_TO_HALT = 3 +# ...but three strikes are not always REACHABLE, and that was a hole. Every +# `unknown` consumes the daily cap, so at the shipped knobs (cap 10, amount 5) +# exactly two deposits fit in a 24h window: the third row the breaker is waiting +# for can never be written, the cap silently absorbs the failure, and the keeper +# goes quiet for a day with nothing logged above WARNING and no halt for anyone +# to find. That is the prod shape — two `unknown` rows, cap exhausted, no alert. +# So once the cap can no longer admit another attempt, a shorter run is enough: +# at that point the keeper is not "done spending today", it is wedged, and a +# halt costs nothing it was still able to do while making it loud and durable. +TOPUP_ERROR_STRIKES_TO_HALT_CAPPED = 2 # Retry backoff between error strikes. The floor exists because the cooldown knob # can legitimately be 0 (an operator wanting prompt refunding), and 0 × any # backoff is still 0 — which is the hammering this exists to stop. @@ -166,12 +191,18 @@ RECLAIM_TX_TIMEOUT_S = (CONTROL_QUEUE_WAIT_S + CONTROL_RECLAIM_TX_S + CONTROL_STATUS_S + CONTROL_DB_S + CONTROL_SLACK_S) # 330s -# HTTP statuses from the control server that PROVE the buyer CLI never ran, so -# the op cost nothing and may be retried freely. Everything else — a read -# timeout, a reset, a 502 from a CLI that exited non-zero, a 504 from a CLI we -# killed mid-broadcast — is inconclusive and must be recorded as `unknown`. -# control.js states this per-branch via `attempted`; the status list is the -# fallback for a response that predates it or comes from something in between. +# HTTP statuses from the control server that PROVE nothing could have been +# broadcast, so the op cost nothing and may be retried freely. Everything else — +# a read timeout, a reset, a 502 from a CLI that exited non-zero, a 504 from a +# CLI we killed mid-broadcast — is inconclusive and must be recorded as +# `unknown`. +# +# This is only the FALLBACK, for a response that predates `attempted` or comes +# from something in between (a proxy, an ingress). control.js states it +# per-branch and that field WINS — including where the two disagree: a 502 whose +# body says `attempted: false` is a CLI that failed before any RPC call, which +# the status code alone cannot express. The fallback is deliberately the +# pessimistic reading of every status it does not list. NOT_ATTEMPTED_STATUSES = frozenset({400, 401, 404, 405, 429}) # The channel-id shape the sidecar accepts — kept in step with CHANNEL_ID_RE in @@ -289,13 +320,15 @@ async def _control_post(self, op: str, body: "dict | None", seam, so a test double cannot widen the set of reachable verbs. Every return carries `attempted`, and it is the field that decides - whether real money may have moved. FALSE means the buyer CLI provably - never ran (no endpoint, a malformed URL, a 400 from the amount - validator, a 429 from the sidecar's queue gate) — the op cost nothing. - TRUE means the request reached the wire and the outcome is unknowable - from here: a read timeout, a reset connection, a 502 from a CLI that - exited non-zero, a 504 from a CLI killed mid-broadcast. Callers must - record the second kind as `unknown` and count it as SPENT. + whether real money may have moved. FALSE means no transaction could have + reached Base mainnet: either nothing was sent at all (no endpoint, a + malformed URL, a 400 from the amount validator, a 429 from the sidecar's + queue gate), or the sidecar ran the CLI and classified its failure as + provably pre-RPC (antseed/broadcast.js). Both cost nothing. TRUE means a + broadcast cannot be ruled out from here: a read timeout, a reset + connection, a 502 from a CLI that failed somewhere unclassifiable, a 504 + from a CLI killed mid-broadcast. Callers must record the second kind as + `unknown` and count it as SPENT. The default on any unrecognised failure is TRUE. Being wrong in that direction burns a slot of the daily cap; being wrong the other way moves @@ -369,9 +402,16 @@ async def control(self, op: str, body: "dict | None" = None, def _outcome_for(resp: dict) -> str: """`fired` / `unknown` / `failed` for a control response that is not ok. - The whole point of C1: only a response that PROVES nothing was attempted - is `failed`, because `failed` consumes neither the daily cap nor the - cooldown. Anything else is `unknown`, which does.""" + The whole point of C1: only a response that PROVES no transaction could + have reached Base mainnet is `failed`, because `failed` consumes neither + the daily cap nor the cooldown. Anything else is `unknown`, which does. + + Deliberately a one-line reading of `attempted` and nothing else. The + evidence for that flag — CLI streams, exit codes, kill signals — lives + where it is complete, in the sidecar; the keeper only ever sees a + truncated single-stream excerpt of it (see `_control_post`), so any + classification done here would be done on strictly less information than + the sidecar already had.""" if resp.get("ok"): return "fired" return "failed" if resp.get("attempted") is False else "unknown" @@ -528,12 +568,16 @@ def _settle_topups(self, pid: str, available: float, if strikes >= TOPUP_ERROR_STRIKES_TO_HALT: # Say which KIND of failure, because the two mean different things to # whoever reads this: `unknown` may have put a transaction on Base - # mainnet, `failed` provably did not and points at configuration (a - # rotated token, a misrouted URL) rather than at the chain. + # mainnet, `failed` provably could not have and points at + # configuration (a rotated token, a misrouted URL, a sidecar image + # missing the buyer CLI) rather than at the chain. Note `failed` no + # longer implies the CLI never ran — since antseed/broadcast.js it + # also covers a CLI that ran and died before any RPC call — so the + # claim made here is about BROADCAST, not about reaching the CLI. unresolved = sum(1 for r in rows[:strikes] if r["outcome"] == "unknown") detail = (f", {unresolved} of which may have moved USDC" if unresolved - else " — none of them reached the buyer CLI, so this is a " - "configuration fault, not a chain one") + else " — none of them could have broadcast a transaction, " + "so this is a configuration fault, not a chain one") return self._halt_topups(pid, "error_halt", f"{strikes} consecutive deposits could not be completed{detail}") return None @@ -862,6 +906,21 @@ async def _maybe_topup(self, pid: str, knobs: Knobs, available: float, # remaining cap is usually below one channel reserve, i.e. dust that only # burns gas. Wait for the 24h window to roll instead. if spend["spent_usdc"] + knobs.topup_amount_usdc > knobs.topup_daily_cap_usdc: + # A cap reached by deposits that DEMONSTRABLY worked is the cap doing + # its job. A cap reached by deposits nobody could measure is a wedged + # keeper about to go quiet for 24h — and, because no further row can + # be written, one the error breaker below can never reach its third + # strike on. Halt on the shorter run instead: it forfeits nothing the + # cap was still going to allow, and it converts a silent day-long + # stall into a persisted, operator-cleared alarm. + strikes = self._error_strikes(pid, "topup", + TOPUP_ERROR_STRIKES_TO_HALT) + if strikes >= TOPUP_ERROR_STRIKES_TO_HALT_CAPPED: + return self._halt_topups(pid, "error_halt", + f"{strikes} consecutive deposits could not be completed and " + f"the {knobs.topup_daily_cap_usdc} USDC daily cap can no " + f"longer admit another attempt ({spend['spent_usdc']:.4f} " + "already consumed by deposits whose effect was never measured)") _log.warning("wallet keeper: %s daily cap reached (%.4f of %.4f USDC " "in 24h) — no top-up", pid, spend["spent_usdc"], knobs.topup_daily_cap_usdc) @@ -937,8 +996,12 @@ async def _maybe_topup(self, pid: str, knobs: Knobs, available: float, "recorded as UNKNOWN and counted as spent; the transaction " "may have landed", pid, resp.get("error")) return "topup_unknown" - _log.warning("wallet keeper: deposit failed on %s without reaching the " - "buyer CLI: %s", pid, resp.get("error")) + # `failed`: the sidecar proved no transaction could have reached Base + # mainnet — either nothing was sent at all, or the buyer CLI ran and died + # before its first RPC call. Costs neither cap nor cooldown; the error + # backoff and the breaker are what stop it repeating. + _log.warning("wallet keeper: deposit on %s failed before it could " + "broadcast: %s", pid, resp.get("error")) return "topup_failed" # ---- one cycle ------------------------------------------------------- From 7997dac1657200bed5258ee13eecd83392fa0c65 Mon Sep 17 00:00:00 2001 From: jmlago Date: Wed, 12 Aug 2026 10:40:47 +0100 Subject: [PATCH 2/2] fix(antseed): derive capped keeper halt threshold --- tests/test_wallet_keeper.py | 22 ++++++++++++++++++++++ wallet_keeper.py | 20 ++++++++++---------- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/tests/test_wallet_keeper.py b/tests/test_wallet_keeper.py index c0e1db2..56b75f9 100644 --- a/tests/test_wallet_keeper.py +++ b/tests/test_wallet_keeper.py @@ -813,6 +813,28 @@ def test_the_daily_cap_can_no_longer_starve_the_error_breaker(caplog): assert _run(_failing_keeper()._maybe_topup(PID, knobs, 0.5)) == "halted" +def test_a_one_attempt_daily_cap_halts_after_the_first_unknown(caplog): + """A cap that admits only one deposit must not make even the shortened + breaker unreachable. Its first unknown consumes every permitted attempt, so + the following cap check has to turn that one strike into a durable halt.""" + knobs = _knobs(topup_cooldown_s=0, topup_daily_cap_usdc=5.0) + assert knobs.topup_daily_cap_usdc / knobs.topup_amount_usdc == 1 + k = _failing_keeper() + seed_buyer_status(PID, deposits_available="0.5", deposits_reserved="0.0") + + _age_all_topups(-1) + assert _run(k._maybe_topup(PID, knobs, 0.5)) == "topup_unknown" + assert host_store.wallet_op_spend_since( + PID, "topup", 0)["spent_usdc"] == 5.0 + + _age_all_topups(-1) + with caplog.at_level("ERROR"): + assert _run(k._maybe_topup(PID, knobs, 0.5)) == "error_halt" + assert "HARD HALT" in caplog.text + assert host_store.wallet_halted(PID, "topup") is True + assert len(k.calls) == 1, "the halt must not fire a second deposit" + + def test_a_cap_reached_by_deposits_that_WORKED_is_just_the_cap(): """The other side of it: the halt above keys off the error run, not off the cap alone. A keeper that funded itself twice today has done its job, and diff --git a/wallet_keeper.py b/wallet_keeper.py index 85f7239..d752afd 100644 --- a/wallet_keeper.py +++ b/wallet_keeper.py @@ -141,15 +141,11 @@ # than a measured miss, hence one more strike before the same hard halt. TOPUP_ERROR_STRIKES_TO_HALT = 3 # ...but three strikes are not always REACHABLE, and that was a hole. Every -# `unknown` consumes the daily cap, so at the shipped knobs (cap 10, amount 5) -# exactly two deposits fit in a 24h window: the third row the breaker is waiting -# for can never be written, the cap silently absorbs the failure, and the keeper -# goes quiet for a day with nothing logged above WARNING and no halt for anyone -# to find. That is the prod shape — two `unknown` rows, cap exhausted, no alert. -# So once the cap can no longer admit another attempt, a shorter run is enough: -# at that point the keeper is not "done spending today", it is wedged, and a -# halt costs nothing it was still able to do while making it loud and durable. -TOPUP_ERROR_STRIKES_TO_HALT_CAPPED = 2 +# `unknown` consumes the daily cap. Once the cap can no longer admit another +# attempt, the breaker threshold must be no larger than the number of full +# deposits that configuration allowed in the first place. Otherwise a cap that +# admits only one or two attempts makes the normal three-strike breaker +# unreachable and the keeper goes quiet for a day without a durable alarm. # Retry backoff between error strikes. The floor exists because the cooldown knob # can legitimately be 0 (an operator wanting prompt refunding), and 0 × any # backoff is still 0 — which is the hammering this exists to stop. @@ -915,7 +911,11 @@ async def _maybe_topup(self, pid: str, knobs: Knobs, available: float, # stall into a persisted, operator-cleared alarm. strikes = self._error_strikes(pid, "topup", TOPUP_ERROR_STRIKES_TO_HALT) - if strikes >= TOPUP_ERROR_STRIKES_TO_HALT_CAPPED: + attempts_per_cap = max( + 1, int(knobs.topup_daily_cap_usdc / knobs.topup_amount_usdc)) + capped_strikes_to_halt = min( + TOPUP_ERROR_STRIKES_TO_HALT, attempts_per_cap) + if strikes >= capped_strikes_to_halt: return self._halt_topups(pid, "error_halt", f"{strikes} consecutive deposits could not be completed and " f"the {knobs.topup_daily_cap_usdc} USDC daily cap can no "