Conversation
imranterranode
changed the title
|
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
imranterranode
changed the title
|
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. |
|
Gentle nudge on the CI gate, and thanks in advance to whoever picks it up.
Whenever one of you has a spare minute: @ty-everett @sirdeggen @BraydenLangley @tonesnotes — no preference at all, first available is perfect. No rush. For context, I think everything else is settled: ty-everett's approval from the 13th still stands, there are no unresolved threads and no conflicts, and the ungated checks are green (Socket Security ×2, SonarCloud quality gate passed). If you're happy to merge once they go green that would save another round, since any further sync with |
|
@ty-everett @sirdeggen @BraydenLangley @tonesnotes friendly nudge on this |
|
4 participants
Program and scope
main.x-bsv-payment. A recipient can now advertise transactions it already possesses and has validated through the optionalx-bsv-payment-known-txids402 response header. AuthFetch passes up to 256 unique, valid lowercase IDs to walletcreateAction, including repriced payments. Missing or invalid-only hints preserve the existing call. Compatible retries reuse the original payment.Impact
Affected packages:
@bsv/sdk2.5.0 (minor: optional behavior and exportedparseKnownTxidsHeader). Existing SDK peers accept this version; no dependent API or dependency-range migration is required. README, API documentation, changelog, release/migration ledger and governed version facts are current. The existing unpublished@bsv/message-box-client2.5.0 minor candidate also records its rebuilt SDK-containing UMD bundle; no further version increment is needed. Publication remains a separate protected workflow action.Verification
pnpm health:check,pnpm lint,pnpm format:check,pnpm typecheck,pnpm build,pnpm audit:security,pnpm docs:examples,pnpm docs:facts:check, and cross-package version checks passed.pnpm --filter @bsv/sdk test:coverage --runInBand— 5,941 tests across 157 suites, plus one snapshot, passed. The 17 known-txid tests cover parsing, normalization, deduplication, exact 256-ID truncation, malformed-only omission, first payment, repricing, authenticated-response parsing, and byte-identical compatible retries with changed or absent hints.515ecf70419d588ac2a3216239c1373b54919cf0. Every emitted CI check passed, and the complete merge gate validates the expected scope-based skips. The repository-owned patch gate passed at 100.00% (17/17 changed line/branch points; target 90%). GitHub still blocks merge because repository ruleset 19713320 requires codecov/patch, while this fork PR intentionally skips external Codecov upload/notification in ci.yml. No Codecov status has been fabricated and no branch-protection setting has been changed.mainmeasured UMD 554,957 → 555,261 raw bytes (+304, 0.055%), gzip 158,185 → 158,309 (+124), Brotli 131,962 → 132,151 (+189). Both bundles contain the same 128 mapped modules; only AuthFetch source content changed. The raw UMD budget moves from 555,000 to 555,500 because the previous build had 43 bytes of headroom. Compressed budgets remain unchanged. Vite raw bytes are 741,743 (+417); esbuild raw bytes are 560,201 (+314), so its raw budget moves from 560,000 to 560,500. Chunk/module composition and all compressed budgets are unchanged. Message Box Client UMD measures 509,963 → 510,247 raw bytes (+284), gzip 145,689 → 145,791 (+102), Brotli 122,096 → 122,173 (+77), with the same 130 mapped modules and only SDK AuthFetch source changed. Its raw budget moves 510,000 → 510,500; compressed limits remain unchanged.Security and dependencies
The hint comes through the existing authenticated
x-bsv-*response envelope. It does not change the amount, payee, signing, or retry authorization. Recipients must advertise only ancestors they already hold and have validated; arbitrary unknown IDs do not inject transactions into wallet BEEF. AuthFetch does not infer recipient knowledge from payer broadcast history. Thesdk-auth-httpmutation target passed at 84.96% (113 mutants; zero uncovered or invalid mutants). No new exception was introduced. Security audit passed the existing governed policy; this change does not add dependencies or advisory exceptions.Dependency evidence
No dependency versions or lockfiles change. SDK 2.5.0 fits existing public peer ranges; workspace SDK references in dependents are development dependencies. The additional browser bytes are measured above, with no new modules.
Release and operations
The SDK minor bump covers the additive API. Existing consumers require no migration. Browser services enabling the hint must expose the optional response header under their existing CORS policy. No service deployment, image, persisted schema or wire encoding changes. Pending SDK release notes are preserved; no artifacts were published.
Completion evidence