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

Reduce tx retries and increase price on retry #1553

Merged
merged 7 commits into from
Sep 25, 2023

Conversation

BedrockSquirrel
Copy link
Collaborator

Why this change is needed

  • We are retrying slightly too aggressively, especially for sims
  • We need to increase the price on retry. If the price is not noticeably increased from submitted tx with the same nonce then the mempool will reject the retry transaction. Most nodes use 10% increase as the cut-off so I've added 20%.

PR checks pre-merging

Please indicate below by ticking the checkbox that you have read and performed the required
PR checks

  • PR checks reviewed and performed

Copy link
Collaborator

@tudor-malene tudor-malene left a 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
Copy link
Collaborator

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?

Copy link
Collaborator Author

@BedrockSquirrel BedrockSquirrel Sep 25, 2023

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.

go/ethadapter/geth_rpc_client.go Outdated Show resolved Hide resolved
@BedrockSquirrel BedrockSquirrel merged commit 07e87fb into main Sep 25, 2023
2 checks passed
@BedrockSquirrel BedrockSquirrel deleted the matt/stop-retries-on-sims branch September 25, 2023 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants