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

use checked arithmetic #31259

Merged
merged 8 commits into from
Apr 21, 2023
Merged

use checked arithmetic #31259

merged 8 commits into from
Apr 21, 2023

Conversation

tao-stones
Copy link
Contributor

@tao-stones tao-stones commented Apr 18, 2023 •
edited
Loading

Problem

Should use checked arithmetic in most cases.

Summary of Changes

  • Add tests
  • Use checked arithmetic
  • Add feature gate

Feature Gate Issue: #31273

@tao-stones tao-stones requested review from t-nelson and mvines April 18, 2023 22:21
@tao-stones tao-stones force-pushed the integer_overflow_in_fee_calc branch 2 times, most recently from 56534a0 to 0e13d0e Compare April 18, 2023 23:39
tao-stones
tao-stones commented Apr 18, 2023
View reviewed changes
runtime/src/accounts.rs Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 19, 2023 •
edited
Loading

Codecov Report

Merging #31259 (5f4caf0) into master (7a393e4) will increase coverage by 0.0%.
The diff coverage is 98.7%.

@@           Coverage Diff           @@
##           master   #31259   +/-   ##
=======================================
  Coverage    81.5%    81.5%           
=======================================
  Files         733      733           
  Lines      207009   207087   +78     
=======================================
+ Hits       168731   168797   +66     
- Misses      38278    38290   +12

t-nelson
t-nelson reviewed Apr 19, 2023
View reviewed changes
runtime/src/accounts.rs Outdated Show resolved Hide resolved
@tao-stones tao-stones marked this pull request as ready for review April 19, 2023 04:17
@tao-stones tao-stones requested a review from t-nelson April 19, 2023 14:28
@tao-stones tao-stones added the feature-gate Pull Request adds or modifies a runtime feature gate label Apr 19, 2023
@tao-stones tao-stones force-pushed the integer_overflow_in_fee_calc branch from 5adc168 to 7e62a48 Compare April 20, 2023 00:39
@tao-stones tao-stones changed the title use saturating arithmetic use checked arithmetic Apr 20, 2023
t-nelson
t-nelson reviewed Apr 20, 2023
View reviewed changes
Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

i think i'm down to just nits

runtime/src/accounts.rs Outdated Show resolved Hide resolved
runtime/src/accounts.rs Outdated Show resolved Hide resolved
@tao-stones tao-stones requested a review from t-nelson April 20, 2023 12:21
mvines
mvines reviewed Apr 20, 2023
View reviewed changes
runtime/src/accounts.rs Outdated Show resolved Hide resolved
t-nelson
t-nelson approved these changes Apr 20, 2023
View reviewed changes
Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

:shipit:

@tao-stones tao-stones merged commit b4331ae into solana-labs:master Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvines mvines mvines left review comments

@t-nelson t-nelson t-nelson approved these changes

Assignees
No one assigned
Labels
feature-gate Pull Request adds or modifies a runtime feature gate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tao-stones @t-nelson @mvines