Skip to content

Commit

Permalink
remove lock on initialize (#499)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dianakocsis
dianakocsis authored Mar 8, 2024
1 parent d8e57f8 commit 0e8e6c9
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 95 deletions.
2 changes: 1 addition & 1 deletion .forge-snapshots/initialize.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
72463
51725
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
23381
23345
1 change: 0 additions & 1 deletion src/PoolManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, ERC6909Claims {
function initialize(PoolKey memory key, uint160 sqrtPriceX96, bytes calldata hookData)
external
override
isLocked
returns (int24 tick)
{
if (key.fee.isStaticFeeTooLarge()) revert FeeTooLarge();
Expand Down
51 changes: 0 additions & 51 deletions src/test/PoolInitializeTest.sol
View file

This file was deleted.

19 changes: 18 additions & 1 deletion src/test/PoolNestedActionsTest.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@ import {Constants} from "../../test/utils/Constants.sol";
import {Test} from "forge-std/Test.sol";
import {BalanceDelta} from "../types/BalanceDelta.sol";
import {Currency} from "../types/Currency.sol";
import {PoolId, PoolIdLibrary} from "../types/PoolId.sol";

enum Action {
NESTED_SELF_LOCK,
NESTED_EXECUTOR_LOCK,
SWAP_AND_SETTLE,
DONATE_AND_SETTLE,
ADD_LIQ_AND_SETTLE,
REMOVE_LIQ_AND_SETTLE
REMOVE_LIQ_AND_SETTLE,
INITIALIZE
}

contract PoolNestedActionsTest is Test, ILockCallback {
Expand Down Expand Up @@ -58,6 +60,8 @@ contract PoolNestedActionsTest is Test, ILockCallback {
}

contract NestedActionExecutor is Test, PoolTestBase {
using PoolIdLibrary for PoolKey;

PoolKey internal key;
address user;

Expand Down Expand Up @@ -92,6 +96,7 @@ contract NestedActionExecutor is Test, PoolTestBase {
else if (action == Action.ADD_LIQ_AND_SETTLE) _addLiquidity();
else if (action == Action.REMOVE_LIQ_AND_SETTLE) _removeLiquidity();
else if (action == Action.DONATE_AND_SETTLE) _donate();
else if (action == Action.INITIALIZE) _initialize();
}
}

Expand Down Expand Up @@ -211,6 +216,18 @@ contract NestedActionExecutor is Test, PoolTestBase {
_settle(key.currency1, user, int128(deltaThisAfter1), true);
}

function _initialize() internal {
address locker = manager.getLocker();
assertTrue(locker != address(this), "Locker wrong");
key.tickSpacing = 50;
PoolId id = key.toId();
(uint256 price,,) = manager.getSlot0(id);
assertEq(price, 0);
manager.initialize(key, Constants.SQRT_RATIO_1_2, Constants.ZERO_BYTES);
(price,,) = manager.getSlot0(id);
assertEq(price, Constants.SQRT_RATIO_1_2);
}

// This will never actually be used - its just to allow us to use the PoolTestBase helper contact
function lockAcquired(bytes calldata) external pure override returns (bytes memory) {
return "";
Expand Down
2 changes: 1 addition & 1 deletion test/DynamicFees.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {
dynamicFeesHook.setFee(1000000);

vm.expectRevert(IFees.FeeTooLarge.selector);
initializeRouter.initialize(key, SQRT_RATIO_1_1, ZERO_BYTES);
manager.initialize(key, SQRT_RATIO_1_1, ZERO_BYTES);
}

function testUpdateFailsWithTooLargeFee() public {
Expand Down
6 changes: 3 additions & 3 deletions test/Hooks.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ contract HooksTest is Test, Deployers, GasSnapshot {
}

function test_initialize_succeedsWithHook() public {
initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, new bytes(123));
manager.initialize(uninitializedKey, SQRT_RATIO_1_1, new bytes(123));

(uint160 sqrtPriceX96,,) = manager.getSlot0(uninitializedKey.toId());
assertEq(sqrtPriceX96, SQRT_RATIO_1_1);
Expand All @@ -55,13 +55,13 @@ contract HooksTest is Test, Deployers, GasSnapshot {
function test_beforeInitialize_invalidReturn() public {
mockHooks.setReturnValue(mockHooks.beforeInitialize.selector, bytes4(0xdeadbeef));
vm.expectRevert(Hooks.InvalidHookResponse.selector);
initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
manager.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
}

function test_afterInitialize_invalidReturn() public {
mockHooks.setReturnValue(mockHooks.afterInitialize.selector, bytes4(0xdeadbeef));
vm.expectRevert(Hooks.InvalidHookResponse.selector);
initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
manager.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
}

function test_beforeAfterAddLiquidity_beforeAfterRemoveLiquidity_succeedsWithHook() public {
Expand Down
5 changes: 5 additions & 0 deletions test/NestedActions.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,9 @@ contract NestedActions is Test, Deployers, GasSnapshot {
actions = [Action.DONATE_AND_SETTLE];
nestedActionRouter.lock(abi.encode(actions));
}

function test_nestedInitialize() public {
actions = [Action.INITIALIZE];
nestedActionRouter.lock(abi.encode(actions));
}
}
2 changes: 1 addition & 1 deletion test/PoolManager.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ contract PoolManagerTest is Test, Deployers, GasSnapshot {
// sets the upper 12 bits
feeController.setSwapFeeForPool(uninitializedKey.toId(), uint16(protocolFee));

initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
manager.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
(Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId());
assertEq(slot0.protocolFee, protocolFee);
}
Expand Down
57 changes: 28 additions & 29 deletions test/PoolManagerInitialize.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,23 +58,23 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {

if (key0.fee & FeeLibrary.STATIC_FEE_MASK >= 1000000) {
vm.expectRevert(abi.encodeWithSelector(IFees.FeeTooLarge.selector));
initializeRouter.initialize(key0, sqrtPriceX96, ZERO_BYTES);
manager.initialize(key0, sqrtPriceX96, ZERO_BYTES);
} else if (key0.tickSpacing > manager.MAX_TICK_SPACING()) {
vm.expectRevert(abi.encodeWithSelector(IPoolManager.TickSpacingTooLarge.selector));
initializeRouter.initialize(key0, sqrtPriceX96, ZERO_BYTES);
manager.initialize(key0, sqrtPriceX96, ZERO_BYTES);
} else if (key0.tickSpacing < manager.MIN_TICK_SPACING()) {
vm.expectRevert(abi.encodeWithSelector(IPoolManager.TickSpacingTooSmall.selector));
initializeRouter.initialize(key0, sqrtPriceX96, ZERO_BYTES);
manager.initialize(key0, sqrtPriceX96, ZERO_BYTES);
} else if (key0.currency0 >= key0.currency1) {
vm.expectRevert(abi.encodeWithSelector(IPoolManager.CurrenciesOutOfOrderOrEqual.selector));
initializeRouter.initialize(key0, sqrtPriceX96, ZERO_BYTES);
manager.initialize(key0, sqrtPriceX96, ZERO_BYTES);
} else if (!key0.hooks.isValidHookAddress(key0.fee)) {
vm.expectRevert(abi.encodeWithSelector(Hooks.HookAddressNotValid.selector, address(key0.hooks)));
initializeRouter.initialize(key0, sqrtPriceX96, ZERO_BYTES);
manager.initialize(key0, sqrtPriceX96, ZERO_BYTES);
} else {
vm.expectEmit(true, true, true, true);
emit Initialize(key0.toId(), key0.currency0, key0.currency1, key0.fee, key0.tickSpacing, key0.hooks);
initializeRouter.initialize(key0, sqrtPriceX96, ZERO_BYTES);
manager.initialize(key0, sqrtPriceX96, ZERO_BYTES);

(Pool.Slot0 memory slot0,,,) = manager.pools(key0.toId());
assertEq(slot0.sqrtPriceX96, sqrtPriceX96);
Expand All @@ -96,7 +96,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.tickSpacing,
uninitializedKey.hooks
);
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);

(Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId());
assertEq(slot0.sqrtPriceX96, sqrtPriceX96);
Expand All @@ -119,16 +119,15 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {

uninitializedKey.hooks = IHooks(mockAddr);

int24 tick = initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
int24 tick = manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
(Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId());
assertEq(slot0.sqrtPriceX96, sqrtPriceX96, "sqrtPrice");

bytes32 beforeSelector = MockHooks.beforeInitialize.selector;
bytes memory beforeParams = abi.encode(address(initializeRouter), uninitializedKey, sqrtPriceX96, ZERO_BYTES);
bytes memory beforeParams = abi.encode(address(this), uninitializedKey, sqrtPriceX96, ZERO_BYTES);

bytes32 afterSelector = MockHooks.afterInitialize.selector;
bytes memory afterParams =
abi.encode(address(initializeRouter), uninitializedKey, sqrtPriceX96, tick, ZERO_BYTES);
bytes memory afterParams = abi.encode(address(this), uninitializedKey, sqrtPriceX96, tick, ZERO_BYTES);

assertEq(MockContract(mockAddr).timesCalledSelector(beforeSelector), 1, "beforeSelector count");
assertTrue(MockContract(mockAddr).calledWithSelector(beforeSelector, beforeParams), "beforeSelector params");
Expand All @@ -152,7 +151,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.hooks
);

initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
}

function test_initialize_succeedsWithEmptyHooks(uint160 sqrtPriceX96) public {
Expand All @@ -167,7 +166,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {

uninitializedKey.hooks = mockHooks;

initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
(Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId());
assertEq(slot0.sqrtPriceX96, sqrtPriceX96);
}
Expand All @@ -180,7 +179,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.currency1 = currency0;

vm.expectRevert(IPoolManager.CurrenciesOutOfOrderOrEqual.selector);
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
}

function test_initialize_revertsWithSameTokenCombo(uint160 sqrtPriceX96) public {
Expand All @@ -191,7 +190,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.currency0 = currency1;

vm.expectRevert(IPoolManager.CurrenciesOutOfOrderOrEqual.selector);
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
}

function test_initialize_fetchFeeWhenController(uint16 protocolFee) public {
Expand All @@ -201,7 +200,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uint8 fee0 = uint8(protocolFee >> 8);
uint8 fee1 = uint8(protocolFee % 256);

initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
manager.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);

(Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId());
assertEq(slot0.sqrtPriceX96, SQRT_RATIO_1_1);
Expand All @@ -216,9 +215,9 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
// Assumptions tested in Pool.t.sol
sqrtPriceX96 = uint160(bound(sqrtPriceX96, TickMath.MIN_SQRT_RATIO, TickMath.MAX_SQRT_RATIO - 1));

initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
vm.expectRevert(Pool.PoolAlreadyInitialized.selector);
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
}

function test_initialize_failsWithIncorrectSelectors() public {
Expand All @@ -235,12 +234,12 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {

// Fails at beforeInitialize hook.
vm.expectRevert(Hooks.InvalidHookResponse.selector);
initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
manager.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);

// Fail at afterInitialize hook.
mockHooks.setReturnValue(mockHooks.beforeInitialize.selector, mockHooks.beforeInitialize.selector);
vm.expectRevert(Hooks.InvalidHookResponse.selector);
initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
manager.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
}

function test_initialize_succeedsWithCorrectSelectors() public {
Expand All @@ -265,7 +264,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.hooks
);

initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
manager.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
}

function test_initialize_failsIfTickSpaceTooLarge(uint160 sqrtPriceX96) public {
Expand All @@ -275,7 +274,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.tickSpacing = manager.MAX_TICK_SPACING() + 1;

vm.expectRevert(abi.encodeWithSelector(IPoolManager.TickSpacingTooLarge.selector));
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
}

function test_initialize_failsIfTickSpaceZero(uint160 sqrtPriceX96) public {
Expand All @@ -285,7 +284,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.tickSpacing = 0;

vm.expectRevert(abi.encodeWithSelector(IPoolManager.TickSpacingTooSmall.selector));
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
}

function test_initialize_failsIfTickSpaceNeg(uint160 sqrtPriceX96) public {
Expand All @@ -295,7 +294,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.tickSpacing = -1;

vm.expectRevert(abi.encodeWithSelector(IPoolManager.TickSpacingTooSmall.selector));
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
}

function test_initialize_succeedsWithOutOfBoundsFeeController(uint160 sqrtPriceX96) public {
Expand All @@ -313,7 +312,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.tickSpacing,
uninitializedKey.hooks
);
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
// protocol fees should default to 0
(Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId());
assertEq(slot0.protocolFee, 0);
Expand All @@ -337,7 +336,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.tickSpacing,
uninitializedKey.hooks
);
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
// protocol fees should default to 0
(Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId());
assertEq(slot0.protocolFee, 0);
Expand All @@ -358,7 +357,7 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.tickSpacing,
uninitializedKey.hooks
);
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
// protocol fees should default to 0
(Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId());
assertEq(slot0.protocolFee, 0);
Expand All @@ -379,15 +378,15 @@ contract PoolManagerInitializeTest is Test, Deployers, GasSnapshot {
uninitializedKey.tickSpacing,
uninitializedKey.hooks
);
initializeRouter.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
manager.initialize(uninitializedKey, sqrtPriceX96, ZERO_BYTES);
// protocol fees should default to 0
(Pool.Slot0 memory slot0,,,) = manager.pools(uninitializedKey.toId());
assertEq(slot0.protocolFee, 0);
}

function test_initialize_gas() public {
snapStart("initialize");
initializeRouter.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
manager.initialize(uninitializedKey, SQRT_RATIO_1_1, ZERO_BYTES);
snapEnd();
}
}
Loading

0 comments on commit 0e8e6c9

Please sign in to comment.