From d5b27702f63cbb3ed11c175ca3630b86f333ccea Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 31 Jul 2026 16:52:06 -0300 Subject: [PATCH 1/2] feat(taproot): P2TR addresses via GetAddress Second of three steps. Removes the `return 0` at transaction.c:188 and makes GetAddress return bc1p addresses for m/86' paths. - compute_address() handles SPENDTAPROOT: tweak the x-only internal key per BIP-86, then bech32m encode it at witness version 1 - path_mismatched() gains an m/86' branch, in BOTH copies (fsm_msg_coin.h and coins.c, per the keep-in-sync note above them) - three official BIP-86 vectors, driven from the published internal keys and again end to end from the mnemonic Two guards worth calling out, because both would have failed silently: - taproot multisig is rejected up front. Without it the request fell through to the p2sh branch and returned a p2sh address for a taproot ask -- a wrong address, not an error. - exactly 32 bytes are passed to segwit_addr_encode(), which only length-checks the witness program for version 0 (segwit_addr.c:182). Any other length would have encoded into a plausible-looking bc1p. Also fixes a pre-existing bug in the PAYTOTAPROOT output gate. It tested `!coin->has_taproot`, but has_taproot is the nanopb presence flag and every coin in coins.def sets it -- only the `taproot` VALUE distinguishes them. Bitcoin and Testnet have taproot=true; the other 41 coins have taproot=false and were all passing the gate, building p2tr outputs for chains that cannot spend them. Now tests the value. Device build clean (MAKE_EXIT=0, no warnings, all variants). Retained cost in firmware.keepkey.elf is 306 bytes; bip340_sign and bip340_verify still garbage-collect out until step 3 references them. 11/11 unit tests green. Depends on keepkey/trezor-firmware#6. --- deps/crypto/trezor-firmware | 2 +- lib/firmware/coins.c | 16 ++++++ lib/firmware/fsm_msg_coin.h | 15 +++++ lib/firmware/signing.c | 5 +- lib/firmware/transaction.c | 32 ++++++++++- unittests/crypto/bip340.cpp | 106 ++++++++++++++++++++++++++++++++++++ 6 files changed, 171 insertions(+), 5 deletions(-) diff --git a/deps/crypto/trezor-firmware b/deps/crypto/trezor-firmware index 45ecd8d8a..faf4f3fe3 160000 --- a/deps/crypto/trezor-firmware +++ b/deps/crypto/trezor-firmware @@ -1 +1 @@ -Subproject commit 45ecd8d8ab600a7b1aaa6428031a17042b7723a4 +Subproject commit faf4f3fe348a2c690ba6cd22b561313bcc792b98 diff --git a/lib/firmware/coins.c b/lib/firmware/coins.c index a231bc41a..964b9cf52 100644 --- a/lib/firmware/coins.c +++ b/lib/firmware/coins.c @@ -228,6 +228,22 @@ static bool path_mismatched(const CoinType* coin, const uint32_t* address_n, return mismatch; } + // m/86' : BIP86 Taproot + // m / purpose' / bip44_account_path' / account' / change / address_index + if (address_n[0] == (0x80000000 + 86)) { + mismatch |= !coin->has_segwit || !coin->segwit; + mismatch |= !coin->has_bech32_prefix; + mismatch |= !coin->has_taproot || !coin->taproot; + mismatch |= (address_n_count != (whole_account ? 3 : 5)); + mismatch |= (address_n[1] != coin->bip44_account_path); + mismatch |= (address_n[2] & 0x80000000) == 0; + if (!whole_account) { + mismatch |= (address_n[3] & 0x80000000) == 0x80000000; + mismatch |= (address_n[4] & 0x80000000) == 0x80000000; + } + return mismatch; + } + return false; } diff --git a/lib/firmware/fsm_msg_coin.h b/lib/firmware/fsm_msg_coin.h index 978c80712..d1b002a1d 100644 --- a/lib/firmware/fsm_msg_coin.h +++ b/lib/firmware/fsm_msg_coin.h @@ -188,6 +188,21 @@ static bool path_mismatched(const CoinType* coin, const GetAddress* msg) { return mismatch; } + // m/86' : BIP86 Taproot + // m / purpose' / bip44_account_path' / account' / change / address_index + if (msg->address_n[0] == (0x80000000 + 86)) { + mismatch |= (msg->script_type != InputScriptType_SPENDTAPROOT); + mismatch |= !coin->has_segwit || !coin->segwit; + mismatch |= !coin->has_bech32_prefix; + mismatch |= !coin->has_taproot || !coin->taproot; + mismatch |= (msg->address_n_count != 5); + mismatch |= (msg->address_n[1] != coin->bip44_account_path); + mismatch |= (msg->address_n[2] & 0x80000000) == 0; + mismatch |= (msg->address_n[3] & 0x80000000) == 0x80000000; + mismatch |= (msg->address_n[4] & 0x80000000) == 0x80000000; + return mismatch; + } + return false; } diff --git a/lib/firmware/signing.c b/lib/firmware/signing.c index a7b367e0f..c91945b4b 100644 --- a/lib/firmware/signing.c +++ b/lib/firmware/signing.c @@ -822,8 +822,11 @@ static bool signing_validate_output(const TxOutputType* txoutput) { return false; } + // has_taproot is the nanopb presence flag, not the value. Every coin in + // coins.def sets it, so testing it alone let all 41 taproot=false coins + // through and built a p2tr output for them. if (txoutput->script_type == OutputScriptType_PAYTOTAPROOT && - !coin->has_taproot) { + (!coin->has_taproot || !coin->taproot)) { fsm_sendFailure(FailureType_Failure_Other, _("Taproot not enabled on this coin.")); signing_abort(); diff --git a/lib/firmware/transaction.c b/lib/firmware/transaction.c index 9d30f1acd..aca2490e0 100644 --- a/lib/firmware/transaction.c +++ b/lib/firmware/transaction.c @@ -32,6 +32,7 @@ #include "keepkey/transport/interface.h" #include "trezor/crypto/address.h" #include "trezor/crypto/base58.h" +#include "trezor/crypto/bip340.h" #include "trezor/crypto/cash_addr.h" #include "trezor/crypto/ecdsa.h" #include "trezor/crypto/memzero.h" @@ -43,6 +44,7 @@ #define _(X) (X) #define SEGWIT_VERSION_0 0 +#define SEGWIT_VERSION_1 1 #define CASHADDR_P2KH (0) #define CASHADDR_P2SH (8) @@ -125,6 +127,11 @@ bool compute_address(const CoinType* coin, InputScriptType script_type, if (has_multisig) { size_t prelen; + // No taproot multisig. Without this the request would fall through to + // the p2sh branch below and hand back a p2sh address for a taproot ask. + if (script_type == InputScriptType_SPENDTAPROOT) { + return 0; + } if (cryptoMultisigPubkeyIndex(coin, multisig, node->public_key) < 0) { return 0; } @@ -186,9 +193,28 @@ bool compute_address(const CoinType* coin, InputScriptType script_type, return 0; } } else if (script_type == InputScriptType_SPENDTAPROOT) { - // we don't handle spendtaproot input types - return 0; - + // p2tr: the witness program is the BIP-86 tweaked output key, bech32m + // encoded at witness version 1. + if ((!coin->has_segwit || !coin->segwit) || !coin->has_bech32_prefix) { + return 0; + } + if (!coin->has_taproot || !coin->taproot) { + return 0; + } + uint8_t output_key[32]; + // node->public_key is compressed; bytes 1..33 are the x-only internal key. + // BIP-341 defines the internal key as x-only, so the odd-y case resolves + // to its even-y counterpart here and in the signer alike. + if (bip340_tweak_pubkey(curve->params, node->public_key + 1, output_key) != + 0) { + return 0; + } + // Exactly 32 bytes: segwit_addr_encode only length-checks the witness + // program for version 0, so a wrong length would encode silently. + if (!segwit_addr_encode(address, coin->bech32_prefix, SEGWIT_VERSION_1, + output_key, sizeof(output_key))) { + return 0; + } } else if (script_type == InputScriptType_SPENDP2SHWITNESS) { // segwit p2wpkh embedded in p2sh if (!coin->has_segwit || !coin->segwit) { diff --git a/unittests/crypto/bip340.cpp b/unittests/crypto/bip340.cpp index e449dbfb8..dd157cbb5 100644 --- a/unittests/crypto/bip340.cpp +++ b/unittests/crypto/bip340.cpp @@ -8,13 +8,18 @@ #include extern "C" { +#include "trezor/crypto/bip32.h" #include "trezor/crypto/bip340.h" +#include "trezor/crypto/bip39.h" +#include "trezor/crypto/curves.h" #include "trezor/crypto/ecdsa.h" #include "trezor/crypto/secp256k1.h" +#include "trezor/crypto/segwit_addr.h" } #include "gtest/gtest.h" +#include #include #include #include @@ -191,6 +196,107 @@ const Vector kVectors[] = { } // namespace +// Official BIP-86 test vectors, from +// https://github.com/bitcoin/bips/blob/master/bip-0086.mediawiki +// (mnemonic "abandon abandon ... about", account m/86'/0'/0'). +// HD derivation is covered at the firmware level; what is pinned here is the +// tweak and the bech32m encoding that turn an internal key into an address. +TEST(BIP340, BIP86Vectors) { + const struct { + const char *path; + const char *internal_key; + const char *output_key; + const char *address; + } vectors[] = { + {"m/86'/0'/0'/0/0", + "cc8a4bc64d897bddc5fbc2f670f7a8ba0b386779106cf1223c6fc5d7cd6fc115", + "a60869f0dbcf1dc659c9cecbaf8050135ea9e8cdc487053f1dc6880949dc684c", + "bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr"}, + {"m/86'/0'/0'/0/1", + "83dfe85a3151d2517290da461fe2815591ef69f2b18a2ce63f01697a8b313145", + "a82f29944d65b86ae6b5e5cc75e294ead6c59391a1edc5e016e3498c67fc7bbb", + "bc1p4qhjn9zdvkux4e44uhx8tc55attvtyu358kutcqkudyccelu0was9fqzwh"}, + {"m/86'/0'/0'/1/0", + "399f1b2f4393f29a18c937859c5dd8a77350103157eb880f02e8c08214277cef", + "882d74e5d0572d5a816cef0041a96b6c1de832f6f9676d9605c44d5e9a97d3dc", + "bc1p3qkhfews2uk44qtvauqyr2ttdsw7svhkl9nkm9s9c3x4ax5h60wqwruhk7"}, + }; + + for (const auto &v : vectors) { + std::vector internal = unhex(v.internal_key); + uint8_t out[BIP340_XONLY_LENGTH] = {0}; + + ASSERT_EQ(0, bip340_tweak_pubkey(&secp256k1, internal.data(), out)) + << v.path; + + std::string got = hex(out, sizeof(out)); + std::transform(got.begin(), got.end(), got.begin(), ::tolower); + ASSERT_EQ(std::string(v.output_key), got) << v.path; + + // Witness version 1 + 32 bytes must come out bech32m, i.e. a bc1p address. + char address[MAX_ADDR_SIZE] = {0}; + ASSERT_EQ(1, segwit_addr_encode(address, "bc", 1, out, sizeof(out))) + << v.path; + ASSERT_EQ(std::string(v.address), std::string(address)) << v.path; + } +} + +// The same BIP-86 vectors driven from the mnemonic, so HD derivation and the +// x-only convention are covered too. compute_address() feeds +// node->public_key + 1 to bip340_tweak_pubkey(); this pins that the byte after +// the compressed prefix really is the internal key BIP-86 expects, which an +// off-by-one would otherwise turn into a valid-looking wrong address. +TEST(BIP340, BIP86FromMnemonic) { + const char *mnemonic = + "abandon abandon abandon abandon abandon abandon abandon abandon abandon " + "abandon abandon about"; + const struct { + uint32_t change; + uint32_t index; + const char *address; + } vectors[] = { + {0, 0, "bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr"}, + {0, 1, "bc1p4qhjn9zdvkux4e44uhx8tc55attvtyu358kutcqkudyccelu0was9fqzwh"}, + {1, 0, "bc1p3qkhfews2uk44qtvauqyr2ttdsw7svhkl9nkm9s9c3x4ax5h60wqwruhk7"}, + }; + + uint8_t seed[64] = {0}; + mnemonic_to_seed(mnemonic, "", seed, nullptr); + + for (const auto &v : vectors) { + HDNode node = {0}; + ASSERT_EQ(1, hdnode_from_seed(seed, sizeof(seed), SECP256K1_NAME, &node)); + // m/86'/0'/0'/change/index + ASSERT_EQ(1, hdnode_private_ckd(&node, 0x80000000 + 86)); + ASSERT_EQ(1, hdnode_private_ckd(&node, 0x80000000 + 0)); + ASSERT_EQ(1, hdnode_private_ckd(&node, 0x80000000 + 0)); + ASSERT_EQ(1, hdnode_private_ckd(&node, v.change)); + ASSERT_EQ(1, hdnode_private_ckd(&node, v.index)); + hdnode_fill_public_key(&node); + + uint8_t out[BIP340_XONLY_LENGTH] = {0}; + ASSERT_EQ(0, bip340_tweak_pubkey(&secp256k1, node.public_key + 1, out)); + + char address[MAX_ADDR_SIZE] = {0}; + ASSERT_EQ(1, segwit_addr_encode(address, "bc", 1, out, sizeof(out))); + ASSERT_EQ(std::string(v.address), std::string(address)) + << "change=" << v.change << " index=" << v.index; + } +} + +TEST(BIP340, TweakRejectsInvalidInternalKey) { + // Vector 5's x coordinate, which is not on the curve. + std::vector bad = + unhex("EEFDEA4CDB677750A420FEE807EACF21EB9898AE79B9768766E4FAA04A2D4A34"); + // And an x coordinate past the field size (vector 14). + std::vector too_big = + unhex("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC30"); + uint8_t out[BIP340_XONLY_LENGTH] = {0}; + + ASSERT_NE(0, bip340_tweak_pubkey(&secp256k1, bad.data(), out)); + ASSERT_NE(0, bip340_tweak_pubkey(&secp256k1, too_big.data(), out)); +} + TEST(BIP340, TaggedHash) { // tagged_hash("BIP0340/challenge", "") == SHA256(h || h) where // h = SHA256("BIP0340/challenge"). Pins the double-tag construction. From 1030ed4ffe906a376edd26ca71c6d65ad613615a Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 31 Jul 2026 17:22:27 -0300 Subject: [PATCH 2/2] test(taproot): pin python-keepkey at the BIP-86 GetAddress test Bumps deps/python-keepkey by one commit to pick up tests/test_msg_getaddress_taproot.py (BitHighlander/python-keepkey#28), which drives the emulator through the full SPENDTAPROOT GetAddress path and asserts the three official BIP-86 addresses. Verified against a locally built kkemu: 1 passed, and mutation checked so the assertions are known not to be vacuous. The test gates on firmware 7.16.0 and CMakeLists is 7.15.0, so it SKIPS until the project version bumps. Deliberate: gating at 7.15.0 would make released 7.15.0-rcN firmware without taproot fail rather than skip. --- deps/python-keepkey | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/python-keepkey b/deps/python-keepkey index d88a073a5..7423fcfc6 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit d88a073a5af2d83e0f1f19574665ca1a44789414 +Subproject commit 7423fcfc647a4622628fe376f3faf2624a310468