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

Restore support for ETH assets #2677

Closed
Tracked by #2589
erwanor opened this issue Jun 5, 2023 · 3 comments · Fixed by #2805
Closed
Tracked by #2589

Restore support for ETH assets #2677

erwanor opened this issue Jun 5, 2023 · 3 comments · Fixed by #2805
Assignees
Labels
A-zswap Area: Design and implementation of ZSwap

Comments

@erwanor
Copy link
Member

erwanor commented Jun 5, 2023

Is your feature request related to a problem? Please describe.
We uncovered a fixpoint bug (#2559) which we temporarily routed around by temporarily limiting DEX values to be at most $2^{60}$ bits wide. So that $k = p R_1 + q R_2$ is at most $\log_2(2*(2^{60} -1)^2) = 121$ bits fitting in a $128$ bits with some headroom.

Describe the solution you'd like
The above solution works as a temporary bandaid, but limits our ability to support assets with larger denom units such as ETH and ERC-20s, which we definitely want to be supporting. Instead of limiting values, we should incorporate finding a working solution as part of #2589.

@erwanor erwanor added the A-zswap Area: Design and implementation of ZSwap label Jun 5, 2023
@erwanor erwanor self-assigned this Jun 5, 2023
@erwanor erwanor removed this from Testnets Jun 5, 2023
@erwanor erwanor added this to Testnets Jun 5, 2023
@erwanor erwanor moved this to Testnet 54: Europa in Testnets Jun 5, 2023
@redshiftzero redshiftzero moved this from Testnet 54: Europa to Testnet 55: Io in Testnets Jun 8, 2023
@conorsch
Copy link
Contributor

@erwanor Thoughts on inclusion in 55? I see there's some other high-priority DEX work still in flight. Please provide an update async when you can.

@aubrika aubrika moved this from Testnet 55: Io to Next in Testnets Jun 23, 2023
@aubrika aubrika moved this from Next to Testnet 56: Callisto in Testnets Jun 23, 2023
@erwanor
Copy link
Member Author

erwanor commented Jul 10, 2023

Moving this to 57, because although this is a small change in terms of LoC, it will require more thorough quality assurance work than time allows for now. In particular, we want to test that error recovery implemented in #2740 works well in the case of complex multi-hop routes. This requires somewhat involved assurance work which I hadn't fully realized during planning.

@erwanor erwanor moved this from Testnet 56: Callisto to Testnet 57: Ganymede in Testnets Jul 10, 2023
@zbuc
Copy link
Contributor

zbuc commented Jul 11, 2023

After this is complete we should also (in a separate issue) re-enable ETH in Osiris and allocate some ETH to the Osiris wallet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-zswap Area: Design and implementation of ZSwap
Projects
No open projects
Status: Testnet 57: Ganymede
Development

Successfully merging a pull request may close this issue.

3 participants