Merge #16237: Have the wallet give out destinations instead of keys

8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf Add GetNewChangeDestination for getting new change Destinations (Andrew Chow)
33d13edd2bda0af90660e275ea4fa96ca9896f2a Replace CReserveKey with ReserveDestinatoin (Andrew Chow)
172213be5b174243dc501c1103ad5fe2fee67a16 Add GetNewDestination to CWallet to fetch new destinations (Andrew Chow)

Pull request description:

  The wallet should give out destinations instead of keys. It should be the one that handles the conversion from key to destination and the setting of the label, not the caller. In order to do this, two new member functions are introduced `GetNewDestination()` and `GetNewChangeDestination()`. Additionally, `CReserveKey` is changed to be `ReserveDestination` and represents destinations whose keys can be returned to the keypool.

ACKs for top commit:
  instagibbs:
    re-utACK https://github.com/bitcoin/bitcoin/pull/16237/commits/8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf
  sipa:
    ACK 8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf. Concept ACK as this gives a much cleaner abstraction to work with, and light code review ACK.
  laanwj:
    ACK 8e7f930828a9f8f9be1c90ff45e3fdfef1980eaf

Tree-SHA512: 5be7051409232b71e0ef2c1fd1a3e76964ed2f5b14d47d06edc2ad3b3687abd0be2803a1adc45c0433aa2c3bed172e14f8a7e9f4a23bff70f86260b5a0497500

1 files changed, 36 insertions, 29 deletions

diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 7b5465c219..14c373bddf 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -85,11 +85,11 @@ static constexpr size_t DUMMY_NESTED_P2WPKH_INPUT_SIZE = 91;
 
 class CCoinControl;
 class COutput;
-class CReserveKey;
 class CScript;
 class CWalletTx;
 struct FeeCalculation;
 enum class FeeEstimateMode;
+class ReserveDestination;
 
 /** (client) version numbers for particular wallet features */
 enum WalletFeature
@@ -254,55 +254,57 @@ public:
     }
 };
 
-/** A wrapper to reserve a key from a wallet keypool
+/** A wrapper to reserve an address from a wallet
  *
- * CReserveKey is used to reserve a key from the keypool. It is passed around
+ * ReserveDestination is used to reserve an address. It is passed around
  * during the CreateTransaction/CommitTransaction procedure.
  *
- * Instantiating a CReserveKey does not reserve a keypool key. To do so,
- * GetReservedKey() needs to be called on the object. Once a key has been
- * reserved, call KeepKey() on the CReserveKey object to make sure it is not
- * returned to the keypool. Call ReturnKey() to return the key to the keypool
- * so it can be re-used (for example, if the key was used in a new transaction
+ * Instantiating a ReserveDestination does not reserve an address. To do so,
+ * GetReservedDestination() needs to be called on the object. Once an address has been
+ * reserved, call KeepDestination() on the ReserveDestination object to make sure it is not
+ * returned. Call ReturnDestination() to return the address so it can be re-used (for
+ * example, if the address was used in a new transaction
  * and that transaction was not completed and needed to be aborted).
  *
- * If a key is reserved and KeepKey() is not called, then the key will be
- * returned to the keypool when the CReserveObject goes out of scope.
+ * If an address is reserved and KeepDestination() is not called, then the address will be
+ * returned when the ReserveDestination goes out of scope.
  */
-class CReserveKey
+class ReserveDestination
 {
 protected:
-    //! The wallet to reserve the keypool key from
+    //! The wallet to reserve from
     CWallet* pwallet;
-    //! The index of the key in the keypool
+    //! The index of the address's key in the keypool
     int64_t nIndex{-1};
-    //! The public key
+    //! The public key for the address
     CPubKey vchPubKey;
+    //! The destination
+    CTxDestination address;
     //! Whether this is from the internal (change output) keypool
     bool fInternal{false};
 
 public:
-    //! Construct a CReserveKey object. This does NOT reserve a key from the keypool yet
-    explicit CReserveKey(CWallet* pwalletIn)
+    //! Construct a ReserveDestination object. This does NOT reserve an address yet
+    explicit ReserveDestination(CWallet* pwalletIn)
     {
         pwallet = pwalletIn;
     }
 
-    CReserveKey(const CReserveKey&) = delete;
-    CReserveKey& operator=(const CReserveKey&) = delete;
+    ReserveDestination(const ReserveDestination&) = delete;
+    ReserveDestination& operator=(const ReserveDestination&) = delete;
 
     //! Destructor. If a key has been reserved and not KeepKey'ed, it will be returned to the keypool
-    ~CReserveKey()
+    ~ReserveDestination()
     {
-        ReturnKey();
+        ReturnDestination();
     }
 
-    //! Reserve a key from the keypool
-    bool GetReservedKey(CPubKey &pubkey, bool internal = false);
-    //! Return a key to the keypool
-    void ReturnKey();
-    //! Keep the key. Do not return it to the keypool when this object goes out of scope
-    void KeepKey();
+    //! Reserve an address
+    bool GetReservedDestination(const OutputType type, CTxDestination& pubkey, bool internal);
+    //! Return reserved address
+    void ReturnDestination();
+    //! Keep the address. Do not return it's key to the keypool when this object goes out of scope
+    void KeepDestination();
 };
 
 /** Address book data */
@@ -835,6 +837,9 @@ private:
      */
     uint256 m_last_block_processed GUARDED_BY(cs_wallet);
 
+    //! Fetches a key from the keypool
+    bool GetKeyFromPool(CPubKey &key, bool internal = false);
+
 public:
     /*
      * Main wallet lock.
@@ -1079,9 +1084,9 @@ public:
      * selected by SelectCoins(); Also create the change output, when needed
      * @note passing nChangePosInOut as -1 will result in setting a random position
      */
-    bool CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut,
+    bool CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, ReserveDestination& reservedest, CAmount& nFeeRet, int& nChangePosInOut,
                            std::string& strFailReason, const CCoinControl& coin_control, bool sign = true);
-    bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, CReserveKey& reservekey, CValidationState& state);
+    bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, ReserveDestination& reservedest, CValidationState& state);
 
     bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, bool use_max_sig = false) const
     {
@@ -1136,7 +1141,6 @@ public:
     bool ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal);
     void KeepKey(int64_t nIndex);
     void ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey);
-    bool GetKeyFromPool(CPubKey &key, bool internal = false);
     int64_t GetOldestKeyPoolTime();
     /**
      * Marks all keys in the keypool up to and including reserve_key as used.
@@ -1149,6 +1153,9 @@ public:
 
     std::set<CTxDestination> GetLabelAddresses(const std::string& label) const;
 
+    bool GetNewDestination(const OutputType type, const std::string label, CTxDestination& dest, std::string& error);
+    bool GetNewChangeDestination(const OutputType type, CTxDestination& dest, std::string& error);
+
     isminetype IsMine(const CTxIn& txin) const;
     /**
      * Returns amount of debit if the input matches the