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

Feat/multi quoter not isolation #57

Merged
merged 17 commits into from
Dec 2, 2024
Merged

Conversation

ChefSnoopy
Copy link
Contributor

All swap results will influence the outcome of subsequent swaps within the same pool

ChefMist
ChefMist previously approved these changes Nov 29, 2024
MixedQuoterRecorder.getPoolSwapTokenAccumulation(poolHash, zeroForOne);
uint256 swapAmountOut;
amountIn += accAmountIn;
(swapAmountOut, gasEstimateForCurAction) = quoteExactInputSingleStable(
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to have "quote*WithAccumulation" version for ss pool as well ? Not familiar with it's charging rule, does the fee also auto get reinvested into the pool ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should not need.
it is different from v2.

/// @param key The pool key.
/// @return poolHash The hash of the pool.
function getV4CLPoolHash(PoolKey memory key) internal pure returns (bytes32) {
return keccak256(abi.encode(key, SWAP_V4_CL));
Copy link
Contributor

Choose a reason for hiding this comment

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

curious, why not directly key.toId() here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no special reason, just do not want to confuse , and keep same style with other pools

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.

lgtm, the only concern is that will it hit gas limit for a block i.e. 30M (for v4, actions are executed repeatedly)

i suggest we can do a quick deployment and let FE verify

@ChefSnoopy ChefSnoopy merged commit 05b4574 into main Dec 2, 2024
2 checks passed
@ChefSnoopy ChefSnoopy deleted the feat/multi-quoter-not-isolate branch December 2, 2024 10:17
@ChefMist ChefMist mentioned this pull request Dec 3, 2024
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