Address numerous TODOs and questions from BIP-85 (#8)

* add WIF unit test, clear up checksum confusion

Makefile

@@ -54,4 +54,3 @@ git-unsaved:
 		echo "There are unsaved changes in the git repository."; \
 		exit 1; \
 	fi
-

README.md

@@ -249,7 +249,7 @@ generalized BIP-32 paths
 
 # TODO
 
-* [ ] File the above and other "TODO" issues to BIP-85
+* [x] File the above and other "TODO" issues to BIP-85
 * [ ] Investigate switch to secure ECDSA libs with constant-time programming and
 side-channel resistance.
     * [ ] https://cryptography.io/en/latest/

src/bipsea/bip85.py

@@ -5,6 +5,7 @@
 from typing import Dict, Union
 
 import base58
+from ecdsa import SECP256k1
 
 from .bip32 import VERSIONS, ExtendedKey
 from .bip32 import derive_key as derive_key_bip32
@@ -53,7 +54,7 @@ def apply_85(derived_key: ExtendedKey, path: str) -> Dict[str, Union[bytes, str]
     if application == "39'":
         language, n_words = indexes[:2]
         if not language == LANGUAGE_CODES["English"]:
-            raise ValueError(f"Only English BIP39 words from BIP85 are supported.")
+            raise ValueError(f"Only English BIP-39 words from BIP-85 are supported.")
         if not n_words in CODE_39_TO_BITS:
             raise ValueError(f"Expected word codes {CODE_39_TO_BITS.keys()}")
         n_bytes = CODE_39_TO_BITS[n_words] // 8
@@ -65,25 +66,22 @@ def apply_85(derived_key: ExtendedKey, path: str) -> Dict[str, Union[bytes, str]
         }
     # WIF
     elif application == "2'":
+        # https://en.bitcoin.it/wiki/Wallet_import_format
         trimmed_entropy = entropy[: 256 // 8]
         prefix = b"\x80" if derived_key.get_network() == "mainnet" else b"\xef"
         suffix = b"\x01"  # use with compressed public keys because BIP32
         extended = prefix + trimmed_entropy + suffix
         hash1 = hashlib.sha256(extended).digest()
         hash2 = hashlib.sha256(hash1).digest()
+        checksum = hash2[:4]
 
         return {
             "entropy": trimmed_entropy,
-            "application": base58.b58encode_check(extended),
-            "checksum": hash2[:4],
+            "application": base58.b58encode(extended + checksum).decode("utf-8"),
         }
     # XPRV
     elif application == "32'":
         derived_key = ExtendedKey(
-            # TODO: file against bip85 that there is no provision to specify
-            # main vs testnet
-            # TODO: file against bip85 that they are inconsistent with
-            # hmac entropy order :shrug:
             version=VERSIONS["mainnet"]["private"],
             depth=bytes(1),
             finger=bytes(4),
@@ -93,12 +91,6 @@ def apply_85(derived_key: ExtendedKey, path: str) -> Dict[str, Union[bytes, str]
         )
 
         return {
-            # TODO: also file against bip85 that there is no consistency about
-            # returned entropy length in test vectors?
-            # TODO: this is wrong on multiple levels; first we use
-            # 64 bytes from the entropy for this application
-            # second this isn't even the chain_code which in some universe
-            # might be considered derived entropy :(
             "entropy": entropy[32:],
             "application": str(derived_key),
         }
@@ -112,7 +104,6 @@ def apply_85(derived_key: ExtendedKey, path: str) -> Dict[str, Union[bytes, str]
     # PWD BASE64
     elif application == "707764'":
         pwd_len = int(indexes[0][:-1])
-        # TODO file Base64 typo in 85 "encode the all 64 bytes of entropy".
         if not (20 <= pwd_len <= 86):
             raise ValueError(f"Expected pwd_len in [20, 86], got {pwd_len}")
 
@@ -168,3 +159,11 @@ def split_and_validate(path: str):
         raise ValueError(f"Unexpected path segments: {path}")
 
     return segments
+
+
+def validate_key(entropy: bytes):
+    """per BIP-85 we should hard fail under these conditions"""
+    assert len(entropy) == 32
+    int_key = int.from_bytes(entropy, "big")
+    if not int_key or int_key > SECP256k1.order:
+        raise ValueError("Invalid derived key. Try again with next child index.")

src/bipsea/bipsea.py

@@ -40,7 +40,7 @@
     "hex": "128169'",
     "words": "39'",
     "wif": "2'",
-    "xprv": "32'",  # TODO file to 85 is there a testnet here
+    "xprv": "32'",
 }
 
 RANGES = {
@@ -230,9 +230,7 @@ def bip85(application, number, index, input):
         path += f"/{number}'/{index}'"
         check_range(number, application)
     elif application == "drng":
-        # TODO file to 85: not clear structure if master root keys; is it {0'}/{index}'?
         path += f"/0'/{index}'"
-    # TODO do we need to derive testnet?
     derived = derive(master, path)
     if application == "drng":
         drng = DRNG(to_entropy(derived.data[1:]))

src/bipsea/seedwords.py

@@ -1,10 +1,5 @@
 #!/usr/bin/python
-"""Complete implementation of BIP-39 in Python with CLI
-https://en.bitcoin.it/wiki/BIP_0039
-
-TODO: CLI design:
-(xprv or seed or entropy) | derivation path > output?
-"""
+"""Complete BIP-39 implementation"""
 
 import hashlib
 import logging
@@ -16,7 +11,7 @@
 
 import click
 
-from .util import LOGGER, from_hex
+from .util import LOGGER
 
 logger = logging.getLogger(LOGGER)
 

src/bipsea/util.py

@@ -16,10 +16,6 @@
 logger = logging.getLogger(LOGGER)
 
 
-def from_hex(input: str, passphrase: str = "") -> bytes:
-    return bytes.fromhex(input + passphrase)
-
-
 def to_hex_string(data: bytes) -> str:
     return binascii.hexlify(data).decode("utf-8")
 

tests/test_bip32.py

@@ -6,7 +6,7 @@
 from bipsea.bip32 import to_master_key
 from bipsea.bip32types import parse_ext_key
 from bipsea.bip85 import derive
-from bipsea.util import LOGGER, from_hex
+from bipsea.util import LOGGER
 
 logger = logging.getLogger(LOGGER)
 
@@ -19,7 +19,7 @@
     ],
 )
 def test_vectors(vector):
-    seed = from_hex(vector["seed_hex"])
+    seed = bytes.fromhex(vector["seed_hex"])
     for ch, tests in vector["chain"].items():
         for type_, expected in tests.items():
             assert type_ in ("ext pub", "ext prv")

tests/test_bip39.py

@@ -10,7 +10,7 @@
 
 from bipsea.bip32 import to_master_key
 from bipsea.seedwords import DICT_HASH, N_MNEMONICS, entropy_to_words, to_master_seed
-from bipsea.util import LOGGER, from_hex
+from bipsea.util import LOGGER
 
 logger = logging.getLogger(LOGGER)
 
@@ -24,7 +24,7 @@ def test_vectors(language, vectors):
     for vector in vectors:
         _, mnemonic, seed, xprv = vector
         expected_words = re.split(r"\s", mnemonic)
-        expected_seed = from_hex(seed)
+        expected_seed = bytes.fromhex(seed)
         computed_seed = to_master_seed(expected_words, passphrase="TREZOR")
         assert expected_seed == computed_seed
         computed_xprv = to_master_key(expected_seed, mainnet=True, private=True)
@@ -39,7 +39,7 @@ def test_seed_word_generation(language, vectors):
         entropy_str, mnemonic, seed, xprv = vector
         expected_words = re.split(r"\s", mnemonic)
         if language == "english":
-            entropy_bytes = from_hex(entropy_str)
+            entropy_bytes = bytes.fromhex(entropy_str)
             if all(b == 0 for b in entropy_bytes):
                 warnings.simplefilter("ignore")
             computed_words = entropy_to_words(

tests/test_bip85.py

@@ -1,5 +1,7 @@
 import logging
+from hashlib import sha256
 
+import base58
 import pytest
 from data.bip85_vectors import (
     BIP39,
@@ -41,6 +43,11 @@ def test_pwd_base64(vector):
     path = vector["path"]
     output = apply_85(derive(master, path), path)
     assert vector["derived_pwd"] == output["application"]
+    # Hardcode what we believe is correct; issue filed to BIP85
+    assert (
+        to_hex_string(output["entropy"])
+        == "74a2e87a9ba0cdd549bdd2f9ea880d554c6c355b08ed25088cfa88f3f1c4f74632b652fd4a8f5fda43074c6f6964a3753b08bb5210c8f5e75c07a4c2a20bf6e9"
+    )
 
 
 @pytest.mark.parametrize("vector", PWD_BASE64)
@@ -95,9 +102,7 @@ def test_wif(vector):
     path = vector["path"]
     output = apply_85(derive(master, path), path)
     assert to_hex_string(output["entropy"]) == vector["derived_entropy"]
-    # TODO: file against BIP85 poor test case does not include WIF checksum
-    # (not a correct WIF)
-    assert output["application"].decode("utf-8") == vector["derived_wif"]
+    assert output["application"] == vector["derived_wif"]
 
 
 @pytest.mark.parametrize("vector", XPRV)
@@ -107,3 +112,20 @@ def test_xprv(vector):
     output = apply_85(derive(master, path), path)
     assert vector["derived_key"] == output["application"]
     assert to_hex_string(output["entropy"]) == vector["derived_entropy"]
+
+
+def test_private_key_to_wif():
+    """https://en.bitcoin.it/wiki/Wallet_import_format"""
+
+
+def test_private_key_to_wif():
+    pkey_hex = "0C28FCA386C7A227600B2FE50B7CAE11EC86D3BF1FBE471BE89827E19D72AA1D"
+    pkey = bytes.fromhex(pkey_hex)
+    extended = b"\x80" + pkey
+    hash1 = sha256(extended).digest()
+    hash2 = sha256(hash1).digest()
+    checksum = hash2[:4]
+    # they say "Base58Check encoding" but that doesn't mean
+    # b58encode_check because we already have a checksum apparently
+    wif = base58.b58encode(extended + checksum)
+    assert wif.decode("utf-8") == "5HueCGU8rMjxEXxiPuD5BDku4MkFqeZyd4dZ1jvhTVqvbTLvyTJ"