Skip to content

Commit

Permalink
[consensus] Remove all the warnings (#14178)
Browse files Browse the repository at this point in the history
Many devs have missed these warnings, and it properly deletes or
moves code to test only code.
  • Loading branch information
gregnazario authored Aug 1, 2024
1 parent 52e4a0e commit 6a71f18
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 101 deletions.
1 change: 1 addition & 0 deletions consensus/src/block_storage/block_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PipelinedBlock> {
self.inner.read().highest_certified_block()
}
Expand Down
1 change: 1 addition & 0 deletions consensus/src/block_storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub trait BlockReader: Send + Sync {
fn path_from_commit_root(&self, block_id: HashValue) -> Option<Vec<Arc<PipelinedBlock>>>;

/// Return the certified block with the highest round.
#[cfg(test)]
fn highest_certified_block(&self) -> Arc<PipelinedBlock>;

/// Return the quorum certificate with the highest round
Expand Down
3 changes: 2 additions & 1 deletion consensus/src/epoch_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1798,7 +1798,7 @@ fn load_dkg_decrypt_key(
}

#[derive(Debug)]
enum NoRandomnessReason {
pub enum NoRandomnessReason {
VTxnDisabled,
FeatureDisabled,
DKGStateResourceMissing(anyhow::Error),
Expand All @@ -1813,4 +1813,5 @@ enum NoRandomnessReason {
KeyPairDeserializationError(bcs::Error),
KeyPairSerializationError(bcs::Error),
KeyPairPersistError(anyhow::Error),
// Test only reasons
}
16 changes: 0 additions & 16 deletions consensus/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,17 +170,13 @@ pub struct NetworkReceivers {

#[async_trait::async_trait]
pub trait QuorumStoreSender: Send + Clone {
async fn send_batch_request(&self, request: BatchRequest, recipients: Vec<Author>);

async fn request_batch(
&self,
request: BatchRequest,
recipient: Author,
timeout: Duration,
) -> anyhow::Result<BatchResponse>;

async fn send_batch(&self, batch: Batch, recipients: Vec<Author>);

async fn send_signed_batch_info_msg(
&self,
signed_batch_infos: Vec<SignedBatchInfo>,
Expand Down Expand Up @@ -474,12 +470,6 @@ impl NetworkSender {

#[async_trait::async_trait]
impl QuorumStoreSender for NetworkSender {
async fn send_batch_request(&self, request: BatchRequest, recipients: Vec<Author>) {
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,
Expand All @@ -506,12 +496,6 @@ impl QuorumStoreSender for NetworkSender {
}
}

async fn send_batch(&self, batch: Batch, recipients: Vec<Author>) {
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<SignedBatchInfo>,
Expand Down
2 changes: 0 additions & 2 deletions consensus/src/payload_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,4 @@ pub trait PayloadClient: Send + Sync {
recent_max_fill_fraction: f32,
block_timestamp: Duration,
) -> anyhow::Result<(Vec<ValidatorTransaction>, Payload), QuorumStoreError>;

fn trace_payloads(&self) {}
}
64 changes: 38 additions & 26 deletions consensus/src/quorum_store/quorum_store_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<HashValue>) -> Result<(), DbError> {
Ok(())
impl MockQuorumStoreDB {
pub fn new() -> Self {
Self {}
}
}

fn get_all_batches(&self) -> Result<HashMap<HashValue, PersistedValue>> {
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<HashValue>) -> Result<(), DbError> {
Ok(())
}

fn get_batch(&self, _: &HashValue) -> Result<Option<PersistedValue>, DbError> {
Ok(None)
}
fn get_all_batches(&self) -> Result<HashMap<HashValue, PersistedValue>> {
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<Option<BatchId>, DbError> {
Ok(Some(BatchId::new_for_test(0)))
}
fn get_batch(&self, _: &HashValue) -> Result<Option<PersistedValue>, 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<Option<BatchId>, DbError> {
Ok(Some(BatchId::new_for_test(0)))
}

fn save_batch_id(&self, _: u64, _: BatchId) -> Result<(), DbError> {
Ok(())
}
}
}
8 changes: 0 additions & 8 deletions consensus/src/quorum_store/tests/batch_requester_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@ impl MockBatchRequester {

#[async_trait::async_trait]
impl QuorumStoreSender for MockBatchRequester {
async fn send_batch_request(&self, _request: BatchRequest, _recipients: Vec<Author>) {
unimplemented!()
}

async fn request_batch(
&self,
_request: BatchRequest,
Expand All @@ -50,10 +46,6 @@ impl QuorumStoreSender for MockBatchRequester {
Ok(self.return_value.clone())
}

async fn send_batch(&self, _batch: Batch, _recipients: Vec<Author>) {
unimplemented!()
}

async fn send_signed_batch_info_msg(
&self,
_signed_batch_infos: Vec<SignedBatchInfo>,
Expand Down
1 change: 1 addition & 0 deletions consensus/src/quorum_store/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ impl ProofQueue {
}
}

#[cfg(test)]
pub(crate) fn batch_summaries_len(&self) -> usize {
self.batch_summaries.len()
}
Expand Down
23 changes: 2 additions & 21 deletions consensus/src/state_replication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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<Randomness>,
) -> ExecutorResult<PipelineExecutionResult> {
self.schedule_compute(block, parent_block_id, randomness)
.await
.await
}

async fn schedule_compute(
&self,
// The block that will be computed.
Expand Down
14 changes: 0 additions & 14 deletions consensus/src/test_utils/mock_quorum_store_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ impl MockQuorumStoreSender {

#[async_trait::async_trait]
impl QuorumStoreSender for MockQuorumStoreSender {
async fn send_batch_request(&self, request: BatchRequest, recipients: Vec<Author>) {
self.tx
.send((ConsensusMsg::BatchRequestMsg(Box::new(request)), recipients))
.await
.expect("could not send");
}

async fn request_batch(
&self,
_request: BatchRequest,
Expand All @@ -42,13 +35,6 @@ impl QuorumStoreSender for MockQuorumStoreSender {
unimplemented!();
}

async fn send_batch(&self, batch: Batch, recipients: Vec<Author>) {
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<SignedBatchInfo>,
Expand Down
13 changes: 0 additions & 13 deletions consensus/src/test_utils/mock_state_computer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Randomness>,
) -> ExecutorResult<PipelineExecutionResult> {
Ok(PipelineExecutionResult::new(
vec![],
StateComputeResult::new_dummy(),
Duration::from_secs(0),
))
}

async fn commit(
&self,
blocks: &[Arc<PipelinedBlock>],
Expand Down

0 comments on commit 6a71f18

Please sign in to comment.