Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABCf662f94f685d: Add function to validate difficulty changes

Summary

The rule against difficulty adjustments changing by more than a factor of 4 can
be helpful for anti-DoS measures in contexts where we lack a full headers
chain, so expose this functionality separately and in the narrow case where we
only know the height, new value, and old value.

The function was adjusted to account for the changes in difficulty algorithms after the 2017 BCH fork. The EDA algorithm no longer limits the difficulty change interval to 2016 blocks but still keeps the rule that the difficulty cannot change by more than a factor 4. The DAA (CW-144) and ASERTI3D algorithms no longer have hard rules for difficulty transitions, so the function always returns true after the november 15 2017 hard fork block height.

This is a backport of core#25717
https://github.com/bitcoin/bitcoin/pull/25717/commits/1d4cfa4272cf2c8b980cc8762c1ff2220d3e8d51

Test Plan

ninja && ninja check

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 26261
Build 52094: Build Diff
Build 52093: arc lint + arc unit

Event Timeline

PiRK created this revision.Jan 10 2024, 08:37

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 10 2024, 08:37

PiRK requested review of this revision.Jan 10 2024, 08:37

teamcity edited the summary of this revision. (Show Details)Jan 10 2024, 08:37

Harbormaster completed remote builds in B26235: Diff 44011.Jan 10 2024, 08:53

PiRK edited the summary of this revision. (Show Details)Jan 10 2024, 14:15

Fabien requested changes to this revision.Jan 10 2024, 16:43

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/pow/eda.cpp
127 ↗	(On Diff #44011)	Why don't you move that in the place you put the DAA check ? This is inconsistent
129 ↗	(On Diff #44011)	This is different from what the source material does if height % params.DifficultyAdjustmentInterval() != 0 && old_nbits == new_nbits (which is the normal case). This will still return true but you have to do all the computation which defeats the purpose of the modulus on height
172–175 ↗	(On Diff #44011)

This revision now requires changes to proceed.Jan 10 2024, 16:43

review: avoid unnecessary computation when the difficulty stays constants, move the UAHF specific check from eda.cpp to pow.cpp, simplify boolean return statement.

Harbormaster completed remote builds in B26261: Diff 44053.Jan 11 2024, 08:21

Fabien requested changes to this revision.Jan 11 2024, 08:35

Fabien added inline comments.

src/pow/pow.cpp
61	This is wrong when you are recomputing the difficulty. If you got a billion blocks in the last interval, clearly using the same difficulty is wrong
70	I still don't get it. UAHF height < DAA height, so why do you need to check both ? If UAHF is enabled, then DAA is enabled. Can't you just return true if UAHF is enabled and then just use the PermittedEDADifficultyTransition algo ?

This revision now requires changes to proceed.Jan 11 2024, 08:35

PiRK added inline comments.Jan 11 2024, 08:59

src/pow/pow.cpp
61	Intervals are defined as a number of blocks, so you cannot have a billion blocks in them. This function really only checks that the difficulty only changes when allowed to do so, and does not change by more than a factor 4. I don't see how PermittedEDADifficultyTransition can ever return false if `old_nbits == new_nbits`.
70	I don't understand what you mean. My understanding is that we need to treat the legacy algorithm (height < uahfHeight) differently from the EDA algorithm (height < daaHeight). In the first case the difficulty is not allowed to change unless `height % params.DifficultyAdjustmentInterval() == 0`. In both cases we can check for the factor 4 rule.

PiRK requested review of this revision.Jan 11 2024, 09:16

As per offline discussion it's worth checking for min pow on every algo

src/pow/pow.cpp
61	yeah I meant short times in the interval. You're correct this can never return false if old_nbits == new_nbits
70	OK the intent was to avoid that computation entirely for the interval [uahf..daa] but this solution is obviously more accurate and the computation cost might not be that terrible on second thought

move powLimit check out of PermittedEDADifficultyTransition and before IsDAAEnabled(params, height - 1), to always check for it.

Tested by rebasing D15129 and running a header sync on mainnet and testnet

Harbormaster completed remote builds in B26263: Diff 44055.Jan 11 2024, 11:35

Fabien requested changes to this revision.Jan 11 2024, 12:57

Fabien added inline comments.

src/pow/eda.cpp
135	This is still relevant, you're clamping the difficulty after it's been multiplied by the largest time span and not comparing new_target directly here
153	dito

This revision now requires changes to proceed.Jan 11 2024, 12:57

restore powLimit clamping in PermittedEDADifficultyTransition, and remove unused height argument

fix lack of verb in comment

Fabien accepted this revision.Jan 11 2024, 15:16

This revision is now accepted and ready to land.Jan 11 2024, 15:16

Harbormaster completed remote builds in B26271: Diff 44067.Jan 11 2024, 15:26

Harbormaster completed remote builds in B26272: Diff 44068.Jan 11 2024, 15:38

Closed by commit rABCf662f94f685d: Add function to validate difficulty changes (authored by Suhas Daftuar <sdaftuar@gmail.com>, committed by PiRK). · Explain WhyJan 12 2024, 07:40

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABCf662f94f685d: Add function to validate difficulty changes.

Add function to validate difficulty changes
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 44053

src/consensus/activation.h

src/consensus/activation.cpp

src/pow/eda.h

src/pow/eda.cpp

src/pow/pow.h

src/pow/pow.cpp

src/pow/test/eda_tests.cpp

Add function to validate difficulty changesClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 44053

src/consensus/activation.h

src/consensus/activation.cpp

src/pow/eda.h

src/pow/eda.cpp

src/pow/pow.h

src/pow/pow.cpp

src/pow/test/eda_tests.cpp

Add function to validate difficulty changes
ClosedPublic
Actions

Revision Contents
Changeset List