Merge #17681: wallet: Keep inactive seeds after sethdseed and derive keys from them as needed

1ed52fbb4d81f7b7634fd4fb6d1d00e1478129dc Remove IBD check in sethdseed (Andrew Chow)
b1810a145a601a8064e4094350cfb6ddafbdb4d8 Test that keys from inactive seeds are generated (Andrew Chow)
c93082ece40b1c72f05b3e2085c022c09eaa4d65 Generate new keys for inactive seeds after marking used (Andrew Chow)
45f2f6a0e8514a0438a87554400bf73cbb90707f Determine inactive HD seeds from key metadata and track them in LegacyScriptPubKeyMan (Andrew Chow)
b59b4504abf96cec860badfed2ac793ae5d40ced have GenerateNewKey and DeriveNewChildKey take a CHDChain as an argument (Andrew Chow)

Pull request description:

  Largely implements the suggestion from https://github.com/bitcoin/bitcoin/pull/17484#issuecomment-560845316.

  After `sethdseed` is called, the CHDChain for the old seed is kept in the wallet. It is kept on the file as a new `inactivehdseed` record and in memory in a map `m_inactive_hd_seeds`. In `LegacyScriptPubKeyMan::MarkUnusedAddresses` we check each used key's metadata for whether it was derived from an inactive seed. If it is, we then check to see how many keys after that key were derived from the inactive seed. If that number does not match the keypool parameter, we derive more keys from the inactive seed until it does match. This way we won't miss transactions belonging to keys outside of the range of the keypool initially.

  The indexes and internal-ness of a key is gotten by checking it's key origin data.

  Because of this change, we no longer need to wait for IBD to finish before `sethdseed` can work so that check is also removed.

  A test case for this is added as well which fails on master.

ACKs for top commit:
  ryanofsky:
    Code review ACK 1ed52fbb4d81f7b7634fd4fb6d1d00e1478129dc. Changes since last review: various commit message, code comment, log message, error checking improvements, and fix for topping up inactive seeds if wallet isn't reloaded after calling sethdseed and test for this
  ariard:
    Code Review ACK 1ed52fb
  jonatack:
    ACK 1ed52fbb4d81f7 thanks for addressing the previous review feedback; would be happy to see the new review questions answered and feedback addressed and re-ack.

Tree-SHA512: e658ae0e1dab94be55d2b62cdda506c94815e73a6881533fd30d41cc77477f82fee2095144957a3a1df0c129e256bdd7b7abe3737d515f393610446cae4edf1c

1 files changed, 31 insertions, 4 deletions

diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
index 7e15042950..c62c970235 100644
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -18,6 +18,8 @@
 
 #include <boost/signals2/signal.hpp>
 
+#include <unordered_map>
+
 enum class OutputType;
 struct bilingual_str;
 
@@ -144,6 +146,17 @@ public:
     }
 };
 
+class KeyIDHasher
+{
+public:
+    KeyIDHasher() {}
+
+    size_t operator()(const CKeyID& id) const
+    {
+        return id.GetUint64(0);
+    }
+};
+
 /*
  * A class implementing ScriptPubKeyMan manages some (or all) scriptPubKeys used in a wallet.
  * It contains the scripts and keys related to the scriptPubKeys it manages.
@@ -288,10 +301,11 @@ private:
     bool AddKeyOriginWithDB(WalletBatch& batch, const CPubKey& pubkey, const KeyOriginInfo& info);
 
     /* the HD chain data model (external chain counters) */
-    CHDChain hdChain;
+    CHDChain m_hd_chain;
+    std::unordered_map<CKeyID, CHDChain, KeyIDHasher> m_inactive_hd_chains;
 
     /* HD derive new child key (on internal or external chain) */
-    void DeriveNewChildKey(WalletBatch& batch, CKeyMetadata& metadata, CKey& secret, bool internal = false) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);
+    void DeriveNewChildKey(WalletBatch& batch, CKeyMetadata& metadata, CKey& secret, CHDChain& hd_chain, bool internal = false) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);
 
     std::set<int64_t> setInternalKeyPool GUARDED_BY(cs_KeyStore);
     std::set<int64_t> setExternalKeyPool GUARDED_BY(cs_KeyStore);
@@ -320,6 +334,18 @@ private:
      */
     bool ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal);
 
+    /**
+     * Like TopUp() but adds keys for inactive HD chains.
+     * Ensures that there are at least -keypool number of keys derived after the given index.
+     *
+     * @param seed_id the CKeyID for the HD seed.
+     * @param index the index to start generating keys from
+     * @param internal whether the internal chain should be used. true for internal chain, false for external chain.
+     *
+     * @return true if seed was found and keys were derived. false if unable to derive seeds
+     */
+    bool TopUpInactiveHDChain(const CKeyID seed_id, int64_t index, bool internal);
+
 public:
     using ScriptPubKeyMan::ScriptPubKeyMan;
 
@@ -393,11 +419,12 @@ public:
     void LoadKeyMetadata(const CKeyID& keyID, const CKeyMetadata &metadata);
     void LoadScriptMetadata(const CScriptID& script_id, const CKeyMetadata &metadata);
     //! Generate a new key
-    CPubKey GenerateNewKey(WalletBatch& batch, bool internal = false) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);
+    CPubKey GenerateNewKey(WalletBatch& batch, CHDChain& hd_chain, bool internal = false) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore);
 
     /* Set the HD chain model (chain child index counters) */
     void SetHDChain(const CHDChain& chain, bool memonly);
-    const CHDChain& GetHDChain() const { return hdChain; }
+    const CHDChain& GetHDChain() const { return m_hd_chain; }
+    void AddInactiveHDChain(const CHDChain& chain);
 
     //! Adds a watch-only address to the store, without saving it to disk (used by LoadWallet)
     bool LoadWatchOnly(const CScript &dest);