coldcard/firmware
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bugfix: do not allow to import duplicate miniscript wallets

Browse Source
This commit is contained in:
scgbckbone 2023-12-04 12:29:39 +01:00 committed by doc-hex
parent 0c68d1a4f4
commit 2388efa8d7
3 changed files with 150 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
releases/EdgeChangeLog.md
View File

@ -7,6 +7,10 @@
- for experimental use. DO NOT use for large Bitcoin amounts.
```
## 6.2.2X - 2023-12-XX
- Bugfix: Do not allow to import duplicate miniscript wallets
## 6.2.1X - 2023-10-26
- New Feature: Enroll Miniscript wallet via USB (requires ckcc `v1.4.0`)

42
shared/miniscript.py
View File

@ -47,13 +47,17 @@ class MiniScriptWallet(BaseWallet):
@property
def keys(self):
if not self._keys:
self._keys = [k.to_string() for k in self.desc.keys]
self._keys = self.desc.keys
if self._keys is not None:
self._keys = [k.to_string() for k in self._keys]
return self._keys
@property
def key(self):
if not self._key:
self._key = self.desc.key.to_string()
self._key = self.desc.key
if self._key is not None:
self._key = self._key.to_string()
return self._key
@property
@ -220,7 +224,7 @@ class MiniScriptWallet(BaseWallet):
return "Taproot tree keys:\n\n" + self.policy
return self.policy
async def _detail(self, new_wallet=False):
async def _detail(self, new_wallet=False, is_duplicate=False):
s = addr_fmt_label(self.af) + "\n\n"
if self.taproot:
@ -229,17 +233,20 @@ class MiniScriptWallet(BaseWallet):
s += self.ux_policy()
story = s + "\n\nPress (1) to see extended public keys"
if new_wallet:
if new_wallet and not is_duplicate:
story += ", OK to approve, X to cancel."
return story
async def show_detail(self, new_wallet=False):
async def show_detail(self, new_wallet=False, duplicates=None):
title = self.name
story = ""
if new_wallet:
if duplicates:
title = None
story += "This wallet is a duplicate of already saved wallet %s\n\n" % duplicates[0].name
elif new_wallet:
title = None
story += "Create new miniscript wallet?\n\nWallet Name:\n %s\n\n" % self.name
story += await self._detail(new_wallet)
story += await self._detail(new_wallet, is_duplicate=duplicates)
while True:
ch = await ux_show_story(story, title=title, escape="1")
if ch == "1":
@ -288,10 +295,27 @@ class MiniScriptWallet(BaseWallet):
wal = cls(desc_obj, name=name, chain_type=desc_obj.keys[0].chain_type)
return wal
def find_duplicates(self):
matches = []
for rv in self.iter_wallets():
if self.key != rv.key:
continue
if self.policy != rv.policy:
continue
if len(self.keys) != len(rv.keys):
continue
if self.keys != rv.keys:
continue
matches.append(rv)
return matches
async def confirm_import(self):
to_save = await self.show_detail(new_wallet=True)
dups = self.find_duplicates()
to_save = await self.show_detail(new_wallet=True, duplicates=dups)
ch = "y" if to_save else "x"
if to_save:
if to_save and not dups:
assert self.storage_idx == -1
self.commit()
await ux_dramatic_pause("Saved.", 2)

124
testing/test_miniscript.py
View File

@ -22,6 +22,23 @@ TREE = {
9: '{{{%s{%s,%s}},{%s,%s}},{{%s,%s},{%s,%s}}}'
}
@pytest.fixture
def offer_minsc_import(cap_story, dev, need_keypress):
def doit(config):
# upload the file, trigger import
file_len, sha = dev.upload_file(config.encode())
open('debug/last-config-msc.txt', 'wt').write(config)
dev.send_recv(CCProtocolPacker.miniscript_enroll(file_len, sha))
time.sleep(.2)
title, story = cap_story()
return title, story
return doit
@pytest.fixture
def import_miniscript(goto_home, pick_menu_item, cap_story, need_keypress):
def doit(fname, way="sd"):
@ -47,6 +64,16 @@ def import_miniscript(goto_home, pick_menu_item, cap_story, need_keypress):
return doit
@pytest.fixture
def import_duplicate(import_miniscript, need_keypress):
def doit(fname, way="sd"):
_, story = import_miniscript(fname, way)
assert "duplicate of already saved wallet" in story
assert "OK to approve" not in story
need_keypress("x")
return doit
@pytest.fixture
def miniscript_descriptors(goto_home, pick_menu_item, need_keypress, cap_story,
microsd_path):
@ -80,7 +107,7 @@ def get_cc_key(dev):
# cc device key
master_xfp_str = struct.pack('<I', dev.master_fingerprint).hex()
cc_key = dev.send_recv(CCProtocolPacker.get_xpub(path), timeout=None)
return f"[{master_xfp_str}/{path}]{cc_key}{'<0;1>/*' if int_ext else '/0/*'}"
return f"[{master_xfp_str}/{path}]{cc_key}{'/<0;1>/*' if int_ext else '/0/*'}"
return doit
@ -207,7 +234,7 @@ def test_liana_miniscripts_simple(addr_fmt, recovery, lt_type, minisc, clear_min
need_keypress, pick_menu_item, cap_menu, cap_story, microsd_path,
use_regtest, bitcoind, microsd_wipe, load_export, dev,
address_explorer_check, get_cc_key, import_miniscript,
bitcoin_core_signer):
bitcoin_core_signer, import_duplicate):
normal_cosign_core = False
recovery_cosign_core = False
if "multi(" in minisc.split("),", 1)[0]:
@ -265,6 +292,7 @@ def test_liana_miniscripts_simple(addr_fmt, recovery, lt_type, minisc, clear_min
assert "Create new miniscript wallet?" in story
# do some checks on policy --> helper function to replace keys with letters
need_keypress("y")
import_duplicate(fname)
menu = cap_menu()
assert menu[0] == name
pick_menu_item(menu[0]) # pick imported descriptor multisig wallet
@ -358,7 +386,8 @@ def test_liana_miniscripts_simple(addr_fmt, recovery, lt_type, minisc, clear_min
def test_liana_miniscripts_complex(addr_fmt, minsc, bitcoind, use_regtest, clear_miniscript,
microsd_path, pick_menu_item, need_keypress, cap_story,
load_export, goto_home, address_explorer_check, cap_menu,
get_cc_key, import_miniscript, bitcoin_core_signer):
get_cc_key, import_miniscript, bitcoin_core_signer,
import_duplicate):
use_regtest()
clear_miniscript()
@ -413,6 +442,7 @@ def test_liana_miniscripts_complex(addr_fmt, minsc, bitcoind, use_regtest, clear
assert "Create new miniscript wallet?" in story
# do some checks on policy --> helper function to replace keys with letters
need_keypress("y")
import_duplicate(fname)
menu = cap_menu()
assert menu[0] == name
pick_menu_item(menu[0]) # pick imported descriptor multisig wallet
@ -500,9 +530,9 @@ def test_liana_miniscripts_complex(addr_fmt, minsc, bitcoind, use_regtest, clear
@pytest.fixture
def bitcoind_miniscript(bitcoind, bitcoind_d_sim_watch, need_keypress, cap_story, load_export,
pick_menu_item, goto_home, cap_menu, microsd_path, use_regtest, get_cc_key,
import_miniscript, bitcoin_core_signer):
def bitcoind_miniscript(bitcoind, need_keypress, cap_story, load_export, get_cc_key,
pick_menu_item, goto_home, cap_menu, microsd_path, use_regtest,
import_miniscript, bitcoin_core_signer, import_duplicate):
def doit(M, N, script_type, internal_key=None, cc_account=0, funded=True, r=None,
tapscript_threshold=False, add_own_pk=False):
@ -604,6 +634,13 @@ def bitcoind_miniscript(bitcoind, bitcoind_d_sim_watch, need_keypress, cap_story
assert "P2SH-P2WSH" in story
# assert "Derivation:\n Varies (2)" in story
need_keypress("y") # approve multisig import
if r:
# unspendable key is generated randomly
# descriptors will differ
with pytest.raises(AssertionError):
import_duplicate(name)
else:
import_duplicate(name)
goto_home()
pick_menu_item('Settings')
pick_menu_item('Miniscript')
@ -811,7 +848,7 @@ def test_tapscript_multisig(cc_first, m_n, internal_key_spendable, use_regtest,
def test_tapscript_pk(num_leafs, use_regtest, clear_miniscript, microsd_wipe, bitcoind,
internal_key_spendable, dev, microsd_path, need_keypress, get_cc_key,
pick_menu_item, cap_story, goto_home, cap_menu, load_export,
import_miniscript, bitcoin_core_signer):
import_miniscript, bitcoin_core_signer, import_duplicate):
use_regtest()
clear_miniscript()
microsd_wipe()
@ -853,6 +890,7 @@ def test_tapscript_pk(num_leafs, use_regtest, clear_miniscript, microsd_wipe, bi
assert "P2TR" in story
need_keypress("y")
import_duplicate(fname)
goto_home()
pick_menu_item('Settings')
pick_menu_item('Miniscript')
@ -946,7 +984,7 @@ def test_tapscript_import_export(clear_miniscript, pick_menu_item, cap_story, ne
def test_duplicate_tapscript_leaves(use_regtest, clear_miniscript, microsd_wipe, bitcoind, dev,
goto_home, pick_menu_item, need_keypress, microsd_path,
cap_story, load_export, get_cc_key, import_miniscript,
bitcoin_core_signer):
bitcoin_core_signer, import_duplicate):
# works in core - but some discussions are ongoing
# https://github.com/bitcoin/bitcoin/issues/27104
# CC also allows this for now... (experimental branch)
@ -974,6 +1012,7 @@ def test_duplicate_tapscript_leaves(use_regtest, clear_miniscript, microsd_wipe,
assert "P2TR" in story
need_keypress("y")
import_duplicate(fname)
goto_home()
pick_menu_item('Settings')
pick_menu_item('Miniscript')
@ -1043,7 +1082,7 @@ def test_duplicate_tapscript_leaves(use_regtest, clear_miniscript, microsd_wipe,
def test_same_key_account_based_minisc(goto_home, need_keypress, pick_menu_item, cap_story,
clear_miniscript, microsd_path, load_export, bitcoind,
import_miniscript):
import_miniscript, import_duplicate):
clear_miniscript()
desc = ("wsh("
"or_d(pk([0f056943/84'/1'/0']tpubDC7jGaaSE66Pn4dgtbAAstde4bCyhSUs4r3P8WhMVvPByvcRrzrwqSvpF9Ghx83Z1LfVugGRrSBko5UEKELCz9HoMv5qKmGq3fqnnbS5E9r/<0;1>/*),"
@ -1061,6 +1100,7 @@ def test_same_key_account_based_minisc(goto_home, need_keypress, pick_menu_item,
assert "Press (1) to see extended public keys" in story
need_keypress("y")
import_duplicate(fname)
goto_home()
pick_menu_item('Settings')
pick_menu_item('Miniscript')
@ -1203,7 +1243,7 @@ def test_minitapscript(leaf2_mine, recovery, lt_type, minisc, clear_miniscript,
need_keypress, pick_menu_item, cap_menu, cap_story, microsd_path,
use_regtest, bitcoind, microsd_wipe, load_export, dev,
address_explorer_check, get_cc_key, import_miniscript,
bitcoin_core_signer):
bitcoin_core_signer, import_duplicate):
# needs this bitcoind branch https://github.com/bitcoin/bitcoin/pull/27255
normal_cosign_core = False
@ -1268,6 +1308,7 @@ def test_minitapscript(leaf2_mine, recovery, lt_type, minisc, clear_miniscript,
assert "Create new miniscript wallet?" in story
# do some checks on policy --> helper function to replace keys with letters
need_keypress("y")
import_duplicate(fname)
menu = cap_menu()
assert menu[0] == name
pick_menu_item(menu[0]) # pick imported descriptor multisig wallet
@ -1589,4 +1630,65 @@ def test_chain_switching(use_mainnet, use_regtest, settings_get, settings_set,
assert fname_xtn.split(".")[0] in m[0]
assert fname_xtn0.split(".")[0] in m[1]
for mi in m:
assert fname_btc not in mi
assert fname_btc not in mi
@pytest.mark.parametrize("taproot_ikspendable", [
(True, False), (True, True), (False, False)
])
@pytest.mark.parametrize("minisc", [
"or_d(pk(@A),and_v(v:pkh(@B),after(100)))",
"or_d(multi(2,@A,@C),and_v(v:pkh(@B),after(100)))",
])
def test_import_same_policy_same_keys_diff_order(taproot_ikspendable, minisc,
clear_miniscript, use_regtest,
get_cc_key, bitcoin_core_signer,
offer_minsc_import, need_keypress,
cap_menu, bitcoind, pick_menu_item):
use_regtest()
clear_miniscript()
taproot, ik_spendable = taproot_ikspendable
if taproot:
minisc = minisc.replace("multi(", "multi_a(")
if ik_spendable:
ik = get_cc_key("84h/1h/100h")
desc = f"tr({ik},{minisc})"
else:
desc = f"tr({H},{minisc})"
else:
desc = f"wsh({minisc})"
cc_key0 = get_cc_key("84h/1h/0h")
signer0, core_key0 = bitcoin_core_signer("s00")
# recevoery path is always B
desc0 = desc.replace("@A", cc_key0)
desc0 = desc0.replace("@B", core_key0)
if "@C" in desc:
signer1, core_key1 = bitcoin_core_signer("s11")
desc0 = desc0.replace("@C", core_key1)
# now just change order of the keys (A,B), but same keys same policy
desc1 = desc.replace("@B", cc_key0)
desc1 = desc1.replace("@A", core_key0)
if "@C" in desc:
desc1 = desc1.replace("@C", core_key1)
# checksum required if via USB
desc_info = bitcoind.supply_wallet.getdescriptorinfo(desc0)
desc0 = desc_info["descriptor"] # with checksum
desc_info = bitcoind.supply_wallet.getdescriptorinfo(desc1)
desc1 = desc_info["descriptor"] # with checksum
title, story = offer_minsc_import(desc0)
assert "Create new miniscript wallet?" in story
need_keypress("y")
time.sleep(.2)
title, story = offer_minsc_import(desc1)
assert "Create new miniscript wallet?" in story
need_keypress("y")
pick_menu_item("Settings")
pick_menu_item("Miniscript")
m = cap_menu()
assert len(m) == 3