diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -29,7 +29,7 @@ class PendingWalletTxImpl : public PendingWalletTx { public: explicit PendingWalletTxImpl(CWallet &wallet) - : m_wallet(wallet), m_key(&wallet) {} + : m_wallet(wallet), m_dest(&wallet) {} const CTransaction &get() override { return *m_tx; } @@ -39,7 +39,7 @@ LOCK(m_wallet.cs_wallet); CValidationState state; if (!m_wallet.CommitTransaction(m_tx, std::move(value_map), - std::move(order_form), m_key, + std::move(order_form), m_dest, state)) { reject_reason = state.GetRejectReason(); return false; @@ -49,7 +49,7 @@ CTransactionRef m_tx; CWallet &m_wallet; - CReserveKey m_key; + ReserveDestination m_dest; }; //! Construct wallet tx struct. @@ -248,7 +248,7 @@ LOCK(m_wallet->cs_wallet); auto pending = std::make_unique(*m_wallet); if (!m_wallet->CreateTransaction( - *locked_chain, recipients, pending->m_tx, pending->m_key, + *locked_chain, recipients, pending->m_tx, pending->m_dest, fee, change_pos, fail_reason, coin_control, sign)) { return {}; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -274,19 +274,15 @@ } } - CReserveKey reservekey(pwallet); - CPubKey vchPubKey; - if (!reservekey.GetReservedKey(vchPubKey, true)) { + ReserveDestination reservedest(pwallet); + CTxDestination dest; + if (!reservedest.GetReservedDestination(output_type, dest, true)) { throw JSONRPCError( RPC_WALLET_KEYPOOL_RAN_OUT, "Error: Keypool ran out, please call keypoolrefill first"); } - reservekey.KeepKey(); - - pwallet->LearnRelatedScripts(vchPubKey, output_type); - CTxDestination dest = GetDestinationForKey(vchPubKey, output_type); - + reservedest.KeepDestination(); return EncodeDestination(dest, config); } @@ -367,7 +363,7 @@ CScript scriptPubKey = GetScriptForDestination(address); // Create and send the transaction - CReserveKey reservekey(pwallet); + ReserveDestination reservedest(pwallet); Amount nFeeRequired; std::string strError; std::vector vecSend; @@ -377,7 +373,7 @@ CCoinControl coinControl; CTransactionRef tx; - if (!pwallet->CreateTransaction(locked_chain, vecSend, tx, reservekey, + if (!pwallet->CreateTransaction(locked_chain, vecSend, tx, reservedest, nFeeRequired, nChangePosRet, strError, coinControl)) { if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) { @@ -389,7 +385,7 @@ } CValidationState state; if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, - reservekey, state)) { + reservedest, state)) { strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(state)); @@ -1065,21 +1061,21 @@ std::shuffle(vecSend.begin(), vecSend.end(), FastRandomContext()); // Send - CReserveKey keyChange(pwallet); + ReserveDestination changedest(pwallet); Amount nFeeRequired = Amount::zero(); int nChangePosRet = -1; std::string strFailReason; CTransactionRef tx; CCoinControl coinControl; bool fCreated = pwallet->CreateTransaction( - *locked_chain, vecSend, tx, keyChange, nFeeRequired, nChangePosRet, + *locked_chain, vecSend, tx, changedest, nFeeRequired, nChangePosRet, strFailReason, coinControl); if (!fCreated) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); } CValidationState state; if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, - keyChange, state)) { + changedest, state)) { strFailReason = strprintf("Transaction commit failed:: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strFailReason); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -392,7 +392,7 @@ CWalletTx &AddTx(CRecipient recipient) { CTransactionRef tx; - CReserveKey reservekey(wallet.get()); + ReserveDestination reservedest(wallet.get()); Amount fee; int changePos = -1; std::string error; @@ -400,11 +400,11 @@ { auto locked_chain = m_chain->lock(); BOOST_CHECK(wallet->CreateTransaction(*locked_chain, {recipient}, - tx, reservekey, fee, + tx, reservedest, fee, changePos, error, dummy)); } CValidationState state; - BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reservekey, state)); + BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, reservedest, state)); CMutableTransaction blocktx; { LOCK(wallet->cs_wallet); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -92,10 +92,10 @@ class CChainParams; class CCoinControl; class COutput; -class CReserveKey; class CScript; class CTxMemPool; class CWalletTx; +class ReserveDestination; /** (client) version numbers for particular wallet features */ enum WalletFeature { @@ -276,51 +276,54 @@ }; /** - * 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 - * and that transaction was not completed and needed to be aborted). + * 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) { pwallet = 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() { ReturnKey(); } - - //! 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(); + ~ReserveDestination() { ReturnDestination(); } + + //! 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 */ @@ -1296,14 +1299,14 @@ */ bool CreateTransaction(interfaces::Chain::Lock &locked_chain, const std::vector &vecSend, - CTransactionRef &tx, CReserveKey &reservekey, + CTransactionRef &tx, ReserveDestination &reservedest, Amount &nFeeRet, int &nChangePosInOut, std::string &strFailReason, const CCoinControl &coin_control, bool sign = true); bool CommitTransaction( CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm, - CReserveKey &reservekey, CValidationState &state); + ReserveDestination &reservedest, CValidationState &state); bool DummySignTx(CMutableTransaction &txNew, const std::set &txouts, bool use_max_sig = false) const { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3155,9 +3155,9 @@ auto locked_chain = chain().lock(); LOCK(cs_wallet); - CReserveKey reservekey(this); + ReserveDestination reservedest(this); CTransactionRef tx_new; - if (!CreateTransaction(*locked_chain, vecSend, tx_new, reservekey, nFeeRet, + if (!CreateTransaction(*locked_chain, vecSend, tx_new, reservedest, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { return false; @@ -3169,7 +3169,7 @@ // We don't have the normal Create/Commit cycle, and don't want to // risk reusing change, so just remove the key from the keypool // here. - reservekey.KeepKey(); + reservedest.KeepDestination(); } // Copy output sizes from new transaction; they may have had the fee @@ -3277,7 +3277,8 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn, const std::vector &vecSend, - CTransactionRef &tx, CReserveKey &reservekey, + CTransactionRef &tx, + ReserveDestination &reservedest, Amount &nFeeRet, int &nChangePosInOut, std::string &strFailReason, const CCoinControl &coinControl, bool sign) { @@ -3319,7 +3320,7 @@ CoinSelectionParams coin_selection_params; // 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; @@ -3346,9 +3347,13 @@ .translated; return false; } - CPubKey vchPubKey; - bool ret; - ret = reservekey.GetReservedKey(vchPubKey, true); + CTxDestination dest; + const OutputType change_type = TransactionChangeType( + coinControl.m_change_type ? *coinControl.m_change_type + : m_default_change_type, + vecSend); + bool ret = + reservedest.GetReservedDestination(change_type, dest, true); if (!ret) { strFailReason = _("Keypool ran out, please call keypoolrefill first") @@ -3356,14 +3361,7 @@ return false; } - const OutputType change_type = TransactionChangeType( - coinControl.m_change_type ? *coinControl.m_change_type - : m_default_change_type, - vecSend); - - LearnRelatedScripts(vchPubKey, change_type); - scriptChange = GetScriptForDestination( - GetDestinationForKey(vchPubKey, change_type)); + scriptChange = GetScriptForDestination(dest); } CTxOut change_prototype_txout(Amount::zero(), scriptChange); coin_selection_params.change_output_size = @@ -3592,8 +3590,8 @@ } if (nChangePosInOut == -1) { - // Return any reserved key if we don't have change - reservekey.ReturnKey(); + // Return any reserved address if we don't have change + reservedest.ReturnDestination(); } // Shuffle selected coins and fill in final vin @@ -3668,7 +3666,7 @@ bool CWallet::CommitTransaction( CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm, - CReserveKey &reservekey, CValidationState &state) { + ReserveDestination &reservedest, CValidationState &state) { auto locked_chain = chain().lock(); LOCK(cs_wallet); @@ -3682,7 +3680,7 @@ wtxNew.tx->ToString()); // Take key pair from key pool so it won't be used again. - reservekey.KeepKey(); + reservedest.KeepDestination(); // Add tx to wallet, because if it has change it's also ours, otherwise just // for transaction history. @@ -4332,7 +4330,9 @@ return result; } -bool CReserveKey::GetReservedKey(CPubKey &pubkey, bool internal) { +bool ReserveDestination::GetReservedDestination(const OutputType type, + CTxDestination &dest, + bool internal) { if (!pwallet->CanGetAddresses(internal)) { return false; } @@ -4348,25 +4348,29 @@ } assert(vchPubKey.IsValid()); - pubkey = vchPubKey; + pwallet->LearnRelatedScripts(vchPubKey, type); + address = GetDestinationForKey(vchPubKey, type); + dest = address; return true; } -void CReserveKey::KeepKey() { +void ReserveDestination::KeepDestination() { if (nIndex != -1) { pwallet->KeepKey(nIndex); } nIndex = -1; vchPubKey = CPubKey(); + address = CNoDestination(); } -void CReserveKey::ReturnKey() { +void ReserveDestination::ReturnDestination() { if (nIndex != -1) { pwallet->ReturnKey(nIndex, fInternal, vchPubKey); } nIndex = -1; vchPubKey = CPubKey(); + address = CNoDestination(); } void CWallet::MarkReserveKeysAsUsed(int64_t keypool_id) {