mempool: recalculate stale BIP68 lockpoints with mempool parents in removeForReorg #35026

javierpmateos commented at 8:13 PM on April 7, 2026: none

When a transaction with nSequence=0 (BIP68 relative locktime 0) enters the mempool with an unconfirmed parent, CalculatePrevHeights assigns nCoinHeight=tip+1 and the resulting LockPoints are cached with maxInputBlock=genesis. After invalidateblock lowers the tip, TestLockPointValidity returns true for genesis (always on chain), so the stale cached lockpoints are reused. CheckSequenceLocksAtTip then incorrectly determines the lock is not satisfied and removes the transaction along with all its descendants.

The fix detects the genesis sentinel (maxInputBlock->nHeight==0 with lp.height>0) in filter_final_and_mature, which indicates lockpoints computed with mempool parent inputs, and forces fresh recalculation via CalculateLockPointsAtTip instead of using the stale cache.

Added mempool_reorg_bip68_stale_lockpoints.py which verifies that both BIP68-disabled and BIP68-enabled children with mempool parents survive a multi-block reorg via invalidateblock.

Reproducer for unpatched nodes: https://gist.github.com/javierpmateos/c55d365973adbf488a852dc5e0b77dec

Same class of issue fixed for TRUC in #33504.

Closes #35007

DrahtBot added the label Mempool on Apr 7, 2026

DrahtBot commented at 8:13 PM on April 7, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35026.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	Bicaru20
Stale ACK	ismaelsadeeq

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

javierpmateos force-pushed on Apr 7, 2026

javierpmateos force-pushed on Apr 8, 2026

instagibbs commented at 1:15 PM on April 8, 2026: member

thanks for the PR. From my recollection this code is pretty complicated sadly. Will take a look soon hopefully to see if I can grasp it

DrahtBot added the label CI failed on Apr 9, 2026

DrahtBot removed the label CI failed on Apr 9, 2026

in src/validation.cpp:353 in 9dc0642ac5 outdated

 349 | @@ -350,7 +350,7 @@ void Chainstate::MaybeUpdateMempoolForReorg(
 350 |          const LockPoints& lp = it->GetLockPoints();
 351 |          // CheckSequenceLocksAtTip checks if the transaction will be final in the next block to be
 352 |          // created on top of the new chain.
 353 | -        if (TestLockPointValidity(m_chain, lp)) {
 354 | +        if (TestLockPointValidity(m_chain, lp) && !(lp.maxInputBlock && lp.maxInputBlock->nHeight == 0 && lp.height > 0)) {

Bicaru20 commented at 12:39 PM on May 3, 2026:

Since we already call TestLockPointValidity, maybe it would be better if this condition: !(lp.maxInputBlock && lp.maxInputBlock->nHeight == 0 && lp.height > 0)) is included inside of the TestLockPointValidity. We don't have to pass any extra parameters and that way the condition on the if is cleaner.

javierpmateos commented at 5:42 PM on May 10, 2026:

Thanks for the suggestion. I attempted this refactor but it breaks the invariant asserted in CTxMemPool::removeForReorg() at txmempool.cpp:390:

assert(TestLockPointValidity(chain, it->GetLockPoints()));

When filter_final_and_mature recalculates lockpoints for a tx with mempool parents, CalculateLockPointsAtTip ignores mempool inputs (height == tip+1) when computing max_input_height, leaving it at 0. The resulting maxInputBlock is tip->GetAncestor(0) = genesis. So the freshly recalculated lockpoints still have maxInputBlock=genesis after UpdateLockPoints.

If the genesis-sentinel check were inside TestLockPointValidity, those legitimately-recalculated lockpoints would fail the post-removal assert in removeForReorg, crashing bitcoind (verified empirically — tested locally and the assertion fires).

Keeping the check at the caller site preserves TestLockPointValidity's broader semantics (it answers "is maxInputBlock still on chain?") while addressing the specific stale-lockpoints case in filter_final_and_mature where lp.height>0 combined with the genesis sentinel signals lockpoints from a previous tip.

Bicaru20 commented at 8:57 AM on May 13, 2026:

I see, I also got the same error. Good catch!

in test/functional/mempool_reorg_bip68_stale_lockpoints.py:60 in 9dc0642ac5

  55 | +            node, output=self.wallet.get_address(), transactions=[],
  56 | +        )["hash"]
  57 | +
  58 | +        node.invalidateblock(block_Y)
  59 | +        node.invalidateblock(block_setup)
  60 | +        assert node.getblockcount() == H - 1

Bicaru20 commented at 12:42 PM on May 3, 2026:

Maybe use assert_equal

        assert_equal(node.getblockcount(), H - 1)

javierpmateos commented at 5:42 PM on May 10, 2026:

Done in latest push.

in test/functional/mempool_reorg_bip68_stale_lockpoints.py:58 in 9dc0642ac5

  53 | +
  54 | +        block_Y = self.generateblock(
  55 | +            node, output=self.wallet.get_address(), transactions=[],
  56 | +        )["hash"]
  57 | +
  58 | +        node.invalidateblock(block_Y)

Bicaru20 commented at 7:39 PM on May 3, 2026:

I'm not getting why do we need this. You create a block and immediately invalidate it. block_setup is needed to reproduce the bug, since the LockPoints were calculated with this blockchain height. However, once that is done, why generate another block? If I am not mistaken this last block does not affect at all on the calculation.

I've tried the test without this block and the behavior is the same.

javierpmateos commented at 5:42 PM on May 10, 2026:

You're right, verified empirically — the bug reproduces without block_Y. Removed in latest push.

javierpmateos force-pushed on May 10, 2026

Bicaru20 commented at 9:00 AM on May 13, 2026: contributor

ACK c281b1ea0b

Bortlesboat commented at 9:45 PM on May 20, 2026: contributor

Tested c281b1ea0b17a969b29f12665c12e1b2f8aab664 on Ubuntu 24.04 WSL2, GCC 13.3, debug build with GUI and IPC disabled.

build/test/functional/test_runner.py mempool_reorg_bip68_stale_lockpoints.py --jobs=1 passes on this branch.

I also ran the same test file against master 489da5df60f797a13e162e8e6ab89c0a7d9f5b2f, where it fails with:

AssertionError: child_B (BIP68 enabled) must survive after fix

So the regression test catches the stale-lockpoint behavior described in #35007.

sedited requested review from ismaelsadeeq on Jul 6, 2026

ismaelsadeeq commented at 2:43 PM on July 15, 2026: member

Good find, but I think this only handles the case where all of the transaction's inputs are unconfirmed that's the only way maxInputBlock ends up at genesis.

If a transaction spends a mix of confirmed and unconfirmed inputs, the cached lp.height still carries the tip+1 assumption from the unconfirmed input, but maxInputBlock points to the confirmed input's block.

If that block survives the reorg, TestLockPointValidity passes, the genesis check added here doesn't fire, and CheckSequenceLocksAtTip below fails against the lowered tip, the transaction is still falsely evicted along with its descendants.

The new test doesn't catch this because both children spend only unconfirmed parents; it fails with this adjustment.

diff --git a/test/functional/mempool_reorg_bip68_stale_lockpoints.py b/test/functional/mempool_reorg_bip68_stale_lockpoints.py
index 8c73f5e5d1..a066b8be8d 100755
--- a/test/functional/mempool_reorg_bip68_stale_lockpoints.py
+++ b/test/functional/mempool_reorg_bip68_stale_lockpoints.py
@@ -23,6 +23,13 @@ class MempoolReorgBip68StaleLocksTest(BitcoinTestFramework):
         self.wallet = MiniWallet(node)
         self.generate(self.wallet, 200)

+        # Confirm a utxo one block BELOW the block we will invalidate, so that
+        # the mixed spend below keeps a confirmed input through the reorg.
+        mixed_funding = self.wallet.send_self_transfer(
+            from_node=node, confirmed_only=True,
+        )
+        self.generate(node, 1)
+
         funding = self.wallet.send_self_transfer_multi(
             from_node=node, num_outputs=2, confirmed_only=True,
         )
@@ -49,8 +56,22 @@ class MempoolReorgBip68StaleLocksTest(BitcoinTestFramework):
             sequence=SEQ_BIP68_ZERO,
         )

+        child_mixed = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[mixed_funding["new_utxo"], child_B["new_utxo"]],
+            sequence=SEQ_BIP68_ZERO,
+        )
+        node.sendrawtransaction(child_mixed["hex"])
+
         self.log.info("child_A seq=0xFFFFFFFE: %s", child_A["txid"][:16])
         self.log.info("child_B seq=0x00000000: %s", child_B["txid"][:16])
+        self.log.info("child_mixed confirmed+unconfirmed inputs, seq=0x00000000: %s", child_mixed["txid"][:16])

         node.invalidateblock(block_setup)
         assert_equal(node.getblockcount(), H - 1)
@@ -59,9 +80,11 @@ class MempoolReorgBip68StaleLocksTest(BitcoinTestFramework):

         assert child_A["txid"] in mp, "child_A (BIP68 disabled) must survive"
         assert child_B["txid"] in mp, "child_B (BIP68 enabled) must survive after fix"
+        assert child_mixed["txid"] in mp, "child_mixed (confirmed + unconfirmed inputs) must survive after fix"

         self.log.info("child_A (BIP68 disabled): SURVIVED")
         self.log.info("child_B (BIP68 enabled):  SURVIVED (stale lockpoints recalculated)")
+        self.log.info("child_mixed (mixed inputs): SURVIVED (stale lockpoints recalculated)")
         self.log.info("PASS: removeForReorg correctly recalculates BIP68 lockpoints")

A simpler fix is to never evict on the cached values alone: recalculate whenever the lockpoints are stale, or the cached check fails, and evict only if the fresh result also fails. Of course, this has a tradeoff, we may recalculate when we don't need to.

diff --git a/src/validation.cpp b/src/validation.cpp
index fac537f99e..adf4fc6e47 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -350,11 +350,7 @@ void Chainstate::MaybeUpdateMempoolForReorg(
         const LockPoints& lp = it->GetLockPoints();
         // CheckSequenceLocksAtTip checks if the transaction will be final in the next block to be
         // created on top of the new chain.
-        if (TestLockPointValidity(m_chain, lp) && !(lp.maxInputBlock && lp.maxInputBlock->nHeight == 0 && lp.height > 0)) {
-            if (!CheckSequenceLocksAtTip(m_chain.Tip(), lp)) {
-                return true;
-            }
-        } else {
+        if (!TestLockPointValidity(m_chain, lp) || !CheckSequenceLocksAtTip(m_chain.Tip(), lp)) {
             const CCoinsViewMemPool view_mempool{&CoinsTip(), *m_mempool};
             const std::optional<LockPoints> new_lock_points{CalculateLockPointsAtTip(m_chain.Tip(), view_mempool, tx)};
             if (new_lock_points.has_value() && CheckSequenceLocksAtTip(m_chain.Tip(), *new_lock_points)) {

instagibbs commented at 3:11 PM on July 15, 2026: member

in addition to what @ismaelsadeeq said it seems it misses time-based variant, if MTP drops post-reorg. his fix seems to work more generally and less brittle?

javierpmateos force-pushed on Jul 16, 2026

javierpmateos commented at 9:34 AM on July 16, 2026: none

Good find, but I think this only handles the case where all of the transaction's inputs are unconfirmed that's the only way maxInputBlock ends up at genesis.

If a transaction spends a mix of confirmed and unconfirmed inputs, the cached lp.height still carries the tip+1 assumption from the unconfirmed input, but maxInputBlock points to the confirmed input's block.

If that block survives the reorg, TestLockPointValidity passes, the genesis check added here doesn't fire, and CheckSequenceLocksAtTip below fails against the lowered tip, the transaction is still falsely evicted along with its descendants.

The new test doesn't catch this because both children spend only unconfirmed parents; it fails with this adjustment.

diff --git a/test/functional/mempool_reorg_bip68_stale_lockpoints.py b/test/functional/mempool_reorg_bip68_stale_lockpoints.py
index 8c73f5e5d1..a066b8be8d 100755
--- a/test/functional/mempool_reorg_bip68_stale_lockpoints.py
+++ b/test/functional/mempool_reorg_bip68_stale_lockpoints.py
@@ -23,6 +23,13 @@ class MempoolReorgBip68StaleLocksTest(BitcoinTestFramework):
         self.wallet = MiniWallet(node)
         self.generate(self.wallet, 200)

+        # Confirm a utxo one block BELOW the block we will invalidate, so that
+        # the mixed spend below keeps a confirmed input through the reorg.
+        mixed_funding = self.wallet.send_self_transfer(
+            from_node=node, confirmed_only=True,
+        )
+        self.generate(node, 1)
+
         funding = self.wallet.send_self_transfer_multi(
             from_node=node, num_outputs=2, confirmed_only=True,
         )
@@ -49,8 +56,22 @@ class MempoolReorgBip68StaleLocksTest(BitcoinTestFramework):
             sequence=SEQ_BIP68_ZERO,
         )

+        child_mixed = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[mixed_funding["new_utxo"], child_B["new_utxo"]],
+            sequence=SEQ_BIP68_ZERO,
+        )
+        node.sendrawtransaction(child_mixed["hex"])
+
         self.log.info("child_A seq=0xFFFFFFFE: %s", child_A["txid"][:16])
         self.log.info("child_B seq=0x00000000: %s", child_B["txid"][:16])
+        self.log.info("child_mixed confirmed+unconfirmed inputs, seq=0x00000000: %s", child_mixed["txid"][:16])

         node.invalidateblock(block_setup)
         assert_equal(node.getblockcount(), H - 1)
@@ -59,9 +80,11 @@ class MempoolReorgBip68StaleLocksTest(BitcoinTestFramework):

         assert child_A["txid"] in mp, "child_A (BIP68 disabled) must survive"
         assert child_B["txid"] in mp, "child_B (BIP68 enabled) must survive after fix"
+        assert child_mixed["txid"] in mp, "child_mixed (confirmed + unconfirmed inputs) must survive after fix"

         self.log.info("child_A (BIP68 disabled): SURVIVED")
         self.log.info("child_B (BIP68 enabled):  SURVIVED (stale lockpoints recalculated)")
+        self.log.info("child_mixed (mixed inputs): SURVIVED (stale lockpoints recalculated)")
         self.log.info("PASS: removeForReorg correctly recalculates BIP68 lockpoints")

A simpler fix is to never evict on the cached values alone: recalculate whenever the lockpoints are stale, or the cached check fails, and evict only if the fresh result also fails. Of course, this has a tradeoff, we may recalculate when we don't need to.

diff --git a/src/validation.cpp b/src/validation.cpp
index fac537f99e..adf4fc6e47 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -350,11 +350,7 @@ void Chainstate::MaybeUpdateMempoolForReorg(
         const LockPoints& lp = it->GetLockPoints();
         // CheckSequenceLocksAtTip checks if the transaction will be final in the next block to be
         // created on top of the new chain.
-        if (TestLockPointValidity(m_chain, lp) && !(lp.maxInputBlock && lp.maxInputBlock->nHeight == 0 && lp.height > 0)) {
-            if (!CheckSequenceLocksAtTip(m_chain.Tip(), lp)) {
-                return true;
-            }
-        } else {
+        if (!TestLockPointValidity(m_chain, lp) || !CheckSequenceLocksAtTip(m_chain.Tip(), lp)) {
             const CCoinsViewMemPool view_mempool{&CoinsTip(), *m_mempool};
             const std::optional<LockPoints> new_lock_points{CalculateLockPointsAtTip(m_chain.Tip(), view_mempool, tx)};
             if (new_lock_points.has_value() && CheckSequenceLocksAtTip(m_chain.Tip(), *new_lock_points)) {

Thanks for the thorough review — you're right on both counts. I've adopted your approach of recalculating whenever the cached validity check or the sequence-lock check fails, rather than detecting the genesis sentinel specifically. It's simpler and covers the mixed confirmed/unconfirmed input case that my original check missed.

I've also added your child_mixed case to the regression test (confirmed input below block_setup + unconfirmed input), which fails on the previous genesis-only approach and passes with the general recalculation.

Confirmed locally: the extended test passes with this fix, and mempool_reorg.py / feature_bip68_sequence.py still pass.

javierpmateos commented at 9:34 AM on July 16, 2026: none

in addition to what @ismaelsadeeq said it seems it misses time-based variant, if MTP drops post-reorg. his fix seems to work more generally and less brittle?

Good point on the time-based variant. The recalculation approach handles it uniformly ... CheckSequenceLocksAtTip evaluates both height and time locks, so any stale cache (height or MTP-based) triggers recalculation the same way.

I tried adding a time-based case to the functional test but reproducing it reliably in regtest is brittle: a time-based relative lock over an unconfirmed parent is rejected as non-BIP68-final on mempool entry (the 512s lock isn't satisfied against tip+1), and forcing the MTP to drop post-reorg requires careful timestamp choreography that would make the test flaky. Given the fix covers height and time uniformly by construction, I've kept the test to the height-based and mixed-input cases that reproduce deterministically. Open to suggestions if you think a robust time-based case is worth adding.

ismaelsadeeq approved

ismaelsadeeq commented at 12:39 PM on July 20, 2026: member

ACK 3813fc8c9d6a70ce2e22fa2295e2ff95837ba67b

nit: commit message is too verbose, and does not capture the time variant indicated by @instagibbs #35026 (comment), this test is sufficient, but it will be nice to capture the time based variant as well to guard against future regression.

DrahtBot requested review from Bicaru20 on Jul 20, 2026

javierpmateos force-pushed on Jul 20, 2026

javierpmateos commented at 7:11 PM on July 20, 2026: none

ACK 3813fc8

nit: commit message is too verbose, and does not capture the time variant indicated by @instagibbs #35026 (comment), this test is sufficient, but it will be nice to capture the time based variant as well to guard against future regression.

Thanks! ... shortened the commit message and added the Co-authored-by trailer.

On the time-based variant: I spent some time trying to build a deterministic functional test for it and I don't think one exists cleanly. The height-based mixed case works as a regression test because lp.height caches an artificially high tip+1 from the unconfirmed input while the confirmed input stays valid at the new tip — so the tx is genuinely valid post-reorg but the stale cache marks it invalid.

For the time-based case, when the reorg lowers the MTP, the transaction tends to become genuinely invalid (the relative time hasn't actually elapsed on the reorged chain), so eviction is correct rather than a bug. I instrumented several scenarios (varying lock magnitude and reorg depth) and confirmed the eviction result is identical on this branch and on master — i.e. not a regression the test could catch.

The fix still covers the time-based path by construction, since the recalculation branch runs whenever CheckSequenceLocksAtTip fails regardless of whether the lock is height- or time-based. If you know of a timestamp arrangement that produces a genuinely-valid-but-stale time-based case, I'm happy to add it — otherwise I've kept the test to the cases that reproduce deterministically.

Note: the only change since your ACK is the commit message and the Co-authored-by trailer — the code (validation.cpp and the test) is byte-identical to 3813fc8.

Bicaru20 commented at 11:31 AM on August 20, 2026: contributor

I've been playing around with the test to see how we could add a test for the time-based variant.

My logic tells me that in order to reproduce the bug, we need to modify the median time of the blocks (which is taken considering the last 11 blocks). So in theory, if we create 6 blocks with a higher timestamp using node.setmocktime we would modify the median (lets call this median median_new and the previous median median_old). Then when we create the transaction with the sequence number for the time-based variant, that transaction would have the minimum time set at the median_new. When we invalidate the last 6 blocks, the locktime from the transaction would need to be less or equal tahn median_old, so if the fix works, the minimum time from the transaction will be set to the median_old and will not be evicted from the mempool. Here is my implementation for the test:

--- a/test/functional/mempool_reorg_bip68_stale_lockpoints.py
+++ b/test/functional/mempool_reorg_bip68_stale_lockpoints.py
@@ -5,11 +5,12 @@
 """Test that removeForReorg correctly handles BIP68 transactions."""
 
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal
+from test_framework.util import assert_equal, assert_greater_than
 from test_framework.wallet import MiniWallet
 
 SEQ_BIP68_DISABLE = 0xFFFFFFFE
 SEQ_BIP68_ZERO = 0x00000000
+SEQ_BIP68_ZERO_TIME = 0x00400000
 
 
 class MempoolReorgBip68StaleLocksTest(BitcoinTestFramework):
@@ -31,7 +32,7 @@ class MempoolReorgBip68StaleLocksTest(BitcoinTestFramework):
         self.generate(node, 1)
 
         funding = self.wallet.send_self_transfer_multi(
-            from_node=node, num_outputs=2, confirmed_only=True,
+            from_node=node, num_outputs=3, confirmed_only=True,
         )
         block_setup = self.generate(node, 1)[0]
         self.wallet.rescan_utxos(include_mempool=True)
@@ -84,6 +85,37 @@ class MempoolReorgBip68StaleLocksTest(BitcoinTestFramework):
         self.log.info("child_mixed (mixed inputs): SURVIVED (stale lockpoints recalculated)")
         self.log.info("PASS: removeForReorg correctly recalculates BIP68 lockpoints")
 
+        H = node.getblockcount()
+        current_time = node.getblockheader(node.getbestblockhash())['time']
+        # Generate 6 blocks with the same time for the median
+        for _ in range(6):
+            node.setmocktime(current_time + 1000)
+            self.generate(node, 1)
+
+        first_median = node.getblockheader(node.getbestblockhash())['mediantime']
+        self.log.info(f"Median time of last block: {first_median}")
+
+        node.setmocktime(current_time + 20000)
+        block_setup = self.generate(node, 6)[0]
+        second_median = node.getblockheader(node.getbestblockhash())['mediantime']
+        self.log.info(f"Median time of last block: {second_median}")
+
+        # To reproduce the bug, the median must be higher than the one we previously had
+        assert_greater_than(second_median, first_median)
+        assert_equal(node.getblockcount(), H + 6 + 6)
+
+        parent_time = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=funding["new_utxos"][2])
+        child_time = self.wallet.send_self_transfer(
+            from_node=node, utxo_to_spend=parent_time["new_utxo"],
+            sequence=SEQ_BIP68_ZERO_TIME,
+        )
+
+        node.invalidateblock(block_setup)
+        breakpoint()
+        assert_equal(node.getblockcount(), H + 6)
+        mp = node.getrawmempool()
+        assert child_time["txid"] in mp, "child_time (BIP68 enabled time-based) must survive after fix"
+        self.log.info(f"Time of last block: {node.getblockheader(node.getbestblockhash())['mediantime']}")

Without the fix, the test shouldn't pass as is. But if we were to create 5 blocks instead of 6, the test should pass as the median is not modified. However, that is not the case since time passes between the creation of the blocks. That makes the test fail withouth the fix when creating 5 or less blocks instead of 6. So in order to make the test constant, I think it is important to make the median time as predictable as possible. That is way before creating the 6 blocks with a higher timestamp, I create 6 blocks with exactly the same timestamp, so the median is constant.

You can try run the test without the fix and you'll see that it will pass if we generate 5 blocks instead of 6.

Let me know what you think about this approach, hope I explained myself clearly about what I am trying to do.

javierpmateos force-pushed on Aug 20, 2026

javierpmateos commented at 10:33 PM on August 20, 2026: none

Thanks ... this works, and the reasoning about stabilising the median first is the piece I was missing. I'd tried a few time-based scenarios and kept hitting cases where the eviction was genuinely correct rather than a bug; using a zero-value time lock (0x00400000) is what separates "valid at the new tip but marked invalid by the stale cache" from "actually invalid", the same way SEQ_BIP68_ZERO does on the height side.

I've added it to the test (dropped the breakpoint(), kept a separate variable for the second setup block, and reset setmocktime at the end). Verified against three binaries:

v31.0rc4 (no fix): fails at child_B
the original genesis-sentinel approach: child_B passes, fails at child_mixed
this branch: all four cases pass

So the test also catches the limitation of the earlier approach, not just the absence of any fix. The median moves from 1787265629 to 1787284629 in the setup, so there's a wide margin rather than the test sitting on the edge.

Happy to add a Co-authored-by trailer for the test if you'd like ... let me know what name/email to use.

Bicaru20 commented at 8:37 PM on August 22, 2026: contributor

ACK 8e416e0598

Happy to add a Co-authored-by trailer for the test if you'd like ... let me know what name/email to use.

That would be great, thanks! Co-Authored-By: Bicaru20 <bicaru2@gmail.com>

DrahtBot requested review from ismaelsadeeq on Aug 22, 2026

mempool: recalculate stale BIP68 lockpoints in removeForReorg

BIP68 transactions entering the mempool with unconfirmed parents cache
LockPoints relative to the current tip (nCoinHeight=tip+1). After
invalidateblock lowers the tip, these cached lockpoints are stale, but
TestLockPointValidity may still return true, so filter_final_and_mature
evicts the transaction and its descendants based on the stale cache.

Recalculate via CalculateLockPointsAtTip whenever the cached validity
check or the sequence-lock check fails, and only evict if the fresh
result also fails. This handles all-unconfirmed, mixed, and time-based
relative locktimes uniformly.

The functional test covers all four cases: BIP68-disabled, height-based
with unconfirmed parents, mixed confirmed/unconfirmed inputs, and a
time-based relative locktime where the reorg lowers the median time past.

Co-authored-by: Abubakar Sadiq Ismail <abubakarsadiqismail@proton.me>
Co-authored-by: Bicaru20 <bicaru2@gmail.com>

Closes #35007

ea2d638bb9

javierpmateos force-pushed on Aug 23, 2026

javierpmateos commented at 4:02 PM on August 23, 2026: none

Added the trailer, thanks @Bicaru20. Only the commit message changed ... the code is byte-identical to 8e416e0.

Bicaru20 commented at 9:34 AM on August 24, 2026: contributor

re-ACK ea2d638bb94542290578bd65c7c16f2849c56071