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 TransactionErr and TPU events to make them more structured and distinctive #2226

Open
dimxy opened this issue Sep 24, 2024 · 0 comments
Labels
improvement: error handling Improvements made on error-handling priority: high Important tasks that need attention soon.

Comments

@dimxy
Copy link
Collaborator

dimxy commented Sep 24, 2024

Problem:

Currently the main variant to convey transaction errors is TransactionErr::Plain(String).
In some cases, when error needs a user reaction, passing errors in String is not convenient for the GUI to process it.

  • For e.g. in case of insufficient funds in a withdraw or simply send_raw_transaction call to Eth network we return this error as a string code: ServerError(-32000), message: \"insufficient funds for gas * price + value: balance ... , tx cost ..., overshot ....
    A multilingual GUI can't just display such an error so it would need to parse it by itself to react (this is what komodo-wallet actually does).
    In case of Ethereum the network does not return a specific code for errors like 'insufficient funds' and I believe this is a task for the API to process them and return as structured errors (for e.g. etherjs code does this).

  • Another case is swap events reflecting failures during the swap. They contain enums like AbortReason, MakerPaymentRefundReason, TakerPaymentRefundReason. Again such objects have attached TransactionErr, ValidatePaymentError errors simply converted to a String. This makes it harder for GUI to show what wrong happened during the swap. Better to make those String descriptions as structured error with error_type and optional params. For example:
    { "error_type": InsufficientFunds", "available": 1000, "required": 1200 }.

  • It's probably not necessary to refactor errors in legacy swap events as we target TPU.

Solution

  1. Parse blockchain errors messages requiring user action (like insufficient funds) and add them as TransactionErr variants as structs (with additional properties if needed for some error type).
  2. In AbortReason, MakerPaymentRefundReason, TakerPaymentRefundReason variants replace the simple String description on structured errors.
@dimxy dimxy changed the title Refactor TransactionErr and TPU events to make them more structured Refactor TransactionErr and TPU events to make them more structured and distinctive Sep 24, 2024
@shamardy shamardy added priority: high Important tasks that need attention soon. improvement: error handling Improvements made on error-handling labels Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement: error handling Improvements made on error-handling priority: high Important tasks that need attention soon.
Projects
None yet
Development

No branches or pull requests

2 participants