Skip to content

Commit

Permalink
Merge pull request #1 from saucepoint/consolidate
Browse files Browse the repository at this point in the history
Consolidate overloaded `modifyPosition` and `swap`
  • Loading branch information
saucepoint authored Sep 19, 2023
2 parents f3623dc + 9f08118 commit 1346029
Show file tree
Hide file tree
Showing 17 changed files with 101 additions and 93 deletions.
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 1 token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
95484
95478
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 2 tokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
152797
152791
2 changes: 1 addition & 1 deletion .forge-snapshots/mint with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
306602
307152
2 changes: 1 addition & 1 deletion .forge-snapshots/mint with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
274534
275102
2 changes: 1 addition & 1 deletion .forge-snapshots/mint.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
293198
293760
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
49147
49720
Original file line number Diff line number Diff line change
@@ -1 +1 @@
124413
124982
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
109120
109692
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
90966
91502
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
49122
49691
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
49147
49720
8 changes: 0 additions & 8 deletions contracts/test/PoolModifyPositionTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,6 @@ contract PoolModifyPositionTest is ILockCallback {
bytes hookData;
}

function modifyPosition(PoolKey memory key, IPoolManager.ModifyPositionParams memory params)
external
payable
returns (BalanceDelta delta)
{
return modifyPosition(key, params, new bytes(0));
}

function modifyPosition(PoolKey memory key, IPoolManager.ModifyPositionParams memory params, bytes memory hookData)
public
payable
Expand Down
8 changes: 0 additions & 8 deletions contracts/test/PoolSwapTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@ contract PoolSwapTest is ILockCallback {
bool settleUsingTransfer;
}

function swap(PoolKey memory key, IPoolManager.SwapParams memory params, TestSettings memory testSettings)
external
payable
returns (BalanceDelta delta)
{
return swap(key, params, testSettings, new bytes(0));
}

function swap(
PoolKey memory key,
IPoolManager.SwapParams memory params,
Expand Down
10 changes: 8 additions & 2 deletions test/foundry-tests/DynamicFees.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {
dynamicFees.setFee(1000000);
vm.expectRevert(IFees.FeeTooLarge.selector);
swapRouter.swap(
key, IPoolManager.SwapParams(false, 1, SQRT_RATIO_1_1 + 1), PoolSwapTest.TestSettings(false, false)
key,
IPoolManager.SwapParams(false, 1, SQRT_RATIO_1_1 + 1),
PoolSwapTest.TestSettings(false, false),
ZERO_BYTES
);
}

Expand All @@ -85,7 +88,10 @@ contract TestDynamicFees is Test, Deployers, GasSnapshot {
emit Swap(key.toId(), address(swapRouter), 0, 0, SQRT_RATIO_1_1 + 1, 0, 0, 123);
snapStart("swap with dynamic fee");
swapRouter.swap(
key, IPoolManager.SwapParams(false, 1, SQRT_RATIO_1_1 + 1), PoolSwapTest.TestSettings(false, false)
key,
IPoolManager.SwapParams(false, 1, SQRT_RATIO_1_1 + 1),
PoolSwapTest.TestSettings(false, false),
ZERO_BYTES
);
snapEnd();
}
Expand Down
39 changes: 22 additions & 17 deletions test/foundry-tests/Fees.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -285,10 +285,10 @@ contract FeesTest is Test, Deployers, TokenFixture, GasSnapshot {
assertEq(getWithdrawFee(slot0.protocolFees), protocolWithdrawFee);

IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-60, 60, 10e18);
modifyPositionRouter.modifyPosition(key0, params);
modifyPositionRouter.modifyPosition(key0, params, ZERO_BYTES);

IPoolManager.ModifyPositionParams memory params2 = IPoolManager.ModifyPositionParams(-60, 60, -10e18);
modifyPositionRouter.modifyPosition(key0, params2);
modifyPositionRouter.modifyPosition(key0, params2, ZERO_BYTES);

// Fees dont accrue when key.fee does not specify a withdrawal param even if the protocol fee is set.
assertEq(manager.protocolFeesAccrued(currency0), 0);
Expand Down Expand Up @@ -345,7 +345,7 @@ contract FeesTest is Test, Deployers, TokenFixture, GasSnapshot {
uint256 underlyingAmount1 = 29;

IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-60, 60, liquidityDelta);
BalanceDelta delta = modifyPositionRouter.modifyPosition(key1, params);
BalanceDelta delta = modifyPositionRouter.modifyPosition(key1, params, ZERO_BYTES);

// Fees dont accrue for positive liquidity delta.
assertEq(manager.protocolFeesAccrued(currency0), 0);
Expand All @@ -354,7 +354,7 @@ contract FeesTest is Test, Deployers, TokenFixture, GasSnapshot {
assertEq(manager.hookFeesAccrued(address(key1.hooks), currency1), 0);

IPoolManager.ModifyPositionParams memory params2 = IPoolManager.ModifyPositionParams(-60, 60, -liquidityDelta);
delta = modifyPositionRouter.modifyPosition(key1, params2);
delta = modifyPositionRouter.modifyPosition(key1, params2, ZERO_BYTES);

uint16 hookFee0 = (hookWithdrawFee % 64);
uint16 hookFee1 = (hookWithdrawFee >> 6);
Expand Down Expand Up @@ -398,7 +398,7 @@ contract FeesTest is Test, Deployers, TokenFixture, GasSnapshot {

int256 liquidityDelta = 10000;
IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-60, 60, liquidityDelta);
modifyPositionRouter.modifyPosition(key3, params);
modifyPositionRouter.modifyPosition(key3, params, ZERO_BYTES);

// Fees dont accrue for positive liquidity delta.
assertEq(manager.protocolFeesAccrued(currency0), 0);
Expand All @@ -407,7 +407,7 @@ contract FeesTest is Test, Deployers, TokenFixture, GasSnapshot {
assertEq(manager.hookFeesAccrued(address(key3.hooks), currency1), 0);

IPoolManager.ModifyPositionParams memory params2 = IPoolManager.ModifyPositionParams(-60, 60, -liquidityDelta);
modifyPositionRouter.modifyPosition(key3, params2);
modifyPositionRouter.modifyPosition(key3, params2, ZERO_BYTES);

uint16 protocolSwapFee1 = (protocolSwapFee >> 6);

Expand All @@ -417,13 +417,14 @@ contract FeesTest is Test, Deployers, TokenFixture, GasSnapshot {

// add larger liquidity
params = IPoolManager.ModifyPositionParams(-60, 60, 10e18);
modifyPositionRouter.modifyPosition(key3, params);
modifyPositionRouter.modifyPosition(key3, params, ZERO_BYTES);

MockERC20(Currency.unwrap(currency1)).approve(address(swapRouter), type(uint256).max);
swapRouter.swap(
key3,
IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1),
PoolSwapTest.TestSettings(true, true)
PoolSwapTest.TestSettings(true, true),
ZERO_BYTES
);
// key3 pool is 30 bps => 10000 * 0.003 (.3%) = 30
uint256 expectedSwapFeeAccrued = 30;
Expand Down Expand Up @@ -451,13 +452,14 @@ contract FeesTest is Test, Deployers, TokenFixture, GasSnapshot {
assertEq(getWithdrawFee(slot0.hookFees), 0);

IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-120, 120, 10e18);
modifyPositionRouter.modifyPosition(key0, params);
modifyPositionRouter.modifyPosition(key0, params, ZERO_BYTES);
// 1 for 0 swap
MockERC20(Currency.unwrap(currency1)).approve(address(swapRouter), type(uint256).max);
swapRouter.swap(
key0,
IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1),
PoolSwapTest.TestSettings(true, true)
PoolSwapTest.TestSettings(true, true),
ZERO_BYTES
);

assertEq(manager.protocolFeesAccrued(currency1), 3); // 10% of 30 is 3
Expand All @@ -483,13 +485,14 @@ contract FeesTest is Test, Deployers, TokenFixture, GasSnapshot {
assertEq(getWithdrawFee(slot0.hookFees), 0);

IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-120, 120, 10e18);
modifyPositionRouter.modifyPosition(key0, params);
modifyPositionRouter.modifyPosition(key0, params, ZERO_BYTES);
// 1 for 0 swap
MockERC20(Currency.unwrap(currency1)).approve(address(swapRouter), type(uint256).max);
swapRouter.swap(
key0,
IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1),
PoolSwapTest.TestSettings(true, true)
PoolSwapTest.TestSettings(true, true),
ZERO_BYTES
);

assertEq(manager.protocolFeesAccrued(currency1), 3); // 10% of 30 is 3
Expand Down Expand Up @@ -517,20 +520,21 @@ contract FeesTest is Test, Deployers, TokenFixture, GasSnapshot {
assertEq(getWithdrawFee(slot0.hookFees), 0); // Even though the contract sets a withdraw fee it will not be applied bc the pool key.fee did not assert a withdraw flag.

IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-120, 120, 10e18);
modifyPositionRouter.modifyPosition(key0, params);
modifyPositionRouter.modifyPosition(key0, params, ZERO_BYTES);
// 1 for 0 swap
MockERC20(Currency.unwrap(currency1)).approve(address(swapRouter), type(uint256).max);
swapRouter.swap(
key0,
IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1),
PoolSwapTest.TestSettings(true, true)
PoolSwapTest.TestSettings(true, true),
ZERO_BYTES
);

assertEq(manager.protocolFeesAccrued(currency1), 0); // No protocol fee was accrued on swap
assertEq(manager.protocolFeesAccrued(currency0), 0); // No protocol fee was accrued on swap
assertEq(manager.hookFeesAccrued(address(key0.hooks), currency1), 7); // 25% on 1 to 0, 25% of 30 is 7.5 so 7

modifyPositionRouter.modifyPosition(key0, IPoolManager.ModifyPositionParams(-120, 120, -10e18));
modifyPositionRouter.modifyPosition(key0, IPoolManager.ModifyPositionParams(-120, 120, -10e18), ZERO_BYTES);

assertEq(manager.protocolFeesAccrued(currency1), 0); // No protocol fee was accrued on withdraw
assertEq(manager.protocolFeesAccrued(currency0), 0); // No protocol fee was accrued on withdraw
Expand All @@ -555,13 +559,14 @@ contract FeesTest is Test, Deployers, TokenFixture, GasSnapshot {
assertEq(getSwapFee(slot0.hookFees), hookFee);

IPoolManager.ModifyPositionParams memory params = IPoolManager.ModifyPositionParams(-120, 120, 10e18);
modifyPositionRouter.modifyPosition(key0, params);
modifyPositionRouter.modifyPosition(key0, params, ZERO_BYTES);
// 1 for 0 swap
MockERC20(Currency.unwrap(currency1)).approve(address(swapRouter), type(uint256).max);
swapRouter.swap(
key0,
IPoolManager.SwapParams(false, 10000, TickMath.MAX_SQRT_RATIO - 1),
PoolSwapTest.TestSettings(true, true)
PoolSwapTest.TestSettings(true, true),
ZERO_BYTES
);

uint256 expectedProtocolFees = 3; // 10% of 30 is 3
Expand Down
18 changes: 13 additions & 5 deletions test/foundry-tests/Hooks.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ contract HooksTest is Test, Deployers, GasSnapshot {
MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 10 ** 18);
IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
vm.expectRevert(Hooks.InvalidHookResponse.selector);
modifyPositionRouter.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 100));
modifyPositionRouter.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 100), ZERO_BYTES);
}

function testAfterModifyPositionInvalidReturn() public {
mockHooks.setReturnValue(mockHooks.afterModifyPosition.selector, bytes4(0xdeadbeef));
MockERC20(Currency.unwrap(key.currency0)).mint(address(this), 10 ** 18);
IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
vm.expectRevert(Hooks.InvalidHookResponse.selector);
modifyPositionRouter.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 100));
modifyPositionRouter.modifyPosition(key, IPoolManager.ModifyPositionParams(0, 60, 100), ZERO_BYTES);
}

function testSwapSucceedsWithHook() public {
Expand All @@ -107,7 +107,10 @@ contract HooksTest is Test, Deployers, GasSnapshot {
IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(swapRouter), 10 ** 18);
vm.expectRevert(Hooks.InvalidHookResponse.selector);
swapRouter.swap(
key, IPoolManager.SwapParams(false, 100, SQRT_RATIO_1_1 + 60), PoolSwapTest.TestSettings(false, false)
key,
IPoolManager.SwapParams(false, 100, SQRT_RATIO_1_1 + 60),
PoolSwapTest.TestSettings(false, false),
ZERO_BYTES
);
}

Expand All @@ -117,7 +120,10 @@ contract HooksTest is Test, Deployers, GasSnapshot {
IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(swapRouter), 10 ** 18);
vm.expectRevert(Hooks.InvalidHookResponse.selector);
swapRouter.swap(
key, IPoolManager.SwapParams(false, 100, SQRT_RATIO_1_1 + 60), PoolSwapTest.TestSettings(false, false)
key,
IPoolManager.SwapParams(false, 100, SQRT_RATIO_1_1 + 60),
PoolSwapTest.TestSettings(false, false),
ZERO_BYTES
);
}

Expand Down Expand Up @@ -631,6 +637,8 @@ contract HooksTest is Test, Deployers, GasSnapshot {
MockERC20(Currency.unwrap(key.currency1)).mint(address(this), 10 ** 18);
IERC20Minimal(Currency.unwrap(key.currency0)).approve(address(modifyPositionRouter), 10 ** 18);
IERC20Minimal(Currency.unwrap(key.currency1)).approve(address(modifyPositionRouter), 10 ** 18);
modifyPositionRouter.modifyPosition(key, IPoolManager.ModifyPositionParams(tickLower, tickUpper, amount));
modifyPositionRouter.modifyPosition(
key, IPoolManager.ModifyPositionParams(tickLower, tickUpper, amount), ZERO_BYTES
);
}
}
Loading

0 comments on commit 1346029

Please sign in to comment.