refactor: update quoter #32
Conversation
src/pool-bin/lens/BinQuoter.sol
Outdated
| // int128 amountSpecifiedActual = (zeroForOne == (amountSpecified < 0)) ? deltas.amount0() : deltas.amount1(); | ||
| // if (amountSpecifiedActual != amountSpecified) { | ||
| // revert NotEnoughLiquidity(poolKey.toId()); | ||
| // } |
There was a problem hiding this comment.
Will remove comments once confirmed
There was a problem hiding this comment.
is this no longer applicable? https://github.com/pancakeswap/pancake-v4-periphery/blob/main/src/pool-bin/lens/BinQuoter.sol#L223-L226
or is it because BinPool will fire BinPool__OutOfLiquidity, so we no longer need to check here?
There was a problem hiding this comment.
- is this no longer applicable? https://github.com/pancakeswap/pancake-v4-periphery/blob/main/src/pool-bin/lens/BinQuoter.sol#L223-L226
Yes, i suspect in previous version can this even be triggered
- or is it because BinPool will fire BinPool__OutOfLiquidity, so we no longer need to check here?
Yes, can imagine if amountOut is less than expected then it must because of the illiquidity of the pool. Then it should revert with BinPool__OutOfLiquidity before the check
There was a problem hiding this comment.
Oh, it just hit me hook might have influence on this. For example, imagine we have stable swap implemented in hook contract. Let add it anyway, wdyt ? 😂
src/pool-cl/interfaces/ICLQuoter.sol
Outdated
| uint160 sqrtPriceLimitX96; | ||
| bytes hookData; | ||
| } | ||
| error NotEnoughLiquidity(PoolId poolId); |
There was a problem hiding this comment.
This is put here i.e. ICLQuoter instead of IQuoter because for BinPool this is gonna be error BinPool__OutOfLiquidity();
chefburger
force-pushed
the
refactor/quoter
branch
from
06c0e2a
to
8ca2365
Compare
| (uint256 reserveIn, uint256 reserveOut) = | ||
| V3SmartRouterHelper.getReserves(factoryV2, params.tokenIn, params.tokenOut); | ||
| amountOut = V3SmartRouterHelper.getAmountOut(params.amountIn, reserveIn, reserveOut); | ||
| gasEstimate = gasBefore - gasleft(); | ||
| } |
There was a problem hiding this comment.
Head up: this could be inaccurate since actual swap is not performed. Feel free to add comment if you guys have better solution
There was a problem hiding this comment.
lets create GH issue on this and take it from there.
- Fixing it in this PR will expand the scope
- its used by FE so we can eventually do a
MixedQuoterV2with proper gas estimate for v2 and stableswap
| try vault.lock(abi.encodeCall(this._quoteExactInputSingle, (params))) {} | ||
| catch (bytes memory reason) { | ||
| return _handleRevertSingle(reason); | ||
| gasEstimate = gasBefore - gasleft(); |
There was a problem hiding this comment.
The gas estimation is accurate than v2/stableSwap because of try-revert manner but still not 100% accurate because in real trade there would be some periphery logic & token settement
|
@ChefMist @ChefSnoopy Can u guys recheck and approve again.. Added back illiquidity check for binPool |
| /// @dev Check that the pool was not illiquid | ||
| /// even BinPool will emit BinPool__OutOfLiquidity when the pool is illiquid | ||
| /// We still need to apply the check in case hook contract manipulates the delta | ||
| int128 amountSpecifiedActual = (zeroForOne == (amountSpecified < 0)) ? deltas.amount0() : deltas.amount1(); |
There was a problem hiding this comment.
im curious, would this means basically if the pool has hook which charge a fee or take tokenIn for swap, quoter will fail?
There was a problem hiding this comment.
Not exactly, this will fail only when the hook does not respect the amountSpecified. For example, for exactInput the pool with hook require more/less input token or for exactOutput the pool with hook return more/less token.
There was a problem hiding this comment.
Revisited this: it seems amountSpecified can never be manipulated through hook i.e. delta.tokenOnSpecifiedSide() will always equal to amountSpecified for binPool. I start to thinking the check here is unnecessary again 😂
For CLPool, the reason why it's needed is because CLPool#swap (step2 in the screenshot) support partial fulfilled which could be BinPool__OutOfLiquidity for binPool.
There was a problem hiding this comment.
got it, thx. feel free to remove that or keep that (no harm i gues) since the previous implementation included that check
This PR mainly focuses on updating the interface for quoters of both pool types including mixQuoter. Key points:
BalanceDeltanow quoter only returns the output for exactIn* and input for exactOut*