From 03945dab4f9abec1deb1c3ca91f22dc26d3fca04 Mon Sep 17 00:00:00 2001 From: Alexandru Gheorghe Date: Wed, 19 Jun 2024 12:18:44 +0300 Subject: [PATCH] Move crypto checks in the approval-distribution Signed-off-by: Alexandru Gheorghe --- Cargo.lock | 1 + .../node/core/approval-voting/src/criteria.rs | 4 +- .../node/core/approval-voting/src/import.rs | 7 +- polkadot/node/core/approval-voting/src/lib.rs | 174 ++++--------- .../network/approval-distribution/Cargo.toml | 2 + .../network/approval-distribution/src/lib.rs | 235 ++++++++++-------- .../approval-distribution/src/metrics.rs | 16 -- polkadot/node/primitives/src/approval.rs | 10 +- polkadot/node/service/src/overseer.rs | 5 +- polkadot/node/subsystem-types/src/messages.rs | 10 +- 10 files changed, 210 insertions(+), 254 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f3e3c3603e8..bee9e0656059 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12643,6 +12643,7 @@ dependencies = [ "futures-timer", "itertools 0.11.0", "log", + "polkadot-node-core-approval-voting", "polkadot-node-jaeger", "polkadot-node-metrics", "polkadot-node-network-protocol", diff --git a/polkadot/node/core/approval-voting/src/criteria.rs b/polkadot/node/core/approval-voting/src/criteria.rs index fb9d281e43bc..b6388804d850 100644 --- a/polkadot/node/core/approval-voting/src/criteria.rs +++ b/polkadot/node/core/approval-voting/src/criteria.rs @@ -254,7 +254,7 @@ impl<'a> From<&'a SessionInfo> for Config { } /// A trait for producing and checking assignments. Used to mock. -pub(crate) trait AssignmentCriteria { +pub trait AssignmentCriteria { fn compute_assignments( &self, keystore: &LocalKeystore, @@ -276,7 +276,7 @@ pub(crate) trait AssignmentCriteria { ) -> Result; } -pub(crate) struct RealAssignmentCriteria; +pub struct RealAssignmentCriteria; impl AssignmentCriteria for RealAssignmentCriteria { fn compute_assignments( diff --git a/polkadot/node/core/approval-voting/src/import.rs b/polkadot/node/core/approval-voting/src/import.rs index e190fda45b02..51341bae0932 100644 --- a/polkadot/node/core/approval-voting/src/import.rs +++ b/polkadot/node/core/approval-voting/src/import.rs @@ -581,9 +581,14 @@ pub(crate) async fn handle_new_head< hash: block_hash, number: block_header.number, parent_hash: block_header.parent_hash, - candidates: included_candidates.iter().map(|(hash, _, _, _)| *hash).collect(), + candidates: included_candidates + .iter() + .map(|(hash, _, core_index, group_index)| (*hash, *core_index, *group_index)) + .collect(), slot, session: session_index, + session_info: session_info.clone(), + vrf_story: relay_vrf_story, }); imported_candidates.push(BlockImportedCandidates { diff --git a/polkadot/node/core/approval-voting/src/lib.rs b/polkadot/node/core/approval-voting/src/lib.rs index a97330b57792..d6208cbf6bac 100644 --- a/polkadot/node/core/approval-voting/src/lib.rs +++ b/polkadot/node/core/approval-voting/src/lib.rs @@ -55,9 +55,8 @@ use polkadot_node_subsystem_util::{ }; use polkadot_primitives::{ ApprovalVoteMultipleCandidates, ApprovalVotingParams, BlockNumber, CandidateHash, - CandidateIndex, CandidateReceipt, CoreIndex, DisputeStatement, ExecutorParams, GroupIndex, - Hash, PvfExecKind, SessionIndex, SessionInfo, ValidDisputeStatementKind, ValidatorId, - ValidatorIndex, ValidatorPair, ValidatorSignature, + CandidateIndex, CandidateReceipt, CoreIndex, ExecutorParams, GroupIndex, Hash, PvfExecKind, + SessionIndex, SessionInfo, ValidatorId, ValidatorIndex, ValidatorPair, ValidatorSignature, }; use sc_keystore::LocalKeystore; use sp_application_crypto::Pair; @@ -91,7 +90,7 @@ use schnellru::{ByLength, LruMap}; use approval_checking::RequiredTranches; use bitvec::{order::Lsb0, vec::BitVec}; -use criteria::{AssignmentCriteria, RealAssignmentCriteria}; +pub use criteria::{AssignmentCriteria, Config as AssignmentConfig, RealAssignmentCriteria}; use persisted_entries::{ApprovalEntry, BlockEntry, CandidateEntry}; use time::{slot_number_to_tick, Clock, ClockExt, DelayedApprovalTimer, SystemClock, Tick}; @@ -123,7 +122,7 @@ const APPROVAL_CHECKING_TIMEOUT: Duration = Duration::from_secs(120); const WAIT_FOR_SIGS_TIMEOUT: Duration = Duration::from_millis(500); const APPROVAL_CACHE_SIZE: u32 = 1024; -const TICK_TOO_FAR_IN_FUTURE: Tick = 20; // 10 seconds. +pub const TICK_TOO_FAR_IN_FUTURE: Tick = 20; // 10 seconds. const APPROVAL_DELAY: Tick = 2; pub(crate) const LOG_TARGET: &str = "parachain::approval-voting"; @@ -1878,14 +1877,39 @@ async fn distribution_messages_for_activation i, + None => continue, + }; approval_meta.push(BlockApprovalMeta { hash: block_hash, number: block_entry.block_number(), parent_hash: block_entry.parent_hash(), - candidates: block_entry.candidates().iter().map(|(_, c_hash)| *c_hash).collect(), + candidates: block_entry + .candidates() + .iter() + .flat_map(|(core_index, c_hash)| { + let candidate = db.load_candidate_entry(c_hash).ok().flatten(); + candidate + .and_then(|entry| { + entry.approval_entry(&block_hash).map(|entry| entry.backing_group()) + }) + .and_then(|group_index| Some((*c_hash, *core_index, group_index))) + }) + .collect(), slot: block_entry.slot(), session: block_entry.session(), + vrf_story: block_entry.relay_vrf_story(), + session_info: session_info.clone(), }); + let mut signatures_queued = HashSet::new(); for (core_index, candidate_hash) in block_entry.candidates() { let _candidate_span = @@ -2155,7 +2179,7 @@ async fn handle_from_overseer< vec![Action::Conclude] }, FromOrchestra::Communication { msg } => match msg { - ApprovalVotingMessage::CheckAndImportAssignment(a, claimed_cores, res) => { + ApprovalVotingMessage::CheckAndImportAssignment(a, claimed_cores, tranche) => { let (check_outcome, actions) = check_and_import_assignment( sender, state, @@ -2163,27 +2187,30 @@ async fn handle_from_overseer< session_info_provider, a, claimed_cores, + tranche, ) .await?; - let _ = res.send(check_outcome); - + if let AssignmentCheckResult::Bad(err) = check_outcome { + gum::error!(target: LOG_TARGET, ?err, "Unexpected fail to check and import assignment"); + } actions }, - ApprovalVotingMessage::CheckAndImportApproval(a, res) => - check_and_import_approval( + ApprovalVotingMessage::CheckAndImportApproval(a) => { + let result = check_and_import_approval( sender, state, db, session_info_provider, metrics, a, - |r| { - let _ = res.send(r); - }, &wakeups, ) - .await? - .0, + .await?; + if let ApprovalCheckResult::Bad(err) = result.1 { + gum::error!(target: LOG_TARGET, ?err, "Unexpected fail to check and import approval"); + } + result.0 + }, ApprovalVotingMessage::ApprovedAncestor(target, lower_bound, res) => { let mut approved_ancestor_span = state .spans @@ -2744,6 +2771,7 @@ async fn check_and_import_assignment( session_info_provider: &mut RuntimeInfo, assignment: IndirectAssignmentCertV2, candidate_indices: CandidateBitfield, + tranche: DelayTranche, ) -> SubsystemResult<(AssignmentCheckResult, Vec)> where Sender: SubsystemSender, @@ -2877,42 +2905,6 @@ where )) } - // Check the assignment certificate. - let res = state.assignment_criteria.check_assignment_cert( - claimed_core_indices - .clone() - .try_into() - .expect("Checked for null assignment above; qed"), - assignment.validator, - &criteria::Config::from(session_info), - block_entry.relay_vrf_story(), - &assignment.cert, - backing_groups, - ); - - let tranche = match res { - Err(crate::criteria::InvalidAssignment(reason)) => - return Ok(( - AssignmentCheckResult::Bad(AssignmentCheckError::InvalidCert( - assignment.validator, - format!("{:?}", reason), - )), - Vec::new(), - )), - Ok(tranche) => { - let current_tranche = - state.clock.tranche_now(state.slot_duration_millis, block_entry.slot()); - - let too_far_in_future = current_tranche + TICK_TOO_FAR_IN_FUTURE as DelayTranche; - - if tranche >= too_far_in_future { - return Ok((AssignmentCheckResult::TooFarInFuture, Vec::new())) - } - - tranche - }, - }; - let mut actions = Vec::new(); let res = { let mut is_duplicate = true; @@ -3002,23 +2994,21 @@ where Ok((res, actions)) } -async fn check_and_import_approval( +async fn check_and_import_approval( sender: &mut Sender, state: &mut State, db: &mut OverlayedBackend<'_, impl Backend>, session_info_provider: &mut RuntimeInfo, metrics: &Metrics, approval: IndirectSignedApprovalVoteV2, - with_response: impl FnOnce(ApprovalCheckResult) -> T, wakeups: &Wakeups, -) -> SubsystemResult<(Vec, T)> +) -> SubsystemResult<(Vec, ApprovalCheckResult)> where Sender: SubsystemSender, { macro_rules! respond_early { ($e: expr) => {{ - let t = with_response($e); - return Ok((Vec::new(), t)) + return Ok((Vec::new(), $e)) }}; } let mut span = state @@ -3072,67 +3062,11 @@ where ), ); - { - let session_info = match get_session_info( - session_info_provider, - sender, - approval.block_hash, - block_entry.session(), - ) - .await - { - Some(s) => s, - None => { - respond_early!(ApprovalCheckResult::Bad(ApprovalCheckError::UnknownSessionIndex( - block_entry.session() - ),)) - }, - }; - - let pubkey = match session_info.validators.get(approval.validator) { - Some(k) => k, - None => respond_early!(ApprovalCheckResult::Bad( - ApprovalCheckError::InvalidValidatorIndex(approval.validator), - )), - }; - - gum::trace!( - target: LOG_TARGET, - "Received approval for num_candidates {:}", - approval.candidate_indices.count_ones() - ); - - let candidate_hashes: Vec = - approved_candidates_info.iter().map(|candidate| candidate.1).collect(); - // Signature check: - match DisputeStatement::Valid( - ValidDisputeStatementKind::ApprovalCheckingMultipleCandidates(candidate_hashes.clone()), - ) - .check_signature( - &pubkey, - if let Some(candidate_hash) = candidate_hashes.first() { - *candidate_hash - } else { - respond_early!(ApprovalCheckResult::Bad(ApprovalCheckError::InvalidValidatorIndex( - approval.validator - ),)) - }, - block_entry.session(), - &approval.signature, - ) { - Err(_) => { - gum::error!( - target: LOG_TARGET, - "Error while checking signature {:}", - approval.candidate_indices.count_ones() - ); - respond_early!(ApprovalCheckResult::Bad(ApprovalCheckError::InvalidSignature( - approval.validator - ),)) - }, - Ok(()) => {}, - }; - } + gum::trace!( + target: LOG_TARGET, + "Received approval for num_candidates {:}", + approval.candidate_indices.count_ones() + ); let mut actions = Vec::new(); for (approval_candidate_index, approved_candidate_hash) in approved_candidates_info { @@ -3196,9 +3130,7 @@ where } // importing the approval can be heavy as it may trigger acceptance for a series of blocks. - let t = with_response(ApprovalCheckResult::Accepted); - - Ok((actions, t)) + Ok((actions, ApprovalCheckResult::Accepted)) } #[derive(Debug)] diff --git a/polkadot/node/network/approval-distribution/Cargo.toml b/polkadot/node/network/approval-distribution/Cargo.toml index d80519b9e2e9..8c94a95651a2 100644 --- a/polkadot/node/network/approval-distribution/Cargo.toml +++ b/polkadot/node/network/approval-distribution/Cargo.toml @@ -17,6 +17,8 @@ polkadot-node-subsystem = { path = "../../subsystem" } polkadot-node-subsystem-util = { path = "../../subsystem-util" } polkadot-primitives = { path = "../../../primitives" } polkadot-node-jaeger = { path = "../../jaeger" } +polkadot-node-core-approval-voting = { path = "../../core/approval-voting"} + rand = "0.8" itertools = "0.11" diff --git a/polkadot/node/network/approval-distribution/src/lib.rs b/polkadot/node/network/approval-distribution/src/lib.rs index 0edd3df36015..56b993a60f17 100644 --- a/polkadot/node/network/approval-distribution/src/lib.rs +++ b/polkadot/node/network/approval-distribution/src/lib.rs @@ -24,9 +24,13 @@ #![warn(missing_docs)] use self::metrics::Metrics; -use futures::{channel::oneshot, select, FutureExt as _}; +use futures::{select, FutureExt as _}; use itertools::Itertools; use net_protocol::peer_set::{ProtocolVersion, ValidationVersion}; +use polkadot_node_core_approval_voting::{ + time::{Clock, ClockExt}, + AssignmentCriteria, TICK_TOO_FAR_IN_FUTURE, +}; use polkadot_node_jaeger as jaeger; use polkadot_node_network_protocol::{ self as net_protocol, filter_by_peer_version, @@ -37,7 +41,8 @@ use polkadot_node_network_protocol::{ }; use polkadot_node_primitives::approval::{ v1::{ - AssignmentCertKind, BlockApprovalMeta, IndirectAssignmentCert, IndirectSignedApprovalVote, + AssignmentCertKind, BlockApprovalMeta, DelayTranche, IndirectAssignmentCert, + IndirectSignedApprovalVote, RelayVRFStory, }, v2::{ AsBitIndex, AssignmentCertKindV2, CandidateBitfield, IndirectAssignmentCertV2, @@ -46,18 +51,20 @@ use polkadot_node_primitives::approval::{ }; use polkadot_node_subsystem::{ messages::{ - ApprovalCheckResult, ApprovalDistributionMessage, ApprovalVotingMessage, - AssignmentCheckResult, NetworkBridgeEvent, NetworkBridgeTxMessage, + ApprovalDistributionMessage, ApprovalVotingMessage, NetworkBridgeEvent, + NetworkBridgeTxMessage, }, overseer, FromOrchestra, OverseerSignal, SpawnedSubsystem, SubsystemError, }; use polkadot_node_subsystem_util::reputation::{ReputationAggregator, REPUTATION_CHANGE_INTERVAL}; use polkadot_primitives::{ - BlockNumber, CandidateIndex, Hash, SessionIndex, ValidatorIndex, ValidatorSignature, + BlockNumber, CandidateHash, CandidateIndex, CoreIndex, DisputeStatement, GroupIndex, Hash, + SessionIndex, SessionInfo, Slot, ValidDisputeStatementKind, ValidatorIndex, ValidatorSignature, }; use rand::{CryptoRng, Rng, SeedableRng}; use std::{ collections::{hash_map, BTreeMap, HashMap, HashSet, VecDeque}, + sync::Arc, time::Duration, }; @@ -86,6 +93,7 @@ const MAX_BITFIELD_SIZE: usize = 500; /// The Approval Distribution subsystem. pub struct ApprovalDistribution { metrics: Metrics, + slot_duration_millis: u64, } /// Contains recently finalized @@ -354,6 +362,9 @@ pub struct State { /// Aggregated reputation change reputation: ReputationAggregator, + + slot_duration_millis: u64, + clock: Option>, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -488,11 +499,19 @@ struct BlockEntry { knowledge: Knowledge, /// A votes entry for each candidate indexed by [`CandidateIndex`]. candidates: Vec, + /// Information about candidate metadata. + candidates_metadata: Vec<(CandidateHash, CoreIndex, GroupIndex)>, /// The session index of this block. session: SessionIndex, /// Approval entries for whole block. These also contain all approvals in the case of multiple /// candidates being claimed by assignments. approval_entries: HashMap<(ValidatorIndex, CandidateBitfield), ApprovalEntry>, + /// Information about session_info + session_info: SessionInfo, + /// The block vrf story. + pub vrf_story: RelayVRFStory, + /// The block slot. + slot: Slot, } impl BlockEntry { @@ -662,6 +681,11 @@ enum PendingMessage { #[overseer::contextbounds(ApprovalDistribution, prefix = self::overseer)] impl State { + /// Build State with specified slot duration. + pub fn with_config(slot_duration_millis: u64, clock: Arc) -> Self { + Self { slot_duration_millis, clock: Some(clock), ..Default::default() } + } + async fn handle_network_msg< N: overseer::SubsystemSender, A: overseer::SubsystemSender, @@ -785,7 +809,14 @@ impl State { rng: &mut (impl CryptoRng + Rng), ) { let mut new_hashes = HashSet::new(); - for meta in &metas { + + gum::debug!( + target: LOG_TARGET, + "Got new blocks {:?}", + metas.iter().map(|m| (m.hash, m.number)).collect::>(), + ); + + for meta in metas { let mut span = self .spans .get(&meta.hash) @@ -809,6 +840,10 @@ impl State { candidates, session: meta.session, approval_entries: HashMap::new(), + candidates_metadata: meta.candidates, + session_info: meta.session_info, + vrf_story: meta.vrf_story, + slot: meta.slot, }); self.topologies.inc_session_refs(meta.session); @@ -823,12 +858,6 @@ impl State { } } - gum::debug!( - target: LOG_TARGET, - "Got new blocks {:?}", - metas.iter().map(|m| (m.hash, m.number)).collect::>(), - ); - { for (peer_id, PeerEntry { view, version }) in self.peer_views.iter() { let intersection = view.iter().filter(|h| new_hashes.contains(h)); @@ -1360,36 +1389,67 @@ impl State { metrics.on_assignment_good_known(); return } + let checker = polkadot_node_core_approval_voting::RealAssignmentCriteria {}; - let (tx, rx) = oneshot::channel(); - - approval_voting_sender - .send_message(ApprovalVotingMessage::CheckAndImportAssignment( - assignment.clone(), - claimed_candidate_indices.clone(), - tx, - )) - .await; - - let timer = metrics.time_awaiting_approval_voting(); - let result = match rx.await { - Ok(result) => result, - Err(_) => { - gum::debug!(target: LOG_TARGET, "The approval voting subsystem is down"); - return - }, - }; - drop(timer); + let claimed_cores = claimed_candidate_indices + .iter_ones() + .flat_map(|candidate_index| { + entry.candidates_metadata.get(candidate_index).map(|(_, core, _)| *core) + }) + .collect::>(); + let backing_groups = claimed_candidate_indices + .iter_ones() + .flat_map(|candidate_index| { + entry.candidates_metadata.get(candidate_index).map(|(_, _, group)| *group) + }) + .collect::>(); - gum::trace!( - target: LOG_TARGET, - ?source, - ?message_subject, - ?result, - "Checked assignment", + let result = checker.check_assignment_cert( + claimed_cores.try_into().expect("Non-empty vec; qed"), + assignment.validator, + &polkadot_node_core_approval_voting::AssignmentConfig::from(&entry.session_info), + entry.vrf_story.clone(), + &assignment.cert, + backing_groups, ); + match result { - AssignmentCheckResult::Accepted => { + Ok(tranche) => { + let current_tranche = self + .clock + .as_ref() + .map(|clock| clock.tranche_now(self.slot_duration_millis, entry.slot)) + .unwrap_or(u32::MAX); + + let too_far_in_future = + current_tranche + TICK_TOO_FAR_IN_FUTURE as DelayTranche; + + if tranche >= too_far_in_future { + gum::debug!( + target: LOG_TARGET, + hash = ?block_hash, + ?peer_id, + "Got an assignment too far in the future", + ); + modify_reputation( + &mut self.reputation, + network_sender, + peer_id, + COST_ASSIGNMENT_TOO_FAR_IN_THE_FUTURE, + ) + .await; + metrics.on_assignment_far(); + + return + } + + approval_voting_sender + .send_message(ApprovalVotingMessage::CheckAndImportAssignment( + assignment.clone(), + claimed_candidate_indices.clone(), + tranche, + )) + .await; modify_reputation( &mut self.reputation, network_sender, @@ -1402,42 +1462,8 @@ impl State { peer_knowledge.received.insert(message_subject.clone(), message_kind); } }, - AssignmentCheckResult::AcceptedDuplicate => { - // "duplicate" assignments aren't necessarily equal. - // There is more than one way each validator can be assigned to each core. - // cf. https://github.com/paritytech/polkadot/pull/2160#discussion_r557628699 - if let Some(peer_knowledge) = entry.known_by.get_mut(&peer_id) { - peer_knowledge.received.insert(message_subject.clone(), message_kind); - } - gum::debug!( - target: LOG_TARGET, - hash = ?block_hash, - ?peer_id, - "Got an `AcceptedDuplicate` assignment", - ); - metrics.on_assignment_duplicatevoting(); - - return - }, - AssignmentCheckResult::TooFarInFuture => { - gum::debug!( - target: LOG_TARGET, - hash = ?block_hash, - ?peer_id, - "Got an assignment too far in the future", - ); - modify_reputation( - &mut self.reputation, - network_sender, - peer_id, - COST_ASSIGNMENT_TOO_FAR_IN_THE_FUTURE, - ) - .await; - metrics.on_assignment_far(); - return - }, - AssignmentCheckResult::Bad(error) => { + Err(error) => { gum::info!( target: LOG_TARGET, hash = ?block_hash, @@ -1733,32 +1759,35 @@ impl State { { return } - - let (tx, rx) = oneshot::channel(); - - approval_voting_sender - .send_message(ApprovalVotingMessage::CheckAndImportApproval(vote.clone(), tx)) - .await; - - let timer = metrics.time_awaiting_approval_voting(); - let result = match rx.await { - Ok(result) => result, - Err(_) => { - gum::debug!(target: LOG_TARGET, "The approval voting subsystem is down"); - return - }, - }; - drop(timer); - - gum::trace!( - target: LOG_TARGET, - ?peer_id, - ?result, - ?vote, - "Checked approval", + let candidate_hashes = vote + .candidate_indices + .iter_ones() + .flat_map(|candidate_index| { + entry + .candidates_metadata + .get(candidate_index) + .map(|(candidate_hash, _, _)| *candidate_hash) + }) + .collect::>(); + let pubkey = entry.session_info.validators.get(vote.validator).unwrap(); + let result = DisputeStatement::Valid( + ValidDisputeStatementKind::ApprovalCheckingMultipleCandidates( + candidate_hashes.clone(), + ), + ) + .check_signature( + &pubkey, + *candidate_hashes.first().unwrap(), + entry.session, + &vote.signature, ); + match result { - ApprovalCheckResult::Accepted => { + Ok(_) => { + approval_voting_sender + .send_message(ApprovalVotingMessage::CheckAndImportApproval(vote.clone())) + .await; + modify_reputation( &mut self.reputation, network_sender, @@ -1776,7 +1805,7 @@ impl State { .insert(approval_knwowledge_key.0.clone(), approval_knwowledge_key.1); } }, - ApprovalCheckResult::Bad(error) => { + Err(_) => { modify_reputation( &mut self.reputation, network_sender, @@ -1787,8 +1816,7 @@ impl State { gum::info!( target: LOG_TARGET, ?peer_id, - %error, - "Got a bad approval from peer", + "Got a badly signed approval from peer", ); metrics.on_approval_bad(); return @@ -2462,12 +2490,13 @@ async fn modify_reputation( #[overseer::contextbounds(ApprovalDistribution, prefix = self::overseer)] impl ApprovalDistribution { /// Create a new instance of the [`ApprovalDistribution`] subsystem. - pub fn new(metrics: Metrics) -> Self { - Self { metrics } + pub fn new(metrics: Metrics, slot_duration_millis: u64) -> Self { + Self { metrics, slot_duration_millis } } async fn run(self, ctx: Context) { - let mut state = State::default(); + let mut state = + State { slot_duration_millis: self.slot_duration_millis, ..Default::default() }; // According to the docs of `rand`, this is a ChaCha12 RNG in practice // and will always be chosen for strong performance and security properties. let mut rng = rand::rngs::StdRng::from_entropy(); diff --git a/polkadot/node/network/approval-distribution/src/metrics.rs b/polkadot/node/network/approval-distribution/src/metrics.rs index 60c7f2f6d3b8..10553c352966 100644 --- a/polkadot/node/network/approval-distribution/src/metrics.rs +++ b/polkadot/node/network/approval-distribution/src/metrics.rs @@ -30,7 +30,6 @@ struct MetricsInner { aggression_l2_messages_total: prometheus::Counter, time_unify_with_peer: prometheus::Histogram, time_import_pending_now_known: prometheus::Histogram, - time_awaiting_approval_voting: prometheus::Histogram, assignments_received_result: prometheus::CounterVec, approvals_received_result: prometheus::CounterVec, } @@ -206,14 +205,6 @@ impl Metrics { } } - pub(crate) fn time_awaiting_approval_voting( - &self, - ) -> Option { - self.0 - .as_ref() - .map(|metrics| metrics.time_awaiting_approval_voting.start_timer()) - } - pub(crate) fn on_aggression_l1(&self) { if let Some(metrics) = &self.0 { metrics.aggression_l1_messages_total.inc(); @@ -288,13 +279,6 @@ impl MetricsTrait for Metrics { ).buckets(vec![0.0001, 0.0004, 0.0016, 0.0064, 0.0256, 0.1024, 0.4096, 1.6384, 3.2768, 4.9152, 6.5536,]))?, registry, )?, - time_awaiting_approval_voting: prometheus::register( - prometheus::Histogram::with_opts(prometheus::HistogramOpts::new( - "polkadot_parachain_time_awaiting_approval_voting", - "Time spent awaiting a reply from the Approval Voting Subsystem.", - ).buckets(vec![0.0001, 0.0004, 0.0016, 0.0064, 0.0256, 0.1024, 0.4096, 1.6384, 3.2768, 4.9152, 6.5536,]))?, - registry, - )?, assignments_received_result: prometheus::register( prometheus::CounterVec::new( prometheus::Opts::new( diff --git a/polkadot/node/primitives/src/approval.rs b/polkadot/node/primitives/src/approval.rs index 66883b33367b..54a1cb4b3ca0 100644 --- a/polkadot/node/primitives/src/approval.rs +++ b/polkadot/node/primitives/src/approval.rs @@ -25,8 +25,8 @@ pub mod v1 { use codec::{Decode, Encode}; use polkadot_primitives::{ - BlockNumber, CandidateHash, CandidateIndex, CoreIndex, Hash, Header, SessionIndex, - ValidatorIndex, ValidatorSignature, + BlockNumber, CandidateHash, CandidateIndex, CoreIndex, GroupIndex, Hash, Header, + SessionIndex, SessionInfo, ValidatorIndex, ValidatorSignature, }; use sp_application_crypto::ByteArray; @@ -128,11 +128,15 @@ pub mod v1 { pub parent_hash: Hash, /// The candidates included by the block. /// Note that these are not the same as the candidates that appear within the block body. - pub candidates: Vec, + pub candidates: Vec<(CandidateHash, CoreIndex, GroupIndex)>, /// The consensus slot of the block. pub slot: Slot, /// The session of the block. pub session: SessionIndex, + /// The session info of the block. + pub session_info: SessionInfo, + /// The vrf story. + pub vrf_story: RelayVRFStory, } /// Errors that can occur during the approvals protocol. diff --git a/polkadot/node/service/src/overseer.rs b/polkadot/node/service/src/overseer.rs index 2aa3dc7b8dd6..1a14b3f85cb1 100644 --- a/polkadot/node/service/src/overseer.rs +++ b/polkadot/node/service/src/overseer.rs @@ -308,7 +308,10 @@ where Metrics::register(registry)?, rand::rngs::StdRng::from_entropy(), )) - .approval_distribution(ApprovalDistributionSubsystem::new(Metrics::register(registry)?)) + .approval_distribution(ApprovalDistributionSubsystem::new( + Metrics::register(registry)?, + approval_voting_config.slot_duration_millis, + )) .approval_voting(ApprovalVotingSubsystem::with_config( approval_voting_config, parachains_db.clone(), diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index 722a97989bce..90bbc235495b 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -33,7 +33,7 @@ use polkadot_node_network_protocol::{ }; use polkadot_node_primitives::{ approval::{ - v1::BlockApprovalMeta, + v1::{BlockApprovalMeta, DelayTranche}, v2::{CandidateBitfield, IndirectAssignmentCertV2, IndirectSignedApprovalVoteV2}, }, AvailableData, BabeEpoch, BlockWeight, CandidateVotes, CollationGenerationConfig, @@ -964,16 +964,12 @@ pub struct HighestApprovedAncestorBlock { pub enum ApprovalVotingMessage { /// Check if the assignment is valid and can be accepted by our view of the protocol. /// Should not be sent unless the block hash is known. - CheckAndImportAssignment( - IndirectAssignmentCertV2, - CandidateBitfield, - oneshot::Sender, - ), + CheckAndImportAssignment(IndirectAssignmentCertV2, CandidateBitfield, DelayTranche), /// Check if the approval vote is valid and can be accepted by our view of the /// protocol. /// /// Should not be sent unless the block hash within the indirect vote is known. - CheckAndImportApproval(IndirectSignedApprovalVoteV2, oneshot::Sender), + CheckAndImportApproval(IndirectSignedApprovalVoteV2), /// Returns the highest possible ancestor hash of the provided block hash which is /// acceptable to vote on finality for. /// The `BlockNumber` provided is the number of the block's ancestor which is the