validation: Pass tx pool reference into CheckSequenceLocks #13783

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1807-txPoolValidation changing 4 files +8 −8

MarcoFalke commented at 6:28 PM on July 27, 2018: member

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)
MarcoFalke added the label Refactoring on Jul 27, 2018
MarcoFalke added the label Validation on Jul 27, 2018
MarcoFalke commented at 8:46 PM on July 30, 2018: member

This is required for and split off of #13804
DrahtBot commented at 11:22 AM on August 1, 2018: member
Reviewers, this pull request conflicts with the following ones:
- #13864 (validation: Document where we are intentionally ignoring bool return values from validation related functions by practicalswift)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
promag commented at 12:41 PM on August 1, 2018: member

Tested ACK fad3b321.
DrahtBot added the label Needs rebase on Aug 25, 2018
MarcoFalke force-pushed on Aug 25, 2018
DrahtBot removed the label Needs rebase on Aug 26, 2018
MarcoFalke force-pushed on Aug 29, 2018
MarcoFalke force-pushed on Sep 10, 2018
MarcoFalke force-pushed on Sep 10, 2018
Pass tx pool reference into CheckSequenceLocks fa511e8dad
MarcoFalke force-pushed on Sep 11, 2018
ryanofsky commented at 8:46 PM on September 12, 2018: member

~utACK fac95398366f644911b58f1605e6bc37fb76782d~. Change is simple and looks good if needed for #13804.
MarcoFalke commented at 6:58 PM on September 17, 2018: member

@ryanofsky It seems the commit you reviewed is unrelated to this pull request?
ryanofsky commented at 7:10 PM on September 17, 2018: member

@ryanofsky It seems the commit you reviewed is unrelated to this pull request?

utACK fa511e8dad87ddee7bf03b82f2ed69e546021004, pasted the wrong hash previously
in src/validation.cpp:366 in fa511e8dad
```
 360 | @@ -361,10 +361,10 @@ bool TestLockPointValidity(const LockPoints* lp)
 361 |      return true;
 362 |  }
 363 |  
 364 | -bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool useExistingLockPoints)
 365 | +bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flags, LockPoints* lp, bool useExistingLockPoints)
 366 |  {
 367 |      AssertLockHeld(cs_main);
```
promag commented at 9:38 PM on September 17, 2018:

Unrelated to this PR, but couldn't this AssertLockHeld be removed?

MarcoFalke commented at 10:07 PM on September 17, 2018:

Good point, but I think we want to keep these for now, since not all compilers support these compile-time annotations and sometimes they have to be disabled due to shortcomings.

promag commented at 10:38 PM on September 17, 2018:

On the other hand, the annotation is incomplete, missing pool.cs?

MarcoFalke commented at 10:50 PM on September 17, 2018:

True, but should be done in a separate pull, since the changes required are non-trivial (more than one line)

promag commented at 6:13 PM on October 8, 2018:

In order to add that annotation txmempool.h must be included in validation.h and I'm not sure if that is correct because of the circular dependency "txmempool -> validation -> txmempool". I don't know what is the plan here but I think mempool should not depend on validation or am I wrong?
promag commented at 9:38 PM on September 17, 2018: member

utACK fa511e8.
MarcoFalke merged this on Oct 27, 2018
MarcoFalke closed this on Oct 27, 2018
MarcoFalke deleted the branch on Oct 27, 2018
MarcoFalke referenced this in commit efaf2d85e3 on Oct 27, 2018
jasonbcox referenced this in commit 6518f3585e on Oct 11, 2019
jonspock referenced this in commit bb61b81193 on Dec 27, 2019
jonspock referenced this in commit df27323a67 on Dec 29, 2019
Munkybooty referenced this in commit 0cc7852056 on Jul 22, 2021
Munkybooty referenced this in commit 47aaacb2fc on Jul 22, 2021
Munkybooty referenced this in commit 8eab98a823 on Jul 23, 2021
Munkybooty referenced this in commit c7d1cbf30d on Jul 23, 2021
Munkybooty referenced this in commit 14af9c67b6 on Jul 26, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Refactoring Validation