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

Update v4 core #74

Merged
merged 13 commits into from
Dec 19, 2023
Merged

Update v4 core #74

merged 13 commits into from
Dec 19, 2023

Conversation

marktoda
Copy link
Contributor

Related Issue

Which issue does this pull request resolve?

Description of changes

This commit updates v4 core to latest and fixes integration issues
test/TWAMM.t.sol Outdated
assertEq(earningsFactor0For1, 1712020976636017581269515821040000);
assertEq(earningsFactor1For0, 1470157410324350030712806974476955);
assertEq(earningsFactor0For1, 1636776489931663248324424309240000);
assertEq(earningsFactor1For0, 1534530274609724617872321172427618);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is sus - I'm not sure what could have changed to cause these values to be different..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is odd....

@marktoda marktoda marked this pull request as ready for review November 17, 2023 22:06
Copy link
Contributor

@hensha256 hensha256 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some thoughts

.env Outdated Show resolved Hide resolved
test/FullRange.t.sol Outdated Show resolved Hide resolved
@@ -102,17 +101,14 @@ contract TestFullRange is Test, Deployers, GasSnapshot {
keyWithLiq = createPoolKey(token0, token2);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is createPoolKey from? we dont have that function in core 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's defined in this file, it was here when i got here


manager.initialize(keyWithLiq, SQRT_RATIO_1_1, ZERO_BYTES);
initPool(keyWithLiq.currency0, keyWithLiq.currency1, fullRange, 3000, SQRT_RATIO_1_1, ZERO_BYTES);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fullRange hook address doesnt seem to be in the keyWithLiq key before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it is... in createPoolKey

test/TWAMM.t.sol Outdated Show resolved Hide resolved
@marktoda marktoda requested a review from hensha256 November 21, 2023 16:16
@@ -204,8 +204,12 @@ contract LimitOrder is BaseHook {
ZERO_BYTES
);

if (delta.amount0() < 0) poolManager.mint(key.currency0, address(this), amount0 = uint128(-delta.amount0()));
if (delta.amount1() < 0) poolManager.mint(key.currency1, address(this), amount1 = uint128(-delta.amount1()));
if (delta.amount0() < 0) {
Copy link
Member

@snreynolds snreynolds Nov 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that a change is needed, bc we'd probably need other checks in place to support this but I think this code no longer works on custom accounting/access-lock-enabled hooks

@@ -0,0 +1,70 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.20;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this? You can pass in hook data with PoolSwapTest

test/TWAMM.t.sol Outdated
assertEq(earningsFactor0For1, 1712020976636017581269515821040000);
assertEq(earningsFactor1For0, 1470157410324350030712806974476955);
assertEq(earningsFactor0For1, 1636776489931663248324424309240000);
assertEq(earningsFactor1For0, 1534530274609724617872321172427618);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

test/TWAMM.t.sol Outdated
assertEq(earningsFactor0For1, 1712020976636017581269515821040000);
assertEq(earningsFactor1For0, 1470157410324350030712806974476955);
assertEq(earningsFactor0For1, 1636776489931663248324424309240000);
assertEq(earningsFactor1For0, 1534530274609724617872321172427618);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is odd....

@saucepoint
Copy link
Collaborator

any chance we can include a change like this #26 for this PR?

Seeing some devs run into problems regarding this

@marktoda
Copy link
Contributor Author

marktoda commented Dec 11, 2023

any chance we can include a change like this #26 for this PR?

ah good idea, can be a separate pr tho

@snreynolds
Copy link
Member

Not sure why linting is failing ❓

contracts/hooks/examples/FullRange.sol Outdated Show resolved Hide resolved
@@ -62,7 +63,7 @@ library PoolGetters {
// all the 1s at or to the right of the current bitPos
uint256 mask = (1 << bitPos) - 1 + (1 << bitPos);
// uint256 masked = self[wordPos] & mask;
uint256 masked = getTickBitmapAtWord(poolManager, poolId, wordPos) & mask;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can probably remove getTickBitmapAtWord btw

Copy link
Contributor Author

@marktoda marktoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cant approve my own PR :D think fine to merge - we def have some cleanup to do here but can be incremental

@snreynolds snreynolds merged commit 0b46125 into main Dec 19, 2023
2 checks passed
@snreynolds snreynolds deleted the update-v4-core branch December 19, 2023 15:11
ChefSnoopy added a commit to ChefSnoopy/v4-periphery that referenced this pull request Mar 10, 2024
* Update to v4-core latest (Uniswap#64)

* first pass on using new router function singatures

* updated v4-core

* updated .getSlot0, as it returns less data now

* snapshots

* add base contracts and interfaces (Uniswap#75)

* Update v4 core (Uniswap#74)

* feat: update v4-core

This commit updates v4 core to latest and fixes integration issues

* fix: tests

* fix: update tests

* fix: test router was borked

* fix: alice comments

* update to latest core

* use prev values

* change twamm to use pool getters

* changes after merging main

* use --via-ir in cli

* fix formatting

* fix FullRange/TWAMM hook

---------

Co-authored-by: Sara Reynolds <[email protected]>

* feat: Revert style quoter (Uniswap#73)

* add PoolTicksCounter library

* quoter exact input single

* quoter test

* return deltas instead

* safe casting to correct types

* QuoteExactInput skeleton

* multiple entries

* break handleRevert by type

* quoteExactInput and unit tests

* more QuoteExactInput tests

* remove lgos

* remove commented out struct

* via-ir in ci

* remove unused imports/functions

* store iteration params locally instead of editing function input

* pull out sqrtPriceLimit to its own function

* PathKey to its own library

* rename initializedTicksCrossed to initializedTicksLoaded

* remove manual abi encoding in yul :p

* fix linter warnings for Quoter

* natspec for IQuoter

* feat: update v4-core

This commit updates v4 core to latest and fixes integration issues

* fix: tests

* style fixes

* inheritdoc

* ExactInSingleBatch

* fix: update tests

* fix: test router was borked

* exact out

* fix: alice comments

* fix ExactOutput

* add ExactOput unit tests

* add quoteExactOutputBatch

* remove solhint config

* remove newline

* add QuoteExactOutput in interface

* refactor lockAcquired

* move magic numbers to constants + doc

* add more natspec

* natspec

* named imports

* self-call branching

* remove old code

* remove console2 import

* refactor PathKeyLib

* amountOutCached

* inherit ILockCallback

* add base contracts and interfaces (Uniswap#75)

* remove unused errors

* test lockAcquired reverts

* remove ...Batch interface

* REASON -> RESPONSE when valid

* complete natspec

* remove SwapInfo imports

* rename to SwapParameters

* move quoter structs into IQuoter interface

* update to latest core

* use prev values

* change twamm to use pool getters

* changes after merging main

* use --via-ir in cli

* fix formatting

* fix FullRange/TWAMM hook

* update ticks counter

* update Quoter test

* typo

* typo

* simplify handleRevertSingle

* merge QuoteInput/OutputSingle structs

* combine IQuoter structs

* using ... ordering

* update snapshots

* move amountOutCached into inner call

* using PathKeyLib for PathKey

* fix amountOutCached

* remove console2 import

* resurface revert reason

* clean up validateRevert

* update natsppec

* remove unused

---------

Co-authored-by: Mark Toda <[email protected]>
Co-authored-by: Tina <[email protected]>
Co-authored-by: Sara Reynolds <[email protected]>

* (Quoter) Avoid IR (Uniswap#93)

* avoid stack too deep

* pack local variables into structs; remove need for IR

* reorg struct

* snapshots

* forge fmt

* restore settings

* remove IR

* ensure tokens are ordered properly by using salts

* gas snapshot

* remove console logs

* chore: update v4-core:latest (Uniswap#89)

* update v4-core

* update to new liquidity hooks

* forge fmt; reuse v4-core justfile

* snapshots

* rename getHooksCalls --> getHookPermissions

* enforce permanent liquidity with beforeRemoveLiquidity

* snapshot

* update v4-core (again)

* snapshots with new v4-core

* v4-core:latest

* pin 0.8.24

* merge in remote; regenerate snapshots

* remove justfile

* repin cancun

* pin token addresses using vm.etch

* snapshots

* forge fmt

* remove via-ir and custom solc from CI

* test nit

* Update v4-core submodule to use https (Uniswap#97)

Co-authored-by: saucepoint <[email protected]>

* chore: add semgrep (Uniswap#94)

---------

Co-authored-by: saucepoint <[email protected]>
Co-authored-by: Tina <[email protected]>
Co-authored-by: marktoda <[email protected]>
Co-authored-by: Sara Reynolds <[email protected]>
Co-authored-by: Zach Yang <[email protected]>
Co-authored-by: 0x57 <[email protected]>
Co-authored-by: mr-uniswap <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Modify Hooks.Calls into Hooks.Permissions
5 participants