Skip to content

Commit

Permalink
fix: do not panic if proof is null (#533)
Browse files Browse the repository at this point in the history
# What ❔

do not panic if proof is null, wait until it's ready instead

## Why ❔

proofs are calculated asynchronously with v26 upgrade, so it can be that
proof is not ready for some short period after withdrawal's block is
finalized

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [ ] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [ ] Tests for the changes have been added / updated.
- [ ] Documentation comments have been added / updated.
- [ ] Code has been formatted via `cargo fmt`.
  • Loading branch information
perekopskiy authored Jan 27, 2025
1 parent bd713eb commit 7994cfa
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 14 deletions.
2 changes: 2 additions & 0 deletions bin/withdrawal-finalizer/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Metrics for main binary
#![allow(unexpected_cfgs)]

use std::time::Duration;

use ethers::types::U256;
Expand Down
2 changes: 2 additions & 0 deletions chain-events/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Metrics for chain events
#![allow(unexpected_cfgs)]

use vise::{Counter, Gauge, Metrics};

/// Chain events metrics.
Expand Down
7 changes: 5 additions & 2 deletions client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,10 +415,13 @@ impl<P: JsonRpcClient> ZksyncMiddleware for Provider<P> {

let sender = log.topics[1].into();

let proof = self
// Proof can be not ready yet.
let Some(proof) = self
.get_log_proof(withdrawal_hash, Some(l2_to_l1_log_index as u64))
.await?
.expect("Log proof should be present. qed");
else {
return Ok(None);
};

let message: Bytes = match ethers::abi::decode(&[ParamType::Bytes], &log.data)
.expect("log data is valid rlp data; qed")
Expand Down
2 changes: 2 additions & 0 deletions client/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Metrics for client
#![allow(unexpected_cfgs)]

use std::time::Duration;

use vise::{Buckets, Histogram, LabeledFamily, Metrics};
Expand Down
4 changes: 3 additions & 1 deletion deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ db-path = "~/.cargo/advisory-db"
db-urls = ["https://github.com/rustsec/advisory-db"]
yanked = "warn"
ignore = [
"RUSTSEC-2023-0052"
"RUSTSEC-2023-0052",
"RUSTSEC-2024-0384",
"RUSTSEC-2024-0421"
]

[licenses]
Expand Down
43 changes: 32 additions & 11 deletions finalizer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ const OUT_OF_FUNDS_BACKOFF: Duration = Duration::from_secs(10);
/// Backoff period if one of the loop iterations has failed.
const LOOP_ITERATION_ERROR_BACKOFF: Duration = Duration::from_secs(5);

/// Interval between successful loop iterations.
const LOOP_ITERATION_OK_INTERVAL: Duration = Duration::from_secs(1);

/// A newtype that represents a set of addresses in JSON format.
#[derive(Debug, Eq, PartialEq)]
pub struct AddrList(pub Vec<Address>);
Expand Down Expand Up @@ -491,11 +494,12 @@ fn is_gas_required_exceeds_allowance<M: Middleware>(e: &<M as Middleware>::Error
}

// Request finalization parameters for a set of withdrawals in parallel.
// Returns `None` if params for some withdrawal are not ready yet.
async fn request_finalize_params<M2>(
pgpool: &PgPool,
middleware: M2,
hash_and_indices: &[(H256, u16, u64)],
) -> Vec<WithdrawalParams>
) -> Option<Vec<WithdrawalParams>>
where
M2: ZksyncMiddleware,
{
Expand All @@ -504,20 +508,30 @@ where
// Run all parametere fetching in parallel.
// Filter out errors and log them and increment a metric counter.
// Return successful fetches.
for (i, result) in futures::future::join_all(hash_and_indices.iter().map(|(h, i, id)| {
let params_opt = futures::future::join_all(hash_and_indices.iter().map(|(h, i, id)| {
middleware
.finalize_withdrawal_params(*h, *i as usize)
.map_ok(|r| {
let mut r = r.expect("always able to ask withdrawal params; qed");
r.id = *id;
.map_ok(|mut r| {
if let Some(r) = r.as_mut() {
r.id = *id;
}
r
})
.map_err(crate::Error::from)
}))
.await
.into_iter()
.enumerate()
{
.await;

// If any element is `None` then return `None`.
let mut params = Vec::with_capacity(params_opt.len());
for result in params_opt {
match result {
Ok(Some(param)) => params.push(Ok(param)),
Ok(None) => return None,
Err(err) => params.push(Err(err)),
}
}

for (i, result) in params.into_iter().enumerate() {
match result {
Ok(r) => ok_results.push(r),
Err(e) => {
Expand All @@ -535,7 +549,7 @@ where
}
}

ok_results
Some(ok_results)
}

// Continiously query the new withdrawals that have been seen by watcher
Expand All @@ -556,6 +570,8 @@ async fn params_fetcher_loop<M1, M2>(
{
tracing::error!("params fetcher iteration ended with {e}");
tokio::time::sleep(LOOP_ITERATION_ERROR_BACKOFF).await;
} else {
tokio::time::sleep(LOOP_ITERATION_OK_INTERVAL).await;
}
}
}
Expand Down Expand Up @@ -584,7 +600,12 @@ where
.map(|p| (p.key.tx_hash, p.key.event_index_in_tx as u16, p.id))
.collect();

let params = request_finalize_params(pool, &middleware, &hash_and_index_and_id).await;
let Some(params) = request_finalize_params(pool, &middleware, &hash_and_index_and_id).await
else {
// Early-return if params are not ready.
tracing::info!("Params are not ready");
return Ok(());
};

let already_finalized: Vec<_> = get_finalized_withdrawals(&params, zksync_contract, l1_bridge)
.await?
Expand Down
2 changes: 2 additions & 0 deletions finalizer/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Metrics for finalizer
#![allow(unexpected_cfgs)]

use vise::{Counter, Gauge, Metrics};

/// Finalizer metrics
Expand Down
2 changes: 2 additions & 0 deletions storage/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Metrics for storage
#![allow(unexpected_cfgs)]

use std::time::Duration;

use vise::{Buckets, Histogram, LabeledFamily, Metrics};
Expand Down
2 changes: 2 additions & 0 deletions tx-sender/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Metrics for tx sender
#![allow(unexpected_cfgs)]

use vise::{Counter, Metrics};

/// TX Sender metrics
Expand Down
2 changes: 2 additions & 0 deletions watcher/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Metrics for withdrawal watcher
#![allow(unexpected_cfgs)]

use vise::{Gauge, Metrics};

/// Watcher metrics
Expand Down
2 changes: 2 additions & 0 deletions withdrawals-meterer/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Metrics for withdrawal meterer
#![allow(unexpected_cfgs)]

use vise::{EncodeLabelSet, EncodeLabelValue, Family, Gauge, LabeledFamily, Metrics};

/// Kinds of withdrawal volumes currently being metered by application
Expand Down

0 comments on commit 7994cfa

Please sign in to comment.