Page MenuHomePhabricator
Differential D16104

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

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

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC22f5560fd2f6: refactor: Move calculation logic out from `CheckSequenceLocksAtTip()`
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

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK created this revision.Fri, May 3, 13:26
Herald added a reviewer: Restricted Project. · View Herald TranscriptFri, May 3, 13:26
teamcity edited the summary of this revision. (Show Details)Fri, May 3, 13:32
PiRK published this revision for review.Fri, May 3, 13:45
PiRK edited the summary of this revision. (Show Details)
PiRK updated this revision to Diff 47600.Fri, May 3, 13:50

use tx.GetId()

Harbormaster completed remote builds in B28875: Diff 47596.Fri, May 3, 13:50
PiRK planned changes to this revision.Fri, May 3, 13:54

i think i need to remove some unused code

PiRK updated this revision to Diff 47601.Fri, May 3, 13:58

remove unneeded max_input_height computation in CalculateLockPointsAtTip

Harbormaster completed remote builds in B28879: Diff 47600.Fri, May 3, 14:08
Harbormaster completed remote builds in B28880: Diff 47601.Fri, May 3, 14:16
Fabien requested changes to this revision.Mon, May 6, 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.Mon, May 6, 08:01
teamcity awarded a token.Mon, May 6, 08:01
PiRK updated this revision to Diff 47628.Mon, May 6, 09:43

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

Fabien accepted this revision.Mon, May 6, 11:33
This revision is now accepted and ready to land.Mon, May 6, 11:33
This revision was landed with ongoing or failed builds.Mon, May 6, 11:54
Closed by commit rABC22f5560fd2f6: refactor: Move calculation logic out from `CheckSequenceLocksAtTip()` (authored by Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>, committed by PiRK). · Explain Why
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC22f5560fd2f6: refactor: Move calculation logic out from `CheckSequenceLocksAtTip()`.

Revision Contents
Changeset List

PathSize
src/
test/
7 lines
33 lines
104 lines

Diff 47631

View Options

src/test/miner_tests.cpp

Loading...
View Options

src/validation.h

Loading...
View Options

src/validation.cpp

Loading...