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

Autoswap returns results as strings; clients compare strings to learn outcome #2335

Closed
Chris-Hibbert opened this issue Feb 4, 2021 · 1 comment · Fixed by #2337
Closed
Assignees
Labels
enhancement New feature or request Zoe Contract Contracts within Zoe

Comments

@Chris-Hibbert
Copy link
Contributor

This is a request for discussion, not a particular proposal for a solution.

What is the Problem Being Solved?

Autoswap (and other contracts) return success and failure indication as strings. Clients must compare strings to learn the outcome. For many contracts the outcome is almost always a success, since either give or want satisfies offer safety. With Autoswap, the distinction between /Swap successfully completed/ and / is insufficient to buy amountOut / tells whether the trade went through.

Description of the Design

Doing a string compare on the results seems infelicitous. Maybe the contracts could publish some enum values? I don't think we have enough common outcomes across contracts to standardize this yet, so it would have to be for each contract.

Security Considerations

This is mostly about usability, I think.

Test Plan

As we figure out what we want

@Chris-Hibbert Chris-Hibbert added enhancement New feature or request Zoe Contract Contracts within Zoe labels Feb 4, 2021
@Chris-Hibbert Chris-Hibbert added this to the Beta Launch milestone Feb 4, 2021
@katelynsills
Copy link
Contributor

katelynsills commented Feb 4, 2021

We need to improve the code in Multipool Autoswap, as this isn't generally how contracts work. They generally work much better - getOfferResult throws and it is the promise rejection that indicates an error. See #2336 for the issue detailing what needs to be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Zoe Contract Contracts within Zoe
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants