-
Notifications
You must be signed in to change notification settings - Fork 220
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
fix: possible overflow in difficulty calculation (fixes #3923) companion #4097
fix: possible overflow in difficulty calculation (fixes #3923) companion #4097
Conversation
Love the enthusiasm, but let #3923 get merged, and then rebase. :) |
Thanks for the encouragement @CjS77, I probably rushed a bit here. In the meanwhile, I will keep looking at the codebase :) |
#4090 is merged, so rebase / reset the changes to remove the conflicts and we should be good |
Just need to resolve the conflicts here |
The PR should be ready to be merged ! Let me know if you have any additional comments/suggestions :) |
@jorgeantonio21 Unfortunately, to merge this we need all commits to be signed. You may also need to run this command to resign all commits: (might need to remove the command from the last merge commit |
b37193b
to
905f8ee
Compare
@sdbondi I followed your instructions, if I haven't screwed up, commits should now be signed and the PR be ready to merge. Thanks for the advice ! |
@jorgeantonio21 Looks like it didn't take :/ |
@jorgeantonio21 I think if you make the commit over again it should work. As in:
|
905f8ee
to
4314bee
Compare
c1f9b7d
to
ed40e6f
Compare
Think there are some commits getting reverted here...
ed40e6f
to
4ce678b
Compare
Description
Quick fix to overflow problem in difficulty. This problem had already been settled for
big_endian_difficulty
.Motivation and Context
In #4090, @CjS77 point it out that the
little_endian_difficulty
method had the same problem. This PR is an attempt to tackle it.How Has This Been Tested?
Adapted tests from previous PR (#4090)