feat(gateway): refine l2 l1 logs proofs #3078
Conversation
There was a problem hiding this comment.
Generally looks good, we also discussed in private that a bit more long-term approach is that each chain calculates chain root on its own. However, it is not needed in this release, but we need to plan to be able to change it.
Also, using sleep in tests is not recommended since I already worked on removing those by creating waitForBlockToBeFinalizedOnL1
core/node/eth_watch/src/event_processors/appended_chain_batch_root_signature.rs
Outdated
Show resolved
Hide resolved
| &mut self, | ||
| sl_chain_id: SLChainId, | ||
| ) -> DalResult<Vec<(L1BatchNumber, H256)>> { | ||
| let result = sqlx::query!( |
There was a problem hiding this comment.
in case on an emergency if the execute transaction happen outside the eth sender, how hard will it be make this function work?
There was a problem hiding this comment.
Should be simple: insert corresponding rows to eth_txs_history and eth_tx tables. However, if it happens and server restarts before we insert these rows it potentially can lead to incorrect batch_chain_merkle_path being saved. I guess I will add a sanity check to events processor: batch root calculated on eth watcher side should be the same as on SL
core/node/eth_watch/src/lib.rs
Outdated
| .get_executed_batch_roots_on_sl(sl_chain_id) | ||
| .await?; | ||
|
|
||
| let next_expected_batch_number = batch_hashes |
There was a problem hiding this comment.
will it work fine when we migrate from L1 to Gateway? It just seems like if the returned array is 0, then it will be strictly expected to see batch 0, which is not correct
There was a problem hiding this comment.
It will work. Variable name is misleading, it's not next expected but "lower bound for the next batch number". I will try to come up with better name
There was a problem hiding this comment.
I ended up with next_batch_number_lower_bound
core/node/eth_watch/src/event_processors/appended_chain_batch_root_signature.rs
Outdated
Show resolved
Hide resolved
| .last() | ||
| .map(|(n, _)| *n + 1) | ||
| .unwrap_or(L1BatchNumber(0)); | ||
| let tree_leaves = batch_hashes.into_iter().map(|(batch_number, batch_root)| { |
There was a problem hiding this comment.
just in case, the message root aggregation will only work on gateway (not L1)
| async fn logs_extended( | ||
| &self, | ||
| filter: &web3::Filter, | ||
| ) -> EnrichedClientResult<Vec<zksync_types::api::Log>> { |
There was a problem hiding this comment.
I personally agree that this is the easiest choice, but I recall I had to refactor my types with some pain to ensure that certain methods are available only for types that do support it (e.g. I added L2-like base fee history only for ForWeb3Network<Net = L2>).
But if you decide to keep it this way, it may make sense to add a comment on what will happen if someone tries to use the method with l1 provider
There was a problem hiding this comment.
I have reworked eth watcher client
What ❔
Reworks how node generates proofs for l2-l1 logs. Adds new eth-watch processor that listens to
AppendedChainBatchRootevents, updates batch tree and saves proof to DB. API reads those proofs on requestsWhy ❔
Checklist
zk_supervisor fmtandzk_supervisor lint.