Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access lock #404

Merged
merged 35 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
1cc37ed
rebase
snreynolds Nov 9, 2023
2a2321e
add access lock flag
snreynolds Nov 14, 2023
65069b3
create AccessLock tests
snreynolds Nov 15, 2023
589026f
add beforeModifyPosition tests for mint/swap/modify/take/donate
snreynolds Nov 16, 2023
70a869e
add beforeSwap tests
snreynolds Nov 17, 2023
e147a7c
add beforeDonate tests
snreynolds Nov 17, 2023
2396461
fix fuzz boundaries
snreynolds Nov 17, 2023
8b6b850
use transient storage
snreynolds Nov 17, 2023
400ce17
fix ci?
snreynolds Nov 17, 2023
7bda2b1
fix: comments
snreynolds Nov 20, 2023
fbda6c4
add revert test
snreynolds Nov 20, 2023
11c9321
more edge case tests
snreynolds Nov 20, 2023
d6864a4
add failing test
snreynolds Nov 20, 2023
d7829b7
fix: hooks can only call functions in the middle of a lock
snreynolds Nov 20, 2023
9d8f00b
fix comments
snreynolds Nov 20, 2023
2a7ccf9
fix: currentHook must be updated only on the first call, and cleared
snreynolds Nov 21, 2023
2e88e5f
merge main
snreynolds Nov 22, 2023
969fb9d
use IHooks, remove 1155 ref
snreynolds Nov 22, 2023
a958a37
remove special no access lock hook
snreynolds Nov 22, 2023
f70443a
add nested lock test
snreynolds Nov 22, 2023
88d9ce1
unset hook for noOp case
snreynolds Nov 26, 2023
448b557
add initialize
snreynolds Nov 27, 2023
fa432c2
add vanilla initialize tests
snreynolds Nov 27, 2023
fb803c3
fmt error
snreynolds Nov 27, 2023
f455e64
add natspec and flatten onlyByLocker check
snreynolds Nov 27, 2023
528ce4e
snaps
snreynolds Nov 27, 2023
83afc1e
move onlyByLocker, add comments
snreynolds Nov 29, 2023
b9645ef
update comments, use custom error, add test cases
snreynolds Nov 29, 2023
7fc2658
add burn test
snreynolds Nov 30, 2023
e571806
remove logs
snreynolds Nov 30, 2023
680faf7
add settle test
snreynolds Nov 30, 2023
53cdf3e
init test
snreynolds Nov 30, 2023
7755acd
add back noop test
snreynolds Nov 30, 2023
852ed5b
remove else
snreynolds Nov 30, 2023
0aee197
Merge branch 'main' into access-lock
snreynolds Dec 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
rebase
  • Loading branch information
snreynolds committed Nov 20, 2023
commit 1cc37eda70c241fa9c6c018511dffeacf3f05254
Original file line number Diff line number Diff line change
@@ -1 +1 @@
189798
192134
2 changes: 1 addition & 1 deletion .forge-snapshots/cached dynamic fee, no hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
145314
145580
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 @@
134915
137163
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 @@
186636
188884
2 changes: 1 addition & 1 deletion .forge-snapshots/initialize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
53843
53880
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 @@
319184
341339
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 @@
198926
201148
2 changes: 1 addition & 1 deletion .forge-snapshots/mint.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
202041
204226
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
24805
25262
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap with native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
190722
192988
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
202425
204691
Original file line number Diff line number Diff line change
@@ -1 +1 @@
121318
121584
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
109143
109409
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn claim for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
127882
128148
1 change: 1 addition & 0 deletions .forge-snapshots/swap mint 1155 as output.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
169819
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint output as claim.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
212594
214860
1 change: 1 addition & 0 deletions .forge-snapshots/swap with 1155 as input.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
156249
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 @@
189038
191374
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
109122
109388
2 changes: 1 addition & 1 deletion .forge-snapshots/update dynamic fee in before swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
195628
197964
44 changes: 37 additions & 7 deletions src/PoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {PoolId, PoolIdLibrary} from "./types/PoolId.sol";
import {BalanceDelta} from "./types/BalanceDelta.sol";
import {Lockers} from "./libraries/Lockers.sol";

import "forge-std/console2.sol";

/// @notice Holds the state for all pools
contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
using PoolIdLibrary for PoolKey;
Expand Down Expand Up @@ -49,6 +51,9 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {

constructor(uint256 controllerGasLimit) Fees(controllerGasLimit) {}

// todo change to transient storage
address currentHook;
snreynolds marked this conversation as resolved.
Show resolved Hide resolved

function _getPool(PoolKey memory key) private view returns (Pool.State storage) {
return pools[key.toId()];
}
Expand Down Expand Up @@ -174,8 +179,13 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
}

modifier onlyByLocker() {
snreynolds marked this conversation as resolved.
Show resolved Hide resolved
snreynolds marked this conversation as resolved.
Show resolved Hide resolved
// todo fix stack too deep :/
address locker = Lockers.getCurrentLocker();
if (msg.sender != locker) revert LockedBy(locker);
if (msg.sender != locker) {
if (msg.sender != currentHook && !Hooks.shouldAccessLock(IHooks(currentHook))) {
revert LockedBy(locker);
snreynolds marked this conversation as resolved.
Show resolved Hide resolved
}
}
_;
}

Expand All @@ -185,11 +195,15 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
IPoolManager.ModifyPositionParams memory params,
bytes calldata hookData
) external override noDelegateCall onlyByLocker returns (BalanceDelta delta) {
_setCurrentHook(address(key.hooks));

snreynolds marked this conversation as resolved.
Show resolved Hide resolved
if (key.hooks.shouldCallBeforeModifyPosition()) {
if (
key.hooks.beforeModifyPosition(msg.sender, key, params, hookData)
!= IHooks.beforeModifyPosition.selector
) {
bytes4 hookReturn = key.hooks.beforeModifyPosition(msg.sender, key, params, hookData);

if (hookReturn == Hooks.OVERRIDE_SELECTOR) {
return delta;
}
if (hookReturn != IHooks.beforeModifyPosition.selector) {
revert Hooks.InvalidHookResponse();
}
}
Expand Down Expand Up @@ -243,8 +257,14 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
onlyByLocker
returns (BalanceDelta delta)
{
_setCurrentHook(address(key.hooks));

if (key.hooks.shouldCallBeforeSwap()) {
if (key.hooks.beforeSwap(msg.sender, key, params, hookData) != IHooks.beforeSwap.selector) {
bytes4 hookReturn = key.hooks.beforeSwap(msg.sender, key, params, hookData);
if (hookReturn == Hooks.OVERRIDE_SELECTOR) {
return delta;
}
if (hookReturn != IHooks.beforeSwap.selector) {
revert Hooks.InvalidHookResponse();
}
}
Expand Down Expand Up @@ -295,8 +315,13 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
onlyByLocker
returns (BalanceDelta delta)
{
_setCurrentHook(address(key.hooks));
if (key.hooks.shouldCallBeforeDonate()) {
if (key.hooks.beforeDonate(msg.sender, key, amount0, amount1, hookData) != IHooks.beforeDonate.selector) {
bytes4 hookReturn = key.hooks.beforeDonate(msg.sender, key, amount0, amount1, hookData);
if (hookReturn == Hooks.OVERRIDE_SELECTOR) {
return delta;
}
if (hookReturn != IHooks.beforeDonate.selector) {
revert Hooks.InvalidHookResponse();
}
}
Expand Down Expand Up @@ -393,6 +418,11 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
return Lockers.nonzeroDeltaCount();
}

// TODO: Use transient storage
function _setCurrentHook(address hookAddr) internal {
currentHook = hookAddr;
}

/// @notice receive native tokens for native pools
receive() external payable {}
}
20 changes: 19 additions & 1 deletion src/libraries/Hooks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ pragma solidity ^0.8.20;
import {IHooks} from "../interfaces/IHooks.sol";
import {FeeLibrary} from "../libraries/FeeLibrary.sol";

import "forge-std/console2.sol";

/// @notice V4 decides whether to invoke specific hooks by inspecting the leading bits of the address that
/// the hooks contract is deployed to.
/// For example, a hooks contract deployed to address: 0x9000000000000000000000000000000000000000
Expand All @@ -19,7 +21,12 @@ library Hooks {
uint256 internal constant AFTER_SWAP_FLAG = 1 << 154;
uint256 internal constant BEFORE_DONATE_FLAG = 1 << 153;
uint256 internal constant AFTER_DONATE_FLAG = 1 << 152;
uint256 internal constant ACCESS_LOCK_FLAG = 1 << 151;
uint256 internal constant OVERRIDE_FLAG = 1 << 150;

bytes4 constant OVERRIDE_SELECTOR = bytes4(keccak256("OVERRIDE_SELECTOR"));

// todo not necessarily all calls now?
struct Calls {
bool beforeInitialize;
bool afterInitialize;
Expand All @@ -29,6 +36,8 @@ library Hooks {
bool afterSwap;
bool beforeDonate;
bool afterDonate;
bool accessLock;
bool overrideSelector;
}

/// @notice Thrown if the address will not lead to the specified hook calls being called
Expand All @@ -50,6 +59,7 @@ library Hooks {
|| calls.afterModifyPosition != shouldCallAfterModifyPosition(self)
|| calls.beforeSwap != shouldCallBeforeSwap(self) || calls.afterSwap != shouldCallAfterSwap(self)
|| calls.beforeDonate != shouldCallBeforeDonate(self) || calls.afterDonate != shouldCallAfterDonate(self)
|| calls.accessLock != shouldAccessLock(self) || calls.overrideSelector != shouldAllowOverride(self)
) {
revert HookAddressNotValid(address(self));
}
Expand All @@ -62,7 +72,7 @@ library Hooks {
return address(hook) == address(0)
? !fee.isDynamicFee() && !fee.hasHookSwapFee() && !fee.hasHookWithdrawFee()
: (
uint160(address(hook)) >= AFTER_DONATE_FLAG || fee.isDynamicFee() || fee.hasHookSwapFee()
uint160(address(hook)) >= ACCESS_LOCK_FLAG || fee.isDynamicFee() || fee.hasHookSwapFee()
|| fee.hasHookWithdrawFee()
);
}
Expand Down Expand Up @@ -98,4 +108,12 @@ library Hooks {
function shouldCallAfterDonate(IHooks self) internal pure returns (bool) {
return uint256(uint160(address(self))) & AFTER_DONATE_FLAG != 0;
}

function shouldAccessLock(IHooks self) internal pure returns (bool) {
snreynolds marked this conversation as resolved.
Show resolved Hide resolved
return uint256(uint160(address(self))) & ACCESS_LOCK_FLAG != 0;
}

function shouldAllowOverride(IHooks self) internal pure returns (bool) {
return (uint256(uint160(address(self))) & OVERRIDE_FLAG) != 0;
}
}
37 changes: 37 additions & 0 deletions src/test/AccessLockHook.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.20;

import {BaseTestHooks} from "./BaseTestHooks.sol";
import {PoolKey} from "../types/PoolKey.sol";
import {IPoolManager} from "../interfaces/IPoolManager.sol";
import {IHooks} from "../interfaces/IHooks.sol";
import {CurrencyLibrary, Currency} from "../types/Currency.sol";
import {Hooks} from "../libraries/Hooks.sol";

import "forge-std/console2.sol";

contract AccessLockHook is BaseTestHooks {
using CurrencyLibrary for Currency;

IPoolManager manager;

constructor(IPoolManager _manager) {
manager = _manager;
}

function beforeModifyPosition(
address sender,
PoolKey calldata key,
IPoolManager.ModifyPositionParams calldata params,
bytes calldata hookData
) external override returns (bytes4) {
// just deal with positive deposits of currency1
(uint256 amount1) = abi.decode(hookData, (uint256));
manager.mint(key.currency1, address(this), amount1);
return Hooks.OVERRIDE_SELECTOR;
}

function onERC1155Received(address, address, uint256, uint256, bytes memory) public returns (bytes4) {
return this.onERC1155Received.selector;
}
}
4 changes: 3 additions & 1 deletion src/test/EmptyTestHooks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ contract EmptyTestHooks is IHooks {
beforeSwap: true,
afterSwap: true,
beforeDonate: true,
afterDonate: true
afterDonate: true,
accessLock: false,
overrideSelector: false
})
);
}
Expand Down
11 changes: 7 additions & 4 deletions src/test/PoolModifyPositionTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import {IPoolManager} from "../interfaces/IPoolManager.sol";
import {BalanceDelta} from "../types/BalanceDelta.sol";
import {PoolKey} from "../types/PoolKey.sol";
import {PoolTestBase} from "./PoolTestBase.sol";
import {Hooks} from "../libraries/Hooks.sol";

import "forge-std/console2.sol";
snreynolds marked this conversation as resolved.
Show resolved Hide resolved

contract PoolModifyPositionTest is PoolTestBase {
using CurrencyLibrary for Currency;
Expand Down Expand Up @@ -45,13 +48,13 @@ contract PoolModifyPositionTest is PoolTestBase {
if (data.params.liquidityDelta > 0) {
assert(delta0 > 0 || delta1 > 0);
assert(!(delta0 < 0 || delta1 < 0));
if (delta0 > 0) _settle(data.key.currency0, data.sender, delta.amount0(), true);
if (delta1 > 0) _settle(data.key.currency1, data.sender, delta.amount1(), true);
if (delta0 > 0) _settle(data.key.currency0, data.sender, int128(delta0), true);
if (delta1 > 0) _settle(data.key.currency1, data.sender, int128(delta1), true);
} else {
assert(delta0 < 0 || delta1 < 0);
assert(!(delta0 > 0 || delta1 > 0));
if (delta0 < 0) _take(data.key.currency0, data.sender, delta.amount0(), true);
if (delta1 < 0) _take(data.key.currency1, data.sender, delta.amount1(), true);
if (delta0 < 0) _take(data.key.currency0, data.sender, int128(delta0), true);
if (delta1 < 0) _take(data.key.currency1, data.sender, int128(delta1), true);
}

return abi.encode(delta);
Expand Down
Loading