Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename BlockValidator to ProposedBlockValidator #4394

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions node/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ All notable changes to this project will be documented in this file. The format



## Next Unreleased

### Changed
* Rename `BlockValidator` component to `ProposedBlockValidator`, and corresponding config section `block_validator` to `proposed_block_validator`.



## Unreleased

### Added
Expand Down
2 changes: 1 addition & 1 deletion node/src/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@

pub(crate) mod block_accumulator;
pub(crate) mod block_synchronizer;
pub(crate) mod block_validator;
pub mod consensus;
pub mod contract_runtime;
pub(crate) mod deploy_acceptor;
Expand All @@ -55,6 +54,7 @@ pub(crate) mod diagnostics_port;
pub(crate) mod event_stream_server;
pub(crate) mod fetcher;
pub(crate) mod gossiper;
pub(crate) mod proposed_block_validator;
// The `in_memory_network` is public for use in doctests.
#[cfg(test)]
pub mod in_memory_network;
Expand Down
10 changes: 5 additions & 5 deletions node/src/components/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ use crate::{
diagnostics_port::DumpConsensusStateRequest,
incoming::{ConsensusDemand, ConsensusMessageIncoming},
requests::{
BlockValidationRequest, ChainspecRawBytesRequest, ConsensusRequest,
ContractRuntimeRequest, DeployBufferRequest, NetworkInfoRequest, NetworkRequest,
StorageRequest,
ChainspecRawBytesRequest, ConsensusRequest, ContractRuntimeRequest,
DeployBufferRequest, NetworkInfoRequest, NetworkRequest,
ProposedBlockValidationRequest, StorageRequest,
},
EffectBuilder, EffectExt, Effects,
},
Expand Down Expand Up @@ -309,7 +309,7 @@ pub(crate) trait ReactorEventT:
+ From<NetworkInfoRequest>
+ From<DeployBufferRequest>
+ From<ConsensusAnnouncement>
+ From<BlockValidationRequest>
+ From<ProposedBlockValidationRequest>
+ From<StorageRequest>
+ From<ContractRuntimeRequest>
+ From<ChainspecRawBytesRequest>
Expand All @@ -328,7 +328,7 @@ impl<REv> ReactorEventT for REv where
+ From<NetworkInfoRequest>
+ From<DeployBufferRequest>
+ From<ConsensusAnnouncement>
+ From<BlockValidationRequest>
+ From<ProposedBlockValidationRequest>
+ From<StorageRequest>
+ From<ContractRuntimeRequest>
+ From<ChainspecRawBytesRequest>
Expand Down
4 changes: 2 additions & 2 deletions node/src/components/consensus/era_supervisor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use crate::{
},
effect::{
announcements::FatalAnnouncement,
requests::{BlockValidationRequest, ContractRuntimeRequest, StorageRequest},
requests::{ContractRuntimeRequest, ProposedBlockValidationRequest, StorageRequest},
AutoClosingResponder, EffectBuilder, EffectExt, Effects, Responder,
},
fatal, protocol,
Expand Down Expand Up @@ -1371,7 +1371,7 @@ async fn check_deploys_for_replay_in_previous_eras_and_validate_block<REv>(
proposed_block: ProposedBlock<ClContext>,
) -> Event
where
REv: From<BlockValidationRequest> + From<StorageRequest>,
REv: From<ProposedBlockValidationRequest> + From<StorageRequest>,
{
let deploys_era_ids = effect_builder
.get_deploys_era_ids(
Expand Down
9 changes: 5 additions & 4 deletions node/src/components/consensus/highway_core/synchronizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,8 @@ impl<C: Context + 'static> Synchronizer<C> {
// state after `dep` is added, rather than `transitive_dependency`.
self.add_missing_dependency(dep.clone(), pv);
// If we already have the dependency and it is a proposal that is currently being
// handled by the block validator, and this sender is already known as a source,
// do nothing.
// handled by the proposed block validator, and this sender is already known as a
// source, do nothing.
if pending_values
.values()
.flatten()
Expand All @@ -403,8 +403,9 @@ impl<C: Context + 'static> Synchronizer<C> {
continue;
}
// If we already have the dependency and it is a proposal that is currently being
// handled by the block validator, and this sender is not yet known as a source,
// we return the proposal as if this sender had sent it to us, so they get added.
// handled by the proposed block validator, and this sender is not yet known as a
// source, we return the proposal as if this sender had sent it to us, so they get
// added.
if let Some((vv, _)) = pending_values
.values()
.flatten()
Expand Down
6 changes: 3 additions & 3 deletions node/src/components/consensus/protocols/highway.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1039,9 +1039,9 @@ where
.collect();
// recursively remove vertices depending on the dropped ones
let _faulty_senders = self.synchronizer.invalid_vertices(dropped_vertex_ids);
// We don't disconnect from the faulty senders here: The block validator considers the
// value "invalid" even if it just couldn't download the deploys, which could just be
// because the original sender went offline.
// We don't disconnect from the faulty senders here: The proposed block validator
// considers the value "invalid" even if it just couldn't download the deploys, which
// could just be because the original sender went offline.
vec![]
}
}
Expand Down
10 changes: 5 additions & 5 deletions node/src/components/consensus/protocols/zug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1706,8 +1706,8 @@ impl<C: Context + 'static> Zug<C> {
true
}

/// Sends a proposal to the `BlockValidator` component for validation. If no validation is
/// needed, immediately calls `insert_proposal`.
/// Sends a proposal to the `ProposedBlockValidator` component for validation. If no validation
/// is needed, immediately calls `insert_proposal`.
fn validate_proposal(
&mut self,
round_id: RoundId,
Expand Down Expand Up @@ -2276,9 +2276,9 @@ where
outcomes.extend(self.update(now));
} else {
for (round_id, proposal, sender) in rounds_and_node_ids {
// We don't disconnect from the faulty sender here: The block validator considers
// the value "invalid" even if it just couldn't download the deploys, which could
// just be because the original sender went offline.
// We don't disconnect from the faulty sender here: The proposed block validator
// considers the value "invalid" even if it just couldn't download the deploys,
// which could just be because the original sender went offline.
let validator_index = self.leader(round_id).0;
info!(
our_idx = self.our_idx(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Block validator
//! Proposed Block Validator
//!
//! The block validator checks whether all the deploys included in the block payload exist, either
//! locally or on the network.
//! The proposed block validator checks whether all the deploys included in the block payload exist,
//! either locally or on the network.
//!
//! When multiple requests are made to validate the same block payload, they will eagerly return
//! true if valid, but only fail if all sources have been exhausted. This is only relevant when
Expand All @@ -27,7 +27,7 @@ use crate::{
Component,
},
effect::{
requests::{BlockValidationRequest, FetcherRequest, StorageRequest},
requests::{FetcherRequest, ProposedBlockValidationRequest, StorageRequest},
EffectBuilder, EffectExt, Effects, Responder,
},
types::{
Expand All @@ -40,7 +40,7 @@ pub use config::Config;
pub(crate) use event::Event;
use state::{AddResponderResult, BlockValidationState, MaybeStartFetching};

const COMPONENT_NAME: &str = "block_validator";
const COMPONENT_NAME: &str = "proposed_block_validator";

impl ProposedBlock<ClContext> {
fn timestamp(&self) -> Timestamp {
Expand All @@ -61,11 +61,11 @@ enum MaybeHandled {
/// The request is already being handled - return the wrapped effects and finish.
Handled(Effects<Event>),
/// The request is new - it still needs to be handled.
NotHandled(BlockValidationRequest),
NotHandled(ProposedBlockValidationRequest),
}

#[derive(DataSize, Debug)]
pub(crate) struct BlockValidator {
pub(crate) struct ProposedBlockValidator {
/// Chainspec loaded for deploy validation.
#[data_size(skip)]
chainspec: Arc<Chainspec>,
Expand All @@ -74,10 +74,10 @@ pub(crate) struct BlockValidator {
validation_states: HashMap<ProposedBlock<ClContext>, BlockValidationState>,
}

impl BlockValidator {
/// Creates a new block validator instance.
impl ProposedBlockValidator {
/// Creates a new proposed block validator instance.
pub(crate) fn new(chainspec: Arc<Chainspec>, config: Config) -> Self {
BlockValidator {
ProposedBlockValidator {
chainspec,
config,
validation_states: HashMap::new(),
Expand All @@ -89,18 +89,18 @@ impl BlockValidator {
fn try_handle_as_existing_request<REv>(
&mut self,
effect_builder: EffectBuilder<REv>,
request: BlockValidationRequest,
request: ProposedBlockValidationRequest,
) -> MaybeHandled
where
REv: From<Event> + From<FetcherRequest<Deploy>> + Send,
{
if let Some(state) = self.validation_states.get_mut(&request.block) {
let BlockValidationRequest {
block,
if let Some(state) = self.validation_states.get_mut(&request.proposed_block) {
let ProposedBlockValidationRequest {
proposed_block,
sender,
responder,
} = request;
debug!(%sender, %block, "already validating proposed block");
debug!(%sender, %proposed_block, "already validating proposed block");
match state.add_responder(responder) {
AddResponderResult::Added => {}
AddResponderResult::ValidationCompleted {
Expand Down Expand Up @@ -142,26 +142,26 @@ impl BlockValidator {
fn handle_new_request<REv>(
&mut self,
effect_builder: EffectBuilder<REv>,
BlockValidationRequest {
block,
ProposedBlockValidationRequest {
proposed_block,
sender,
responder,
}: BlockValidationRequest,
}: ProposedBlockValidationRequest,
) -> Effects<Event>
where
REv: From<Event> + From<FetcherRequest<Deploy>> + Send,
{
debug!(%sender, %block, "validating new proposed block");
debug_assert!(!self.validation_states.contains_key(&block));
debug!(%sender, %proposed_block, "validating new proposed block");
debug_assert!(!self.validation_states.contains_key(&proposed_block));
let (mut state, maybe_responder) =
BlockValidationState::new(&block, sender, responder, self.chainspec.as_ref());
BlockValidationState::new(&proposed_block, sender, responder, self.chainspec.as_ref());
let effects = match state.start_fetching() {
MaybeStartFetching::Start {
holder,
missing_deploys,
} => fetch_deploys(effect_builder, holder, missing_deploys),
MaybeStartFetching::ValidationSucceeded => {
debug!("no deploys - block validation complete");
debug!("no deploys - proposed block validation complete");
debug_assert!(maybe_responder.is_some());
respond(true, maybe_responder)
}
Expand All @@ -171,12 +171,12 @@ impl BlockValidator {
}
MaybeStartFetching::Ongoing | MaybeStartFetching::Unable => {
// This `MaybeStartFetching` variant should never be returned here.
error!(%state, "invalid state while handling new block validation");
error!(%state, "invalid state while handling new proposed block validation");
debug_assert!(false, "invalid state {}", state);
respond(false, state.take_responders())
}
};
self.validation_states.insert(block, state);
self.validation_states.insert(proposed_block, state);
self.purge_oldest_complete();
effects
}
Expand All @@ -202,7 +202,7 @@ impl BlockValidator {
debug!(
%state,
num_completed_remaining = (completed_times.len() - 1),
"purging completed block validation state"
"purging completed proposed block validation state"
);
let _ = completed_times.pop();
return false;
Expand Down Expand Up @@ -327,10 +327,10 @@ impl BlockValidator {
}
}

impl<REv> Component<REv> for BlockValidator
impl<REv> Component<REv> for ProposedBlockValidator
where
REv: From<Event>
+ From<BlockValidationRequest>
+ From<ProposedBlockValidationRequest>
+ From<FetcherRequest<Deploy>>
+ From<StorageRequest>
+ Send,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use datasize::DataSize;
use serde::{Deserialize, Serialize};

/// Configuration options for block validation.
/// Configuration options for proposed block validation.
#[derive(Copy, Clone, DataSize, Debug, Deserialize, Serialize)]
pub struct Config {
pub max_completed_entries: u32,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ use derive_more::{Display, From};

use crate::{
components::fetcher::FetchResult,
effect::requests::BlockValidationRequest,
effect::requests::ProposedBlockValidationRequest,
types::{Deploy, DeployOrTransferHash},
};

#[derive(Debug, From, Display)]
pub(crate) enum Event {
#[from]
Request(BlockValidationRequest),
Request(ProposedBlockValidationRequest),

#[display(fmt = "{} fetched", dt_hash)]
DeployFetched {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl ApprovalInfo {
}
}

/// State of the current process of block validation.
/// State of the current process of proposed block validation.
///
/// Tracks whether or not there are deploys still missing and who is interested in the final result.
#[derive(DataSize, Debug)]
Expand Down Expand Up @@ -205,7 +205,7 @@ impl BlockValidationState {
debug!(
block_timestamp = %appendable_block.timestamp(),
peer = %entry.key(),
"already registered peer as holder for block validation"
"already registered peer as holder for proposed block validation"
);
}
Entry::Vacant(entry) => {
Expand Down Expand Up @@ -325,13 +325,13 @@ impl BlockValidationState {
debug!(
block_timestamp = %appendable_block.timestamp(),
missing_deploys_len = missing_deploys.len(),
"still missing deploys - block validation incomplete"
"still missing deploys - proposed block validation incomplete"
);
return vec![];
}
debug!(
block_timestamp = %appendable_block.timestamp(),
"no further missing deploys - block validation complete"
"no further missing deploys - proposed block validation complete"
);
let new_state = BlockValidationState::Valid(appendable_block.timestamp());
(new_state, mem::take(responders))
Expand Down
Loading
Loading