-
Notifications
You must be signed in to change notification settings - Fork 37
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
[PART-2] support same owner multiple positions #46
[PART-2] support same owner multiple positions #46
Conversation
@@ -396,10 +398,21 @@ library BinPool { | |||
amountsLeft = params.amountIn; | |||
|
|||
for (uint256 i; i < params.liquidityConfigs.length;) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checked and it did help to save gas.. thx for reminding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look good, will approve once we have salt !=0
tests
src/pool-cl/libraries/CLPosition.sol
Outdated
mstore(0x06, tickUpper) | ||
mstore(0x03, tickLower) | ||
mstore(0x00, owner) | ||
key := keccak256(0x0c, 0x1a) | ||
key := keccak256(0x0c, 0x3a) | ||
// only 0x00 ~ 0x40 is scratch space |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: 0x00 - 0x3f
is scratch space
mstore(0x03, binId) | ||
mstore(0x00, owner) | ||
key := keccak256(0x0c, 0x17) | ||
key := keccak256(0x0c, 0x37) | ||
// only 0x00 ~ 0x40 is scratch space |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: 0x00 - 0x3f
is scratch space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious why is to 3f
not 40
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because it starts from 0x00 😂 (2 slots i.e. 64 bytes in total)
/// @param amounts List of amount added to each bin | ||
/// @param compositionFee fee occurred | ||
/// @param pFee Protocol fee from the swap: token0 and token1 amount | ||
event Mint( | ||
PoolId indexed id, | ||
address indexed sender, | ||
uint256[] ids, | ||
bytes32 salt, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just wondering if there will be cases requiring bytes32[] salts
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that might make Mint quite complicated 😂... ppl could actually separate it into multiple calls if they want to do so
/// @param amounts List of amount removed from each bin | ||
event Burn(PoolId indexed id, address indexed sender, uint256[] ids, bytes32[] amounts); | ||
event Burn(PoolId indexed id, address indexed sender, uint256[] ids, bytes32 salt, bytes32[] amounts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just wondering if there will be cases requiring bytes32[] salts
?
…es (#47) * feat: finish updating for cl-pool * feat: finish updating for bin-pool * chore: correct comments removed unnecessary isValid function
* feat: enable hooks to return deltas to influence pool behavior * test: fix & add test cases hook returns delta * [PART-2] support same owner multiple positions (#46) * feat: add salt to distinguish positions for same owner in cl-pool * feat: add salt to distinguish positions for same owner in bin-pool * optimiazaiton: small tweaks per comments * test: added salt != 0 cases * [PART-3] allow `beforeSwap` to return temp lpFee & delta for both sides (#47) * feat: finish updating for cl-pool * feat: finish updating for bin-pool * chore: correct comments removed unnecessary isValid function
Per internal discussion