fix: multisig PSBT signing with Keycard/Satochip cards - #444
Open
3rdIteration wants to merge 1 commit into
Open
Conversation
Two bugs affected multisig PSBT signing with a card signer (Keycard and Satochip both route through the smartcard multisig path): - The multisig card flow parsed the PSBT without passing the network, so the parser defaulted to mainnet. On testnet every address rendered with the mainnet HRP (tb1q... shown as bc1q...). Pass the configured network through when pre-parsing in PSBTSelectSeedView. - sign_psbt_with_keycard skipped inputs with more than one derivation in bip32_derivations, which is every multisig input (one entry per cosigner). The card therefore never produced a signature and Approve returned to the same screen. Now multi-derivation inputs resolve which derivation belongs to the card by exporting each candidate path's public key and matching it, then sign only the match.
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.
Summary
Fixes two bugs that prevented multisig PSBT signing with a smartcard signer on non-mainnet networks. Both affected Keycard and Satochip (they share the smartcard multisig path in
PSBTSelectSeedView).Bug 1: wrong address HRP on testnet (bc1q... instead of tb1q...)
The multisig card flow in
PSBTSelectSeedViewcreated thePSBTParserwithout passingnetwork, so it defaulted to mainnet. On testnet every address rendered with the mainnet HRP (e.g. a tb1q... recipient showed as bc1q...) and did not match what the coordinator (Sparrow) showed.Fix: pass the configured network when pre-parsing the PSBT in the multisig card path.
Bug 2: "Approve transaction" did nothing on multisig
sign_psbt_with_keycardskipped any input with more than onebip32_derivationsentry — which is every multisig input (one derivation per cosigner). The card therefore never produced a signature,PSBTFinalizeViewdetected no new signatures, and the screen just flipped back to itself.Fix: for multi-derivation inputs, export each candidate path's public key from the card and match it against the derivation's public key, then sign only the matching derivation (same pubkey-matching approach the Satochip signer already uses). Single-derivation inputs keep the existing path-sign fallback.
Tests
tests/test_keycard_sign_psbt.py: multisig input signs the card's own cosigner pubkey (not a cosigner's); signs nothing when no pubkey matches.tests/test_flows_psbt.py: the smartcard multisig flow parses with the configured network and renderstb1...addresses on testnet.Regression check:
test_psbt_parser.py,test_psbt_test_suite.py,test_flows_psbt.py, and the card-signer suites all pass.