Skip to content

Commit 083f835

Browse files
committed
bitcoin: display account name when sending to self
Before, we did this only when sending to the same account. This commit extends this to work when sending to an address of a *different* account of the same keystore. The bitcoin transaction script_configs field is used for inputs and change outputs. We add a similar list, `output_script_configs`, which allows to attach script configs of different accounts to outputs, with `output_script_config_index` to reference them in the output. To ensure backwards compatibility, `output_script_config_index` is added as a new field, which takes precedence over `script_config_index` when set. The output script configs are validated, but since one can send to any address of the same keystore, the output script config validation allows any combination of script configs, unlike the input script configs. To not duplicate lots of code, a few refactors are done: - validate_script_configs is now used for generic validation and is used to validate the output script configs as well as the input script configs - validate_input_script_configs performs additional validation as before (can't mix multisig and single sig inputs, can't mix inputs from different accounts, etc). - the multisig/policy name is stored in ValidatedScriptConfig, so we don't have to fetch it twice The Python lib version was downgraded from 8.0.0 to 7.0.0, it seems it was bumped before in error - the previous release tag and release is 6.3.0.
1 parent 821a48f commit 083f835

File tree

15 files changed

+581
-179
lines changed

15 files changed

+581
-179
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
1111
- Ethereum: remove deprecated Goerli network
1212
- SD card: solve backup bug when sd card is re-inserted
1313
- Cardano: allow serialization using 258-tagged sets
14+
- Bitcoin: identify outputs belonging to different accounts of the same keystore
1415

1516
### 9.21.0
1617
- Bitcoin: add support for sending to silent payment (BIP-352) addresses

messages/btc.proto

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ message BTCSignInitRequest {
115115
}
116116
FormatUnit format_unit = 8;
117117
bool contains_silent_payment_outputs = 9;
118+
// used script configs for outputs that send to an address of the same keystore, but not
119+
// necessarily the same account (as defined by `script_configs` above).
120+
repeated BTCScriptConfigWithKeypath output_script_configs = 10;
118121
}
119122

120123
message BTCSignNextResponse {
@@ -174,12 +177,19 @@ message BTCSignOutputRequest {
174177
uint64 value = 3;
175178
bytes payload = 4; // if ours is false. Renamed from `hash`.
176179
repeated uint32 keypath = 5; // if ours is true
177-
// If ours is true. References a script config from BTCSignInitRequest
180+
// If ours is true and `output_script_config_index` is absent. References a script config from
181+
// BTCSignInitRequest. This allows change output identification and allows us to identify
182+
// non-change outputs to the same account, so we can display this info to the user.
178183
uint32 script_config_index = 6;
179184
optional uint32 payment_request_index = 7;
180185
// If provided, `type` and `payload` is ignored. The generated output pkScript is returned in
181186
// BTCSignNextResponse. `contains_silent_payment_outputs` in the init request must be true.
182187
SilentPayment silent_payment = 8;
188+
// If ours is true. If set, `script_config_index` is ignored. References an output script config
189+
// from BTCSignInitRequest. This enables verification that an output belongs to the same keystore,
190+
// even if it is from a different account than we spend from, allowing us to display this info to
191+
// the user.
192+
optional uint32 output_script_config_index = 9;
183193
}
184194

185195
message BTCScriptConfigRegistration {

py/bitbox02/CHANGELOG.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,12 @@
22

33
## [Unreleased]
44

5-
# 8.0.0
6-
- SD card: Remove API to prompt removal of the microSD card from the device
7-
- Add support for regtest (supported from firmware version v9.21.0)
8-
95
# 7.0.0
106
- get_info: add optional device initialized boolean to returned tuple
117
- eth_sign: add address_case field, which should be initialized by the client
8+
- SD card: Remove API to prompt removal of the microSD card from the device
9+
- Add support for regtest (supported from firmware version v9.21.0)
10+
- btc_sign: allow identifying outputs belonging to different accounts of the same keystore
1211

1312
# 6.3.0
1413
- Allow infering product and version via API call instead of via USB descriptor

py/bitbox02/bitbox02/bitbox02/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
from __future__ import print_function
1717
import sys
1818

19-
__version__ = "8.0.0"
19+
__version__ = "7.0.0"
2020

2121
if sys.version_info.major != 3 or sys.version_info.minor < 6:
2222
print(

py/bitbox02/bitbox02/bitbox02/bitbox02.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,20 @@ class BTCInputType(TypedDict):
117117
class BTCOutputInternal:
118118
# TODO: Use NamedTuple, but not playing well with protobuf types.
119119

120-
def __init__(self, keypath: Sequence[int], value: int, script_config_index: int):
120+
def __init__(
121+
self,
122+
keypath: Sequence[int],
123+
value: int,
124+
script_config_index: int,
125+
output_script_config_index: Optional[int] = None,
126+
):
121127
"""
122128
keypath: keypath to the change output.
123129
"""
124130
self.keypath = keypath
125131
self.value = value
126132
self.script_config_index = script_config_index
133+
self.output_script_config_index = output_script_config_index
127134

128135

129136
class BTCOutputExternal:
@@ -390,13 +397,14 @@ def btc_sign(
390397
version: int = 1,
391398
locktime: int = 0,
392399
format_unit: "btc.BTCSignInitRequest.FormatUnit.V" = btc.BTCSignInitRequest.FormatUnit.DEFAULT,
400+
output_script_configs: Optional[Sequence[btc.BTCScriptConfigWithKeypath]] = None,
393401
) -> Sequence[Tuple[int, bytes]]:
394402
"""
395403
coin: the first element of all provided keypaths must match the coin:
396404
- BTC: 0 + HARDENED
397405
- Testnets: 1 + HARDENED
398406
- LTC: 2 + HARDENED
399-
script_configs: types of all inputs and change outputs. The first element of all provided
407+
script_configs: types of all inputs and outputs belonging to the same account (change or non-change). The first element of all provided
400408
keypaths must match this type:
401409
- SCRIPT_P2PKH: 44 + HARDENED
402410
- SCRIPT_P2WPKH_P2SH: 49 + HARDENED
@@ -407,6 +415,8 @@ def btc_sign(
407415
outputs: transaction outputs. Can be an external output
408416
(BTCOutputExternal) or an internal output for change (BTCOutputInternal).
409417
version, locktime: reserved for future use.
418+
format_unit: defines in which unit amounts will be displayed
419+
output_script_configs: script types for outputs belonging to the same keystore
410420
Returns: list of (input index, signature) tuples.
411421
Raises Bitbox02Exception with ERR_USER_ABORT on user abort.
412422
"""
@@ -418,6 +428,10 @@ def btc_sign(
418428
if any(map(is_taproot, script_configs)):
419429
self._require_atleast(semver.VersionInfo(9, 10, 0))
420430

431+
if output_script_configs:
432+
# Attaching output info supported since v9.22.0.
433+
self._require_atleast(semver.VersionInfo(9, 22, 0))
434+
421435
supports_antiklepto = self.version >= semver.VersionInfo(9, 4, 0)
422436

423437
sigs: List[Tuple[int, bytes]] = []
@@ -433,6 +447,7 @@ def btc_sign(
433447
num_outputs=len(outputs),
434448
locktime=locktime,
435449
format_unit=format_unit,
450+
output_script_configs=output_script_configs,
436451
)
437452
)
438453
next_response = self._msg_query(request, expected_response="btc_sign_next").btc_sign_next
@@ -552,12 +567,16 @@ def btc_sign(
552567

553568
request = hww.Request()
554569
if isinstance(tx_output, BTCOutputInternal):
570+
if tx_output.output_script_config_index is not None:
571+
# Attaching output info supported since v9.22.0.
572+
self._require_atleast(semver.VersionInfo(9, 22, 0))
555573
request.btc_sign_output.CopyFrom(
556574
btc.BTCSignOutputRequest(
557575
ours=True,
558576
value=tx_output.value,
559577
keypath=tx_output.keypath,
560578
script_config_index=tx_output.script_config_index,
579+
output_script_config_index=tx_output.output_script_config_index,
561580
)
562581
)
563582
elif isinstance(tx_output, BTCOutputExternal):

py/bitbox02/bitbox02/communication/generated/btc_pb2.py

Lines changed: 50 additions & 50 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

py/bitbox02/bitbox02/communication/generated/btc_pb2.pyi

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ class BTCSignInitRequest(google.protobuf.message.Message):
293293
LOCKTIME_FIELD_NUMBER: builtins.int
294294
FORMAT_UNIT_FIELD_NUMBER: builtins.int
295295
CONTAINS_SILENT_PAYMENT_OUTPUTS_FIELD_NUMBER: builtins.int
296+
OUTPUT_SCRIPT_CONFIGS_FIELD_NUMBER: builtins.int
296297
coin: global___BTCCoin.ValueType
297298
@property
298299
def script_configs(self) -> google.protobuf.internal.containers.RepeatedCompositeFieldContainer[global___BTCScriptConfigWithKeypath]:
@@ -308,6 +309,12 @@ class BTCSignInitRequest(google.protobuf.message.Message):
308309

309310
format_unit: global___BTCSignInitRequest.FormatUnit.ValueType
310311
contains_silent_payment_outputs: builtins.bool
312+
@property
313+
def output_script_configs(self) -> google.protobuf.internal.containers.RepeatedCompositeFieldContainer[global___BTCScriptConfigWithKeypath]:
314+
"""used script configs for outputs that send to an address of the same keystore, but not
315+
necessarily the same account (as defined by `script_configs` above).
316+
"""
317+
pass
311318
def __init__(self,
312319
*,
313320
coin: global___BTCCoin.ValueType = ...,
@@ -318,8 +325,9 @@ class BTCSignInitRequest(google.protobuf.message.Message):
318325
locktime: builtins.int = ...,
319326
format_unit: global___BTCSignInitRequest.FormatUnit.ValueType = ...,
320327
contains_silent_payment_outputs: builtins.bool = ...,
328+
output_script_configs: typing.Optional[typing.Iterable[global___BTCScriptConfigWithKeypath]] = ...,
321329
) -> None: ...
322-
def ClearField(self, field_name: typing_extensions.Literal["coin",b"coin","contains_silent_payment_outputs",b"contains_silent_payment_outputs","format_unit",b"format_unit","locktime",b"locktime","num_inputs",b"num_inputs","num_outputs",b"num_outputs","script_configs",b"script_configs","version",b"version"]) -> None: ...
330+
def ClearField(self, field_name: typing_extensions.Literal["coin",b"coin","contains_silent_payment_outputs",b"contains_silent_payment_outputs","format_unit",b"format_unit","locktime",b"locktime","num_inputs",b"num_inputs","num_outputs",b"num_outputs","output_script_configs",b"output_script_configs","script_configs",b"script_configs","version",b"version"]) -> None: ...
323331
global___BTCSignInitRequest = BTCSignInitRequest
324332

325333
class BTCSignNextResponse(google.protobuf.message.Message):
@@ -454,6 +462,7 @@ class BTCSignOutputRequest(google.protobuf.message.Message):
454462
SCRIPT_CONFIG_INDEX_FIELD_NUMBER: builtins.int
455463
PAYMENT_REQUEST_INDEX_FIELD_NUMBER: builtins.int
456464
SILENT_PAYMENT_FIELD_NUMBER: builtins.int
465+
OUTPUT_SCRIPT_CONFIG_INDEX_FIELD_NUMBER: builtins.int
457466
ours: builtins.bool
458467
type: global___BTCOutputType.ValueType
459468
"""if ours is false"""
@@ -469,7 +478,10 @@ class BTCSignOutputRequest(google.protobuf.message.Message):
469478
"""if ours is true"""
470479
pass
471480
script_config_index: builtins.int
472-
"""If ours is true. References a script config from BTCSignInitRequest"""
481+
"""If ours is true and `output_script_config_index` is absent. References a script config from
482+
BTCSignInitRequest. This allows change output identification and allows us to identify
483+
non-change outputs to the same account, so we can display this info to the user.
484+
"""
473485

474486
payment_request_index: builtins.int
475487
@property
@@ -478,6 +490,13 @@ class BTCSignOutputRequest(google.protobuf.message.Message):
478490
BTCSignNextResponse. `contains_silent_payment_outputs` in the init request must be true.
479491
"""
480492
pass
493+
output_script_config_index: builtins.int
494+
"""If ours is true. If set, `script_config_index` is ignored. References an output script config
495+
from BTCSignInitRequest. This enables verification that an output belongs to the same keystore,
496+
even if it is from a different account than we spend from, allowing us to display this info to
497+
the user.
498+
"""
499+
481500
def __init__(self,
482501
*,
483502
ours: builtins.bool = ...,
@@ -488,9 +507,13 @@ class BTCSignOutputRequest(google.protobuf.message.Message):
488507
script_config_index: builtins.int = ...,
489508
payment_request_index: typing.Optional[builtins.int] = ...,
490509
silent_payment: typing.Optional[global___BTCSignOutputRequest.SilentPayment] = ...,
510+
output_script_config_index: typing.Optional[builtins.int] = ...,
491511
) -> None: ...
492-
def HasField(self, field_name: typing_extensions.Literal["_payment_request_index",b"_payment_request_index","payment_request_index",b"payment_request_index","silent_payment",b"silent_payment"]) -> builtins.bool: ...
493-
def ClearField(self, field_name: typing_extensions.Literal["_payment_request_index",b"_payment_request_index","keypath",b"keypath","ours",b"ours","payload",b"payload","payment_request_index",b"payment_request_index","script_config_index",b"script_config_index","silent_payment",b"silent_payment","type",b"type","value",b"value"]) -> None: ...
512+
def HasField(self, field_name: typing_extensions.Literal["_output_script_config_index",b"_output_script_config_index","_payment_request_index",b"_payment_request_index","output_script_config_index",b"output_script_config_index","payment_request_index",b"payment_request_index","silent_payment",b"silent_payment"]) -> builtins.bool: ...
513+
def ClearField(self, field_name: typing_extensions.Literal["_output_script_config_index",b"_output_script_config_index","_payment_request_index",b"_payment_request_index","keypath",b"keypath","ours",b"ours","output_script_config_index",b"output_script_config_index","payload",b"payload","payment_request_index",b"payment_request_index","script_config_index",b"script_config_index","silent_payment",b"silent_payment","type",b"type","value",b"value"]) -> None: ...
514+
@typing.overload
515+
def WhichOneof(self, oneof_group: typing_extensions.Literal["_output_script_config_index",b"_output_script_config_index"]) -> typing.Optional[typing_extensions.Literal["output_script_config_index"]]: ...
516+
@typing.overload
494517
def WhichOneof(self, oneof_group: typing_extensions.Literal["_payment_request_index",b"_payment_request_index"]) -> typing.Optional[typing_extensions.Literal["payment_request_index"]]: ...
495518
global___BTCSignOutputRequest = BTCSignOutputRequest
496519

py/bitbox02/bitbox02/communication/generated/cardano_pb2.pyi

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,10 @@ class CardanoSignTransactionRequest(google.protobuf.message.Message):
299299
"""include ttl even if it is zero"""
300300

301301
tag_cbor_sets: builtins.bool
302+
"""Tag arrays in the transaction serialization with the 258 tag.
303+
See https://github.com/IntersectMBO/cardano-ledger/blob/6e2d37cc0f47bd02e89b4ce9f78b59c35c958e96/eras/conway/impl/cddl-files/extra.cddl#L5
304+
"""
305+
302306
def __init__(self,
303307
*,
304308
network: global___CardanoNetwork.ValueType = ...,

py/send_message.py

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ def _sign_btc_normal(
526526
for input_index, sig in sigs:
527527
print("Signature for input {}: {}".format(input_index, sig.hex()))
528528

529-
def _sign_btc_send_to_self(
529+
def _sign_btc_send_to_self_same_account(
530530
self,
531531
format_unit: "bitbox02.btc.BTCSignInitRequest.FormatUnit.V" = bitbox02.btc.BTCSignInitRequest.FormatUnit.DEFAULT,
532532
) -> None:
@@ -561,6 +561,50 @@ def _sign_btc_send_to_self(
561561
for input_index, sig in sigs:
562562
print("Signature for input {}: {}".format(input_index, sig.hex()))
563563

564+
def _sign_btc_send_to_self_different_account(
565+
self,
566+
format_unit: "bitbox02.btc.BTCSignInitRequest.FormatUnit.V" = bitbox02.btc.BTCSignInitRequest.FormatUnit.DEFAULT,
567+
) -> None:
568+
# pylint: disable=no-member
569+
bip44_account: int = 0 + HARDENED
570+
inputs, outputs = _btc_demo_inputs_outputs(bip44_account)
571+
outputs[1] = bitbox02.BTCOutputInternal(
572+
keypath=[84 + HARDENED, 0 + HARDENED, 1 + HARDENED, 0, 0],
573+
value=int(1e8 * 0.2),
574+
script_config_index=0,
575+
output_script_config_index=0,
576+
)
577+
sigs = self._device.btc_sign(
578+
bitbox02.btc.BTC,
579+
[
580+
bitbox02.btc.BTCScriptConfigWithKeypath(
581+
script_config=bitbox02.btc.BTCScriptConfig(
582+
simple_type=bitbox02.btc.BTCScriptConfig.P2WPKH
583+
),
584+
keypath=[84 + HARDENED, 0 + HARDENED, bip44_account],
585+
),
586+
bitbox02.btc.BTCScriptConfigWithKeypath(
587+
script_config=bitbox02.btc.BTCScriptConfig(
588+
simple_type=bitbox02.btc.BTCScriptConfig.P2WPKH_P2SH
589+
),
590+
keypath=[49 + HARDENED, 0 + HARDENED, bip44_account],
591+
),
592+
],
593+
inputs=inputs,
594+
outputs=outputs,
595+
format_unit=format_unit,
596+
output_script_configs=[
597+
bitbox02.btc.BTCScriptConfigWithKeypath(
598+
script_config=bitbox02.btc.BTCScriptConfig(
599+
simple_type=bitbox02.btc.BTCScriptConfig.P2WPKH
600+
),
601+
keypath=[84 + HARDENED, 0 + HARDENED, 1 + HARDENED],
602+
),
603+
],
604+
)
605+
for input_index, sig in sigs:
606+
print("Signature for input {}: {}".format(input_index, sig.hex()))
607+
564608
def _sign_btc_high_fee(self) -> None:
565609
# pylint: disable=no-member
566610
bip44_account: int = 0 + HARDENED
@@ -832,7 +876,8 @@ def _sign_btc_tx(self) -> None:
832876
format_unit=bitbox02.btc.BTCSignInitRequest.FormatUnit.SAT
833877
),
834878
),
835-
("Send to self", self._sign_btc_send_to_self),
879+
("Send to self (same account)", self._sign_btc_send_to_self_same_account),
880+
("Send to self (different account)", self._sign_btc_send_to_self_different_account),
836881
("High fee warning", self._sign_btc_high_fee),
837882
("Multiple change outputs", self._sign_btc_multiple_changes),
838883
("Locktime/RBF", self._sign_btc_locktime_rbf),

src/rust/bitbox02-rust/src/hww/api/bitcoin.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,10 +241,7 @@ async fn address_policy(
241241

242242
let parsed = policies::parse(policy, coin)?;
243243

244-
let name = match policies::get_name(coin, policy)? {
245-
Some(name) => name,
246-
None => return Err(Error::InvalidInput),
247-
};
244+
let name = parsed.name(coin_params)?.ok_or(Error::InvalidInput)?;
248245

249246
let title = "Receive to";
250247

0 commit comments

Comments
 (0)