v4-core icon indicating copy to clipboard operation
v4-core copied to clipboard

Better information management when tracking positions

Open guil-lambert opened this issue 2 years ago • 1 comments

The keccak256(abi.encodePacked(owner, tickLower, tickUpper)) hash stores 26 bytes of information inside a bytes32 space in Position.sol. There are 6 extra bytes of unused space.

What about removing the hash and change the encoding so that the owner+tickLower+tickUpper+tickSpacing are all part of a uint256 positionId? The positionId could be one of the parameter in ModifyPositionParams in Pool.sol and decoded when needed:

uint256 positionId = (24bits of nothing)(24bits of tickLower)(24bits of tickUpper)(24bits of tickSpacing)(160bits of owner address)

That way, position = self[positionId]

- address owner = address(uint160(positionId));
- int24 tickLower = int24((positionId >> 160) % 24);
- int24 tickUpper = int24((positionId >> 184) % 24);
- int24 tickSpacing = int24((positionId >> 208) % 24); 

and

   struct ModifyPositionParams {
        // the id of the position
        uint256 positionId;
        // any change in liquidity
        int128 liquidityDelta;
    }

This would also make writing the roll position function described in https://github.com/Uniswap/core-next/issues/129 a bit simpler: the rollPosition function only needs two positionId parameters (no need to supply a liquidityDelta):

rollPosition(self, oldPositionId, newPositionId);


Further improvements could be made for cross-pool tracking if positionId also encode for the poolId: uint256 positionId = (24bits of tickLower)(24bits of tickUpper)(16bits of tickSpacing)(96bits of owner address)(96bits of pool address)

guil-lambert avatar Jul 03 '22 02:07 guil-lambert

Ah this is a cool idea -- why add tickSpacing on the position-level data though as it's a pool-level field (already stored in PoolKey)

marktoda avatar Jul 07 '22 00:07 marktoda

Hi @guil-lambert! Since we now have a bytes32 salt that's part of ModifyLiquidityParams, we decided not to go with this approach, so I'll be closing this issue. But very cool idea!

dianakocsis avatar May 24 '24 20:05 dianakocsis