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

Define and Implement Dynamic Fee Update Event #926

Closed

Conversation

floor-licker
Copy link

Related Issue

#920

Description of changes

This PR declares a new event DynamicLPFeeUpdated within the IPoolManager contract (found in IPoolManager.sol) which is essentially an interface contract to be inherited by the PoolManager contract (found in PoolManager.sol). The new event is emitted whenever the function updateDynamicLPFee in is called, and is called on the last line of the function definition to ensure it is only called in the case that the function has executed properly.

In using

grep -rl "is IPoolManager" --include="*.sol" .

We can see that the only two contracts in the v4-core repo which inherit from IPoolManager are in ./src/test/ProxyPoolManager.sol and ./src/test/ProxyPoolManager.sol. I’ve implemented the event emissions appropriately in both cases.

In response to issue Uniswap#920. This event should then be emitted in every call to `updateDynamicLPFee` in contracts which inherit from `IPoolManager`
…r.sol

In response to Issue Uniswap#920. The event is declared in `IPoolManager.sol` which `PoolManager.sol` inherits from.
@floor-licker floor-licker requested a review from a team as a code owner December 12, 2024 20:49
@hensha256
Copy link
Contributor

Hi! We appreciate your contribution, however the PoolManager bytecode is frozen. We will not be able to make any more changes 🙏

@hensha256 hensha256 closed this Dec 13, 2024
@floor-licker
Copy link
Author

@hensha256 Ok makes sense. Is it safe to say that all the smart contracts in v4-core are in the same boat or just specifically that directory?

@kaber2
Copy link

kaber2 commented Jan 4, 2025

This event is essential to get a proper view of the pool state externally. Are you saying you'd rather release it in this very difficult to work with state instead of making this trivial change?

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.

3 participants