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

Add Simplified Multi-tick donate #465

Closed
wants to merge 0 commits into from
Closed

Conversation

Philogy
Copy link
Contributor

@Philogy Philogy commented Jan 27, 2024

Related Issue

#346

Description of changes

Implements a simplified multi-tick donate function. This is still a prototype therefore the DRAFT status. Many TODOs left for productionization:

  • Remove console logs
  • More tests
  • End tick validation?
  • Split donate into donate above/below? separate hooks?

Copy link
Member

@snreynolds snreynolds left a comment

Choose a reason for hiding this comment

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

whats the bytecode change?

/// @dev If donating above the current tick (startTick > getSlot0(id).tick) the provided amounts
/// will be consumed backwards in the order that ticks are traversed i.e. amounts0[0], amounts1[0]
/// will be distributed to the highest tick and amounts0[length-1], amounts1[length-1] to the lowest.
function donate(PoolKey memory key, MultiDonateParams calldata params, bytes calldata hookData)
Copy link
Member

Choose a reason for hiding this comment

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

my thought was that this would also just be able to case the "donate in range" and we just expose one donate functional externally.

Copy link
Member

Choose a reason for hiding this comment

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

And then we don't need to have separate hook permissions for different "kinds" of donate. feels odd to me

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.

2 participants