diff --git a/src/init.cpp b/src/init.cpp index cd98e6f568d8..0c40d23905ef 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1908,7 +1908,7 @@ bool AppInitMain() #ifdef ENABLE_WALLET if (pwalletMain) { - pwalletMain->postInitProcess(threadGroup); + pwalletMain->postInitProcess(scheduler); // StakeMiner thread disabled by default on regtest if (gArgs.GetBoolArg("-staking", !Params().IsRegTestNet() && DEFAULT_STAKING)) { diff --git a/src/miner.cpp b/src/miner.cpp index 650541bb4fc9..89628ce6e922 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -521,9 +521,6 @@ bool ProcessBlockFound(const std::shared_ptr& pblock, CWallet& wal if (reservekey) reservekey->KeepKey(); - // Inform about the new block - GetMainSignals().BlockFound(pblock->GetHash()); - // Process this block the same as if we had received it from another node CValidationState state; if (!ProcessNewBlock(state, nullptr, pblock, nullptr)) { diff --git a/src/net.cpp b/src/net.cpp index 792ff0a05ed4..63eb1f31ea69 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2142,7 +2142,7 @@ bool CConnman::Start(CScheduler& scheduler, std::string& strNodeError, Options c threadMessageHandler = std::thread(&TraceThread >, "msghand", std::function(std::bind(&CConnman::ThreadMessageHandler, this))); // Dump network addresses - scheduler.scheduleEvery(std::bind(&CConnman::DumpData, this), DUMP_ADDRESSES_INTERVAL); + scheduler.scheduleEvery(std::bind(&CConnman::DumpData, this), DUMP_ADDRESSES_INTERVAL * 1000); return true; } diff --git a/src/net_processing.h b/src/net_processing.h index 9924cf704d00..bc7abeb5ebdf 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -41,8 +41,8 @@ class PeerLogicValidation : public CValidationInterface { PeerLogicValidation(CConnman* connmanIn); ~PeerLogicValidation() = default; - virtual void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload); - virtual void BlockChecked(const CBlock& block, const CValidationState& state); + void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; + void BlockChecked(const CBlock& block, const CValidationState& state) override; }; struct CNodeStateStats { diff --git a/src/sapling/saplingscriptpubkeyman.cpp b/src/sapling/saplingscriptpubkeyman.cpp index 196d21242931..562bb5d2f612 100644 --- a/src/sapling/saplingscriptpubkeyman.cpp +++ b/src/sapling/saplingscriptpubkeyman.cpp @@ -300,11 +300,11 @@ void DecrementNoteWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t n } } -void SaplingScriptPubKeyMan::DecrementNoteWitnesses(const CBlockIndex* pindex) +void SaplingScriptPubKeyMan::DecrementNoteWitnesses(int nChainHeight) { LOCK(wallet->cs_wallet); for (std::pair& wtxItem : wallet->mapWallet) { - ::DecrementNoteWitnesses(wtxItem.second.mapSaplingNoteData, pindex->nHeight, nWitnessCacheSize); + ::DecrementNoteWitnesses(wtxItem.second.mapSaplingNoteData, nChainHeight, nWitnessCacheSize); } nWitnessCacheSize -= 1; nWitnessCacheNeedsUpdate = true; diff --git a/src/sapling/saplingscriptpubkeyman.h b/src/sapling/saplingscriptpubkeyman.h index dec4b7b3abac..ae2f800c7951 100644 --- a/src/sapling/saplingscriptpubkeyman.h +++ b/src/sapling/saplingscriptpubkeyman.h @@ -72,7 +72,7 @@ class SaplingNoteData * Block height corresponding to the most current witness. * * When we first create a SaplingNoteData in SaplingScriptPubKeyMan::FindMySaplingNotes, this is set to - * -1 as a placeholder. The next time CWallet::ChainTip is called, we can + * -1 as a placeholder. The next time CWallet::BlockConnected/CWallet::BlockDisconnected is called, we can * determine what height the witness cache for this note is valid for (even * if no witnesses were cached), and so can set the correct value in * SaplingScriptPubKeyMan::IncrementNoteWitnesses and SaplingScriptPubKeyMan::DecrementNoteWitnesses. @@ -163,9 +163,9 @@ class SaplingScriptPubKeyMan { const CBlock* pblock, SaplingMerkleTree& saplingTree); /** - * pindex is the old tip being disconnected. + * nChainHeight is the old tip height being disconnected. */ - void DecrementNoteWitnesses(const CBlockIndex* pindex); + void DecrementNoteWitnesses(int nChainHeight); /** * Update mapSaplingNullifiersToNotes diff --git a/src/scheduler.cpp b/src/scheduler.cpp index e757e9ffa8d9..bb671645b360 100644 --- a/src/scheduler.cpp +++ b/src/scheduler.cpp @@ -93,20 +93,20 @@ void CScheduler::schedule(CScheduler::Function f, boost::chrono::system_clock::t newTaskScheduled.notify_one(); } -void CScheduler::scheduleFromNow(CScheduler::Function f, int64_t deltaSeconds) +void CScheduler::scheduleFromNow(CScheduler::Function f, int64_t deltaMilliSeconds) { - schedule(f, boost::chrono::system_clock::now() + boost::chrono::seconds(deltaSeconds)); + schedule(f, boost::chrono::system_clock::now() + boost::chrono::milliseconds(deltaMilliSeconds)); } -static void Repeat(CScheduler* s, CScheduler::Function f, int64_t deltaSeconds) +static void Repeat(CScheduler* s, CScheduler::Function f, int64_t deltaMilliSeconds) { f(); - s->scheduleFromNow(std::bind(&Repeat, s, f, deltaSeconds), deltaSeconds); + s->scheduleFromNow(std::bind(&Repeat, s, f, deltaMilliSeconds), deltaMilliSeconds); } -void CScheduler::scheduleEvery(CScheduler::Function f, int64_t deltaSeconds) +void CScheduler::scheduleEvery(CScheduler::Function f, int64_t deltaMilliSeconds) { - scheduleFromNow(std::bind(&Repeat, this, f, deltaSeconds), deltaSeconds); + scheduleFromNow(std::bind(&Repeat, this, f, deltaMilliSeconds), deltaMilliSeconds); } size_t CScheduler::getQueueInfo(boost::chrono::system_clock::time_point &first, diff --git a/src/scheduler.h b/src/scheduler.h index ced6098958f4..9f1b703dd8f0 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -44,15 +44,15 @@ class CScheduler // Call func at/after time t void schedule(Function f, boost::chrono::system_clock::time_point t); - // Convenience method: call f once deltaSeconds from now - void scheduleFromNow(Function f, int64_t deltaSeconds); + // Convenience method: call f once deltaMilliSeconds from now + void scheduleFromNow(Function f, int64_t deltaMilliSeconds); // Another convenience method: call f approximately - // every deltaSeconds forever, starting deltaSeconds from now. + // every deltaMilliSeconds forever, starting deltaMilliSeconds from now. // To be more precise: every time f is finished, it - // is rescheduled to run deltaSeconds later. If you + // is rescheduled to run deltaMilliSeconds later. If you // need more accurate scheduling, don't use this method. - void scheduleEvery(Function f, int64_t deltaSeconds); + void scheduleEvery(Function f, int64_t deltaMilliSeconds); // To keep things as simple as possible, there is no unschedule. diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 9ed326af740a..0ca5a76ae982 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -53,18 +53,18 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) CTxMemPool testPool(CFeeRate(0)); - std::vector removed; // Nothing in pool, remove should do nothing: - testPool.removeRecursive(txParent, &removed); - BOOST_CHECK_EQUAL(removed.size(), 0); + unsigned int poolSize = testPool.size(); + testPool.removeRecursive(txParent); + BOOST_CHECK_EQUAL(testPool.size(), poolSize); // Just the parent: testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); - testPool.removeRecursive(txParent, &removed); - BOOST_CHECK_EQUAL(removed.size(), 1); - removed.clear(); - + poolSize = testPool.size(); + testPool.removeRecursive(txParent); + BOOST_CHECK_EQUAL(testPool.size(), poolSize - 1); + // Parent, children, grandchildren: testPool.addUnchecked(txParent.GetHash(), entry.FromTx(txParent)); for (int i = 0; i < 3; i++) @@ -73,19 +73,21 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) testPool.addUnchecked(txGrandChild[i].GetHash(), entry.FromTx(txGrandChild[i])); } // Remove Child[0], GrandChild[0] should be removed: - testPool.removeRecursive(txChild[0], &removed); - BOOST_CHECK_EQUAL(removed.size(), 2); - removed.clear(); + poolSize = testPool.size(); + testPool.removeRecursive(txChild[0]); + BOOST_CHECK_EQUAL(testPool.size(), poolSize - 2); // ... make sure grandchild and child are gone: - testPool.removeRecursive(txGrandChild[0], &removed); - BOOST_CHECK_EQUAL(removed.size(), 0); - testPool.removeRecursive(txChild[0], &removed); - BOOST_CHECK_EQUAL(removed.size(), 0); + poolSize = testPool.size(); + testPool.removeRecursive(txGrandChild[0]); + BOOST_CHECK_EQUAL(testPool.size(), poolSize); + poolSize = testPool.size(); + testPool.removeRecursive(txChild[0]); + BOOST_CHECK_EQUAL(testPool.size(), poolSize); // Remove parent, all children/grandchildren should go: - testPool.removeRecursive(txParent, &removed); - BOOST_CHECK_EQUAL(removed.size(), 5); + poolSize = testPool.size(); + testPool.removeRecursive(txParent); + BOOST_CHECK_EQUAL(testPool.size(), poolSize - 5); BOOST_CHECK_EQUAL(testPool.size(), 0); - removed.clear(); // Add children and grandchildren, but NOT the parent (simulate the parent being in a block) for (int i = 0; i < 3; i++) @@ -95,10 +97,10 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) } // Now remove the parent, as might happen if a block-re-org occurs but the parent cannot be // put into the mempool (maybe because it is non-standard): - testPool.removeRecursive(txParent, &removed); - BOOST_CHECK_EQUAL(removed.size(), 6); + poolSize = testPool.size(); + testPool.removeRecursive(txParent); + BOOST_CHECK_EQUAL(testPool.size(), poolSize - 6); BOOST_CHECK_EQUAL(testPool.size(), 0); - removed.clear(); } template @@ -415,7 +417,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) /* after tx6 is mined, tx7 should move up in the sort */ std::vector vtx; vtx.emplace_back(MakeTransactionRef(tx6)); - pool.removeForBlock(vtx, 1, nullptr, false); + pool.removeForBlock(vtx, 1); sortedOrder.erase(sortedOrder.begin()+1); // Ties are broken by hash diff --git a/src/txmempool.cpp b/src/txmempool.cpp index cc1797a63a79..2e9987e9c228 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -368,13 +368,12 @@ void CTxMemPool::AddTransactionsUpdated(unsigned int n) nTransactionsUpdated += n; } - bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate) { + NotifyEntryAdded(entry.GetSharedTx()); // Add to memory pool without checking anything. // Used by AcceptToMemoryPool(), which DOES do all the appropriate checks. LOCK(cs); - indexed_transaction_set::iterator newit = mapTx.insert(entry).first; mapLinks.insert(make_pair(newit, TxLinks())); @@ -433,8 +432,9 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, return true; } -void CTxMemPool::removeUnchecked(txiter it) +void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) { + NotifyEntryRemoved(it->GetSharedTx(), reason); AssertLockHeld(cs); const CTransaction& tx = it->GetTx(); for (const CTxIn& txin : tx.vin) @@ -483,7 +483,7 @@ void CTxMemPool::CalculateDescendants(txiter entryit, setEntries &setDescendants } } -void CTxMemPool::removeRecursive(const CTransaction& origTx, std::vector* removed) +void CTxMemPool::removeRecursive(const CTransaction& origTx, MemPoolRemovalReason reason) { // Remove transaction from memory pool { @@ -510,12 +510,8 @@ void CTxMemPool::removeRecursive(const CTransaction& origTx, std::vectoremplace_back(it->GetSharedTx()); - } - } - RemoveStaged(setAllRemoves, false); + + RemoveStaged(setAllRemoves, false, reason); } } @@ -547,7 +543,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem for (txiter it : txToRemove) { CalculateDescendants(it, setAllRemoves); } - RemoveStaged(setAllRemoves, false); + RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG); } void CTxMemPool::removeWithAnchor(const uint256& invalidRoot) @@ -574,7 +570,7 @@ void CTxMemPool::removeWithAnchor(const uint256& invalidRoot) } } -void CTxMemPool::removeConflicts(const CTransaction& tx, std::vector* removed) +void CTxMemPool::removeConflicts(const CTransaction& tx) { // Remove transactions which depend on inputs of tx, recursively std::list result; @@ -584,7 +580,8 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::vectorsecond; if (txConflict != tx) { - removeRecursive(txConflict, removed); + removeRecursive(txConflict, MemPoolRemovalReason::CONFLICT); + ClearPrioritisation(txConflict.GetHash()); } } } @@ -595,7 +592,8 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::vectorsecond; if (txConflict != tx) { - removeRecursive(txConflict, removed); + removeRecursive(txConflict, MemPoolRemovalReason::CONFLICT); + ClearPrioritisation(txConflict.GetHash()); } } } @@ -606,7 +604,7 @@ void CTxMemPool::removeConflicts(const CTransaction& tx, std::vector& vtx, unsigned int nBlockHeight, - std::vector* conflicts, bool fCurrentEstimate) + bool fCurrentEstimate) { LOCK(cs); std::vector entries; @@ -621,9 +619,9 @@ void CTxMemPool::removeForBlock(const std::vector& vtx, unsigne if (it != mapTx.end()) { setEntries stage; stage.insert(it); - RemoveStaged(stage, true); + RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK); } - removeConflicts(*tx, conflicts); + removeConflicts(*tx); ClearPrioritisation(tx->GetHash()); } // After the txs in the new block have been removed from the mempool, update policy estimates @@ -1064,12 +1062,12 @@ size_t CTxMemPool::DynamicMemoryUsage() const memusage::DynamicUsage(mapSaplingNullifiers); } -void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants) +void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason) { AssertLockHeld(cs); UpdateForRemoveFromMempool(stage, updateDescendants); for (const txiter& it : stage) { - removeUnchecked(it); + removeUnchecked(it, reason); } } @@ -1086,7 +1084,7 @@ int CTxMemPool::Expire(int64_t time) for (const txiter& removeit : toremove) { CalculateDescendants(removeit, stage); } - RemoveStaged(stage, false); + RemoveStaged(stage, false, MemPoolRemovalReason::EXPIRY); return stage.size(); } @@ -1197,7 +1195,7 @@ void CTxMemPool::TrimToSize(size_t sizelimit, std::vector* pvNoSpends for (txiter it: stage) txn.push_back(it->GetTx()); } - RemoveStaged(stage, false); + RemoveStaged(stage, false, MemPoolRemovalReason::SIZELIMIT); if (pvNoSpendsRemaining) { for (const CTransaction& tx: txn) { for (const CTxIn& txin: tx.vin) { diff --git a/src/txmempool.h b/src/txmempool.h index 027aa3c0d91e..6e29cd796507 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -23,6 +23,8 @@ #include "boost/multi_index/ordered_index.hpp" #include "boost/multi_index/hashed_index.hpp" +#include + class CAutoFile; inline double AllowFreeThreshold() @@ -303,6 +305,19 @@ struct TxMempoolInfo int64_t nFeeDelta; }; +/** Reason why a transaction was removed from the mempool, + * this is passed to the notification signal. + */ +enum class MemPoolRemovalReason { + UNKNOWN = 0, //! Manually removed or unknown reason + EXPIRY, //! Expired from mempool + SIZELIMIT, //! Removed in size limiting + REORG, //! Removed for reorganization + BLOCK, //! Removed for block + CONFLICT, //! Removed for conflict with in-block transaction + REPLACED //! Removed for replacement +}; + /** * CTxMemPool stores valid-according-to-the-current-best-chain * transactions that may be included in the next block. @@ -522,12 +537,14 @@ class CTxMemPool // then invoke the second version. bool addUnchecked(const uint256& hash, const CTxMemPoolEntry& entry, bool fCurrentEstimate = true); bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, setEntries &setAncestors, bool fCurrentEstimate = true); - void removeRecursive(const CTransaction& tx, std::vector* removed = nullptr); + + void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags); void removeWithAnchor(const uint256& invalidRoot); - void removeConflicts(const CTransaction& tx, std::vector* removed = nullptr); + void removeConflicts(const CTransaction& tx); void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, - std::vector* conflicts = nullptr, bool fCurrentEstimate = true); + bool fCurrentEstimate = true); + void clear(); void _clear(); // lock-free bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); @@ -556,7 +573,7 @@ class CTxMemPool * Set updateDescendants to true when removing a tx that was in a block, so * that any in-mempool descendants have their ancestor state updated. */ - void RemoveStaged(setEntries &stage, bool updateDescendants); + void RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); /** When adding transactions from a disconnected block back to the mempool, * new mempool entries may have children in the mempool (which is generally @@ -650,6 +667,9 @@ class CTxMemPool size_t DynamicMemoryUsage() const; + boost::signals2::signal NotifyEntryAdded; + boost::signals2::signal NotifyEntryRemoved; + private: /** UpdateForDescendants is used by UpdateTransactionsFromBlock to update * the descendants for a single transaction that has been added to the @@ -690,7 +710,7 @@ class CTxMemPool * transactions in a chain before we've updated all the state for the * removal. */ - void removeUnchecked(txiter entry); + void removeUnchecked(txiter entry, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); }; /** diff --git a/src/validation.cpp b/src/validation.cpp index b2cb3df078ec..97a2bf7b9af7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -26,8 +26,8 @@ #include "guiinterface.h" #include "init.h" #include "invalid.h" +#include "interfaces/handler.h" #include "legacy/validation_zerocoin_legacy.h" -#include "libzerocoin/Denominations.h" #include "kernel.h" #include "masternode-payments.h" #include "masternode-sync.h" @@ -174,7 +174,6 @@ std::set setDirtyBlockIndex; std::set setDirtyFileInfo; } // anon namespace - CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) { // Find the first block the caller has in the main chain @@ -606,7 +605,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool full"); } - GetMainSignals().SyncTransaction(tx, nullptr, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); + GetMainSignals().TransactionAddedToMempool(_tx); return true; } @@ -1952,7 +1951,8 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara CBlockIndex* pindexDelete = chainActive.Tip(); assert(pindexDelete); // Read block from disk. - CBlock block; + std::shared_ptr pblock = std::make_shared(); + CBlock& block = *pblock; if (!ReadBlockFromDisk(block, pindexDelete)) return AbortNode(state, "Failed to read block"); // Apply the block atomically to the chain state. @@ -1975,7 +1975,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara // ignore validation errors in resurrected transactions CValidationState stateDummy; if (tx->IsCoinBase() || tx->IsCoinStake() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, nullptr, true)) { - mempool.removeRecursive(*tx); + mempool.removeRecursive(*tx, MemPoolRemovalReason::REORG); } else if (mempool.exists(tx->GetHash())) { vHashUpdate.push_back(tx->GetHash()); } @@ -2004,14 +2004,7 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara UpdateTip(pindexDelete->pprev); // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: - for (const auto& tx : block.vtx) { - GetMainSignals().SyncTransaction(*tx, pindexDelete->pprev, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); - } - - if (chainparams.GetConsensus().NetworkUpgradeActive(pindexDelete->nHeight, Consensus::UPGRADE_V5_0)) { - // Update Sapling cached incremental witnesses - GetMainSignals().ChainTip(pindexDelete, &block, nullopt); - } + GetMainSignals().BlockDisconnected(pblock, pindexDelete->nHeight); return true; } @@ -2022,22 +2015,77 @@ static int64_t nTimeFlush = 0; static int64_t nTimeChainState = 0; static int64_t nTimePostConnect = 0; +struct PerBlockConnectTrace { + CBlockIndex* pindex = nullptr; + std::shared_ptr pblock; + std::shared_ptr> conflictedTxs; + PerBlockConnectTrace() : conflictedTxs(std::make_shared>()) {} +}; /** - * Used to track conflicted transactions removed from mempool and transactions - * applied to the UTXO state as a part of a single ActivateBestChainStep call. + * Used to track blocks whose transactions were applied to the UTXO state as a + * part of a single ActivateBestChainStep call. + * + * This class also tracks transactions that are removed from the mempool as + * conflicts (per block) and can be used to pass all those transactions + * through SyncTransaction. + * + * This class assumes (and asserts) that the conflicted transactions for a given + * block are added via mempool callbacks prior to the BlockConnected() associated + * with those transactions. If any transactions are marked conflicted, it is + * assumed that an associated block will always be added. + * + * This class is single-use, once you call GetBlocksConnected() you have to throw + * it away and make a new one. */ -struct ConnectTrace { - std::vector txConflicted; - std::vector > > blocksConnected; +class ConnectTrace { +private: + std::vector blocksConnected; + CTxMemPool &pool; + std::unique_ptr m_handler_notify_entry_removed; + +public: + ConnectTrace(CTxMemPool &_pool) : blocksConnected(1), pool(_pool) { + m_handler_notify_entry_removed = interfaces::MakeHandler(pool.NotifyEntryRemoved.connect(std::bind(&ConnectTrace::NotifyEntryRemoved, this, std::placeholders::_1, std::placeholders::_2))); + } + + ~ConnectTrace() { + m_handler_notify_entry_removed->disconnect(); + } + + void BlockConnected(CBlockIndex* pindex, std::shared_ptr pblock) { + assert(!blocksConnected.back().pindex); + assert(pindex); + assert(pblock); + blocksConnected.back().pindex = pindex; + blocksConnected.back().pblock = std::move(pblock); + blocksConnected.emplace_back(); + } + + std::vector& GetBlocksConnected() { + // We always keep one extra block at the end of our list because + // blocks are added after all the conflicted transactions have + // been filled in. Thus, the last entry should always be an empty + // one waiting for the transactions from the next block. We pop + // the last entry here to make sure the list we return is sane. + assert(!blocksConnected.back().pindex); + assert(blocksConnected.back().conflictedTxs->empty()); + blocksConnected.pop_back(); + return blocksConnected; + } + + void NotifyEntryRemoved(CTransactionRef txRemoved, MemPoolRemovalReason reason) { + assert(!blocksConnected.back().pindex); + if (reason == MemPoolRemovalReason::CONFLICT) { + blocksConnected.back().conflictedTxs->emplace_back(std::move(txRemoved)); + } + } }; /** * Connect a new block to chainActive. pblock is either NULL or a pointer to a CBlock * corresponding to pindexNew, to bypass loading it again from disk. * - * The block is always added to connectTrace (either after loading from disk or by copying - * pblock) - if that is not intended, care must be taken to remove the last entry in - * blocksConnected in case of failure. + * The block is added to connectTrace if connection succeeds. */ bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr& pblock, bool fAlreadyChecked, ConnectTrace& connectTrace) { @@ -2048,15 +2096,16 @@ bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const st // Read block from disk. int64_t nTime1 = GetTimeMicros(); + std::shared_ptr pthisBlock; if (!pblock) { std::shared_ptr pblockNew = std::make_shared(); - connectTrace.blocksConnected.emplace_back(pindexNew, pblockNew); if (!ReadBlockFromDisk(*pblockNew, pindexNew)) return AbortNode(state, "Failed to read block"); + pthisBlock = pblockNew; } else { - connectTrace.blocksConnected.emplace_back(pindexNew, pblock); + pthisBlock = pblock; } - const CBlock& blockConnecting = *connectTrace.blocksConnected.back().second; + const CBlock& blockConnecting = *pthisBlock; // Apply the block atomically to the chain state. int64_t nTime2 = GetTimeMicros(); @@ -2092,7 +2141,7 @@ bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const st LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs]\n", (nTime5 - nTime4) * 0.001, nTimeChainState * 0.000001); // Remove conflicting transactions from the mempool. - mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, &connectTrace.txConflicted, !IsInitialBlockDownload()); + mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight, !IsInitialBlockDownload()); // Update chainActive & related variables. UpdateTip(pindexNew); // Update MN manager cache @@ -2104,6 +2153,8 @@ bool static ConnectTip(CValidationState& state, CBlockIndex* pindexNew, const st nTimeTotal += nTime6 - nTime1; LogPrint(BCLog::BENCH, " - Connect postprocess: %.2fms [%.2fs]\n", (nTime6 - nTime5) * 0.001, nTimePostConnect * 0.000001); LogPrint(BCLog::BENCH, "- Connect block: %.2fms [%.2fs]\n", (nTime6 - nTime1) * 0.001, nTimeTotal * 0.000001); + + connectTrace.BlockConnected(pindexNew, std::move(pthisBlock)); return true; } @@ -2227,8 +2278,6 @@ static bool ActivateBestChainStep(CValidationState& state, CBlockIndex* pindexMo state = CValidationState(); fInvalidFound = true; fContinue = false; - // If we didn't actually connect the block, don't notify listeners about it - connectTrace.blocksConnected.pop_back(); break; } else { // A system error occurred (disk space, database error, ...). @@ -2280,7 +2329,6 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb boost::this_thread::interruption_point(); const CBlockIndex *pindexFork; - ConnectTrace connectTrace; bool fInitialDownload; while (true) { TRY_LOCK(cs_main, lockMain); @@ -2288,6 +2336,7 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb MilliSleep(50); continue; } + ConnectTrace connectTrace(mempool); // Destructed before cs_main is unlocked CBlockIndex *pindexOldTip = chainActive.Tip(); pindexMostWork = FindMostWorkChain(); @@ -2304,33 +2353,9 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb pindexFork = chainActive.FindFork(pindexOldTip); fInitialDownload = IsInitialBlockDownload(); - // throw all transactions though the signal-interface - for (const auto &tx : connectTrace.txConflicted) { - GetMainSignals().SyncTransaction(*tx, pindexNewTip, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); - } - // ... and about transactions that got confirmed: - for (const auto& pair : connectTrace.blocksConnected) { - assert(pair.second); - const CBlock& block = *(pair.second); - for (unsigned int i = 0; i < block.vtx.size(); i++) { - GetMainSignals().SyncTransaction(*block.vtx[i], pair.first, i); - } - - // Sapling: notify wallet about the connected blocks ordered - // Get prev block tree anchor - CBlockIndex* pprev = pair.first->pprev; - SaplingMerkleTree oldSaplingTree; - bool isSaplingActive = (pprev) != nullptr && - Params().GetConsensus().NetworkUpgradeActive(pprev->nHeight, - Consensus::UPGRADE_V5_0); - if (isSaplingActive) { - assert(pcoinsTip->GetSaplingAnchorAt(pprev->hashFinalSaplingRoot, oldSaplingTree)); - } else { - assert(pcoinsTip->GetSaplingAnchorAt(SaplingMerkleTree::empty_root(), oldSaplingTree)); - } - - // Sapling: Update cached incremental witnesses - GetMainSignals().ChainTip(pair.first, &block, oldSaplingTree); + for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) { + assert(trace.pblock && trace.pindex); + GetMainSignals().BlockConnected(trace.pblock, trace.pindex, *trace.conflictedTxs); } break; diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index addcc91f29fa..9fa5029fe023 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -11,24 +11,29 @@ struct ValidationInterfaceConnections { boost::signals2::scoped_connection UpdatedBlockTip; - boost::signals2::scoped_connection SyncTransaction; + boost::signals2::scoped_connection TransactionAddedToMempool; + boost::signals2::scoped_connection BlockConnected; + boost::signals2::scoped_connection BlockDisconnected; boost::signals2::scoped_connection NotifyTransactionLock; boost::signals2::scoped_connection UpdatedTransaction; boost::signals2::scoped_connection SetBestChain; boost::signals2::scoped_connection Broadcast; boost::signals2::scoped_connection BlockChecked; - boost::signals2::scoped_connection BlockFound; - boost::signals2::scoped_connection ChainTip; }; struct MainSignalsInstance { -// XX42 boost::signals2::signal EraseTransaction; + /** Notifies listeners of updated block chain tip */ boost::signals2::signal UpdatedBlockTip; - /** A posInBlock value for SyncTransaction which indicates the transaction was conflicted, disconnected, or not in a block */ - static const int SYNC_TRANSACTION_NOT_IN_BLOCK = -1; - /** Notifies listeners of updated transaction data (transaction, and optionally the block it is found in. */ - boost::signals2::signal SyncTransaction; + /** Notifies listeners of a transaction having been added to mempool. */ + boost::signals2::signal TransactionAddedToMempool; + /** + * Notifies listeners of a block being connected. + * Provides a vector of transactions evicted from the mempool as a result. + */ + boost::signals2::signal &, const CBlockIndex *pindex, const std::vector &)> BlockConnected; + /** Notifies listeners of a block being disconnected */ + boost::signals2::signal &, int nBlockHeight)> BlockDisconnected; /** Notifies listeners of an updated transaction lock without new data. */ boost::signals2::signal NotifyTransactionLock; /** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */ @@ -39,11 +44,6 @@ struct MainSignalsInstance { boost::signals2::signal Broadcast; /** Notifies listeners of a block validation result */ boost::signals2::signal BlockChecked; - /** Notifies listeners that a block has been successfully mined */ - boost::signals2::signal BlockFound; - - /** Notifies listeners of a change to the tip of the active block chain. */ - boost::signals2::signal)> ChainTip; std::unordered_map m_connMainSignals; }; @@ -63,14 +63,14 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { ValidationInterfaceConnections& conns = g_signals.m_internals->m_connMainSignals[pwalletIn]; conns.UpdatedBlockTip = g_signals.m_internals->UpdatedBlockTip.connect(std::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); - conns.SyncTransaction = g_signals.m_internals->SyncTransaction.connect(std::bind(&CValidationInterface::SyncTransaction, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); - conns.ChainTip = g_signals.m_internals->ChainTip.connect(std::bind(&CValidationInterface::ChainTip, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); + conns.TransactionAddedToMempool = g_signals.m_internals->TransactionAddedToMempool.connect(std::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, std::placeholders::_1)); + conns.BlockConnected = g_signals.m_internals->BlockConnected.connect(std::bind(&CValidationInterface::BlockConnected, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); + conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect(std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, std::placeholders::_1, std::placeholders::_2)); conns.NotifyTransactionLock = g_signals.m_internals->NotifyTransactionLock.connect(std::bind(&CValidationInterface::NotifyTransactionLock, pwalletIn, std::placeholders::_1)); conns.UpdatedTransaction = g_signals.m_internals->UpdatedTransaction.connect(std::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, std::placeholders::_1)); conns.SetBestChain = g_signals.m_internals->SetBestChain.connect(std::bind(&CValidationInterface::SetBestChain, pwalletIn, std::placeholders::_1)); conns.Broadcast = g_signals.m_internals->Broadcast.connect(std::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, std::placeholders::_1)); conns.BlockChecked = g_signals.m_internals->BlockChecked.connect(std::bind(&CValidationInterface::BlockChecked, pwalletIn, std::placeholders::_1, std::placeholders::_2)); - conns.BlockFound = g_signals.m_internals->BlockFound.connect(std::bind(&CValidationInterface::ResetRequestCount, pwalletIn, std::placeholders::_1)); } void UnregisterValidationInterface(CValidationInterface* pwalletIn) @@ -92,8 +92,16 @@ void CMainSignals::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockInd m_internals->UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); } -void CMainSignals::SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, int posInBlock) { - m_internals->SyncTransaction(tx, pindex, posInBlock); +void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptxn) { + m_internals->TransactionAddedToMempool(ptxn); +} + +void CMainSignals::BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::vector &txnConflicted) { + m_internals->BlockConnected(block, pindex, txnConflicted); +} + +void CMainSignals::BlockDisconnected(const std::shared_ptr &block, int nBlockHeight) { + m_internals->BlockDisconnected(block, nBlockHeight); } void CMainSignals::NotifyTransactionLock(const CTransaction& tx) { @@ -115,11 +123,3 @@ void CMainSignals::Broadcast(CConnman* connman) { void CMainSignals::BlockChecked(const CBlock& block, const CValidationState& state) { m_internals->BlockChecked(block, state); } - -void CMainSignals::BlockFound(const uint256& hash) { - m_internals->BlockFound(hash); -} - -void CMainSignals::ChainTip(const CBlockIndex* pindex, const CBlock* block, Optional tree) { - m_internals->ChainTip(pindex, block, tree); -} \ No newline at end of file diff --git a/src/validationinterface.h b/src/validationinterface.h index 3519791c7404..af980450a250 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -9,13 +9,12 @@ #include "optional.h" #include "sapling/incrementalmerkletree.h" +#include "primitives/transaction.h" class CBlock; struct CBlockLocator; class CBlockIndex; class CConnman; -class CReserveScript; -class CTransaction; class CValidationInterface; class CValidationState; class uint256; @@ -33,8 +32,9 @@ class CValidationInterface { protected: /** Notifies listeners of updated block chain tip */ virtual void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) {} - virtual void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock) {} - virtual void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, Optional added) {} + virtual void TransactionAddedToMempool(const CTransactionRef &ptxn) {} + virtual void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::vector &txnConflicted) {} + virtual void BlockDisconnected(const std::shared_ptr &block, int nBlockHeight) {} virtual void NotifyTransactionLock(const CTransaction &tx) {} /** Notifies listeners of the new active block chain on-disk. */ virtual void SetBestChain(const CBlockLocator &locator) {} @@ -42,7 +42,6 @@ class CValidationInterface { /** Tells listeners to broadcast their data. */ virtual void ResendWalletTransactions(CConnman* connman) {} virtual void BlockChecked(const CBlock&, const CValidationState&) {} - virtual void ResetRequestCount(const uint256 &hash) {}; friend void ::RegisterValidationInterface(CValidationInterface*); friend void ::UnregisterValidationInterface(CValidationInterface*); friend void ::UnregisterAllValidationInterfaces(); @@ -59,18 +58,15 @@ class CMainSignals { public: CMainSignals(); - /** A posInBlock value for SyncTransaction which indicates the transaction was conflicted, disconnected, or not in a block */ - static const int SYNC_TRANSACTION_NOT_IN_BLOCK = -1; - void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload); - void SyncTransaction(const CTransaction &, const CBlockIndex *pindex, int posInBlock); + void TransactionAddedToMempool(const CTransactionRef &ptxn); + void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::vector &txnConflicted); + void BlockDisconnected(const std::shared_ptr &block, int nBlockHeight); void NotifyTransactionLock(const CTransaction&); void UpdatedTransaction(const uint256 &); void SetBestChain(const CBlockLocator &); void Broadcast(CConnman* connman); void BlockChecked(const CBlock&, const CValidationState&); - void BlockFound(const uint256&); - void ChainTip(const CBlockIndex *, const CBlock *, Optional); }; CMainSignals& GetMainSignals(); diff --git a/src/wallet/test/wallet_shielded_balances_tests.cpp b/src/wallet/test/wallet_shielded_balances_tests.cpp index a6dff16969f9..2fc4bcdcc169 100644 --- a/src/wallet/test/wallet_shielded_balances_tests.cpp +++ b/src/wallet/test/wallet_shielded_balances_tests.cpp @@ -348,7 +348,9 @@ BOOST_AUTO_TEST_CASE(GetShieldedAvailableCredit) // 2) Confirm the tx SaplingMerkleTree tree; FakeBlock fakeBlock = SimpleFakeMine(wtxUpdated, tree); - wallet.ChainTip(fakeBlock.pindex, &fakeBlock.block, tree); + // Simulate receiving a new block and updating the witnesses/nullifiers + wallet.IncrementNoteWitnesses(fakeBlock.pindex, &fakeBlock.block, tree); + wallet.GetSaplingScriptPubKeyMan()->UpdateSaplingNullifierNoteMapForBlock(&fakeBlock.block); wtxUpdated = wallet.mapWallet[wtxUpdated.GetHash()]; // 3) Now can spend one output and recalculate the shielded credit. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3b80076a2bf7..a0832e61d527 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -15,6 +15,7 @@ #include "policy/policy.h" #include "sapling/key_io_sapling.h" #include "script/sign.h" +#include "scheduler.h" #include "spork.h" #include "util.h" #include "utilmoneystr.h" @@ -444,18 +445,6 @@ void CWallet::ChainTipAdded(const CBlockIndex *pindex, m_sspk_man->UpdateSaplingNullifierNoteMapForBlock(pblock); } -void CWallet::ChainTip(const CBlockIndex *pindex, - const CBlock *pblock, - Optional added) -{ - if (added) { - ChainTipAdded(pindex, pblock, added.get()); - } else { - DecrementNoteWitnesses(pindex); - m_sspk_man->UpdateSaplingNullifierNoteMapForBlock(pblock); - } -} - void CWallet::SetBestChain(const CBlockLocator& loc) { CWalletDB walletdb(*dbw); @@ -1047,12 +1036,21 @@ void CWallet::AddExternalNotesDataToTx(CWalletTx& wtx) const } /** - * Add a transaction to the wallet, or update it. - * pblock is optional, but should be provided if the transaction is known to be in a block. + * Add a transaction to the wallet, or update it. pIndex and posInBlock should + * be set when the transaction was known to be included in a block. When + * posInBlock = SYNC_TRANSACTION_NOT_IN_BLOCK (-1) , then wallet state is not + * updated in AddToWallet, but notifications happen and cached balances are + * marked dirty. * If fUpdate is true, existing transactions will be updated. + * TODO: One exception to this is that the abandoned state is cleared under the + * assumption that any further notification of a transaction that was considered + * abandoned is an indication that it is not safe to be considered abandoned. + * Abandoned state should probably be more carefully tracked via different + * posInBlock signals or by checking mempool presence when necessary. */ -bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const uint256& blockHash, int posInBlock, bool fUpdate) +bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const uint256& blockHash, int posInBlock, bool fUpdate) { + const CTransaction& tx = *ptx; { AssertLockHeld(cs_wallet); @@ -1231,13 +1229,71 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) } } -void CWallet::SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock) +void CWallet::SyncTransaction(const CTransactionRef& ptx, const CBlockIndex *pindexBlockConnected, int posInBlock) { - LOCK(cs_wallet); - if (!AddToWalletIfInvolvingMe(tx, (pindex) ? pindex->GetBlockHash() : uint256(), posInBlock, true)) + if (!AddToWalletIfInvolvingMe(ptx, + (pindexBlockConnected) ? pindexBlockConnected->GetBlockHash() : uint256(), + posInBlock, + true)) { return; // Not one of ours + } + + MarkAffectedTransactionsDirty(*ptx); +} + +void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) +{ + LOCK2(cs_main, cs_wallet); + SyncTransaction(ptx, NULL, -1); +} + +void CWallet::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) +{ + LOCK2(cs_main, cs_wallet); + // TODO: Tempoarily ensure that mempool removals are notified before + // connected transactions. This shouldn't matter, but the abandoned + // state of transactions in our wallet is currently cleared when we + // receive another notification and there is a race condition where + // notification of a connected conflict might cause an outside process + // to abandon a transaction and then have it inadvertently cleared by + // the notification that the conflicted transaction was evicted. + + for (const CTransactionRef& ptx : vtxConflicted) { + SyncTransaction(ptx, nullptr, -1); + } + for (size_t i = 0; i < pblock->vtx.size(); i++) { + SyncTransaction(pblock->vtx[i], pindex, i); + } + + // Sapling: notify about the connected block + // Get prev block tree anchor + CBlockIndex* pprev = pindex->pprev; + SaplingMerkleTree oldSaplingTree; + bool isSaplingActive = (pprev) != nullptr && + Params().GetConsensus().NetworkUpgradeActive(pprev->nHeight, + Consensus::UPGRADE_V5_0); + if (isSaplingActive) { + assert(pcoinsTip->GetSaplingAnchorAt(pprev->hashFinalSaplingRoot, oldSaplingTree)); + } else { + assert(pcoinsTip->GetSaplingAnchorAt(SaplingMerkleTree::empty_root(), oldSaplingTree)); + } + + // Sapling: Update cached incremental witnesses + ChainTipAdded(pindex, pblock.get(), oldSaplingTree); +} + +void CWallet::BlockDisconnected(const std::shared_ptr& pblock, int nBlockHeight) +{ + LOCK2(cs_main, cs_wallet); + for (const CTransactionRef& ptx : pblock->vtx) { + SyncTransaction(ptx, NULL, -1); + } - MarkAffectedTransactionsDirty(tx); + if (Params().GetConsensus().NetworkUpgradeActive(nBlockHeight, Consensus::UPGRADE_V5_0)) { + // Update Sapling cached incremental witnesses + m_sspk_man->DecrementNoteWitnesses(nBlockHeight); + m_sspk_man->UpdateSaplingNullifierNoteMapForBlock(pblock.get()); + } } void CWallet::MarkAffectedTransactionsDirty(const CTransaction& tx) @@ -1701,7 +1757,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate, b int posInBlock; for (posInBlock = 0; posInBlock < (int)block.vtx.size(); posInBlock++) { const auto& tx = block.vtx[posInBlock]; - if (AddToWalletIfInvolvingMe(*tx, pindex->GetBlockHash(), posInBlock, fUpdate)) { + if (AddToWalletIfInvolvingMe(tx, pindex->GetBlockHash(), posInBlock, fUpdate)) { myTxHashes.push_back(tx->GetHash()); ret++; } @@ -1779,10 +1835,7 @@ void CWallet::ReacceptWalletTransactions(bool fFirstLoad) bool CWalletTx::InMempool() const { LOCK(mempool.cs); - if (mempool.exists(GetHash())) { - return true; - } - return false; + return mempool.exists(GetHash()); } void CWalletTx::RelayWalletTransaction(CConnman* connman) @@ -4128,16 +4181,16 @@ bool CWallet::InitLoadWallet() return true; } -std::atomic CWallet::fFlushThreadRunning(false); +std::atomic CWallet::fFlushScheduled(false); -void CWallet::postInitProcess(boost::thread_group& threadGroup) +void CWallet::postInitProcess(CScheduler& scheduler) { // Add wallet transactions that aren't already in a block to mapTransactions ReacceptWalletTransactions(/*fFirstLoad*/true); // Run a thread to flush wallet periodically - if (!CWallet::fFlushThreadRunning.exchange(true)) { - threadGroup.create_thread(ThreadFlushWalletDB); + if (!CWallet::fFlushScheduled.exchange(true)) { + scheduler.scheduleEvery(MaybeCompactWalletDB, 500); } } @@ -4447,7 +4500,7 @@ void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex, const CBlock* pblock, SaplingMerkleTree& saplingTree) { m_sspk_man->IncrementNoteWitnesses(pindex, pblock, saplingTree); } -void CWallet::DecrementNoteWitnesses(const CBlockIndex* pindex) { m_sspk_man->DecrementNoteWitnesses(pindex); } +void CWallet::DecrementNoteWitnesses(const CBlockIndex* pindex) { m_sspk_man->DecrementNoteWitnesses(pindex->nHeight); } bool CWallet::AddSaplingZKey(const libzcash::SaplingExtendedSpendingKey &key) { return m_sspk_man->AddSaplingZKey(key); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 92c56abda32f..b8453670bf5c 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -44,8 +44,6 @@ #include #include -#include - extern CWallet* pwalletMain; /** @@ -94,6 +92,7 @@ class COutput; class CStakeableOutput; class CReserveKey; class CScript; +class CScheduler; class CWalletTx; class ScriptPubKeyMan; class SaplingScriptPubKeyMan; @@ -265,7 +264,7 @@ typedef std::map mapSaplingNoteData_t; class CWallet : public CCryptoKeyStore, public CValidationInterface { private: - static std::atomic fFlushThreadRunning; + static std::atomic fFlushScheduled; //! keeps track of whether Unlock has run a thorough check before bool fDecryptionThoroughlyChecked{false}; @@ -302,6 +301,9 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void SyncMetaData(std::pair::iterator, typename TxSpendMap::iterator> range); void ChainTipAdded(const CBlockIndex *pindex, const CBlock *pblock, SaplingMerkleTree saplingTree); + /* Used by TransactionAddedToMemorypool/BlockConnected/Disconnected */ + void SyncTransaction(const CTransactionRef& tx, const CBlockIndex *pindexBlockConnected, int posInBlock); + bool IsKeyUsed(const CPubKey& vchPubKey); struct OutputAvailabilityResult @@ -552,7 +554,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface //////////// End Sapling ////////////// //! Adds a key to the store, and saves it to disk. - bool AddKeyPubKey(const CKey& key, const CPubKey& pubkey); + bool AddKeyPubKey(const CKey& key, const CPubKey& pubkey) override; //! Adds a key to the store, without saving it to disk (used by LoadWallet) bool LoadKey(const CKey& key, const CPubKey& pubkey) { return CCryptoKeyStore::AddKeyPubKey(key, pubkey); } //! Load metadata (used by LoadWallet) @@ -561,10 +563,10 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool LoadMinVersion(int nVersion); //! Adds an encrypted key to the store, and saves it to disk. - bool AddCryptedKey(const CPubKey& vchPubKey, const std::vector& vchCryptedSecret); + bool AddCryptedKey(const CPubKey& vchPubKey, const std::vector& vchCryptedSecret) override; //! Adds an encrypted key to the store, without saving it to disk (used by LoadWallet) bool LoadCryptedKey(const CPubKey& vchPubKey, const std::vector& vchCryptedSecret); - bool AddCScript(const CScript& redeemScript); + bool AddCScript(const CScript& redeemScript) override; bool LoadCScript(const CScript& redeemScript); //! Adds a destination data tuple to the store, and saves it to disk @@ -575,8 +577,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool LoadDestData(const CTxDestination& dest, const std::string& key, const std::string& value); //! Adds a watch-only address to the store, and saves it to disk. - bool AddWatchOnly(const CScript& dest); - bool RemoveWatchOnly(const CScript& dest); + bool AddWatchOnly(const CScript& dest) override; + bool RemoveWatchOnly(const CScript& dest) override; //! Adds a watch-only address to the store, without saving it to disk (used by LoadWallet) bool LoadWatchOnly(const CScript& dest); @@ -599,8 +601,10 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void MarkDirty(); bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose = true); bool LoadToWallet(const CWalletTx& wtxIn); - void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock); - bool AddToWalletIfInvolvingMe(const CTransaction& tx, const uint256& blockHash, int posInBlock, bool fUpdate); + void TransactionAddedToMempool(const CTransactionRef& tx) override; + void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) override; + void BlockDisconnected(const std::shared_ptr& pblock, int nBlockHeight) override; + bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const uint256& blockHash, int posInBlock, bool fUpdate); void EraseFromWallet(const uint256& hash); /** @@ -611,7 +615,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false, bool fromStartup = false); void ReacceptWalletTransactions(bool fFirstLoad = false); - void ResendWalletTransactions(CConnman* connman); + void ResendWalletTransactions(CConnman* connman) override; CAmount loopTxsBalance(std::functionmethod) const; CAmount GetAvailableBalance(bool fIncludeDelegated = true, bool fIncludeShielded = true) const; @@ -720,12 +724,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface CAmount GetCredit(const CWalletTx& tx, const isminefilter& filter) const; CAmount GetChange(const CTransaction& tx) const; - //! Sapling merkle tree update - void ChainTip(const CBlockIndex *pindex, - const CBlock *pblock, - Optional added); - - void SetBestChain(const CBlockLocator& loc); + void SetBestChain(const CBlockLocator& loc) override; void SetBestChainInternal(CWalletDB& walletdb, const CBlockLocator& loc); // only public for testing purposes, must never be called directly in any other situation // Force balance recomputation if any transaction got conflicted void MarkAffectedTransactionsDirty(const CTransaction& tx); // only public for testing purposes, must never be called directly in any other situation @@ -749,7 +748,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void LoadAddressBookName(const CWDestination& dest, const std::string& strName); void LoadAddressBookPurpose(const CWDestination& dest, const std::string& strPurpose); - bool UpdatedTransaction(const uint256& hashTx); + bool UpdatedTransaction(const uint256& hashTx) override; unsigned int GetKeyPoolSize(); unsigned int GetStakingKeyPoolSize(); @@ -786,7 +785,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface * Wallet post-init setup * Gives the wallet a chance to register repetitive tasks and complete post-init tasks */ - void postInitProcess(boost::thread_group& threadGroup); + void postInitProcess(CScheduler& scheduler); /** * Address book entry changed. diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c871c3868b90..d59523a4c5ad 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -891,35 +891,31 @@ DBErrors CWalletDB::ZapWalletTx(CWallet* pwallet, std::vector& vWtx) return DB_LOAD_OK; } -void ThreadFlushWalletDB() +void MaybeCompactWalletDB() { - // Make this thread recognisable as the wallet flushing thread - util::ThreadRename("pivx-wallet"); - - static bool fOneThread; - if (fOneThread) + static std::atomic fOneThread; + if (fOneThread.exchange(true)) { return; - fOneThread = true; - if (!gArgs.GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) + } + if (!gArgs.GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { return; + } - unsigned int nLastSeen = CWalletDB::GetUpdateCounter(); - unsigned int nLastFlushed = CWalletDB::GetUpdateCounter(); - int64_t nLastWalletUpdate = GetTime(); - while (true) { - MilliSleep(500); + static unsigned int nLastSeen = CWalletDB::GetUpdateCounter(); + static unsigned int nLastFlushed = CWalletDB::GetUpdateCounter(); + static int64_t nLastWalletUpdate = GetTime(); - if (nLastSeen != CWalletDB::GetUpdateCounter()) { - nLastSeen = CWalletDB::GetUpdateCounter(); - nLastWalletUpdate = GetTime(); - } + if (nLastSeen != CWalletDB::GetUpdateCounter()) { + nLastSeen = CWalletDB::GetUpdateCounter(); + nLastWalletUpdate = GetTime(); + } - if (nLastFlushed != CWalletDB::GetUpdateCounter() && GetTime() - nLastWalletUpdate >= 2) { - if (CDB::PeriodicFlush(pwalletMain->GetDBHandle())) { - nLastFlushed = CWalletDB::GetUpdateCounter(); - } + if (nLastFlushed != CWalletDB::GetUpdateCounter() && GetTime() - nLastWalletUpdate >= 2) { + if (CDB::PeriodicFlush(pwalletMain->GetDBHandle())) { + nLastFlushed = CWalletDB::GetUpdateCounter(); } } + fOneThread = false; } void NotifyBacked(const CWallet& wallet, bool fSuccess, std::string strMessage) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 73ad1b4842f5..f7cb0b0593e2 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -230,6 +230,7 @@ void NotifyBacked(const CWallet& wallet, bool fSuccess, std::string strMessage); bool BackupWallet(const CWallet& wallet, const fs::path& strDest); bool AttemptBackupWallet(const CWallet& wallet, const fs::path& pathSrc, const fs::path& pathDest); -void ThreadFlushWalletDB(); +//! Compacts BDB state so that wallet.dat is self-contained (if there are changes) +void MaybeCompactWalletDB(); #endif // BITCOIN_WALLETDB_H diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index 9c1b798d2fd9..fffa3bbb023f 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -145,8 +145,12 @@ void CZMQNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, co } } -void CZMQNotificationInterface::SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, int posInBlock) +void CZMQNotificationInterface::TransactionAddedToMempool(const CTransactionRef& ptx) { + // Used by BlockConnected and BlockDisconnected as well, because they're + // all the same external callback. + const CTransaction& tx = *ptx; + for (std::list::iterator i = notifiers.begin(); i!=notifiers.end(); ) { CZMQAbstractNotifier *notifier = *i; @@ -162,6 +166,22 @@ void CZMQNotificationInterface::SyncTransaction(const CTransaction& tx, const CB } } +void CZMQNotificationInterface::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted) +{ + for (const CTransactionRef& ptx : pblock->vtx) { + // Do a normal notify for each transaction added in the block + TransactionAddedToMempool(ptx); + } +} + +void CZMQNotificationInterface::BlockDisconnected(const std::shared_ptr& pblock, int nBlockHeight) +{ + for (const CTransactionRef& ptx : pblock->vtx) { + // Do a normal notify for each transaction removed in block disconnection + TransactionAddedToMempool(ptx); + } +} + void CZMQNotificationInterface::NotifyTransactionLock(const CTransaction &tx) { for (std::list::iterator i = notifiers.begin(); i!=notifiers.end(); ) diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h index eb7f7e7cef58..136724e102c3 100644 --- a/src/zmq/zmqnotificationinterface.h +++ b/src/zmq/zmqnotificationinterface.h @@ -8,6 +8,7 @@ #include "validationinterface.h" #include #include +#include class CBlockIndex; class CZMQAbstractNotifier; @@ -24,9 +25,11 @@ class CZMQNotificationInterface : public CValidationInterface void Shutdown(); // CValidationInterface - void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock); - void NotifyTransactionLock(const CTransaction &tx); - void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload); + void TransactionAddedToMempool(const CTransactionRef& tx) override; + void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted) override; + void BlockDisconnected(const std::shared_ptr& pblock, int nBlockHeight) override; + void NotifyTransactionLock(const CTransaction &tx) override; + void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; private: CZMQNotificationInterface(); diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index 708e42abc2f7..109c2114c851 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -18,17 +18,20 @@ def set_test_params(self): self.setup_clean_chain = True def setup_network(self): - self.alert_filename = os.path.join(self.options.tmpdir, "alert.txt") - self.block_filename = os.path.join(self.options.tmpdir, "blocks.txt") - self.tx_filename = os.path.join(self.options.tmpdir, "transactions.txt") + self.alertnotify_dir = os.path.join(self.options.tmpdir, "alertnotify") + self.blocknotify_dir = os.path.join(self.options.tmpdir, "blocknotify") + self.walletnotify_dir = os.path.join(self.options.tmpdir, "walletnotify") + os.mkdir(self.alertnotify_dir) + os.mkdir(self.blocknotify_dir) + os.mkdir(self.walletnotify_dir) # -alertnotify and -blocknotify on node0, walletnotify on node1 - self.extra_args = [["-blockversion=4", - "-alertnotify=echo %%s >> %s" % self.alert_filename, - "-blocknotify=echo %%s >> %s" % self.block_filename], + self.extra_args = [["-blockversion=8", + "-alertnotify=echo > {}".format(os.path.join(self.alertnotify_dir, '%s')), + "-blocknotify=echo > {}".format(os.path.join(self.blocknotify_dir, '%s'))], ["-blockversion=211", "-rescan", - "-walletnotify=echo %%s >> %s" % self.tx_filename]] + "-walletnotify=echo > {}".format(os.path.join(self.walletnotify_dir, '%s'))]] super().setup_network() def run_test(self): @@ -36,55 +39,51 @@ def run_test(self): block_count = 10 blocks = self.nodes[1].generate(block_count) - # wait at most 10 seconds for expected file size before reading the content - wait_until(lambda: os.path.isfile(self.block_filename) and os.stat(self.block_filename).st_size >= (block_count * 65), timeout=10) + # wait at most 10 seconds for expected number of files before reading the content + wait_until(lambda: len(os.listdir(self.blocknotify_dir)) == block_count, timeout=10) - # file content should equal the generated blocks hashes - with open(self.block_filename, 'r') as f: - assert_equal(sorted(blocks), sorted(f.read().splitlines())) + # directory content should equal the generated blocks hashes + assert_equal(sorted(blocks), sorted(os.listdir(self.blocknotify_dir))) self.log.info("test -walletnotify") - # wait at most 10 seconds for expected file size before reading the content - wait_until(lambda: os.path.isfile(self.tx_filename) and os.stat(self.tx_filename).st_size >= (block_count * 65), timeout=10) + # wait at most 10 seconds for expected number of files before reading the content + wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) - # file content should equal the generated transaction hashes + # directory content should equal the generated transaction hashes txids_rpc = list(map(lambda t: t['txid'], self.nodes[1].listtransactions("*", block_count))) - with open(self.tx_filename, 'r') as f: - assert_equal(sorted(txids_rpc), sorted(f.read().splitlines())) - os.remove(self.tx_filename) + assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir))) + self.stop_node(1) + for tx_file in os.listdir(self.walletnotify_dir): + os.remove(os.path.join(self.walletnotify_dir, tx_file)) self.log.info("test -walletnotify after rescan") # restart node to rescan to force wallet notifications - self.restart_node(1) + self.start_node(1) connect_nodes(self.nodes[0], 1) - wait_until(lambda: os.path.isfile(self.tx_filename) and os.stat(self.tx_filename).st_size >= (block_count * 65), timeout=10) + wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) - # file content should equal the generated transaction hashes + # directory content should equal the generated transaction hashes txids_rpc = list(map(lambda t: t['txid'], self.nodes[1].listtransactions("*", block_count))) - with open(self.tx_filename, 'r') as f: - assert_equal(sorted(txids_rpc), sorted(f.read().splitlines())) + assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir))) # Mine another 41 up-version blocks. -alertnotify should trigger on the 51st. self.log.info("test -alertnotify") self.nodes[1].generate(51) self.sync_all() - # Give pivxd 10 seconds to write the alert notification - wait_until(lambda: os.path.isfile(self.alert_filename) and os.path.getsize(self.alert_filename), timeout=10) + # Give bitcoind 10 seconds to write the alert notification + wait_until(lambda: len(os.listdir(self.alertnotify_dir)), timeout=10) - with open(self.alert_filename, 'r', encoding='utf8') as f: - alert_text = f.read() + for notify_file in os.listdir(self.alertnotify_dir): + os.remove(os.path.join(self.alertnotify_dir, notify_file)) # Mine more up-version blocks, should not get more alerts: self.nodes[1].generate(2) self.sync_all() - with open(self.alert_filename, 'r', encoding='utf8') as f: - alert_text2 = f.read() - self.log.info("-alertnotify should not continue notifying for more unknown version blocks") - assert_equal(alert_text, alert_text2) + assert_equal(len(os.listdir(self.alertnotify_dir)), 0) if __name__ == '__main__': NotificationsTest().main()