Page MenuHomePhabricator

refactor: Move calculation logic out from `CheckSequenceLocksAtTip()`
ClosedPublic

Authored by PiRK on May 3 2024, 13:26.

Details

Summary

It is not obvious that CheckSequenceLocksAtTip() function can modify its LockPoints* lp parameter which leads to bugs such as https://github.com/bitcoin/bitcoin/pull/22677#discussion_r762040101 (bug avoided in D12441 by squashing the fix with the commit introducing it)

This is a backport of core#23897

Notes:

  • that this codebase has a single call site for CheckSequenceLocksAtTip in PreChecks. This is called via AcceptToMemoryPool in DisconnectedBlockTransactions::updateMempoolForReorg, so we don't have to deal with the code duplication in Core's Chainstate::MaybeUpdateMempoolForReorg function
  • this codebase no longer has the LockPoint::maxInputBlock computation code (see D15788)
Test Plan

ninja all check-all

Event Timeline

PiRK published this revision for review.May 3 2024, 13:45
PiRK edited the summary of this revision. (Show Details)
PiRK planned changes to this revision.May 3 2024, 13:54

i think i need to remove some unused code

remove unneeded max_input_height computation in CalculateLockPointsAtTip

Fabien requested changes to this revision.May 6 2024, 08:01
Fabien added a subscriber: Fabien.

@bot build-ibd

src/validation.h
449–451 ↗(On Diff #47601)

This comment makes no sense, you mixed it somehow

This revision now requires changes to proceed.May 6 2024, 08:01

fix doxygen comment for lock_points (remove duplicated part of a sentence from the coins_view doxygen comment from the previous function)

This revision is now accepted and ready to land.May 6 2024, 11:33