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

feat: prevent possible division by zero in difficulty calculation #5828

Conversation

hansieodendaal
Copy link
Contributor

Description

Prevented possible division by zero panic in fn u256_scalar_to_difficulty(scalar: U256) -> Result<Difficulty, DifficultyError>

Motivation and Context

Code should not be able to panic.

How Has This Been Tested?

Unit test added

What process can a PR reviewer use to test or verify this change?

Code walkthrough

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

Prevented division by zero panic
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Test Results (CI)

1 218 tests   1 218 ✔️  10m 4s ⏱️
     39 suites         0 💤
       1 files           0

Results for commit c846977.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Oct 2, 2023
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Test Results (Integration tests)

29 tests  +29   29 ✔️ +29   13m 23s ⏱️ + 13m 23s
11 suites +11     0 💤 ±  0 
  2 files   +  2     0 ±  0 

Results for commit c846977. ± Comparison against base commit 754fb16.

Copy link
Collaborator

@AaronFeickert AaronFeickert 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. Manually confirmed that the new test will catch any regression.

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Oct 2, 2023
@AaronFeickert
Copy link
Collaborator

AaronFeickert commented Oct 2, 2023

ACK

@ghpbot-tari-project ghpbot-tari-project removed the P-acks_required Process - Requires more ACKs or utACKs label Oct 2, 2023
@SWvheerden SWvheerden merged commit f85a878 into tari-project:development Oct 3, 2023
14 checks passed
@hansieodendaal hansieodendaal deleted the ho_prevent_division_by_zero_panic branch October 4, 2023 15:39
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

4 participants