From 57c408ccd42226ab6a5407b87b8ee7a4f8b62b8d Mon Sep 17 00:00:00 2001 From: Krisztian Kovacs Date: Mon, 2 Dec 2024 17:11:02 +0100 Subject: [PATCH 1/6] fix(storage/block): fix reorg storage problem As part of the still-in-development "aggregate bloom filters feature we have added a SQL statement to `purge_block()` that causes issues during a reorg because it references a non-existent table when compiling Pathfinder _without_ the "aggregate_bloom" feature. This change fixes that by gating the SQL statement behind the feature. --- crates/storage/src/connection/block.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/storage/src/connection/block.rs b/crates/storage/src/connection/block.rs index c2b6ddfb8f..9e6dcf60c5 100644 --- a/crates/storage/src/connection/block.rs +++ b/crates/storage/src/connection/block.rs @@ -116,15 +116,18 @@ impl Transaction<'_> { /// /// This includes block header, block body and state update information. pub fn purge_block(&self, block: BlockNumber) -> anyhow::Result<()> { - self.inner() - .execute( - r" + #[cfg(feature = "aggregate_bloom")] + { + self.inner() + .execute( + r" DELETE FROM starknet_events_filters_aggregate WHERE from_block <= :block AND to_block >= :block ", - named_params![":block": &block], - ) - .context("Deleting aggregate bloom filter")?; + named_params![":block": &block], + ) + .context("Deleting aggregate bloom filter")?; + } self.inner() .execute( "DELETE FROM starknet_events_filters WHERE block_number = ?", From 1e4609a889cca9f6c1b440666a2c6ed6257e7553 Mon Sep 17 00:00:00 2001 From: Krisztian Kovacs Date: Mon, 2 Dec 2024 17:16:52 +0100 Subject: [PATCH 2/6] chore: update CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f399d9e372..0d3e13af38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ More expansive patch notes and explanations may be found in the specific [pathfi The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- Pathfinder fails to properly do a reorg due to a SQL statement referring a table that does not exist. + ## [0.15.1] - 2024-12-02 ### Fixed From 24601d59223fc284f372890851513543719c7eb2 Mon Sep 17 00:00:00 2001 From: Krisztian Kovacs Date: Tue, 3 Dec 2024 15:48:45 +0100 Subject: [PATCH 3/6] fix(storage/event): really check max uncached bloom filter load limit There was a bug in the code that made the limit on loading uncached Bloom filters ineffective. This change fixes that by moving the check to the correct place in the loop so that it's actually enforced. --- crates/storage/src/connection/event.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/storage/src/connection/event.rs b/crates/storage/src/connection/event.rs index 304ed2248c..82f569dcc8 100644 --- a/crates/storage/src/connection/event.rs +++ b/crates/storage/src/connection/event.rs @@ -348,6 +348,12 @@ impl Transaction<'_> { break ScanResult::Done; } + // Check if we've reached our Bloom filter load limit + if bloom_filters_loaded >= max_uncached_bloom_filters_to_load.get() { + tracing::trace!("Bloom filter limit reached"); + break ScanResult::ContinueFrom(block_number); + } + // Check bloom filter if !key_filter_is_empty || filter.contract_address.is_some() { let bloom = self.load_bloom(reorg_counter, block_number)?; @@ -398,12 +404,6 @@ impl Transaction<'_> { } block_number += 1; - - // Check if we've reached our Bloom filter load limit - if bloom_filters_loaded >= max_uncached_bloom_filters_to_load.get() { - tracing::trace!("Bloom filter limit reached"); - break ScanResult::ContinueFrom(block_number); - } }; match result { From fa9352484fffa83a367477ca80e9d88e1385d536 Mon Sep 17 00:00:00 2001 From: Vaclav Barta Date: Wed, 4 Dec 2024 08:46:03 +0100 Subject: [PATCH 4/6] chore: remove explicit initialization of env_logger in tests --- Cargo.lock | 22 +++++++++++++------- Cargo.toml | 3 +-- crates/p2p/Cargo.toml | 3 +-- crates/p2p/src/client/peer_agnostic/tests.rs | 6 ------ crates/p2p/src/tests.rs | 8 ------- 5 files changed, 17 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f931119ff3..f3c49b8619 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4398,6 +4398,15 @@ dependencies = [ "syn 2.0.77", ] +[[package]] +name = "env_filter" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4f2c92ceda6ceec50f43169f9ee8424fe2db276791afde7b2cd8bc084cb376ab" +dependencies = [ + "log", +] + [[package]] name = "env_logger" version = "0.9.3" @@ -4413,15 +4422,14 @@ dependencies = [ [[package]] name = "env_logger" -version = "0.10.2" +version = "0.11.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4cd405aab171cb85d6735e5c8d9db038c17d3ca007a4d2c25f337935c3d90580" +checksum = "e13fa619b91fb2381732789fc5de83b45675e882f66623b7d8cb4f643017018d" dependencies = [ - "humantime", - "is-terminal", + "anstream", + "anstyle", + "env_filter", "log", - "regex", - "termcolor", ] [[package]] @@ -7137,7 +7145,6 @@ dependencies = [ "async-trait", "base64 0.13.1", "clap", - "env_logger 0.10.2", "fake", "flate2", "futures", @@ -9805,6 +9812,7 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3dffced63c2b5c7be278154d76b479f9f9920ed34e7574201407f0b14e2bbb93" dependencies = [ + "env_logger 0.11.5", "test-log-macros", "tracing-subscriber", ] diff --git a/Cargo.toml b/Cargo.toml index 2021ccc302..4f679ac66f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,7 +75,6 @@ const-decoder = "0.3.0" const_format = "0.2.31" criterion = "0.5.1" dashmap = "6.1" -env_logger = "0.10.0" fake = "2.8.0" ff = "0.13" flate2 = "1.0.27" @@ -131,7 +130,7 @@ starknet_api = { git = "https://github.com/eqlabs/sequencer", branch = "eqlabs/m starknet-types-core = "=0.1.5" syn = "1.0" tempfile = "3.8" -test-log = { version = "0.2.12", default-features = false } +test-log = { version = "0.2.12", features = ["trace"] } thiserror = "1.0.48" time = "0.3.36" tokio = "1.37.0" diff --git a/crates/p2p/Cargo.toml b/crates/p2p/Cargo.toml index 39d1043e2b..b5cc5d643d 100644 --- a/crates/p2p/Cargo.toml +++ b/crates/p2p/Cargo.toml @@ -60,12 +60,11 @@ zeroize = { workspace = true } [dev-dependencies] clap = { workspace = true, features = ["derive", "env", "wrap_help"] } -env_logger = { workspace = true } hex = { workspace = true } rand = { workspace = true } rstest = { workspace = true } tagged = { path = "../tagged" } tagged-debug-derive = { path = "../tagged-debug-derive" } -test-log = { workspace = true, features = ["trace"] } +test-log = { workspace = true } tokio = { workspace = true, features = ["test-util"] } tracing-subscriber = { workspace = true, features = ["env-filter"] } diff --git a/crates/p2p/src/client/peer_agnostic/tests.rs b/crates/p2p/src/client/peer_agnostic/tests.rs index c3d78e038e..752c4446c2 100644 --- a/crates/p2p/src/client/peer_agnostic/tests.rs +++ b/crates/p2p/src/client/peer_agnostic/tests.rs @@ -112,8 +112,6 @@ async fn make_header_stream( #[case] responses: Vec), TestPeer>>, #[case] expected_stream: Vec<(TestPeer, SignedBlockHeader)>, ) { - let _ = env_logger::builder().is_test(true).try_init(); - for (reverse, direction) in [(false, "forward"), (true, "backward")] { let (peers, responses) = unzip_fixtures(responses.clone()); let get_peers = move || { @@ -295,7 +293,6 @@ async fn make_transaction_stream( #[case] num_txns_per_block: Vec, #[case] expected_stream: Vec), ()>>, ) { - let _ = env_logger::builder().is_test(true).try_init(); let (peers, responses) = unzip_fixtures(responses); let get_peers = move || { let peers = peers.clone(); @@ -518,7 +515,6 @@ async fn make_state_diff_stream( #[case] state_diff_len_per_block: Vec, #[case] expected_stream: Vec>, ) { - let _ = env_logger::builder().is_test(true).try_init(); let (peers, responses) = unzip_fixtures(responses); let get_peers = move || { let peers = peers.clone(); @@ -713,7 +709,6 @@ async fn make_class_definition_stream( #[case] declared_classes_per_block: Vec, #[case] expected_stream: Vec>, ) { - let _ = env_logger::builder().is_test(true).try_init(); let (peers, responses) = unzip_fixtures(responses); let get_peers = move || { let peers = peers.clone(); @@ -904,7 +899,6 @@ async fn make_event_stream( #[case] events_per_block: Vec, #[case] expected_stream: Vec>, ) { - let _ = env_logger::builder().is_test(true).try_init(); let (peers, responses) = unzip_fixtures(responses); let get_peers = move || { let peers = peers.clone(); diff --git a/crates/p2p/src/tests.rs b/crates/p2p/src/tests.rs index 250dda7924..23d98c4ee8 100644 --- a/crates/p2p/src/tests.rs +++ b/crates/p2p/src/tests.rs @@ -98,7 +98,6 @@ async fn client_to_server() -> (TestPeer, TestPeer) { #[test_log::test(tokio::test)] async fn dial() { - let _ = env_logger::builder().is_test(true).try_init(); // tokio::time::pause() does not make a difference let mut peer1 = TestPeer::default(); let mut peer2 = TestPeer::default(); @@ -119,8 +118,6 @@ async fn dial() { #[test_log::test(tokio::test)] async fn disconnect() { - let _ = env_logger::builder().is_test(true).try_init(); - let mut peer1 = TestPeer::default(); let mut peer2 = TestPeer::default(); @@ -157,8 +154,6 @@ async fn disconnect() { #[test_log::test(tokio::test)] async fn periodic_bootstrap() { - let _ = env_logger::builder().is_test(true).try_init(); - const BOOTSTRAP_PERIOD: Duration = Duration::from_millis(500); let cfg = Config { bootstrap_period: Some(BOOTSTRAP_PERIOD), @@ -785,7 +780,6 @@ async fn rate_limit() { #[case::client_to_server(client_to_server().await)] #[test_log::test(tokio::test)] async fn provide_capability(#[case] peers: (TestPeer, TestPeer)) { - let _ = env_logger::builder().is_test(true).try_init(); let (peer1, peer2) = peers; let mut peer1_started_providing = filter_events(peer1.event_receiver, |event| match event { @@ -811,7 +805,6 @@ async fn provide_capability(#[case] peers: (TestPeer, TestPeer)) { #[case::client_to_server(client_to_server().await)] #[test_log::test(tokio::test)] async fn subscription_and_propagation(#[case] peers: (TestPeer, TestPeer)) { - let _ = env_logger::builder().is_test(true).try_init(); let (peer1, peer2) = peers; let mut peer2_subscribed_to_peer1 = filter_events(peer1.event_receiver, |event| match event { @@ -855,7 +848,6 @@ mod successful_sync { #[case::client_to_server(client_to_server().await)] #[test_log::test(tokio::test)] async fn $test_name(#[case] peers: (TestPeer, TestPeer)) { - let _ = env_logger::builder().is_test(true).try_init(); let (peer1, peer2) = peers; // Fake some request for peer2 to send to peer1 let expected_request = Faker.fake::<$req_type>(); From eee1d35981247cccf057926caa00057ca9900894 Mon Sep 17 00:00:00 2001 From: Krisztian Kovacs Date: Wed, 4 Dec 2024 12:33:28 +0100 Subject: [PATCH 5/6] chore(cargo): bump version to 0.15.2 --- Cargo.lock | 46 ++++++++++++++++++------------------- Cargo.toml | 2 +- crates/load-test/Cargo.lock | 2 +- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f3c49b8619..434a38a1c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4809,7 +4809,7 @@ checksum = "42012b0f064e01aa58b545fe3727f90f7dd4020f4a3ea735b50344965f5a57e9" [[package]] name = "gateway-test-utils" -version = "0.15.1" +version = "0.15.2" dependencies = [ "reqwest", "serde_json", @@ -6538,7 +6538,7 @@ dependencies = [ [[package]] name = "make-stream" -version = "0.15.1" +version = "0.15.2" dependencies = [ "tokio", "tokio-stream", @@ -7139,7 +7139,7 @@ checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" [[package]] name = "p2p" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "async-trait", @@ -7180,7 +7180,7 @@ dependencies = [ [[package]] name = "p2p_proto" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "fake", @@ -7201,7 +7201,7 @@ dependencies = [ [[package]] name = "p2p_proto_derive" -version = "0.15.1" +version = "0.15.2" dependencies = [ "proc-macro2", "quote", @@ -7210,7 +7210,7 @@ dependencies = [ [[package]] name = "p2p_stream" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "async-trait", @@ -7339,7 +7339,7 @@ checksum = "17359afc20d7ab31fdb42bb844c8b3bb1dabd7dcf7e68428492da7f16966fcef" [[package]] name = "pathfinder" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "assert_matches", @@ -7407,7 +7407,7 @@ dependencies = [ [[package]] name = "pathfinder-block-hashes" -version = "0.15.1" +version = "0.15.2" dependencies = [ "pathfinder-common", "pathfinder-crypto", @@ -7415,7 +7415,7 @@ dependencies = [ [[package]] name = "pathfinder-common" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "bitvec", @@ -7440,7 +7440,7 @@ dependencies = [ [[package]] name = "pathfinder-compiler" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "cairo-lang-starknet 1.0.0-alpha.6", @@ -7461,7 +7461,7 @@ dependencies = [ [[package]] name = "pathfinder-crypto" -version = "0.15.1" +version = "0.15.2" dependencies = [ "ark-ff 0.5.0", "assert_matches", @@ -7478,7 +7478,7 @@ dependencies = [ [[package]] name = "pathfinder-ethereum" -version = "0.15.1" +version = "0.15.2" dependencies = [ "alloy", "anyhow", @@ -7498,7 +7498,7 @@ dependencies = [ [[package]] name = "pathfinder-executor" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "blockifier", @@ -7518,7 +7518,7 @@ dependencies = [ [[package]] name = "pathfinder-merkle-tree" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "bitvec", @@ -7534,7 +7534,7 @@ dependencies = [ [[package]] name = "pathfinder-retry" -version = "0.15.1" +version = "0.15.2" dependencies = [ "tokio", "tokio-retry", @@ -7542,7 +7542,7 @@ dependencies = [ [[package]] name = "pathfinder-rpc" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "assert_matches", @@ -7595,7 +7595,7 @@ dependencies = [ [[package]] name = "pathfinder-serde" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "num-bigint 0.4.6", @@ -7610,7 +7610,7 @@ dependencies = [ [[package]] name = "pathfinder-storage" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "assert_matches", @@ -9455,7 +9455,7 @@ dependencies = [ [[package]] name = "starknet-gateway-client" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "assert_matches", @@ -9488,7 +9488,7 @@ dependencies = [ [[package]] name = "starknet-gateway-test-fixtures" -version = "0.15.1" +version = "0.15.2" dependencies = [ "pathfinder-common", "pathfinder-crypto", @@ -9496,7 +9496,7 @@ dependencies = [ [[package]] name = "starknet-gateway-types" -version = "0.15.1" +version = "0.15.2" dependencies = [ "anyhow", "assert_matches", @@ -9735,7 +9735,7 @@ dependencies = [ [[package]] name = "tagged" -version = "0.15.1" +version = "0.15.2" dependencies = [ "fake", "pretty_assertions_sorted", @@ -9744,7 +9744,7 @@ dependencies = [ [[package]] name = "tagged-debug-derive" -version = "0.15.1" +version = "0.15.2" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 4f679ac66f..5e4c82e1c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,7 @@ lto = true opt-level = 3 [workspace.package] -version = "0.15.1" +version = "0.15.2" edition = "2021" license = "MIT OR Apache-2.0" rust-version = "1.80" diff --git a/crates/load-test/Cargo.lock b/crates/load-test/Cargo.lock index a3b5d7a88d..8cdb69a37a 100644 --- a/crates/load-test/Cargo.lock +++ b/crates/load-test/Cargo.lock @@ -976,7 +976,7 @@ dependencies = [ [[package]] name = "pathfinder-crypto" -version = "0.15.1" +version = "0.15.2" dependencies = [ "bitvec", "fake", From db0ba544e4184f22e552d5f1dd13f7bab991b971 Mon Sep 17 00:00:00 2001 From: Krisztian Kovacs Date: Wed, 4 Dec 2024 12:33:40 +0100 Subject: [PATCH 6/6] chore: update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d3e13af38..845cbbfba7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,12 @@ More expansive patch notes and explanations may be found in the specific [pathfi The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## [0.15.2] - 2024-12-04 ### Fixed - Pathfinder fails to properly do a reorg due to a SQL statement referring a table that does not exist. +- `--rpc.get-events-max-uncached-bloom-filters-to-load` setting is ineffective. ## [0.15.1] - 2024-12-02