Skip to content

Fix Irrecoverable State for Queued to Sent Batch Failure Condition#251

Merged
willmeister merged 4 commits intomasterfrom
YAS-647/FixBatchSubmissionQueuedToSentFailureCondition
Sep 8, 2020
Merged

Fix Irrecoverable State for Queued to Sent Batch Failure Condition#251
willmeister merged 4 commits intomasterfrom
YAS-647/FixBatchSubmissionQueuedToSentFailureCondition

Conversation

@willmeister
Copy link

Description

Shortens window during which a process restart would require manual intervention to fix rollup batches.

Questions

  • Is there an easy way to get the tx hash ahead of sending so that we can eliminate this race condition? Note: my main problem is with estimating and setting gasLimit and gasPrice -- I know it's doable by doing a lot more of what ethers wraps manually, but this seems like the right ratio of improvement : time.
  • Does it make sense to just add wallet nonce to the batch submission table and update that ahead of sending a batch? That way, we can always check to see that the pending tx count is as expected. But if a nonce is stored, can't find the tx associated with it so 🤷

Metadata

Fixes

Contributing Agreement

@karlfloersch
Copy link
Contributor

karlfloersch commented Sep 3, 2020

Is there an easy way to get the tx hash ahead of sending so that we can eliminate this race condition?

You should be able to get the tx hash by hashing the output of: ethers-io/ethers.js#535 (comment)

EDIT: You figured this out and my suggestion wasn't quite right! 😄

Does it make sense to just add wallet nonce to the batch submission table and update that ahead of sending a batch?

Ah yes I think this does actually make sense. Especially it makes sense because it will make it way easier to 'bump up the gas price' for txs that are taking a really long time to be included. You can just see 'ok how many of my txs have gone through... which one is taking a while? how long ago did i submit it? oh it's been too long let me increase the gas price".

I know that DYDX's relayer has this logic for instance. I'm sure other relayers too but that's the one that I've used and seen in action

@tynes
Copy link
Contributor

tynes commented Sep 4, 2020

BatchSubmissionStatus.QUEUED is when its in the mempool and BatchSubmissionStatus.SUBMITTING is when its being sent to the mempool but before its confirmed to enter?

@willmeister willmeister changed the title Shortening Race Condition window for Queued to Sent Batch Failure Condition Fix Irrecoverable State for Queued to Sent Batch Failure Condition Sep 4, 2020
@willmeister
Copy link
Author

willmeister commented Sep 4, 2020

BatchSubmissionStatus.QUEUED is when its in the mempool and BatchSubmissionStatus.SUBMITTING is when its being sent to the mempool but before its confirmed to enter?

  • BatchSubmissionStatus.QUEUED it is guaranteed to have never been submitted to L1
  • BatchSubmissionStatus.SUBMITTING we are trying to submit it, but if our process fails here, it may or may not have been submitted
  • BatchSubmissionStatus.SENT sent and got at least 1 confirm on chain
  • BatchSubmissionStatus.FINALIZED There were at least < configurable finality blocks > confirms

Copy link
Contributor

@karlfloersch karlfloersch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright great! Seems it wasn't too verbose to calculate the tx hash up front & so that's good. And with the submission logic everything seems right to me!

LGTM!


protected async getBatchSubmissionBlockNumber(): Promise<number> {
// TODO: This will eventually be part of the output metadata from L2 tx outputs
// Need to update geth to have this functionality so this is a mock for now
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 ah yes we definitely need this. Got to double check that we have it represented as a ticket

stateRootBatch.startIndex
)

txHash = keccak256(signedTx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@willmeister willmeister merged commit 3c00289 into master Sep 8, 2020
@willmeister willmeister deleted the YAS-647/FixBatchSubmissionQueuedToSentFailureCondition branch September 8, 2020 15:09
snario pushed a commit that referenced this pull request Apr 14, 2021
* [Fix] CI on merge

* rename
shenkeyao referenced this pull request in EspressoSystems/optimism-espresso-integration Mar 1, 2025
QuentinI referenced this pull request in EspressoSystems/optimism-espresso-integration May 27, 2025
theochap pushed a commit that referenced this pull request Dec 10, 2025
## Overview

Adds a function in the `BootInfo` module that pulls in the rollup config
based on the passed L2 chain ID.
emhane pushed a commit that referenced this pull request Feb 3, 2026
Fixes #237 

Adds more concrete error variants for `OpProofsStorageError`

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>
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.

3 participants