fix fixSkippedDeposits

[boyuan-chen]

Description

When we ran the load testing, we found the queued element might not be equal to L1 block data in the batch submitter. We found these two situations:

• The timestamp of the queued element > timestamp of L1 block and block number of the queued element === block number of L1 block.
• The timestamp of the queued element > timestamp of L1 block and block number of the queued element > block number of L1 block.

For the second circumstance, it can be fixed through the fixMonotonicity function. However, fixMonotonicity and fixSkippedDeposits can't fix the first circumstance.

To solve the problem, we need to make some changes to fixSkippedDeposits. fixedBatch.push(ele) pushes the original element to fixedBatch, which has the wrong timestamp. The ideal logic is to only push the fixed batch to fixedBatch, so commenting out this line should fix the issue.

Metadata

• Fixes Batch Submitter can't fix batch #1214

[changeset-bot]

⚠️ No Changeset found

Latest commit: 35149fb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[karlfloersch]

Thank you so much for the PR @cby3149 !

It will increase the scope of this PR, but we're trying to avoid making changes to the batch submitter without unit tests. Would it be possible for you to pull out the function fixSkippedDeposits(...), add it as a library, unit test it, and then use that library inside of the _fixBatch(..) function? It might be easier if you pass in getNextQueueIndex() & getNumPendingQueueElements() so that mocking the calls is easy to do with unit tests.

Here's an example of us starting to decouple the batch submitter - #1024 . In the near-term decoupling the batch submitter is going to slow down development, but in the long term it's going to result in 100x more maintainable code.

[maurelian]

I think we can justifiable close this based on the lack of tests, time passed, and hopefully this code just not running for much longer (ie. #1537).