feat: add lock target #300

marktoda · 2023-07-06T23:09:40Z

This commit adds a lockTarget parameter to lock where any caller can
specify a locker to handle the routing logic on their behalf. This can
allow safe storage via tstore and passing usage of the lock initiator
to hook callbacks

closes #298

Related Issue

Which issue does this pull request resolve?

Description of changes

marktoda · 2023-07-06T23:10:06Z

[WIP] needs tests

ewilz · 2023-07-07T02:04:04Z

contracts/PoolManager.sol

@@ -171,12 +171,12 @@ contract PoolManager is IPoolManager, Owned, NoDelegateCall, ERC1155, IERC1155Re
    }

    /// @inheritdoc IPoolManager
-    function lock(bytes calldata data) external override returns (bytes memory result) {
+    function lock(address lockTarget, bytes calldata data) external override returns (bytes memory result) {


I imagine we won't see the true gas changes this involves until the tests start calling PoolManager directly....I'd honestly expect gas to go down rather than up? I just dont' see a huge benefit tho, bc ultimately a router will need to call swap, so msg.sender through swap will still be a router..... am I missing something?

Yeah gas goes down about 2k since it avoids the extra call!

Basic idea is if a hook needs to know the swapper address that currently requires a trusted router to pass through the swapper data. With this, the swapper could just call the poolmanager directly and hook could use PM's msg.sender to determine the swapper

hook could use PM's msg.sender to determine the swapper

need a way to pass PM's msg.sender into hook. I've discussed one way here: https://www.notion.so/uniswaplabs/V4-Arbitrary-Data-msg-sender-73d4a2d3f43241049249af6278abd2d5?pvs=4#0327195ab2ca4827829e9d64c398a38b

With this, the swapper could just call the poolmanager directly and hook could use PM's msg.sender to determine the swapper

my confusion here is: the hook get's called inside swap, which is called by the router, NOT the EOA which called lock, so by the time we're calling the hook, the original msg.sender is lost.

I really didn't (don't) like the idea of tracking msg.sender in core, but now I'm realizing that it's so hard to do it from the router, bc a hook would have to be 4ever paired to a router which doesn't seem ideal. I think the t-storing the lock's msg.senderin core is not going to be straightforward, bc just like locker addr, it will have to be a recursive stack, and I'm still wondering whether we really want to support the permissioned usecase enough to merit the complexity...

That said I still think PR is cool and worth it, regardless of permissioned usecases!!

my confusion here is: the hook get's called inside swap, which is called by the router, NOT the EOA which called lock, so by the time we're calling the hook, the original msg.sender is lost.

Yes this is true. I addressed this issue above in #300 (comment).

I think the t-storing the lock's msg.sender in core is not going to be straightforward, bc just like locker addr, it will have to be recursive

I don't follow - can you be more specific? I believe the method I've described in #300 (comment) is sufficient.

marktoda · 2023-07-07T03:24:01Z

if we do this, future TODO is to tstore the actual msg.sender and pass it as a param to hook calls

azflin · 2023-07-07T15:34:08Z

contracts/interfaces/IPoolManager.sol

    /// @param data Any data to pass to the callback, via `ILockCallback(msg.sender).lockCallback(data)`
    /// @return The data returned by the call to `ILockCallback(msg.sender).lockCallback(data)`
-    function lock(bytes calldata data) external returns (bytes memory);
+    function lock(address lockTarget, bytes calldata data) external returns (bytes memory);


I thought your original idea was to add a new lock function, instead of modifying existing one. Why change of mind?

i always planned on just modifying the existing one, not a huge benefit of having a whole separate lock entrypoint that I can see?

Fair enough. Maybe tiny gas savings in calling lock with 1 param instead of 2 is the only benefit

test/foundry-tests/PoolManager.t.sol

NoahZinsmeister

my overall concern with this feature after reading the PR is that it doesn't actually guarantee that the lock caller is a "real user" and not a contract/router/whatever; in fact it locks users into a specific calling format if a hook is expecting this

i'm also worried that this isn't going to scale in an AA world and that hooks are just going to have to resort to other identification methods instead

imo we should add arbitrary calldata as a 1st class citizen and let that be the bailout for hooks that want this behavior, rather than adding this feature

marktoda · 2023-11-17T23:43:30Z

Seemingly pretty serious gas improvements:

simple swap: 203264
simple swap EOA initiated: 174949
simple swap native: 191605
simple swap EOA initiated native: 168498

marktoda · 2023-11-17T23:44:34Z

src/libraries/Lockers.sol

+        }
+    }
+
+    function getLockOriginator(uint256 i) internal view returns (address locker) {


name suggestions appreciated! I'm currently thinking maybe we should call this locker and call the other lockTarget

yeah heh I was confused by the name. so there are two addrs now

the address that implements the lockCallback (this is the locker) ?

the address that triggers the lock call (this is the lockCaller) ?

thats the best i could come up w though 😄

ahh lockCaller is better

marktoda · 2023-11-17T23:46:54Z

test/PoolManager.t.sol

+        );
+        snapEnd();
+    }
+


planning on adding some more tests here fyi

Yeah maybe a revert test with a lockTarget of an invalid address ? (address0, or addr that doesnt implement lockCallback)

src/interfaces/callback/ILockCallback.sol

snreynolds · 2023-11-20T15:30:12Z

src/libraries/Lockers.sol

    // The slot holding the number of nonzero deltas.
    uint256 constant NONZERO_DELTA_COUNT = uint256(keccak256("NonzeroDeltaCount")) - 1;

-    function push(address locker) internal {
+    // pushes an address tuple (address locker, address lockOriginator)
+    // to the locker array, so each length of the array represents 2 slots of tstorage


interesting.. So when we change this library this would be something like AddressStruct[] transient lockerInfo ?

Maybe we'd consider actually changing it to an address[] array and then a sep mapping. So we are not repeatedly saving 2 of the same addresses in the callback case (when originator == implementer)

wait wdym by a sep mapping? like from locker => lockOriginator? Don't we want it to be split up by lock though, not by locker? i.e. I can have multiple locks with the same locker but different lockOriginators

oh yeah thats true can have multiple of the same locker

snreynolds · 2023-11-20T23:37:49Z

src/libraries/Lockers.sol

+        }
+    }
+
+    function getLockOriginator(uint256 i) internal view returns (address locker) {


yeah heh I was confused by the name. so there are two addrs now

the address that implements the lockCallback (this is the locker) ?

the address that triggers the lock call (this is the lockCaller) ?

snreynolds · 2023-11-20T23:38:08Z

src/libraries/Lockers.sol

+        }
+    }
+
+    function getLockOriginator(uint256 i) internal view returns (address locker) {


thats the best i could come up w though 😄

snreynolds · 2023-11-20T23:46:21Z

test/PoolManager.t.sol

+        );
+        snapEnd();
+    }
+


Yeah maybe a revert test with a lockTarget of an invalid address ? (address0, or addr that doesnt implement lockCallback)

snreynolds · 2023-11-27T22:54:05Z

src/test/PoolInitializeTest.sol

    }

-    function lockAcquired(bytes calldata rawData) external returns (bytes memory) {
+    function lockAcquired(address _lockCaller, bytes calldata rawData) external returns (bytes memory) {


tiny tiny nit: just seeing some params as _lockCaller and some as lockCaller? I think either are fine but maybe we just pick one?

oh the underscore was for when it's not used.. ill just omit the name in that case

snreynolds

lgtm 🚢

hensha256 · 2023-12-04T21:35:16Z

src/PoolManager.sol

@@ -149,11 +149,11 @@ contract PoolManager is IPoolManager, Fees, NoDelegateCall, Claims {
    }

    /// @inheritdoc IPoolManager
-    function lock(bytes calldata data) external override returns (bytes memory result) {
-        Lockers.push(msg.sender);
+    function lock(address lockTarget, bytes calldata data) external payable override returns (bytes memory result) {


did you consider adding a if (msg.value > 0) settle() to the lock function?
definitely not needed but it would probably bring down gas further for our ETH input cases (which is a huge % of our volume)

it just feels weird that the ETH is sent in this function but is actually used in a different functions

* feat: add lock target * feat: store lockOriginator * fix: tests * feat: lockOriginator => lockCaller * feat: add invalid locker tests * fix: remove unused params

* forge install: erc-6909 * migrate 1155 to 6909 * rm old 6909 * forge install: erc-6909 * add event arg * rm old 6909 * forge install: erc-6909 * update test event * Add gas snaps * squash: support arbitrary calldata on test routers (#361) * Chore: update licenses (#364) * chore: update README * chore: update interface licenses * chore: update Hooks.sol license * chore: update types licenses * Migrate SwapMath tests to foundry (#363) * write SwapMath Tests * write gas snapshots tests * delete SwapMath hardhat implementation * eliminate SwapMathTest + add gas snapshots * delete js snapshots * migrate echidna test * forge fmt * test titles * remove console import --------- Co-authored-by: Job Mwitah <jobmwitah@gmail.com> Co-authored-by: Mwitah <136892656+MwitahJob@users.noreply.github.com> * add base hook for tests (#377) * change natspec to ILockCallback.lockAcquired (#376) * feat: update to solidity 0.8.22 (#378) - Enforce evm_version to avoid compiling push0 - Remove unchecked loops which are unchecked by default in 0.8.22 * Cache dynamic fee in slot0 (#360) * Bug: Require different currencies (#380) * Require different currencies * hardhat snapshots * Add new custom type function * remove extra paren * Replace MockERC20 with solmate's MockERC20 (#374) * rename MockERC20 -> UniMockERC20 * remove UniMockERC20 in favor of solmate/MockERC20 * update snapshots * Fixing compiler warnings (#386) * fix: add gas snaps for swaps with 1155 as input/output (#383) * Add snaps for 1155 swaps * remove lib * Add gas prefix * Delete Hardhat (#372) Co-authored-by: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> Co-authored-by: Alice <34962750+hensha256@users.noreply.github.com> Co-authored-by: Sara Reynolds <snreynolds2506@gmail.com> * feat: use standard forge directory structure (#389) * feat: standard foundry directory structure This commit moves to the standard foundry directory structure with contracts in src/ and tests in test/. * feat: remove JS stuff * fix: remove out dir * fix: workflows * fix: re-add js scripts * fix: add back js stuff * feat: yarn lock * fix: alice comments * Improve forge tests (#391) * Updating lots of tests * Fixed final test * remove console2 * mark PR comment * Fix Issue #397: Incorrect Documentation (#399) * feat: move whitepapers to docs dir to cleanup root (#393) * feat: move whitepapers to docs dir to cleanup root * fix: remove draft so links dont break * add snapshots to CI with tolerance (#401) * Implement Claims accounting as minimal balance (#379) * Add MinimalBalance * Initial commmit * Router custodies Claims, has access to priviledged burnFrom anbd tests * updategas * remove 6909 lib * yarn snapshots * Add gas snaps for swapping from claims balance * fix gas snaps by removing aux logic in router * gas * remove lib * Add transfer to minimalBalance, update tests * nit: rename * add back custom errors * move addition out of unchecked * Add transfer overflow check * Rename impl test * nit comments * comment# * Remove unused inheritance * remove comment * Remove poolClaimTest * fix interfaces * Feedback * Add address(0) and address(this) check for transfer * remove address(0) check * Remove batchBurn * Move mock claims to diff file * Add gas snaps for collect protocol fees * Add balance checks, make balances mapping private * Fix imports * fix fs perms * Remove uint256 in mapping and use Currency * feedback * Add gas snaps * fix: whitepaper link (#400) I accidentally broke the link from readme * Add solc binaries and transient storage lock library (#395) * Updated solc setup and instructions (#406) * Added contribution instructions * missing space * Updated solc config * feat: move JS scripts to subdir and add helper (#405) This commit moves our JS helper scripts into a test subdirectory and adds a helper abstract contract to more easily build the ffi commands to interop with javascript testers * Set tick spacing range as constants (#369) * Set tick spacing range as constants While tick and tick spacing both use int24 as their type, each has a different range. Tick spacing has a range of [1, 32767]. This commit updates Tick test cases to use proper tick spacing range instead that of tick. Resolves issue #371 * Restore a unit test on tick spacing liquidity This commit adds back the unit test that checks for tick spacing liquidity given the entire tick range as the input argument. This is an alternative change mentioned on the issue referred below. resolves #369 * Remove duplicate constants from test suite This commit moves MIN_TICK, MAX_TICK, MIN_TICK_SPACING, and MAX_TICK_SPACING constants from test suite constants file to TickMath library. Previous to this commit, TickMath library declared MIN_TICK and MAX_TICK constants with the same value from the test suite constants file. Removing duplicate constants from the test file and referencing them from the production file prevents future dicrepancies between production and test environments. * Remove unused import * Remove unnecessary override keywords * Update forge snapshots * Part 1: Improve forge tests (#407) * Revert messages * Common take and settle contract for tests * improving swap and take tests * add asserts for modify position * extra asserts in modify position * asserts in donate test * More deployment helpers * native set up in deployer * simplify initialize tests * few more corrections * more cleanup * remove console logs * snapshots and test fix post merge * snapshots * accidentally pushed foundry.toml * PR comments * remove forge snapshot --check (#417) * feat: update justfile with custom solc (#418) For folks who dont want to update their global env they can use `just test` or `just build` which sets solc using cli arg * Fix typos (#365) * Fix typos * Fix typo in src/libraries/Lockers.sol * Update just prep and build (#421) * udpate prep and add build * Update justfile * fix tests * remove totalsupply * remove lib * remove solmate * forge install: solmate main * Add comment * Add solmate 6909 and remove claims * feat: add variadic args to justfile (#423) allows to pass on args to forge i.e. `forge test --mt fuzz` * move up settle and remove solmate * forge install: solmate * copy erc6909 locally and use _mint and _burn * forge fmt * remove lib * forge install: solmate v6 * rmeove solmate * forge install: solmate 2001af43ae * fix gas snaps * remove unchecked without totalSupply * Lock on initialize (#424) * Lock on initialize * Rename initialize error * fix CI fuzz edge case * NoOp implementation with flag (#324) * cherrypicking * tests running and update snaps * test supoprt * format and run hardhat tests * remove irrelevant comment * test noops * add tests for noop * lint and snapshots * fix hook tests * clean * foundry toml * helper function * Revert early if pool isnt initialized * Extra tests * Tests NoOps on disallowed hooks fail * linting * snapshots * helper function for initialized pool * PR comments * PR comment test coverage * Final PR comments * comments about sentinel value * Fix masking tests * decrease calls to assume --------- Co-authored-by: Alex <dalexwatts@gmail.com> Co-authored-by: hensha256 <henshawalice@gmail.com> * update testss * rm solmate * forge install: solmate main * Use solmate 6909 * fmt * cache msg sender * Explain different solc options (#429) * explain solc options * Add reference in readme * random linting issue * Access lock (#404) * Fix broken tests that were using old claims balance format * Add V46909 and reorder params * Add V46909 and Mock contract for test * Add revert tests * burnFrom internal * remove old gas snaps and add native tests * feat: add lock target (#300) * feat: add lock target * feat: store lockOriginator * fix: tests * feat: lockOriginator => lockCaller * feat: add invalid locker tests * fix: remove unused params * fix tests and snaps * forge fmt * expectEmit() all * expect emit all * Prevent ProtocolFeeController from bricking pool initialization on revert (#362) * Default protocol fees to 0 if protocolFeeController reverts * snapshots * fix comment * add another malicious contract * remove useless var assignment * make call and decode return in assembly * fix typo in var assingmenet * clean up tests * update snapshots# * check withdrawFee == 0 on tests too * add test * separate out fetchProtocolFees and checkProtocolFees * manually revert in setProtocolFees * Add success return value to fetchProtocolFEes * Add test for FeeTooLarge passing on initialzie but not on setProtocolFees * check low level call success * clean up * add comment * simplify * udpate snaps * fix compiler warnings * fix result type * fix formatting * Add more descriptive error message * Change error name again * Add comments * update gas specs * fix merge conflict t4ests * fix tests * Add tests for setProtocolFee with invalid controllers * Add missing snaps * Feedback changes * fix initialize tests * fix fmt * Fix comment * comments and revise order --------- Co-authored-by: Sara Reynolds <snreynolds2506@gmail.com> * feedback * Add IERC69009, does not compile * Revert "Add IERC69009, does not compile" This reverts commit 4e89408. * added delta overflow checks (#433) * add pool getters (#438) * add getters * update snaps * clean test * A few cleanup tasks (#437) * Fix compiler warnings * todo for mapping transient * fixing tests with fuzzing * remove console logs * Update PoolDonateTest.sol * remove amount overload --------- Co-authored-by: Sara Reynolds <snreynolds2506@gmail.com> * Remove hook fees and protocol fee on withdrawal (#432) * Remove hook fees and protocol fee on withdrawal * hook fee tests * Update AccessLockHook.sol * assume -> bound * more comments on fee grnaularity * refactor: hooks callsites (#439) * feat: hooks refactor * fix: tests * feat: noop -> shouldExecute * fix: remove shouldCall functions for helper * fix: using for in test * fix: alice comment * merge conflicts * fix fialing test * Copy solmate 6909 locally to get interface inheritance * natspec * expose getters in pool interface * Use id for mint/burn instead of currency * forge fmt * Add burnFrom no approval test * feedback * feedback * forge install: ERC-6909 main * remove lib * Add commit SHA * merge conflicts galore * forge install: solmate 4b47a19038b798b4a33d9749d25e570443520647 * fix conflict * fix libs * del * forge install: openzeppelin-contracts v4.4.2 * fix tests from conflcits and gas * forge fmt * removed unused file * fix: faling fuzz tests (#441) * Add failing fuzz test * Add bounds * added out of range checks * changed assume to bound * forge fmt * changed Position -> Liquidity on merge main * moved amount helper to utils --------- Co-authored-by: Austin Adams <austino256@gmail.com> * fix gas snap * forge test * feedback --------- Co-authored-by: jtriley.eth <Jtriley15@gmail.com> Co-authored-by: saucepoint <98790946+saucepoint@users.noreply.github.com> Co-authored-by: Erin Koen <46381469+erin-koen@users.noreply.github.com> Co-authored-by: Emily Williams <emag3m@gmail.com> Co-authored-by: Job Mwitah <jobmwitah@gmail.com> Co-authored-by: Mwitah <136892656+MwitahJob@users.noreply.github.com> Co-authored-by: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> Co-authored-by: Ed Mazurek <Edward.R.Mazurek@gmail.com> Co-authored-by: marktoda <toda.mark@gmail.com> Co-authored-by: Alice <34962750+hensha256@users.noreply.github.com> Co-authored-by: Sara Reynolds <snreynolds2506@gmail.com> Co-authored-by: Jose Carlos Montero Gomez <sipox11@gmail.com> Co-authored-by: hyunchel <3271191+hyunchel@users.noreply.github.com> Co-authored-by: xiaolou86 <20718693+xiaolou86@users.noreply.github.com> Co-authored-by: emma <57429431+emmaguo13@users.noreply.github.com> Co-authored-by: Alex <dalexwatts@gmail.com> Co-authored-by: hensha256 <henshawalice@gmail.com> Co-authored-by: Austin Adams <36116882+aadams@users.noreply.github.com> Co-authored-by: Austin Adams <austino256@gmail.com>

ewilz · 2024-01-31T19:03:57Z

are gas savings the only reason for this PR? bc I imagine we'll inevitably go through the router anyway since we tend to aggregate across versions, so I imagine this will actually be a gas increase overall for our app? UNless we want to special case v4-only swaps and save for that subset of swaps... But for aggregators etc, gas goes up

* feat: add lock target * feat: store lockOriginator * fix: tests * feat: lockOriginator => lockCaller * feat: add invalid locker tests * fix: remove unused params

* forge install: erc-6909 * migrate 1155 to 6909 * rm old 6909 * forge install: erc-6909 * add event arg * rm old 6909 * forge install: erc-6909 * update test event * Add gas snaps * squash: support arbitrary calldata on test routers (Uniswap#361) * Chore: update licenses (Uniswap#364) * chore: update README * chore: update interface licenses * chore: update Hooks.sol license * chore: update types licenses * Migrate SwapMath tests to foundry (Uniswap#363) * write SwapMath Tests * write gas snapshots tests * delete SwapMath hardhat implementation * eliminate SwapMathTest + add gas snapshots * delete js snapshots * migrate echidna test * forge fmt * test titles * remove console import --------- Co-authored-by: Job Mwitah <jobmwitah@gmail.com> Co-authored-by: Mwitah <136892656+MwitahJob@users.noreply.github.com> * add base hook for tests (Uniswap#377) * change natspec to ILockCallback.lockAcquired (Uniswap#376) * feat: update to solidity 0.8.22 (Uniswap#378) - Enforce evm_version to avoid compiling push0 - Remove unchecked loops which are unchecked by default in 0.8.22 * Cache dynamic fee in slot0 (Uniswap#360) * Bug: Require different currencies (Uniswap#380) * Require different currencies * hardhat snapshots * Add new custom type function * remove extra paren * Replace MockERC20 with solmate's MockERC20 (Uniswap#374) * rename MockERC20 -> UniMockERC20 * remove UniMockERC20 in favor of solmate/MockERC20 * update snapshots * Fixing compiler warnings (Uniswap#386) * fix: add gas snaps for swaps with 1155 as input/output (Uniswap#383) * Add snaps for 1155 swaps * remove lib * Add gas prefix * Delete Hardhat (Uniswap#372) Co-authored-by: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> Co-authored-by: Alice <34962750+hensha256@users.noreply.github.com> Co-authored-by: Sara Reynolds <snreynolds2506@gmail.com> * feat: use standard forge directory structure (Uniswap#389) * feat: standard foundry directory structure This commit moves to the standard foundry directory structure with contracts in src/ and tests in test/. * feat: remove JS stuff * fix: remove out dir * fix: workflows * fix: re-add js scripts * fix: add back js stuff * feat: yarn lock * fix: alice comments * Improve forge tests (Uniswap#391) * Updating lots of tests * Fixed final test * remove console2 * mark PR comment * Fix Issue Uniswap#397: Incorrect Documentation (Uniswap#399) * feat: move whitepapers to docs dir to cleanup root (Uniswap#393) * feat: move whitepapers to docs dir to cleanup root * fix: remove draft so links dont break * add snapshots to CI with tolerance (Uniswap#401) * Implement Claims accounting as minimal balance (Uniswap#379) * Add MinimalBalance * Initial commmit * Router custodies Claims, has access to priviledged burnFrom anbd tests * updategas * remove 6909 lib * yarn snapshots * Add gas snaps for swapping from claims balance * fix gas snaps by removing aux logic in router * gas * remove lib * Add transfer to minimalBalance, update tests * nit: rename * add back custom errors * move addition out of unchecked * Add transfer overflow check * Rename impl test * nit comments * comment# * Remove unused inheritance * remove comment * Remove poolClaimTest * fix interfaces * Feedback * Add address(0) and address(this) check for transfer * remove address(0) check * Remove batchBurn * Move mock claims to diff file * Add gas snaps for collect protocol fees * Add balance checks, make balances mapping private * Fix imports * fix fs perms * Remove uint256 in mapping and use Currency * feedback * Add gas snaps * fix: whitepaper link (Uniswap#400) I accidentally broke the link from readme * Add solc binaries and transient storage lock library (Uniswap#395) * Updated solc setup and instructions (Uniswap#406) * Added contribution instructions * missing space * Updated solc config * feat: move JS scripts to subdir and add helper (Uniswap#405) This commit moves our JS helper scripts into a test subdirectory and adds a helper abstract contract to more easily build the ffi commands to interop with javascript testers * Set tick spacing range as constants (Uniswap#369) * Set tick spacing range as constants While tick and tick spacing both use int24 as their type, each has a different range. Tick spacing has a range of [1, 32767]. This commit updates Tick test cases to use proper tick spacing range instead that of tick. Resolves issue Uniswap#371 * Restore a unit test on tick spacing liquidity This commit adds back the unit test that checks for tick spacing liquidity given the entire tick range as the input argument. This is an alternative change mentioned on the issue referred below. resolves Uniswap#369 * Remove duplicate constants from test suite This commit moves MIN_TICK, MAX_TICK, MIN_TICK_SPACING, and MAX_TICK_SPACING constants from test suite constants file to TickMath library. Previous to this commit, TickMath library declared MIN_TICK and MAX_TICK constants with the same value from the test suite constants file. Removing duplicate constants from the test file and referencing them from the production file prevents future dicrepancies between production and test environments. * Remove unused import * Remove unnecessary override keywords * Update forge snapshots * Part 1: Improve forge tests (Uniswap#407) * Revert messages * Common take and settle contract for tests * improving swap and take tests * add asserts for modify position * extra asserts in modify position * asserts in donate test * More deployment helpers * native set up in deployer * simplify initialize tests * few more corrections * more cleanup * remove console logs * snapshots and test fix post merge * snapshots * accidentally pushed foundry.toml * PR comments * remove forge snapshot --check (Uniswap#417) * feat: update justfile with custom solc (Uniswap#418) For folks who dont want to update their global env they can use `just test` or `just build` which sets solc using cli arg * Fix typos (Uniswap#365) * Fix typos * Fix typo in src/libraries/Lockers.sol * Update just prep and build (Uniswap#421) * udpate prep and add build * Update justfile * fix tests * remove totalsupply * remove lib * remove solmate * forge install: solmate main * Add comment * Add solmate 6909 and remove claims * feat: add variadic args to justfile (Uniswap#423) allows to pass on args to forge i.e. `forge test --mt fuzz` * move up settle and remove solmate * forge install: solmate * copy erc6909 locally and use _mint and _burn * forge fmt * remove lib * forge install: solmate v6 * rmeove solmate * forge install: solmate 2001af43ae * fix gas snaps * remove unchecked without totalSupply * Lock on initialize (Uniswap#424) * Lock on initialize * Rename initialize error * fix CI fuzz edge case * NoOp implementation with flag (Uniswap#324) * cherrypicking * tests running and update snaps * test supoprt * format and run hardhat tests * remove irrelevant comment * test noops * add tests for noop * lint and snapshots * fix hook tests * clean * foundry toml * helper function * Revert early if pool isnt initialized * Extra tests * Tests NoOps on disallowed hooks fail * linting * snapshots * helper function for initialized pool * PR comments * PR comment test coverage * Final PR comments * comments about sentinel value * Fix masking tests * decrease calls to assume --------- Co-authored-by: Alex <dalexwatts@gmail.com> Co-authored-by: hensha256 <henshawalice@gmail.com> * update testss * rm solmate * forge install: solmate main * Use solmate 6909 * fmt * cache msg sender * Explain different solc options (Uniswap#429) * explain solc options * Add reference in readme * random linting issue * Access lock (Uniswap#404) * Fix broken tests that were using old claims balance format * Add V46909 and reorder params * Add V46909 and Mock contract for test * Add revert tests * burnFrom internal * remove old gas snaps and add native tests * feat: add lock target (Uniswap#300) * feat: add lock target * feat: store lockOriginator * fix: tests * feat: lockOriginator => lockCaller * feat: add invalid locker tests * fix: remove unused params * fix tests and snaps * forge fmt * expectEmit() all * expect emit all * Prevent ProtocolFeeController from bricking pool initialization on revert (Uniswap#362) * Default protocol fees to 0 if protocolFeeController reverts * snapshots * fix comment * add another malicious contract * remove useless var assignment * make call and decode return in assembly * fix typo in var assingmenet * clean up tests * update snapshots# * check withdrawFee == 0 on tests too * add test * separate out fetchProtocolFees and checkProtocolFees * manually revert in setProtocolFees * Add success return value to fetchProtocolFEes * Add test for FeeTooLarge passing on initialzie but not on setProtocolFees * check low level call success * clean up * add comment * simplify * udpate snaps * fix compiler warnings * fix result type * fix formatting * Add more descriptive error message * Change error name again * Add comments * update gas specs * fix merge conflict t4ests * fix tests * Add tests for setProtocolFee with invalid controllers * Add missing snaps * Feedback changes * fix initialize tests * fix fmt * Fix comment * comments and revise order --------- Co-authored-by: Sara Reynolds <snreynolds2506@gmail.com> * feedback * Add IERC69009, does not compile * Revert "Add IERC69009, does not compile" This reverts commit 4e89408. * added delta overflow checks (Uniswap#433) * add pool getters (Uniswap#438) * add getters * update snaps * clean test * A few cleanup tasks (Uniswap#437) * Fix compiler warnings * todo for mapping transient * fixing tests with fuzzing * remove console logs * Update PoolDonateTest.sol * remove amount overload --------- Co-authored-by: Sara Reynolds <snreynolds2506@gmail.com> * Remove hook fees and protocol fee on withdrawal (Uniswap#432) * Remove hook fees and protocol fee on withdrawal * hook fee tests * Update AccessLockHook.sol * assume -> bound * more comments on fee grnaularity * refactor: hooks callsites (Uniswap#439) * feat: hooks refactor * fix: tests * feat: noop -> shouldExecute * fix: remove shouldCall functions for helper * fix: using for in test * fix: alice comment * merge conflicts * fix fialing test * Copy solmate 6909 locally to get interface inheritance * natspec * expose getters in pool interface * Use id for mint/burn instead of currency * forge fmt * Add burnFrom no approval test * feedback * feedback * forge install: ERC-6909 main * remove lib * Add commit SHA * merge conflicts galore * forge install: solmate 4b47a19038b798b4a33d9749d25e570443520647 * fix conflict * fix libs * del * forge install: openzeppelin-contracts v4.4.2 * fix tests from conflcits and gas * forge fmt * removed unused file * fix: faling fuzz tests (Uniswap#441) * Add failing fuzz test * Add bounds * added out of range checks * changed assume to bound * forge fmt * changed Position -> Liquidity on merge main * moved amount helper to utils --------- Co-authored-by: Austin Adams <austino256@gmail.com> * fix gas snap * forge test * feedback --------- Co-authored-by: jtriley.eth <Jtriley15@gmail.com> Co-authored-by: saucepoint <98790946+saucepoint@users.noreply.github.com> Co-authored-by: Erin Koen <46381469+erin-koen@users.noreply.github.com> Co-authored-by: Emily Williams <emag3m@gmail.com> Co-authored-by: Job Mwitah <jobmwitah@gmail.com> Co-authored-by: Mwitah <136892656+MwitahJob@users.noreply.github.com> Co-authored-by: Sara Reynolds <30504811+snreynolds@users.noreply.github.com> Co-authored-by: Ed Mazurek <Edward.R.Mazurek@gmail.com> Co-authored-by: marktoda <toda.mark@gmail.com> Co-authored-by: Alice <34962750+hensha256@users.noreply.github.com> Co-authored-by: Sara Reynolds <snreynolds2506@gmail.com> Co-authored-by: Jose Carlos Montero Gomez <sipox11@gmail.com> Co-authored-by: hyunchel <3271191+hyunchel@users.noreply.github.com> Co-authored-by: xiaolou86 <20718693+xiaolou86@users.noreply.github.com> Co-authored-by: emma <57429431+emmaguo13@users.noreply.github.com> Co-authored-by: Alex <dalexwatts@gmail.com> Co-authored-by: hensha256 <henshawalice@gmail.com> Co-authored-by: Austin Adams <36116882+aadams@users.noreply.github.com> Co-authored-by: Austin Adams <austino256@gmail.com>

marktoda marked this pull request as draft July 6, 2023 23:09

marktoda force-pushed the add-lock-targets branch 2 times, most recently from 4ac5895 to 9887255 Compare July 6, 2023 23:13

ewilz reviewed Jul 7, 2023

View reviewed changes

marktoda force-pushed the add-lock-targets branch from e7ef569 to 69c7b30 Compare July 7, 2023 03:24

marktoda marked this pull request as ready for review July 7, 2023 03:24

azflin reviewed Jul 7, 2023

View reviewed changes

emmaguo13 reviewed Jul 7, 2023

View reviewed changes

test/foundry-tests/PoolManager.t.sol Outdated Show resolved Hide resolved

NoahZinsmeister reviewed Jul 11, 2023

View reviewed changes

feat: add lock target

675fd8a

marktoda force-pushed the add-lock-targets branch from 69c7b30 to 675fd8a Compare November 14, 2023 19:50

marktoda added 2 commits November 17, 2023 17:27

Merge branch 'main' into add-lock-targets

a4e83aa

feat: store lockOriginator

ce09370

marktoda force-pushed the add-lock-targets branch 3 times, most recently from 0e54322 to d4703e8 Compare November 17, 2023 23:43

marktoda commented Nov 17, 2023

View reviewed changes

marktoda force-pushed the add-lock-targets branch from d4703e8 to c824c2d Compare November 17, 2023 23:45

fix: tests

81350fa

marktoda force-pushed the add-lock-targets branch from c824c2d to 81350fa Compare November 17, 2023 23:45

marktoda commented Nov 17, 2023

View reviewed changes

snreynolds requested changes Nov 20, 2023

View reviewed changes

feat: lockOriginator => lockCaller

d7fd9b8

marktoda force-pushed the add-lock-targets branch from 0fb0438 to d7fd9b8 Compare November 21, 2023 22:53

marktoda added 3 commits November 21, 2023 17:54

Merge branch 'main' into add-lock-targets

6c390b2

feat: add invalid locker tests

1c523ff

Merge remote-tracking branch 'origin/main' into add-lock-targets

31fe369

marktoda force-pushed the add-lock-targets branch from 086b8b2 to 31fe369 Compare November 27, 2023 19:15

snreynolds reviewed Nov 29, 2023

View reviewed changes

marktoda added 2 commits December 4, 2023 13:01

Merge remote-tracking branch 'origin/main' into add-lock-targets

73b954d

fix: remove unused params

aeb1f6e

marktoda requested a review from snreynolds December 4, 2023 19:20

snreynolds approved these changes Dec 4, 2023

View reviewed changes

marktoda merged commit b9b0d2a into main Dec 4, 2023
4 checks passed

marktoda deleted the add-lock-targets branch December 4, 2023 20:59

hensha256 reviewed Dec 4, 2023

View reviewed changes

zhongeric pushed a commit that referenced this pull request Dec 14, 2023

feat: add lock target (#300)

0ea1932

* feat: add lock target * feat: store lockOriginator * fix: tests * feat: lockOriginator => lockCaller * feat: add invalid locker tests * fix: remove unused params

zhongeric pushed a commit that referenced this pull request Dec 14, 2023

feat: add lock target (#300)

22d4964

* feat: add lock target * feat: store lockOriginator * fix: tests * feat: lockOriginator => lockCaller * feat: add invalid locker tests * fix: remove unused params

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add lock target #300

feat: add lock target #300

marktoda commented Jul 6, 2023

marktoda commented Jul 6, 2023

ewilz Jul 7, 2023 •

edited

marktoda Jul 7, 2023

azflin Jul 7, 2023

ewilz Jul 10, 2023 •

edited

azflin Jul 10, 2023 •

edited

marktoda commented Jul 7, 2023

azflin Jul 7, 2023

marktoda Jul 7, 2023

azflin Jul 7, 2023

NoahZinsmeister left a comment

marktoda commented Nov 17, 2023

marktoda Nov 17, 2023

snreynolds Nov 20, 2023

snreynolds Nov 20, 2023

marktoda Nov 21, 2023

marktoda Nov 17, 2023

snreynolds Nov 20, 2023

snreynolds Nov 20, 2023

snreynolds Nov 20, 2023

marktoda Nov 21, 2023

snreynolds Nov 27, 2023

snreynolds Nov 20, 2023

snreynolds Nov 20, 2023

snreynolds Nov 20, 2023

snreynolds Nov 27, 2023

marktoda Dec 4, 2023 •

edited

snreynolds left a comment

hensha256 Dec 4, 2023

hensha256 Dec 4, 2023

ewilz commented Jan 31, 2024 •

edited

feat: add lock target #300

feat: add lock target #300

Conversation

marktoda commented Jul 6, 2023

Related Issue

Description of changes

marktoda commented Jul 6, 2023

ewilz Jul 7, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewilz Jul 10, 2023 • edited

Choose a reason for hiding this comment

azflin Jul 10, 2023 • edited

Choose a reason for hiding this comment

marktoda commented Jul 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NoahZinsmeister left a comment

Choose a reason for hiding this comment

marktoda commented Nov 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marktoda Dec 4, 2023 • edited

Choose a reason for hiding this comment

snreynolds left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewilz commented Jan 31, 2024 • edited

ewilz Jul 7, 2023 •

edited

ewilz Jul 10, 2023 •

edited

azflin Jul 10, 2023 •

edited

marktoda Dec 4, 2023 •

edited

ewilz commented Jan 31, 2024 •

edited