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

Add extra check for decimal math #15419

Merged
merged 9 commits into from
Nov 28, 2024
Merged

Add extra check for decimal math #15419

merged 9 commits into from
Nov 28, 2024

Conversation

RensR
Copy link
Contributor

@RensR RensR commented Nov 26, 2024

Add extra checks for decimal math in pools
Clean up USDC tests

@RensR RensR requested review from a team as code owners November 26, 2024 14:32
Copy link
Contributor

github-actions bot commented Nov 26, 2024

Static analysis results are available

Hey @RensR, you can view Slither reports in the job summary here or download them as artifact here.
Please check them before merging and make sure you have addressed all issues.

Copy link
Contributor

github-actions bot commented Nov 26, 2024

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@RensR RensR requested review from a team as code owners November 27, 2024 11:29
/// @dev This function assumes the inputs don't overflow and does no checks to avoid this. For any normal inputs, this
/// should not be a problem. The only way to overflow is when the given arguments cannot be represented in the uint256
/// type, which means the inputs are invalid.
/// @dev This function protects against overflows. If there is a transaction that hits the overflow check, it is
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// @dev This function protects against overflows. If there is a transaction that hits the overflow check, it is
/// @dev This function protects against overflows. If there is a transaction that hits the overflow check, it is

RayXpub
RayXpub previously approved these changes Nov 27, 2024
// would overflow.
if (
i_tokenDecimals - remoteDecimals > 77
|| remoteAmount > type(uint256).max / (10 ** (i_tokenDecimals - remoteDecimals))
Copy link
Member

Choose a reason for hiding this comment

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

We do these calculations multiple times
Can we compute then once and then use it further

Example

decimalsDifference = remoteDecimals - i_tokenDecimals;

factor = 10 ** decimalsDifference;

mateusz-sekara
mateusz-sekara previously approved these changes Nov 28, 2024
@RensR RensR enabled auto-merge November 28, 2024 10:54
makramkd
makramkd previously approved these changes Nov 28, 2024
@RensR RensR disabled auto-merge November 28, 2024 11:24
@RensR RensR dismissed stale reviews from makramkd, mateusz-sekara, and RayXpub via 5750d81 November 28, 2024 11:27
0xsuryansh
0xsuryansh previously approved these changes Nov 28, 2024
Copy link
Member

@0xsuryansh 0xsuryansh left a comment

Choose a reason for hiding this comment

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

LGTM!

@RensR RensR enabled auto-merge November 28, 2024 11:41
@RensR RensR added this pull request to the merge queue Nov 28, 2024
Merged via the queue into develop with commit 22bcd4b Nov 28, 2024
194 of 195 checks passed
@RensR RensR deleted the improve-pool-tests branch November 28, 2024 13:04
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.

5 participants