Encode BIP137 SegWit message signature headers#454
Conversation
j0ntz
left a comment
There was a problem hiding this comment.
Automated code review (workflow-backed, high effort). All findings are advisory; the core BIP137 header fix is correct and matches the task requirement. Three notes below.
| // SegWit (bip84 / P2WPKH) and nested SegWit (bip49 / P2SH-P2WPKH) each have | ||
| // their own header range; verifiers such as Bringin reject a SegWit address | ||
| // whose message was signed with a legacy (bip44) header byte. | ||
| const segwitType = |
There was a problem hiding this comment.
Minor (robustness): the format -> segwitType mapping compares raw 'bip84'/'bip49' string literals rather than routing through the codebase's canonical BIP43PurposeTypeEnum / bip43PurposeNumberToTypeEnum. If a new SegWit CurrencyFormat is ever added, this ternary is easy to miss (no compiler linkage) and the new format silently falls through to undefined, producing a legacy header that BIP137 verifiers reject. Consider deriving the segwit kind from the purpose enum so the two stay in lockstep.
There was a problem hiding this comment.
Leaving open as an optional cleanup. The coin-prefix fix (the operator's requested scope) does not depend on it, so this change stays minimal; routing the segwitType selection through BIP43PurposeTypeEnum is a good follow-up but out of scope for this task.
signMessageBase64 always emitted a legacy header byte (27-34) regardless of the signing address type, so BIP137 verifiers (e.g. Bringin) rejected signatures from SegWit addresses. Thread the address format through and select the matching segwitType: bip49 -> p2sh(p2wpkh) (header 35-38), bip84 -> p2wpkh (header 39-42), bip44/bip32 unchanged (31-34).
0c9e6a2 to
9594db2
Compare

CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
none
Description
Asana task
Edge's BTC message signing (used for CEX/ramp withdrawal ownership proofs, e.g. Bringin / NiceHash Travel Rule) produced a signature whose header byte always encoded a legacy P2PKH type (27-34), regardless of the signing address. BIP137 verifiers reject a legacy-header signature that comes from a SegWit address: nested SegWit (P2SH-P2WPKH) requires header 35-38 and native SegWit (P2WPKH / bech32) requires 39-42. Because Edge BTC wallets default to SegWit, the signatures Edge emitted were rejected by Bringin; a user had to run a script to rewrite the header to BIP137 before it was accepted, or fall back to selfie/ID verification.
signMessageBase64inkeymanager.tscalledbitcoinMessage.sign(message, privKey, compressed)with nosegwitType, so the header byte only reflected the compressed flag. This threads the address's derivationformatthrough and selects the matchingsegwitType:bip84(native SegWit / P2WPKH) -> header 39-42bip49(nested SegWit / P2SH-P2WPKH) -> header 35-38bip44/bip32(legacy P2PKH) -> header 31-34 (unchanged)The ECDSA signature and message hash are unchanged; only the header byte differs, so legacy P2PKH signing is byte-for-byte identical. The fix is general across all UTXO coins that support SegWit, since
path.formatreflects the actual address derivation.Testing
bip44,bip49, andbip84(extendedsignMessageTestsin the keymanager spec).tsc,eslint, and the fullmochasuite pass.bitcoinjs-message.verifyagainst its real P2PKH / P2SH-P2WPKH / bech32 address.EdgeCurrencyWallet.signMessagepath with this build linked, on the wallet's own receive addresses. The nested-SegWit3...address produced header 35 and the native-SegWitbc1q...address produced header 40, both verifying against their respective addresses (screenshot attached). Before the fix both emitted header 31, which fails BIP137 verification for a SegWit address.Asana: https://app.asana.com/1/9976422036640/project/1215088146871429/task/1216403654258303
Note
Medium Risk
Changes cryptographic message output for SegWit and all coins using coin-specific prefixes; legacy bip44 Bitcoin ECDSA payload is unchanged but integrators comparing old signatures on altcoins may see differences.
Overview
Fixes UTXO message signing so ramp/CEX ownership proofs verify on BIP137-compliant checkers (e.g. Bringin) when users sign from SegWit receive addresses.
signMessageBase64now takes the walletpath.formatandcoin, maps bip84 / bip49 to the correctsegwitTypeforbitcoinjs-message, and hashes with each coin’smessagePrefixinstead of Bitcoin-only defaults.UtxoWalletTools.signMessageBase64passespath.formatand the plugin coin id through.Legacy bip44 signing is unchanged aside from explicit coin prefixes (which can change signatures for non-Bitcoin coins vs the old implicit behavior). Tests add bip44/bip49/bip84 vectors for Bitcoin and Litecoin.
Reviewed by Cursor Bugbot for commit 9594db2. Bugbot is set up for automated code reviews on this repo. Configure here.