Skip to content

Commit 4caac19

Browse files
authoredJun 18, 2024··
noDelegateCall on locked functions (#743)
* noDelegateCall on locked functions * changes re comments * check in the missing snapshots * remove noDelegateCall from mint/burn and settle/take
1 parent ae86975 commit 4caac19

File tree

6 files changed

+254
-6
lines changed

6 files changed

+254
-6
lines changed
 
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
23805
1+
23821
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
208496
1+
208547

‎src/PoolManager.sol

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
104104
}
105105

106106
/// @inheritdoc IPoolManager
107-
function unlock(bytes calldata data) external override noDelegateCall returns (bytes memory result) {
107+
function unlock(bytes calldata data) external override returns (bytes memory result) {
108108
if (Lock.isUnlocked()) AlreadyUnlocked.selector.revertWith();
109109

110110
Lock.unlock();
@@ -150,7 +150,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
150150
PoolKey memory key,
151151
IPoolManager.ModifyLiquidityParams memory params,
152152
bytes calldata hookData
153-
) external override onlyWhenUnlocked returns (BalanceDelta callerDelta, BalanceDelta feesAccrued) {
153+
) external override onlyWhenUnlocked noDelegateCall returns (BalanceDelta callerDelta, BalanceDelta feesAccrued) {
154154
PoolId id = key.toId();
155155
Pool.State storage pool = _getPool(id);
156156
pool.checkPoolInitialized();
@@ -188,6 +188,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
188188
external
189189
override
190190
onlyWhenUnlocked
191+
noDelegateCall
191192
returns (BalanceDelta swapDelta)
192193
{
193194
if (params.amountSpecified == 0) SwapAmountCannotBeZero.selector.revertWith();
@@ -249,6 +250,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
249250
external
250251
override
251252
onlyWhenUnlocked
253+
noDelegateCall
252254
returns (BalanceDelta delta)
253255
{
254256
Pool.State storage pool = _getPool(key.toId());

‎src/test/ProxyPoolManager.sol

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
pragma solidity ^0.8.24;
3+
4+
import {Hooks} from "../libraries/Hooks.sol";
5+
import {Pool} from "../libraries/Pool.sol";
6+
import {SafeCast} from "../libraries/SafeCast.sol";
7+
import {Position} from "../libraries/Position.sol";
8+
import {LPFeeLibrary} from "../libraries/LPFeeLibrary.sol";
9+
import {Currency, CurrencyLibrary} from "../types/Currency.sol";
10+
import {PoolKey} from "../types/PoolKey.sol";
11+
import {TickMath} from "../libraries/TickMath.sol";
12+
import {NoDelegateCall} from "../NoDelegateCall.sol";
13+
import {IHooks} from "../interfaces/IHooks.sol";
14+
import {IPoolManager} from "../interfaces/IPoolManager.sol";
15+
import {IUnlockCallback} from "../interfaces/callback/IUnlockCallback.sol";
16+
import {ProtocolFees} from "../ProtocolFees.sol";
17+
import {ERC6909Claims} from "../ERC6909Claims.sol";
18+
import {PoolId, PoolIdLibrary} from "../types/PoolId.sol";
19+
import {BalanceDelta, BalanceDeltaLibrary, toBalanceDelta} from "../types/BalanceDelta.sol";
20+
import {BeforeSwapDelta} from "../types/BeforeSwapDelta.sol";
21+
import {Lock} from "../libraries/Lock.sol";
22+
import {CurrencyDelta} from "../libraries/CurrencyDelta.sol";
23+
import {NonZeroDeltaCount} from "../libraries/NonZeroDeltaCount.sol";
24+
import {Reserves} from "../libraries/Reserves.sol";
25+
import {Extsload} from "../Extsload.sol";
26+
import {Exttload} from "../Exttload.sol";
27+
import {CustomRevert} from "../libraries/CustomRevert.sol";
28+
29+
/// @notice A proxy pool manager that delegates calls to the real/delegate pool manager
30+
contract ProxyPoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claims, Extsload, Exttload {
31+
using PoolIdLibrary for PoolKey;
32+
using SafeCast for *;
33+
using Pool for *;
34+
using Hooks for IHooks;
35+
using Position for mapping(bytes32 => Position.Info);
36+
using CurrencyDelta for Currency;
37+
using LPFeeLibrary for uint24;
38+
using Reserves for Currency;
39+
using CustomRevert for bytes4;
40+
41+
/// @inheritdoc IPoolManager
42+
int24 public constant MAX_TICK_SPACING = TickMath.MAX_TICK_SPACING;
43+
44+
/// @inheritdoc IPoolManager
45+
int24 public constant MIN_TICK_SPACING = TickMath.MIN_TICK_SPACING;
46+
47+
mapping(PoolId id => Pool.State) internal _pools;
48+
49+
address internal immutable _delegateManager;
50+
51+
constructor(address delegateManager, uint256 controllerGasLimit) ProtocolFees(controllerGasLimit) {
52+
_delegateManager = delegateManager;
53+
}
54+
55+
/// @notice This will revert if the contract is locked
56+
modifier onlyWhenUnlocked() {
57+
if (!Lock.isUnlocked()) ManagerLocked.selector.revertWith();
58+
_;
59+
}
60+
61+
/// @inheritdoc IPoolManager
62+
function unlock(bytes calldata data) external override noDelegateCall returns (bytes memory result) {
63+
if (Lock.isUnlocked()) AlreadyUnlocked.selector.revertWith();
64+
65+
Lock.unlock();
66+
67+
// the caller does everything in this callback, including paying what they owe via calls to settle
68+
result = IUnlockCallback(msg.sender).unlockCallback(data);
69+
70+
if (NonZeroDeltaCount.read() != 0) CurrencyNotSettled.selector.revertWith();
71+
Lock.lock();
72+
}
73+
74+
/// @inheritdoc IPoolManager
75+
function initialize(PoolKey memory key, uint160 sqrtPriceX96, bytes calldata hookData)
76+
external
77+
override
78+
noDelegateCall
79+
returns (int24 tick)
80+
{
81+
// see TickBitmap.sol for overflow conditions that can arise from tick spacing being too large
82+
if (key.tickSpacing > MAX_TICK_SPACING) TickSpacingTooLarge.selector.revertWith();
83+
if (key.tickSpacing < MIN_TICK_SPACING) TickSpacingTooSmall.selector.revertWith();
84+
if (key.currency0 >= key.currency1) CurrenciesOutOfOrderOrEqual.selector.revertWith();
85+
if (!key.hooks.isValidHookAddress(key.fee)) Hooks.HookAddressNotValid.selector.revertWith(address(key.hooks));
86+
87+
uint24 lpFee = key.fee.getInitialLPFee();
88+
89+
key.hooks.beforeInitialize(key, sqrtPriceX96, hookData);
90+
91+
PoolId id = key.toId();
92+
(, uint24 protocolFee) = _fetchProtocolFee(key);
93+
94+
tick = _pools[id].initialize(sqrtPriceX96, protocolFee, lpFee);
95+
96+
key.hooks.afterInitialize(key, sqrtPriceX96, tick, hookData);
97+
98+
// emit all details of a pool key. poolkeys are not saved in storage and must always be provided by the caller
99+
// the key's fee may be a static fee or a sentinel to denote a dynamic fee.
100+
emit Initialize(id, key.currency0, key.currency1, key.fee, key.tickSpacing, key.hooks);
101+
}
102+
103+
/// @inheritdoc IPoolManager
104+
function modifyLiquidity(
105+
PoolKey memory key,
106+
IPoolManager.ModifyLiquidityParams memory params,
107+
bytes calldata hookData
108+
) external override onlyWhenUnlocked noDelegateCall returns (BalanceDelta callerDelta, BalanceDelta feesAccrued) {
109+
bytes memory result = _delegateCall(
110+
_delegateManager, abi.encodeWithSelector(this.modifyLiquidity.selector, key, params, hookData)
111+
);
112+
113+
return abi.decode(result, (BalanceDelta, BalanceDelta));
114+
}
115+
116+
/// @inheritdoc IPoolManager
117+
function swap(PoolKey memory key, IPoolManager.SwapParams memory params, bytes calldata hookData)
118+
external
119+
override
120+
onlyWhenUnlocked
121+
noDelegateCall
122+
returns (BalanceDelta swapDelta)
123+
{
124+
bytes memory result =
125+
_delegateCall(_delegateManager, abi.encodeWithSelector(this.swap.selector, key, params, hookData));
126+
127+
return abi.decode(result, (BalanceDelta));
128+
}
129+
130+
/// @inheritdoc IPoolManager
131+
function donate(PoolKey memory key, uint256 amount0, uint256 amount1, bytes calldata hookData)
132+
external
133+
override
134+
onlyWhenUnlocked
135+
noDelegateCall
136+
returns (BalanceDelta delta)
137+
{
138+
bytes memory result = _delegateCall(
139+
_delegateManager, abi.encodeWithSelector(this.donate.selector, key, amount0, amount1, hookData)
140+
);
141+
142+
return abi.decode(result, (BalanceDelta));
143+
}
144+
145+
/// @inheritdoc IPoolManager
146+
function sync(Currency currency) public returns (uint256 balance) {
147+
balance = currency.balanceOfSelf();
148+
currency.setReserves(balance);
149+
}
150+
151+
/// @inheritdoc IPoolManager
152+
function take(Currency currency, address to, uint256 amount) external override onlyWhenUnlocked noDelegateCall {
153+
_delegateCall(_delegateManager, abi.encodeWithSelector(this.take.selector, currency, to, amount));
154+
}
155+
156+
/// @inheritdoc IPoolManager
157+
function settle(Currency currency)
158+
external
159+
payable
160+
override
161+
onlyWhenUnlocked
162+
noDelegateCall
163+
returns (uint256 paid)
164+
{
165+
bytes memory result = _delegateCall(_delegateManager, abi.encodeWithSelector(this.settle.selector, currency));
166+
167+
return abi.decode(result, (uint256));
168+
}
169+
170+
/// @inheritdoc IPoolManager
171+
function mint(address to, uint256 id, uint256 amount) external override onlyWhenUnlocked noDelegateCall {
172+
_delegateCall(_delegateManager, abi.encodeWithSelector(this.mint.selector, to, id, amount));
173+
}
174+
175+
/// @inheritdoc IPoolManager
176+
function burn(address from, uint256 id, uint256 amount) external override onlyWhenUnlocked noDelegateCall {
177+
_delegateCall(_delegateManager, abi.encodeWithSelector(this.burn.selector, from, id, amount));
178+
}
179+
180+
/// @inheritdoc IPoolManager
181+
function updateDynamicLPFee(PoolKey memory key, uint24 newDynamicLPFee) external {
182+
if (!key.fee.isDynamicFee() || msg.sender != address(key.hooks)) {
183+
UnauthorizedDynamicLPFeeUpdate.selector.revertWith();
184+
}
185+
newDynamicLPFee.validate();
186+
PoolId id = key.toId();
187+
_pools[id].setLPFee(newDynamicLPFee);
188+
}
189+
190+
/// @notice Make a delegate call, bubble up any error or return the result
191+
function _delegateCall(address target, bytes memory data) internal returns (bytes memory result) {
192+
(bool success, bytes memory returnData) = target.delegatecall(data);
193+
194+
if (!success) {
195+
if (returnData.length == 0) {
196+
revert();
197+
} else {
198+
assembly {
199+
let size := mload(returnData)
200+
revert(add(32, returnData), size)
201+
}
202+
}
203+
}
204+
205+
return returnData;
206+
}
207+
208+
/// @notice Implementation of the _getPool function defined in ProtocolFees
209+
function _getPool(PoolId id) internal view override returns (Pool.State storage) {
210+
return _pools[id];
211+
}
212+
}

‎test/NoDelegateCall.t.sol

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,26 @@ pragma solidity ^0.8.20;
33

44
import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol";
55
import {Test} from "forge-std/Test.sol";
6+
import {IPoolManager} from "../src/interfaces/IPoolManager.sol";
7+
import {PoolSwapTest} from "../src/test/PoolSwapTest.sol";
8+
import {ProxyPoolManager} from "../src/test/ProxyPoolManager.sol";
69
import {NoDelegateCallTest} from "../src/test/NoDelegateCallTest.sol";
10+
import {PoolManager} from "../src/PoolManager.sol";
711
import {NoDelegateCall} from "../src/NoDelegateCall.sol";
12+
import {Deployers} from "./utils/Deployers.sol";
13+
14+
contract TestDelegateCall is Test, Deployers, GasSnapshot {
15+
// override to use ProxyPoolManager
16+
function deployFreshManager() internal virtual override {
17+
IPoolManager delegateManager = new PoolManager(500000);
18+
manager = new ProxyPoolManager(address(delegateManager), 500000);
19+
}
820

9-
contract TestDelegateCall is Test, GasSnapshot {
1021
NoDelegateCallTest noDelegateCallTest;
1122

1223
function setUp() public {
24+
deployFreshManagerAndRouters();
25+
1326
noDelegateCallTest = new NoDelegateCallTest();
1427
}
1528

@@ -47,4 +60,25 @@ contract TestDelegateCall is Test, GasSnapshot {
4760
// note vm.expectRevert inverts success, so a true result here means it reverted
4861
assertTrue(success);
4962
}
63+
64+
function test_modifyLiquidity_noDelegateCall() public {
65+
vm.expectRevert(NoDelegateCall.DelegateCallNotAllowed.selector);
66+
modifyLiquidityRouter.modifyLiquidity(uninitializedKey, LIQUIDITY_PARAMS, ZERO_BYTES);
67+
68+
vm.expectRevert(NoDelegateCall.DelegateCallNotAllowed.selector);
69+
modifyLiquidityRouter.modifyLiquidity(uninitializedKey, REMOVE_LIQUIDITY_PARAMS, ZERO_BYTES);
70+
}
71+
72+
function test_swap_noDelegateCall() public {
73+
PoolSwapTest.TestSettings memory testSettings =
74+
PoolSwapTest.TestSettings({takeClaims: false, settleUsingBurn: false});
75+
76+
vm.expectRevert(NoDelegateCall.DelegateCallNotAllowed.selector);
77+
swapRouter.swap(key, SWAP_PARAMS, testSettings, ZERO_BYTES);
78+
}
79+
80+
function test_donate_noDelegateCall() public {
81+
vm.expectRevert(NoDelegateCall.DelegateCallNotAllowed.selector);
82+
donateRouter.donate(key, 100, 200, ZERO_BYTES);
83+
}
5084
}

‎test/utils/Deployers.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ contract Deployers {
9191
}
9292
}
9393

94-
function deployFreshManager() internal {
94+
function deployFreshManager() internal virtual {
9595
manager = new PoolManager(500000);
9696
}
9797

0 commit comments

Comments
 (0)
Please sign in to comment.