diff --git a/src/main.cpp b/src/main.cpp index 5a5bd9e95..4e1d816dd 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4062,8 +4062,8 @@ static int64_t nTimeChainState = 0; static int64_t nTimePostConnect = 0; // Protected by cs_main -std::map> recentlyConflictedTxs; -uint64_t nRecentlyConflictedSequence = 0; +std::map> recentlyConflictedTxs; +uint64_t nConnectedSequence = 0; uint64_t nNotifiedSequence = 0; /** @@ -4115,7 +4115,9 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, // Cache the conflicted transactions for subsequent notification. // Updates to connected wallets are triggered by ThreadNotifyWallets recentlyConflictedTxs.insert(std::make_pair(pindexNew, txConflicted)); - nRecentlyConflictedSequence += 1; + + // Increment the count of `ConnectTip` calls. + nConnectedSequence += 1; EnforceNodeDeprecation(pindexNew->nHeight); @@ -4126,29 +4128,34 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, return true; } -std::pair>, uint64_t> DrainRecentlyConflicted() +std::pair, std::optional> TakeRecentlyConflicted(const CBlockIndex* pindex) { - uint64_t recentlyConflictedSequence; - std::map> txs; - { - LOCK(cs_main); - recentlyConflictedSequence = nRecentlyConflictedSequence; - txs.swap(recentlyConflictedTxs); - } + AssertLockHeld(cs_main); - return std::make_pair(txs, recentlyConflictedSequence); + std::list conflictedTxs = recentlyConflictedTxs.at(pindex); + recentlyConflictedTxs.erase(pindex); + if (recentlyConflictedTxs.empty()) { + return std::make_pair(conflictedTxs, nConnectedSequence); + } else { + return std::make_pair(conflictedTxs, std::nullopt); + } } -void SetChainNotifiedSequence(const CChainParams& chainparams, uint64_t recentlyConflictedSequence) { +uint64_t GetChainConnectedSequence() { + LOCK(cs_main); + return nConnectedSequence; +} + +void SetChainNotifiedSequence(const CChainParams& chainparams, uint64_t connectedSequence) { assert(chainparams.NetworkIDString() == "regtest"); LOCK(cs_main); - nNotifiedSequence = recentlyConflictedSequence; + nNotifiedSequence = connectedSequence; } bool ChainIsFullyNotified(const CChainParams& chainparams) { assert(chainparams.NetworkIDString() == "regtest"); LOCK(cs_main); - return nRecentlyConflictedSequence == nNotifiedSequence; + return nConnectedSequence == nNotifiedSequence; } /** diff --git a/src/main.h b/src/main.h index 6cc4df786..dabba715d 100644 --- a/src/main.h +++ b/src/main.h @@ -668,7 +668,8 @@ CMutableTransaction CreateNewContextualCMutableTransaction( int nHeight, bool requireV4); -std::pair>, uint64_t> DrainRecentlyConflicted(); +std::pair, std::optional> TakeRecentlyConflicted(const CBlockIndex* pindex); +uint64_t GetChainConnectedSequence(); void SetChainNotifiedSequence(const CChainParams& chainparams, uint64_t recentlyConflictedSequence); bool ChainIsFullyNotified(const CChainParams& chainparams); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 476dab819..446f9ee53 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -163,8 +163,9 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip) // The stack of blocks we will notify as having been connected. // Pushed in reverse, popped in order. std::vector blockStack; - // Transactions that have been recently conflicted out of the mempool. - std::pair>, uint64_t> recentlyConflicted; + // Sequence number indicating that we have notified wallets of transactions up to + // the ConnectBlock() call that generated this sequence number. + std::optional chainNotifiedSequence; // Transactions that have been recently added to the mempool. std::pair, uint64_t> recentlyAdded; @@ -176,12 +177,8 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip) CBlockIndex *pindex = chainActive.Tip(); pindexFork = chainActive.FindFork(pindexLastTip); - // Fetch recently-conflicted transactions. These will include any - // block that has been connected since the last cycle, but we only - // notify for the conflicts created by the current active chain. - recentlyConflicted = DrainRecentlyConflicted(); - // Iterate backwards over the connected blocks we need to notify. + bool originalTipAtFork = pindex && pindex == pindexFork; while (pindex && pindex != pindexFork) { MerkleFrontiers oldFrontiers; // Get the Sprout commitment tree as of the start of this block. @@ -214,15 +211,38 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip) OrchardMerkleFrontier::empty_root(), oldFrontiers.orchard)); } + // Fetch recently-conflicted transactions. These will include any + // block that has been connected since the last cycle, but we only + // notify for the conflicts created by the current active chain. + auto recentlyConflicted = TakeRecentlyConflicted(pindex); + blockStack.emplace_back( pindex, oldFrontiers, - recentlyConflicted.first[pindex]); + recentlyConflicted.first); + + chainNotifiedSequence = recentlyConflicted.second; pindex = pindex->pprev; } - recentlyAdded = mempool.DrainRecentlyAdded(); + // This conditional can be true in the case that in the interval + // since the last second-boundary, two reorgs occurred: one that + // shifted over to a different chain history, and then a second + // that returned the chain to the original pre-reorg tip. This + // should never occur unless a caller has manually used + // `invalidateblock` to force the second reorg or we have a long + // persistent set of dueling chains. In such a case, wallets may + // not be fully notified of conflicted transactions, but they will + // still have a correct view of the current main chain, and they + // will still be notified properly of the current state of + // transactions in the mempool. + if (originalTipAtFork) { + chainNotifiedSequence = GetChainConnectedSequence(); + } + if (chainNotifiedSequence.has_value()) { + recentlyAdded = mempool.DrainRecentlyAdded(); + } } // @@ -540,8 +560,12 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip) // Update the notified sequence numbers. We only need this in regtest mode, // and should not lock on cs or cs_main here otherwise. if (chainParams.NetworkIDString() == "regtest") { - SetChainNotifiedSequence(chainParams, recentlyConflicted.second); - mempool.SetNotifiedSequence(recentlyAdded.second); + if (chainNotifiedSequence.has_value()) { + SetChainNotifiedSequence(chainParams, chainNotifiedSequence.value()); + } + if (recentlyAdded.second > 0) { + mempool.SetNotifiedSequence(recentlyAdded.second); + } } } }