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

Speedup CL spot price check and swap by removing an iterator #7258

Merged
merged 3 commits into from
Jan 7, 2024

Conversation

ValarDragon
Copy link
Member

@ValarDragon ValarDragon commented Jan 7, 2024

Speedup the CL check for if a pool is initialized.

Right now we do a very expensive iterator call. This technically saves gas for user swaps, but this is really small (at not all commensurate with the CPU costs it induces)

More pressingly, this is a notable delay to block processing time, due to occurrence in:

  • Protorev (iterators always hit IAVL right now)
  • TWAP
  • Swaps

Notice that SpotPrice is 0 iff there is no position in the pool. You can convince yourself of this by looking at:

Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

LGTM!

Confirmed that this is valid per:

x/concentrated-liquidity/pool.go Outdated Show resolved Hide resolved
// the current sqrt price being 0.
// We also check that the current tick is 0, which is also guaranteed in this situation.
// That derisks any edge-case where the sqrt price is 0 but the tick is not 0.
func (k Keeper) PoolHasPosition(ctx sdk.Context, pool types.ConcentratedPoolExtension) bool {
Copy link
Member

Choose a reason for hiding this comment

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

Would it be appropriate to define this method on the pool itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

Great idea!

Copy link
Member Author

@ValarDragon ValarDragon Jan 7, 2024

Choose a reason for hiding this comment

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

Actually on further thought I'm not sure, this is a convention set by the keeper not the pool itself. (Adding a position doesn't go through a pool.AddPosition method)

Copy link
Member

@czarcas7ic czarcas7ic left a comment

Choose a reason for hiding this comment

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

ACK, merging due to 3 approvals

@czarcas7ic czarcas7ic merged commit 7d6ee67 into main Jan 7, 2024
1 check passed
@czarcas7ic czarcas7ic deleted the dev/speedup_cl_spotprice_and_swap branch January 7, 2024 16:56
@github-actions github-actions bot mentioned this pull request Feb 1, 2024
@github-actions github-actions bot mentioned this pull request Mar 15, 2024
@github-actions github-actions bot mentioned this pull request May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants