Custom swap accounting with NoOps #299
Conversation
There was a problem hiding this comment.
i wonder if it's worth allowing pools to use a returned hooksDelta even in non-NO_OP cases...one reason is i think it's awkward if initialize can NO_OP (for example, sqrtPriceX96 remains 0 in this case, which i think means the pool is borked unless it exclusively uses no-ops going forward, which is fine for some pools but maybe not for others, and feels like a bit of a footgun). so, i think we should consider disallowing no-op on initialize, but the delta logic could still be useful for hooks to levy an initial fee or incentivize the creator or something?
i also don't really think we should allow calling after* hooks when NO_OP is used
finally i think a version of _accountNoOp that applies fees would be valuable to see. for the sake of clarity it might make sense to write that as a stacked PR on top of this one
|
NoOping on initialize will prevent the pool with that hook from being included in our pools mapping, so I think it should be removed. |
contracts/PoolManager.sol
Outdated
| @@ -108,6 +108,25 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC1155, IERC1155Rec | |||
| return pools[id].positions.get(owner, tickLower, tickUpper); | |||
| } | |||
|
|
|||
| function _accountNoOp(PoolKey memory key, BalanceDelta hookDelta) internal { | |||
| // NOTE: we are in the lock state of the user, not the hook, unless the hook is the user | |||
| _accountDelta(key.currency0, hookDelta.amount0()); | |||
There was a problem hiding this comment.
nit: why not just use _accountPoolBalanceDelta for the whole hookDelta?
There was a problem hiding this comment.
updated in most current commit, but it increases gas for modifyPosition by 56 and swap by 17
|
Closing in favor of this NoOp impl #324 |
Inspired by NoOp design from #291. @thogard785's commit is merged with this PR