From b6002b07a36f0d58dc6becd04bfcf78599056b7c Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 3 Dec 2021 18:00:00 +0000 Subject: [PATCH 1/2] MOVEONLY: update_lock_points to txmempool.h --- src/txmempool.cpp | 10 ---------- src/txmempool.h | 10 ++++++++++ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index fcfc27d38ee..ba1bdb197ba 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -64,16 +64,6 @@ private: int64_t feeDelta; }; -struct update_lock_points -{ - explicit update_lock_points(const LockPoints& _lp) : lp(_lp) { } - - void operator() (CTxMemPoolEntry &e) { e.UpdateLockPoints(lp); } - -private: - const LockPoints& lp; -}; - bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp) { AssertLockHeld(cs_main); diff --git a/src/txmempool.h b/src/txmempool.h index f87ecc9cd08..ba1d381a5cc 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -312,6 +312,16 @@ public: } }; +struct update_lock_points +{ + explicit update_lock_points(const LockPoints& _lp) : lp(_lp) { } + + void operator() (CTxMemPoolEntry &e) { e.UpdateLockPoints(lp); } + +private: + const LockPoints& lp; +}; + // Multi_index tag names struct descendant_score {}; struct entry_time {}; From b4adc5ad6769e4a5a6179dfff271cd4c9dc47a5b Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 3 Dec 2021 18:03:16 +0000 Subject: [PATCH 2/2] [bugfix] update lockpoints correctly during reorg During a reorg, we re-check timelocks on all mempool entries using CheckSequenceLocks(useExistingLockPoints=false) and remove any now-invalid entries. CheckSequenceLocks() also mutates the LockPoints passed in, and we update valid entries' LockPoints using update_lock_points. Thus, update_lock_points(lp) needs to be called right after CheckSequenceLocks(lp), otherwise we lose the data in lp. commit bedf246 introduced a bug by separating those two loops. --- src/txmempool.cpp | 5 +---- src/validation.cpp | 2 ++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ba1bdb197ba..26cb2ff4b42 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -639,10 +639,7 @@ void CTxMemPool::removeForReorg(CChain& chain, std::function check } RemoveStaged(setAllRemoves, false, MemPoolRemovalReason::REORG); for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { - const LockPoints lp{it->GetLockPoints()}; - if (!TestLockPointValidity(chain, lp)) { - mapTx.modify(it, update_lock_points(lp)); - } + assert(TestLockPointValidity(chain, it->GetLockPoints())); } } diff --git a/src/validation.cpp b/src/validation.cpp index 203bd576765..68729e48637 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -378,6 +378,8 @@ void CChainState::MaybeUpdateMempoolForReorg( } } } + // CheckSequenceLocks updates lp. Update the mempool entry LockPoints. + if (!validLP) m_mempool->mapTx.modify(it, update_lock_points(lp)); return should_remove; };