hermes icon indicating copy to clipboard operation
hermes copied to clipboard

Handle some gas corner-cases

Open ljoss17 opened this issue 2 years ago • 1 comments

Closes: #2487

Description

Error 1: "Too much gas wanted"

For this issue Hermes report has been changed from "success" to "submitted" to avoid confusion.

### Error 2: "out of gas in location" during CheckTx

A warning during the health-check is displayed if the gas_multiplier is lower than 1.1

Error 3: "out of gas in location" during DeliverTx

The interval between client refresh calls has been changed to be the client's trusting_period/100. In addition, the client refresh method has been updated in order to catch ChainError resulting events.


PR author checklist:

  • [ ] Added changelog entry, using unclog.
  • [ ] Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • [x] Linked to GitHub issue.
  • [ ] Updated code comments and documentation (e.g., docs/).
  • [ ] Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • [ ] Reviewed Files changed in the GitHub PR explorer.
  • [ ] Manually tested (in case integration/unit/mock tests are absent).

ljoss17 avatar Aug 08 '22 10:08 ljoss17

The interval between client refresh calls has been changed to be the client's trusting_period/100. In addition, the client refresh method has been updated in order to catch ChainError resulting events.

Notes from discussing with Luca:

It seems that there are 3 cases:

  1. no update required at the moment -> the client worker can backoff with a large window eg several hours, a fraction of the refresh_period
  2. successful update -> similar to case 1, but the refresh window could be even larger, still a fraction of refresh_period
  3. failed to update -> the client worker should retry aggressively; if the errors keep coming up (such as it is signaled in #2487 error (3)), then the backoff between retries can be increased (eg Fibonacci or linear increase, but not constant as it currently is).

adizere avatar Aug 11 '22 09:08 adizere

@ljoss17 your changes are causing the integration tests to hang indefinitely. Can you merge #2567 to your branch and also fix the tests?

soareschen avatar Aug 16 '22 09:08 soareschen