Merge remote-tracking branch 'benma/self-send'

Author: benma

CHANGELOG.md

@@ -11,6 +11,7 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
 - Ethereum: remove deprecated Goerli network
 - SD card: solve backup bug when sd card is re-inserted
 - Cardano: allow serialization using 258-tagged sets
+- Bitcoin: identify outputs belonging to different accounts of the same keystore
 
 ### 9.21.0
 - Bitcoin: add support for sending to silent payment (BIP-352) addresses

messages/btc.proto

@@ -115,6 +115,9 @@ message BTCSignInitRequest {
   }
   FormatUnit format_unit = 8;
   bool contains_silent_payment_outputs = 9;
+  // used script configs for outputs that send to an address of the same keystore, but not
+  // necessarily the same account (as defined by `script_configs` above).
+  repeated BTCScriptConfigWithKeypath output_script_configs = 10;
 }
 
 message BTCSignNextResponse {
@@ -174,12 +177,19 @@ message BTCSignOutputRequest {
   uint64 value = 3;
   bytes payload = 4; // if ours is false. Renamed from `hash`.
   repeated uint32 keypath = 5; // if ours is true
-  // If ours is true. References a script config from BTCSignInitRequest
+  // If ours is true and `output_script_config_index` is absent. References a script config from
+  // BTCSignInitRequest. This allows change output identification and allows us to identify
+  // non-change outputs to the same account, so we can display this info to the user.
   uint32 script_config_index = 6;
   optional uint32 payment_request_index = 7;
   // If provided, `type` and `payload` is ignored. The generated output pkScript is returned in
   // BTCSignNextResponse. `contains_silent_payment_outputs` in the init request must be true.
   SilentPayment silent_payment = 8;
+  // If ours is true. If set, `script_config_index` is ignored. References an output script config
+  // from BTCSignInitRequest. This enables verification that an output belongs to the same keystore,
+  // even if it is from a different account than we spend from, allowing us to display this info to
+  // the user.
+  optional uint32 output_script_config_index = 9;
 }
 
 message BTCScriptConfigRegistration {

py/bitbox02/CHANGELOG.md

@@ -2,13 +2,12 @@
 
 ## [Unreleased]
 
-# 8.0.0
-- SD card: Remove API to prompt removal of the microSD card from the device
-- Add support for regtest (supported from firmware version v9.21.0)
-
 # 7.0.0
 - get_info: add optional device initialized boolean to returned tuple
 - eth_sign: add address_case field, which should be initialized by the client
+- SD card: Remove API to prompt removal of the microSD card from the device
+- Add support for regtest (supported from firmware version v9.21.0)
+- btc_sign: allow identifying outputs belonging to different accounts of the same keystore
 
 # 6.3.0
 - Allow infering product and version via API call instead of via USB descriptor

py/bitbox02/bitbox02/bitbox02/__init__.py

@@ -16,7 +16,7 @@
 from __future__ import print_function
 import sys
 
-__version__ = "8.0.0"
+__version__ = "7.0.0"
 
 if sys.version_info.major != 3 or sys.version_info.minor < 6:
     print(

py/bitbox02/bitbox02/bitbox02/bitbox02.py

@@ -117,13 +117,20 @@ class BTCInputType(TypedDict):
 class BTCOutputInternal:
     # TODO: Use NamedTuple, but not playing well with protobuf types.
 
-    def __init__(self, keypath: Sequence[int], value: int, script_config_index: int):
+    def __init__(
+        self,
+        keypath: Sequence[int],
+        value: int,
+        script_config_index: int,
+        output_script_config_index: Optional[int] = None,
+    ):
         """
         keypath: keypath to the change output.
         """
         self.keypath = keypath
         self.value = value
         self.script_config_index = script_config_index
+        self.output_script_config_index = output_script_config_index
 
 
 class BTCOutputExternal:
@@ -390,13 +397,14 @@ def btc_sign(
         version: int = 1,
         locktime: int = 0,
         format_unit: "btc.BTCSignInitRequest.FormatUnit.V" = btc.BTCSignInitRequest.FormatUnit.DEFAULT,
+        output_script_configs: Optional[Sequence[btc.BTCScriptConfigWithKeypath]] = None,
     ) -> Sequence[Tuple[int, bytes]]:
         """
         coin: the first element of all provided keypaths must match the coin:
         - BTC: 0 + HARDENED
         - Testnets: 1 + HARDENED
         - LTC: 2 + HARDENED
-        script_configs: types of all inputs and change outputs. The first element of all provided
+        script_configs: types of all inputs and outputs belonging to the same account (change or non-change). The first element of all provided
         keypaths must match this type:
         - SCRIPT_P2PKH: 44 + HARDENED
         - SCRIPT_P2WPKH_P2SH: 49 + HARDENED
@@ -407,6 +415,8 @@ def btc_sign(
         outputs: transaction outputs. Can be an external output
         (BTCOutputExternal) or an internal output for change (BTCOutputInternal).
         version, locktime: reserved for future use.
+        format_unit: defines in which unit amounts will be displayed
+        output_script_configs: script types for outputs belonging to the same keystore
         Returns: list of (input index, signature) tuples.
         Raises Bitbox02Exception with ERR_USER_ABORT on user abort.
         """
@@ -418,6 +428,10 @@ def btc_sign(
         if any(map(is_taproot, script_configs)):
             self._require_atleast(semver.VersionInfo(9, 10, 0))
 
+        if output_script_configs:
+            # Attaching output info supported since v9.22.0.
+            self._require_atleast(semver.VersionInfo(9, 22, 0))
+
         supports_antiklepto = self.version >= semver.VersionInfo(9, 4, 0)
 
         sigs: List[Tuple[int, bytes]] = []
@@ -433,6 +447,7 @@ def btc_sign(
                 num_outputs=len(outputs),
                 locktime=locktime,
                 format_unit=format_unit,
+                output_script_configs=output_script_configs,
             )
         )
         next_response = self._msg_query(request, expected_response="btc_sign_next").btc_sign_next
@@ -552,12 +567,16 @@ def btc_sign(
 
                 request = hww.Request()
                 if isinstance(tx_output, BTCOutputInternal):
+                    if tx_output.output_script_config_index is not None:
+                        # Attaching output info supported since v9.22.0.
+                        self._require_atleast(semver.VersionInfo(9, 22, 0))
                     request.btc_sign_output.CopyFrom(
                         btc.BTCSignOutputRequest(
                             ours=True,
                             value=tx_output.value,
                             keypath=tx_output.keypath,
                             script_config_index=tx_output.script_config_index,
+                            output_script_config_index=tx_output.output_script_config_index,
                         )
                     )
                 elif isinstance(tx_output, BTCOutputExternal):

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

@@ -293,6 +293,7 @@ class BTCSignInitRequest(google.protobuf.message.Message):
     LOCKTIME_FIELD_NUMBER: builtins.int
     FORMAT_UNIT_FIELD_NUMBER: builtins.int
     CONTAINS_SILENT_PAYMENT_OUTPUTS_FIELD_NUMBER: builtins.int
+    OUTPUT_SCRIPT_CONFIGS_FIELD_NUMBER: builtins.int
     coin: global___BTCCoin.ValueType
     @property
     def script_configs(self) -> google.protobuf.internal.containers.RepeatedCompositeFieldContainer[global___BTCScriptConfigWithKeypath]:
@@ -308,6 +309,12 @@ class BTCSignInitRequest(google.protobuf.message.Message):
 
     format_unit: global___BTCSignInitRequest.FormatUnit.ValueType
     contains_silent_payment_outputs: builtins.bool
+    @property
+    def output_script_configs(self) -> google.protobuf.internal.containers.RepeatedCompositeFieldContainer[global___BTCScriptConfigWithKeypath]:
+        """used script configs for outputs that send to an address of the same keystore, but not
+        necessarily the same account (as defined by `script_configs` above).
+        """
+        pass
     def __init__(self,
         *,
         coin: global___BTCCoin.ValueType = ...,
@@ -318,8 +325,9 @@ class BTCSignInitRequest(google.protobuf.message.Message):
         locktime: builtins.int = ...,
         format_unit: global___BTCSignInitRequest.FormatUnit.ValueType = ...,
         contains_silent_payment_outputs: builtins.bool = ...,
+        output_script_configs: typing.Optional[typing.Iterable[global___BTCScriptConfigWithKeypath]] = ...,
         ) -> None: ...
-    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: ...
+    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: ...
 global___BTCSignInitRequest = BTCSignInitRequest
 
 class BTCSignNextResponse(google.protobuf.message.Message):
@@ -454,6 +462,7 @@ class BTCSignOutputRequest(google.protobuf.message.Message):
     SCRIPT_CONFIG_INDEX_FIELD_NUMBER: builtins.int
     PAYMENT_REQUEST_INDEX_FIELD_NUMBER: builtins.int
     SILENT_PAYMENT_FIELD_NUMBER: builtins.int
+    OUTPUT_SCRIPT_CONFIG_INDEX_FIELD_NUMBER: builtins.int
     ours: builtins.bool
     type: global___BTCOutputType.ValueType
     """if ours is false"""
@@ -469,7 +478,10 @@ class BTCSignOutputRequest(google.protobuf.message.Message):
         """if ours is true"""
         pass
     script_config_index: builtins.int
-    """If ours is true. References a script config from BTCSignInitRequest"""
+    """If ours is true and `output_script_config_index` is absent. References a script config from
+    BTCSignInitRequest. This allows change output identification and allows us to identify
+    non-change outputs to the same account, so we can display this info to the user.
+    """
 
     payment_request_index: builtins.int
     @property
@@ -478,6 +490,13 @@ class BTCSignOutputRequest(google.protobuf.message.Message):
         BTCSignNextResponse. `contains_silent_payment_outputs` in the init request must be true.
         """
         pass
+    output_script_config_index: builtins.int
+    """If ours is true. If set, `script_config_index` is ignored. References an output script config
+    from BTCSignInitRequest. This enables verification that an output belongs to the same keystore,
+    even if it is from a different account than we spend from, allowing us to display this info to
+    the user.
+    """
+
     def __init__(self,
         *,
         ours: builtins.bool = ...,
@@ -488,9 +507,13 @@ class BTCSignOutputRequest(google.protobuf.message.Message):
         script_config_index: builtins.int = ...,
         payment_request_index: typing.Optional[builtins.int] = ...,
         silent_payment: typing.Optional[global___BTCSignOutputRequest.SilentPayment] = ...,
+        output_script_config_index: typing.Optional[builtins.int] = ...,
         ) -> None: ...
-    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: ...
-    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: ...
+    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: ...
+    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: ...
+    @typing.overload
+    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"]]: ...
+    @typing.overload
     def WhichOneof(self, oneof_group: typing_extensions.Literal["_payment_request_index",b"_payment_request_index"]) -> typing.Optional[typing_extensions.Literal["payment_request_index"]]: ...
 global___BTCSignOutputRequest = BTCSignOutputRequest
 

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

@@ -299,6 +299,10 @@ class CardanoSignTransactionRequest(google.protobuf.message.Message):
     """include ttl even if it is zero"""
 
     tag_cbor_sets: builtins.bool
+    """Tag arrays in the transaction serialization with the 258 tag.
+    See https://github.com/IntersectMBO/cardano-ledger/blob/6e2d37cc0f47bd02e89b4ce9f78b59c35c958e96/eras/conway/impl/cddl-files/extra.cddl#L5
+    """
+
     def __init__(self,
         *,
         network: global___CardanoNetwork.ValueType = ...,

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

@@ -526,7 +526,7 @@ def _sign_btc_normal(
         for input_index, sig in sigs:
             print("Signature for input {}: {}".format(input_index, sig.hex()))
 
-    def _sign_btc_send_to_self(
+    def _sign_btc_send_to_self_same_account(
         self,
         format_unit: "bitbox02.btc.BTCSignInitRequest.FormatUnit.V" = bitbox02.btc.BTCSignInitRequest.FormatUnit.DEFAULT,
     ) -> None:
@@ -561,6 +561,50 @@ def _sign_btc_send_to_self(
         for input_index, sig in sigs:
             print("Signature for input {}: {}".format(input_index, sig.hex()))
 
+    def _sign_btc_send_to_self_different_account(
+        self,
+        format_unit: "bitbox02.btc.BTCSignInitRequest.FormatUnit.V" = bitbox02.btc.BTCSignInitRequest.FormatUnit.DEFAULT,
+    ) -> None:
+        # pylint: disable=no-member
+        bip44_account: int = 0 + HARDENED
+        inputs, outputs = _btc_demo_inputs_outputs(bip44_account)
+        outputs[1] = bitbox02.BTCOutputInternal(
+            keypath=[84 + HARDENED, 0 + HARDENED, 1 + HARDENED, 0, 0],
+            value=int(1e8 * 0.2),
+            script_config_index=0,
+            output_script_config_index=0,
+        )
+        sigs = self._device.btc_sign(
+            bitbox02.btc.BTC,
+            [
+                bitbox02.btc.BTCScriptConfigWithKeypath(
+                    script_config=bitbox02.btc.BTCScriptConfig(
+                        simple_type=bitbox02.btc.BTCScriptConfig.P2WPKH
+                    ),
+                    keypath=[84 + HARDENED, 0 + HARDENED, bip44_account],
+                ),
+                bitbox02.btc.BTCScriptConfigWithKeypath(
+                    script_config=bitbox02.btc.BTCScriptConfig(
+                        simple_type=bitbox02.btc.BTCScriptConfig.P2WPKH_P2SH
+                    ),
+                    keypath=[49 + HARDENED, 0 + HARDENED, bip44_account],
+                ),
+            ],
+            inputs=inputs,
+            outputs=outputs,
+            format_unit=format_unit,
+            output_script_configs=[
+                bitbox02.btc.BTCScriptConfigWithKeypath(
+                    script_config=bitbox02.btc.BTCScriptConfig(
+                        simple_type=bitbox02.btc.BTCScriptConfig.P2WPKH
+                    ),
+                    keypath=[84 + HARDENED, 0 + HARDENED, 1 + HARDENED],
+                ),
+            ],
+        )
+        for input_index, sig in sigs:
+            print("Signature for input {}: {}".format(input_index, sig.hex()))
+
     def _sign_btc_high_fee(self) -> None:
         # pylint: disable=no-member
         bip44_account: int = 0 + HARDENED
@@ -832,7 +876,8 @@ def _sign_btc_tx(self) -> None:
                     format_unit=bitbox02.btc.BTCSignInitRequest.FormatUnit.SAT
                 ),
             ),
-            ("Send to self", self._sign_btc_send_to_self),
+            ("Send to self (same account)", self._sign_btc_send_to_self_same_account),
+            ("Send to self (different account)", self._sign_btc_send_to_self_different_account),
             ("High fee warning", self._sign_btc_high_fee),
             ("Multiple change outputs", self._sign_btc_multiple_changes),
             ("Locktime/RBF", self._sign_btc_locktime_rbf),

py/send_message.py

@@ -241,10 +241,7 @@ async fn address_policy(
 
     let parsed = policies::parse(policy, coin)?;
 
-    let name = match policies::get_name(coin, policy)? {
-        Some(name) => name,
-        None => return Err(Error::InvalidInput),
-    };
+    let name = parsed.name(coin_params)?.ok_or(Error::InvalidInput)?;
 
     let title = "Receive to";
 

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

@@ -191,13 +191,15 @@ impl Payload {
             ValidatedScriptConfig::SimpleType(simple_type) => {
                 Self::from_simple(xpub_cache, params, *simple_type, keypath)
             }
-            ValidatedScriptConfig::Multisig(multisig) => Self::from_multisig(
+            ValidatedScriptConfig::Multisig { multisig, .. } => Self::from_multisig(
                 params,
                 multisig,
                 keypath[keypath.len() - 2],
                 keypath[keypath.len() - 1],
             ),
-            ValidatedScriptConfig::Policy(policy) => Self::from_policy(params, policy, keypath),
+            ValidatedScriptConfig::Policy { parsed_policy, .. } => {
+                Self::from_policy(params, parsed_policy, keypath)
+            }
         }
     }
 

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

@@ -272,6 +272,13 @@ pub struct ParsedPolicy<'a> {
 }
 
 impl ParsedPolicy<'_> {
+    /// Get the name of a registered policy account.
+    ///
+    /// Returns the name of the registered policy account if it exists or None otherwise.
+    pub fn name(&self, params: &Params) -> Result<Option<String>, ()> {
+        get_name(params.coin, self.policy)
+    }
+
     /// Iterates over the placeholder keys in this descriptor. For tr() descriptors, this covers the
     /// internal key and every key in every leaf script.
     /// This iterates the keys "left-to-right" in the descriptor.
@@ -739,7 +746,7 @@ pub fn get_hash(coin: BtcCoin, policy: &Policy) -> Result<Vec<u8>, ()> {
     Ok(hasher.finalize().as_slice().into())
 }
 
-/// Get the name of a registered policy account. The poliy is not validated, it must be
+/// Get the name of a registered policy account. The policy is not validated, it must be
 /// pre-validated!
 ///
 /// Returns the name of the registered policy account if it exists or None otherwise.