Skip to content

Commit

Permalink
Merge #4462
Browse files Browse the repository at this point in the history
4462: Rename `BlockValidator` to `ProposedBlockValidator` r=Fraser999 a=Fraser999

This PR is a simple rename of the given component and related variables/config options.

Note that the name of memory metrics for the component was left unchanged as `"mem_block_validator"` in order to avoid breaking the generated metrics API.  This can be updated in 2.0.

Closes #4309.

Co-authored-by: Fraser Hutchison <[email protected]>
  • Loading branch information
casperlabs-bors-ng[bot] and Fraser999 authored Dec 18, 2023
2 parents 19bbdf5 + 6390d75 commit 682fb17
Show file tree
Hide file tree
Showing 21 changed files with 160 additions and 138 deletions.
9 changes: 9 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



## Unreleased

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



## 1.5.5

### Added
Expand Down Expand Up @@ -61,6 +68,8 @@ All notable changes to this project will be documented in this file. The format
### Removed
* There is no more weighted rate limiting on incoming traffic, instead the nodes dynamically adjusts allowed rates from peers based on available resources. This resulted in the removal of the `estimator_weights` configuration option and the `accumulated_incoming_limiter_delay` metric.



## 1.5.3

### 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 @@ -43,9 +43,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 @@ -456,7 +456,7 @@ pub(crate) trait ReactorEventT:
+ From<NetworkInfoRequest>
+ From<DeployBufferRequest>
+ From<ConsensusAnnouncement>
+ From<BlockValidationRequest>
+ From<ProposedBlockValidationRequest>
+ From<StorageRequest>
+ From<ContractRuntimeRequest>
+ From<ChainspecRawBytesRequest>
Expand All @@ -475,7 +475,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 @@ -51,7 +51,7 @@ use crate::{
consensus::ValidationError,
effect::{
announcements::FatalAnnouncement,
requests::{BlockValidationRequest, ContractRuntimeRequest, StorageRequest},
requests::{ContractRuntimeRequest, ProposedBlockValidationRequest, StorageRequest},
AutoClosingResponder, EffectBuilder, EffectExt, Effects, Responder,
},
failpoints::Failpoint,
Expand Down Expand Up @@ -1396,7 +1396,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 @@ -1047,9 +1047,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![]
} else {
let mut outcomes = self
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 @@ -1708,8 +1708,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 @@ -2283,9 +2283,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 @@ -28,7 +28,7 @@ use crate::{
},
consensus::ValidationError,
effect::{
requests::{BlockValidationRequest, FetcherRequest, StorageRequest},
requests::{FetcherRequest, ProposedBlockValidationRequest, StorageRequest},
EffectBuilder, EffectExt, Effects, Responder,
},
types::{
Expand All @@ -41,7 +41,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 @@ -62,11 +62,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 @@ -75,10 +75,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 @@ -90,18 +90,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 @@ -146,26 +146,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(Ok(()), maybe_responder)
}
Expand All @@ -180,15 +180,15 @@ impl BlockValidator {
// Programmer error, we should only request each validation once!

// 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(
Err(ValidationError::DuplicateValidationAttempt),
state.take_responders(),
)
}
};
self.validation_states.insert(block, state);
self.validation_states.insert(proposed_block, state);
self.purge_oldest_complete();
effects
}
Expand All @@ -214,7 +214,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 @@ -380,10 +380,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 @@ -80,7 +80,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 @@ -227,7 +227,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 @@ -353,13 +353,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

0 comments on commit 682fb17

Please sign in to comment.