Skip to content

Commit

Permalink
fix(block-production): fix bouncer, error reporting, debug messages (#…
Browse files Browse the repository at this point in the history
…271)

Co-authored-by: antiyro <[email protected]>
Co-authored-by: Arun Jangra <[email protected]>
Co-authored-by: apoorvsadana <[email protected]>
Co-authored-by: mohiiit <[email protected]>
Co-authored-by: Trantorian1 <[email protected]>
  • Loading branch information
6 people authored Sep 23, 2024
1 parent a2bad13 commit 1b9b0d5
Show file tree
Hide file tree
Showing 53 changed files with 722 additions and 618 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:

- uses: software-mansion/setup-scarb@v1
with:
scarb-version: "2.7.0"
scarb-version: "2.8.2"

- uses: foundry-rs/foundry-toolchain@v1
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/linters-cargo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:

- uses: software-mansion/setup-scarb@v1
with:
scarb-version: "2.7.0"
scarb-version: "2.8.2"

- name: Setup build deps
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/rust-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:

- uses: software-mansion/setup-scarb@v1
with:
scarb-version: "2.7.0"
scarb-version: "2.8.2"

- name: Check the project
run: |
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Next release

- fix(block-production): fix bouncer calculation and declared classes
- fix: Fix pending block sync and add real FGW tests
- test: tests added for verify and apply task in l2 sync
- fix: UDC cairo 0 migration & events logic fix
Expand Down
8 changes: 8 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ tempfile = "3.10.1"
env_logger = "0.11.3"
mockall = "0.13.0"
serial_test = "3.1.1"
itertools = "0.13.0"

[patch.crates-io]
starknet-core = { git = "https://github.com/kasarlabs/starknet-rs.git", branch = "fork" }
Expand Down
6 changes: 3 additions & 3 deletions cairo/Scarb.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
name = "madara_contracts"
version = "0.1.0"
edition = "2023_11"
cairo-version = "2.7.0"
scarb-version = "2.7.0"
cairo-version = "2.8.2"
scarb-version = "2.8.2"
authors = ["The Madara Committee <https://madara.build>"]
description = "Cairo contracts for boostrapping a madara network."

[dependencies]
openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts.git", tag = "v0.15.1" }
starknet = "2.7.0"
starknet = "2.8.2"

[dev-dependencies]
snforge_std = { git = "https://github.com/foundry-rs/starknet-foundry", tag = "v0.27.0" }
Expand Down
4 changes: 4 additions & 0 deletions crates/client/block_import/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,18 @@ version.workspace = true
license.workspace = true

[dependencies]
anyhow.workspace = true
bitvec.workspace = true
itertools.workspace = true
log.workspace = true
num-traits.workspace = true
rayon.workspace = true
serde.workspace = true
thiserror.workspace = true
tokio.workspace = true

mc-db = { workspace = true }
mc-metrics = { workspace = true }
mp-block = { workspace = true }
mp-chain-config = { workspace = true }
mp-class = { workspace = true }
Expand Down
43 changes: 40 additions & 3 deletions crates/client/block_import/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,15 @@
//! to check for errors.
//! A signature verification mode should be added to allow the skipping of block validation entirely if the block is signed.
use anyhow::Context;
use mc_db::{MadaraBackend, MadaraStorageError};
use mc_metrics::MetricsRegistry;
use metrics::BlockMetrics;
use mp_class::{class_hash::ComputeClassHashError, compile::ClassCompilationError};
use starknet_core::types::Felt;
use std::{borrow::Cow, sync::Arc};

mod metrics;
mod pre_validate;
mod rayon;
pub mod tests;
Expand Down Expand Up @@ -113,13 +117,39 @@ impl BlockImportError {
}
pub struct BlockImporter {
pool: Arc<RayonPool>,
backend: Arc<MadaraBackend>,
verify_apply: VerifyApply,
metrics: BlockMetrics,
always_force_flush: bool,
}

impl BlockImporter {
pub fn new(backend: Arc<MadaraBackend>) -> Self {
/// The starting block is used for metrics. Setting it to None means it will look at the database latest block number.
pub fn new(
backend: Arc<MadaraBackend>,
metrics_registry: &MetricsRegistry,
starting_block: Option<u64>,
always_force_flush: bool,
) -> anyhow::Result<Self> {
let pool = Arc::new(RayonPool::new());
Self { verify_apply: VerifyApply::new(Arc::clone(&backend), Arc::clone(&pool)), pool }
let starting_block = if let Some(n) = starting_block {
n
} else {
backend
.get_latest_block_n()
.context("Getting latest block in database")?
.map(|b| /* next block */ b + 1)
.unwrap_or(0 /* genesis */)
};

Ok(Self {
verify_apply: VerifyApply::new(Arc::clone(&backend), Arc::clone(&pool)),
pool,
metrics: BlockMetrics::register(starting_block, metrics_registry)
.context("Registering metrics for block import")?,
backend,
always_force_flush,
})
}

/// Perform [`BlockImporter::pre_validate`] followed by [`BlockImporter::verify_apply`] to import a block.
Expand All @@ -145,7 +175,14 @@ impl BlockImporter {
block: PreValidatedBlock,
validation: BlockValidationContext,
) -> Result<BlockImportResult, BlockImportError> {
self.verify_apply.verify_apply(block, validation).await
let result = self.verify_apply.verify_apply(block, validation).await?;
// Flush step.
let force = self.always_force_flush;
self.backend
.maybe_flush(force)
.map_err(|err| BlockImportError::Internal(format!("DB flushing error: {err:#}").into()))?;
self.metrics.update(&result.header, &self.backend);
Ok(result)
}

pub async fn pre_validate_pending(
Expand Down
96 changes: 96 additions & 0 deletions crates/client/block_import/src/metrics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
use mc_db::MadaraBackend;
use mc_metrics::{Gauge, MetricsRegistry, PrometheusError, F64};
use mp_block::Header;
use num_traits::FromPrimitive;
use std::{
sync::Mutex,
time::{Duration, Instant},
};

#[derive(Debug)]
pub struct BlockMetrics {
/// Starting block
pub starting_block: u64,
pub starting_time: Instant,
pub last_update_instant: Mutex<Option<Instant>>,
pub last_db_metrics_update_instant: Mutex<Option<Instant>>,

// L2 network metrics
pub l2_block_number: Gauge<F64>,
pub l2_sync_time: Gauge<F64>,
pub l2_avg_sync_time: Gauge<F64>,
pub l2_latest_sync_time: Gauge<F64>,
pub l2_state_size: Gauge<F64>, // TODO: remove this, as well as the return value from db_metrics update.
pub transaction_count: Gauge<F64>,
pub event_count: Gauge<F64>,
// L1 network metrics
pub l1_gas_price_wei: Gauge<F64>,
pub l1_gas_price_strk: Gauge<F64>,
}

impl BlockMetrics {
pub fn register(starting_block: u64, registry: &MetricsRegistry) -> Result<Self, PrometheusError> {
Ok(Self {
starting_block,
starting_time: Instant::now(),
last_update_instant: Default::default(),
last_db_metrics_update_instant: Default::default(),

l2_block_number: registry.register(Gauge::new("madara_l2_block_number", "Current block number")?)?,
l2_sync_time: registry.register(Gauge::new(
"madara_l2_sync_time",
"Complete sync time since startup in secs (does not account for restarts)",
)?)?,
l2_avg_sync_time: registry.register(Gauge::new(
"madara_l2_avg_sync_time",
"Average time spent between blocks since startup in secs",
)?)?,
l2_latest_sync_time: registry
.register(Gauge::new("madara_l2_latest_sync_time", "Latest time spent between blocks in secs")?)?,
l2_state_size: registry.register(Gauge::new("madara_l2_state_size", "Node storage usage in GB")?)?,
transaction_count: registry
.register(Gauge::new("madara_transaction_count", "Latest block transaction count")?)?,
event_count: registry.register(Gauge::new("madara_event_count", "Latest block event count")?)?,
l1_gas_price_wei: registry
.register(Gauge::new("madara_l1_block_gas_price", "Latest block L1 ETH gas price")?)?,
l1_gas_price_strk: registry
.register(Gauge::new("madara_l1_block_gas_price_strk", "Latest block L1 STRK gas price")?)?,
})
}

pub fn update(&self, block_header: &Header, backend: &MadaraBackend) {
let now = Instant::now();

// Update Block sync time metrics
let latest_sync_time = {
let mut last_update = self.last_update_instant.lock().expect("Poisoned lock");
let latest_sync_time = last_update.map(|inst| now.duration_since(inst)).unwrap_or_default();
*last_update = Some(now);
latest_sync_time.as_secs_f64()
};

let total_sync_time = now.duration_since(self.starting_time).as_secs_f64();
self.l2_sync_time.set(total_sync_time);
self.l2_latest_sync_time.set(latest_sync_time);
self.l2_avg_sync_time.set(total_sync_time / (block_header.block_number - self.starting_block) as f64);

self.l2_block_number.set(block_header.block_number as f64);
self.transaction_count.set(f64::from_u64(block_header.transaction_count).unwrap_or(0f64));
self.event_count.set(f64::from_u64(block_header.event_count).unwrap_or(0f64));

self.l1_gas_price_wei.set(f64::from_u128(block_header.l1_gas_price.eth_l1_gas_price).unwrap_or(0f64));
self.l1_gas_price_strk.set(f64::from_u128(block_header.l1_gas_price.strk_l1_gas_price).unwrap_or(0f64));

{
let mut last_db_instant = self.last_db_metrics_update_instant.lock().expect("Poisoned lock");
let last_update_duration = last_db_instant.map(|inst| now.duration_since(inst));

if last_update_duration.is_none() || last_update_duration.is_some_and(|d| d >= Duration::from_secs(5)) {
*last_db_instant = Some(now);
let storage_size = backend.update_metrics();
let size_gb = storage_size as f64 / (1024 * 1024 * 1024) as f64;
self.l2_state_size.set(size_gb);
}
}
}
}
14 changes: 13 additions & 1 deletion crates/client/block_import/src/pre_validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ pub fn pre_validate_inner(
.map(|f| f())
.collect::<Result<(), _>>()?;

converted_classes.extend(block.trusted_converted_classes);

Ok(PreValidatedBlock {
header: block.header,
transactions: block.transactions,
Expand Down Expand Up @@ -172,7 +174,17 @@ fn class_conversion(
}
DeclaredClass::Legacy(legacy) => {
log::trace!("Converting legacy class with hash {:#x}", legacy.class_hash);
// TODO: verify that the class hash is correct
if !validation.trust_class_hashes {
let class_hash = legacy
.contract_class
.compute_class_hash()
.map_err(|e| BlockImportError::ComputeClassHash { class_hash: legacy.class_hash, error: e })?;
if class_hash != legacy.class_hash {
// TODO: For now we skip the exceptions for the legacy class hash mismatch
log::debug!("Class hash mismatch: got {:#x}, expected {:#x}", class_hash, legacy.class_hash,);
// return Err(BlockImportError::ClassHash { got: class_hash, expected: legacy.class_hash });
}
}
Ok(ConvertedClass::Legacy(LegacyConvertedClass {
class_hash: legacy.class_hash,
info: LegacyClassInfo { contract_class: Arc::new(legacy.contract_class) },
Expand Down
3 changes: 3 additions & 0 deletions crates/client/block_import/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ pub struct UnverifiedFullBlock {
pub transactions: Vec<Transaction>,
pub receipts: Vec<TransactionReceipt>,
pub declared_classes: Vec<DeclaredClass>,
/// Classes that are already compiled and hashed.
#[serde(skip)]
pub trusted_converted_classes: Vec<ConvertedClass>,
pub commitments: UnverifiedCommitments,
}

Expand Down
Loading

0 comments on commit 1b9b0d5

Please sign in to comment.