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

fix: increase timeouts on transaction querying on L1 #215

Merged
merged 3 commits into from
Oct 2, 2023

Conversation

montekki
Copy link
Contributor

@montekki montekki commented Oct 2, 2023

Digging into recent panics in prod it looks like they are just the case of insufficient timeouts on L1 transaction fetching.

@RomanBrodetski
Copy link

@montekki can you please quickly summmarize why this is necessary?

@montekki
Copy link
Contributor Author

montekki commented Oct 2, 2023

@RomanBrodetski so the race in question here is on https://matter-labs.app.eu.opsgenie.com/alert/detail/2d02ea8d-0a36-4c8e-a273-56c5fff79742-1695969365058/details, racing on transaction https://etherscan.io/tx/0x0f5171baf1c5537e09606cc527cdca8b3efd2684b731791a442a342332606fd4. The timestamps on both suggest that transaction went in at 6:35:35 UTC, our panic happened in 6:36 (+some seconds).

Furthermore this transaction went in ok, was not a part of any block reorg, currently it belongs to a finalized block. As such I see no other reason for the panic above to happen other than insufficient timeouts on the transaction querying. Before this patch panic happened if the TX couldn't be queried from RPC in 15 seconds (intuitively block time), however this limit looks insufficient looks like.

It would probably make sense to make this configurable however at this moment i don't think it is worth adding a new config var.

@montekki montekki merged commit a3760d5 into main Oct 2, 2023
@montekki montekki deleted the fvs-pla-591-another-race-on-infura-api branch October 2, 2023 11:11
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