add posm view lib #237
add posm view lib #237
Conversation
| import {PositionConfig} from "./PositionConfig.sol"; | ||
|
|
||
| /// @notice A library for reading posm liquidity balances from core, wraps core's StateLibrary | ||
| library PosmLiquidityLibrary { |
There was a problem hiding this comment.
I do wonder if we should be keeping the name posm out of the final code ... i feel like no one else knows what it means?
There was a problem hiding this comment.
oh also i dont think it needs to be positionmanager specific at all? really any contract that adds liquidity could use this library right?
i feel like it should be called LiquidityLibrary and bytes32 salt should be passed in instead of tokenId?
And then its just a generic library for liquidity providing contracts to use for their liquidity look ups?
| @@ -75,4 +82,13 @@ contract PosmTestSetup is Test, Deployers, DeployPermit2, LiquidityOperations { | |||
| delta = delta + hook.deltas(i); | |||
| } | |||
| } | |||
|
|
|||
| function _getPositionLiquidity(uint256 tokenId, PositionConfig memory _config) | |||
There was a problem hiding this comment.
this PR introduces a library that does exactly this, feels wrong that our tests that integrate then cant access it and youve had to copy the same code into the tests?
maybe its worth exposing a public function on position manager called getPositionLiquidity(tokenId, config) that returns the liquidity? and then tests/integrations can use that?
thoughts?
There was a problem hiding this comment.
we could also have the library accept the posm address as a param? thats how StateLibrary works
There was a problem hiding this comment.
mmm im not sure i like that given its to be used internally? like we dont use the StateLibrary internally in PM? but i dont feel strongly
|
|
||
| import {PositionConfig} from "./PositionConfig.sol"; | ||
|
|
||
| /// @notice A library for reading posm liquidity balances from core, wraps core's StateLibrary |
There was a problem hiding this comment.
| /// @notice A library for reading posm liquidity balances from core, wraps core's StateLibrary | |
| /// @notice A library for reading liquidity balances owned by the current contract from the PoolManager |
|
replaced by #270 |
trying to clean up base PositionManager.sol contract, moving internal func to a lib & cleaning up some tests