Skip to content

Conversation

@ErikBjare
Copy link
Member

No description provided.

Comment on lines +525 to +528
# FIXME: How to calculate this properly?
# - https://docs.uniswap.org/reference/libraries/SqrtPriceMath
# - https://github.com/Uniswap/uniswap-v3-sdk/blob/main/src/swapRouter.ts
sqrtPriceLimitX96 = 0
Copy link
Member Author

Choose a reason for hiding this comment

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

This is new, haven't looked into it...

Also, uniswap-v3-sdk is a great reference.

@ErikBjare ErikBjare mentioned this pull request May 15, 2021
8 tasks
@ErikBjare
Copy link
Member Author

ErikBjare commented May 17, 2021

Been investigating some weirdness in the Quoter:

These functions are not marked view because they rely on calling non-view functions and reverting to compute the result. They are also not gas efficient and should not be called on-chain.

https://docs.uniswap.org/reference/periphery/interfaces/IQuoter

Found @shanefontaine's post https://medium.com/authereum/getting-ethereum-transaction-revert-reasons-the-easy-way-24203a4d1844, but would need the equivalent for Python lol

@ErikBjare
Copy link
Member Author

ErikBjare commented May 17, 2021

All v3 tests are now passing for me locally. Not sure why CI is failing...

However, two tests for v2 are failing, as well as one for v1.

Probably going to merge this for now, we can continue the work in a follow-up PR.

@ErikBjare
Copy link
Member Author

I just remembered that CI is likely failing due not having access to secrets in PRs (maybe it's just because the branch is on my own fork?).

After some debugging, I got the tests to print this:

<Result MissingSchema("Invalid URL '': No schema supplied. Perhaps you meant http://?")>

@ErikBjare
Copy link
Member Author

Tried setting the secret in my own fork, but no success.

Gonna merge this and hope for the best...

@ErikBjare ErikBjare merged commit d93e86f into uniswap-python:master May 17, 2021
@ErikBjare ErikBjare deleted the dev/v3-mvp branch May 17, 2021 13:58
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.

1 participant