feat(sdk): add optional multiplyPoint for wallet-side point masking - #488
Open
connormurray2 wants to merge 3 commits into
Open
feat(sdk): add optional multiplyPoint for wallet-side point masking#488connormurray2 wants to merge 3 commits into
connormurray2 wants to merge 3 commits into
Conversation
…-side Supersedes bsv-blockchain#487, which was overbuilt. That branch added two methods, a wire call code and substrate plumbing across six transports. This is the single operation actually missing, and nothing else. Context. Masking a point through a BRC-100 wallet is already possible today, via revealCounterpartyKeyLinkage plus decrypt, and masks produced that way commute across independent wallets. What has no route through the interface is removing a mask: that needs multiplication by the modular inverse of the derived key. Feeding a*C back through the linkage recipe yields a^2*C, not C. The primitives already exist and this method composes them rather than introducing anything new: const masked = new PublicKey(key.deriveSharedSecret(point)) const inverse = new PrivateKey(key.invm(new Curve().n)) const unmasked = inverse.deriveSharedSecret(masked) // === point That composition requires the private key in application memory. WalletInterface exposes 29 methods and no route to a scalar -- keyDeriver is a property of the in-process class, not part of the interface, so over a substrate there is none. An application whose keys live in a wallet therefore cannot complete the second step. This method runs both steps where the key already is. The first test asserts the output is identical to the composition above, so the behaviour is pinned to the existing primitives rather than to a new definition. Optional, deliberately. BRC-100's value is that it does not change, so a method added later cannot be mandatory: declaring it required on WalletInterface broke 23 call sites across every substrate plus the KV store, registry and identity clients, and declaring it required on ProtoWallet broke @bsv/wallet-toolbox, where Wallet, PrivilegedKeyManager and the wallet managers satisfy the class structurally without extending it. Applications feature-detect and degrade. Wire substrate support is deliberately excluded here; it needs a call code, which is an interface-version decision rather than a library one. Key derivation is mandatory rather than stylistic. For a counterparty point Q, d*Q IS the ECDH shared secret with Q, so performing this with a spending or identity key would hand any caller that secret and break encryption to that counterparty. The key is always derived from protocolID/keyID/counterparty and no identityKey option is offered. On validation: PublicKey.fromString accepts '02' + 'ff'.repeat(32), an x-coordinate greater than the field prime, reduces it silently to 0x1000003d0, and validate() then returns true. A test asserts both halves of that so the reason for the range check is visible. The check runs before the parser, since the parser performs the reduction. go-sdk has the same behaviour independently. Verified: tsc -b clean, oxlint --deny-warnings clean, prettier clean on the files this adds to (the two existing warnings in Wallet.interfaces.ts predate it), full sdk suite green at 157 suites / 5925 tests, and wallet-toolbox at its 4 pre-existing TS2307 baseline with 0 attributable here. 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 13:59
CI failure on bsv-blockchain#488 was six lanes -- merge-gate, build-and-test, browser packages, SDK coverage, wallet browser and wallet mobile -- all one cause: bundle size budgets, and zero type errors anywhere. Measured rather than guessed. Building the UMD bundle from main's SDK gives 554475 bytes against a 555000 budget, so the ratchet was sitting 525 bytes above head. multiplyPoint adds 963, which crosses it. These budgets are deliberate ratchets set just above current size, so any real addition trips them and the fix is to advance them by what the addition actually costs. Reduced the cost before raising anything: the error messages carried a redundant 'the supplied'/'the result is' phrasing that bought no diagnostic value, since the stack already names the function. Trimming those took the delta from 1043 to 963 bytes. Tests still pass -- they match on the specific part of each message, not the prose. Six raw budgets advanced to the next 1000-byte boundary above the observed size, matching the existing convention: sdk umd 555000 -> 556000 (observed 555438) sdk esbuild 560000 -> 561000 (observed 560710) sdk vite 742000 -> 743000 (observed 742268) message-box umd 510000 -> 511000 (observed 510105) wallet client 1607000 -> 1608000 (observed 1607943) wallet mobile 3367000 -> 3368000 (observed 3367997) Only the raw dimension moves. The checker throws on the first dimension over budget, which would have meant discovering these one CI round at a time, so I instrumented it locally to print every measurement at once and then restored it unmodified. That surfaced the esbuild and vite overages before CI reported them. Compressed dimensions have far more slack -- gzip sits 2808 under and brotli 4044 under, against 562 for raw -- because a kilobyte of new source compresses to a few hundred bytes. CI agrees: every failing lane named raw and nothing else. Verified: sdk test:browser passes the full exact-tarball browser contract, tsc -b clean, oxlint clean repo-wide, prettier clean, and the multiplyPoint suite green at 10 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…mension Second round on the same cause, so this time it covers the whole surface rather than only what CI named. CI reported two more overages after the first budget commit: esbuild browser raw at 1253340 against 1252500, and Hermes mobile bytecode gzip at 1366133 against 1366000. Zero type errors, again -- these are purely size ratchets. The Hermes gzip breach is the informative one. My earlier reasoning was that only the raw dimension could realistically breach, because a kilobyte of new source compresses to a few hundred bytes. That held for the SDK, where gzip had 2808 bytes of slack, but it is wrong here: Hermes gzip was cut to 133 bytes above head. These budgets are set that fine on every dimension, so whichever is tightest breaches first and patching one at a time invites another CI round for the same kilobyte of code. So both breached dimensions are raised to the next 500-byte step above the observed value, and the sibling dimensions on the same bundles get the same small allowance -- vite and esbuild gzip/brotli on the client, hermes brotli and the whole metro triple on mobile. Every increase is 500 to 1500 bytes, proportional to the roughly one kilobyte of source multiplyPoint adds, and none of them loosens a budget beyond what that growth accounts for. I tried to measure these locally rather than infer them, instrumenting check-wallet-toolbox-platform.mjs to print every dimension the way I did for the SDK checker. The wallet lanes pack a tarball and resolve it as an external consumer, which needs CI's setup, so the run fails before measuring. The script is restored unmodified -- confirmed by an empty diff under scripts/. Verified: oxlint clean repo-wide, sdk tsc -b clean, the multiplyPoint suite green at 10 tests, prettier clean on both budget files, the SDK exact-tarball browser contract still passing at raw 555438 / gzip 159192 / brotli 131956, and the diff containing nothing but the two budget files. 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.
Supersedes #487, which was overbuilt — thanks @deggen for pushing back on it. That branch added two methods, a wire call code, and substrate plumbing across six transports. This is the one operation actually missing.
What's already possible
Masking a point through a BRC-100 wallet works today.
revealCounterpartyKeyLinkage+decryptreturnsa·Pfor an arbitrary point, and I verified masks produced that way commute across independent wallets — which is the property mental poker rests on. No new method needed for that half.What has no route through the interface
Removing a mask. That needs multiplication by the modular inverse of the derived key. Feeding
a·Cback through the linkage recipe givesa²·C, notC— it masks again rather than stripping.To be clear about the maths, since this came up:
a⁻¹here is the modular inverse of a scalar mod n — extended Euclid, microseconds. It is not the discrete log. The wallet already holdsa, soa⁻¹is free, and recoveringafrom(C, a·C)is exactly as hard as from(G, a·G).invertchanges nothing about that.The primitives already exist
This method composes them rather than introducing anything new:
That composition needs the private key in application memory.
WalletInterfaceexposes 29 methods and no route to a scalar —keyDeriveris a property of the in-process class, not part of the interface, so over a substrate there is none at all. An application whose keys live in BSV Desktop can do the first step and not the second.multiplyPointruns both steps where the key already is. The first test asserts its output is byte-identical to the composition above, so the behaviour is pinned to the existing primitives rather than to a new definition.Why it's optional
BRC-100's value is that it doesn't change, and I have the bruises to prove the point:
WalletInterface→ 23 broken call sites: every substrate (WalletClient,HTTPWalletJSON,WalletWireTransceiver,window.CWI,XDM,ReactNativeWebView) plus the KV store, registry, and identity clients.ProtoWallet→ broke@bsv/wallet-toolbox, whereWallet,PrivilegedKeyManager, and the wallet managers satisfy the class structurally without extending it.Optional plus feature detection was the only shape that compiled. A test pins the structural contract, since every behavioural test passed while the type was wrong.
Wire substrate support is deliberately excluded. It needs a call code, which is an interface-version decision rather than a library one — happy to follow the coordination process for that separately.
Why derivation is mandatory
For a counterparty point
Q,d·Qis the ECDH shared secret withQ. Performing this with a spending or identity key would hand any caller that secret and break BRC-2 encryption to that counterparty. The key is always derived fromprotocolID/keyID/counterparty, and noidentityKeyoption is offered.One validation note
PublicKey.fromStringaccepts'02' + 'ff'.repeat(32)— an x-coordinate greater than the field prime — reduces it silently to0x1000003d0, andvalidate()then returnstrue. A test asserts both halves so the reason for the range check is visible in the code. The check runs before the parser, since the parser is what performs the reduction.go-sdkbehaves the same way independently.Use case
A non-custodial poker game: players hold their own keys in BSV Desktop, the pot is n-of-n multisig with each seat co-signing settlement, and pre-signed nLockTime refunds mean a stall can't trap funds. The money side already works on teratestnet. The deal is Barnett–Smart mental poker — card i encoded as
(i+1)·G— which needs mask and strip per player, per hand. Mask works today; strip is what this adds.Verification
tsc -bcleanoxlint --deny-warningscleanprettier --checkclean on the files this adds to (the two existing warnings inWallet.interfaces.tsare at lines 212 and 884 and predate this)@bsv/wallet-toolboxat its 4 pre-existingTS2307baseline, 0 attributable hereEntirely happy to be told this belongs outside the wallet — masking scalars are per-hand secrets, not custody, and an app holding ephemeral keys for them is a legitimate answer. Raising it here so the interface question gets decided rather than assumed.