Uniswap / v4-periphery

🦄 🦄 🦄 🦄 Peripheral smart contracts for interacting with Uniswap v4
https://blog.uniswap.org/uniswap-v4
MIT License
716 stars 495 forks source link

[Bug]: Argument Mismatch in afterAddLiquidity and afterRemoveLiquidity Functions with v4-core #328

Closed DamirS09 closed 2 months ago

DamirS09 commented 2 months ago

Describe the bug

There is a discrepancy in the function signatures of afterAddLiquidity and afterRemoveLiquidity between the v4-periphery and v4-core repositories.

Expected Behavior

The function signatures in v4-periphery should be updated to match those in v4-core, including the BalanceDelta feesAccrued parameter. This ensures consistency and compatibility between the core and periphery layers.

To Reproduce

v4-core: afterAddLiquidity v4-periphery: afterAddLiquidity v4-core: afterRemoveLiquidity v4-periphery: afterRemoveLiquidity

Additional context

In v4-core, both functions take an additional parameter: BalanceDelta feesAccrued. The v4-periphery implementation lacks this parameter, which leads to incompatibility issues.

This discrepancy is present in the v4-periphery repository, specifically in the file: src/base/hooks/BaseHook.sol.

linear[bot] commented 2 months ago

PROTO-572 [Bug]: Argument Mismatch in afterAddLiquidity and afterRemoveLiquidity Functions with v4-core

hensha256 commented 2 months ago

Hi! Yes we have to manually update the core commit that is used in periphery. We changed core a few days ago so need to manually update in periphery. Will aim to get that in today or tomorrow for you :)

hensha256 commented 2 months ago

The update to core was created here https://github.com/Uniswap/v4-core/commit/3407bce4b39869fe41ad5ec724b2df308c34900f

DamirS09 commented 2 months ago

Hi! Yes we have to manually update the core commit that is used in periphery. We changed core a few days ago so need to manually update in periphery. Will aim to get that in today or tomorrow for you :)

Hi! Thank you for the quick response!

I appreciate the update and will look forward to the changes being implemented in the periphery repository. Thanks again! 😊

saucepoint commented 2 months ago

should be fixed now with:

https://github.com/Uniswap/v4-periphery/pull/337