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

refactor(math): revert to int256 #60

Merged
merged 5 commits into from
Nov 9, 2023
Merged

refactor(math): revert to int256 #60

merged 5 commits into from
Nov 9, 2023

Conversation

Rubilmax
Copy link
Contributor

@Rubilmax Rubilmax commented Nov 6, 2023

@Rubilmax Rubilmax mentioned this pull request Nov 6, 2023
Merged
@Rubilmax Rubilmax marked this pull request as ready for review November 6, 2023 10:28
@Rubilmax
Copy link
Contributor Author

Rubilmax commented Nov 6, 2023

I understand now that it's inconsistent with what I could have suggested: having constants typed the most restrictive (uint256) to make sure they are not negative. But in the end, this stricter typing doesn't allow to save checks so there's no real benefit of using it and it's even easier for us to type them int256 because we use them in int256 contexts

QGarchery
QGarchery requested changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@QGarchery QGarchery left a comment

Choose a reason for hiding this comment

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

Nice idea but a few things to sort out IMO

src/SpeedJumpIrm.sol Outdated Show resolved Hide resolved
src/SpeedJumpIrm.sol Outdated Show resolved Hide resolved
src/SpeedJumpIrm.sol Show resolved Hide resolved
src/SpeedJumpIrm.sol Outdated Show resolved Hide resolved
@Rubilmax Rubilmax requested a review from QGarchery November 6, 2023 15:19
@MerlinEgalite MerlinEgalite force-pushed the refactor/curve branch 2 times, most recently from a136b50 to 80e075e Compare November 8, 2023 16:00
@MathisGD
docs: comment adjustment speed non negative
3419458 
Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
@Rubilmax
Copy link
Contributor Author

Rubilmax commented Nov 9, 2023

I let you handle it @MathisGD

@MathisGD MathisGD merged commit 9614561 into refactor/curve Nov 9, 2023
3 checks passed
@MathisGD MathisGD deleted the refactor/w-mul-down branch November 9, 2023 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jean-Grimal Jean-Grimal Jean-Grimal approved these changes

@MathisGD MathisGD MathisGD approved these changes

@QGarchery QGarchery QGarchery approved these changes

@MerlinEgalite MerlinEgalite Awaiting requested review from MerlinEgalite

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Rubilmax @QGarchery @MathisGD @Jean-Grimal