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.

Sign up for GitHub

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

Jump to bottom

[Valora] Extend the transactionTimeout Time #5353

Closed
aslawson opened this issue Oct 16, 2020 · 0 comments · Fixed by #5620
Closed

[Valora] Extend the transactionTimeout Time #5353

aslawson opened this issue Oct 16, 2020 · 0 comments · Fixed by #5620
Assignees
@nategraf
Labels
commit epic Priority: P1 Critical verifications
Milestone
Milestone 7

Comments

@aslawson
Copy link
Contributor

aslawson commented Oct 16, 2020 •
edited
Loading

Some transactionTimeouts can be due to gas estimation latency or node connection issues. In investigation, increasing this timeout to 60s should cover most cases.
https://celo-org.slack.com/archives/C017SK5RBFS/p1600110565219600

Band-aid solution
@aslawson aslawson added this to the Milestone 7 milestone Oct 16, 2020
@nategraf nategraf mentioned this issue Oct 28, 2020
Merged
@mergify mergify bot closed this as completed in #5620 Oct 30, 2020
mergify bot pushed a commit that referenced this issue Oct 30, 2020
Mitigate transaction timeout issues due to backgrounding and slow net…
31e2844 
…works (#5620)

### Description

One of the most common errors we see in the verification flow at the moment is transaction timeout, which is added by the Valora app and currently specified to 45 seconds.

We have observed a number of cases where a transaction timeout will occur, and then the transaction will continue and be mined successfully not too long after. Looking at a number of these events, raising the timeout period would appear to increase the success rate of transactions, provided the user is willing to wait. (In cases where they do not want to wait, they may cancel the action they are taking and the transaction will not be monitored further.)

In this PR, the timeout is increased to 90s, which I have observed will handle most of the cases where the timeout triggers on transaction that eventually end up being mined. This should be a useful measure to increase transaction reliability until we can revisit the sending flow to incorporate additional improvements.

### Other changes

* Add some comments to the verifications flow.
* Added a grace period to after initial transaction timeout to mitigate #5142.
* Change view call for balance fetch to select from redux state to avoid expensive view call.
* Log full transaction details in the `GethNativeBridgeWallet` to improve debugging in Sentry.

### Tested

Tested on iOS simulator to confirm that without the changes, transaction report as timeout after being backgrounded for more than 90s. With the grace period, I observed that the timeout did not occur. On my full-speed WiFi, it got the transaction confirmation within 1 second of returning from suspension. On simulated 3G, it confirmed within 3 seconds of returning to the background.

### Related issues

- Fixed #5353
- Mitigates #5142

### Backwards compatibility

100%
@snyk-bot snyk-bot mentioned this issue Apr 24, 2023
Open
@leojoy95 leojoy95 mentioned this issue Apr 25, 2023
Open
This was referenced Jun 21, 2023
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nategraf nategraf

Labels
commit epic Priority: P1 Critical verifications
Projects
None yet
Milestone
Milestone 7
Development

Successfully merging a pull request may close this issue.

Mitigate transaction timeout issues due to backgrounding and slow networks celo-org/celo-monorepo
2 participants
@nategraf @aslawson