From 54e596bb78cf539bf692a0ec57e27ff6dd7733e9 Mon Sep 17 00:00:00 2001 From: Krzysztof Lis Date: Fri, 5 Jul 2024 16:28:37 +0200 Subject: [PATCH] test: fix get_headers proptest and track sync test assertions --- .../src/p2p_network/sync_handlers.rs | 17 +++++- .../src/p2p_network/sync_handlers/tests.rs | 15 ++++- crates/pathfinder/src/sync/track.rs | 59 ++++++++++--------- crates/storage/src/fake.rs | 44 ++++++-------- 4 files changed, 74 insertions(+), 61 deletions(-) diff --git a/crates/pathfinder/src/p2p_network/sync_handlers.rs b/crates/pathfinder/src/p2p_network/sync_handlers.rs index e0bbd9d557..a1071ab215 100644 --- a/crates/pathfinder/src/p2p_network/sync_handlers.rs +++ b/crates/pathfinder/src/p2p_network/sync_handlers.rs @@ -25,10 +25,11 @@ use p2p_proto::state::{ }; use p2p_proto::transaction::{TransactionWithReceipt, TransactionsRequest, TransactionsResponse}; use pathfinder_common::{class_definition, BlockHash, BlockNumber}; -use pathfinder_crypto::Felt; use pathfinder_storage::{Storage, Transaction}; use tokio::sync::mpsc; +use crate::state::block_hash::calculate_receipt_commitment; + #[cfg(test)] mod tests; @@ -140,6 +141,18 @@ fn get_header( if let Some((state_diff_commitment, state_diff_len)) = state_diff_cl { tracing::trace!(?header, "Sending block header"); + // TODO this is a temporary solution until receipt commitment is stored in the + // database + let receipts = db_tx + .transaction_data_for_block(block_number.into()) + .context("Getting receipts")? + .context("No receipts found for block")? + .into_iter() + .map(|(_, r, _)| r) + .collect::>(); + let receipt_commitment = calculate_receipt_commitment(&receipts) + .context("Calculating receipt commitment")?; + let txn_count = header .transaction_count .try_into() @@ -167,7 +180,7 @@ fn get_header( .context("invalid event count")?, root: Hash(header.event_commitment.0), }, - receipts: Hash(Felt::ZERO), // TODO + receipts: Hash(receipt_commitment.0), protocol_version: header.starknet_version.to_string(), gas_price_wei: header.eth_l1_gas_price.0, gas_price_fri: header.strk_l1_gas_price.0, diff --git a/crates/pathfinder/src/p2p_network/sync_handlers/tests.rs b/crates/pathfinder/src/p2p_network/sync_handlers/tests.rs index 348583724a..f94d2e6c8e 100644 --- a/crates/pathfinder/src/p2p_network/sync_handlers/tests.rs +++ b/crates/pathfinder/src/p2p_network/sync_handlers/tests.rs @@ -498,10 +498,12 @@ mod prop { /// Fixtures for prop tests mod fixtures { - use pathfinder_storage::fake::{with_n_blocks_and_rng, Block}; + use pathfinder_storage::fake::init::Config; + use pathfinder_storage::fake::{with_n_blocks_rng_and_config, Block}; use pathfinder_storage::{Storage, StorageBuilder}; use crate::p2p_network::sync_handlers::MAX_COUNT_IN_TESTS; + use crate::state::block_hash::calculate_receipt_commitment; pub const MAX_NUM_BLOCKS: u64 = MAX_COUNT_IN_TESTS * 2; @@ -510,8 +512,15 @@ mod prop { let storage = StorageBuilder::in_memory().unwrap(); // Explicitly choose RNG to make sure seeded storage is always reproducible let mut rng = rand_chacha::ChaCha12Rng::seed_from_u64(seed); - let initializer = - with_n_blocks_and_rng(&storage, num_blocks.try_into().unwrap(), &mut rng); + let initializer = with_n_blocks_rng_and_config( + &storage, + num_blocks.try_into().unwrap(), + &mut rng, + Config { + calculate_receipt_commitment: Box::new(calculate_receipt_commitment), + ..Default::default() + }, + ); (storage, initializer) } } diff --git a/crates/pathfinder/src/sync/track.rs b/crates/pathfinder/src/sync/track.rs index e5318cdc55..bf6242d907 100644 --- a/crates/pathfinder/src/sync/track.rs +++ b/crates/pathfinder/src/sync/track.rs @@ -848,14 +848,14 @@ mod tests { const N: usize = 10; let blocks = fake::init::with_n_blocks_and_config( N, - Config::new( - |sbh: &SignedBlockHeader, rc: ReceiptCommitment| { + Config { + calculate_block_hash: Box::new(|sbh: &SignedBlockHeader, rc: ReceiptCommitment| { compute_final_hash(&BlockHeaderData::from_signed_header(sbh, rc)) - }, - calculate_transaction_commitment, - calculate_receipt_commitment, - calculate_event_commitment, - ), + }), + calculate_transaction_commitment: Box::new(calculate_transaction_commitment), + calculate_receipt_commitment: Box::new(calculate_receipt_commitment), + calculate_event_commitment: Box::new(calculate_event_commitment), + }, ); let BlockHeader { hash, number, .. } = blocks.last().unwrap().header.header; @@ -882,12 +882,12 @@ mod tests { let mut db = storage.connection().unwrap(); let db = db.transaction().unwrap(); - for mut block in blocks { + for mut expected in blocks { // TODO p2p sync does not update class and storage tries yet - block.header.header.class_commitment = ClassCommitment::ZERO; - block.header.header.storage_commitment = StorageCommitment::ZERO; + expected.header.header.class_commitment = ClassCommitment::ZERO; + expected.header.header.storage_commitment = StorageCommitment::ZERO; - let block_number = block.header.header.number; + let block_number = expected.header.header.number; let block_id = block_number.into(); let header = db.block_header(block_id).unwrap().unwrap(); let signature = db.signature(block_id).unwrap().unwrap(); @@ -916,30 +916,31 @@ mod tests { } } - pretty_assertions_sorted::assert_eq!(header, block.header.header); - pretty_assertions_sorted::assert_eq!(signature, block.header.signature); + pretty_assertions_sorted::assert_eq!(header, expected.header.header); + pretty_assertions_sorted::assert_eq!(signature, expected.header.signature); pretty_assertions_sorted::assert_eq!( state_diff_commitment, - block.header.state_diff_commitment + expected.header.state_diff_commitment ); pretty_assertions_sorted::assert_eq!( state_diff_length as u64, - block.header.state_diff_length + expected.header.state_diff_length + ); + pretty_assertions_sorted::assert_eq!(transaction_data, expected.transaction_data); + pretty_assertions_sorted::assert_eq!(state_update_data, expected.state_update.into()); + pretty_assertions_sorted::assert_eq!( + cairo_defs, + expected.cairo_defs.into_iter().collect::>() + ); + pretty_assertions_sorted::assert_eq!( + sierra_defs, + expected + .sierra_defs + .into_iter() + // All sierra fixtures are not compile-able + .map(|(h, s, _)| (h, (s, b"I'm from the fgw!".to_vec()))) + .collect::>() ); - pretty_assertions_sorted::assert_eq!(transaction_data, block.transaction_data); - pretty_assertions_sorted::assert_eq!(state_update_data, block.state_update.into()); - // pretty_assertions_sorted::assert_eq!( - // cairo_defs, - // block.cairo_defs.into_iter().collect::>() - // ); - // pretty_assertions_sorted::assert_eq!( - // sierra_defs, - // block - // .sierra_defs - // .into_iter() - // .map(|(h, s, c)| (h, (s, c))) - // .collect::>() - // ); } } diff --git a/crates/storage/src/fake.rs b/crates/storage/src/fake.rs index f2b21353b5..3932677114 100644 --- a/crates/storage/src/fake.rs +++ b/crates/storage/src/fake.rs @@ -99,6 +99,19 @@ pub fn with_n_blocks_and_rng(storage: &Storage, n: usize, rng: &mut R) - blocks } +/// Same as [`with_n_blocks`] except caller can specify the rng and additional +/// configuration +pub fn with_n_blocks_rng_and_config( + storage: &Storage, + n: usize, + rng: &mut R, + config: init::Config, +) -> Vec { + let blocks = init::with_n_blocks_rng_and_config(n, rng, config); + fill(storage, &blocks); + blocks +} + /// Raw _fake state initializers_ pub mod init { use std::collections::{HashMap, HashSet}; @@ -146,33 +159,10 @@ pub mod init { >; pub struct Config { - calculate_block_hash: BlockHashFn, - calculate_transaction_commitment: TransactionCommitmentFn, - calculate_receipt_commitment: ReceiptCommitmentFn, - calculate_event_commitment: EventCommitmentFn, - } - - impl Config { - pub fn new( - calculate_block_hash: impl Fn(&SignedBlockHeader, ReceiptCommitment) -> anyhow::Result - + 'static, - calculate_transaction_commitment: impl Fn(&[Transaction], StarknetVersion) -> anyhow::Result - + 'static, - calculate_receipt_commitment: impl Fn(&[Receipt]) -> anyhow::Result - + 'static, - calculate_event_commitment: impl Fn( - &[(TransactionHash, &[Event])], - StarknetVersion, - ) -> anyhow::Result - + 'static, - ) -> Self { - Self { - calculate_block_hash: Box::new(calculate_block_hash), - calculate_transaction_commitment: Box::new(calculate_transaction_commitment), - calculate_receipt_commitment: Box::new(calculate_receipt_commitment), - calculate_event_commitment: Box::new(calculate_event_commitment), - } - } + pub calculate_block_hash: BlockHashFn, + pub calculate_transaction_commitment: TransactionCommitmentFn, + pub calculate_receipt_commitment: ReceiptCommitmentFn, + pub calculate_event_commitment: EventCommitmentFn, } impl Default for Config {