Honor bip122 signMessage protocol - #6222
Conversation
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
peachbits
left a comment
There was a problem hiding this comment.
The protocol plumbing and the account/address checks look right. This needs to land together with the edge-currency-plugins bump that includes EdgeApp/edge-currency-plugins#461. The published plugin cleans an unknown signatureFormat to electrum, so until the bump a bip322 request would silently get an Electrum signature.
|
Agreed on the ordering: this lands only together with the edge-currency-plugins bump that includes #461. Until that version is published the plugin cleans the unknown signatureFormat to electrum, so this PR stays unmerged. |
asLegacyTokenId is deprecated outside notification-server payloads. asOptional(asEdgeTokenId, null) keeps the same undefined-to-null mapping for payload parsers that omit tokenId on the native asset.
WalletConnect bip122 signMessage requests carry a protocol of ecdsa (the default) or bip322, but the modal always signed with BIP-137. A dapp asking for BIP-322 got a 65-byte BIP-137 signature back, the same bytes it got for ecdsa. The request's protocol now reaches WcSignMessageModal: bip322 signs with the UTXO plugin's bip322 signatureFormat, and ecdsa keeps BIP-137. An unknown protocol fails the cleaner and rejects the request. The optional address param is checked against the session address the same way as account.
c2f88fa to
27d5b5f
Compare
Description
WalletConnect bip122
signMessagerequests carry aprotocolofecdsa(the default) orbip322.WcSignMessageModalalways signed with BIP-137, so a dapp asking for BIP-322 got the same 65-byte BIP-137 signature it got forecdsa.The request's
protocolnow reaches the modal.bip322signs with the UTXO plugin's newbip322signatureFormat, andecdsakeeps BIP-137. An unknownprotocolfails the cleaner and rejects the request. The optionaladdressparam is checked against the session address the same way asaccount.A separate commit swaps the deprecated
asLegacyTokenIdin the WalletConnect payload cleaner forasOptional(asEdgeTokenId, null), which maps a missingtokenIdtonullthe same way.The
bip322path needs edge-currency-plugins with EdgeApp/edge-currency-plugins#461. The current published plugin does not know thebip322format, and this branch does not bump the plugin version.Tested on the iOS simulator with the plugin change linked in: a WalletConnect test dapp paired through an
edge://wcdeep link and requested both protocols from a native SegWit wallet.bip322: a 107-byte witness that an independent BIP-322 verifier (hand-built BIP-143 sighash, secp256k1 verify) accepts for the wallet's bc1q address and rejects for a tampered message.ecdsa: a 65-byte BIP-137 signature that verifies with bitcoinjs-message.Asana: WalletConnect Bitcoin proof of ownership
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
EdgeApp/edge-currency-plugins#461
Requirements
If you have made any visual changes to the GUI. Make sure you have:
Note
Medium Risk
Changes WalletConnect Bitcoin proof-of-signing behavior and depends on an unreleased UTXO plugin for the new
bip322format; invalid protocols are rejected at parse time.Overview
WalletConnect bip122
signMessagecalls can ask forecdsa(BIP-137) orbip322. The app previously always signed with BIP-137, so dapps requesting BIP-322 still got a recoverable ECDSA-style signature.The service now parses optional
protocol,address, andaccountfrom the request, rejects requests that name an unservable address/account, and passesprotocolintoWcSignMessageModal, which mapsbip322→signatureFormat: 'bip322'andecdsa→'bip137'before hex-encoding the result for the spec.Separately, the WalletConnect payload amount cleaner replaces deprecated
asLegacyTokenIdwithasOptional(asEdgeTokenId, null)so native transfers without atokenIdfield parse asnull.Note: BIP-322 signing requires a matching
edge-currency-pluginsrelease; this repo does not bump that dependency in this PR.Reviewed by Cursor Bugbot for commit c2f88fa. Bugbot is set up for automated code reviews on this repo. Configure here.
Test evidence
c2f88faHonor bip122 signMessage protocol
2026-09-25
1. wc connect
2. bip322 modal
3. ecdsa modal