feat(sdk): implement BRC-229 multiplyPoint in ProtoWallet - #487
Closed
connormurray2 wants to merge 4 commits into
Closed
feat(sdk): implement BRC-229 multiplyPoint in ProtoWallet#487connormurray2 wants to merge 4 commits into
connormurray2 wants to merge 4 commits into
Conversation
Reference implementation for BRC-229 (bsv-blockchain/BRCs#230). Multiplies a caller-supplied secp256k1 point by a BRC-42/43 derived private key without disclosing the key, so commutative-masking protocols -- Barnett-Smart mental poker, verifiable shuffles, oblivious transfer -- can run against a wallet-held key instead of requiring the application to generate and store secp256k1 keys of its own. In the mental-poker case those application-held keys are exactly what the privacy of a player's hand depends on. Every primitive already existed: KeyDeriver.derivePrivateKey, Point.mul, and BigNumber.invm for the invert path. The work here is validation and key discipline, not new cryptography. multiplyPoint is OPTIONAL on WalletInterface, and that is a correction to the spec rather than a convenience. Declaring it required broke 23 call sites across every substrate (WalletClient, HTTPWalletJSON, WalletWireTransceiver, window.CWI, XDM, ReactNativeWebView) plus the KV store, registry and identity clients. BRC-100 is billed as an unchanging interface, so a method added after the fact cannot be mandatory without invalidating every wallet already shipped. Applications must feature-detect and degrade. The canonical-encoding rule the spec makes normative is confirmed necessary in THIS package, not merely in theory: PublicKey.fromString accepts '02' + 'ff'*32, an x-coordinate greater than the field prime, silently reduces it to 0x1000003d0, and validate() then returns true. Verified by probe before writing the check. go-sdk has the same defect independently, which makes it an interoperability hazard rather than one library's quirk. parseValidPoint rejects the coordinate BEFORE parsing, since the parser performs the reduction. VerifiableCertificate.decryptFields is retyped from ProtoWallet to Pick<ProtoWallet, 'decrypt'>. Adding any method to ProtoWallet narrows what structurally satisfies it, which broke a call passing a WalletInterface; the parameter was over-specified, as the body only ever calls decrypt(). Tests assert the properties a deal depends on rather than that the method returns a string: masks from independent wallets commute, invert recovers the original point, a three-way mask strips in an order different from the one applied, protocol/keyID/counterparty each separate keys, two wallets never derive the same protocol key, a 52-card deck masks to 52 distinct points with a selective reveal leaving the rest unreadable, and the non-canonical x-coordinate is rejected. Verified: tsc -b clean (the pre-existing TS5095 in tsconfig.cjs.json aside), oxlint --deny-warnings clean, prettier clean on both touched files, and the full sdk suite green at 157 suites / 5924 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
connormurray2
requested review from
BraydenLangley,
sirdeggen,
tonesnotes and
ty-everett
as code owners
August 20, 2026 00:34
Without this the primitive works in-process only, which excludes every real wallet: BSV Desktop and anything else an application reaches is on the far side of a substrate. This is the half that makes the method usable from a browser. Call code 29, taking the next free slot after getVersion = 28, matching the BRC-100 call code table. Frame layout: the point travels as its 33 raw bytes rather than as hex, matching how getPublicKey returns a key, followed by the existing key-related parameter block (protocolID, keyID, counterparty, privileged, privilegedReason) and then invert and seekPermission as optional booleans. invert is written with the optional-boolean encoding rather than a bare flag specifically so a wallet reading a frame that lacks it cannot mistake absence for true -- silently inverting a mask would corrupt a deal rather than fail it. Because multiplyPoint is optional on the interface, the processor checks for the method before dispatching and returns a legible wire error instead of calling undefined. WalletClient does the same and additionally exposes supportsMultiplyPoint(), so an application can feature-detect and choose a fallback before committing to a protocol that needs the primitive. That is the concrete form of the feature detection BRC-229 requires. Coverage across InvokableWalletBase (window.CWI, XDM, ReactNativeWebView), HTTPWalletJSON, WalletWireTransceiver/Processor and WalletClient. Seven wire tests. The load-bearing ones: a mask applied through the wire strips through the wire, and the wire result equals the in-process result for the same key and point, so the encoding demonstrably loses nothing. Also covered: call code 29 is where the table says, masks commute across two wallets each reached over a substrate, counterparty and invert survive the frame independently, the non-canonical x-coordinate is still rejected after the round trip, and a wallet lacking the method produces a clear error. Verified: tsc -b clean (pre-existing TS5095 aside), oxlint --deny-warnings clean, prettier clean on the files this commit newly touches -- WalletWireCalls, InvokableWalletBase and WalletClient were already unformatted on a clean tree and are deliberately left that way rather than reformatted here. Full sdk suite green at 158 suites / 5931 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI failure, and my own inconsistency: multiplyPoint was declared optional on WalletInterface but required on the ProtoWallet class. A required member -- method or property, an arrow property is no different -- narrows what structurally satisfies ProtoWallet, so every implementor that does not extend the class stops type-checking. That broke @bsv/wallet-toolbox with 6 compile errors: Wallet and PrivilegedKeyManager 'incorrectly implements class ProtoWallet', three 'Argument of type this is not assignable to parameter of type ProtoWallet', and proveCertificate reporting multiplyPoint missing from Wallet. Confirmed against a baseline build of main's SDK, which leaves wallet-toolbox at its 4 pre-existing TS2307 errors for unrelated missing express middleware packages -- so all 6 were mine. Declaring the member with and assigning the implementation keeps ProtoWallet as wide a structural type as it was before BRC-229 existed, while the class still provides the method. This is the same conclusion the interface change already reached, applied consistently: an opt-in capability must be opt-in on every type that carries it, or it is not opt-in at all. Adds a regression test asserting the structural shape rather than the behaviour, since the behaviour tests all passed while the type was wrong. It pins that an object with no multiplyPoint still satisfies Pick<ProtoWallet, 'multiplyPoint'>. Verified: wallet-toolbox back to its 4 baseline errors with 0 attributable to this branch, sdk tsc -b clean, oxlint clean repo-wide, prettier clean on every file this branch touches, full sdk suite green at 158 suites / 5932 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Clears the two SonarCloud findings that failed the zero-new-findings gate on PR bsv-blockchain#487 (typescript:S7786, WalletClient.ts:248 and WalletWireProcessor.ts:1186): 'new Error() is too unspecific for a type check. Use new TypeError() instead.' Sonar is right on the substance rather than merely on style. Both sites test typeof wallet.multiplyPoint !== 'function' -- a failed type check, which is exactly what TypeError denotes. A caller feature-detecting BRC-229 support can now distinguish 'this wallet lacks the capability' from a protocol or validation failure by error type instead of by parsing a message. Also splits the parseValidPoint guard, which conflated two different faults in one condition. A non-string argument is a type error; a string in the wrong format is not. They now raise TypeError and Error respectively. Sonar did not flag this one, but it is the same class of defect and would likely surface once the rule is applied to the file again. Verified: sdk tsc -b clean, oxlint clean repo-wide, prettier clean on every touched file, full sdk suite green at 158 suites / 5932 tests, and wallet-toolbox still at its 4 pre-existing TS2307 baseline with 0 attributable to this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference implementation for BRC-229, proposed in bsv-blockchain/BRCs#230.
multiplyPointmultiplies a caller-supplied secp256k1 point by a BRC-42/43 derived private key and returns the result, without disclosing the key. Aninvertflag multiplies by the modular inverse, so a mask the wallet applies is a mask the wallet can remove.Why
Commutative-masking protocols rest on
a·(b·P) == b·(a·P): Barnett–Smart mental poker, verifiable shuffles, several oblivious-transfer constructions.getPublicKeygivesd·G, but nothing givesd·Pfor an arbitraryP, so an application needing this must generate and hold its own secp256k1 keys outside the wallet. In the mental-poker case those keys are exactly what the privacy of a player's hand depends on — whoever holds the masking scalars can read every card.Every primitive already existed:
KeyDeriver.derivePrivateKey,Point.mul, andBigNumber.invmfor the invert path. The work here is validation and key discipline, not new cryptography.multiplyPointis optional, and that is a correction to the specI first declared it required on
WalletInterface, as BRC-229 originally specified. That broke 23 call sites — every substrate (WalletClient,HTTPWalletJSON,WalletWireTransceiver,window.CWI,XDM,ReactNativeWebView) plus the KV store, registry, and identity clients.BRC-100 is billed as an unchanging interface, so a method added after the fact cannot be mandatory without invalidating every wallet already shipped. It has to be optional, with applications feature-detecting (
typeof wallet.multiplyPoint === 'function') and degrading. I'll amend the BRC text accordingly.The canonical-encoding check is not redundant with the on-curve check
PublicKey.fromStringaccepts'02' + 'ff'.repeat(32)— an x-coordinate numerically greater than the field prime — silently reduces it to0x1000003d0, andvalidate()then returnstrue. An implementation validating only withvalidate()therefore accepts a point that was never validly encoded, which is the entry point for invalid-curve attacks.I verified this by probe against this package before writing the check, and
go-sdkhas the same defect independently — so it is an interoperability hazard rather than one library's quirk.parseValidPointrejects the coordinate before parsing, since the parser is what performs the reduction. A test asserts this exact value is rejected.One incidental type fix
VerifiableCertificate.decryptFieldstook a concreteProtoWalletbut only ever callsdecrypt(). Adding any method toProtoWalletnarrows what structurally satisfies it, which broke a call passing aWalletInterface. Retyped toPick<ProtoWallet, 'decrypt'>— the parameter was over-specified and my change merely exposed it.Tests
Nine tests asserting the properties a deal depends on, rather than that the method returns a string: masks from independent wallets commute,
invertrecovers the original point, a three-way mask strips in an order different from the one applied,protocolID/keyID/counterpartyeach separate keys, two wallets never derive the same protocol key, a 52-card deck masks to 52 distinct points with a selective reveal leaving the rest unreadable, and the non-canonical x-coordinate is rejected.Verification
tsc -b— clean (the pre-existingTS5095intsconfig.cjs.jsonis unrelated and reproduces on a clean tree)oxlint --deny-warnings— cleanprettier --check— clean on both files I touchedjest— 157 suites, 5924 tests, all passingNot included
Wire-substrate support.
packages/sdk/src/wallet/BRC100ByteEncoding.tswould need call code 29 registered for the serialized substrates, andpackages/walletwould need the corresponding handler. Those are separable from theProtoWalletprimitive and I'd rather land this first than mix concerns — happy to follow up if you'd like them in the same PR.