diff --git a/src/init.cpp b/src/init.cpp index d7b4cbbcf53a..8fa9fafecca4 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -314,7 +314,6 @@ void PrepareShutdown() // Disconnect all slots UnregisterAllValidationInterfaces(); GetMainSignals().UnregisterBackgroundSignalScheduler(); - GetMainSignals().UnregisterWithMempoolSignals(mempool); #ifndef WIN32 try { @@ -1253,7 +1252,6 @@ bool AppInitMain() threadGroup.create_thread(std::bind(&TraceThread, "scheduler", serviceLoop)); GetMainSignals().RegisterBackgroundSignalScheduler(scheduler); - GetMainSignals().RegisterWithMempoolSignals(mempool); // Initialize Sapling circuit parameters LoadSaplingParams(); diff --git a/src/test/librust/sapling_rpc_wallet_tests.cpp b/src/test/librust/sapling_rpc_wallet_tests.cpp index c3a86997c2e2..e8c74b24e923 100644 --- a/src/test/librust/sapling_rpc_wallet_tests.cpp +++ b/src/test/librust/sapling_rpc_wallet_tests.cpp @@ -434,8 +434,7 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling) chainActive.SetTip(&fakeIndex); BOOST_CHECK(chainActive.Contains(&fakeIndex)); BOOST_CHECK_EQUAL(1, chainActive.Height()); - std::vector vtxConflicted; - pwalletMain->BlockConnected(std::make_shared(block), mi->second, vtxConflicted); + pwalletMain->BlockConnected(std::make_shared(block), mi->second); BOOST_CHECK_MESSAGE(pwalletMain->GetAvailableBalance() > 0, "tx not confirmed"); // Context that shieldsendmany requires diff --git a/src/test/librust/sapling_wallet_tests.cpp b/src/test/librust/sapling_wallet_tests.cpp index af0cc84688ae..c842f87ee07f 100644 --- a/src/test/librust/sapling_wallet_tests.cpp +++ b/src/test/librust/sapling_wallet_tests.cpp @@ -351,8 +351,7 @@ BOOST_AUTO_TEST_CASE(SaplingNullifierIsSpent) { BOOST_CHECK(chainActive.Contains(&fakeIndex)); BOOST_CHECK_EQUAL(0, chainActive.Height()); - std::vector vtxConflicted; - wallet.BlockConnected(std::make_shared(block), mi->second, vtxConflicted); + wallet.BlockConnected(std::make_shared(block), mi->second); // Verify note has been spent BOOST_CHECK(wallet.GetSaplingScriptPubKeyMan()->IsSaplingSpent(nullifier)); diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 16e7bb27b7be..9e9e57655985 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -12,7 +12,7 @@ #include -BOOST_FIXTURE_TEST_SUITE(policyestimator_tests, BasicTestingSetup) +BOOST_FIXTURE_TEST_SUITE(policyestimator_tests, TestingSetup) BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) { diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp index adad49eb9192..d2bc777c9ee0 100644 --- a/src/test/test_pivx.cpp +++ b/src/test/test_pivx.cpp @@ -68,7 +68,6 @@ TestingSetup::TestingSetup() // callbacks via CValidationInterface are unreliable, but that's OK, // our unit tests aren't testing multiple parts of the code at once. GetMainSignals().RegisterBackgroundSignalScheduler(scheduler); - GetMainSignals().RegisterWithMempoolSignals(mempool); // Ideally we'd move all the RPC tests to the functional testing framework // instead of unit tests, but for now we need these here. @@ -102,7 +101,6 @@ TestingSetup::~TestingSetup() GetMainSignals().FlushBackgroundCallbacks(); UnregisterAllValidationInterfaces(); GetMainSignals().UnregisterBackgroundSignalScheduler(); - GetMainSignals().UnregisterWithMempoolSignals(mempool); UnloadBlockIndex(); delete pcoinsTip; delete pcoinsdbview; diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index dba3c04f1b63..94445b0a6b9f 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -32,7 +32,7 @@ struct TestSubscriber : public CValidationInterface { BOOST_CHECK_EQUAL(m_expected_tip, pindexNew->GetBlockHash()); } - void BlockConnected(const std::shared_ptr& block, const CBlockIndex* pindex, const std::vector& txnConflicted) + void BlockConnected(const std::shared_ptr& block, const CBlockIndex* pindex) { BOOST_CHECK_EQUAL(m_expected_tip, block->hashPrevBlock); BOOST_CHECK_EQUAL(m_expected_tip, pindex->pprev->GetBlockHash()); @@ -170,6 +170,7 @@ BOOST_AUTO_TEST_CASE(processnewblock_signals_ordering) MilliSleep(100); } + SyncWithValidationInterfaceQueue(); UnregisterValidationInterface(&sub); BOOST_CHECK_EQUAL(sub.m_expected_tip, WITH_LOCK(cs_main, return chainActive.Tip()->GetBlockHash())); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 91747ba11f4e..feb663acfb9a 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -16,6 +16,7 @@ #include "utiltime.h" #include "version.h" #include "validation.h" +#include "validationinterface.h" @@ -370,7 +371,6 @@ void CTxMemPool::AddTransactionsUpdated(unsigned int 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); @@ -434,7 +434,14 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry, void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason) { - NotifyEntryRemoved(it->GetSharedTx(), reason); + if (reason != MemPoolRemovalReason::BLOCK) { + // Notify clients that a transaction has been removed from the mempool + // for any reason except being included in a block. Clients interested + // in transactions included in blocks can subscribe to the BlockConnected + // notification. + GetMainSignals().TransactionRemovedFromMempool(it->GetSharedTx(), reason); + } + AssertLockHeld(cs); const CTransaction& tx = it->GetTx(); for (const CTxIn& txin : tx.vin) diff --git a/src/txmempool.h b/src/txmempool.h index 5212e7b538db..f4355cdbff53 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -24,8 +24,6 @@ #include "boost/multi_index/hashed_index.hpp" #include -#include - class CAutoFile; inline double AllowFreeThreshold() @@ -684,9 +682,6 @@ 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 diff --git a/src/validation.cpp b/src/validation.cpp index be53443ebdb4..628eaf19f885 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2011,39 +2011,21 @@ static int64_t nTimePostConnect = 0; struct PerBlockConnectTrace { CBlockIndex* pindex = nullptr; std::shared_ptr pblock; - std::shared_ptr> conflictedTxs; - PerBlockConnectTrace() : conflictedTxs(std::make_shared>()) {} + PerBlockConnectTrace() {} }; /** * 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. */ 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(); - } + ConnectTrace() : blocksConnected(1) {} void BlockConnected(CBlockIndex* pindex, std::shared_ptr pblock) { assert(!blocksConnected.back().pindex); @@ -2061,17 +2043,9 @@ class ConnectTrace { // 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)); - } - } }; /** @@ -2353,7 +2327,7 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb do { // We absolutely may not unlock cs_main until we've made forward progress // (with the exception of shutdown due to hardware issues, low disk space, etc). - ConnectTrace connectTrace(mempool); // Destructed before cs_main is unlocked + ConnectTrace connectTrace; // Destructed before cs_main is unlocked if (pindexMostWork == nullptr) { pindexMostWork = FindMostWorkChain(); @@ -2378,7 +2352,7 @@ bool ActivateBestChain(CValidationState& state, std::shared_ptr pb for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) { assert(trace.pblock && trace.pindex); - GetMainSignals().BlockConnected(trace.pblock, trace.pindex, trace.conflictedTxs); + GetMainSignals().BlockConnected(trace.pblock, trace.pindex); } } while (!chainActive.Tip() || (starting_tip && CBlockIndexWorkComparator()(chainActive.Tip(), starting_tip))); if (!blocks_connected) return true; diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 76a5ff27be39..2b980368923a 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -6,7 +6,6 @@ #include "validationinterface.h" #include "scheduler.h" -#include "txmempool.h" #include "validation.h" #include @@ -35,11 +34,11 @@ struct MainSignalsInstance { * 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; + boost::signals2::signal &, const CBlockIndex *pindex)> BlockConnected; /** Notifies listeners of a block being disconnected */ boost::signals2::signal &, const uint256& blockHash, int nBlockHeight, int64_t blockTime)> BlockDisconnected; /** Notifies listeners of a transaction removal from the mempool */ - boost::signals2::signal TransactionRemovedFromMempool; + boost::signals2::signal TransactionRemovedFromMempool; /** Notifies listeners of a new active block chain. */ boost::signals2::signal SetBestChain; /** Tells listeners to broadcast their data. */ @@ -79,14 +78,6 @@ size_t CMainSignals::CallbacksPending() { return m_internals->m_schedulerClient.CallbacksPending(); } -void CMainSignals::RegisterWithMempoolSignals(CTxMemPool& pool) { - pool.NotifyEntryRemoved.connect(std::bind(&CMainSignals::MempoolEntryRemoved, this, std::placeholders::_1, std::placeholders::_2)); -} - -void CMainSignals::UnregisterWithMempoolSignals(CTxMemPool& pool) { - pool.NotifyEntryRemoved.disconnect_all_slots(); -} - CMainSignals& GetMainSignals() { return g_signals; @@ -97,9 +88,9 @@ 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.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.BlockConnected = g_signals.m_internals->BlockConnected.connect(std::bind(&CValidationInterface::BlockConnected, pwalletIn, std::placeholders::_1, std::placeholders::_2)); conns.BlockDisconnected = g_signals.m_internals->BlockDisconnected.connect(std::bind(&CValidationInterface::BlockDisconnected, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4)); - conns.TransactionRemovedFromMempool = g_signals.m_internals->TransactionRemovedFromMempool.connect(std::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, std::placeholders::_1)); + conns.TransactionRemovedFromMempool = g_signals.m_internals->TransactionRemovedFromMempool.connect(std::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, std::placeholders::_1, std::placeholders::_2)); 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)); @@ -137,14 +128,6 @@ void SyncWithValidationInterfaceQueue() { promise.get_future().wait(); } -void CMainSignals::MempoolEntryRemoved(CTransactionRef ptx, MemPoolRemovalReason reason) { - if (reason != MemPoolRemovalReason::BLOCK && reason != MemPoolRemovalReason::CONFLICT) { - m_internals->m_schedulerClient.AddToProcessQueue([ptx, this] { - m_internals->TransactionRemovedFromMempool(ptx); - }); - } -} - void CMainSignals::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) { // Dependencies exist that require UpdatedBlockTip events to be delivered in the order in which // the chain actually updates. One way to ensure this is for the caller to invoke this signal @@ -161,9 +144,15 @@ void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx) { }); } -void CMainSignals::BlockConnected(const std::shared_ptr &pblock, const CBlockIndex *pindex, const std::shared_ptr>& pvtxConflicted) { - m_internals->m_schedulerClient.AddToProcessQueue([pblock, pindex, pvtxConflicted, this] { - m_internals->BlockConnected(pblock, pindex, *pvtxConflicted); +void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef& ptx, MemPoolRemovalReason reason) { + m_internals->m_schedulerClient.AddToProcessQueue([ptx, reason, this] { + m_internals->TransactionRemovedFromMempool(ptx, reason); + }); +} + +void CMainSignals::BlockConnected(const std::shared_ptr &pblock, const CBlockIndex *pindex) { + m_internals->m_schedulerClient.AddToProcessQueue([pblock, pindex, this] { + m_internals->BlockConnected(pblock, pindex); }); } diff --git a/src/validationinterface.h b/src/validationinterface.h index 91ec2ee5b46c..4dd8c54681d2 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -22,7 +22,6 @@ class CValidationInterface; class CValidationState; class uint256; class CScheduler; -class CTxMemPool; enum class MemPoolRemovalReason; // These functions dispatch to one or all registered wallets @@ -92,21 +91,43 @@ class CValidationInterface { /** * Notifies listeners of a transaction leaving mempool. * - * This only fires for transactions which leave mempool because of expiry, - * size limiting, reorg (changes in lock times/coinbase/coinstake maturity), or - * replacement. This does not include any transactions which are included - * in BlockConnectedDisconnected either in block->vtx or in txnConflicted. + * This notification fires for transactions that are removed from the + * mempool for the following reasons: + * + * - EXPIRY (expired from mempool after -mempoolexpiry hours) + * - SIZELIMIT (removed in size limiting if the mempool exceeds -maxmempool megabytes) + * - REORG (removed during a reorg) + * - CONFLICT (removed because it conflicts with in-block transaction) + * - REPLACED (removed due to RBF replacement) -- not supported yet -- + * + * This does not fire for transactions that are removed from the mempool + * because they have been included in a block. Any client that is interested + * in transactions removed from the mempool for inclusion in a block can learn + * about those transactions from the BlockConnected notification. + * + * Transactions that are removed from the mempool because they conflict + * with a transaction in the new block will have + * TransactionRemovedFromMempool events fired *before* the BlockConnected + * event is fired. If multiple blocks are connected in one step, then the + * ordering could be: + * + * - TransactionRemovedFromMempool(tx1 from block A) + * - TransactionRemovedFromMempool(tx2 from block A) + * - TransactionRemovedFromMempool(tx1 from block B) + * - TransactionRemovedFromMempool(tx2 from block B) + * - BlockConnected(A) + * - BlockConnected(B) * * Called on a background thread. */ - virtual void TransactionRemovedFromMempool(const CTransactionRef &ptx) {} + virtual void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {} /** * Notifies listeners of a block being connected. * Provides a vector of transactions evicted from the mempool as a result. * * Called on a background thread. */ - virtual void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::vector &txnConflicted) {} + virtual void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex) {} /** * Notifies listeners of a block being disconnected * @@ -132,8 +153,6 @@ class CMainSignals { private: std::unique_ptr m_internals; - void MempoolEntryRemoved(CTransactionRef tx, MemPoolRemovalReason reason); - friend void ::RegisterValidationInterface(CValidationInterface*); friend void ::UnregisterValidationInterface(CValidationInterface*); friend void ::UnregisterAllValidationInterfaces(); @@ -149,14 +168,10 @@ class CMainSignals { size_t CallbacksPending(); - /** Register with mempool to call TransactionRemovedFromMempool callbacks */ - void RegisterWithMempoolSignals(CTxMemPool& pool); - /** Unregister with mempool */ - void UnregisterWithMempoolSignals(CTxMemPool& pool); - void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload); void TransactionAddedToMempool(const CTransactionRef &ptxn); - void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::shared_ptr> &); + void TransactionRemovedFromMempool(const CTransactionRef&, MemPoolRemovalReason); + void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex); void BlockDisconnected(const std::shared_ptr &block, const uint256& blockHash, int nBlockHeight, int64_t blockTime); void SetBestChain(const CBlockLocator &); void Broadcast(CConnman* connman); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index beee18dc5e26..913941063716 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1296,15 +1296,45 @@ void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) } } -void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx) { +void CWallet::TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) { LOCK(cs_wallet); auto it = mapWallet.find(ptx->GetHash()); if (it != mapWallet.end()) { it->second.fInMempool = false; } -} - -void CWallet::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) + // Handle transactions that were removed from the mempool because they + // conflict with transactions in a newly connected block. + if (reason == MemPoolRemovalReason::CONFLICT) { + // Call SyncNotifications, so external -walletnotify notifications will + // be triggered for these transactions. Set Status::UNCONFIRMED instead + // of Status::CONFLICTED for a few reasons: + // + // 1. The transactionRemovedFromMempool callback does not currently + // provide the conflicting block's hash and height, and for backwards + // compatibility reasons it may not be not safe to store conflicted + // wallet transactions with a null block hash. See + // https://github.com/bitcoin/bitcoin/pull/18600#discussion_r420195993. + // 2. For most of these transactions, the wallet's internal conflict + // detection in the blockConnected handler will subsequently call + // MarkConflicted and update them with CONFLICTED status anyway. This + // applies to any wallet transaction that has inputs spent in the + // block, or that has ancestors in the wallet with inputs spent by + // the block. + // 3. Longstanding behavior since the sync implementation in + // https://github.com/bitcoin/bitcoin/pull/9371 and the prior sync + // implementation before that was to mark these transactions + // unconfirmed rather than conflicted. + // + // Nothing described above should be seen as an unchangeable requirement + // when improving this code in the future. The wallet's heuristics for + // distinguishing between conflicted and unconfirmed transactions are + // imperfect, and could be improved in general, see + // https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking + SyncTransaction(ptx, {CWalletTx::Status::UNCONFIRMED, /* block height */ 0, /* block hash */ {}, /* index */ 0}); + } +} + +void CWallet::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex) { { LOCK(cs_wallet); @@ -1316,10 +1346,7 @@ void CWallet::BlockConnected(const std::shared_ptr& pblock, const CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, m_last_block_processed_height, m_last_block_processed, index); SyncTransaction(pblock->vtx[index], confirm); - TransactionRemovedFromMempool(pblock->vtx[index]); - } - for (const CTransactionRef& ptx : vtxConflicted) { - TransactionRemovedFromMempool(ptx); + TransactionRemovedFromMempool(pblock->vtx[index], MemPoolRemovalReason::BLOCK); } // Sapling: notify about the connected block diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5791665acfef..fe93df85f980 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -956,7 +956,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose = true); bool LoadToWallet(CWalletTx& wtxIn); void TransactionAddedToMempool(const CTransactionRef& tx) override; - void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) override; + void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex) override; void BlockDisconnected(const std::shared_ptr& pblock, const uint256& blockHash, int nBlockHeight, int64_t blockTime) override; bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const CWalletTx::Confirmation& confirm, bool fUpdate); void EraseFromWallet(const uint256& hash); @@ -969,7 +969,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update); CBlockIndex* ScanForWalletTransactions(CBlockIndex* pindexStart, CBlockIndex* pindexStop, const WalletRescanReserver& reserver, bool fUpdate = false, bool fromStartup = false); - void TransactionRemovedFromMempool(const CTransactionRef &ptx) override; + void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override; void ReacceptWalletTransactions(bool fFirstLoad = false); void ResendWalletTransactions(CConnman* connman) override; diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index 30d522c3d92e..110e2df44ba4 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -164,7 +164,7 @@ void CZMQNotificationInterface::TransactionAddedToMempool(const CTransactionRef& } } -void CZMQNotificationInterface::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted) +void CZMQNotificationInterface::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected) { for (const CTransactionRef& ptx : pblock->vtx) { // Do a normal notify for each transaction added in the block diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h index 5bb17cd6c504..7ca65bd94ac4 100644 --- a/src/zmq/zmqnotificationinterface.h +++ b/src/zmq/zmqnotificationinterface.h @@ -26,7 +26,7 @@ class CZMQNotificationInterface : public CValidationInterface // CValidationInterface void TransactionAddedToMempool(const CTransactionRef& tx) override; - void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted) override; + void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected) override; void BlockDisconnected(const std::shared_ptr& pblock, const uint256& blockHash, int nBlockHeight, int64_t blockTime) override; void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index 109c2114c851..df2b42f901b8 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -34,6 +34,10 @@ def setup_network(self): "-walletnotify=echo > {}".format(os.path.join(self.walletnotify_dir, '%s'))]] super().setup_network() + # Hardcoded, always true, function for now to make future back ports easier. + def is_wallet_compiled(self): + return True + def run_test(self): self.log.info("test -blocknotify") block_count = 10 @@ -45,27 +49,28 @@ def run_test(self): # 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 number of files before reading the content - wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) + if self.is_wallet_compiled(): + self.log.info("test -walletnotify") + # 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) - # directory content should equal the generated transaction hashes - txids_rpc = list(map(lambda t: t['txid'], self.nodes[1].listtransactions("*", block_count))) - 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)) + # directory content should equal the generated transaction hashes + txids_rpc = list(map(lambda t: t['txid'], self.nodes[1].listtransactions("*", block_count))) + 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.start_node(1) - connect_nodes(self.nodes[0], 1) + self.log.info("test -walletnotify after rescan") + # restart node to rescan to force wallet notifications + self.start_node(1) + connect_nodes(self.nodes[0], 1) - wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) + wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) - # directory content should equal the generated transaction hashes - txids_rpc = list(map(lambda t: t['txid'], self.nodes[1].listtransactions("*", block_count))) - assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir))) + # directory content should equal the generated transaction hashes + txids_rpc = list(map(lambda t: t['txid'], self.nodes[1].listtransactions("*", block_count))) + 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")