fix(wallet-toolbox): pass trx to findProvenTxs in getProvenOrRawTx

[imranterranode]

Summary

StorageKnex.getProvenOrRawTx(txid, trx) receives a database transaction, uses it for its second query, and drops it on the first. On SQLite that deadlocks the connection pool and kills the proof-fetching task before it processes a single request.

The consequence, downstream, is that BEEF payment headers grow without bound until Cloudflare's 32 KB request-header cap refuses them — at which point the wallet cannot pay at all, and cannot recover by retrying.

- r.proven = verifyOneOrNone(await this.findProvenTxs({ partial: { txid } }))
+ r.proven = verifyOneOrNone(await this.findProvenTxs({ partial: { txid }, trx }))

One line. trx is already a parameter of the enclosing function.

The mechanism

override async getProvenOrRawTx (txid: string, trx?: TrxToken): Promise<ProvenOrRawTx> {
  const k = this.toDb(trx)                                                   // bound for the 2nd query
  ...
  r.proven = verifyOneOrNone(await this.findProvenTxs({ partial: { txid } })) // trx dropped
  if (r.proven == null) {
    const reqRawTx = verifyOneOrNone(await k('proven_tx_reqs')...)            // trx used correctly

findProvenTxs → findProvenTxsQuery → setupQuery, which builds the query as this.toDb(args.trx)(table). With args.trx absent, toDb(undefined) returns this.knex — the pool, not the caller's transaction.

knex forces { min: 1, max: 1 } on the sqlite dialect (knex/lib/dialects/sqlite3/index.js), so there is exactly one connection. The caller's transaction is holding it. This query then waits for a second connection that cannot be granted: the transaction will not release until the query returns, and the query cannot run until the transaction releases.

After acquireConnectionTimeout (60s default) it throws:

KnexTimeoutError: Timeout acquiring a connection. The pool is probably full.
Are you missing a .transacting(trx) call?

The caller is the proof-completion path:

findTransactions(…, trx)
  → validateRawTransaction(t, trx)
    → getRawTxOfKnownValidTransaction(…, trx)
      → getProvenOrRawTx(txid, trx)      ← chain breaks here

So TaskCheckForProofs dies before touching any request. That is why an affected wallet's proven_tx_reqs backlog sits at zero attempts — the rows are not being tried and failing, they are never reached.

Why this matters beyond the deadlock

A payment transaction must prove its inputs are real without the server asking anyone. A confirmed input needs only a merkle path (a few hundred bytes); an unconfirmed one requires the entire parent transaction, and proof of its inputs, recursively.

Ancestry therefore stops at the first parent holding a merkle proof. When proofs never arrive, every payment re-ships the whole unproven chain. Measured on mainnet: +~300 bytes per payment, linearly, with no upper bound, crossing 32 KB around the 80th consecutive purchase.

Two properties make it worse than a slow leak:

• It is unrecoverable by retry. A refused payment has still been broadcast and paid for, so it becomes one more item in the history, making the next attempt larger. Observed going 32,870 → 33,174 that way. Every retry costs a satoshi and moves further from working.
• The error points at the wrong thing. Cloudflare's refusal is unsigned HTML, so the client library reports missing headers: x-bsv-auth-version, x-bsv-auth-identity-key, x-bsv-auth-signature. It reads like an authentication failure, and anyone hitting it goes looking in the wrong place.

Evidence

Reproduced away from any app — standalone script, plain Node, no Electron, no other process on the database, default pool, running the proof task against a copy of a real wallet database:

Run	Outstanding	Result	Proofs
unmodified 2.4.4	343	KnexTimeoutError after 60s	0
unmodified 2.4.4	5	KnexTimeoutError after 60s	0
with trx passed	343	completed in 56.6s, no errors	200

It is not caused by backlog size. Five outstanding requests deadlock exactly as 343 do — this is a code path that does not complete, not a wallet that has been overwhelmed. Volume only changed how visible it was.

Verified live, in a BSV Desktop wallet pinned to 2.4.4 using better-sqlite3 (a different driver from the harness above, so two independent environments failing the same way). The wallet had been stuck for hours, with TaskCheckForProofs and ReviewStatus throwing KnexTimeoutError roughly every 90 seconds:

Metric	Before	After
proven_tx_reqs completed	18	119
outstanding (unmined)	139	38
proven_txs	19	120
monitor KnexTimeoutError events	27	27 — none new

101 proofs drained within minutes of restarting, against a backlog that had previously moved 20–40 per day.

Functional result. The same wallet was at a 33,058-byte payment header and was being refused by Cloudflare (400 Request Header Or Cookie Too Large, unsigned HTML from cdn-cgi — confirming the refusal happens at the edge and never reaches the API). After the fix:

33,058 bytes  ← refused by Cloudflare
25,494 bytes  ← first payment after the fix, HTTP 200, first attempt
13,666 bytes  ← second payment

Walking the ancestor chain the way the payment builder does, on a wallet drained to zero outstanding, predicts a header of ~1,600 bytes versus ~35,300 unrepaired — and that unrepaired prediction came within 1% of a header measured independently on the live edge.

Scope and limitations

• Affected wallets repair themselves. Outstanding requests remain valid; once a fixed version runs, the backlog drains and header size returns to normal. No migration or manual recovery needed.
• Bounded, not immune. With proofs current, the header stops climbing forever and becomes a sawtooth — growing while payments are unconfirmed, dropping back to the floor on the next block. At ~300 bytes per payment that still allows roughly 100 purchases inside a single block interval before 32 KB is reachable again. This removes the permanent failure, not the ceiling.
• Unchanged in 2.4.4 and 2.6.1, so upgrading does not avoid it. The path is shared by every consumer on SQLite.

Known unknown, stated rather than smoothed over

If this path deadlocks with five requests, it should deadlock with one — yet proofs do complete for affected wallets (124 over a month in the wallet studied, and during live verification above, completed rose while the task was still throwing). So a second route finishes some requests without entering the transaction path — most likely getProofs completing a request already linked to a proven transaction.

That does not weaken the case for this change: the deadlock is real, reproducible at any backlog size, and removing it demonstrably drains a queue that was otherwise stationary. But it means this fix is necessary and measurably effective, and may not be the whole story for proof-drain throughput.

Separately, readSettings() (immediately above the patched function) hardcodes this.toDb(undefined) and takes no trx parameter at all, so any StorageKnex whose first database access occurs inside a transaction self-deadlocks identically. It is reached from verifyReadyForDatabaseAccess, which every write passes through. Left out of this PR deliberately to keep the change to a single reviewable line; worth a follow-up.

Testing

Type-safe by construction: FindProvenTxsArgs extends FindSincePagedArgs, which declares trx?: TrxToken — the same type as the enclosing function's own parameter.

Build and full test suite run on this branch (Node 24.13.0, pnpm 10.33.2):

pnpm --filter "@bsv/wallet-toolbox..." run build     → all 4 packages Done
jest --testPathIgnorePatterns=man.test.ts

Test Suites: 5 skipped, 120 passed, 120 of 125 total
Tests:       27 skipped, 1097 passed, 1124 total
Time:        89.3 s

No failures, no new skips.

There is no regression test specific to this fix, and it is worth being explicit about why: reproducing it requires a transaction held open against a single-connection pool while a second query is issued, and asserting on a 60-second acquireConnectionTimeout. That is doable — lower acquireConnectionTimeout, open a knex.transaction, call getProvenOrRawTx inside it, assert it resolves rather than throwing KnexTimeoutError — but it is a slow, timing-shaped test and I would rather reviewers decide whether it belongs here or as a follow-up. Happy to add it to this PR on request.

[sonarqubecloud]

Quality Gate failed

Failed conditions
E Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[sirdeggen]

Review: the fix is correct, and it is already on main

The diagnosis holds up and the write-up is excellent — but this exact change landed three days before this PR was opened.

main today, packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts:149:

r.proven = verifyOneOrNone(await this.findProvenTxs({ partial: { txid }, trx }))

git log -L 149,149:packages/wallet/wallet-toolbox/src/storage/StorageKnex.ts attributes it to ef710c3 — "perf(wallet): bring createAction under remote latency budgets" (#426), merged 2026-08-04, which made the identical { partial: { txid } } → { partial: { txid }, trx } edit. It was a one-line drive-by inside a larger perf PR, which is presumably why it wasn't visible when this was found.

This branch forked from main before #426, so GitHub still renders the change as a 1-line diff against the fork point. Merged, it would be a no-op.

Why CI looks the way it does

• SonarCloud — "E Maintainability Rating on New Code" is not about the changed line. The branch is far enough behind main that Sonar's "new code" set is much larger than the diff. mergeable_state is behind; a rebase onto current main should clear it (and would also reduce this PR to an empty diff).
• The main ci.yml workflow never ran here — only Sonar and the two Socket checks. Fork PRs need a maintainer to approve workflow runs.

So there is nothing to fix on the code side, and nothing I can push: the head branch lives on a fork.

The follow-up in the description is still open, and still real

readSettings() on main (line 137) is unchanged:

async readSettings (): Promise<TableSettings> {
  return this.validateEntity(verifyOne(await this.toDb()<TableSettings>('settings')))
}

No trx parameter, so toDb() resolves to the pool. It is reached from verifyReadyForDatabaseAccess, which every write path calls, so the self-deadlock described here is reachable whenever a StorageKnex instance's first settings read happens inside a transaction on SQLite's forced { min: 1, max: 1 } pool. It is the only remaining toDb() call in the file that cannot receive a caller's transaction. That is worth its own issue or PR — and it is where the "known unknown" energy in this description would pay off.

The regression test you sketched (lower acquireConnectionTimeout, open a knex.transaction, call inside it, assert it resolves) also still has value, since #426 fixed the line without locking the behaviour down. Written against readSettings it would cover both.

Recommendation

Close this PR as already-fixed, and open a follow-up for readSettings() plus the regression test. The investigation stands on its own regardless — the mainnet header-growth measurements and the "Cloudflare refusal masquerades as an auth failure" finding are worth preserving somewhere durable, since they explain a failure mode that is genuinely hard to recognise from the error text.

---

Generated by Claude Code

[imranterranode]

Verified — this landed on main in ef710c3 (#426) on 2026-08-04, three days before I opened this. My local main was 326 commits behind and I never fetched, so I was diffing against a stale tree. Merged this would be a no-op. Closing.

For anyone who finds this later: the published npm 2.4.4 does not contain #426, so the deadlock and the measurements in the description are real for that version — backlog 139 → 38, proofs completed 18 → 119, payment header 33,058 → 13,666 bytes. Just not news for main.

Follow-up for readSettings() plus the regression test to come.