Merge #470: Rename addrtype enum values to remove misnomer

a95ccd1 Change address type default to WIT (Andrew Chow)
de0a19d Rename AddrType enums (Andrew Chow)

Pull request description:

  As noted previously, the current `AddrType` enums are a misnomer for multisig addresses, and in general, things that are not single keys. This PR fixes that by renaming them. PKH is now LEGACY, SH_WPKH is SH_WIT_V0, and WPKH is WIT_V0.

  I decided to go wit_v0 to be more explicit about the segwit version. This will let us unambiguously have future address types that are other segwit versions.

  Additionally this PR makes WIT_V0 the default address type instead of LEGACY.

Top commit has no ACKs.

Tree-SHA512: f87114cd68950aa26a2e51c03eef821133edcd7bb135919d1e65c6c924c9a32107494ce6396903157f69a9691be3c0b8a76559c103d994d6b87ce6e0213cbd1d

docs/examples/bitcoin-core-usage.rst

@@ -52,7 +52,7 @@ specified, both receiving and change address descriptors are generated.
 
 ::
 
-    $ ./hwi.py -f e5dbc9cb getkeypool --addr-type wpkh 0 1000
+    $ ./hwi.py -f e5dbc9cb getkeypool 0 1000
     [{"desc": "wpkh([e5dbc9cb/84'/0'/0']xpub6CbtS57jivMSuzcvp5YZxp6JhUU8YWup2axi2xkQRVHY8w4otp8YkEvfWBHgE5rA2AJYNHquuRoLFFdWeSi1UgVohcUeM7SkE9c8NftRwRJ/0/*)#cwyap6p3", "range": [0, 1000], "timestamp": "now", "internal": false, "keypool": true, "active": true, "watchonly": true}, {"desc": "wpkh([e5dbc9cb/84'/0'/0']xpub6CbtS57jivMSuzcvp5YZxp6JhUU8YWup2axi2xkQRVHY8w4otp8YkEvfWBHgE5rA2AJYNHquuRoLFFdWeSi1UgVohcUeM7SkE9c8NftRwRJ/1/*)#f6puu03f", "range": [0, 1000], "timestamp": "now", "internal": true, "keypool": true, "active": true, "watchonly": true}]
 
 We now create a new Bitcoin Core Descriptor Wallet and import the keys into Bitcoin Core. The output is formatted properly for Bitcoin Core so it can be copy and pasted.
@@ -276,9 +276,9 @@ Derivation Path BIP Compliance
 ==============================
 
 The instructions above use BIP 84 to derive keys used for P2WPKH addresses (bech32 addresses).
-HWI follows BIPs 44, 84, and 49. By default, descriptors will be for P2PKH addresses with keys derived at ``m/44h/0h/0h/0`` for normal receiving keys and ``m/44h/0h/0h/1`` for change keys.
-Using the ``--addr-type wpkh`` option will result in P2WPKH addresses with keys derived at ``m/84h/0h/0h/0`` for normal receiving keys and ``m/84h/0h/0h/1`` for change keys.
-Using the ``--addr-type sh_wpkh`` option will result in P2SH nested P2WPKH addresses with keys derived at ``m/49h/0h/0h/0`` for normal receiving keys and ``m/49h/0h/0h/1`` for change keys.
+HWI follows BIPs 44, 84, and 49. By default, descriptors will be for P2WPKH addresses with keys derived at ``m/84h/0h/0h/0`` for normal receiving keys and ``m/84h/0h/0h/1`` for change keys.
+Using the ``--addr-type legacy`` option will result in P2PKH addresses with keys derived at ``m/44h/0h/0h/0`` for normal receiving keys and ``m/44h/0h/0h/1`` for change keys.
+Using the ``--addr-type sh_wit`` option will result in P2SH nested P2WPKH addresses with keys derived at ``m/49h/0h/0h/0`` for normal receiving keys and ``m/49h/0h/0h/1`` for change keys.
 
 To actually get the correct address type when using ``getnewaddress`` from Bitcoin Core, you will need to additionally set ``-addresstype=p2sh-segwit`` and ``-changetype=p2sh-segwit``.
 This can be set in the command line (as shown in the example) or in your bitcoin.conf file.

docs/examples/walkthrough/walkthrough.rst

@@ -35,8 +35,8 @@ Then in another terminal run commands similar to these, adapted to your environm
   # of unknown address on Trezor display. This is not recommended. Use the correct derivation path
   # for the corresponding network!
   wallet=wallet.test
-  rec=$(hwi --testnet -f $FINGERPRINT_TESTNET getkeypool --addr-type wpkh --path m/84h/${DERIVATIONPATH_TESTNET}h/0h/0/* --keypool 0 1000)
-  chg=$(hwi --testnet -f $FINGERPRINT_TESTNET getkeypool --addr-type wpkh --path m/84h/${DERIVATIONPATH_TESTNET}h/0h/1/* --keypool --internal 0 1000)
+  rec=$(hwi --testnet -f $FINGERPRINT_TESTNET getkeypool --addr-type wit --path m/84h/${DERIVATIONPATH_TESTNET}h/0h/0/* --keypool 0 1000)
+  chg=$(hwi --testnet -f $FINGERPRINT_TESTNET getkeypool --addr-type wit --path m/84h/${DERIVATIONPATH_TESTNET}h/0h/1/* --keypool --internal 0 1000)
   bitcoin-cli -testnet createwallet "$wallet" true
   bitcoin-cli -testnet -rpcwallet="$wallet" importmulti "$rec"
   bitcoin-cli -testnet -rpcwallet="$wallet" importmulti "$chg"
@@ -184,5 +184,5 @@ Versions Used
 =============
 
 * This walk-trough was done in Janary 2021
-* HWI version 1.2.1
+* HWI version 2.0.0-dev
 * Bitcoin 0.21.0

hwilib/_cli.py

@@ -174,7 +174,7 @@ def get_parser() -> HWIArgumentParser:
     kparg_group.add_argument('--nokeypool', action='store_false', dest='keypool', help='Indicates that the keys are not to be imported to the keypool', default=False)
     getkeypool_parser.add_argument('--internal', action='store_true', help='Indicates that the keys are change keys')
     kp_type_group = getkeypool_parser.add_mutually_exclusive_group()
-    kp_type_group.add_argument("--addr-type", help="The address type (and default derivation path) to produce descriptors for", type=AddressType.argparse, choices=list(AddressType), default=AddressType.PKH) # type: ignore
+    kp_type_group.add_argument("--addr-type", help="The address type (and default derivation path) to produce descriptors for", type=AddressType.argparse, choices=list(AddressType), default=AddressType.WIT) # type: ignore
     kp_type_group.add_argument('--all', action='store_true', help='Generate addresses for all standard address types (default paths: ``m/{44,49,84}h/0h/0h/[0,1]/*)``')
     getkeypool_parser.add_argument('--account', help='BIP43 account', type=int, default=0)
     getkeypool_parser.add_argument('--path', help='Derivation path, default follows BIP43 convention, e.g. ``m/84h/0h/0h/1/*`` with --addr-type wpkh --internal. If this argument and --internal is not given, both internal and external keypools will be returned.')
@@ -190,7 +190,7 @@ def get_parser() -> HWIArgumentParser:
     group = displayaddr_parser.add_mutually_exclusive_group(required=True)
     group.add_argument('--desc', help='Output Descriptor. E.g. wpkh([00000000/84h/0h/0h]xpub.../0/0), where 00000000 must match --fingerprint and xpub can be obtained with getxpub. See doc/descriptors.md in Bitcoin Core')
     group.add_argument('--path', help='The BIP 32 derivation path of the key embedded in the address, default follows BIP43 convention, e.g. ``m/84h/0h/0h/1/*``')
-    displayaddr_parser.add_argument("--addr-type", help="The address type to display", type=AddressType.argparse, choices=list(AddressType), default=AddressType.PKH) # type: ignore
+    displayaddr_parser.add_argument("--addr-type", help="The address type to display", type=AddressType.argparse, choices=list(AddressType), default=AddressType.WIT) # type: ignore
     displayaddr_parser.set_defaults(func=displayaddress_handler)
 
     setupdev_parser = subparsers.add_parser('setup', help='Setup a device. Passphrase protection uses the password given by -p. Requires interactive mode')

hwilib/_gui.py

@@ -171,11 +171,14 @@ def __init__(self, client):
     @Slot()
     def go_button_clicked(self):
         path = self.ui.path_lineedit.text()
-        addrtype = AddressType.PKH
         if self.ui.sh_wpkh_radio.isChecked():
-            addrtype = AddressType.SH_WPKH
+            addrtype = AddressType.SH_WIT
         elif self.ui.wpkh_radio.isChecked():
-            addrtype = AddressType.WPKH
+            addrtype = AddressType.WIT
+        elif self.ui.pkh_radio.isChecked():
+            addrtype = AddressType.LEGACY
+        else:
+            assert False # How did this happen?
         res = do_command(commands.displayaddress, self.client, path, addr_type=addrtype)
         self.ui.address_lineedit.setText(res['address'])
 
@@ -204,9 +207,9 @@ def __init__(self, opts):
             self.ui.path_lineedit.setEnabled(True)
             self.ui.account_spinbox.setEnabled(False)
             self.ui.path_lineedit.setText(opts['path'])
-        self.ui.sh_wpkh_radio.setChecked(opts['addrtype'] == AddressType.SH_WPKH)
-        self.ui.wpkh_radio.setChecked(opts['addrtype'] == AddressType.WPKH)
-        self.ui.pkh_radio.setChecked(opts['addrtype'] == AddressType.PKH)
+        self.ui.sh_wpkh_radio.setChecked(opts['addrtype'] == AddressType.SH_WIT)
+        self.ui.wpkh_radio.setChecked(opts['addrtype'] == AddressType.WIT)
+        self.ui.pkh_radio.setChecked(opts['addrtype'] == AddressType.LEGACY)
 
         self.ui.account_radio.toggled.connect(self.toggle_account)
 
@@ -282,7 +285,7 @@ def __init__(self, passphrase='', chain=Chain.MAIN):
             'account': 0,
             'internal': False,
             'keypool': True,
-            'addrtype': AddressType.SH_WPKH,
+            'addrtype': AddressType.SH_WIT,
             'path': None,
             'account_used': True
         }
@@ -440,11 +443,13 @@ def getkeypooloptionsdialog_accepted(self):
         self.getkeypool_opts['end'] = self.current_dialog.ui.end_spinbox.value()
         self.getkeypool_opts['internal'] = self.current_dialog.ui.internal_checkbox.isChecked()
         self.getkeypool_opts['keypool'] = self.current_dialog.ui.keypool_checkbox.isChecked()
-        self.getkeypool_opts['addrtype'] = AddressType.PKH
+        self.getkeypool_opts['addrtype'] = AddressType.LEGACY
         if self.current_dialog.ui.sh_wpkh_radio.isChecked():
-            self.getkeypool_opts['addrtype'] = AddressType.SH_WPKH
+            self.getkeypool_opts['addrtype'] = AddressType.SH_WIT
         if self.current_dialog.ui.wpkh_radio.isChecked():
-            self.getkeypool_opts['addrtype'] = AddressType.WPKH
+            self.getkeypool_opts['addrtype'] = AddressType.WIT
+        if self.current_dialog.ui.pkh_radio.isChecked():
+            self.getkeypool_opts['addrtype'] = AddressType.LEGACY
         if self.current_dialog.ui.account_radio.isChecked():
             self.getkeypool_opts['account'] = self.current_dialog.ui.account_spinbox.value()
             self.getkeypool_opts['account_used'] = True

hwilib/commands.py

@@ -229,7 +229,7 @@ def getkeypool_inner(
     internal: bool = False,
     keypool: bool = True,
     account: int = 0,
-    addr_type: AddressType = AddressType.WPKH
+    addr_type: AddressType = AddressType.WIT
 ) -> List[Dict[str, Any]]:
     """
     :meta private:
@@ -267,7 +267,7 @@ def getdescriptor(
     master_fpr: bytes,
     path: Optional[str] = None,
     internal: bool = False,
-    addr_type: AddressType = AddressType.WPKH,
+    addr_type: AddressType = AddressType.WIT,
     account: int = 0,
     start: Optional[int] = None,
     end: Optional[int] = None
@@ -286,8 +286,8 @@ def getdescriptor(
     :return: The descriptor constructed given the above arguments and key fetched from the device
     :raises: BadArgumentError: if an argument is malformed or missing.
     """
-    is_wpkh = addr_type is AddressType.WPKH
-    is_sh_wpkh = addr_type is AddressType.SH_WPKH
+    is_wpkh = addr_type is AddressType.WIT
+    is_sh_wpkh = addr_type is AddressType.SH_WIT
 
     parsed_path = []
     if not path:
@@ -297,7 +297,7 @@ def getdescriptor(
         elif is_sh_wpkh:
             parsed_path.append(H_(49))
         else:
-            assert addr_type == AddressType.PKH
+            assert addr_type == AddressType.LEGACY
             parsed_path.append(H_(44))
 
         # Coin type
@@ -357,7 +357,7 @@ def getkeypool(
     internal: bool = False,
     keypool: bool = True,
     account: int = 0,
-    addr_type: AddressType = AddressType.PKH,
+    addr_type: AddressType = AddressType.WIT,
     addr_all: bool = False
 ) -> List[Dict[str, Any]]:
     """
@@ -412,7 +412,7 @@ def getdescriptors(
 
     for internal in [False, True]:
         descriptors = []
-        for addr_type in (AddressType.PKH, AddressType.SH_WPKH, AddressType.WPKH):
+        for addr_type in (AddressType.LEGACY, AddressType.SH_WIT, AddressType.WIT):
             try:
                 desc = getdescriptor(client, master_fpr=master_fpr, internal=internal, addr_type=addr_type, account=account)
             except UnavailableActionError:
@@ -432,7 +432,7 @@ def displayaddress(
     client: HardwareWalletClient,
     path: Optional[str] = None,
     desc: Optional[str] = None,
-    addr_type: AddressType = AddressType.PKH
+    addr_type: AddressType = AddressType.WIT
 ) -> Dict[str, str]:
     """
     Display an address on the device for client.
@@ -450,7 +450,7 @@ def displayaddress(
         return {"address": client.display_singlesig_address(path, addr_type)}
     elif desc is not None:
         descriptor = parse_descriptor(desc)
-        addr_type = AddressType.PKH
+        addr_type = AddressType.LEGACY
         is_sh = isinstance(descriptor, SHDescriptor)
         is_wsh = isinstance(descriptor, WSHDescriptor)
         if is_sh or is_wsh:
@@ -462,9 +462,9 @@ def displayaddress(
                 descriptor = descriptor.subdescriptor
             if isinstance(descriptor, MultisigDescriptor):
                 if is_sh and is_wsh:
-                    addr_type = AddressType.SH_WPKH
+                    addr_type = AddressType.SH_WIT
                 elif not is_sh and is_wsh:
-                    addr_type = AddressType.WPKH
+                    addr_type = AddressType.WIT
                 return {"address": client.display_multisig_address(descriptor.thresh, descriptor.pubkeys, addr_type)}
         is_wpkh = isinstance(descriptor, WPKHDescriptor)
         if isinstance(descriptor, PKHDescriptor) or is_wpkh:
@@ -477,9 +477,9 @@ def displayaddress(
             if pubkey.pubkey != xpub and pubkey.pubkey != xpub_to_pub_hex(xpub):
                 raise BadArgumentError(f"Key in descriptor does not match device: {desc}")
             if is_sh and is_wpkh:
-                addr_type = AddressType.SH_WPKH
+                addr_type = AddressType.SH_WIT
             elif not is_sh and is_wpkh:
-                addr_type = AddressType.WPKH
+                addr_type = AddressType.WIT
             return {"address": client.display_singlesig_address(pubkey.get_full_derivation_path(0), addr_type)}
     raise BadArgumentError("Missing both path and descriptor")
 

hwilib/common.py

@@ -37,9 +37,9 @@ class AddressType(Enum):
     """
     The type of address to use
     """
-    PKH = 1 #: Legacy address type. P2PKH for single sig, P2SH for scripts.
-    WPKH = 2 #: Native segwit address type. P2WPKH for single sig, P2WPSH for scripts.
-    SH_WPKH = 3 #: Nested segwit address type. P2SH-P2WPKH for single sig, P2SH-P2WPSH for scripts.
+    LEGACY = 1 #: Legacy address type. P2PKH for single sig, P2SH for scripts.
+    WIT = 2 #: Native segwit v0 address type. P2WPKH for single sig, P2WPSH for scripts.
+    SH_WIT = 3 #: Nested segwit v0 address type. P2SH-P2WPKH for single sig, P2SH-P2WPSH for scripts.
 
     def __str__(self) -> str:
         return self.name.lower()

hwilib/devices/bitbox02.py

@@ -451,18 +451,20 @@ def display_singlesig_address(
         bip32_path: str,
         addr_type: AddressType,
     ) -> str:
-        if addr_type == AddressType.SH_WPKH:
+        if addr_type == AddressType.SH_WIT:
             script_config = bitbox02.btc.BTCScriptConfig(
                 simple_type=bitbox02.btc.BTCScriptConfig.P2WPKH_P2SH
             )
-        elif addr_type == AddressType.WPKH:
+        elif addr_type == AddressType.WIT:
             script_config = bitbox02.btc.BTCScriptConfig(
                 simple_type=bitbox02.btc.BTCScriptConfig.P2WPKH
             )
-        else:
+        elif addr_type == AddressType.LEGACY:
             raise UnavailableActionError(
                 "The BitBox02 does not support legacy p2pkh addresses"
             )
+        else:
+            raise BadArgumentError("Unknown address type")
         address = self.init().btc_address(
             parse_path(bip32_path),
             coin=self._get_coin(),
@@ -492,9 +494,9 @@ def display_multisig_address(
             key_origin_infos[pk.extkey.serialize()] = pk.origin
             keypaths[pk.extkey.serialize()] = pk.get_full_derivation_path(0)
 
-        if addr_type == AddressType.SH_WPKH:
+        if addr_type == AddressType.SH_WIT:
             script_type = bitbox02.btc.BTCScriptConfig.Multisig.P2WSH_P2SH
-        elif addr_type == AddressType.WPKH:
+        elif addr_type == AddressType.WIT:
             script_type = bitbox02.btc.BTCScriptConfig.Multisig.P2WSH
         else:
             raise BadArgumentError(

hwilib/devices/coldcard.py

@@ -235,12 +235,14 @@ def display_singlesig_address(
         keypath = keypath.replace('h', '\'')
         keypath = keypath.replace('H', '\'')
 
-        if addr_type == AddressType.SH_WPKH:
+        if addr_type == AddressType.SH_WIT:
             addr_fmt = AF_P2WPKH_P2SH
-        elif addr_type == AddressType.WPKH:
+        elif addr_type == AddressType.WIT:
             addr_fmt = AF_P2WPKH
-        else:
+        elif addr_type == AddressType.LEGACY:
             addr_fmt = AF_CLASSIC
+        else:
+            raise BadArgumentError("Unknown address type")
 
         payload = CCProtocolPacker.show_address(keypath, addr_fmt=addr_fmt)
 
@@ -260,12 +262,14 @@ def display_multisig_address(
     ) -> str:
         self.device.check_mitm()
 
-        if addr_type == AddressType.SH_WPKH:
+        if addr_type == AddressType.SH_WIT:
             addr_fmt = AF_P2WSH_P2SH
-        elif addr_type == AddressType.WPKH:
+        elif addr_type == AddressType.WIT:
             addr_fmt = AF_P2WSH
-        else:
+        elif addr_type == AddressType.LEGACY:
             addr_fmt = AF_P2SH
+        else:
+            raise BadArgumentError("Unknown address type")
 
         if not 1 <= len(pubkeys) <= 15:
             raise BadArgumentError("Must provide 1 to 15 keypaths to display a multisig address")

hwilib/devices/ledger.py

@@ -352,8 +352,8 @@ def display_singlesig_address(
     ) -> str:
         if not check_keypath(keypath):
             raise BadArgumentError("Invalid keypath")
-        p2sh_p2wpkh = addr_type == AddressType.SH_WPKH
-        bech32 = addr_type == AddressType.WPKH
+        p2sh_p2wpkh = addr_type == AddressType.SH_WIT
+        bech32 = addr_type == AddressType.WIT
         output = self.app.getWalletPublicKey(keypath[2:], True, p2sh_p2wpkh or bech32, bech32)
         assert isinstance(output["address"], str)
         return output['address'][12:-2] # HACK: A bug in getWalletPublicKey results in the address being returned as the string "bytearray(b'<address>')". This extracts the actual address to work around this.

hwilib/devices/trezor.py

@@ -539,12 +539,14 @@ def display_singlesig_address(
         self._check_unlocked()
 
         # Script type
-        if addr_type == AddressType.SH_WPKH:
+        if addr_type == AddressType.SH_WIT:
             script_type = messages.InputScriptType.SPENDP2SHWITNESS
-        elif addr_type == AddressType.WPKH:
+        elif addr_type == AddressType.WIT:
             script_type = messages.InputScriptType.SPENDWITNESS
-        else:
+        elif addr_type == AddressType.LEGACY:
             script_type = messages.InputScriptType.SPENDADDRESS
+        else:
+            raise BadArgumentError("Unknown address type")
 
         expanded_path = parse_path(keypath)
 
@@ -586,12 +588,14 @@ def display_multisig_address(
         multisig = messages.MultisigRedeemScriptType(m=threshold, signatures=[b''] * len(pubkey_objs), pubkeys=pubkey_objs)
 
         # Script type
-        if addr_type == AddressType.SH_WPKH:
+        if addr_type == AddressType.SH_WIT:
             script_type = messages.InputScriptType.SPENDP2SHWITNESS
-        elif addr_type == AddressType.WPKH:
+        elif addr_type == AddressType.WIT:
             script_type = messages.InputScriptType.SPENDWITNESS
-        else:
+        elif addr_type == AddressType.LEGACY:
             script_type = messages.InputScriptType.SPENDMULTISIG
+        else:
+            raise BadArgumentError("Unknown address type")
 
         for p in pubkeys:
             keypath = p.origin.get_derivation_path() if p.origin is not None else "m/"