fix: [hexen-r9] add overflow check for bin liquidity #211
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chefburger
force-pushed
the
audit/hexens-2nd-round-part3
branch
from
a9c86c5 to
8c00656
Compare
ChefSnoopy
reviewed
Nov 11, 2024
|
ChefSnoopy
previously approved these changes
Nov 11, 2024
ChefMist
reviewed
Nov 12, 2024
| @@ -168,6 +169,14 @@ library BinPool { | |||
| amountsInWithFees = amountsInWithFees.sub(pFee); | |||
| } | |||
|
|
|||
| if ( | |||
There was a problem hiding this comment.
can i trouble you to try to add some test cases for swap/mint case? would be good exercise to further our knowledge on bin related especially with #213 issue
ChefMist
reviewed
Nov 12, 2024
src/pool-bin/libraries/BinPool.sol
Outdated
| @@ -168,6 +169,14 @@ library BinPool { | |||
| amountsInWithFees = amountsInWithFees.sub(pFee); | |||
| } | |||
|
|
|||
| if ( | |||
| (binReserves.add(amountsInWithFees).sub(amountsOutOfBin)).getLiquidity( | |||
There was a problem hiding this comment.
1/ i wonder if it will be cheaper in gas if you cast a variable to store binReserves.add(amountsInWithFees).sub(amountsOutOfBin) like how u did elsewhere
2/ or if we apply the changes within BinHelper.getAmountsOut and Binhelper.getAmountsIn instead (as the price is already computed, we don't need to compute again like what we did here swapState.activeId.getPriceFromId(params.binStep)
There was a problem hiding this comment.
oh, that's actually what i did before, what it failed the forge coverage 😂
There was a problem hiding this comment.
But let me try option 2
There was a problem hiding this comment.
It did help on gas, thx 👍
ChefMist
previously approved these changes
Nov 12, 2024
ChefSnoopy
previously approved these changes
Nov 12, 2024
ChefMist
previously approved these changes
Nov 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.