-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: prevent uncommitted chain migrating from gw #4314
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
base: draft-v29
Are you sure you want to change the base?
fix: prevent uncommitted chain migrating from gw #4314
Conversation
This reverts commit ca41b8a.
8ffc816
to
590cff1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this code tested?
@perekopskiy It was tested locally: migrating back from gateway takes longer because of these additional checks. It doesn't run on CI as that step it was accidentally removed recently as part of #4255 |
pub async fn get_latest_processed_interop_root_l1_batch_number( | ||
&mut self, | ||
) -> DalResult<Option<u32>> { | ||
Ok(sqlx::query_scalar!( | ||
r#" | ||
SELECT | ||
l1_batch_number | ||
FROM | ||
miniblocks | ||
WHERE | ||
number = (SELECT MAX(processed_block_number) FROM interop_roots) | ||
"# | ||
) | ||
.instrument("get_latest_processed_interop_root_l1_batch_number") | ||
.fetch_one(self.storage) | ||
.await? | ||
.map(|max| max as u32)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately it won't work fully, just because for not sealed batch, we don't have l1 batch attached to miniblocks. so for non sealed batch but with interop roots, this check will fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the batch is not sealed then get_latest_processed_interop_root_l1_batch_number
should return None -> the function is_waiting_for_batches_with_interop_roots_to_be_committed
will return true
, which is the intended effect as batches with blocks containing interop roots have not yet been committed. See:
zksync-era/core/node/eth_sender/src/eth_tx_aggregator.rs
Lines 1160 to 1171 in 98c960d
async fn is_waiting_for_batches_with_interop_roots_to_be_committed( | |
&self, | |
storage: &mut Connection<'_, Core>, | |
) -> Result<bool, EthSenderError> { | |
let latest_processed_l1_batch_number = storage | |
.interop_root_dal() | |
.get_latest_processed_interop_root_l1_batch_number() | |
.await?; | |
if latest_processed_l1_batch_number.is_none() { | |
return Ok(true); | |
} |
What ❔
Chains migrating from gateway must not have uncommitted batches, as these can contain interop roots. If a chain tries to commit a batch with interop roots on the L1 the commit transaction would fail. The idea then is to make it so that during migration from gateway 1) interop roots stop being added to the database 2) all interop roots are processed 3) all batches containing interop roots get committed.
Why ❔
Is this a breaking change?
Operational changes
Checklist
zkstack dev fmt
andzkstack dev lint
.