diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index cbea0fa54df96..843c9402f5ac7 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -143,7 +143,7 @@ class Chain virtual bool hasChainLock(int height, const uint256& hash) = 0; //! Return list of MN Collateral from outputs - virtual std::vector listMNCollaterials(const std::vector>& outputs) = 0; + virtual std::vector listMNCollaterials(const std::vector>& outputs) = 0; //! Return whether node has the block and optionally return block metadata //! or contents. virtual bool findBlock(const uint256& hash, const FoundBlock& block={}) = 0; diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 6de6776ef5d26..57a01676a32d9 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -145,10 +145,10 @@ class Wallet virtual bool isLockedCoin(const COutPoint& output) = 0; //! List locked coins. - virtual void listLockedCoins(std::vector& outputs) = 0; + virtual std::vector listLockedCoins() = 0; //! List protx coins. - virtual void listProTxCoins(std::vector& vOutpts) = 0; + virtual std::vector listProTxCoins() = 0; //! Create transaction. virtual CTransactionRef createTransaction(const std::vector& recipients, diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 90a4c51d7e4c4..94be4f81537c4 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -786,7 +786,7 @@ class ChainImpl : public Chain if (m_node.llmq_ctx == nullptr || m_node.llmq_ctx->clhandler == nullptr) return false; return m_node.llmq_ctx->clhandler->HasChainLock(height, hash); } - std::vector listMNCollaterials(const std::vector>& outputs) override + std::vector listMNCollaterials(const std::vector>& outputs) override { const CBlockIndex *tip = WITH_LOCK(::cs_main, return chainman().ActiveChain().Tip()); CDeterministicMNList mnList{}; diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 541e8b2980365..e2946648a7cdf 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -426,11 +426,10 @@ void CoinControlDialog::viewItemChanged(QTreeWidgetItem* item, int column) // shows count of locked unspent outputs void CoinControlDialog::updateLabelLocked() { - std::vector vOutpts; - model->wallet().listLockedCoins(vOutpts); - if (vOutpts.size() > 0) + auto locked_coins{model->wallet().listLockedCoins().size()}; + if (locked_coins > 0) { - ui->labelLocked->setText(tr("(%1 locked)").arg(vOutpts.size())); + ui->labelLocked->setText(tr("(%1 locked)").arg(locked_coins)); ui->labelLocked->setVisible(true); } else ui->labelLocked->setVisible(false); diff --git a/src/qt/masternodelist.cpp b/src/qt/masternodelist.cpp index ef6c47e75fd28..79bb165a86a51 100644 --- a/src/qt/masternodelist.cpp +++ b/src/qt/masternodelist.cpp @@ -205,9 +205,7 @@ void MasternodeList::updateDIP3List() std::set setOutpts; if (walletModel && ui->checkBoxMyMasternodesOnly->isChecked()) { - std::vector vOutpts; - walletModel->wallet().listProTxCoins(vOutpts); - for (const auto& outpt : vOutpts) { + for (const auto& outpt : walletModel->wallet().listProTxCoins()) { setOutpts.emplace(outpt); } } diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index b20ca26ee1dc1..c3434560422bd 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -1390,10 +1390,8 @@ static RPCHelpMan protx_list() throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid height specified"); } - std::vector vOutpts; - wallet->ListProTxCoins(vOutpts); std::set setOutpts; - for (const auto& outpt : vOutpts) { + for (const auto& outpt : wallet->ListProTxCoins()) { setOutpts.emplace(outpt); } diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index c00924572abd7..a4fbb369bcc5c 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -255,15 +255,15 @@ class WalletImpl : public Wallet LOCK(m_wallet->cs_wallet); return m_wallet->IsLockedCoin(output.hash, output.n); } - void listLockedCoins(std::vector& outputs) override + std::vector listLockedCoins() override { LOCK(m_wallet->cs_wallet); - return m_wallet->ListLockedCoins(outputs); + return m_wallet->ListLockedCoins(); } - void listProTxCoins(std::vector& outputs) override + std::vector listProTxCoins() override { LOCK(m_wallet->cs_wallet); - return m_wallet->ListProTxCoins(outputs); + return m_wallet->ListProTxCoins(); } CTransactionRef createTransaction(const std::vector& recipients, const CCoinControl& coin_control, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 0e9c5aa778c0e..f50b92a49d1de 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2371,14 +2371,9 @@ static RPCHelpMan listlockunspent() LOCK(pwallet->cs_wallet); - std::vector vOutpts; - pwallet->ListLockedCoins(vOutpts); - UniValue ret(UniValue::VARR); - - for (const COutPoint& outpt : vOutpts) { + for (const COutPoint& outpt : pwallet->ListLockedCoins()) { UniValue o(UniValue::VOBJ); - o.pushKV("txid", outpt.hash.GetHex()); o.pushKV("vout", (int)outpt.n); ret.push_back(o); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4f6794f5ddc9c..358010c7ce4e3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -914,6 +914,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio CWalletTx& wtx = (*ret.first).second; bool fInsertedNew = ret.second; bool fUpdated = update_wtx && update_wtx(wtx, fInsertedNew); + std::set candidates; if (fInsertedNew) { wtx.m_confirm = confirm; wtx.nTimeReceived = GetTime(); @@ -921,20 +922,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); wtx.nTimeSmart = ComputeTimeSmart(wtx); AddToSpends(hash, &batch); - - std::vector> outputs; - for(unsigned int i = 0; i < wtx.tx->vout.size(); ++i) { - if (IsMine(wtx.tx->vout[i]) && !IsSpent(hash, i)) { - setWalletUTXO.insert(COutPoint(hash, i)); - outputs.emplace_back(wtx.tx, i); - } - } - // TODO: refactor duplicated code between CWallet::AddToWallet and CWallet::AutoLockMasternodeCollaterals - if (m_chain) { - for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { - LockCoin(outPoint, &batch); - } - } + candidates = AddWalletUTXOs(wtx.tx, /*ret_dups=*/true); } if (!fInsertedNew) @@ -950,25 +938,12 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio assert(wtx.m_confirm.hashBlock == confirm.hashBlock); assert(wtx.m_confirm.block_height == confirm.block_height); } - - std::vector> outputs; - for(unsigned int i = 0; i < wtx.tx->vout.size(); ++i) { - if (IsMine(wtx.tx->vout[i]) && !IsSpent(hash, i)) { - bool new_utxo = setWalletUTXO.insert(COutPoint(hash, i)).second; - if (new_utxo) { - outputs.emplace_back(wtx.tx, i); - fUpdated = true; - } - } - } - // TODO: refactor duplicated code with case fInstertedNew - if (m_chain) { - for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { - LockCoin(outPoint); - } - } + candidates = AddWalletUTXOs(wtx.tx, /*ret_dups=*/false); + if (!candidates.empty()) fUpdated = true; } + LockProTxCoins(candidates, &batch); + //// debug print WalletLogPrintf("AddToWallet %s %s%s\n", hash.ToString(), (fInsertedNew ? "new" : ""), (fUpdated ? "update" : "")); @@ -1062,6 +1037,21 @@ bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx return true; } +std::set CWallet::AddWalletUTXOs(CTransactionRef tx, bool ret_dups) +{ + AssertLockHeld(cs_wallet); + std::set ret; + uint256 hash{tx->GetHash()}; + for (size_t idx = 0; idx < tx->vout.size(); ++idx) { + if (IsMine(tx->vout[idx]) && !IsSpent(hash, idx)) { + if (auto [_, inserted] = setWalletUTXO.emplace(hash, idx); inserted || ret_dups) { + ret.emplace(hash, idx); + } + } + } + return ret; +} + bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool fUpdate) { const CTransaction& tx = *ptx; @@ -2799,12 +2789,10 @@ std::map> CWallet::ListCoins() const } } - std::vector lockedCoins; - ListLockedCoins(lockedCoins); // Include watch-only for LegacyScriptPubKeyMan wallets without private keys const bool include_watch_only = GetLegacyScriptPubKeyMan() && IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); const isminetype is_mine_filter = include_watch_only ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; - for (const COutPoint& output : lockedCoins) { + for (const COutPoint& output : setLockedCoins) { auto it = mapWallet.find(output.hash); if (it != mapWallet.end()) { int depth = it->second.GetDepthInMainChain(); @@ -4056,21 +4044,14 @@ DBErrors CWallet::LoadWallet() // This avoids accidental spending of collaterals. They can still be unlocked manually if a spend is really intended. void CWallet::AutoLockMasternodeCollaterals() { - if (!m_chain) return; - - std::vector> outputs; - + std::set candidates; LOCK(cs_wallet); - for (const auto& pair : mapWallet) { - for (unsigned int i = 0; i < pair.second.tx->vout.size(); ++i) { - if (IsMine(pair.second.tx->vout[i]) && !IsSpent(pair.first, i)) { - outputs.emplace_back(pair.second.tx, i); - } - } - } - for (const auto& outPoint : m_chain->listMNCollaterials(outputs)) { - LockCoin(outPoint); + for (const auto& [txid, wtx] : mapWallet) { + auto tx_utxos{AddWalletUTXOs(wtx.tx, /*ret_dups=*/true)}; + candidates.insert(tx_utxos.begin(), tx_utxos.end()); } + WalletBatch batch(GetDatabase()); + LockProTxCoins(candidates, &batch); } DBErrors CWallet::ZapSelectTx(std::vector& vHashIn, std::vector& vHashOut) @@ -4502,34 +4483,36 @@ bool CWallet::IsLockedCoin(uint256 hash, unsigned int n) const return (setLockedCoins.count(outpt) > 0); } -void CWallet::ListLockedCoins(std::vector& vOutpts) const +std::vector CWallet::ListLockedCoins() const { AssertLockHeld(cs_wallet); - for (std::set::iterator it = setLockedCoins.begin(); - it != setLockedCoins.end(); it++) { - COutPoint outpt = (*it); - vOutpts.push_back(outpt); - } + return std::vector(setLockedCoins.begin(), setLockedCoins.end()); } -void CWallet::ListProTxCoins(std::vector& vOutpts) const +std::vector CWallet::ListProTxCoins() const { return ListProTxCoins(setWalletUTXO); } + +std::vector CWallet::ListProTxCoins(const std::set& utxos) const { - // TODO: refactor duplicated code between CWallet::AutoLockMasternodeCollaterals and CWallet::ListProTxCoins - if (!m_chain) { - vOutpts.clear(); - return; + AssertLockHeld(cs_wallet); + + if (!m_chain) return std::vector(); + + std::vector> candidates; + for (const auto& output : utxos) { + if (auto it = mapWallet.find(output.hash); it != mapWallet.end()) { + const auto& [hash, wtx] = *it; + candidates.emplace_back(wtx.tx, output.n); + } } - std::vector> outputs; + return m_chain->listMNCollaterials(candidates); +} +void CWallet::LockProTxCoins(const std::set& utxos, WalletBatch* batch) +{ AssertLockHeld(cs_wallet); - for (const auto &o : setWalletUTXO) { - auto it = mapWallet.find(o.hash); - if (it != mapWallet.end()) { - const auto &ptx = it->second; - outputs.emplace_back(ptx.tx, o.n); - } + for (const auto& utxo : ListProTxCoins(utxos)) { + LockCoin(utxo, batch); } - vOutpts = m_chain->listMNCollaterials(outputs); } /** @} */ // end of Actions diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f2b8713d77d74..5a36f9d662bf9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -763,6 +763,12 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void AddToSpends(const uint256& wtxid, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::set setWalletUTXO; + /** Add new UTXOs to the wallet UTXO set + * + * @param[in] tx Transaction to scan eligible UTXOs from + * @param[in] ret_dups Allow UTXOs already in set to be included in return value + * @returns Set of all new UTXOs (eligible to be) added to set */ + std::set AddWalletUTXOs(CTransactionRef tx, bool ret_dups) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); mutable std::map mapOutpointRoundsCache GUARDED_BY(cs_wallet); /** @@ -1038,8 +1044,10 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati bool LockCoin(const COutPoint& output, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool UnlockCoin(const COutPoint& output, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool UnlockAllCoins() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void ListLockedCoins(std::vector& vOutpts) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void ListProTxCoins(std::vector& vOutpts) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + std::vector ListLockedCoins() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + std::vector ListProTxCoins() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + std::vector ListProTxCoins(const std::set& utxos) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void LockProTxCoins(const std::set& utxos, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /* * Rescan abort properties