Skip to content
Permalink
Browse files

Merge #13783: validation: Pass tx pool reference into CheckSequenceLocks

fa511e8 Pass tx pool reference into CheckSequenceLocks (MarcoFalke)

Pull request description:

  `CheckSequenceLocks` is called from ATMP and the member function `CTxMemPool::removeForReorg` without passing in the tx pool object that is used in those function's scope and instead using the global `::mempool` instance.

  This fix should be refactoring only, since currently there is only one (global) tx pool in normal operation. Though, it fixes hard to track down issues in future settings where more than one mempool exists at a time. (E.g. for tests, rpc or p2p tx relay purposes)

Tree-SHA512: f0804588c7d29bb6ff05ec14f22a16422b89ab31ae714f38cd07f811d7dc7907bfd14e799c4c1c3121144ff22711019bbe9212b39e2fd4531936a4119950fa49
  • Loading branch information...
MarcoFalke committed Oct 27, 2018
2 parents c70f9c0 + fa511e8 commit efaf2d85e3a20ed381ae5156707c970ee0c4fa65
Showing with 8 additions and 8 deletions.
  1. +2 −2 src/test/miner_tests.cpp
  2. +1 −1 src/txmempool.cpp
  3. +4 −4 src/validation.cpp
  4. +1 −1 src/validation.h
@@ -92,8 +92,8 @@ static CBlockIndex CreateBlockIndex(int nHeight)

static bool TestSequenceLocks(const CTransaction &tx, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
LOCK(mempool.cs);
return CheckSequenceLocks(tx, flags);
LOCK(::mempool.cs);
return CheckSequenceLocks(::mempool, tx, flags);
}

// Test suite for ancestor feerate transaction selection.
@@ -498,7 +498,7 @@ void CTxMemPool::removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMem
const CTransaction& tx = it->GetTx();
LockPoints lp = it->GetLockPoints();
bool validLP = TestLockPointValidity(&lp);
if (!CheckFinalTx(tx, flags) || !CheckSequenceLocks(tx, flags, &lp, validLP)) {
if (!CheckFinalTx(tx, flags) || !CheckSequenceLocks(*this, 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);
@@ -361,10 +361,10 @@ bool TestLockPointValidity(const LockPoints* lp)
return true;
}

bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool useExistingLockPoints)
bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flags, LockPoints* lp, bool useExistingLockPoints)
{
AssertLockHeld(cs_main);
AssertLockHeld(mempool.cs);
AssertLockHeld(pool.cs);

CBlockIndex* tip = chainActive.Tip();
assert(tip != nullptr);
@@ -387,7 +387,7 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool
}
else {
// pcoinsTip contains the UTXO set for chainActive.Tip()
CCoinsViewMemPool viewMemPool(pcoinsTip.get(), mempool);
CCoinsViewMemPool viewMemPool(pcoinsTip.get(), pool);
std::vector<int> prevheights;
prevheights.resize(tx.vin.size());
for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) {
@@ -679,7 +679,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
// be mined yet.
// Must keep pool.cs for this unless we change CheckSequenceLocks to take a
// CoinsViewCache instead of create its own
if (!CheckSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp))
if (!CheckSequenceLocks(pool, tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp))
return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final");

CAmount nFees = 0;
@@ -347,7 +347,7 @@ bool TestLockPointValidity(const LockPoints* lp) EXCLUSIVE_LOCKS_REQUIRED(cs_mai
*
* See consensus/consensus.h for flag definitions.
*/
bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

/**
* Closure representing one script verification

0 comments on commit efaf2d8

Please sign in to comment.
You can’t perform that action at this time.