-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(deps): resolve circular dependency that breaks clean builds #964
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
Conversation
This assumes Uniswap/v4-core#964 has been merged.
239aa9e
to
4c9fab1
Compare
src/types/PoolOperation.sol
Outdated
import {BalanceDelta} from "../types/BalanceDelta.sol"; | ||
|
||
/// @notice Common types used by both IPoolManager and IHooks | ||
interface PoolOperation { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason these need to be inside an interface? can the structs just be in the top level file and imported individually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if so this can stay in types
otherwise i think it should move to interfaces
probably?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on naming I'd maybe go PoolOperationParams
or something to show its the params that are being defined, its not defining the operations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah they could be top level rather than wrapped in an interface. Will make that change.
8cc4b41
to
0d2dc9a
Compare
This assumes Uniswap/v4-core#964 has been merged.
import {BalanceDelta} from "../types/BalanceDelta.sol"; | ||
|
||
/// @notice Parameter struct for `ModifyLiquidity` pool operations | ||
struct ModifyLiquidityParams { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you removed the interface, can we confirm that this still solves the issue? When I implemented this without the interface, it didn't fix the circular dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this does still solve the issue. Once these structs are moved out of IPoolManager
, the circular references between IHooks
and IPoolManager
are eliminated and the monorepo builds cleanly.
0d2dc9a
to
d75f10d
Compare
Signed-off-by: Chris Cashwell <[email protected]>
Signed-off-by: Chris Cashwell <[email protected]>
Signed-off-by: Chris Cashwell <[email protected]>
d75f10d
to
c083db7
Compare
Related Issue
This resolves an issue that causes builds to fail in certain conditions.
Description of changes
types/PoolOperation.sol
consisting of the structsModifyLiquidityParams
andSwapParams
which are removed frominterfaces/IPoolManager.sol