-
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
refactor: flip the sign for balanceDelta #49
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
182049 | ||
181729 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
185296 | ||
185616 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
331250 | ||
331570 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
138533 | ||
138277 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
148635 | ||
148315 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
296486 | ||
296230 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
131743 | ||
131487 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
120042 | ||
120359 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
977200 | ||
977517 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
339390 | ||
339707 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
342867 | ||
343184 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
145810 | ||
146127 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
309383 | ||
309700 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
362972 | ||
362978 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
178140 | ||
178146 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
248255 | ||
248182 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
170425 | ||
170745 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
114348 | ||
114589 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
124482 | ||
124243 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
141710 | ||
141453 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
175389 | ||
174968 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
25094185 | ||
161331 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
79576 | ||
79416 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
153925 | ||
153614 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
96383 | ||
96223 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
79579 | ||
79419 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
32291 | ||
29467 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
2237 | ||
2239 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
3022 | ||
3029 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
1994 | ||
1981 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
3022 | ||
3029 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
2227 | ||
2229 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
3175 | ||
3182 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
1984 | ||
1971 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
3175 | ||
3182 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
7136 | ||
7171 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
81625 | ||
81624 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -154,13 +154,14 @@ interface IBinPoolManager is IProtocolFees, IPoolManager, IExtsload { | |
function initialize(PoolKey memory key, uint24 activeId, bytes calldata hookData) external; | ||
|
||
/// @notice Add liquidity to a pool | ||
/// @return delta BalanceDelta, will be positive indicating how much total amt0 and amt1 liquidity added | ||
/// @return delta BalanceDelta, will be negative indicating how much total amt0 and amt1 liquidity added | ||
/// @return mintArray Liquidity added in which ids, how much amt0, amt1 and how much liquidity added | ||
function mint(PoolKey memory key, IBinPoolManager.MintParams calldata params, bytes calldata hookData) | ||
external | ||
returns (BalanceDelta delta, BinPool.MintArrays memory mintArray); | ||
|
||
/// @notice Remove liquidity from a pool | ||
/// @return delta BalanceDelta, will be positive indicating how much total amt0 and amt1 liquidity removed | ||
function burn(PoolKey memory key, IBinPoolManager.BurnParams memory params, bytes calldata hookData) | ||
external | ||
returns (BalanceDelta delta); | ||
|
@@ -171,7 +172,7 @@ interface IBinPoolManager is IProtocolFees, IPoolManager, IExtsload { | |
returns (BalanceDelta delta); | ||
|
||
/// @notice Donate the given currency amounts to the pool with the given pool key. | ||
/// @return delta Positive amt means the caller owes the vault, while negative amt means the vault owes the caller | ||
/// @return delta Negative amt means the caller owes the vault, while positive amt means the vault owes the caller | ||
/// @return binId The donated bin id, which is the current active bin id. if no-op happen, binId will be 0 | ||
function donate(PoolKey memory key, uint128 amount0, uint128 amount1, bytes calldata hookData) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's also the
|
||
external | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,10 +126,10 @@ library BinHooks { | |
|
||
if (hookDeltaSpecified != 0) { | ||
/// @dev default overflow check make sure the swap amount is always valid | ||
if (hookDeltaSpecified > 0) { | ||
amountToSwap += uint128(hookDeltaSpecified); | ||
} else { | ||
if (hookDeltaSpecified < 0) { | ||
amountToSwap -= uint128(-hookDeltaSpecified); | ||
} else { | ||
amountToSwap += uint128(hookDeltaSpecified); | ||
} | ||
} | ||
} | ||
|
@@ -163,9 +163,11 @@ library BinHooks { | |
hookDeltaUnspecified += beforeSwapDelta.getUnspecifiedDelta(); | ||
|
||
if (hookDeltaUnspecified != 0 || hookDeltaSpecified != 0) { | ||
// the hookDelta is the delta added into amountIn which is in the opposite direction of the balanceDelta | ||
// i.e. in a reasonable case, amountIn will basiaclly equal to "-BalanceDelta.amount0()/amount1()" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is now completely opposite when comparing with cl-pool because for cl-pool when That's why in the following call i.e. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. want to clarify: does the hook for both Bin and CL have the same behavour for their return value? eg. eg. 1/ what does negative There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this right?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for bin-pool, I think it should be opposite.
Let's elaborate case 3 for example: if from trader perspective: The amountIn delta returned from and we know from hook perspective:
This will count a negative amountIn delta on hook, so inside hook callback there must be a payment to the pool There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2/ what does negative int128 in afterSwap means? Also, should be opposite In fact can see they are combined in afterSwap before generating the final hookDelta: |
||
hookDelta = swapForY | ||
? toBalanceDelta(hookDeltaSpecified, hookDeltaUnspecified) | ||
: toBalanceDelta(hookDeltaUnspecified, hookDeltaSpecified); | ||
? toBalanceDelta(-hookDeltaSpecified, -hookDeltaUnspecified) | ||
: toBalanceDelta(-hookDeltaUnspecified, -hookDeltaSpecified); | ||
|
||
// the caller has to pay for (or receive) the hook's delta | ||
swapperDelta = delta - hookDelta; | ||
|
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, what does this comment mean. does it mean that it will revert eventually in the transaction if the target has non-zero delta?
eg. the comment might imply that if target has positive delta, this method will revert
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.
when calling
settleFor
,msg.sender
is trying to cover the payment for target right ?If the target has a positive delta, it means that the target doesn't owe anything to the pool and, in fact, the pool owes tokens to them..(after updating everything flip) In this case, pass in amount=0 (i.e. ettle all outstanding debt for target) is invalid.
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 when there's a comment like
/// It will revert if target has positive delta
-- people might think there's a revert in thissettleFor
method. But its basically theCurrencyNotSettled()
revert that happens at the end right? or is there actually a validation for thistarget has positive delta
specifically?If the comment is correct, would it mean we can write the same comment in
take/settle
etc..it will revert if caller has positive delta
as well?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.
“But its basically the CurrencyNotSettled() revert that happens at the end right? or is there actually a validation for this target has positive delta specifically?”
No, it will revert as in
SafeCast.toUint256
because we cant cast a negative number (given delta is positive, minus it results in negative) to uint256