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.

Sign up for GitHub

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

Jump to bottom

Testnet min-difficulty rules are incorrectly specified #416

Closed
str4d opened this issue Nov 11, 2020 · 0 comments · Fixed by #417
Closed

Testnet min-difficulty rules are incorrectly specified #416

str4d opened this issue Nov 11, 2020 · 0 comments · Fixed by #417
Milestone
Core Sprint 2020-45

Comments

@str4d
Copy link
Collaborator

str4d commented Nov 11, 2020 •
edited
Loading

ZIP 205 (which documented the testnet min-difficulty rule after it was implemented in zcash/zcash#3559) says:

This algorithm changed on testnet, starting from block 299188, to allow "minimum-difficulty" blocks. If the block time of a block from this height onward is greater than 15 minutes after that of the preceding block, then the block is a minimum-difficulty block, and its target threshold is set to the value of PoWLimit for testnet (see 2 section 5.3). However, its nBits field is still computed according to the original difficulty adjustment algorithm.

This is incorrect. The nBits field of a min-difficulty block is set to the minimum difficulty:
https://github.com/zcash/zcash/blob/091f5d78162e76eabb3a59840e9e32febb3f0fae/src/miner.cpp#L113-L116
https://github.com/zcash/zcash/blob/091f5d78162e76eabb3a59840e9e32febb3f0fae/src/pow.cpp#L32-L36

This does not affect how the minimum-difficulty block is treated for subsequent difficulty adjustments. In particular, only the nBits field computed by the original algorithm is used for the purpose of computing the MeanTarget values from which subsequent difficulty changes are calculated.

This is also incorrect. The difficulty adjustment algorithm was not altered to be aware of min-difficulty blocks, and since their nBits fields are set to the minimum difficulty per above, their min-difficulty affects the adjustment:
https://github.com/zcash/zcash/blob/091f5d78162e76eabb3a59840e9e32febb3f0fae/src/pow.cpp#L43-L48

ZIP 208 (which documented the effect of the block target interval change on this rule) has similar issues.
This was referenced Nov 11, 2020
Closed
Merged
daira added a commit that referenced this issue Nov 11, 2020
@daira
Change the specification of minimum-difficulty blocks to reflect the …
d1f1b27 
…zcashd implementation

(which alters nBits rather than just the target threshold). fixes #416

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
@daira daira mentioned this issue Nov 11, 2020
Merged
@r3ld3v r3ld3v added this to the Core Sprint 2020-45 milestone Nov 11, 2020
daira added a commit that referenced this issue Nov 11, 2020
@daira
Change the specification of minimum-difficulty blocks to reflect the …
8c28907 
…zcashd implementation

(which alters nBits rather than just the target threshold). fixes #416

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
teor2345 added a commit to teor2345/zebra that referenced this issue Nov 11, 2020
@teor2345
Update the testnet minimum-difficulty tests
14bf4ac 
ZIPs 205 and 208 describe the testnet minimum-difficulty rules
incorrectly: testnet minimum difficulty changes the difficulty
adjustment algorithm, not the difficulty filter.

See zcash/zips#416 for details.
teor2345 added a commit to teor2345/zebra that referenced this issue Nov 12, 2020
@teor2345
Difficulty RFC: Revised based on spec fixes
2476c0d 
Update the design based on the spec bugs in ZcashFoundation#1276, ZcashFoundation#1277, and
zcash/zips#416.

These changes make the difficulty filter into a context-free check,
so we remove it from this contextual validation RFC.
teor2345 added a commit to ZcashFoundation/zebra that referenced this issue Nov 26, 2020
@teor2345
RFC: Contextual Difficulty (#1246)
fc7d37c 
* Difficulty Contextual RFC: Introduction
Add a header, summary, and motivation

* Difficulty RFC: Add draft definitions
And update the state RFC definitions to match.

* Difficulty RFC: Add relevant chain
* Difficulty RFC: draft guide-level explanation
Outline the core calculations and checks.

* Difficulty RFC: Revised based on spec fixes
Update the design based on the spec bugs in #1276, #1277, and
zcash/zips#416.

These changes make the difficulty filter into a context-free check,
so we remove it from this contextual validation RFC.

* Difficulty RFC: Explain how Zebra's calculations can match the spec
* Difficulty RFC: write most of the reference section
Includes most of the implementation, modules for each function, and
draft notes for some of the remaining parts of the RFC.

* Difficulty RFC: Add an AdjustedDifficulty struct
* Difficulty RFC: Summarise module structure in the one place
* Difficulty RFC: Create implementation notes subsections
* Difficulty RFC: add consensus critical order of operations
* Difficulty RFC: Use the ValidateContextError type
* Difficulty RFC: make the median_time arg mut owned

We have to clone the data to pass a fixed-length array to a function,
so we might as well sort that array to find the median, and avoid a
copy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Core Sprint 2020-45
2 participants
@str4d @r3ld3v