Fix for DA L2 sync race condition #2512
Conversation
|
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
added
the
no changelog
rymnc
force-pushed
the
chore/add-tests-for-v1-gas-service
branch
5 times, most recently
from
ff8243c to
eb202cf
Compare
…A-L2-sync-race-condition
…A-L2-sync-race-condition
…A-L2-sync-race-condition
| pub fn new( | ||
| client: BlockCommitter, | ||
| last_recorded_height: Option<BlockHeight>, | ||
| ) -> Self { | ||
| Self { | ||
| client, | ||
| last_recorded_height, | ||
| } |
There was a problem hiding this comment.
@MitchTurner was this change intended?
There was a problem hiding this comment.
Yes, because in practice we only set the last_recorded_height after we've constructed the BlockCommitterDaBlockCosts.
There was a problem hiding this comment.
there are some merge conflicts around this
| if let Some(cost) = raw_da_block_costs.last() { | ||
| self.last_recorded_height = Some(BlockHeight::from(cost.end_height)); | ||
| } |
There was a problem hiding this comment.
how do we set it here then? do we move the last_recorded_height elsewhere?
ignore pls
There was a problem hiding this comment.
It's held by the DA service instead now:
pub struct DaSourceService<Source> {
poll_interval: Interval,
source: Source,
shared_state: SharedState,
latest_l2_height: Arc<Mutex<BlockHeight>>,
recorded_height: Option<BlockHeight>,
}…A-L2-sync-race-condition
MitchTurner
merged commit 31fd51f
into
chore/add-tests-for-v1-gas-service
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
latest_l2_heightto theDaSourceServicewhich will filter out any DA bundles that include L2 blocks after the current heightDaSourceServicewhere it probably should have been to begin withChecklist
Before requesting review
After merging, notify other teams
[Add or remove entries as needed]