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 8e7f930828
  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

src/coinjoin/client.cpp

@@ -1509,12 +1509,12 @@ bool CCoinJoinClientSession::CreateCollateralTransaction(CMutableTransaction& tx
     if (txout.nValue >= CCoinJoin::GetCollateralAmount() * 2) {
         // make our change address
         CScript scriptChange;
-        CPubKey vchPubKey;
-        CReserveKey reservekey(&mixingWallet);
-        bool success = reservekey.GetReservedKey(vchPubKey, true);
+        CTxDestination dest;
+        ReserveDestination reserveDest(&mixingWallet);
+        bool success = reserveDest.GetReservedDestination(dest, true);
         assert(success); // should never fail, as we just unlocked
-        scriptChange = GetScriptForDestination(vchPubKey.GetID());
-        reservekey.KeepKey();
+        scriptChange = GetScriptForDestination(dest);
+        reserveDest.KeepDestination();
         // return change
         txCollateral.vout.emplace_back(txout.nValue - CCoinJoin::GetCollateralAmount(), scriptChange);
     } else { // txout.nValue < CCoinJoin::GetCollateralAmount() * 2

src/coinjoin/util.cpp

@@ -21,24 +21,24 @@ inline unsigned int GetSizeOfCompactSizeDiff(uint64_t nSizePrev, uint64_t nSizeN
 }
 
 CKeyHolder::CKeyHolder(CWallet* pwallet) :
-    reserveKey(pwallet)
+    reserveDestination(pwallet)
 {
-    reserveKey.GetReservedKey(pubKey, false);
+    reserveDestination.GetReservedDestination(dest, false);
 }
 
 void CKeyHolder::KeepKey()
 {
-    reserveKey.KeepKey();
+    reserveDestination.KeepDestination();
 }
 
 void CKeyHolder::ReturnKey()
 {
-    reserveKey.ReturnKey();
+    reserveDestination.ReturnDestination();
 }
 
 CScript CKeyHolder::GetScriptForDestination() const
 {
-    return ::GetScriptForDestination(pubKey.GetID());
+    return ::GetScriptForDestination(dest);
 }
 
 
@@ -89,13 +89,13 @@ void CKeyHolderStorage::ReturnAll()
 
 CTransactionBuilderOutput::CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, std::shared_ptr<CWallet> pwalletIn, CAmount nAmountIn) :
     pTxBuilder(pTxBuilderIn),
-    key(pwalletIn.get()),
+    dest(pwalletIn.get()),
     nAmount(nAmountIn)
 {
     assert(pTxBuilder);
-    CPubKey pubKey;
-    key.GetReservedKey(pubKey, false);
-    script = ::GetScriptForDestination(pubKey.GetID());
+    CTxDestination txdest;
+    dest.GetReservedDestination(txdest, false);
+    script = ::GetScriptForDestination(txdest);
 }
 
 bool CTransactionBuilderOutput::UpdateAmount(const CAmount nNewAmount)
@@ -109,7 +109,7 @@ bool CTransactionBuilderOutput::UpdateAmount(const CAmount nNewAmount)
 
 CTransactionBuilder::CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, const CompactTallyItem& tallyItemIn) :
     pwallet(pwalletIn),
-    dummyReserveKey(pwalletIn.get()),
+    dummyReserveDestination(pwalletIn.get()),
     tallyItem(tallyItemIn)
 {
     // Generate a feerate which will be used to consider if the remainder is dust and will go into fees or not
@@ -170,7 +170,7 @@ void CTransactionBuilder::Clear()
         }
     }
     // Always return this key just to make sure..
-    dummyReserveKey.ReturnKey();
+    dummyReserveDestination.ReturnDestination();
 }
 
 bool CTransactionBuilder::CouldAddOutput(CAmount nAmountOutput) const

src/coinjoin/util.h

@@ -13,8 +13,8 @@ struct bilingual_str;
 class CKeyHolder
 {
 private:
-    CReserveKey reserveKey;
-    CPubKey pubKey;
+    ReserveDestination reserveDestination;
+    CTxDestination dest;
 
 public:
     explicit CKeyHolder(CWallet* pwalletIn);
@@ -40,14 +40,14 @@ public:
 
 /**
  * @brief Used by CTransactionBuilder to represent its transaction outputs.
- * It creates a CReserveKey for the given CWallet as destination.
+ * It creates a ReserveDestination for the given CWallet as destination.
  */
 class CTransactionBuilderOutput
 {
     /// Used for amount updates
     CTransactionBuilder* pTxBuilder{nullptr};
     /// Reserve key where the amount of this output will end up
-    CReserveKey key;
+    ReserveDestination dest;
     /// Amount this output will receive
     CAmount nAmount{0};
     /// ScriptPubKey of this output
@@ -64,9 +64,9 @@ public:
     /// Try update the amount of this output. Returns true if it was successful and false if not (e.g. insufficient amount left).
     bool UpdateAmount(CAmount nAmount);
     /// Tell the wallet to remove the key used by this output from the keypool
-    void KeepKey() { key.KeepKey(); }
+    void KeepKey() { dest.KeepDestination(); }
     /// Tell the wallet to return the key used by this output to the keypool
-    void ReturnKey() { key.ReturnKey(); }
+    void ReturnKey() { dest.ReturnDestination(); }
 };
 
 /**
@@ -83,7 +83,7 @@ class CTransactionBuilder
     /// Dummy since we anyway use tallyItem's destination as change destination in coincontrol.
     /// Its a member just to make sure ReturnKey can be called in destructor just in case it gets generated/kept
     /// somewhere in CWallet code.
-    CReserveKey dummyReserveKey;
+    ReserveDestination dummyReserveDestination;
     /// Contains all utxos available to generate this transactions. They are all from the same address.
     CompactTallyItem tallyItem;
     /// Contains the number of bytes required for a transaction with only the inputs of tallyItems, no outputs

src/interfaces/wallet.cpp

@@ -181,13 +181,14 @@ public:
     }
     int64_t getKeysLeftSinceAutoBackup() override { return m_wallet->nKeysLeftSinceAutoBackup; }
     std::string getWalletName() override { return m_wallet->GetName(); }
-    bool getKeyFromPool(bool internal, CPubKey& pub_key) override
+    bool getNewDestination(const std::string label, CTxDestination& dest) override
     {
         auto spk_man = m_wallet->GetLegacyScriptPubKeyMan();
         if (!spk_man) {
             return false;
         }
-        return spk_man->GetKeyFromPool(pub_key, internal);
+        std::string error;
+        return m_wallet->GetNewDestination(label, dest, error);
     }
     bool getPubKey(const CKeyID& address, CPubKey& pub_key) override {
         auto spk_man = m_wallet->GetLegacyScriptPubKeyMan();
@@ -300,7 +301,7 @@ public:
         bilingual_str& fail_reason) override
     {
         LOCK(m_wallet->cs_wallet);
-        CReserveKey m_key(m_wallet.get());
+        ReserveDestination m_dest(m_wallet.get());
         CTransactionRef tx;
         if (!m_wallet->CreateTransaction(recipients, tx, fee, change_pos,
                 fail_reason, coin_control, sign)) {
@@ -313,7 +314,7 @@ public:
         WalletOrderForm order_form) override
     {
         LOCK2(m_wallet->cs_wallet, cs_main);
-        CReserveKey m_key(m_wallet.get());
+        ReserveDestination m_dest(m_wallet.get());
         m_wallet->CommitTransaction(std::move(tx), std::move(value_map), std::move(order_form));
     }
     bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }

src/interfaces/wallet.h

@@ -111,8 +111,8 @@ public:
     //! Get wallet name.
     virtual std::string getWalletName() = 0;
 
-    // Get key from pool.
-    virtual bool getKeyFromPool(bool internal, CPubKey& pub_key) = 0;
+    // Get a new address.
+    virtual bool getNewDestination(const std::string label, CTxDestination& dest) = 0;
 
     //! Get public key.
     virtual bool getPubKey(const CKeyID& address, CPubKey& pub_key) = 0;

src/qt/addresstablemodel.cpp

@@ -364,12 +364,14 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
                 return QString();
             }
         }
+        // Add entry
+        walletModel->wallet().setAddressBook(DecodeDestination(strAddress), strLabel, "send");
     }
     else if(type == Receive)
     {
         // Generate a new address to associate with given label
-        CPubKey newKey;
-        if(!walletModel->wallet().getKeyFromPool(false /* internal */, newKey))
+        CTxDestination dest;
+        if(!walletModel->wallet().getNewDestination(strLabel, dest))
         {
             WalletModel::UnlockContext ctx(walletModel->requestUnlock());
             if(!ctx.isValid())
@@ -378,22 +380,18 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
                 editStatus = WALLET_UNLOCK_FAILURE;
                 return QString();
             }
-            if(!walletModel->wallet().getKeyFromPool(false /* internal */, newKey))
+            if(!walletModel->wallet().getNewDestination(strLabel, dest))
             {
                 editStatus = KEY_GENERATION_FAILURE;
                 return QString();
             }
         }
-        strAddress = EncodeDestination(newKey.GetID());
+        strAddress = EncodeDestination(dest);
     }
     else
     {
         return QString();
     }
-
-    // Add entry
-    walletModel->wallet().setAddressBook(DecodeDestination(strAddress), strLabel,
-                           (type == Send ? "send" : "receive"));
     return QString::fromStdString(strAddress);
 }
 

src/test/util.cpp

@@ -23,16 +23,11 @@ const std::string ADDRESS_BCRT1_UNSPENDABLE = "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqq
 #ifdef ENABLE_WALLET
 std::string getnewaddress(CWallet& w)
 {
-    auto spk_man = w.GetLegacyScriptPubKeyMan();
-    assert(spk_man != nullptr);
+    CTxDestination dest;
+    std::string error;
+    if (!w.GetNewDestination("", dest, error)) assert(false);
 
-    CPubKey new_key;
-    if (!spk_man->GetKeyFromPool(new_key, false)) assert(false);
-
-    CKeyID keyID = new_key.GetID();
-    w.SetAddressBook(keyID, /* label */ "", "receive");
-
-    return EncodeDestination(keyID);
+    return EncodeDestination(dest);
 }
 
 void importaddress(CWallet& wallet, const std::string& address)

src/wallet/rpcwallet.cpp

@@ -204,20 +204,12 @@ UniValue getnewaddress(const JSONRPCRequest& request)
     if (!request.params[0].isNull())
         label = LabelFromValue(request.params[0]);
 
-    if (!pwallet->IsLocked(true)) {
-        pwallet->TopUpKeyPool();
+    CTxDestination dest;
+    std::string error;
+    if (!pwallet->GetNewDestination(label, dest, error)) {
+        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error);
     }
-
-    // Generate a new key that is added to wallet
-    CPubKey newKey;
-    if (!spk_man->GetKeyFromPool(newKey, false)) {
-        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first");
-    }
-    CKeyID keyID = newKey.GetID();
-
-    pwallet->SetAddressBook(keyID, label, "receive");
-
-    return EncodeDestination(keyID);
+    return EncodeDestination(dest);
 }
 
 static UniValue getrawchangeaddress(const JSONRPCRequest& request)
@@ -245,20 +237,12 @@ static UniValue getrawchangeaddress(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_WALLET_ERROR, "Error: This wallet has no available keys");
     }
 
-    if (!pwallet->IsLocked(true)) {
-        pwallet->TopUpKeyPool();
+    CTxDestination dest;
+    std::string error;
+    if (!pwallet->GetNewChangeDestination(dest, error)) {
+        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, error);
     }
-
-    CReserveKey reservekey(pwallet);
-    CPubKey vchPubKey;
-    if (!reservekey.GetReservedKey(vchPubKey, true))
-        throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first");
-
-    reservekey.KeepKey();
-
-    CKeyID keyID = vchPubKey.GetID();
-
-    return EncodeDestination(keyID);
+    return EncodeDestination(dest);
 }
 
 

src/wallet/scriptpubkeyman.cpp

@@ -12,6 +12,25 @@
 #include <wallet/scriptpubkeyman.h>
 #include <wallet/wallet.h>
 
+bool LegacyScriptPubKeyMan::GetNewDestination(const std::string label, CTxDestination& dest, std::string& error)
+{
+    LOCK(cs_wallet);
+    error.clear();
+    TopUpKeyPool();
+
+    // Generate a new key that is added to wallet
+    CPubKey new_key;
+    if (!GetKeyFromPool(new_key, false)) {
+        error = "Error: Keypool ran out, please call keypoolrefill first";
+        return false;
+    }
+    //LearnRelatedScripts(new_key);
+    dest = new_key.GetID();
+
+    m_wallet.SetAddressBook(dest, label, "receive");
+    return true;
+}
+
 typedef std::vector<unsigned char> valtype;
 
 namespace {

src/wallet/scriptpubkeyman.h

@@ -286,13 +286,14 @@ 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 fInternal /*= false*/);
+    //! Fetches a key from the keypool
+    bool GetKeyFromPool(CPubKey &key, bool internal /* = false */);
     int64_t GetOldestKeyPoolTime();
+
     /**
      * Marks all keys in the keypool up to and including reserve_key as used.
      */
     void MarkReserveKeysAsUsed(int64_t keypool_id) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
-    const std::map<CKeyID, int64_t>& GetAllReserveKeys() const { return m_pool_key_to_index; }
 
     isminetype IsMine(const CScript& script) const;
     isminetype IsMine(const CTxDestination& dest) const;
@@ -347,6 +348,8 @@ public:
 
     /** Implement lookup of key origin information through wallet key metadata. */
     bool GetKeyOrigin(const CKeyID& keyid, KeyOriginInfo& info) const override;
+    const std::map<CKeyID, int64_t>& GetAllReserveKeys() const { return m_pool_key_to_index; }
+    bool GetNewDestination(const std::string label, CTxDestination& dest, std::string& error);
 
     /** Add a KeyOriginInfo to the wallet */
     bool AddKeyOrigin(const CPubKey& pubkey, const KeyOriginInfo& info);

src/wallet/test/coinjoin_tests.cpp

@@ -175,14 +175,12 @@ public:
     {
         CompactTallyItem tallyItem;
         CTransactionRef tx;
-        CReserveKey destKey(wallet.get());
+        ReserveDestination reserveDest(wallet.get());
         CAmount nFeeRet;
         int nChangePosRet = -1;
         bilingual_str strError;
         CCoinControl coinControl;
-        CPubKey pubKey;
-        BOOST_CHECK(destKey.GetReservedKey(pubKey, false));
-        tallyItem.txdest = pubKey.GetID();
+        BOOST_CHECK(reserveDest.GetReservedDestination(tallyItem.txdest, false));
         for (CAmount nAmount : vecAmounts) {
             BOOST_CHECK(wallet->CreateTransaction({{GetScriptForDestination(tallyItem.txdest), nAmount, false}}, tx, nFeeRet, nChangePosRet, strError, coinControl));
             {
@@ -200,7 +198,7 @@ public:
             }
         }
         assert(tallyItem.vecInputCoins.size() == vecAmounts.size());
-        destKey.KeepKey();
+        reserveDest.KeepDestination();
         return tallyItem;
     }
 };

src/wallet/test/wallet_tests.cpp

@@ -1008,8 +1008,9 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
     wallet->SetMinVersion(FEATURE_LATEST);
     wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
     BOOST_CHECK(!wallet->TopUpKeyPool(1000));
-    CPubKey pubkey;
-    BOOST_CHECK(!wallet->GetLegacyScriptPubKeyMan()->GetKeyFromPool(pubkey, false));
+    CTxDestination dest;
+    std::string error;
+    BOOST_CHECK(!wallet->GetNewDestination("", dest, error));
 }
 
 BOOST_AUTO_TEST_SUITE_END()

src/wallet/wallet.cpp

@@ -3142,7 +3142,7 @@ bool CWallet::GetBudgetSystemCollateralTX(CTransactionRef& tx, uint256 hash, CAm
 bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, bool sign, int nExtraPayloadSize)
 {
     CAmount nValue = 0;
-    CReserveKey reservekey(this);
+    ReserveDestination reservedest(this);
     int nChangePosRequest = nChangePosInOut;
     unsigned int nSubtractFeeFromAmount = 0;
     for (const auto& recipient : vecSend)
@@ -3186,7 +3186,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
             }
 
             // Create change script that will be used if we need change
-            // TODO: pass in scriptChange instead of reservekey so
+            // TODO: pass in scriptChange instead of reservedest so
             // change transaction isn't always pay-to-bitcoin-address
             CScript scriptChange;
 
@@ -3206,16 +3206,15 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
                     error = _("Can't generate a change-address key. No keys in the internal keypool and can't generate any keys.");
                     return false;
                 }
-                CPubKey vchPubKey;
-                bool ret;
-                ret = reservekey.GetReservedKey(vchPubKey, true);
+                CTxDestination dest;
+                bool ret = reservedest.GetReservedDestination(dest, true);
                 if (!ret)
                 {
                     error = _("Keypool ran out, please call keypoolrefill first");
                     return false;
                 }
 
-                scriptChange = GetScriptForDestination(vchPubKey.GetID());
+                scriptChange = GetScriptForDestination(dest);
             }
 
             nFeeRet = 0;
@@ -3510,7 +3509,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac
 
     // Before we return success, we assume any change key will be used to prevent
     // accidental re-use.
-    reservekey.KeepKey();
+    reservedest.KeepDestination();
 
     WalletLogPrintf("Fee Calculation: Fee:%d Bytes:%u Tgt:%d (requested %d) Reason:\"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n",
               nFeeRet, nBytes, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay,
@@ -3742,6 +3741,32 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize)
     return res;
 }
 
+bool CWallet::GetNewDestination(const std::string label, CTxDestination& dest, std::string& error)
+{
+    error.clear();
+    bool result = false;
+    auto spk_man = m_spk_man.get();
+    if (spk_man) {
+        result = spk_man->GetNewDestination(label, dest, error);
+    }
+    return result;
+}
+
+bool CWallet::GetNewChangeDestination(CTxDestination& dest, std::string& error)
+{
+    error.clear();
+    m_spk_man->TopUpKeyPool();
+
+    ReserveDestination reservedest(this);
+    if (!reservedest.GetReservedDestination(dest, true)) {
+        error = "Error: Keypool ran out, please call keypoolrefill first";
+        return false;
+    }
+
+    reservedest.KeepDestination();
+    return true;
+}
+
 std::map<CTxDestination, CAmount> CWallet::GetAddressBalances()
 {
     std::map<CTxDestination, CAmount> balances;
@@ -3889,7 +3914,7 @@ std::set<CTxDestination> CWallet::GetLabelAddresses(const std::string& label) co
     return result;
 }
 
-bool CReserveKey::GetReservedKey(CPubKey& pubkey, bool fInternalIn)
+bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool fInternalIn)
 {
     m_spk_man = pwallet->GetLegacyScriptPubKeyMan();
     if (!m_spk_man) {
@@ -3910,26 +3935,29 @@ bool CReserveKey::GetReservedKey(CPubKey& pubkey, bool fInternalIn)
         fInternal = keypool.fInternal;
     }
     assert(vchPubKey.IsValid());
-    pubkey = vchPubKey;
+    address = vchPubKey.GetID();
+    dest = address;
     return true;
 }
 
-void CReserveKey::KeepKey()
+void ReserveDestination::KeepDestination()
 {
     if (nIndex != -1) {
         m_spk_man->KeepKey(nIndex);
     }
     nIndex = -1;
     vchPubKey = CPubKey();
+    address = CNoDestination();
 }
 
-void CReserveKey::ReturnKey()
+void ReserveDestination::ReturnDestination()
 {
     if (nIndex != -1) {
         m_spk_man->ReturnKey(nIndex, fInternal, vchPubKey);
     }
     nIndex = -1;
     vchPubKey = CPubKey();
+    address = CNoDestination();
 }
 
 void CWallet::LockCoin(const COutPoint& output)

src/wallet/wallet.h

@@ -100,12 +100,12 @@ static const bool DEFAULT_USE_HD_WALLET = false;
 class CCoinControl;
 class CKey;
 class COutput;
-class CReserveKey;
 class CScript;
 class CTxDSIn;
 class CWalletTx;
 struct FeeCalculation;
 enum class FeeEstimateMode;
+class ReserveDestination;
 
 extern CCriticalSection cs_main;
 
@@ -136,57 +136,60 @@ static const std::map<std::string,WalletFlags> WALLET_FLAG_MAP{
 
 extern const std::map<uint64_t,std::string> WALLET_FLAG_CAVEATS;
 
-/** 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 final : public CReserveScript
+class ReserveDestination : public CReserveScript
 {
 protected:
-    //! The wallet to reserve the keypool key from
+    //! The wallet to reserve from
     CWallet* pwallet;
     LegacyScriptPubKeyMan* m_spk_man{nullptr};
-    //! 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 fInternalIn /*= 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();
-    void KeepScript() override { KeepKey(); }
+    //! Reserve an address
+    bool GetReservedDestination(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();
+    void KeepScript() override { KeepDestination(); }
 };
 
 /** Address book data */
@@ -1063,6 +1066,9 @@ public:
 
     std::set<CTxDestination> GetLabelAddresses(const std::string& label) const;
 
+    bool GetNewDestination(const std::string label, CTxDestination& dest, std::string& error);
+    bool GetNewChangeDestination(CTxDestination& dest, std::string& error);
+
     isminetype IsMine(const CTxDestination& dest) const;
     isminetype IsMine(const CScript& script) const;
     isminetype IsMine(const CTxIn& txin) const;