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 for DA L2 sync race condition #2512

Open
wants to merge 5 commits into
base: chore/add-tests-for-v1-gas-service
Choose a base branch
from

Conversation

MitchTurner
Copy link
Member

@MitchTurner MitchTurner commented Dec 20, 2024

Linked Issues/PRs

Description

In the case we are syncing from an existing network, it is possible for the L2 blocks to sync slower than the DA costs, in which case you will try to apply DA costs to an algorithm that is missing the corresponding unrecorded L2 blocks.

This PR

  • introduces a latest_l2_height to the DaSourceService which will filter out any DA bundles that include L2 blocks after the current height
  • moves the recorded height concept into the DaSourceService where it probably should have been to begin with

Checklist

  • Breaking changes are clearly marked as such in the PR description and changelog
  • New behavior is reflected in tests
  • The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

  • I have reviewed the code myself
  • I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

[Add or remove entries as needed]

@rymnc
Copy link
Member

rymnc commented Dec 20, 2024

this error occurs because we call the latest_block_costs from the block committer. i think we can just pass the l2 block height into the da source service (not an arc) and make the da source service fetch all the cost data from that l2 block onwards from the block committer

@MitchTurner MitchTurner added the no changelog Skip the CI check of the changelog modification label Dec 20, 2024
@MitchTurner MitchTurner marked this pull request as ready for review December 20, 2024 20:03
@rymnc rymnc force-pushed the chore/add-tests-for-v1-gas-service branch 2 times, most recently from 1ded825 to bb007d3 Compare December 25, 2024 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Skip the CI check of the changelog modification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants