diff --git a/releases/Next-ChangeLog.md b/releases/Next-ChangeLog.md index 9c5afe3a..0952721f 100644 --- a/releases/Next-ChangeLog.md +++ b/releases/Next-ChangeLog.md @@ -41,6 +41,7 @@ This lists the new changes that have not yet been published in a normal release. - Bugfix: Handle malformed NDEF records robustly. Thanks, @Damir - Bugfix: Ignore `bkpw` if added to backup. Thanks [@dmonakhov](https://github.com/dmonakhov) - Bugfix: Keep NFC export tag live for repeated probes +- Bugfix: Fix 1of1 multisig signing failure # Mk Specific Changes diff --git a/shared/psbt.py b/shared/psbt.py index 7e66d575..599637f8 100644 --- a/shared/psbt.py +++ b/shared/psbt.py @@ -829,17 +829,20 @@ class psbtInputProxy(psbtProxy): self.scriptSig = redeem_script - # new cheat: psbt creator probably telling us exactly what key - # to use, by providing exactly one. This is ideal for p2sh wrapped p2pkh - if len(subpaths) == 1: + if not addr_is_segwit and len(redeem_script) == 22 and \ + redeem_script[0] == 0 and redeem_script[1] == 20: + # segwit p2pkh wrapped in p2sh: exactly one key, not multisig. + # psbt creator tells us the key by providing exactly one subpath. + self.addr_fmt = AF_P2WPKH_P2SH + addr = redeem_script[2:22] + self.is_segwit = True + + assert len(subpaths) == 1, "p2sh-p2wpkh needs one key" which_key, = subpaths.keys() else: - # Assume we'll be signing with any key we know - # - limitation: we cannot be two legs of a multisig (only if CCC feature used) - # - but if partial sig already in place, ignore that one - if not which_key: - which_key = set() + self.is_multisig = True + which_key = set() for pubkey, path in subpaths.items(): if self.part_sigs and (pubkey in self.part_sigs): # pubkey has already signed, so ignore @@ -850,20 +853,8 @@ class psbtInputProxy(psbtProxy): which_key.add(pubkey) elif pubkey in psbt.wif_store: - # maybe sset some input value which_key.add(pubkey) - if not addr_is_segwit and \ - len(redeem_script) == 22 and \ - redeem_script[0] == 0 and redeem_script[1] == 20: - # it's actually segwit p2pkh inside p2sh - self.addr_fmt = AF_P2WPKH_P2SH - addr = redeem_script[2:22] - self.is_segwit = True - else: - # multiple keys involved - self.is_multisig = True - if self.witness_script and not self.is_segwit and self.is_multisig: # bugfix self.addr_fmt = AF_P2WSH_P2SH diff --git a/testing/test_multisig.py b/testing/test_multisig.py index 98a183e3..14e02a39 100644 --- a/testing/test_multisig.py +++ b/testing/test_multisig.py @@ -1510,6 +1510,24 @@ def test_ms_sign_simple(M_N, num_ins, dev, addr_fmt, clear_ms, incl_xpubs, impor else: try_sign(psbt) + +@pytest.mark.parametrize("finalize", [True, False]) +def test_1of1_multisig_sign(finalize, clear_ms, import_ms_wallet, fake_ms_txn, start_sign, + end_sign, cap_story): + # Minimal 1-of-1 multisig: import the wallet, then sign a 1-in/1-out PSBT. + clear_ms() + M = N = 1 + keys = import_ms_wallet(M, N, accept=True) + + psbt = fake_ms_txn(1, 1, M, keys) + + start_sign(psbt, finalize=finalize) + title, story = cap_story() + assert title == "OK TO SEND?" + + end_sign(accept=True, finalize=finalize) + + @pytest.mark.unfinalized @pytest.mark.bitcoind @pytest.mark.parametrize('num_ins', [ 15 ]) diff --git a/testing/test_sign.py b/testing/test_sign.py index 1c93284b..1c0f3873 100644 --- a/testing/test_sign.py +++ b/testing/test_sign.py @@ -1664,6 +1664,21 @@ def test_wrong_pubkey(dev, try_sign, fake_txn): msg = ee.value.args[0] assert ('pubkey vs. address wrong' in msg) + +def test_p2sh_p2wpkh_multiple_bip32_paths_rejected(fake_txn, start_sign, cap_story): + psbt = fake_txn(1, 1, segwit_in=True, wrapped=True) + po = BasicPSBT().parse(psbt) + + other = BIP32Node.from_master_secret(os.urandom(32)) + other_key = other.subkey_for_path("0/0") + po.inputs[0].bip32_paths[other_key.sec()] = other.fingerprint() + struct.pack('