Pack BucketState struct into a single slot #645
Merged
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
* Add Position struct to keep lps and deposit time of memorialized position. remove positionLPs and positionDepositTime mappings, keep only one positions mapping to Position struct Emit events at the end of functions Small gas improvements by reusing pool address / reading only once from storage Rename RemoveLiquidityFailed error to RemovePositionFailed * Approve and reset partial LPs Consistent naming LPs, replaced all LP tokens occurences * Change transferLps to allow partial transfer of deposit, transfer the min(approvedAmount, availableAmount), account bucket bankruptcy on new owner Add tests * changes after review - create IPool only once
…at memorialized indexes
mattcushman
approved these changes
Mar 4, 2023
since we're packing this with bucket rate we could increase the uint128 for LPs and reduce the one for rate? (I suspect uint128 for rate is huge but we'd like to be safer for lps?) |
grandizzy
approved these changes
Mar 4, 2023
EdNoepel
approved these changes
Mar 4, 2023
There was a problem hiding this comment.
The description's comment about token supply doesn't seem relevant, because neither field stores a token balance. If exchange rates get too low, LPs will go to infinity. That said, we have bigger problems if the exchange rate goes to 0. Since this is isolated to the rewards contract, I'm fine with it.
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.
Description of change
High level
Gas usage
Pre Change
Post Change