-
Notifications
You must be signed in to change notification settings - Fork 38
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
Reduce tx retries and increase price on retry #1553
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm.
@@ -25,8 +26,9 @@ import ( | |||
) | |||
|
|||
const ( | |||
connRetryMaxWait = 10 * time.Minute // after this duration, we will stop retrying to connect and return the failure | |||
connRetryInterval = 500 * time.Millisecond | |||
connRetryMaxWait = 10 * time.Minute // after this duration, we will stop retrying to connect and return the failure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be in block time units as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more to do with establishing a connection with the L1 RPC endpoint so not sure blocktime is as important. I don't think we handle this case well though, 10mins feels a bit arbitrary... (FWIW I didn't change this here, it's just whitespace on the line)
Feels very relevant to that pool of L1 URLs we talked about earlier, I'll add a comment on that PR to rethink how we handle and configure this timeout.
Why this change is needed
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks