Skip to content

cut the validation <-> txmempool circular dependency 2/2 #22677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
maflcko merged 5 commits into from
Dec 1, 2021
Merged

cut the validation <-> txmempool circular dependency 2/2 #22677

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1b3a11e
MOVEONLY: TestLockPointValidity to txmempool
glozow Nov 30, 2021
bedf246
[mempool] only update lockpoints for non-removed entries
glozow Nov 30, 2021
bb9078e
[refactor] put finality and maturity checking into a lambda
glozow Sep 29, 2021
64e4963
[mempool] always assert coin spent
glozow Sep 29, 2021
a64078e
Break validation <-> txmempool circular dependency
glozow Sep 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/txmempool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,24 @@ struct update_lock_points
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)
Expand Down
6 changes: 6 additions & 0 deletions src/txmempool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <utility>
#include <vector>

#include <chain.h>

@theStack theStack Dec 1, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could use the forward-declaration class CChain; instead to reduce header dependencies

@glozow glozow Dec 2, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

taken in #23649

#include <coins.h>
#include <consensus/amount.h>
#include <indirectmap.h>
Expand Down Expand Up @@ -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&)
Expand Down
18 changes: 0 additions & 18 deletions src/validation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 0 additions & 5 deletions src/validation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,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,
Expand Down