diff --git a/docs/limitations.md b/docs/limitations.md index e61e598f..5383314d 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -200,14 +200,14 @@ We will summarize transaction outputs as "change" back into same wallet, however # CCC Feature (ColdCard Cosigning) +- only 12 or 24 word seeds (not XPRV) are accepted for "key C" - velocy limit: - based on a max magnitude per txn, and a required minimum block height - change, based on previous `nLockTime` value in last PSBT/signed transaction. + gap, based on previous `nLockTime` value in last-signed PSBT. - if you sign a transaction, but never broadcast it, you will still have to wait out the velocity policy. - - PSBT creator must put in accurate lock times (most already do to avoid fee sniping) -- maximum of 25 whitelisted addresses can be enabled + - PSBT creator must put in `nLockTime` block heights (most already do to avoid fee sniping) +- maximum of 25 whitelisted addresses can be stored - Web2FA: any number of mobile devices can be enrolled, but all will have the same shared secret -- any warning from the PSBT, such as huge fees, will disable CCC cosign. -- only 12 or 24 word seeds (not XPRV) are accepted for "key C" +- any warning from the PSBT, such as huge fees, will prevent CCC cosign. diff --git a/shared/ccc.py b/shared/ccc.py index cc94bdb4..8cf07dc1 100644 --- a/shared/ccc.py +++ b/shared/ccc.py @@ -15,7 +15,7 @@ from exceptions import CCCPolicyViolationError # nLockTime in transaction above this value is a unix timestamp (time_t) not block height. NLOCK_IS_TIME = const(500000000) -# limit to number of addresses in list TODO +# limit to number of addresses in list MAX_WHITELIST = const(25) # TODO: if A has already signed the PSBT, and we don't need key C, don't try; maybe show warning @@ -395,6 +395,7 @@ class CCCAddrWhitelist(MenuSystem): def construct(self): # list of addresses addrs = CCCFeature.get_policy().get('addrs', []) + maxxed = (len(addrs) >= MAX_WHITELIST) items = [MenuItem(truncate_address(a), f=self.edit_addr, arg=a) for a in addrs] @@ -402,9 +403,11 @@ class CCCAddrWhitelist(MenuSystem): items.append(MenuItem("(none yet)")) if version.has_qr: - items.append(MenuItem('Scan QR', f=self.scan_qr, shortcut=KEY_QR)) + items.append(MenuItem('Scan QR', f=(self.maxed_out if maxxed else self.scan_qr), + shortcut=KEY_QR)) - items.append(MenuItem('Import from File', f=self.import_file)) + items.append(MenuItem('Import from File', + f=(self.maxed_out if maxxed else self.import_file))) return items @@ -454,7 +457,7 @@ class CCCAddrWhitelist(MenuSystem): # pick a likely-looking file: just looking at size and extension fn = await file_picker(suffix=['csv', 'txt'], - min_size=20, max_size=5000, + min_size=20, max_size=20000, none_msg="Must contain payment addresses", **choice) if not fn: return @@ -464,7 +467,6 @@ class CCCAddrWhitelist(MenuSystem): with open(fn, 'rt') as fd: for ln in fd.readlines(): for here in pat.split(ln): - print(here) if len(here) >= 4: try: addr = cleanup_payment_address(here) @@ -474,7 +476,8 @@ class CCCAddrWhitelist(MenuSystem): if not results: await ux_show_story("Unable to find any payment addresses in that file.") else: - await self.add_addresses(results) + # silently limit to first 25 results; lets them use addresses.csv easily + await self.add_addresses(results[:MAX_WHITELIST]) async def scan_qr(self, *a): @@ -495,6 +498,9 @@ class CCCAddrWhitelist(MenuSystem): # import them await self.add_addresses(got) + async def maxed_out(self, *a): + await ux_show_story("Max %d items in whitelist. Please make room first." % MAX_WHITELIST) + async def add_addresses(self, more_addrs): # add new entries, if unique; preserve ordering addrs = CCCFeature.get_policy().get('addrs', []) @@ -508,6 +514,9 @@ class CCCAddrWhitelist(MenuSystem): await ux_show_story("Already in whitelist:\n\n" + '\n\n'.join(more_addrs)) return + if len(addrs) > MAX_WHITELIST: + return await self.maxed_out() + CCCFeature.update_policy_key(addrs=addrs) self.update_contents() @@ -568,8 +577,9 @@ class CCCPolicyMenu(MenuSystem): await web2fa.web2fa_enroll('CCC', ss) async def set_magnitude(self, *a): + # Looks decent on both Q and Mk4... was = self.policy.get('mag', 0) - val = await ux_enter_number('Per Txn Max Out', max_value=int(1e8), + val = await ux_enter_number('Transaction Max:', max_value=int(1e8), can_cancel=True, value=(was or '')) args = dict(mag=val) @@ -577,7 +587,7 @@ class CCCPolicyMenu(MenuSystem): msg = "Did not change" val = was else: - msg = "You can have set the" + msg = "You have set the" unchanged = False if not val: @@ -729,10 +739,16 @@ async def toggle_ccc_feature(*a): # - collect a policy setup, maybe 2FA enrol too # - lock that down ch = await ux_show_story('''\ -This feature creates a new 2-of-3 multisig wallet. A, B, and C keys are as follows:\n -A=This Coldcard, B=Backup Key, C=Policy Key ... blah balh +Adds an additional seed to your Coldcard, and enforces a "spending policy" whenever \ +it signs with that key. Spending policies can restrict: magnitude (BTC out), \ +velocity (blocks between txn), address whitelisting, and/or require confirmation by 2FA phone app. + +Assuming the use of a 2-of-3 multisig wallet, keys are as follows:\n +A=Coldcard (master seed), B=Backup Key (offline/recovery), C=Spending Policy Key. + +Spending policy cannot be viewed or changed without knowledge of key C.\ ''', - title="Coldcard Co-Signing") + title="Coldcard Co-Signing" if version.has_qwerty else 'CC Cosigning') if ch != 'y': # just a tourist @@ -748,12 +764,10 @@ async def enable_step1(words): # do BIP-32 basics: capture XFP and XPUB and encoded version of the secret CCCFeature.init_setup(words) - # push them directly into policy submenu first time. + # continue into config menu m = CCCConfigMenu() the_ux.push(m) - # that will lead back to a "nested" menu other setup - async def modify_ccc_settings(): # Generally not expecting changes to policy on the fly because # that's the whole point. Use the B key to override individual spends @@ -783,15 +797,15 @@ setup and debug is finished, or all benefit of this feature is lost!''') # debug hack: skip word entry bypass = True + elif ch != 'y': return + if bypass: - # - doing full decode cycle here for better testing + # doing full decode cycle here for better testing chk, raw, _ = SecretStash.decode(enc) assert chk == 'words' words = bip39.b2a_words(raw).split(' ') await key_c_challenge(words) return - - if ch != 'y': return # small info-leak here: exposing 12 vs 24 words, but we expect most to be 12 anyway nwords = CCCFeature.get_num_words() diff --git a/shared/ux_q1.py b/shared/ux_q1.py index 2feb6a73..db4fca57 100644 --- a/shared/ux_q1.py +++ b/shared/ux_q1.py @@ -125,6 +125,7 @@ async def ux_enter_number(prompt, max_value, can_cancel=False, value=''): elif ch == KEY_DELETE: if value: value = value[0:-1] + dis.text(0, 4, ' '*CHARS_W) elif ch == KEY_CLEAR: value = '' dis.text(0, 4, ' '*CHARS_W) diff --git a/testing/test_ccc.py b/testing/test_ccc.py index 299f3616..93fea38f 100644 --- a/testing/test_ccc.py +++ b/testing/test_ccc.py @@ -670,6 +670,9 @@ def test_ccc_velocity(velocity_mi, setup_ccc, enter_enabled_ccc, ccc_ms_setup, # - check txn re-sign fails (if velocity in effect) # - check any warning is blocked # - check too-big fee is blocked +# - "export cc xpubs" path +# - 'build 2-of-N' path +# - maxed out values: 24 words, 25 whitelisted p2wsh values # EOF