Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust the difficulty in the first difficultyAdjustmentWindowSize blocks #1592

Merged
merged 7 commits into from
Mar 10, 2021

Conversation

elichai
Copy link
Member

@elichai elichai commented Mar 10, 2021

Closes #1546

The solution is this: #1546 (comment)

  1. Exclude genesis from the window.
  2. Don't pad the window with genesis.
  3. Divide the target by the current window size (which will be smaller than 2640 for the first 2640 blocks).
  4. We might need to clamp the timestamp difference(max-min) to make it no lower than 1 second(targetTime.MilliSeconds()), depending on how PastMedianTime works for those blocks.

@elichai elichai added this to the 0.10.0 milestone Mar 10, 2021
@elichai elichai added this to In progress in Mainnet via automation Mar 10, 2021
@@ -46,6 +50,13 @@ func (pmtm *pastMedianTimeManager) PastMedianTime(blockHash *externalapi.DomainH
if err != nil {
return 0, err
}
if len(window) == 0 {
header, err := pmtm.blockHeaderStore.BlockHeader(pmtm.databaseContext, pmtm.genesisHash)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very weird to check the window size and then give the timestamp of a hard coded hash. I would expect to choose between:
a. if window size is 0 -> return blockHash's timetstamp
b. if blockHash == genesis -> return genesis' timetstamp

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it's not only the genesis, it's also any block pointing at the genesis

if (shouldPass && err != nil) || (!shouldPass && !errors.Is(err, ruleerrors.ErrUnfinalizedTx)) {
t.Fatalf("Unexpected error: %+v", err)
t.Fatalf("ShouldPass: %t Unexpected error: %+v", shouldPass, err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why capital S?

@codecov
Copy link

codecov bot commented Mar 10, 2021

Codecov Report

Merging #1592 (b705f64) into v0.10.0-dev (cd27f28) will increase coverage by 0.16%.
The diff coverage is 81.48%.

Impacted file tree graph

@@               Coverage Diff               @@
##           v0.10.0-dev    #1592      +/-   ##
===============================================
+ Coverage        59.89%   60.06%   +0.16%     
===============================================
  Files              514      514              
  Lines            20255    20251       -4     
===============================================
+ Hits             12132    12163      +31     
+ Misses            6202     6163      -39     
- Partials          1921     1925       +4     
Impacted Files Coverage Δ
...nsensus/processes/difficultymanager/blockwindow.go 82.35% <ø> (+12.35%) ⬆️
domain/consensus/utils/sorters/timesorter.go 75.00% <0.00%> (ø)
.../processes/blockvalidator/block_body_in_context.go 74.02% <33.33%> (-0.66%) ⬇️
...ses/pastmediantimemanager/pastmediantimemanager.go 69.23% <75.00%> (-3.50%) ⬇️
domain/consensus/factory.go 90.93% <100.00%> (+0.05%) ⬆️
...ocesses/dagtraversalmanager/dagtraversalmanager.go 70.76% <100.00%> (+0.92%) ⬆️
.../consensus/processes/dagtraversalmanager/window.go 64.28% <100.00%> (-5.42%) ⬇️
...s/processes/difficultymanager/difficultymanager.go 88.88% <100.00%> (+9.40%) ⬆️
util/math/min.go 100.00% <100.00%> (ø)
...k/netadapter/server/grpcserver/connection_loops.go 53.84% <0.00%> (-3.85%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd27f28...b705f64. Read the comment docs.

@elichai elichai merged commit 1486a63 into v0.10.0-dev Mar 10, 2021
Mainnet automation moved this from In progress to Done Mar 10, 2021
@elichai elichai deleted the requireddifficulty branch March 10, 2021 14:11
@elichai elichai added this to Done in Mainnet May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants