feat(sdk): let a 402 declare txids the payer may omit from payment ancestry - #445
Conversation
|
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. |
|
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 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. |
7a26e9b to
6a77057
Compare
6a77057 to
b998079
Compare
|
Done both of the things I offered above, so this is now out of draft and merge-framed.
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 One ask: CI, CodeQL and Conformance are all sitting at |
|
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.
b998079 to
8942d72
Compare
|
|
Rebased onto current The failure wasn't in this change: 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 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. |



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-paymentcarries 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:Measured on mainnet against a Cloudflare-fronted origin, the request is refused at 32 KB of total request headers —
400 Request Header Or Cookie Too Large, unsigned HTML fromcdn-cgi, before the origin sees it. Reproduced deliberately at 33,058 bytes.Two properties make it worse than a slow leak:
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 marker0200BEEF) 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/mergeTxidOnlywith BEEF V2 as the constructor default; the Go SDK hasNewBeefV2/TxidOnly().wallet-toolboxis wired end to end —mergeAllocatedChangeBeefsforwardsoptions.knownTxids,getBeefForTransactioncallsmergeTxidOnly, andWallet.verifyReturnedTxidOnlyAtomicBEEFguards the return path. Even2.4.4honours 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-txidsoff the 402 and threads it intocreateAction's existingoptions.knownTxids.createActioncall 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.createActioncall.Two things I checked on the toolbox side while writing this, since the change is only useful if the wallet honours it:
trustSelfgate.ActionBatchPlannerappliesmakeTxidOnlyfor every entry inoptions.knownTxidsunconditionally (ActionBatchPlanner.ts#L200), so passing the list alone is sufficient —trustSelf: 'known'is not additionally required. TheCreateActionOptionsJSDoc fortrustSelfreads as though it might be, which is why I verified it.autoKnownTxids. It defaults tofalse, so by default there is no interaction. When a wallet does enable it,prepareKnownTxidsForCreateActionreturns early ifoptions.knownTxidsis 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
knownTxids—fundingPlanSerializedCostandchooseLowestSerializedFundingPlanpick 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
AuthFetchis 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-txidsis invented — it is not specified in any BRC. It sits inside thex-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-middlewareonly serialises response headers matchingx-bsv-and notx-bsv-authinto the signed envelope (index.ts#L1406-1407), whichAuthFetch.parseAuthenticatedResponsethen 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: whichx-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
constand a doc block. Worth deciding deliberately now rather than unpicking it after servers depend on it.Testing
oxlint --deny-warningsclean onsrc/auth/clients.AuthFetchsuites: 125 passed, 6 suites.createActionon 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_requiredon this PR and have never run — a maintainer approval would be needed to get an actual CI signal.