hermes
hermes copied to clipboard
Handle some gas corner-cases
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).
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:
- no update required at the moment -> the client worker can backoff with a large window eg several hours, a fraction of the
refresh_period
- successful update -> similar to case 1, but the refresh window could be even larger, still a fraction of
refresh_period
- 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).
@ljoss17 your changes are causing the integration tests to hang indefinitely. Can you merge #2567 to your branch and also fix the tests?