merge bitcoin#22677: cut the validation <-> txmempool circular dependency

This commit is contained in:
Kittywhiskers Van Gogh 2024-10-03 19:51:30 +00:00
parent ee49383cd6
commit 8ab99290f9
No known key found for this signature in database
GPG Key ID: 30CD0C065E5C4AAD
5 changed files with 73 additions and 56 deletions

View File

@ -18,7 +18,6 @@
#include <util/moneystr.h>
#include <util/system.h>
#include <util/time.h>
#include <validation.h>
#include <validationinterface.h>
#include <evo/specialtx.h>
@ -82,6 +81,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_count, LockPoints lp)
@ -859,44 +876,27 @@ void CTxMemPool::removeRecursive(const CTransaction &origTx, MemPoolRemovalReaso
RemoveStaged(setAllRemoves, false, reason);
}
void CTxMemPool::removeForReorg(CChainState& active_chainstate, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
void CTxMemPool::removeForReorg(CChain& chain, std::function<bool(txiter)> check_final_and_mature) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
// 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)

View File

@ -17,6 +17,7 @@
#include <addressindex.h>
#include <spentindex.h>
#include <amount.h>
#include <chain.h>
#include <coins.h>
#include <gsl/pointers.h>
#include <indirectmap.h>
@ -59,6 +60,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&)
@ -615,7 +621,10 @@ public:
bool removeSpentIndex(const uint256 txhash);
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 removeProTxPubKeyConflicts(const CTransaction &tx, const CKeyID &keyId) EXCLUSIVE_LOCKS_REQUIRED(cs);
void removeProTxPubKeyConflicts(const CTransaction &tx, const CBLSLazyPublicKey &pubKey) EXCLUSIVE_LOCKS_REQUIRED(cs);

View File

@ -172,24 +172,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 ::ChainActive() 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,
@ -389,8 +371,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,

View File

@ -284,11 +284,6 @@ int GetUTXOConfirmations(CChainState& active_chainstate, const COutPoint& outpoi
*/
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,

View File

@ -18,7 +18,6 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"qt/addresstablemodel -> qt/walletmodel -> qt/addresstablemodel"
"qt/recentrequeststablemodel -> qt/walletmodel -> qt/recentrequeststablemodel"
"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"
@ -63,12 +62,13 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"llmq/chainlocks -> validation -> llmq/chainlocks"
"coinjoin/coinjoin -> llmq/chainlocks -> net -> coinjoin/coinjoin"
"evo/assetlocktx -> validation -> txmempool -> evo/assetlocktx"
"evo/deterministicmns -> llmq/utils -> llmq/snapshot -> evo/simplifiedmns -> evo/deterministicmns"
"evo/deterministicmns -> llmq/utils -> net -> evo/deterministicmns"
"evo/deterministicmns -> validation -> txmempool -> evo/deterministicmns"
"policy/policy -> policy/settings -> policy/policy"
"consensus/tx_verify -> evo/assetlocktx -> validation -> consensus/tx_verify"
"consensus/tx_verify -> evo/assetlocktx -> llmq/signing -> net_processing -> txmempool -> consensus/tx_verify"
"evo/assetlocktx -> llmq/signing -> net_processing -> txmempool -> evo/assetlocktx"
"consensus/tx_verify -> evo/assetlocktx -> validation -> txmempool -> consensus/tx_verify"
"evo/simplifiedmns -> llmq/blockprocessor -> llmq/utils -> llmq/snapshot -> evo/simplifiedmns"
"llmq/blockprocessor -> llmq/utils -> llmq/snapshot -> llmq/blockprocessor"