<WIP> feat: off-chain farming #41
Conversation
| CLPosition.Info storage position = self.positions.get(params.owner, params.tickLower, params.tickUpper); | ||
| if (position.liquidity == 0 && position.feeGrowthInside0LastX128 == 0 && position.feeGrowthInside1LastX128 == 0) | ||
| { | ||
| self.positionKeys.push(PositionKey(params.owner, params.tickLower, params.tickUpper)); |
There was a problem hiding this comment.
should we consider doing this off-chain ?
There was a problem hiding this comment.
on the same page, why are we doing this on-chain? the PR description doesn't explain this
There was a problem hiding this comment.
got it, let me explain it in today's v4 sync
| feeGrowthInside1LastX128: info.feeGrowthInside1LastX128 | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
( correct if i am wrong ) So i guess what u want here is basically an array to keep track of all non-empty position, so that can do:
for i = 0 to length - 1:
gather and calc farming info for each position
But why not just listen the event ModifyLiquidity in a off-chain manner, and commit update to db instead
event ModifyLiquidity(
PoolId indexed id,
address indexed sender,
int24 tickLower,
int24 tickUpper,
int256 liquidityDelta,
int128 feesOwed0,
int128 feesOwed1
);
| params.tickLower, | ||
| params.tickUpper, | ||
| params.liquidityDelta, | ||
| feeDelta.amount0(), |
There was a problem hiding this comment.
do we need this? if the intend is to know the feeDelta, either
1/ when simulate modifyLiqudiity, it returns feeDelta
2/ or calculate this value off-chain, via:
a. get the liquidity, feeGrowthInside0LastX128 and feeGrowthInside1LastX128 for this position via poolManager.getPosition(poolId, owner, tickLower, tickUpper)
b. get the global fee growth feeGrowthGlobal1X128/feeGrowthGlobal1X128 via poolmanager.pools(poolId)
calculate feeDelta by doing the math with those value. ref: https://github.com/pancakeswap/pancake-v4-core/blob/main/src/pool-cl/libraries/CLPosition.sol#L72-L77
There was a problem hiding this comment.
for 1/
yes actually the steps 1&2 in the my notion docs is to simulate modifyLiquidity and get feeDelta results
however for step 3, since unlike NonfungiblePositionManager, there are no tokensOwed states for each position in v4-core, we need to somehow record the feesOwed when there are modifyLiquidity(collect) invoked within the epoch
for 2/
that's a more complicated way just as I proposed in my previous version
besides, this method will still require extra event params like feeGrowthInside
also the contract needs to do the computation no matter there is off-chain farming or not, so I don't think emitting the feeDelta results alongside will incur like a unacceptable cost
| @@ -70,6 +76,7 @@ library CLPool { | |||
| mapping(int24 => Tick.Info) ticks; | |||
| mapping(int16 => uint256) tickBitmap; | |||
| mapping(bytes32 => CLPosition.Info) positions; | |||
| PositionKey[] positionKeys; | |||
There was a problem hiding this comment.
Btw, have u guys ever heard of an attack vector that is based on unlimited sized array. Given that:
- Pushing element into array doesn't cost much in our case (consider addLiquidity 1 to a new range or through new addr)
- Array elements are laid linearly in evm storage which means the slot position of a given element will be just a few slots away from current one (unlike for mapping it's hash basically so slot allocation is in fact random)
If a new element's slot position hits exactly a existing contract variable for example address _owner, hacker could manipulate that value through constantly push array until they get that position
I cant remember where did i see it and not sure if this is feasible. Just thought it is worth to point out as reference
this PR is mainly to solve the missing fee problem when users are not managing positions through PCS official NonfungiblePositionManager
see https://www.notion.so/pancakeswap/in-house-v4-off-chain-farming-53dca6e8877f4ca9ad20cdd91d2382ce?pvs=4#5302191e85a34787880a6c245e610c81
incur some gas cost