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

[s24 | s25 | s29] Internal audit part 1 #55

Merged
merged 4 commits into from
Nov 27, 2024
Merged

Conversation

ChefMist
Copy link
Collaborator

No description provided.

@ChefMist ChefMist changed the title Internal audit part 1 [s24 and s25] Internal audit part 1 Nov 26, 2024
@ChefMist ChefMist changed the title [s24 and s25] Internal audit part 1 [s24 | s25 | s29] Internal audit part 1 Nov 27, 2024
try clPoolManager.initialize(key, sqrtPriceX96) returns (int24 tick) {
return tick;
} catch {
return type(int24).max;
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually this means the pool is already initialized, should we just return slot0.tick ?

Copy link
Contributor

Choose a reason for hiding this comment

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

considered contract size , i agree with the current max logic.
if not , it is better to query latest tick.

Copy link
Contributor

@chefburger chefburger left a comment

Choose a reason for hiding this comment

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

rest looks good to me

Copy link
Contributor

@chef-omelette chef-omelette left a comment

Choose a reason for hiding this comment

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

LGTM

@ChefMist ChefMist merged commit 74529e4 into main Nov 27, 2024
2 checks passed
@ChefMist ChefMist deleted the feat/internal-audit-26-nov branch November 27, 2024 08:26
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.

4 participants