From 6a71f18b8091e67ff567fe47715176c06a508917 Mon Sep 17 00:00:00 2001 From: Greg Nazario Date: Wed, 31 Jul 2024 22:51:13 -0700 Subject: [PATCH] [consensus] Remove all the warnings (#14178) Many devs have missed these warnings, and it properly deletes or moves code to test only code. --- consensus/src/block_storage/block_store.rs | 1 + consensus/src/block_storage/mod.rs | 1 + consensus/src/epoch_manager.rs | 3 +- consensus/src/network.rs | 16 ----- consensus/src/payload_client/mod.rs | 2 - consensus/src/quorum_store/quorum_store_db.rs | 64 +++++++++++-------- .../tests/batch_requester_test.rs | 8 --- consensus/src/quorum_store/utils.rs | 1 + consensus/src/state_replication.rs | 23 +------ .../test_utils/mock_quorum_store_sender.rs | 14 ---- .../src/test_utils/mock_state_computer.rs | 13 ---- 11 files changed, 45 insertions(+), 101 deletions(-) diff --git a/consensus/src/block_storage/block_store.rs b/consensus/src/block_storage/block_store.rs index 087e3f62acb2e..12c5959fe7e2f 100644 --- a/consensus/src/block_storage/block_store.rs +++ b/consensus/src/block_storage/block_store.rs @@ -513,6 +513,7 @@ impl BlockReader for BlockStore { self.inner.read().path_from_commit_root(block_id) } + #[cfg(test)] fn highest_certified_block(&self) -> Arc { self.inner.read().highest_certified_block() } diff --git a/consensus/src/block_storage/mod.rs b/consensus/src/block_storage/mod.rs index 5a95b72d9b6db..51e6520f3d05e 100644 --- a/consensus/src/block_storage/mod.rs +++ b/consensus/src/block_storage/mod.rs @@ -48,6 +48,7 @@ pub trait BlockReader: Send + Sync { fn path_from_commit_root(&self, block_id: HashValue) -> Option>>; /// Return the certified block with the highest round. + #[cfg(test)] fn highest_certified_block(&self) -> Arc; /// Return the quorum certificate with the highest round diff --git a/consensus/src/epoch_manager.rs b/consensus/src/epoch_manager.rs index fb5b64bd44f1e..cf7d87c425dc5 100644 --- a/consensus/src/epoch_manager.rs +++ b/consensus/src/epoch_manager.rs @@ -1798,7 +1798,7 @@ fn load_dkg_decrypt_key( } #[derive(Debug)] -enum NoRandomnessReason { +pub enum NoRandomnessReason { VTxnDisabled, FeatureDisabled, DKGStateResourceMissing(anyhow::Error), @@ -1813,4 +1813,5 @@ enum NoRandomnessReason { KeyPairDeserializationError(bcs::Error), KeyPairSerializationError(bcs::Error), KeyPairPersistError(anyhow::Error), + // Test only reasons } diff --git a/consensus/src/network.rs b/consensus/src/network.rs index 13dcf905784a7..698e089638513 100644 --- a/consensus/src/network.rs +++ b/consensus/src/network.rs @@ -170,8 +170,6 @@ pub struct NetworkReceivers { #[async_trait::async_trait] pub trait QuorumStoreSender: Send + Clone { - async fn send_batch_request(&self, request: BatchRequest, recipients: Vec); - async fn request_batch( &self, request: BatchRequest, @@ -179,8 +177,6 @@ pub trait QuorumStoreSender: Send + Clone { timeout: Duration, ) -> anyhow::Result; - async fn send_batch(&self, batch: Batch, recipients: Vec); - async fn send_signed_batch_info_msg( &self, signed_batch_infos: Vec, @@ -474,12 +470,6 @@ impl NetworkSender { #[async_trait::async_trait] impl QuorumStoreSender for NetworkSender { - async fn send_batch_request(&self, request: BatchRequest, recipients: Vec) { - fail_point!("consensus::send::batch_request", |_| ()); - let msg = ConsensusMsg::BatchRequestMsg(Box::new(request)); - self.send(msg, recipients).await - } - async fn request_batch( &self, request: BatchRequest, @@ -506,12 +496,6 @@ impl QuorumStoreSender for NetworkSender { } } - async fn send_batch(&self, batch: Batch, recipients: Vec) { - fail_point!("consensus::send::batch", |_| ()); - let msg = ConsensusMsg::BatchResponse(Box::new(batch)); - self.send(msg, recipients).await - } - async fn send_signed_batch_info_msg( &self, signed_batch_infos: Vec, diff --git a/consensus/src/payload_client/mod.rs b/consensus/src/payload_client/mod.rs index cef8293d7500f..c91ee0cb5fed8 100644 --- a/consensus/src/payload_client/mod.rs +++ b/consensus/src/payload_client/mod.rs @@ -30,6 +30,4 @@ pub trait PayloadClient: Send + Sync { recent_max_fill_fraction: f32, block_timestamp: Duration, ) -> anyhow::Result<(Vec, Payload), QuorumStoreError>; - - fn trace_payloads(&self) {} } diff --git a/consensus/src/quorum_store/quorum_store_db.rs b/consensus/src/quorum_store/quorum_store_db.rs index ba9221cdd0b3d..fccb002fc0b32 100644 --- a/consensus/src/quorum_store/quorum_store_db.rs +++ b/consensus/src/quorum_store/quorum_store_db.rs @@ -122,41 +122,53 @@ impl QuorumStoreStorage for QuorumStoreDB { } } -pub(crate) struct MockQuorumStoreDB {} +#[cfg(test)] +pub(crate) use mock::MockQuorumStoreDB; -impl MockQuorumStoreDB { - #[cfg(test)] - pub fn new() -> Self { - Self {} - } -} +#[cfg(test)] +pub mod mock { + use super::*; + pub struct MockQuorumStoreDB {} -impl QuorumStoreStorage for MockQuorumStoreDB { - fn delete_batches(&self, _: Vec) -> Result<(), DbError> { - Ok(()) + impl MockQuorumStoreDB { + pub fn new() -> Self { + Self {} + } } - fn get_all_batches(&self) -> Result> { - Ok(HashMap::new()) + impl Default for MockQuorumStoreDB { + fn default() -> Self { + Self::new() + } } - fn save_batch(&self, _: PersistedValue) -> Result<(), DbError> { - Ok(()) - } + impl QuorumStoreStorage for MockQuorumStoreDB { + fn delete_batches(&self, _: Vec) -> Result<(), DbError> { + Ok(()) + } - fn get_batch(&self, _: &HashValue) -> Result, DbError> { - Ok(None) - } + fn get_all_batches(&self) -> Result> { + Ok(HashMap::new()) + } - fn delete_batch_id(&self, _: u64) -> Result<(), DbError> { - Ok(()) - } + fn save_batch(&self, _: PersistedValue) -> Result<(), DbError> { + Ok(()) + } - fn clean_and_get_batch_id(&self, _: u64) -> Result, DbError> { - Ok(Some(BatchId::new_for_test(0))) - } + fn get_batch(&self, _: &HashValue) -> Result, DbError> { + Ok(None) + } - fn save_batch_id(&self, _: u64, _: BatchId) -> Result<(), DbError> { - Ok(()) + fn delete_batch_id(&self, _: u64) -> Result<(), DbError> { + Ok(()) + } + + fn clean_and_get_batch_id(&self, _: u64) -> Result, DbError> { + Ok(Some(BatchId::new_for_test(0))) + } + + fn save_batch_id(&self, _: u64, _: BatchId) -> Result<(), DbError> { + Ok(()) + } } } diff --git a/consensus/src/quorum_store/tests/batch_requester_test.rs b/consensus/src/quorum_store/tests/batch_requester_test.rs index d11f00e85d630..e9975b35eaeef 100644 --- a/consensus/src/quorum_store/tests/batch_requester_test.rs +++ b/consensus/src/quorum_store/tests/batch_requester_test.rs @@ -37,10 +37,6 @@ impl MockBatchRequester { #[async_trait::async_trait] impl QuorumStoreSender for MockBatchRequester { - async fn send_batch_request(&self, _request: BatchRequest, _recipients: Vec) { - unimplemented!() - } - async fn request_batch( &self, _request: BatchRequest, @@ -50,10 +46,6 @@ impl QuorumStoreSender for MockBatchRequester { Ok(self.return_value.clone()) } - async fn send_batch(&self, _batch: Batch, _recipients: Vec) { - unimplemented!() - } - async fn send_signed_batch_info_msg( &self, _signed_batch_infos: Vec, diff --git a/consensus/src/quorum_store/utils.rs b/consensus/src/quorum_store/utils.rs index 790613b02f20e..6e631e06afe2a 100644 --- a/consensus/src/quorum_store/utils.rs +++ b/consensus/src/quorum_store/utils.rs @@ -256,6 +256,7 @@ impl ProofQueue { } } + #[cfg(test)] pub(crate) fn batch_summaries_len(&self) -> usize { self.batch_summaries.len() } diff --git a/consensus/src/state_replication.rs b/consensus/src/state_replication.rs index fde757e1365bc..881213fca6d09 100644 --- a/consensus/src/state_replication.rs +++ b/consensus/src/state_replication.rs @@ -3,11 +3,8 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{ - error::StateSyncError, - payload_manager::TPayloadManager, - state_computer::{PipelineExecutionResult, StateComputeResultFut}, - transaction_deduper::TransactionDeduper, - transaction_shuffler::TransactionShuffler, + error::StateSyncError, payload_manager::TPayloadManager, state_computer::StateComputeResultFut, + transaction_deduper::TransactionDeduper, transaction_shuffler::TransactionShuffler, }; use anyhow::Result; use aptos_consensus_types::{block::Block, pipelined_block::PipelinedBlock}; @@ -27,22 +24,6 @@ pub type StateComputerCommitCallBackType = /// StateComputer is using proposed block ids for identifying the transactions. #[async_trait::async_trait] pub trait StateComputer: Send + Sync { - /// How to execute a sequence of transactions and obtain the next state. While some of the - /// transactions succeed, some of them can fail. - /// In case all the transactions are failed, new_state_id is equal to the previous state id. - async fn compute( - &self, - // The block that will be computed. - block: &Block, - // The parent block root hash. - parent_block_id: HashValue, - randomness: Option, - ) -> ExecutorResult { - self.schedule_compute(block, parent_block_id, randomness) - .await - .await - } - async fn schedule_compute( &self, // The block that will be computed. diff --git a/consensus/src/test_utils/mock_quorum_store_sender.rs b/consensus/src/test_utils/mock_quorum_store_sender.rs index 616bb29ac1b8c..bd962d348b51f 100644 --- a/consensus/src/test_utils/mock_quorum_store_sender.rs +++ b/consensus/src/test_utils/mock_quorum_store_sender.rs @@ -26,13 +26,6 @@ impl MockQuorumStoreSender { #[async_trait::async_trait] impl QuorumStoreSender for MockQuorumStoreSender { - async fn send_batch_request(&self, request: BatchRequest, recipients: Vec) { - self.tx - .send((ConsensusMsg::BatchRequestMsg(Box::new(request)), recipients)) - .await - .expect("could not send"); - } - async fn request_batch( &self, _request: BatchRequest, @@ -42,13 +35,6 @@ impl QuorumStoreSender for MockQuorumStoreSender { unimplemented!(); } - async fn send_batch(&self, batch: Batch, recipients: Vec) { - self.tx - .send((ConsensusMsg::BatchResponse(Box::new(batch)), recipients)) - .await - .expect("could not send"); - } - async fn send_signed_batch_info_msg( &self, signed_batch_infos: Vec, diff --git a/consensus/src/test_utils/mock_state_computer.rs b/consensus/src/test_utils/mock_state_computer.rs index 16f226ef3f253..50d377fa7d794 100644 --- a/consensus/src/test_utils/mock_state_computer.rs +++ b/consensus/src/test_utils/mock_state_computer.rs @@ -36,19 +36,6 @@ impl EmptyStateComputer { #[async_trait::async_trait] impl StateComputer for EmptyStateComputer { - async fn compute( - &self, - _block: &Block, - _parent_block_id: HashValue, - _randomness: Option, - ) -> ExecutorResult { - Ok(PipelineExecutionResult::new( - vec![], - StateComputeResult::new_dummy(), - Duration::from_secs(0), - )) - } - async fn commit( &self, blocks: &[Arc],