diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index abd710809..b1c8a65bd 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -167,8 +167,8 @@ public: void disconnect() override { if (m_notifications) { - m_notifications = nullptr; UnregisterValidationInterface(this); + m_notifications = nullptr; } } void TransactionAddedToMempool(const CTransactionRef& tx) override @@ -347,15 +347,17 @@ public: { return MakeUnique(*this, notifications); } - void waitForNotificationsIfNewBlocksConnected(const uint256& old_tip) override + bool waitForNotificationsIfTipIsNotSame(const uint256& tip) override { - if (!old_tip.IsNull()) { + if (!tip.IsNull()) { LOCK(::cs_main); - if (old_tip == ::ChainActive().Tip()->GetBlockHash()) return; - CBlockIndex* block = LookupBlockIndex(old_tip); - if (block && block->GetAncestor(::ChainActive().Height()) == ::ChainActive().Tip()) return; + auto chainTip = ::ChainActive().Tip(); + if (tip == chainTip->GetBlockHash()) return true; + CBlockIndex* block = LookupBlockIndex(tip); + if (block && block->GetAncestor(::ChainActive().Height()) == chainTip) return true; } SyncWithValidationInterfaceQueue(); + return false; } std::unique_ptr handleRpc(const CRPCCommand& command) override { diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 0a5febaf8..5433ba324 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -230,7 +230,7 @@ public: //! Wait for pending notifications to be processed unless block hash points to the current //! chain tip, or to a possible descendant of the current chain tip that isn't currently //! connected. - virtual void waitForNotificationsIfNewBlocksConnected(const uint256& old_tip) = 0; + virtual bool waitForNotificationsIfTipIsNotSame(const uint256& tip) = 0; //! Register handler for RPC. Command is not copied, so reference //! needs to remain valid until Handler is disconnected. diff --git a/src/scheduler.cpp b/src/scheduler.cpp index fdc859b3a..164e1010a 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -197,12 +197,16 @@ void SingleThreadedSchedulerClient::AddToProcessQueue(std::function fun } void SingleThreadedSchedulerClient::EmptyQueue() { - assert(!m_pscheduler->AreThreadsServicingQueue()); - bool should_continue = true; + if (!m_pscheduler->AreThreadsServicingQueue()) + return; + auto pendingCallbacks = [this]() -> bool { + LOCK(m_cs_callbacks_pending); + return !m_callbacks_pending.empty(); + }; + bool should_continue = pendingCallbacks(); while (should_continue) { ProcessQueue(); - LOCK(m_cs_callbacks_pending); - should_continue = !m_callbacks_pending.empty(); + should_continue = pendingCallbacks(); } } diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 59a620ab9..4479eb991 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -104,6 +104,7 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { void UnregisterValidationInterface(CValidationInterface* pwalletIn) { if (g_signals.m_internals) { g_signals.m_internals->m_connMainSignals.erase(pwalletIn); + g_signals.FlushBackgroundCallbacks(); } } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8d3143612..fb57c635f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -326,6 +326,9 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet if (nValue <= 0) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid amount"); + if (nValue > curBalance) + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds"); + // Parse Bitcoin address const CScript scriptPubKey = prefix + GetScriptForDestination(address); diff --git a/src/wallet/test/claim_rpc_tests.cpp b/src/wallet/test/claim_rpc_tests.cpp index 17d18ecef..89ab5188a 100644 --- a/src/wallet/test/claim_rpc_tests.cpp +++ b/src/wallet/test/claim_rpc_tests.cpp @@ -107,6 +107,15 @@ UniValue LookupAllNames() { return rpc_method(req); } +UniValue PruneAbandonFunds(const uint256& txid) { + auto rpc_method = tableRPC["removeprunedfunds"]; + JSONRPCRequest req; + req.URI = "/wallet/tester_wallet"; + req.params = UniValue(UniValue::VARR); + req.params.push_back(txid.GetHex()); + return rpc_method(req); +} + std::vector generateBlock(int blocks = 1) { auto rpc_method = tableRPC["generatetoaddress"]; JSONRPCRequest req; @@ -505,5 +514,17 @@ BOOST_AUTO_TEST_CASE(can_claim_after_each_fork) BOOST_CHECK_EQUAL(looked.size(), 4U); } +BOOST_AUTO_TEST_CASE(remove_pruned_funds) +{ + generateBlock(140); + // claim a name + auto txid = ClaimAName("tester", "deadbeef", "1.0"); + generateBlock(); + // abandon claim + AbandonAClaim(txid); + generateBlock(); + PruneAbandonFunds(txid); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1a77c52a6..15ff14d6c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -56,7 +56,7 @@ bool AddWallet(const std::shared_ptr& wallet) { LOCK(cs_wallets); assert(wallet); - std::vector>::const_iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet); + auto i = std::find(vpwallets.begin(), vpwallets.end(), wallet); if (i != vpwallets.end()) return false; vpwallets.push_back(wallet); return true; @@ -107,13 +107,9 @@ static std::set g_unloading_wallet_set; // Custom deleter for shared_ptr. static void ReleaseWallet(CWallet* wallet) { - // Unregister and delete the wallet right after BlockUntilSyncedToCurrentChain - // so that it's in sync with the current chainstate. const std::string name = wallet->GetName(); wallet->WalletLogPrintf("Releasing wallet\n"); - wallet->BlockUntilSyncedToCurrentChain(); wallet->Flush(); - wallet->m_chain_notifications_handler.reset(); delete wallet; // Wallet is now released, notify UnloadWallet, if any. { @@ -139,6 +135,8 @@ void UnloadWallet(std::shared_ptr&& wallet) // Notify the unload intent so that all remaining shared pointers are // released. wallet->NotifyUnload(); + wallet->BlockUntilSyncedToCurrentChain(); + wallet->m_chain_notifications_handler.reset(); // Time to ditch our shared_ptr and wait for ReleaseWallet call. wallet.reset(); { @@ -396,8 +394,8 @@ bool CWallet::AddKeyPubKeyWithDB(WalletBatch& batch, const CKey& secret, const C if (!IsCrypted()) { return batch.WriteKey(pubkey, - secret.GetPrivKey(), - mapKeyMetadata[pubkey.GetID()]); + secret.GetPrivKey(), + mapKeyMetadata[pubkey.GetID()]); } UnsetWalletFlagWithDB(batch, WALLET_FLAG_BLANK_WALLET); return true; @@ -418,12 +416,12 @@ bool CWallet::AddCryptedKey(const CPubKey &vchPubKey, LOCK(cs_wallet); if (encrypted_batch) return encrypted_batch->WriteCryptedKey(vchPubKey, - vchCryptedSecret, - mapKeyMetadata[vchPubKey.GetID()]); + vchCryptedSecret, + mapKeyMetadata[vchPubKey.GetID()]); else return WalletBatch(*database).WriteCryptedKey(vchPubKey, - vchCryptedSecret, - mapKeyMetadata[vchPubKey.GetID()]); + vchCryptedSecret, + mapKeyMetadata[vchPubKey.GetID()]); } } @@ -742,8 +740,6 @@ std::set CWallet::GetConflicts(const uint256& txid) const return result; const CWalletTx& wtx = it->second; - std::pair range; - for (const CTxIn& txin : wtx.tx->vin) { auto hitTx = mapTxSpends.find(txin.prevout.hash); @@ -792,10 +788,11 @@ void CWallet::SyncMetaData(const COutPoint& outPoint) auto hitN = hitTx->second.find(outPoint.n); if (hitN != hitTx->second.end()) { for (auto &spend: hitN->second) { - const CWalletTx *wtx = &mapWallet.at(spend); - if (wtx->nOrderPos < nMinOrderPos) { - nMinOrderPos = wtx->nOrderPos; - copyFrom = wtx; + if (auto wtx = GetWalletTx(spend)) { + if (wtx->nOrderPos < nMinOrderPos) { + nMinOrderPos = wtx->nOrderPos; + copyFrom = wtx; + } } } if (!copyFrom) { @@ -804,7 +801,9 @@ void CWallet::SyncMetaData(const COutPoint& outPoint) // Now copy data from copyFrom to rest: for (auto &hash: hitN->second) { - CWalletTx *copyTo = &mapWallet.at(hash); + auto it = mapWallet.find(hash); + if (it != mapWallet.end()) continue; + CWalletTx* copyTo = &it->second; if (copyFrom == copyTo) continue; assert(copyFrom && "Oldest wallet transaction in range assumed to have been found."); if (!copyFrom->IsEquivalentTo(*copyTo)) continue; @@ -853,16 +852,13 @@ void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid) } -void CWallet::AddToSpends(const uint256& wtxid) +void CWallet::AddToSpends(const CWalletTx& wtx) { - auto it = mapWallet.find(wtxid); - assert(it != mapWallet.end()); - CWalletTx& thisTx = it->second; - if (thisTx.IsCoinBase()) // Coinbases don't spend anything! + if (wtx.IsCoinBase()) // Coinbases don't spend anything! return; - for (const CTxIn& txin : thisTx.tx->vin) - AddToSpends(txin.prevout, wtxid); + for (const CTxIn& txin : wtx.tx->vin) + AddToSpends(txin.prevout, wtx.GetHash()); } bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) @@ -1121,7 +1117,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) WalletBatch batch(*database, "r+", fFlushOnClose); - uint256 hash = wtxIn.GetHash(); + const uint256& hash = wtxIn.GetHash(); if (IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { // Mark used destinations @@ -1145,7 +1141,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) wtx.nOrderPos = IncOrderPosNext(&batch); wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); wtx.nTimeSmart = ComputeTimeSmart(wtx); - AddToSpends(hash); + AddToSpends(wtx); } bool fUpdated = false; @@ -1219,14 +1215,14 @@ void CWallet::LoadToWallet(CWalletTx& wtxIn) wtxIn.m_confirm.nIndex = 0; } } - uint256 hash = wtxIn.GetHash(); + const uint256& hash = wtxIn.GetHash(); const auto& ins = mapWallet.emplace(hash, wtxIn); CWalletTx& wtx = ins.first->second; wtx.BindWallet(this); if (/* insertion took place */ ins.second) { wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); } - AddToSpends(hash); + AddToSpends(wtx); for (const CTxIn& txin : wtx.tx->vin) { auto it = mapWallet.find(txin.prevout.hash); if (it != mapWallet.end()) { @@ -1502,8 +1498,13 @@ void CWallet::BlockUntilSyncedToCurrentChain() { // ::ChainActive().Tip(), otherwise put a callback in the validation interface queue and wait // for the queue to drain enough to execute it (indicating we are caught up // at least with the time we entered this function). - uint256 last_block_hash = WITH_LOCK(cs_wallet, return m_last_block_processed); - chain().waitForNotificationsIfNewBlocksConnected(last_block_hash); + auto lastProcessedBlock = [this]() -> uint256 { + return WITH_LOCK(cs_wallet, return m_last_block_processed); + }; + bool tipIsSame = false; + uint256 last_block_hash; + while (!tipIsSame && !(last_block_hash = lastProcessedBlock()).IsNull()) + tipIsSame = chain().waitForNotificationsIfTipIsNotSame(last_block_hash); } @@ -2242,7 +2243,7 @@ std::set CWalletTx::GetConflicts() const std::set result; if (pwallet != nullptr) { - uint256 myHash = GetHash(); + const uint256& myHash = GetHash(); result = pwallet->GetConflicts(myHash); result.erase(myHash); } @@ -2317,7 +2318,7 @@ CAmount CWalletTx::GetAvailableCredit(interfaces::Chain::Lock& locked_chain, boo bool allow_used_addresses = (filter & ISMINE_USED) || !pwallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE); CAmount nCredit = 0; - uint256 hashTx = GetHash(); + const uint256& hashTx = GetHash(); for (unsigned int i = 0; i < tx->vout.size(); i++) { if (!pwallet->IsSpent(locked_chain, hashTx, i) && (allow_used_addresses || !pwallet->IsUsedDestination(hashTx, i))) { @@ -3450,9 +3451,18 @@ DBErrors CWallet::ZapSelectTx(std::vector& vHashIn, std::vectorsecond.m_it_wtxOrdered); + for (auto& txin : it->second.tx->vin) { + auto mit = mapTxSpends.find(txin.prevout.hash); + if (mit != mapTxSpends.end()) { + if (mit->second.erase(txin.prevout.n) && mit->second.empty()) + mapTxSpends.erase(mit); + } + } mapWallet.erase(it); NotifyTransactionChanged(this, hash, CT_DELETED); } @@ -4096,7 +4106,7 @@ void CWallet::UnlockAllCoins() setLockedCoins.clear(); } -bool CWallet::IsLockedCoin(uint256 hash, unsigned int n) const +bool CWallet::IsLockedCoin(const uint256& hash, unsigned int n) const { AssertLockHeld(cs_wallet); COutPoint outpt(hash, n); @@ -4662,7 +4672,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, for (const CWalletTx& wtxOld : vWtx) { - uint256 hash = wtxOld.GetHash(); + const uint256& hash = wtxOld.GetHash(); auto mi = walletInstance->mapWallet.find(hash); if (mi != walletInstance->mapWallet.end()) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 472366e8d..a2e5d99c5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -789,7 +789,7 @@ private: typedef robin_hood::unordered_map>> TxSpends; TxSpends mapTxSpends GUARDED_BY(cs_wallet); void AddToSpends(const COutPoint& outpoint, const uint256& wtxid) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void AddToSpends(const uint256& wtxid) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void AddToSpends(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** * Add a transaction to the wallet, or update it. pIndex and posInBlock should @@ -1012,7 +1012,7 @@ public: std::vector GroupOutputs(const std::vector& outputs, bool single_coin) const; - bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool IsLockedCoin(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void LockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void UnlockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void UnlockAllCoins() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -1406,8 +1406,8 @@ public: /** Returns a bracketed wallet name for displaying in logs, will return [default wallet] if the wallet has no name */ const std::string GetDisplayName() const { - std::string wallet_name = GetName().length() == 0 ? "default wallet" : GetName(); - return strprintf("[%s]", wallet_name); + const std::string wallet_name = GetName(); + return strprintf("[%s]", wallet_name.empty() ? "default wallet" : wallet_name); }; /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 635997afc..0638ec555 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -78,7 +78,7 @@ bool WalletBatch::WriteTx(const CWalletTx& wtx) return WriteIC(std::make_pair(DBKeys::TX, wtx.GetHash()), wtx); } -bool WalletBatch::EraseTx(uint256 hash) +bool WalletBatch::EraseTx(const uint256& hash) { return EraseIC(std::make_pair(DBKeys::TX, hash)); } @@ -615,10 +615,10 @@ DBErrors WalletBatch::ZapSelectTx(std::vector& vTxHashIn, std::vector::iterator it = vTxHashIn.begin(); + auto it = vTxHashIn.begin(); for (const uint256& hash : vTxHash) { - while (it < vTxHashIn.end() && (*it) < hash) { - it++; + while (it != vTxHashIn.end() && (*it) < hash) { + ++it; } if (it == vTxHashIn.end()) { break; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 0fee35934..ab26d1e08 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -220,7 +220,7 @@ public: bool ErasePurpose(const std::string& strAddress); bool WriteTx(const CWalletTx& wtx); - bool EraseTx(uint256 hash); + bool EraseTx(const uint256& hash); bool WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, const bool overwrite); bool WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata &keyMeta);