Page MenuHomePhabricator

policy: Remove unused locktime flags
ClosedPublic

Authored by Fabien on Jan 5 2023, 16:34.

Details

Reviewers
sdulfari
Group Reviewers
Restricted Project
Commits
rABC1c4d06f75dc9: policy: Remove unused locktime flags
Summary
The locktime flags have many issues:

    They are passed in by a default argument, which is fragile. It has already lead to bugs like the one fixed in commit e30b6ea.
    They are negative (signed), which doesn't make sense for flags (unsigned in general). According to the review comments when the code was added: "The max on the flags is a fairly weird operation." (

BIP-113: Mempool-only median time-past as endpoint for lock-time calculations #6566 (comment))
No call site relies on the default argument and they all pass in a single compile-time constant, rendering most of the code dead and untested.
The dead code calls GetAdjustedTime (network adjusted time), which has its own issues. See

    AddTimeData will never update nTimeOffset past 199 samples #4521

Fix all issues by removing them

Backport of core#24080.

Depends on D12949 and D12950.

Test Plan
ninja all check-all

Diff Detail

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

Event Timeline

Fabien requested review of this revision.Jan 5 2023, 16:34
sdulfari added a subscriber: sdulfari.
sdulfari added inline comments.
src/validation.cpp
4061 ↗(On Diff #37463)

Nit

src/validation.h
571 ↗(On Diff #37463)

Missing?

This revision is now accepted and ready to land.Jan 5 2023, 18:00
This revision was automatically updated to reflect the committed changes.