Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge bitcoin#23683: bug fix: valid but different LockPoints after a …
…reorg b4adc5a [bugfix] update lockpoints correctly during reorg (glozow) b6002b0 MOVEONLY: update_lock_points to txmempool.h (glozow) Pull request description: I introduced a bug in bitcoin#22677 (sorry! 😅) Mempool entries cache `LockPoints`, containing the first height/blockhash/`CBlockIndex*` at which the transaction becomes valid. 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`. I incorrectly assumed they could be called in separate loops. The incorrect behavior introduced is: if we have a reorg in which a timelocked mempool transaction is still valid but becomes valid at a different block, the cached `LockPoints` will be incorrect. This PR fixes the bug, adds a test, and adds an assertion at the end of `removeForReorg()` to check that all mempool entries' lockpoints are valid. You can reproduce the bug by running the test added in the [test] commit on the code before the [bugfix] commit. ACKs for top commit: jnewbery: ACK b4adc5a vasild: ACK b4adc5a mzumsande: Code Review ACK b4adc5a hebasto: ACK b4adc5a MarcoFalke: re-ACK b4adc5a 🏁 Tree-SHA512: 16b59f6ff8140d0229079ca1c6b04f2f4a00a2e49931275150e4f3fe5ac4ec109698b083fa6b223ba9511f328271cc1ab081263669d5da020af7fee83c13e401
- Loading branch information