From cee8d5979a774e701610fb56847bb078a3757fb8 Mon Sep 17 00:00:00 2001 From: Grzegorz Prusak Date: Tue, 1 Oct 2024 11:34:27 +0200 Subject: [PATCH] invalid justification test --- node/actors/network/src/config.rs | 2 ++ node/actors/network/src/gossip/runner.rs | 26 +++++++++++++----- .../network/src/rpc/push_block_store_state.rs | 6 +++++ node/libs/storage/src/testonly/in_memory.rs | 5 +++- node/libs/storage/src/tests.rs | 27 ++++++++++++++++++- 5 files changed, 57 insertions(+), 9 deletions(-) diff --git a/node/actors/network/src/config.rs b/node/actors/network/src/config.rs index aa8ebb90..d5e2b87d 100644 --- a/node/actors/network/src/config.rs +++ b/node/actors/network/src/config.rs @@ -111,6 +111,8 @@ pub struct Config { pub tcp_accept_rate: limiter::Rate, /// Rate limiting config for RPCs. pub rpc: RpcConfig, + /// Enables syncing blocks before genesis. + pub enable_pre_genesis_support: bool, /// Maximum number of not-yet-persisted blocks fetched from the network. /// If reached, network actor will wait for more blocks to get persisted /// before fetching the next ones. It is useful for limiting memory consumption diff --git a/node/actors/network/src/gossip/runner.rs b/node/actors/network/src/gossip/runner.rs index 07a5dc72..a692e45c 100644 --- a/node/actors/network/src/gossip/runner.rs +++ b/node/actors/network/src/gossip/runner.rs @@ -90,8 +90,11 @@ impl rpc::Handler for &PushServer<'_> { async fn handle( &self, _ctx: &ctx::Ctx, - req: rpc::push_block_store_state::Req, + mut req: rpc::push_block_store_state::Req, ) -> anyhow::Result<()> { + if !self.net.cfg.enable_pre_genesis_support { + req.clear_pre_genesis_info(); + } let state = req.state(); state.verify(self.net.genesis())?; self.blocks.send_replace(state); @@ -109,7 +112,11 @@ impl rpc::Handler for &BlockStore { ctx: &ctx::Ctx, req: rpc::get_block::Req, ) -> anyhow::Result { - Ok(rpc::get_block::Resp(self.block(ctx, req.0).await?)) + let mut resp = rpc::get_block::Resp(self.block(ctx, req.0).await?; + if !self.cfg.enable_pre_genesis_support { + resp.clear_pre_genesis_info(); + } + Ok(resp) } } @@ -205,7 +212,10 @@ impl Network { s.spawn::<()>(async { let mut state = self.block_store.queued(); loop { - let req = rpc::push_block_store_state::Req::new(state.clone(), self.genesis()); + let mut req = rpc::push_block_store_state::Req::new(state.clone(), self.genesis()); + if !self.cfg.enable_pre_genesis_support { + req.clear_pre_genesis_info(); + } push_block_store_state_client.call(ctx, &req, kB).await?; state = self.block_store.wait_for_queued_change(ctx, &state).await?; } @@ -251,11 +261,13 @@ impl Network { let ctx_with_timeout = self.cfg.rpc.get_block_timeout.map(|t| ctx.with_timeout(t)); let ctx = ctx_with_timeout.as_ref().unwrap_or(ctx); - let block = call + let mut resp = call .call(ctx, &req, self.cfg.max_block_size.saturating_add(kB)) - .await? - .0 - .context("empty response")?; + .await?; + if self.cfg.enable_pre_genesis_support { + resp.0.clear_pre_genesis_info(); + } + let block = resp.0.context("empty response")?; anyhow::ensure!(block.number() == req.0, "received wrong block"); // Storing the block will fail in case block is invalid. self.block_store diff --git a/node/actors/network/src/rpc/push_block_store_state.rs b/node/actors/network/src/rpc/push_block_store_state.rs index 9518322c..62a1783c 100644 --- a/node/actors/network/src/rpc/push_block_store_state.rs +++ b/node/actors/network/src/rpc/push_block_store_state.rs @@ -54,6 +54,12 @@ impl Req { }, } } + + /// Clears pre-genesis info from the request. + /// Use to simulate node behavior before pre-genesis support. + pub(crate) fn clear_pre_genesis_info(&mut self) { + self.state = None; + } } impl ProtoRepr for proto::Last { diff --git a/node/libs/storage/src/testonly/in_memory.rs b/node/libs/storage/src/testonly/in_memory.rs index 48b9de29..fad3dbf2 100644 --- a/node/libs/storage/src/testonly/in_memory.rs +++ b/node/libs/storage/src/testonly/in_memory.rs @@ -104,7 +104,10 @@ impl PersistentBlockStore for BlockStore { Ok(blocks.get(idx as usize).context("not found")?.clone()) } - async fn queue_next_block(&self, _ctx: &ctx::Ctx, block: validator::Block) -> ctx::Result<()> { + async fn queue_next_block(&self, ctx: &ctx::Ctx, block: validator::Block) -> ctx::Result<()> { + if let validator::Block::PreGenesis(b) = &block { + self.verify_pre_genesis_block(ctx, b).await?; + } let mut blocks = self.0.blocks.lock().unwrap(); let want = self.0.persisted.borrow().next(); if block.number() < want { diff --git a/node/libs/storage/src/tests.rs b/node/libs/storage/src/tests.rs index 9c0d1d7d..44b53fd4 100644 --- a/node/libs/storage/src/tests.rs +++ b/node/libs/storage/src/tests.rs @@ -1,7 +1,8 @@ use super::*; use crate::{testonly::TestMemoryStorage, ReplicaState}; use zksync_concurrency::{ctx, scope, sync, testonly::abort_on_panic}; -use zksync_consensus_roles::validator::testonly::Setup; +use zksync_consensus_roles::{validator,validator::testonly::{SetupSpec,Setup}}; +use rand::Rng as _; #[tokio::test] async fn test_inmemory_block_store() { @@ -27,6 +28,30 @@ async fn test_inmemory_block_store() { } } +// Test checking that store doesn't accept pre-genesis blocks with invalid justification. +#[tokio::test] +async fn test_invalid_justification() { + abort_on_panic(); + let ctx = &ctx::test_root(&ctx::RealClock); + let rng = &mut ctx.rng(); + let mut spec = SetupSpec::new(rng, 1); + spec.first_block = spec.first_pregenesis_block+2; + let setup = Setup::from_spec(rng, spec); + scope::run!(ctx, |ctx,s| async { + let store = TestMemoryStorage::new(ctx, &setup).await; + s.spawn_bg(store.runner.run(ctx)); + let store = store.blocks; + // Insert a correct block first. + store.queue_block(ctx, setup.blocks[0].clone()).await.unwrap(); + // Insert an incorrect second block. + let validator::Block::PreGenesis(mut b) = setup.blocks[1].clone() else { panic!() }; + b.justification = rng.gen(); + store.queue_block(ctx, b.into()).await.unwrap_err(); + Ok(()) + }).await.unwrap(); +} + + #[test] fn test_schema_encode_decode() { let ctx = ctx::test_root(&ctx::RealClock);