aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/txmempool.cpp56
-rw-r--r--src/txmempool.h11
-rw-r--r--src/validation.cpp51
-rw-r--r--src/validation.h5
-rwxr-xr-xtest/lint/lint-circular-dependencies.sh2
5 files changed, 70 insertions, 55 deletions
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 502a27dc6b..27fbb8acac 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -16,7 +16,6 @@
#include <util/moneystr.h>
#include <util/system.h>
#include <util/time.h>
-#include <validation.h>
#include <validationinterface.h>
#include <cmath>
@@ -74,6 +73,24 @@ private:
const LockPoints& lp;
};
+bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp)
+{
+ AssertLockHeld(cs_main);
+ assert(lp);
+ // If there are relative lock times then the maxInputBlock will be set
+ // If there are no relative lock times, the LockPoints don't depend on the chain
+ if (lp->maxInputBlock) {
+ // Check whether active_chain is an extension of the block at which the LockPoints
+ // calculation was valid. If not LockPoints are no longer valid
+ if (!active_chain.Contains(lp->maxInputBlock)) {
+ return false;
+ }
+ }
+
+ // LockPoints still valid
+ return true;
+}
+
CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee,
int64_t time, unsigned int entry_height,
bool spends_coinbase, int64_t sigops_cost, LockPoints lp)
@@ -616,44 +633,27 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso
RemoveStaged(setAllRemoves, false, reason);
}
-void CTxMemPool::removeForReorg(CChainState& active_chainstate, int flags)
+void CTxMemPool::removeForReorg(CChain& chain, std::function<bool(txiter)> check_final_and_mature)
{
// Remove transactions spending a coinbase which are now immature and no-longer-final transactions
AssertLockHeld(cs);
+ AssertLockHeld(::cs_main);
+
setEntries txToRemove;
for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {
- const CTransaction& tx = it->GetTx();
- LockPoints lp = it->GetLockPoints();
- bool validLP = TestLockPointValidity(active_chainstate.m_chain, &lp);
- CCoinsViewMemPool view_mempool(&active_chainstate.CoinsTip(), *this);
- if (!CheckFinalTx(active_chainstate.m_chain.Tip(), tx, flags)
- || !CheckSequenceLocks(active_chainstate.m_chain.Tip(), view_mempool, tx, flags, &lp, validLP)) {
- // Note if CheckSequenceLocks fails the LockPoints may still be invalid
- // So it's critical that we remove the tx and not depend on the LockPoints.
- txToRemove.insert(it);
- } else if (it->GetSpendsCoinbase()) {
- for (const CTxIn& txin : tx.vin) {
- indexed_transaction_set::const_iterator it2 = mapTx.find(txin.prevout.hash);
- if (it2 != mapTx.end())
- continue;
- const Coin &coin = active_chainstate.CoinsTip().AccessCoin(txin.prevout);
- if (m_check_ratio != 0) assert(!coin.IsSpent());
- unsigned int nMemPoolHeight = active_chainstate.m_chain.Tip()->nHeight + 1;
- if (coin.IsSpent() || (coin.IsCoinBase() && ((signed long)nMemPoolHeight) - coin.nHeight < COINBASE_MATURITY)) {
- txToRemove.insert(it);
- break;
- }
- }
- }
- if (!validLP) {
- mapTx.modify(it, update_lock_points(lp));
- }
+ if (check_final_and_mature(it)) txToRemove.insert(it);
}
setEntries setAllRemoves;
for (txiter it : txToRemove) {
CalculateDescendants(it, setAllRemoves);
}
RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG);
+ for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) {
+ LockPoints lp = it->GetLockPoints();
+ if (!TestLockPointValidity(chain, &lp)) {
+ mapTx.modify(it, update_lock_points(lp));
+ }
+ }
}
void CTxMemPool::removeConflicts(const CTransaction &tx)
diff --git a/src/txmempool.h b/src/txmempool.h
index 85417ac3fc..c6e08a3ca5 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -14,6 +14,7 @@
#include <utility>
#include <vector>
+#include <chain.h>
#include <coins.h>
#include <consensus/amount.h>
#include <indirectmap.h>
@@ -49,6 +50,11 @@ struct LockPoints {
CBlockIndex* maxInputBlock{nullptr};
};
+/**
+ * Test whether the LockPoints height and time are still valid on the current chain
+ */
+bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+
struct CompareIteratorByHash {
// SFINAE for T where T is either a pointer type (e.g., a txiter) or a reference_wrapper<T>
// (e.g. a wrapped CTxMemPoolEntry&)
@@ -583,7 +589,10 @@ public:
void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
- void removeForReorg(CChainState& active_chainstate, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
+ /** After reorg, check if mempool entries are now non-final, premature coinbase spends, or have
+ * invalid lockpoints. Update lockpoints and remove entries (and descendants of entries) that
+ * are no longer valid. */
+ void removeForReorg(CChain& chain, std::function<bool(txiter)> check_final_and_mature) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);
diff --git a/src/validation.cpp b/src/validation.cpp
index a9b8dba367..6ed5e7a548 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -212,24 +212,6 @@ bool CheckFinalTx(const CBlockIndex* active_chain_tip, const CTransaction &tx, i
return IsFinalTx(tx, nBlockHeight, nBlockTime);
}
-bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp)
-{
- AssertLockHeld(cs_main);
- assert(lp);
- // If there are relative lock times then the maxInputBlock will be set
- // If there are no relative lock times, the LockPoints don't depend on the chain
- if (lp->maxInputBlock) {
- // Check whether active_chain is an extension of the block at which the LockPoints
- // calculation was valid. If not LockPoints are no longer valid
- if (!active_chain.Contains(lp->maxInputBlock)) {
- return false;
- }
- }
-
- // LockPoints still valid
- return true;
-}
-
bool CheckSequenceLocks(CBlockIndex* tip,
const CCoinsView& coins_view,
const CTransaction& tx,
@@ -368,8 +350,39 @@ void CChainState::MaybeUpdateMempoolForReorg(
// the disconnectpool that were added back and cleans up the mempool state.
m_mempool->UpdateTransactionsFromBlock(vHashUpdate);
+ const auto check_final_and_mature = [this, flags=STANDARD_LOCKTIME_VERIFY_FLAGS](CTxMemPool::txiter it)
+ EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs, ::cs_main) {
+ bool should_remove = false;
+ AssertLockHeld(m_mempool->cs);
+ AssertLockHeld(::cs_main);
+ const CTransaction& tx = it->GetTx();
+ LockPoints lp = it->GetLockPoints();
+ bool validLP = TestLockPointValidity(m_chain, &lp);
+ CCoinsViewMemPool view_mempool(&CoinsTip(), *m_mempool);
+ if (!CheckFinalTx(m_chain.Tip(), tx, flags)
+ || !CheckSequenceLocks(m_chain.Tip(), view_mempool, tx, flags, &lp, validLP)) {
+ // Note if CheckSequenceLocks fails the LockPoints may still be invalid
+ // So it's critical that we remove the tx and not depend on the LockPoints.
+ should_remove = true;
+ } else if (it->GetSpendsCoinbase()) {
+ for (const CTxIn& txin : tx.vin) {
+ auto it2 = m_mempool->mapTx.find(txin.prevout.hash);
+ if (it2 != m_mempool->mapTx.end())
+ continue;
+ const Coin &coin = CoinsTip().AccessCoin(txin.prevout);
+ assert(!coin.IsSpent());
+ unsigned int nMemPoolHeight = m_chain.Tip()->nHeight + 1;
+ if (coin.IsSpent() || (coin.IsCoinBase() && ((signed long)nMemPoolHeight) - coin.nHeight < COINBASE_MATURITY)) {
+ should_remove = true;
+ break;
+ }
+ }
+ }
+ return should_remove;
+ };
+
// We also need to remove any now-immature transactions
- m_mempool->removeForReorg(*this, STANDARD_LOCKTIME_VERIFY_FLAGS);
+ m_mempool->removeForReorg(m_chain, check_final_and_mature);
// Re-limit mempool size, in case we added any transactions
LimitMempoolSize(
*m_mempool,
diff --git a/src/validation.h b/src/validation.h
index a57045ad0f..881438f37a 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -254,11 +254,6 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx
bool CheckFinalTx(const CBlockIndex* active_chain_tip, const CTransaction &tx, int flags = -1) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
/**
- * Test whether the LockPoints height and time are still valid on the current chain
- */
-bool TestLockPointValidity(CChain& active_chain, const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
-
-/**
* Check if transaction will be BIP68 final in the next block to be created on top of tip.
* @param[in] tip Chain tip to check tx sequence locks against. For example,
* the tip of the current active chain.
diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh
index ab3866d23e..69185090d1 100755
--- a/test/lint/lint-circular-dependencies.sh
+++ b/test/lint/lint-circular-dependencies.sh
@@ -15,12 +15,10 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"index/base -> validation -> index/blockfilterindex -> index/base"
"index/coinstatsindex -> node/coinstats -> index/coinstatsindex"
"policy/fees -> txmempool -> policy/fees"
- "policy/rbf -> txmempool -> validation -> policy/rbf"
"qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel"
"qt/recentrequeststablemodel -> qt/walletmodel -> qt/recentrequeststablemodel"
"qt/sendcoinsdialog -> qt/walletmodel -> qt/sendcoinsdialog"
"qt/transactiontablemodel -> qt/walletmodel -> qt/transactiontablemodel"
- "txmempool -> validation -> txmempool"
"wallet/fees -> wallet/wallet -> wallet/fees"
"wallet/wallet -> wallet/walletdb -> wallet/wallet"
"node/coinstats -> validation -> node/coinstats"