diff options
-rw-r--r-- | src/key.cpp | 1 | ||||
-rw-r--r-- | src/key.h | 4 | ||||
-rw-r--r-- | src/pubkey.cpp | 1 | ||||
-rw-r--r-- | src/pubkey.h | 4 | ||||
-rw-r--r-- | src/script/descriptor.cpp | 11 | ||||
-rw-r--r-- | src/test/bip32_tests.cpp | 18 | ||||
-rw-r--r-- | src/test/descriptor_tests.cpp | 4 | ||||
-rw-r--r-- | src/wallet/scriptpubkeyman.cpp | 15 |
8 files changed, 42 insertions, 16 deletions
diff --git a/src/key.cpp b/src/key.cpp index 9b0971a2dd..199808505d 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -333,6 +333,7 @@ bool CKey::Derive(CKey& keyChild, ChainCode &ccChild, unsigned int nChild, const } bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const { + if (nDepth == std::numeric_limits<unsigned char>::max()) return false; out.nDepth = nDepth + 1; CKeyID id = key.GetPubKey().GetID(); memcpy(out.vchFingerprint, &id, 4); @@ -146,7 +146,7 @@ public: bool SignSchnorr(const uint256& hash, Span<unsigned char> sig, const uint256* merkle_root, const uint256& aux) const; //! Derive BIP32 child key. - bool Derive(CKey& keyChild, ChainCode &ccChild, unsigned int nChild, const ChainCode& cc) const; + [[nodiscard]] bool Derive(CKey& keyChild, ChainCode &ccChild, unsigned int nChild, const ChainCode& cc) const; /** * Verify thoroughly whether a private key and a public key match. @@ -176,7 +176,7 @@ struct CExtKey { void Encode(unsigned char code[BIP32_EXTKEY_SIZE]) const; void Decode(const unsigned char code[BIP32_EXTKEY_SIZE]); - bool Derive(CExtKey& out, unsigned int nChild) const; + [[nodiscard]] bool Derive(CExtKey& out, unsigned int nChild) const; CExtPubKey Neuter() const; void SetSeed(Span<const std::byte> seed); }; diff --git a/src/pubkey.cpp b/src/pubkey.cpp index a4a1be6388..2e37e16690 100644 --- a/src/pubkey.cpp +++ b/src/pubkey.cpp @@ -365,6 +365,7 @@ void CExtPubKey::DecodeWithVersion(const unsigned char code[BIP32_EXTKEY_WITH_VE } bool CExtPubKey::Derive(CExtPubKey &out, unsigned int _nChild) const { + if (nDepth == std::numeric_limits<unsigned char>::max()) return false; out.nDepth = nDepth + 1; CKeyID id = pubkey.GetID(); memcpy(out.vchFingerprint, &id, 4); diff --git a/src/pubkey.h b/src/pubkey.h index 463efe1b00..0485a38f72 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -218,7 +218,7 @@ public: bool Decompress(); //! Derive BIP32 child pubkey. - bool Derive(CPubKey& pubkeyChild, ChainCode &ccChild, unsigned int nChild, const ChainCode& cc) const; + [[nodiscard]] bool Derive(CPubKey& pubkeyChild, ChainCode &ccChild, unsigned int nChild, const ChainCode& cc) const; }; class XOnlyPubKey @@ -327,7 +327,7 @@ struct CExtPubKey { void Decode(const unsigned char code[BIP32_EXTKEY_SIZE]); void EncodeWithVersion(unsigned char code[BIP32_EXTKEY_WITH_VERSION_SIZE]) const; void DecodeWithVersion(const unsigned char code[BIP32_EXTKEY_WITH_VERSION_SIZE]); - bool Derive(CExtPubKey& out, unsigned int nChild) const; + [[nodiscard]] bool Derive(CExtPubKey& out, unsigned int nChild) const; }; /** Users of this module must hold an ECCVerifyHandle. The constructor and diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index db386c9ab8..9bcbe1ceef 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -328,7 +328,7 @@ class BIP32PubkeyProvider final : public PubkeyProvider { if (!GetExtKey(arg, xprv)) return false; for (auto entry : m_path) { - xprv.Derive(xprv, entry); + if (!xprv.Derive(xprv, entry)) return false; if (entry >> 31) { last_hardened = xprv; } @@ -388,14 +388,13 @@ public: } } else { for (auto entry : m_path) { - der = parent_extkey.Derive(parent_extkey, entry); - assert(der); + if (!parent_extkey.Derive(parent_extkey, entry)) return false; } final_extkey = parent_extkey; if (m_derive == DeriveType::UNHARDENED) der = parent_extkey.Derive(final_extkey, pos); assert(m_derive != DeriveType::HARDENED); } - assert(der); + if (!der) return false; final_info_out = final_info_out_tmp; key_out = final_extkey.pubkey; @@ -498,8 +497,8 @@ public: CExtKey extkey; CExtKey dummy; if (!GetDerivedExtKey(arg, extkey, dummy)) return false; - if (m_derive == DeriveType::UNHARDENED) extkey.Derive(extkey, pos); - if (m_derive == DeriveType::HARDENED) extkey.Derive(extkey, pos | 0x80000000UL); + if (m_derive == DeriveType::UNHARDENED && !extkey.Derive(extkey, pos)) return false; + if (m_derive == DeriveType::HARDENED && !extkey.Derive(extkey, pos | 0x80000000UL)) return false; key = extkey.key; return true; } diff --git a/src/test/bip32_tests.cpp b/src/test/bip32_tests.cpp index 64cc924239..75b29ae0aa 100644 --- a/src/test/bip32_tests.cpp +++ b/src/test/bip32_tests.cpp @@ -184,4 +184,22 @@ BOOST_AUTO_TEST_CASE(bip32_test5) { } } +BOOST_AUTO_TEST_CASE(bip32_max_depth) { + CExtKey key_parent{DecodeExtKey(test1.vDerive[0].prv)}, key_child; + CExtPubKey pubkey_parent{DecodeExtPubKey(test1.vDerive[0].pub)}, pubkey_child; + + // We can derive up to the 255th depth.. + for (auto i = 0; i++ < 255;) { + BOOST_CHECK(key_parent.Derive(key_child, 0)); + std::swap(key_parent, key_child); + BOOST_CHECK(pubkey_parent.Derive(pubkey_child, 0)); + std::swap(pubkey_parent, pubkey_child); + } + + // But trying to derive a non-existent 256th depth will fail! + BOOST_CHECK(key_parent.nDepth == 255 && pubkey_parent.nDepth == 255); + BOOST_CHECK(!key_parent.Derive(key_child, 0)); + BOOST_CHECK(!pubkey_parent.Derive(pubkey_child, 0)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index a8c666079d..8a17472cda 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -233,7 +233,7 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string& for (const auto& xpub_pair : parent_xpub_cache) { const CExtPubKey& xpub = xpub_pair.second; CExtPubKey der; - xpub.Derive(der, i); + BOOST_CHECK(xpub.Derive(der, i)); pubkeys.insert(der.pubkey); } int count_pks = 0; @@ -265,7 +265,7 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string& const CExtPubKey& xpub = xpub_pair.second; pubkeys.insert(xpub.pubkey); CExtPubKey der; - xpub.Derive(der, i); + BOOST_CHECK(xpub.Derive(der, i)); pubkeys.insert(der.pubkey); } int count_pks = 0; diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 9d88b4632e..e2cf081259 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1080,6 +1080,13 @@ CPubKey LegacyScriptPubKeyMan::GenerateNewKey(WalletBatch &batch, CHDChain& hd_c return pubkey; } +//! Try to derive an extended key, throw if it fails. +static void DeriveExtKey(CExtKey& key_in, unsigned int index, CExtKey& key_out) { + if (!key_in.Derive(key_out, index)) { + throw std::runtime_error("Could not derive extended key"); + } +} + void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& metadata, CKey& secret, CHDChain& hd_chain, bool internal) { // for now we use a fixed keypath scheme of m/0'/0'/k @@ -1097,11 +1104,11 @@ void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& // derive m/0' // use hardened derivation (child keys >= 0x80000000 are hardened after bip32) - masterKey.Derive(accountKey, BIP32_HARDENED_KEY_LIMIT); + DeriveExtKey(masterKey, BIP32_HARDENED_KEY_LIMIT, accountKey); // derive m/0'/0' (external chain) OR m/0'/1' (internal chain) assert(internal ? m_storage.CanSupportFeature(FEATURE_HD_SPLIT) : true); - accountKey.Derive(chainChildKey, BIP32_HARDENED_KEY_LIMIT+(internal ? 1 : 0)); + DeriveExtKey(accountKey, BIP32_HARDENED_KEY_LIMIT+(internal ? 1 : 0), chainChildKey); // derive child key at next index, skip keys already known to the wallet do { @@ -1109,7 +1116,7 @@ void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& // childIndex | BIP32_HARDENED_KEY_LIMIT = derive childIndex in hardened child-index-range // example: 1 | BIP32_HARDENED_KEY_LIMIT == 0x80000001 == 2147483649 if (internal) { - chainChildKey.Derive(childKey, hd_chain.nInternalChainCounter | BIP32_HARDENED_KEY_LIMIT); + DeriveExtKey(chainChildKey, hd_chain.nInternalChainCounter | BIP32_HARDENED_KEY_LIMIT, childKey); metadata.hdKeypath = "m/0'/1'/" + ToString(hd_chain.nInternalChainCounter) + "'"; metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); metadata.key_origin.path.push_back(1 | BIP32_HARDENED_KEY_LIMIT); @@ -1117,7 +1124,7 @@ void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& hd_chain.nInternalChainCounter++; } else { - chainChildKey.Derive(childKey, hd_chain.nExternalChainCounter | BIP32_HARDENED_KEY_LIMIT); + DeriveExtKey(chainChildKey, hd_chain.nExternalChainCounter | BIP32_HARDENED_KEY_LIMIT, childKey); metadata.hdKeypath = "m/0'/0'/" + ToString(hd_chain.nExternalChainCounter) + "'"; metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); |