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

blockchain: Move diff retarget log to connect. #2765

Merged

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Oct 14, 2021

This is rebased on #2761.

The method that calculates the next required difficulty currently logs difficulty retargets which means it has side effects when it really should free from them given its purpose.

This can be evidenced by the fact the logging is actually slightly incorrect in a couple of ways due to those side effects. Namely, it can potentially log when checking block templates when it shouldn't and it is also currently logging when the headers are connected as opposed to when the blocks themselves are actually connected which was the original intention prior to the new headers-first logic.

Thus, this moves the debug logging for difficulty retargets from the required difficulty calculation function to the block connection function and also takes the opportunity to impose a new condition to only log when the difficulty actually changes as opposed to the current behavior of always logging on a retarget interval regardless.

@davecgh davecgh added this to the 1.7.0 milestone Oct 14, 2021
@davecgh davecgh force-pushed the blockchain_diff_retarget_connect_block branch from 65adf8d to eb0b6a9 Compare October 14, 2021 17:42
Copy link
Member

@rstaudt2 rstaudt2 left a comment

Choose a reason for hiding this comment

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

Looks good, it definitely makes more sense here than where it was previously.

The method that calculates the next required difficulty currently logs
difficulty retargets which means it has side effects when it really
should free from them given its purpose.

This can be evidenced by the fact the logging is actually slightly
incorrect in a couple of ways due to those side effects.  Namely, it can
potentially log when checking block templates when it shouldn't and it
is also currently logging when the headers are connected as opposed to
when the blocks themselves are actually connected which was the original
intention prior to the new headers-first logic.

Thus, this moves the debug logging for difficulty retargets from the
required difficulty calculation function to the block connection
function and also takes the opportunity to impose a new condition to
only log when the difficulty actually changes as opposed to the current
behavior of always logging on a retarget interval regardless.
@davecgh davecgh force-pushed the blockchain_diff_retarget_connect_block branch from eb0b6a9 to c98c610 Compare October 20, 2021 01:07
@davecgh davecgh merged commit c98c610 into decred:master Oct 20, 2021
@davecgh davecgh deleted the blockchain_diff_retarget_connect_block branch October 20, 2021 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants