feat: Add anti-sniping hook contract #20
Conversation
| liqParams.tickUpper = params.tickUpper; | ||
| liqParams.salt = params.salt; | ||
| positionKeyToLiquidityParams[positionKey] = liqParams; | ||
| if (positionCreationBlock[poolId][positionKey] != 0) revert PositionAlreadyExists(); |
There was a problem hiding this comment.
So ppl can not add liquidity into an existing position ?
There was a problem hiding this comment.
it can, but it will prevent not be sniping attack by MEV in swap and donate
There was a problem hiding this comment.
how @ChefCupcake ? doesn't this condition blocks user from adding liquidity to the same position?
There was a problem hiding this comment.
and what happens if user add liquidity via CLPositionManager?
There was a problem hiding this comment.
in raw project it is not allow add, only mint, but i will try to open the restrict, and test if it is ok
There was a problem hiding this comment.
yea same question here, I don't think users can still add liquidity to the same position
There was a problem hiding this comment.
oh, i think the raw project not allow add reason is the hook need to remember the timestamp when the liquidity was created, otherwise the timestamp will be last added, then it will conflict with the lock duration
There was a problem hiding this comment.
@ChefCupcake does this means user cannot increase liquidity for the same tokenId? (yes or no)?
if the answer is yes, please prep a doc on some suggestions on how we can fix this
There was a problem hiding this comment.
the answer is yes, if user increase liquidity, then the timestamp should the last action, the firstBlockFeesToken0 and firstBlockFeesToken1 will be lost, beside as the hook doesn't allow partial remove liquidity, so it will extend the lock duration
| returns (bytes4, BeforeSwapDelta, uint24) | ||
| { | ||
| PoolId poolId = key.toId(); | ||
| collectLastBlockInfo(poolId); |
There was a problem hiding this comment.
Can u add some test cases so that we can compare the gas snapshot ? I am a bit curious that how many gas overhead will this function bring
There was a problem hiding this comment.
yes, will add testcase later
| liqParams.tickUpper = params.tickUpper; | ||
| liqParams.salt = params.salt; | ||
| positionKeyToLiquidityParams[positionKey] = liqParams; | ||
| if (positionCreationBlock[poolId][positionKey] != 0) revert PositionAlreadyExists(); |
There was a problem hiding this comment.
how @ChefCupcake ? doesn't this condition blocks user from adding liquidity to the same position?
| liqParams.tickUpper = params.tickUpper; | ||
| liqParams.salt = params.salt; | ||
| positionKeyToLiquidityParams[positionKey] = liqParams; | ||
| if (positionCreationBlock[poolId][positionKey] != 0) revert PositionAlreadyExists(); |
There was a problem hiding this comment.
and what happens if user add liquidity via CLPositionManager?
| // If the pool is empty, the fees are not donated and are returned to the sender | ||
| hookDelta = BalanceDeltaLibrary.ZERO_DELTA; | ||
| } | ||
| return (this.afterRemoveLiquidity.selector, hookDelta); |
There was a problem hiding this comment.
any reason why we are not doing the Cleanup stored data for the position?
There was a problem hiding this comment.
same question here
maybe it's fine to not clean up if we are assuming that all users will only use position manager to add/removeLiq (position keys will always be different due to the fact that salt=tokenId)
but what if the user is not using position manager?
also as a plus, delete can save gas costs
There was a problem hiding this comment.
because only mint and remove all liquidity are allowed, so tokenId will not be repeat in this scenario for extra gas costs,but yes i can add delete in this func
test/pool-cl/CLAntiSniping.t.sol
Outdated
|
|
||
| uint24 constant FEE = 3000; | ||
| uint128 constant POSITION_LOCK_DURATION = 1000; | ||
| uint128 constant SAME_BLOCK_POSITIONS_LIMIT = 5; |
There was a problem hiding this comment.
is 5 enough in real world scenario? can you add test cases where different settings applied and let's see the gas snapshot comparison?
There was a problem hiding this comment.
thanks, I added a testcase to show that no matter the SAME_BLOCK_POSITIONS_LIMIT is, the gas won't cost more because it will delete each time
| mapping(PoolId => mapping(bytes32 => uint256)) public positionCreationBlock; | ||
|
|
||
| /// @notice The duration (in blocks) for which a position must remain locked before it can be removed. | ||
| uint128 public positionLockDuration; |
There was a problem hiding this comment.
can make this immutable
| liqParams.tickLower = params.tickLower; | ||
| liqParams.tickUpper = params.tickUpper; | ||
| liqParams.salt = params.salt; | ||
| positionKeyToLiquidityParams[positionKey] = liqParams; |
There was a problem hiding this comment.
liqParams is storage , do not need this line.
positionKeyToLiquidityParams[positionKey] = liqParams;
No description provided.