feat(wallets): per-cosigner message signing across keystores - #511
Closed
bucko13 wants to merge 33 commits into
Closed
feat(wallets): per-cosigner message signing across keystores#511bucko13 wants to merge 33 commits into
bucko13 wants to merge 33 commits into
Conversation
🦋 Changeset detectedLatest commit: 948c273 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Adds packages/caravan-wallets/src/messages.ts with:
- Entry: canonical record returned by each keystore's
SignMessage.run() (bip32Path, signature, legacy, expectedPubkey).
Self-describing so the verifier does not need per-keystore
knowledge.
- verifyMessageSignature({message, entry}): pubkey-aware verifier
wrapping bip322-js in loose mode. Derives a canonical P2WPKH
address from entry.expectedPubkey. Loose mode is required because
BIP-322 spec prohibits BIP-137 over P2WPKH, and caravan's cosigner
paths in P2WSH multisig wallets canonicalize to P2WPKH; strict
mode would reject every Ledger / Trezor / Jade output against
such a path.
- validateMessage: UTF-8 + no NUL + <=240-byte encoding gate.
Whitespace handling is left to UI surfaces since on-device approval
screens render whitespace invisibly.
- MessageSigningError: closed-set discriminated union covering
UnsupportedProtocol, UnsupportedAddressType, DeviceRejected,
TransportError, MalformedResponse, MalformedRequest.
- NormalizeSignature, CapabilityFlags: contracts each per-keystore
SignMessage interaction will implement.
Tests cover encoding policy, single-path verifier round-trips
(BIP-322 Simple + BIP-137 in loose mode) with negatives, and
per-fixture round-trips across TEST_FIXTURES.multisigs (P2SH /
P2SH-P2WSH / P2WSH x testnet/mainnet). Each fixture also exercises
a "claim unchained pubkey" negative using publicKeys[]; the
unchained seed is not in TEST_FIXTURES, same constraint the existing
PSBT signingTests operate under.
bip322-js added as @caravan/wallets dependency; @scure/bip32,
@scure/bip39, bitcoinjs-message, wif as devDependencies for tests.
bip322-js@3 transitively pulls bitcoinjs-lib@6 under its subtree;
the top-level bitcoinjs-lib@5 dep is unaffected.
…tedPubkey
LedgerSignMessage changes:
- Constructor accepts expectedPubkey (required).
- Validates the message via validateMessage from messages.ts
(UTF-8, no NUL, <=240 bytes). Throws MessageSigningError
{MalformedRequest} on policy violations before contacting the
device.
- .run() returns Promise<Entry>. Normalizes Ledger's {v,r,s}
response to a base64 BIP-137 65-byte signature (header byte =
v + 27 + 4, then r||s) via a new normalizeLedgerSignature helper.
caravan's loose-mode verifier ignores the address-type bits in
the header.
- Fixes a latent bug: the previous code called
app.signMessageNew(...), which does not exist in
@ledgerhq/hw-app-btc. The SDK exposes app.signMessage(path,
messageHex). Hex-encodes the UTF-8 message before the SDK call.
This class is the BIP-137 implementation for Ledger.
SignMessage factory:
- Adds expectedPubkey (required) to the input type. Only the
LEDGER case threads it; JADE / TREZOR cases will be adapted in
follow-up commits.
Tests:
- Constructor throw on oversize message.
- End-to-end .run() with mocked transport asserting the SDK call
shape (positional path + hex-encoded message) and the resulting
Entry fields.
TrezorSignMessage changes:
- Constructor accepts expectedPubkey (required) plus network so the
base TrezorInteraction can resolve coin code.
- Validates the message via validateMessage (UTF-8, no NUL,
<=240 bytes).
- parsePayload now returns Entry. Trezor's SDK returns
{address, signature}; address is ignored — caravan verifies
against expectedPubkey, not the device-derived address.
Fixes a latent bug: the base class previously called
`super({ network: Network[network] })`. `Network` is a string enum,
so the reverse-lookup is always undefined and `trezorCoin()` always
fell into the testnet branch. Pass `network` through unchanged.
SignMessage factory:
- Adds optional `network` param. Threaded for TREZOR.
- Threads expectedPubkey for TREZOR.
Tests:
- Constructor throw on oversize message.
- connectParams returns the expected (method, params) shape with
the right coin label for mainnet vs testnet.
- parsePayload maps {address, signature} → Entry.
…byte
JadeSignMessage changes:
- Constructor accepts expectedPubkey (required).
- Validates the message via validateMessage.
- .run() returns Entry. Jade's SDK emits a raw 64-byte EC signature
with no recovery byte (the firmware exposes the sig without
protocol-level conformance to BIP-137 or BIP-322), so the
interaction reconstructs the canonical BIP-137 wire form.
normalizeJadeSignature:
- Tries both `v` candidates ({0, 1}) by building
[v + 27 + 4][r][s] and verifying against expectedPubkey via the
loose-mode verifier in messages.ts.
- Throws MessageSigningError{MalformedResponse} if neither candidate
recovers. This doubles as a guard against a wrong wallet loaded on
the device.
- Guards against the anti-exfil tuple shape: when useAeSignatures is
enabled the SDK returns [sig, hostCommitment]; caravan does not
request anti-exfil mode, so the tuple shape is unexpected and the
normalizer throws MalformedResponse rather than misinterpreting
the bytes.
SignMessage factory:
- Threads expectedPubkey for JADE.
Tests:
- Constructor throw on oversize message.
- Raw sig of wrong length → MalformedResponse.
- Anti-exfil tuple shape → MalformedResponse.
- Sig that does not recover to expectedPubkey under either v →
MalformedResponse.
- Happy-path round-trip: synthesize a Jade-style raw EC sig (strip
the header byte off a bitcoinjs-message BIP-137 sig), confirm
.run() reconstructs a canonical Entry that verifies.
…eystore
LedgerSignMessage now dispatches internally on app generation. The
class already supported the legacy Bitcoin app via
@ledgerhq/hw-app-btc; this commit adds support for the v2 Bitcoin app
exposed by ledger-bitcoin's AppClient.
Run-loop changes:
- After isAppSupported() / isLegacyApp() resolve, the run path picks
between Btc.signMessage(path, messageHex) (legacy, returns {v,r,s}
— normalizer wraps as [v+31][r][s] base64) and
AppClient.signMessage(messageBuf, path) (v2, returns base64
BIP-137 directly — note the reversed positional order vs the
legacy SDK).
- Sets isV2Supported = true. Was previously a placeholder.
SignMessage factory:
- LEDGER and LEDGER_V2 both route to LedgerSignMessage; the class
detects the app version at .run() time.
Tests:
- Mocked legacy-app path: assert SDK call shape and normalized sig.
- Mocked v2-app path: assert SDK call shape (reversed args) and
pass-through base64.
Wires per-cosigner message signing into the in-app keystore test
runner so contributors with a physical device can validate the new
SignMessage factory + verifier end to end.
apps/coordinator/src/tests/messageSigning.jsx:
- For each TEST_FIXTURES.multisigs fixture, build a MessageSigningTest
bound to the open_source cosigner's expected pubkey at the
fixture's bip32Path.
- Test description renders address type, message, BIP-32 path,
expected pubkey, and protocol (BIP-137 loose-mode verification).
- interaction() calls SignMessage({keystore, network, bip32Path,
message, expectedPubkey}).
- matches() invokes verifyMessageSignature cryptographically — not
byte equality — because BIP-322 Simple is non-deterministic at the
wire level.
apps/coordinator/src/tests/index.js:
- Adds messageSigningTests to the test category lookup, keyed on
TYPES.MESSAGE_SIGNING.
- Threads the category through buildTests / fixturesForType so the
/#/test UI renders it alongside the existing signing tests.
- Wired for direct keystores: LEDGER, LEDGER_V2, TREZOR, JADE,
BITBOX. COLDCARD's indirect (SD-card) flow lands in a separate
commit alongside the keystore class. BCUR2 / Hermit / Custom are
excluded.
Coordinator tests pass; e2e does not exercise this surface since it
requires a physical device.
Adds BitBoxSignMessage to the BitBox keystore driver, completing
SignMessage coverage for direct-transport keystores.
BitBoxSignMessage:
- Constructor: requires network (for BitBox's coin code via
convertNetwork — btc/tbtc/rbtc), bip32Path, message, expectedPubkey.
Validates the message via validateMessage.
- .run() calls pairedBitBox.btcSignMessage(coin, {scriptConfig:
{simpleType: 'p2wpkh'}, keypath}, TextEncoder().encode(message))
inside withDevice. Returns Entry.
- BitBox returns {sig, recid, electrumSig65}. electrumSig65 is
already the canonical 65-byte BIP-137 wire form (header + r + s)
with the Electrum header convention; caravan base64-encodes it
directly. Loose-mode verification tolerates any header.
- Validates the response: throws MessageSigningError{MalformedResponse}
if electrumSig65 is missing or not exactly 65 bytes.
SignMessage factory:
- Threads network for BITBOX. Throws MessageSigningError
{MalformedRequest} if network is missing — BitBox cannot sign
without a coin code.
Tests:
- Constructor throw on oversize message.
- Mocked happy path: assert btcSignMessage SDK call shape and the
resulting Entry.
- MalformedResponse on a wrong-length electrumSig65.
- Coin code selection on mainnet/testnet.
Adds ColdcardSignMessage to the Coldcard keystore driver. Coldcard
is an indirect keystore, so the interaction is file-based rather
than transport-based.
ColdcardSignMessage extends ColdcardInteraction
(IndirectKeystoreInteraction):
- workflow = ["request", "parse"] — both indirect phases are required.
- request(): builds the 3-line .txt Coldcard expects on SD card:
{message}\n{bip32Path}\np2wpkh\n
- parse(file): consumes Coldcard's armored "Bitcoin Signed Message"
output, extracts the base64 signature from the line after the
address line under BEGIN SIGNATURE, returns Entry.
- messages(): PENDING messages walk the user through "Download +
save to SD card → Advanced > File Management > Sign Text File on
Coldcard → upload signed file".
- Validates the message via validateMessage at construction time.
Throws MessageSigningError{MalformedResponse} on empty/garbage
uploads.
BIP-322 mode is intentionally not supported on this class. Coldcard's
BIP-322 firmware path is the Proof-of-Reserve PSBT flow (Mk 5.5.0 /
Q 1.4.0Q+), which proves wallet-level UTXO control via the BIP-322
FULL form — a different use case from per-cosigner-key BIP-322 Simple.
A future ColdcardSignMessageBIP322 can wrap the PoR PSBT flow if
caravan needs that capability; this class implements BIP-137 only.
SignMessage factory:
- Wires the COLDCARD case to ColdcardSignMessage.
Coordinator:
- ColdcardSignMessage now appears in the messageSigning test
category via the standard indirect-keystore wiring in
apps/coordinator/src/tests/index.js.
Tests:
- Constructor throw on oversize message.
- request() returns the canonical 3-line .txt.
- parse() extracts the base64 sig from a canonical armored file.
- parse() throws MalformedResponse on empty input, missing
delimiters, and missing address+signature lines.
BCUR2 message signing is deferred — the UR convention is unresolved.
Documents the message-signing API surface introduced across the
preceding implementation commits.
packages/caravan-wallets/CLAUDE.md SignMessage section:
- Documents the canonical Entry shape and verifier behaviour.
- Lists per-keystore capabilities (legacy + bip322 flags),
noting Coldcard's BIP-322 PoR PSBT path as a future separate
class.
- Calls out the bitcoinjs-lib v5 vs v6 alignment under bip322-js.
- Calls out the Trezor `Network[network]` reverse-lookup bug fix
that landed alongside TrezorSignMessage.
.changeset/message-signing-keystore-support.md:
- @caravan/wallets bumped to major (0.10.1 → 1.0.0). The package is
broadly deployed and existing callers receive `.run()` return
shapes that have changed (Ledger {v,r,s}, Trezor
{address,signature}, Jade raw hex EC sig → now all return Entry).
The factory's expectedPubkey argument is also newly required.
- Drop unused NormalizeSignature export — the per-keystore
normalizers diverge in signature (Jade needs `message` too) and
no caller uses the type. Future shape can re-introduce if a real
interface emerges.
- Drop UnsupportedAddressType from MessageSigningErrorKind — never
thrown anywhere. Verifier returns false for unsupported pubkey
shapes; pre-device address-type checks aren't in scope.
- Differentiate MessageSigningError.message vs userMessage:
e.message now carries a structured `[Kind/KEYSTORE]` prefix for
logs; e.userMessage stays bare for UI surfaces.
- verifyMessageSignature now takes `{message, signature,
expectedPubkey}` directly instead of an Entry wrapper. Drops the
awkward fabricated `bip32Path: ""` in Jade's internal recovery-
byte search and gives non-Entry callers a clean entry point.
- Lift validateMessage into the SignMessage factory (single call
site before the switch) and drop it from each per-keystore
constructor. Centralizes the "throw before contacting any device"
contract and removes five copies of the same call.
- Comment MAX_MESSAGE_BYTES = 240 with the Coldcard SD-card
justification.
- Replace per-keystore "constructor throws on oversize" tests (five
near-duplicates) with one factory-level test that loops over all
supported keystores, asserting each surfaces MalformedRequest with
the right `keystore` label. Catches a future keystore added to
the factory without going through validateMessage.
Every error reachable from SignMessage.run() now surfaces as MessageSigningError so consumers can branch on `e.kind` without falling through to raw SDK exceptions. `wrapSdkError(keystore, err)` is the shared classifier. It passes existing MessageSigningError instances through untouched (so MalformedResponse / MalformedRequest from the keystore layer aren't clobbered), then uses a lenient heuristic to label the rest as DeviceRejected (Ledger statusCode 0x6985; "cancel" / "reject" / "denied" / "declined" / "abort" in the message) or TransportError (everything else). Over-tagging a transport drop as DeviceRejected is preferred over the reverse — the user-facing string is still sensible the wrong way for a transport drop, but confusing the wrong way for a cancellation. Call sites: - LedgerSignMessage.run() wraps the SDK call (both legacy and v2 app branches). - TrezorSignMessage.run() overrides the base class run() so that the raw `Error(result.payload.error)` thrown for `result.success === false` (cancellations, timeouts, device errors) is reclassified. - BitBoxSignMessage.run() wraps btcSignMessage; MalformedResponse for non-65-byte electrumSig65 still throws unchanged. - JadeSignMessage.run() wraps jade.signMessage; the anti-exfil tuple guard and recovery-byte failure still throw MalformedResponse. Coldcard is file-based — no transport surface to wrap. Tests cover the classifier (passthrough, statusCode 0x6985, cancellation keywords, transport-shaped, non-Error throws) plus a keystore-rejected and keystore-transport case for each of the four direct keystores.
parse() previously assumed "second non-empty non-delimiter line after BEGIN SIGNATURE" with no further validation. That works for Coldcard's canonical output but silently produces garbage if the format drifts (extra metadata line, wrapped address, comment). Two guards: 1. Trim each candidate line before filtering, so a sig with surrounding whitespace still resolves. 2. Validate the resulting signature matches a base64 shape covering the BIP-137 65-byte wire form (80-120 base64 chars + optional `=` padding). A line that isn't base64 now throws MalformedResponse with a preview of the bad input instead of being handed downstream where verifyMessageSignature would silently return false. Tests cover: non-base64 signature slot, too-short signature, surrounding whitespace, and blank lines between marker and address.
- Ledger: add a v2-Bitcoin-app error-wrapping test mirroring the
legacy-app coverage. Previously the v2 happy-path test was the
only v2 case and it just asserted "mock returned what we told it
to"; with this addition the v2 branch has at least one assertion
that exercises real interaction-layer logic (the wrap in run()).
- BitBox: add a regtest test exercising the third branch of
convertNetwork ('rbtc'). Mainnet ('btc') and testnet ('tbtc')
were already covered; regtest is the path that gets hit in the
docker e2e setup.
- Coordinator messageSigning fixture wiring: guard relativePath
derivation with a startsWith check. If a future
TEST_FIXTURES.multisigs entry has a bip32Path that doesn't sit
under its branchPath, throw at suite-construction time with both
paths in the message instead of silently deriving the wrong
pubkey and showing a verification failure on-device.
Rewrite to match what actually lands:
- verifyMessageSignature takes {message, signature, expectedPubkey}
directly (no Entry wrapper).
- MessageSigningErrorKind is 4 members, not 6 — UnsupportedAddressType
and UnsupportedProtocol dropped along with the runtime legacy flag.
- New wrapSdkError export covers DeviceRejected/TransportError
classification each per-keystore run() goes through.
- e.message is structured ([Kind/KEYSTORE] prefix); e.userMessage
stays bare for UI surfaces.
The previous version was a design doc; the changelog only needs the consumer-facing summary. Detail lives in the PR description.
The verifier, Entry type, and MessageSigningError taxonomy aren't keystore-specific — they're the recovery side of any per-cosigner message-signing flow. Pulling them into their own package now (at @caravan/wallets's 1.0 inflection) so consumers can depend on the verification surface without pulling the keystore drivers, and so the future Sign-Message UI has a clean import target. @caravan/wallets keeps the SignMessage factory, per-keystore classes, and wrapSdkError (keystore SDK error classification stays with the drivers). Signature fixtures live in packages/messages/test-fixtures/signatures.json, regenerated via `npm run generate-fixtures --workspace=@caravan/messages` when TEST_FIXTURES changes.
private:true (won't be published) — matches the pattern in @caravan/build-plugins. Moved to devDependencies in @caravan/wallets so tsup inlines it into the published wallets bundle. Drops the changeset for @caravan/messages since private packages don't release.
The "expected" framing leaks the verifier's perspective into the signing primitive. The signer side just has a pubkey; whether it's "expected" is the caller's framing at verify time. Renamed across @caravan/messages (Entry, verifyMessageSignature args, fixtures) and @caravan/wallets (SignMessage factory, per-keystore fields). Coordinator UI label "Expected pubkey:" stays — that's the right framing for the user reading the test row.
- Strip internal-process language from production docstrings: no
more sparrow / trezor-firmware issue links, SeedSigner PR
pointers, or paragraph-length explanations of what protocols the
class doesn't implement. Each per-keystore SignMessage class now
says what it does in 2 lines.
- Replace try/catch error-assertion blocks with
`rejects.toMatchObject({kind, keystore})` across ledger / trezor /
jade / errors test files. Collapses each ~10-line block to 3-4
lines.
- Extract mountLedgerApp helper (was 6 lines of spy plumbing
repeated 5×) and a similar mountBitBoxSDK helper. Collapse the
two BitBox coin-code tests + the mainnet happy-path into one
parameterized it.each across all three networks.
Pre-computed BIP-137 and BIP-322 signatures now live as a `signedMessages` field on each `TEST_FIXTURES.multisigs` entry, signed at the entry's bip32Path with the open_source seed. Signatures are deterministic so the values can be static literals — no generator script needed. Drop the runtime generator from @caravan/messages along with the @scure/bip32, @scure/bip39, bitcoinjs-message, wif, and tsx devDeps it was the only consumer of. Tests load TEST_FIXTURES directly. Also strip a leftover comment from the implementation notes.
Contributor
Author
|
Running manual tests, Ledger: |
Contributor
Author
|
Jade passed (after some fixes): |
Contributor
Author
|
Trezor (mostly) passed. Note that P2SH signing fails, but it also fails on normal transaction signing due to a forbidden key path error. |
BitBox firmware refuses sign_message at caravan's multisig-purpose cosigner paths (BIP-45/48) — validates against BIP-49/84 only.
Add download-request-file button + .txt upload reader for the messageSigning test category. Previously the test renderer fell through to the xpub-upload branch (workflow=["parse"] only) and showed the wrong prompt.
Test.runParse() unpacks Coldcard parse() output via Object.values()[0],
which assumes the legacy {pubkey: [sig]} shape and grabs bip32Path off
the SignMessageResult. Override runParse() in MessageSigningTest so
matches() receives the entry intact.
Contributor
Author
|
And confirmed with coldcard (also needed a fix), this was MK3 only: 
|
bucko13
marked this pull request as ready for review
May 21, 2026 21:36
Covers the messageSigning test category and Coldcard SD-card UX added on this branch. Also unlocks the coordinator docker/e2e CI jobs gated on caravan-coordinator appearing in a changeset.
5 tasks
Contributor
|
This pull request has been inactive for 30 days and has been marked as stale. It will be closed in 7 days if no further activity occurs. To keep this PR open, add the "long-lived" label or comment on it. |
Contributor
|
This pull request has been automatically closed due to inactivity for 7 days after being marked as stale. Feel free to reopen if needed! |
Contributor
|
This pull request has been inactive for 30 days and has been marked as stale. It will be closed in 7 days if no further activity occurs. To keep this PR open, add the "long-lived" label or comment on it. |
Contributor
|
This pull request has been automatically closed due to inactivity for 7 days after being marked as stale. Feel free to reopen if needed! |
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.
Adds
SignMessage({keystore, network?, bip32Path, message, expectedPubkey})across Ledger v1+v2, Trezor, Jade, BitBox, and Coldcard. All keystores return a normalizedEntry = {bip32Path, signature, expectedPubkey}and a singleverifyMessageSignature(loose-mode bip322-js) handles the recovery side.This only implements BIP137 support as most devices still don't have any BIP322 support. I might put up a follow up that covers BIP322 for coldcard which recently added support.
Also wires the new flow into
/#/testas a Message Signing category overTEST_FIXTURES.multisigs.Note:
@caravan/walletsbumps to 1.0.0 due to the backwards incompatible changes to the existing interactions normalizing the return string.What's left before merging
Need to validate in the test suite:
- [x] Bitboxnot actually supportedNot included: