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

Refactor - Add NaN to expected values (Pool 9) #201

Open
wants to merge 2 commits into
base: test/swap
Choose a base branch
from

Conversation

dubzn
Copy link
Contributor

@dubzn dubzn commented Dec 21, 2023

Just like Uniswap V3 in the case of Pool 9 (it also applies to Pool 8 but is done in the corresponding PR), when the deltas resulting from a transaction are 0, and then you want to calculate the execution_price, this results in NaN.

In this PR, that change is applied to provide more clarity and be more faithful to the results of Uniswap tests.

@rcatalan98
Copy link
Collaborator

This could work but the long term solution would be to create an Enum for the execution price and have some sort of definitions that wraps NaN and the numbers when done correctly. What do you think?

@uri-99
Copy link
Collaborator

uri-99 commented Dec 29, 2023

This could work but the long term solution would be to create an Enum for the execution price and have some sort of definitions that wraps NaN and the numbers when done correctly. What do you think?

I like this approach, though it will require a refactor in the expected_values structure, having to generate all of them again.
Is it a good "first issue"? I think it may be a good fit for a talented intern.

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.

None yet

4 participants