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..56b75f9 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,154 @@ 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_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 + 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..d752afd 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,12 @@ # 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. 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. @@ -166,12 +187,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 +316,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 +398,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 +564,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 +902,25 @@ 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) + 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 " + 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 -------------------------------------------------------