Skip to content

feat(sdk): let a 402 declare txids the payer may omit from payment ancestry - #445

Open
imranterranode wants to merge 1 commit into
bsv-blockchain:mainfrom
imranterranode:proposal/authfetch-known-txids
Open

feat(sdk): let a 402 declare txids the payer may omit from payment ancestry#445
imranterranode wants to merge 1 commit into
bsv-blockchain:mainfrom
imranterranode:proposal/authfetch-known-txids

Conversation

@imranterranode

@imranterranode imranterranode commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Narrowed from the earlier discussion PR to the BRC-96 change alone, per @ty-everett's read that it "should land regardless, because it's opt-in, it's sensibly constrained, and it's fail-soft." The BRC-118 multipart-transport half — the structural fix for the ceiling — now lives in #473 so it can be designed on its own terms.

The problem, measured

x-bsv-payment carries the payment transaction plus its ancestry. An unconfirmed parent has no merkle path, so there is nothing shorter to send. Chained payments each spend the previous payment's unconfirmed change, so every payment re-ships the whole unconfirmed run:

payment 1   [tx1] + merkle proof of a confirmed input     11.3 KB
payment 2   [tx2] + [tx1 in full]                         11.6 KB
payment 3   [tx3] + [tx2] + [tx1]                          11.9 KB
   …                                              +~0.3 KB each
payment ~80 [tx80] … [tx1]                                 32 KB — refused

Measured on mainnet against a Cloudflare-fronted origin, the request is refused at 32 KB of total request headers400 Request Header Or Cookie Too Large, unsigned HTML from cdn-cgi, before the origin sees it. Reproduced deliberately at 33,058 bytes.

Two properties make it worse than a slow leak:

  • It does not self-correct. A refused payment is still broadcast and paid for, so it becomes one more ancestor and the next attempt is larger. Observed 32,870 → 33,174 that way — a satoshi per attempt, each one 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.

The mechanism already exists and is never exercised

BRC-96 — Txid Only Extension lets an ancestor ship as a bare 32-byte txid (Tx Data Format 02, version marker 0200BEEF) rather than in full, "when parties exchanging BEEFs have already validated certain transactions".

It is implemented on both sides already. The TS SDK has makeTxidOnly/mergeTxidOnly with BEEF V2 as the constructor default; the Go SDK has NewBeefV2/TxidOnly(). wallet-toolbox is wired end to end — mergeAllocatedChangeBeefs forwards options.knownTxids, getBeefForTransaction calls mergeTxidOnly, and Wallet.verifyReturnedTxidOnlyAtomicBEEF guards the return path. Even 2.4.4 honours it.

The only gap is that nothing tells the payer which txids the recipient already has, so the list is always empty. Measured on the wire: 11 full transactions, 1 BUMP, 0 txid-only.

What this does

Reads an optional x-bsv-payment-known-txids off the 402 and threads it into createAction's existing options.knownTxids.

  • Opt-in on the server side. Absent header → the option is omitted entirely → the createAction call is byte-identical to what the SDK sends today. Application developers do nothing and never see the feature; a server enables it by emitting the header.
  • Fail-soft. Malformed entries are dropped rather than thrown. A bad or hostile header costs bytes on the wire, never a failed payment.
  • Bounded. Capped at 256 entries so a server cannot inflate the createAction call.
  • Safe by construction. Per BRC-96 a txid-only entry "is treated as implicitly valid" — the recipient verifies nothing about it. So only the recipient may ever declare one. Omitting an ancestor the recipient lacks makes the payment unverifiable, which is why the list must come from the recipient's own records and must never be inferred by the payer, not even from its own broadcast history: a payment can be broadcast and still not internalised.

Two things I checked on the toolbox side while writing this, since the change is only useful if the wallet honours it:

  • No trustSelf gate. ActionBatchPlanner applies makeTxidOnly for every entry in options.knownTxids unconditionally (ActionBatchPlanner.ts#L200), so passing the list alone is sufficient — trustSelf: 'known' is not additionally required. The CreateActionOptions JSDoc for trustSelf reads as though it might be, which is why I verified it.
  • Interaction with autoKnownTxids. It defaults to false, so by default there is no interaction. When a wallet does enable it, prepareKnownTxidsForCreateAction returns early if options.knownTxids is already set (Wallet.ts#L113), so a server-declared list replaces the wallet's auto-derived one rather than merging with it. For payments that is the correct direction: the auto list is "txids this wallet claims to know", which is not the same set as "txids the recipient knows" — marking those txid-only in a payment would hand the recipient a BEEF it cannot verify. Worth a maintainer's eye, since it means the two features deliberately do not compose here.

Both paths that build a payment forward the list, including the regeneration branch taken when a server adjusts its price mid-flight — a repriced retry is already the largest request in the exchange, so dropping the optimisation there would re-ship full ancestry at the worst moment. There is a regression test for that path specifically.

Relationship to #457

#457 already consumes knownTxidsfundingPlanSerializedCost and chooseLowestSerializedFundingPlan pick the funding plan with the smallest serialized BEEF. But nothing populates it on the payment path today, so that comparison currently runs blind: every candidate plan serializes its ancestry in full and the cost function cannot distinguish them on the axis it is measuring.

This PR is the producer for that consumer. It is also independent of it — no shared files, and AuthFetch is untouched across #457's 61 files — so the two can land in either order.

Open question for maintainers: the header name

x-bsv-payment-known-txids is invented — it is not specified in any BRC. It sits inside the x-bsv-payment-* namespace BRC-105 defines, but the string itself is mine. If this merges, the SDK makes it the de facto standard: any server wanting the mechanism has to emit that exact string, without it having gone through the spec.

One constraint worth knowing when deciding: the name must stay inside the x-bsv- namespace. auth-express-middleware only serialises response headers matching x-bsv- and not x-bsv-auth into the signed envelope (index.ts#L1406-1407), which AuthFetch.parseAuthenticatedResponse then rebuilds. A header outside that namespace would be silently dropped in transit on the mutual-auth path — so the question is narrower than it looks: which x-bsv-payment-* string, not which string.

I have no attachment to it. Happy to rename it, or to fold the declaration into BRC-105/118 transport negotiation rather than have it stand alone as its own header — it is one const and a doc block. Worth deciding deliberately now rather than unpicking it after servers depend on it.

Testing

  • SDK typecheck clean; oxlint --deny-warnings clean on src/auth/clients.
  • AuthFetch suites: 125 passed, 6 suites.
  • New tests cover absent, empty, whitespace-only, all-malformed, mixed-valid, duplicate, uppercase and oversized headers — every failure mode degrades to current behaviour rather than throwing.
  • Wiring tests assert the parsed list reaches createAction on the fresh-payment path, that the option is absent (not []) when the server declares nothing, and that it survives the mid-flight reprice path.

Note that CI, CodeQL and Conformance are all sitting at action_required on this PR and have never run — a maintainer approval would be needed to get an actual CI signal.

@imranterranode imranterranode changed the title proposal(sdk): let a 402 declare txids the payer may omit from payment ancestry discussion: payment header size — BRC-118 transport and BRC-96 txid-only ancestry Aug 9, 2026
@ty-everett

Copy link
Copy Markdown
Collaborator

I like this BRC-96 approach a lot, and I like all of the code here. This BRC96 change should land regardless, because it's opt-in, it's sensibly constrained, and it's fail-soft. A very smart approach to this.

I also think the multipart approach deserves more attention, and I'll mess around with some implementations when I get the chance.

@imranterranode

Copy link
Copy Markdown
Contributor Author

Thanks — glad the shape works.

One thing worth flagging before this lands, since it would be easy to let it through by default: the header name x-bsv-payment-known-txids is mine, not from any BRC. It sits in the x-bsv-payment-* namespace BRC-105 defines, but it isn't specified there. If this merges, the SDK makes it the de facto standard — any server wanting to use the mechanism has to emit that exact string — without it having gone through the spec.

I have no attachment to it. Happy for it to be renamed, or folded into BRC-105/118 transport negotiation rather than standing alone as its own header. Worth deciding deliberately now rather than unpicking it later if BRC-118 ends up defining something similar in its negotiation.

Also happy to split the BRC-118 material out into its own issue so this PR is purely the BRC-96 change, and take it out of draft — say the word and I'll do both.

@imranterranode
imranterranode force-pushed the proposal/authfetch-known-txids branch from 7a26e9b to 6a77057 Compare August 10, 2026 12:07
@imranterranode
imranterranode force-pushed the proposal/authfetch-known-txids branch from 6a77057 to b998079 Compare August 13, 2026 06:57
@imranterranode imranterranode changed the title discussion: payment header size — BRC-118 transport and BRC-96 txid-only ancestry feat(sdk): let a 402 declare txids the payer may omit from payment ancestry Aug 13, 2026
@imranterranode
imranterranode marked this pull request as ready for review August 13, 2026 06:57
@imranterranode

Copy link
Copy Markdown
Contributor Author

Done both of the things I offered above, so this is now out of draft and merge-framed.

  • BRC-118 split out into BRC-118 multipart body transport is unimplemented, and header-carried payments fail at 32KB behind Cloudflare #473, so the multipart transport can be designed without this PR waiting on it. Your note about wanting to experiment with implementations is referenced there.
  • The code is unchanged in shape from what you endorsed — same opt-in header, same fail-soft parse, same 256 cap. Two additions: it now also forwards the list on the mid-flight reprice path (the requirementsChanged branch built a second transaction and dropped the option, which is the worst place to drop it), plus a regression test for that; and I verified on the toolbox side that knownTxids needs no trustSelf: 'known' companion, and that a server-declared list replaces rather than merges with autoKnownTxids. Both written up in the body.
  • Rebased onto current main and squashed to one commit. 125/125 across the AuthFetch suites, typecheck and oxlint --deny-warnings clean locally.

The header-name question is still open and now has its own section in the body rather than sitting in a comment. One useful constraint I found while checking: the name has to stay inside the x-bsv- namespace, because auth-express-middleware only serialises x-bsv--and-not-x-bsv-auth response headers into the signed envelope — anything else is silently dropped in transit. So it narrows to which x-bsv-payment-* string. Happy with whatever you pick, including folding it into BRC-105/118 negotiation instead.

One ask: CI, CodeQL and Conformance are all sitting at action_required and have never run on this PR, on any push — they need a maintainer to approve the workflow run. Would be good to have a real CI signal on it before anyone relies on my local run.

@ty-everett

Copy link
Copy Markdown
Collaborator

Approved the CI. Looks like some small CI stuff, but then it's all good after that! For the header, it's not technically specified, but I'm happy to entertain a PR on the BRCs repo to amend 105 with a new, optional header field. If you do that feel free to put yourself in the list of spec authors too :)

Nice work on this, I think the initial header stuff is good. I agree with splitting the other changes out as you've done. I'm not as familiar with the multipart stuff, so I'll leave it for othes to discuss and review. It also sounds reasonable at a high level though.

…cestry

An x-bsv-payment carries the payment transaction plus its ancestry, so the
recipient can verify it without asking anyone. Any ancestor the recipient
already holds is redundant weight, but the payer cannot know which those are,
so it sends all of them. Chained payments therefore grow without bound: each
spends the previous payment's unconfirmed change, so every payment re-ships the
whole unconfirmed run until a block collapses it to a merkle path.

BRC-96 already specifies the shorter encoding (Tx Data Format 02, version
marker 0200BEEF), the SDK already implements it, and wallet-toolbox already
honours createAction's knownTxids end to end. The only missing piece is that
nothing tells the payer which txids the recipient has.

Read an optional x-bsv-payment-known-txids off the 402 and thread it into
createAction. Absent header omits the option entirely, so behaviour is
byte-identical to before. Parsing is fail-soft — malformed entries are dropped
rather than thrown — because a bad header should cost bytes, never a payment.
The list is capped at 256 so a hostile server cannot inflate the createAction
call.

Only the recipient may populate this. Per BRC-96 a txid-only entry "is treated
as implicitly valid", i.e. the recipient verifies nothing about it, so omitting
an ancestor the recipient lacks makes the payment unverifiable. The list must
come from the recipient's own records and must never be inferred by the payer.

Both paths that build a payment forward the list, including the regeneration
branch taken when a server adjusts its price mid-flight — a repriced retry is
already the largest request in the exchange.
@imranterranode
imranterranode force-pushed the proposal/authfetch-known-txids branch from b998079 to 8942d72 Compare August 14, 2026 06:19
@sonarqubecloud

Copy link
Copy Markdown

@imranterranode

Copy link
Copy Markdown
Contributor Author

Rebased onto current main — could you re-approve the workflow runs when you get a chance? All three are sitting at action_required again on the new SHA.

The failure wasn't in this change: pnpm audit:security was tripping on GHSA-2v37-7h3g-55p8 (nanoid <3.3.18, via .>vite>postcss>nanoid), and dcf5fda10 widened the workspace override from nanoid@<3.3.17 to <3.3.18 about four hours after my run started. The other three red jobs were cascades of it (prepare=failure, everything else skipped).

The commit is unchanged — same two files, byte-identical patch, only the base moved. I reproduced the job locally on Node 24 after a clean --frozen-lockfile install: audit:security, ops:check, check-versions, check-sdk-peer, typecheck and lint --deny-warnings all pass, and the full @bsv/sdk suite is 5878/5878 across 154 suites.

On the header name — will do, thanks, and appreciated. I'll open a BRC-105 amendment adding it as an optional field once this lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants