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

Skip calls to hooks when msg.sender is the hook contract #471

Closed
danrobinson opened this issue Feb 1, 2024 · 0 comments
Closed

Skip calls to hooks when msg.sender is the hook contract #471

danrobinson opened this issue Feb 1, 2024 · 0 comments
Assignees

Comments

@danrobinson
Copy link
Contributor

Component

Gas Optimization

Describe the suggested feature and problem it solves.

In some cases, DEXes built on v4 may want to have the hook contract make calls into the pool (to swap, to provide liquidity, to donate etc). Examples include:

  • Any design where the hook intercepts a user's trade and executes it on their behalf in multiple steps (such as to change fees or add or remove liquidity between ticks)
  • Any design where all calls must be gated through a particular contract (which might as well be the hook contract)
  • Any design where the hook manages its own liquidity, or manages liquidity for any users
  • Any design where the hook uses donate()

In those cases, I would argue that it never makes sense for the pool to do any callbacks to the hook during the operation. If there is any desired logic that should happen before or after the operation, the hook contract can perform that logic before or after calling, rather than during a reentrant call. Having the swap call back into the hook involves a wasteful contract call.

One motivating example is any design where the hook intercepts a user's trade in a beforeSwap hook (such as using the design in #463) and splits the trade into multiple pieces (in order to do something in between each tick, such as donating to the pool, updating some accumulator, or computing the proper swap size). In this case, each tick cross would involve a wasted CALL to the hook.

Describe the desired implementation.

Change the line here to:

if (key.hooks.hasPermission(BEFORE_SWAP_FLAG) && (msg.sender != address(self)))

And make similar changes for all other hook calls.

Describe alternatives.

No response

Additional context.

The main downside is the special-casing—it's possible it could be confusing for hook developers. On the other hand, I think it actually might be more intuitive in most cases—it automatically prevents a potentially infinite recursion when a beforeSwap hook calls swap, for example.

The main other cost is the additional gas of the check, but it should be on the order of ~10-20 gas per check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants