Skip to content

fix(wasm-utxo): reject duplicate wallet role keys - #414

Draft
ralph-bitgo[bot] wants to merge 1 commit into
masterfrom
WCN-1863-distinct-wallet-roles
Draft

ralph-bitgo[bot] wants to merge 1 commit into
masterfrom
WCN-1863-distinct-wallet-roles

Conversation

@ralph-bitgo

@ralph-bitgo ralph-bitgo Bot commented Sep 29, 2026

Copy link
Copy Markdown

What

  • RootWalletKeys::new / new_with_derivation_prefixes are now fallible and reject repeated role xpubs with a typed WalletKeyError::DuplicateRootKeys, surfaced to JS/WASM callers as a branchable code: "WalletKeyError.DuplicateRootKeys" error instead of a StringError.
  • Added require_distinct_pubkeys and enforced it at the shared sink WalletScripts::new and in every script family: build_multisig_script_2_of_3 (builder and parser), ScriptP2tr::new, ScriptP2mr::new, build_tap_tree_for_output, create_tap_bip32_derivation_for_output. The second, derived-key check protects custom derivation prefixes and any future constructor that bypasses the root check.
  • Removed the Taproot expect calls: BitGoMusigError (e.g. InvalidPubkeyCount for identical user/BitGo keys) is now a typed WasmUtxoError::BitGoMusig propagated through ScriptP2tr::new and the PSBT/BIP-322 callsites, instead of a panic that traps the whole WASM instance.
  • Regression coverage: all three duplicate-pair permutations plus the all-equal triple, default and custom derivation prefixes, every output script family, both Taproot aggregation modes, and both the TypeScript wrapper and raw WASM constructors (including a post-failure call proving the module stays usable).

Why

A wallet threshold counts distinct authorization principals, but nothing in this library enforced that (AnchorWatch finding csf_be4ee1b1031cad5e945939bf2, WCN-1863). With the same xpub in two roles, the legacy builders emitted a nominal 2-of-3 OP_CHECKMULTISIG script [A,A,B]; Bitcoin consensus accepts one valid signature copied into two stack positions, so a single key holder could spend unilaterally once a victim funded an address derived from the malicious triple. On Taproot chains the duplicate user/BitGo key reached expect("valid aggregation") and aborted the entire WASM instance rather than returning a typed error. This closes the quorum-collapse path fail-closed at the root-key boundary and again on derived keys immediately before script assembly.

Test plan

  • CI: cargo test + cargo clippy + cargo fmt (wasm-utxo)
  • CI: npm run build:test && npm run test:mocha (wasm-utxo), including test/fixedScript/walletKeys.ts
  • Review: duplicate-role rejections typed as WalletKeyError.DuplicateRootKeys / DuplicateDerivedKeys, not string errors
  • Review: Taproot duplicate user/BitGo keys return BitGoMusigError.InvalidPubkeyCount instead of panicking

Verification note: this sandbox has no cargo/rustc, no Nix dev shell, and no node_modules/generated WASM bindings, so the suites could not be executed locally before opening this draft. The branch CI is the verification gate — do not mark ready for review until the above checks are green.

Ticket: WCN-1863

Reject duplicate root xpubs and derived keys before wallet scripts or
PSBT metadata creation. Propagate typed Taproot aggregation errors.

Prevent one authority from occupying multiple quorum positions and
trapping WASM when aggregation fails.

Ticket: WCN-1863
Session-Id: d899491b-4b54-442d-bfa3-99fc93352b00
Task-Id: 05aabeb7-a6fa-45bb-adb0-5b6323263435
Requested-By: David Kaplan <davidkaplan@bitgo.com>
@linear-code

linear-code Bot commented Sep 29, 2026

Copy link
Copy Markdown

WCN-1863

This branch has not been deployed

No deployments
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