Merge bitcoin/bitcoin#25642: Don't wrap around when deriving an extended key at a too large depth

fb9faffae3a26b8aed8b671864ba679747163019 extended keys: fail to derive too large depth instead of wrapping around (Antoine Poinsot)
8dc6670ce159c2b080e9f735c6603a601d40b6ac descriptor: don't assert success of extended key derivation (Antoine Poinsot)
50cfc9e7613d6cf6b534df6e551238b80678c70d (pubk)key: mark Derive() as nodiscard (Antoine Poinsot)
0ca258a5ace798c4e54308aa8a09b1ab3302cd7e descriptor: never ignore the return value when deriving an extended key (Antoine Poinsot)
d3599c22bd4c6b3cfaaadd675e95ebe3b3cb1749 spkman: don't ignore the return value when deriving an extended key (Antoine Poinsot)

Pull request description:

  We would previously  silently wrap the derived child's depth back to `0`. Instead, explicitly fail when trying to derive an impossible depth, and handle the error in callers.

  An extended fuzzing corpus of `descriptor_parse` triggered this behaviour, which was reported by MarcoFalke.

  Fixes #25751.

ACKs for top commit:
  achow101:
    re-ACK fb9faffae3a26b8aed8b671864ba679747163019
  instagibbs:
    utACK  https://github.com/bitcoin/bitcoin/pull/25642/commits/fb9faffae3a26b8aed8b671864ba679747163019

Tree-SHA512: 9f75c23572ce847239bd15e5497df2960b6bd63c61ea72347959d968b5c4c9a4bfeee284e76bdcd7bacbf9eeb70feee85ffd3e316f353ca6eca30e93aafad343

2 files changed, 20 insertions, 2 deletions

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;