From 8529fdefc15d3cd87910ff9c55c62d6174b56b7b Mon Sep 17 00:00:00 2001 From: Grzegorz Prusak Date: Mon, 4 Dec 2023 11:08:37 +0100 Subject: [PATCH] added validate_proposal test --- node/actors/bft/src/replica/tests.rs | 42 ++++++++++++---------- node/actors/bft/src/testonly/ut_harness.rs | 22 +++++------- node/libs/storage/src/traits.rs | 9 +++-- 3 files changed, 36 insertions(+), 37 deletions(-) diff --git a/node/actors/bft/src/replica/tests.rs b/node/actors/bft/src/replica/tests.rs index 17127923..fc30e110 100644 --- a/node/actors/bft/src/replica/tests.rs +++ b/node/actors/bft/src/replica/tests.rs @@ -6,8 +6,9 @@ use crate::{inner::ConsensusInner, leader::ReplicaPrepareError, testonly::ut_har use assert_matches::assert_matches; use rand::Rng; use zksync_consensus_roles::validator::{ + self, LeaderPrepare, Payload, ReplicaCommit, - ReplicaPrepare, ViewNumber, + ReplicaPrepare, ViewNumber, CommitQC, }; #[tokio::test] @@ -118,29 +119,34 @@ async fn leader_prepare_old_view() { ); } -/* /// Tests that `WriteBlockStore::verify_payload` is applied before signing a vote. #[tokio::test] async fn leader_prepare_invalid_payload() { - let mut util = UTHarness::new_one().await; - let leader_prepare = util.new_procedural_leader_prepare_one().await; - let mut leader_prepare = util - .new_procedural_leader_prepare_one() - .await - .cast::() - .unwrap() - .msg; - let leader_prepare = util - .owner_key() - .sign_msg(ConsensusMsg::LeaderPrepare(leader_prepare)); + zksync_concurrency::testonly::abort_on_panic(); + let ctx = &ctx::test_root(&ctx::RealClock); + let mut util = UTHarness::new(ctx,1).await; + let leader_prepare = util.new_procedural_leader_prepare(ctx).await; + // Insert a finalized block to the storage. + // Default implementation of verify_payload() fails if + // head block number >= proposal block number. + let block = validator::FinalBlock { + header: leader_prepare.msg.proposal.clone(), + payload: leader_prepare.msg.proposal_payload.clone().unwrap(), + justification: CommitQC::from( + &[util.keys[0].sign_msg(ReplicaCommit { + protocol_version: validator::ProtocolVersion::EARLIEST, + view: util.consensus.replica.view, + proposal: leader_prepare.msg.proposal.clone(), + })], + &util.validator_set(), + ).unwrap(), + }; + util.consensus.replica.storage.put_block(ctx,&block).await.unwrap(); let res = util.process_leader_prepare(ctx,leader_prepare).await; - assert_matches!( - res, - Err(LeaderPrepareError::ProposalInvalidPayload(..)), - ); -}*/ + assert_matches!(res,Err(LeaderPrepareError::ProposalInvalidPayload(..))); +} #[tokio::test] async fn leader_prepare_invalid_sig() { diff --git a/node/actors/bft/src/testonly/ut_harness.rs b/node/actors/bft/src/testonly/ut_harness.rs index 402f63a0..c197fbf4 100644 --- a/node/actors/bft/src/testonly/ut_harness.rs +++ b/node/actors/bft/src/testonly/ut_harness.rs @@ -286,38 +286,32 @@ impl UTHarness { self.consensus.inner.view_leader(view) } + pub(crate) fn validator_set(&self) -> validator::ValidatorSet { + validator::ValidatorSet::new(self.keys.iter().map(|k| k.public())).unwrap() + } + pub(crate) fn new_commit_qc(&self, mutate_fn: impl FnOnce(&mut ReplicaCommit)) -> CommitQC { - let validator_set = - validator::ValidatorSet::new(self.keys.iter().map(|k| k.public())).unwrap(); let msg = self.new_current_replica_commit(mutate_fn).msg; let msgs: Vec<_> = self.keys.iter().map(|k| k.sign_msg(msg.clone())).collect(); - CommitQC::from(&msgs, &validator_set).unwrap() + CommitQC::from(&msgs, &self.validator_set()).unwrap() } pub(crate) fn new_prepare_qc(&self, mutate_fn: impl FnOnce(&mut ReplicaPrepare)) -> PrepareQC { - let validator_set = - validator::ValidatorSet::new(self.keys.iter().map(|k| k.public())).unwrap(); let msg = self.new_current_replica_prepare(mutate_fn).msg; let msgs: Vec<_> = self.keys.iter().map(|k| k.sign_msg(msg.clone())).collect(); - PrepareQC::from(&msgs, &validator_set).unwrap() + PrepareQC::from(&msgs, &self.validator_set()).unwrap() } pub(crate) fn new_prepare_qc_many( &mut self, mut mutate_fn: impl FnMut(&mut ReplicaPrepare), ) -> PrepareQC { - let validator_set = - validator::ValidatorSet::new(self.keys.iter().map(|k| k.public())).unwrap(); - - let signed_messages: Vec<_> = self - .keys - .iter() + let msgs: Vec<_> = self.keys.iter() .map(|sk| { let msg = self.new_current_replica_prepare(|msg| mutate_fn(msg)).msg; sk.sign_msg(msg) }) .collect(); - - PrepareQC::from(&signed_messages, &validator_set).unwrap() + PrepareQC::from(&msgs, &self.validator_set()).unwrap() } } diff --git a/node/libs/storage/src/traits.rs b/node/libs/storage/src/traits.rs index b2b184eb..20bb2c64 100644 --- a/node/libs/storage/src/traits.rs +++ b/node/libs/storage/src/traits.rs @@ -86,12 +86,11 @@ pub trait WriteBlockStore: BlockStore { &self, ctx: &ctx::Ctx, block_number: BlockNumber, - payload: &Payload, + _payload: &Payload, ) -> ctx::Result<()> { - if let Some(block) = self.block(ctx,block_number).await? { - if &block.payload!=payload { - Err(anyhow::anyhow!("block already stored with different payload"))?; - } + let head_number = self.head_block(ctx).await?.header.number; + if head_number >= block_number { + Err(anyhow::anyhow!("received proposal for block {block_number:?}, while head is at {head_number:?}"))?; } Ok(()) }