From 14727c5a1fb27365667e48506055be9da64967df 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 | 3 + .../node/core/approval-voting/src/criteria.rs | 8 +- .../node/core/approval-voting/src/import.rs | 6 +- polkadot/node/core/approval-voting/src/lib.rs | 182 +- .../node/core/approval-voting/src/tests.rs | 168 +- .../network/approval-distribution/Cargo.toml | 3 + .../network/approval-distribution/src/lib.rs | 464 +- .../approval-distribution/src/metrics.rs | 16 - .../approval-distribution/src/tests.rs | 5883 +++++++++-------- polkadot/node/overseer/src/lib.rs | 1 + polkadot/node/primitives/src/approval.rs | 8 +- polkadot/node/service/src/overseer.rs | 5 +- .../subsystem-bench/src/lib/approval/mod.rs | 7 +- polkadot/node/subsystem-types/src/messages.rs | 10 +- 14 files changed, 3733 insertions(+), 3031 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9972285780f3..e91bd6400042 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12755,6 +12755,7 @@ dependencies = [ "futures-timer", "itertools 0.11.0", "log", + "polkadot-node-core-approval-voting", "polkadot-node-jaeger", "polkadot-node-metrics", "polkadot-node-network-protocol", @@ -12767,7 +12768,9 @@ dependencies = [ "rand", "rand_chacha", "rand_core", + "sc-keystore", "schnorrkel 0.11.4", + "sp-application-crypto", "sp-authority-discovery", "sp-core", "tracing-gum", diff --git a/polkadot/node/core/approval-voting/src/criteria.rs b/polkadot/node/core/approval-voting/src/criteria.rs index fb9d281e43bc..4a1e87868c2d 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( @@ -614,7 +614,7 @@ fn compute_relay_vrf_delay_assignments( /// Assignment invalid. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct InvalidAssignment(pub(crate) InvalidAssignmentReason); +pub struct InvalidAssignment(pub InvalidAssignmentReason); impl std::fmt::Display for InvalidAssignment { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { @@ -626,7 +626,7 @@ impl std::error::Error for InvalidAssignment {} /// Failure conditions when checking an assignment cert. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum InvalidAssignmentReason { +pub enum InvalidAssignmentReason { ValidatorIndexOutOfBounds, SampleOutOfBounds, CoreIndexOutOfBounds, diff --git a/polkadot/node/core/approval-voting/src/import.rs b/polkadot/node/core/approval-voting/src/import.rs index 3ddef1e01c45..a138af1188d2 100644 --- a/polkadot/node/core/approval-voting/src/import.rs +++ b/polkadot/node/core/approval-voting/src/import.rs @@ -574,9 +574,13 @@ 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, + 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 d4b6855a44d0..5d588a53f3d1 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"; @@ -1607,9 +1606,21 @@ async fn distribution_messages_for_activation( 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(), }); let mut signatures_queued = HashSet::new(); for (core_index, candidate_hash) in block_entry.candidates() { @@ -1872,7 +1883,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, tx) => { let (check_outcome, actions) = check_and_import_assignment( ctx.sender(), state, @@ -1880,27 +1891,39 @@ async fn handle_from_overseer( session_info_provider, a, claimed_cores, + tranche, ) .await?; - let _ = res.send(check_outcome); - + // approval-distribution already sanity checks the assignment is valid and expected, + // so this import should never fail if it does it means there is a bug in the + // code. + if let AssignmentCheckResult::Bad(ref err) = check_outcome { + gum::error!(target: LOG_TARGET, ?err, "Unexpected fail to check and import assignment"); + } + let _ = tx.map(|tx| tx.send(check_outcome)); actions }, - ApprovalVotingMessage::CheckAndImportApproval(a, res) => - check_and_import_approval( + ApprovalVotingMessage::CheckAndImportApproval(a, tx) => { + let result = check_and_import_approval( ctx.sender(), state, db, session_info_provider, metrics, a, - |r| { - let _ = res.send(r); - }, &wakeups, ) - .await? - .0, + .await?; + // approval-distribution already sanity checks the vote is valid and expected, + // so this import should never fail if it does it means there is a bug in the + // code. + if let ApprovalCheckResult::Bad(ref err) = result.1 { + gum::error!(target: LOG_TARGET, ?err, "Unexpected fail to check and import approval"); + } + let _ = tx.map(|tx| tx.send(result.1)); + + result.0 + }, ApprovalVotingMessage::ApprovedAncestor(target, lower_bound, res) => { let mut approved_ancestor_span = state .spans @@ -2384,7 +2407,12 @@ fn schedule_wakeup_action( last_assignment_tick.map(|l| l + APPROVAL_DELAY).filter(|t| t > &tick_now), next_no_show, ) - .map(|tick| Action::ScheduleWakeup { block_hash, block_number, candidate_hash, tick }) + .map(|tick| Action::ScheduleWakeup { + block_hash, + block_number, + candidate_hash, + tick, + }) }, RequiredTranches::Pending { considered, next_no_show, clock_drift, .. } => { // select the minimum of `next_no_show`, or the tick of the next non-empty tranche @@ -2446,6 +2474,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, @@ -2514,8 +2543,6 @@ where )) } - // The Compact VRF modulo assignment cert has multiple core assignments. - let mut backing_groups = Vec::new(); let mut claimed_core_indices = Vec::new(); let mut assigned_candidate_hashes = Vec::new(); @@ -2551,7 +2578,7 @@ where format!("{:?}", jaeger::hash_to_trace_identifier(assigned_candidate_hash.0)), ); - let approval_entry = match candidate_entry.approval_entry_mut(&assignment.block_hash) { + let _approval_entry = match candidate_entry.approval_entry_mut(&assignment.block_hash) { Some(a) => a, None => return Ok(( @@ -2563,7 +2590,6 @@ where )), }; - backing_groups.push(approval_entry.backing_group()); claimed_core_indices.push(claimed_core_index); assigned_candidate_hashes.push(assigned_candidate_hash); } @@ -2579,42 +2605,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; @@ -2704,23 +2694,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 @@ -2774,67 +2762,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 { @@ -2898,9 +2830,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/core/approval-voting/src/tests.rs b/polkadot/node/core/approval-voting/src/tests.rs index 64ae86bc013a..8aab353f9b84 100644 --- a/polkadot/node/core/approval-voting/src/tests.rs +++ b/polkadot/node/core/approval-voting/src/tests.rs @@ -41,8 +41,9 @@ use polkadot_node_subsystem_test_helpers as test_helpers; use polkadot_node_subsystem_util::TimeoutExt; use polkadot_overseer::HeadSupportsParachains; use polkadot_primitives::{ - ApprovalVote, CandidateCommitments, CandidateEvent, CoreIndex, GroupIndex, Header, - Id as ParaId, IndexedVec, NodeFeatures, ValidationCode, ValidatorSignature, + ApprovalVote, CandidateCommitments, CandidateEvent, CoreIndex, DisputeStatement, GroupIndex, + Header, Id as ParaId, IndexedVec, NodeFeatures, ValidDisputeStatementKind, ValidationCode, + ValidatorSignature, }; use std::{cmp::max, time::Duration}; @@ -262,7 +263,8 @@ where _relay_vrf_story: polkadot_node_primitives::approval::v1::RelayVRFStory, _assignment: &polkadot_node_primitives::approval::v2::AssignmentCertV2, _backing_groups: Vec, - ) -> Result { + ) -> Result + { self.1(validator_index) } } @@ -677,7 +679,7 @@ async fn check_and_import_approval( validator, signature, }, - tx, + Some(tx), ), }, ) @@ -698,6 +700,7 @@ async fn check_and_import_assignment( block_hash: Hash, candidate_index: CandidateIndex, validator: ValidatorIndex, + tranche: DelayTranche, ) -> oneshot::Receiver { let (tx, rx) = oneshot::channel(); overseer_send( @@ -711,7 +714,8 @@ async fn check_and_import_assignment( .into(), }, candidate_index.into(), - tx, + tranche, + Some(tx), ), }, ) @@ -744,7 +748,8 @@ async fn check_and_import_assignment_v2( }), }, core_indices.try_into().unwrap(), - tx, + 0, + Some(tx), ), }, ) @@ -1130,6 +1135,7 @@ fn subsystem_rejects_bad_assignment_ok_criteria() { block_hash, candidate_index, validator, + 0, ) .await; @@ -1143,6 +1149,7 @@ fn subsystem_rejects_bad_assignment_ok_criteria() { unknown_hash, candidate_index, validator, + 0, ) .await; @@ -1155,59 +1162,6 @@ fn subsystem_rejects_bad_assignment_ok_criteria() { }); } -#[test] -fn subsystem_rejects_bad_assignment_err_criteria() { - let assignment_criteria = Box::new(MockAssignmentCriteria::check_only(move |_| { - Err(criteria::InvalidAssignment( - criteria::InvalidAssignmentReason::ValidatorIndexOutOfBounds, - )) - })); - let config = HarnessConfigBuilder::default().assignment_criteria(assignment_criteria).build(); - test_harness(config, |test_harness| async move { - let TestHarness { mut virtual_overseer, sync_oracle_handle: _sync_oracle_handle, .. } = - test_harness; - assert_matches!( - overseer_recv(&mut virtual_overseer).await, - AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => { - rx.send(Ok(0)).unwrap(); - } - ); - - let block_hash = Hash::repeat_byte(0x01); - let candidate_index = 0; - let validator = ValidatorIndex(0); - - let head: Hash = ChainBuilder::GENESIS_HASH; - let mut builder = ChainBuilder::new(); - let slot = Slot::from(1 as u64); - builder.add_block( - block_hash, - head, - 1, - BlockConfig { slot, candidates: None, session_info: None, end_syncing: false }, - ); - builder.build(&mut virtual_overseer).await; - - let rx = check_and_import_assignment( - &mut virtual_overseer, - block_hash, - candidate_index, - validator, - ) - .await; - - assert_eq!( - rx.await, - Ok(AssignmentCheckResult::Bad(AssignmentCheckError::InvalidCert( - ValidatorIndex(0), - "ValidatorIndexOutOfBounds".to_string(), - ))), - ); - - virtual_overseer - }); -} - #[test] fn blank_subsystem_act_on_bad_block() { test_harness(HarnessConfig::default(), |test_harness| async move { @@ -1236,7 +1190,8 @@ fn blank_subsystem_act_on_bad_block() { .into(), }, 0u32.into(), - tx, + 0, + Some(tx), ), }, ) @@ -1428,68 +1383,6 @@ fn subsystem_rejects_approval_before_assignment() { }); } -#[test] -fn subsystem_rejects_assignment_in_future() { - let assignment_criteria = - Box::new(MockAssignmentCriteria::check_only(|_| Ok(TICK_TOO_FAR_IN_FUTURE as _))); - let config = HarnessConfigBuilder::default().assignment_criteria(assignment_criteria).build(); - test_harness(config, |test_harness| async move { - let TestHarness { mut virtual_overseer, clock, sync_oracle_handle: _sync_oracle_handle } = - test_harness; - assert_matches!( - overseer_recv(&mut virtual_overseer).await, - AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber(rx)) => { - rx.send(Ok(0)).unwrap(); - } - ); - - let block_hash = Hash::repeat_byte(0x01); - let candidate_index = 0; - let validator = ValidatorIndex(0); - - // Add block hash 00. - ChainBuilder::new() - .add_block( - block_hash, - ChainBuilder::GENESIS_HASH, - 1, - BlockConfig { - slot: Slot::from(0), - candidates: None, - session_info: None, - end_syncing: false, - }, - ) - .build(&mut virtual_overseer) - .await; - - let rx = check_and_import_assignment( - &mut virtual_overseer, - block_hash, - candidate_index, - validator, - ) - .await; - - assert_eq!(rx.await, Ok(AssignmentCheckResult::TooFarInFuture)); - - // Advance clock to make assignment reasonably near. - clock.inner.lock().set_tick(9); - - let rx = check_and_import_assignment( - &mut virtual_overseer, - block_hash, - candidate_index, - validator, - ) - .await; - - assert_eq!(rx.await, Ok(AssignmentCheckResult::Accepted)); - - virtual_overseer - }); -} - #[test] fn subsystem_accepts_duplicate_assignment() { test_harness(HarnessConfig::default(), |test_harness| async move { @@ -1555,6 +1448,7 @@ fn subsystem_accepts_duplicate_assignment() { block_hash, candidate_index, validator, + 0, ) .await; @@ -1613,6 +1507,7 @@ fn subsystem_rejects_assignment_with_unknown_candidate() { block_hash, candidate_index, validator, + 0, ) .await; @@ -1663,6 +1558,7 @@ fn subsystem_rejects_oversized_bitfields() { block_hash, candidate_index, validator, + 0, ) .await; @@ -1736,6 +1632,7 @@ fn subsystem_accepts_and_imports_approval_after_assignment() { block_hash, candidate_index, validator, + 0, ) .await; @@ -1828,6 +1725,7 @@ fn subsystem_second_approval_import_only_schedules_wakeups() { block_hash, candidate_index, validator, + 0, ) .await; @@ -1916,6 +1814,7 @@ fn subsystem_assignment_import_updates_candidate_entry_and_schedules_wakeup() { block_hash, candidate_index, validator, + 0, ) .await; @@ -2038,6 +1937,7 @@ fn test_approvals_on_fork_are_always_considered_after_no_show( block_hash, candidate_index, validator, + 0, ) .await; assert_eq!(rx.await, Ok(AssignmentCheckResult::Accepted)); @@ -2047,6 +1947,7 @@ fn test_approvals_on_fork_are_always_considered_after_no_show( block_hash_fork, candidate_index, validator, + 0, ) .await; @@ -2151,6 +2052,7 @@ fn subsystem_process_wakeup_schedules_wakeup() { block_hash, candidate_index, validator, + 0, ) .await; @@ -2215,7 +2117,8 @@ fn linear_import_act_on_leaf() { .into(), }, 0u32.into(), - tx, + 0, + Some(tx), ), }, ) @@ -2286,7 +2189,8 @@ fn forkful_import_at_same_height_act_on_leaf() { .into(), }, 0u32.into(), - tx, + 0, + Some(tx), ), }, ) @@ -2448,6 +2352,7 @@ fn import_checked_approval_updates_entries_and_schedules() { block_hash, candidate_index, validator_index_a, + 0, ) .await; @@ -2458,6 +2363,7 @@ fn import_checked_approval_updates_entries_and_schedules() { block_hash, candidate_index, validator_index_b, + 0, ) .await; @@ -2611,6 +2517,7 @@ fn subsystem_import_checked_approval_sets_one_block_bit_at_a_time() { block_hash, candidate_index, validator, + 0, ) .await; assert_eq!(rx.await, Ok(AssignmentCheckResult::Accepted)); @@ -2896,6 +2803,7 @@ fn approved_ancestor_test( *block_hash, candidate_index, validator, + 0, ) .await; assert_eq!(rx.await, Ok(AssignmentCheckResult::Accepted)); @@ -3359,7 +3267,8 @@ where F1: 'static + Fn(ValidatorIndex) -> Result + Send - + Sync, + + Sync + + Clone, F2: Fn(Tick) -> bool, { let TriggersAssignmentConfig { @@ -3388,7 +3297,7 @@ where ); assignments }, - assign_validator_tranche, + assign_validator_tranche.clone(), )); let config = HarnessConfigBuilder::default().assignment_criteria(assignment_criteria).build(); let store = config.backend(); @@ -3454,6 +3363,7 @@ where block_hash, candidate_index, ValidatorIndex(validator), + assign_validator_tranche(ValidatorIndex(validator)).unwrap(), ) .await; assert_eq!(rx.await, Ok(AssignmentCheckResult::Accepted)); @@ -3772,6 +3682,7 @@ fn pre_covers_dont_stall_approval() { block_hash, candidate_index, validator_index_a, + 0, ) .await; @@ -3782,6 +3693,7 @@ fn pre_covers_dont_stall_approval() { block_hash, candidate_index, validator_index_b, + 0, ) .await; @@ -3792,6 +3704,7 @@ fn pre_covers_dont_stall_approval() { block_hash, candidate_index, validator_index_c, + 1, ) .await; @@ -3950,6 +3863,7 @@ fn waits_until_approving_assignments_are_old_enough() { block_hash, candidate_index, validator_index_a, + 0, ) .await; @@ -3960,6 +3874,7 @@ fn waits_until_approving_assignments_are_old_enough() { block_hash, candidate_index, validator_index_b, + 0, ) .await; @@ -4996,7 +4911,6 @@ fn subsystem_sends_pending_approvals_on_approval_restart() { })); } ); - assert_matches!( overseer_recv(&mut virtual_overseer).await, AllMessages::RuntimeApi( diff --git a/polkadot/node/network/approval-distribution/Cargo.toml b/polkadot/node/network/approval-distribution/Cargo.toml index a85cde303b61..d19ded99c7e6 100644 --- a/polkadot/node/network/approval-distribution/Cargo.toml +++ b/polkadot/node/network/approval-distribution/Cargo.toml @@ -10,6 +10,7 @@ license.workspace = true workspace = true [dependencies] +polkadot-node-core-approval-voting = { workspace = true, default-features = true } polkadot-node-metrics = { workspace = true, default-features = true } polkadot-node-network-protocol = { workspace = true, default-features = true } polkadot-node-primitives = { workspace = true, default-features = true } @@ -26,6 +27,8 @@ gum = { workspace = true, default-features = true } bitvec = { features = ["alloc"], workspace = true } [dev-dependencies] +sc-keystore = { workspace = true } +sp-application-crypto = { workspace = true, default-features = true } sp-authority-discovery = { workspace = true, default-features = true } sp-core = { features = ["std"], workspace = true, default-features = true } diff --git a/polkadot/node/network/approval-distribution/src/lib.rs b/polkadot/node/network/approval-distribution/src/lib.rs index 26cedf1a2a13..33136c1b864d 100644 --- a/polkadot/node/network/approval-distribution/src/lib.rs +++ b/polkadot/node/network/approval-distribution/src/lib.rs @@ -24,9 +24,14 @@ #![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::{ + criteria::InvalidAssignment, + time::{Clock, ClockExt, SystemClock}, + AssignmentCriteria, RealAssignmentCriteria, TICK_TOO_FAR_IN_FUTURE, +}; use polkadot_node_jaeger as jaeger; use polkadot_node_network_protocol::{ self as net_protocol, filter_by_peer_version, @@ -35,25 +40,33 @@ use polkadot_node_network_protocol::{ v1 as protocol_v1, v2 as protocol_v2, v3 as protocol_v3, PeerId, UnifiedReputationChange as Rep, Versioned, View, }; -use polkadot_node_primitives::approval::{ - v1::{ - AssignmentCertKind, BlockApprovalMeta, IndirectAssignmentCert, IndirectSignedApprovalVote, - }, - v2::{ - AsBitIndex, AssignmentCertKindV2, CandidateBitfield, IndirectAssignmentCertV2, - IndirectSignedApprovalVoteV2, +use polkadot_node_primitives::{ + approval::{ + v1::{ + AssignmentCertKind, BlockApprovalMeta, DelayTranche, IndirectAssignmentCert, + IndirectSignedApprovalVote, RelayVRFStory, + }, + v2::{ + AsBitIndex, AssignmentCertKindV2, CandidateBitfield, IndirectAssignmentCertV2, + IndirectSignedApprovalVoteV2, + }, }, + DISPUTE_WINDOW, }; use polkadot_node_subsystem::{ messages::{ - ApprovalCheckResult, ApprovalDistributionMessage, ApprovalVotingMessage, - AssignmentCheckResult, NetworkBridgeEvent, NetworkBridgeTxMessage, + ApprovalDistributionMessage, ApprovalVotingMessage, NetworkBridgeEvent, + NetworkBridgeTxMessage, RuntimeApiMessage, }, overseer, FromOrchestra, OverseerSignal, SpawnedSubsystem, SubsystemError, }; -use polkadot_node_subsystem_util::reputation::{ReputationAggregator, REPUTATION_CHANGE_INTERVAL}; +use polkadot_node_subsystem_util::{ + reputation::{ReputationAggregator, REPUTATION_CHANGE_INTERVAL}, + runtime::{Config as RuntimeInfoConfig, ExtendedSessionInfo, RuntimeInfo}, +}; use polkadot_primitives::{ - BlockNumber, CandidateIndex, Hash, SessionIndex, ValidatorIndex, ValidatorSignature, + BlockNumber, CandidateHash, CandidateIndex, CoreIndex, DisputeStatement, GroupIndex, Hash, + SessionIndex, Slot, ValidDisputeStatementKind, ValidatorIndex, ValidatorSignature, }; use rand::{CryptoRng, Rng, SeedableRng}; use std::{ @@ -86,6 +99,8 @@ const MAX_BITFIELD_SIZE: usize = 500; /// The Approval Distribution subsystem. pub struct ApprovalDistribution { metrics: Metrics, + slot_duration_millis: u64, + clock: Box, } /// Contains recently finalized @@ -354,6 +369,8 @@ pub struct State { /// Aggregated reputation change reputation: ReputationAggregator, + /// Slot duration in millis + slot_duration_millis: u64, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -488,11 +505,17 @@ 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>, + /// The block vrf story. + pub vrf_story: RelayVRFStory, + /// The block slot. + slot: Slot, } impl BlockEntry { @@ -646,6 +669,33 @@ enum MessageSource { Local, } +// Encountered error while validating an assignment. +#[derive(Debug)] +enum InvalidAssignmentError { + // The the vrf check for the assignment failed. + #[allow(dead_code)] + CryptoCheckFailed(InvalidAssignment), + // The assignment did not claim any valid candidate. + NoClaimedCandidates, + // Session Info was not found for the block hash in the assignment. + #[allow(dead_code)] + SessionInfoNotFound(polkadot_node_subsystem_util::runtime::Error), +} + +// Encountered error while validating an approval. +#[derive(Debug)] +enum InvalidVoteError { + // The candidate index was out of bounds. + CandidateIndexOutOfBounds, + // The validator index was out of bounds. + ValidatorIndexOutOfBounds, + // The signature of the vote was invalid. + InvalidSignature, + // Session Info was not found for the block hash in the approval. + #[allow(dead_code)] + SessionInfoNotFound(polkadot_node_subsystem_util::runtime::Error), +} + impl MessageSource { fn peer_id(&self) -> Option { match self { @@ -662,16 +712,26 @@ enum PendingMessage { #[overseer::contextbounds(ApprovalDistribution, prefix = self::overseer)] impl State { + /// Build State with specified slot duration. + pub fn with_config(slot_duration_millis: u64) -> Self { + Self { slot_duration_millis, ..Default::default() } + } + async fn handle_network_msg< N: overseer::SubsystemSender, A: overseer::SubsystemSender, + RA: overseer::SubsystemSender, >( &mut self, approval_voting_sender: &mut A, network_sender: &mut N, + runtime_api_sender: &mut RA, metrics: &Metrics, event: NetworkBridgeEvent, rng: &mut (impl CryptoRng + Rng), + assignment_criteria: &impl AssignmentCriteria, + clock: &(impl Clock + ?Sized), + session_info_provider: &mut RuntimeInfo, ) { match event { NetworkBridgeEvent::PeerConnected(peer_id, role, version, authority_ids) => { @@ -727,10 +787,14 @@ impl State { self.process_incoming_peer_message( approval_voting_sender, network_sender, + runtime_api_sender, metrics, peer_id, message, rng, + assignment_criteria, + clock, + session_info_provider, ) .await; }, @@ -776,16 +840,28 @@ impl State { async fn handle_new_blocks< N: overseer::SubsystemSender, A: overseer::SubsystemSender, + RA: overseer::SubsystemSender, >( &mut self, approval_voting_sender: &mut A, network_sender: &mut N, + runtime_api_sender: &mut RA, metrics: &Metrics, metas: Vec, rng: &mut (impl CryptoRng + Rng), + assignment_criteria: &impl AssignmentCriteria, + clock: &(impl Clock + ?Sized), + session_info_provider: &mut RuntimeInfo, ) { 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 +885,9 @@ impl State { candidates, session: meta.session, approval_entries: HashMap::new(), + candidates_metadata: meta.candidates, + vrf_story: meta.vrf_story, + slot: meta.slot, }); self.topologies.inc_session_refs(meta.session); @@ -823,12 +902,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)); @@ -883,11 +956,15 @@ impl State { self.import_and_circulate_assignment( approval_voting_sender, network_sender, + runtime_api_sender, metrics, MessageSource::Peer(peer_id), assignment, claimed_indices, rng, + assignment_criteria, + clock, + session_info_provider, ) .await; }, @@ -895,9 +972,11 @@ impl State { self.import_and_circulate_approval( approval_voting_sender, network_sender, + runtime_api_sender, metrics, MessageSource::Peer(peer_id), approval_vote, + session_info_provider, ) .await; }, @@ -946,15 +1025,20 @@ impl State { async fn process_incoming_assignments< N: overseer::SubsystemSender, A: overseer::SubsystemSender, + RA: overseer::SubsystemSender, R, >( &mut self, approval_voting_sender: &mut A, network_sender: &mut N, + runtime_api_sender: &mut RA, metrics: &Metrics, peer_id: PeerId, assignments: Vec<(IndirectAssignmentCertV2, CandidateBitfield)>, rng: &mut R, + assignment_criteria: &impl AssignmentCriteria, + clock: &(impl Clock + ?Sized), + session_info_provider: &mut RuntimeInfo, ) where R: CryptoRng + Rng, { @@ -980,11 +1064,15 @@ impl State { self.import_and_circulate_assignment( approval_voting_sender, network_sender, + runtime_api_sender, metrics, MessageSource::Peer(peer_id), assignment, claimed_indices, rng, + assignment_criteria, + clock, + session_info_provider, ) .await; } @@ -994,13 +1082,16 @@ impl State { async fn process_incoming_approvals< N: overseer::SubsystemSender, A: overseer::SubsystemSender, + RA: overseer::SubsystemSender, >( &mut self, approval_voting_sender: &mut A, network_sender: &mut N, + runtime_api_sender: &mut RA, metrics: &Metrics, peer_id: PeerId, approvals: Vec, + session_info_provider: &mut RuntimeInfo, ) { gum::trace!( target: LOG_TARGET, @@ -1030,9 +1121,11 @@ impl State { self.import_and_circulate_approval( approval_voting_sender, network_sender, + runtime_api_sender, metrics, MessageSource::Peer(peer_id), approval_vote, + session_info_provider, ) .await; } @@ -1041,11 +1134,13 @@ impl State { async fn process_incoming_peer_message< N: overseer::SubsystemSender, A: overseer::SubsystemSender, + RA: overseer::SubsystemSender, R, >( &mut self, approval_voting_sender: &mut A, network_sender: &mut N, + runtime_api_sender: &mut RA, metrics: &Metrics, peer_id: PeerId, msg: Versioned< @@ -1054,6 +1149,9 @@ impl State { protocol_v3::ApprovalDistributionMessage, >, rng: &mut R, + assignment_criteria: &impl AssignmentCriteria, + clock: &(impl Clock + ?Sized), + session_info_provider: &mut RuntimeInfo, ) where R: CryptoRng + Rng, { @@ -1071,10 +1169,14 @@ impl State { self.process_incoming_assignments( approval_voting_sender, network_sender, + runtime_api_sender, metrics, peer_id, sanitized_assignments, rng, + assignment_criteria, + clock, + session_info_provider, ) .await; }, @@ -1093,10 +1195,14 @@ impl State { self.process_incoming_assignments( approval_voting_sender, network_sender, + runtime_api_sender, metrics, peer_id, sanitized_assignments, rng, + assignment_criteria, + clock, + session_info_provider, ) .await; }, @@ -1106,9 +1212,11 @@ impl State { self.process_incoming_approvals( approval_voting_sender, network_sender, + runtime_api_sender, metrics, peer_id, sanitized_approvals, + session_info_provider, ) .await; }, @@ -1119,9 +1227,11 @@ impl State { self.process_incoming_approvals( approval_voting_sender, network_sender, + runtime_api_sender, metrics, peer_id, sanitized_approvals, + session_info_provider, ) .await; }, @@ -1225,16 +1335,21 @@ impl State { async fn import_and_circulate_assignment< N: overseer::SubsystemSender, A: overseer::SubsystemSender, + RA: overseer::SubsystemSender, R, >( &mut self, approval_voting_sender: &mut A, network_sender: &mut N, + runtime_api_sender: &mut RA, metrics: &Metrics, source: MessageSource, assignment: IndirectAssignmentCertV2, claimed_candidate_indices: CandidateBitfield, rng: &mut R, + assignment_criteria: &impl AssignmentCriteria, + clock: &(impl Clock + ?Sized), + session_info_provider: &mut RuntimeInfo, ) where R: CryptoRng + Rng, { @@ -1361,35 +1476,49 @@ impl State { return } - let (tx, rx) = oneshot::channel(); + let result = Self::check_assignment_valid( + assignment_criteria, + &entry, + &assignment, + &claimed_candidate_indices, + session_info_provider, + runtime_api_sender, + ) + .await; - approval_voting_sender - .send_message(ApprovalVotingMessage::CheckAndImportAssignment( - assignment.clone(), - claimed_candidate_indices.clone(), - tx, - )) - .await; + match result { + Ok(tranche) => { + let current_tranche = clock.tranche_now(self.slot_duration_millis, entry.slot); + let too_far_in_future = + current_tranche + TICK_TOO_FAR_IN_FUTURE as DelayTranche; - 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); + 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(); - gum::trace!( - target: LOG_TARGET, - ?source, - ?message_subject, - ?result, - "Checked assignment", - ); - match result { - AssignmentCheckResult::Accepted => { + return + } + + approval_voting_sender + .send_message(ApprovalVotingMessage::CheckAndImportAssignment( + assignment.clone(), + claimed_candidate_indices.clone(), + tranche, + None, + )) + .await; modify_reputation( &mut self.reputation, network_sender, @@ -1402,47 +1531,12 @@ 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, ?peer_id, - %error, + ?error, "Got a bad assignment from peer", ); modify_reputation( @@ -1583,6 +1677,46 @@ impl State { } } + async fn check_assignment_valid>( + assignment_criteria: &impl AssignmentCriteria, + entry: &BlockEntry, + assignment: &IndirectAssignmentCertV2, + claimed_candidate_indices: &CandidateBitfield, + runtime_info: &mut RuntimeInfo, + runtime_api_sender: &mut RA, + ) -> Result { + let claimed_cores: Vec = 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::>(); + if claimed_cores.is_empty() { + return Err(InvalidAssignmentError::NoClaimedCandidates) + } + + let ExtendedSessionInfo { ref session_info, .. } = runtime_info + .get_session_info_by_index(runtime_api_sender, assignment.block_hash, entry.session) + .await + .map_err(|err| InvalidAssignmentError::SessionInfoNotFound(err))?; + + assignment_criteria + .check_assignment_cert( + claimed_cores.try_into().expect("Non-empty vec; qed"), + assignment.validator, + &polkadot_node_core_approval_voting::AssignmentConfig::from(session_info), + entry.vrf_story.clone(), + &assignment.cert, + backing_groups, + ) + .map_err(|err| InvalidAssignmentError::CryptoCheckFailed(err)) + } // Checks if an approval can be processed. // Returns true if we can continue with processing the approval and false otherwise. async fn check_approval_can_be_processed< @@ -1672,13 +1806,16 @@ impl State { async fn import_and_circulate_approval< N: overseer::SubsystemSender, A: overseer::SubsystemSender, + RA: overseer::SubsystemSender, >( &mut self, approval_voting_sender: &mut A, network_sender: &mut N, + runtime_api_sender: &mut RA, metrics: &Metrics, source: MessageSource, vote: IndirectSignedApprovalVoteV2, + session_info_provider: &mut RuntimeInfo, ) { let _span = self .spans @@ -1746,31 +1883,19 @@ 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); + let result = + Self::check_vote_valid(&vote, &entry, session_info_provider, runtime_api_sender) + .await; - gum::trace!( - target: LOG_TARGET, - ?peer_id, - ?result, - ?vote, - "Checked approval", - ); match result { - ApprovalCheckResult::Accepted => { + Ok(_) => { + approval_voting_sender + .send_message(ApprovalVotingMessage::CheckAndImportApproval( + vote.clone(), + None, + )) + .await; + modify_reputation( &mut self.reputation, network_sender, @@ -1788,7 +1913,7 @@ impl State { .insert(approval_knwowledge_key.0.clone(), approval_knwowledge_key.1); } }, - ApprovalCheckResult::Bad(error) => { + Err(err) => { modify_reputation( &mut self.reputation, network_sender, @@ -1796,10 +1921,11 @@ impl State { COST_INVALID_MESSAGE, ) .await; + gum::info!( target: LOG_TARGET, ?peer_id, - %error, + ?err, "Got a bad approval from peer", ); metrics.on_approval_bad(); @@ -1897,6 +2023,49 @@ impl State { } } + // Checks if the approval vote is valid. + async fn check_vote_valid>( + vote: &IndirectSignedApprovalVoteV2, + entry: &BlockEntry, + runtime_info: &mut RuntimeInfo, + runtime_api_sender: &mut RA, + ) -> Result<(), InvalidVoteError> { + if vote.candidate_indices.len() > entry.candidates_metadata.len() { + return Err(InvalidVoteError::CandidateIndexOutOfBounds) + } + + 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 ExtendedSessionInfo { ref session_info, .. } = runtime_info + .get_session_info_by_index(runtime_api_sender, vote.block_hash, entry.session) + .await + .map_err(|err| InvalidVoteError::SessionInfoNotFound(err))?; + + let pubkey = session_info + .validators + .get(vote.validator) + .ok_or(InvalidVoteError::ValidatorIndexOutOfBounds)?; + DisputeStatement::Valid(ValidDisputeStatementKind::ApprovalCheckingMultipleCandidates( + candidate_hashes.clone(), + )) + .check_signature( + &pubkey, + *candidate_hashes.first().unwrap(), + entry.session, + &vote.signature, + ) + .map_err(|_| InvalidVoteError::InvalidSignature) + } + /// Retrieve approval signatures from state for the given relay block/indices: fn get_approval_signatures( &mut self, @@ -2474,16 +2643,40 @@ 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::new_with_clock(metrics, slot_duration_millis, Box::new(SystemClock)) + } + + /// Create a new instance of the [`ApprovalDistribution`] subsystem, with a custom clock. + pub fn new_with_clock( + metrics: Metrics, + slot_duration_millis: u64, + clock: Box, + ) -> Self { + Self { metrics, slot_duration_millis, clock } } 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(); - self.run_inner(ctx, &mut state, REPUTATION_CHANGE_INTERVAL, &mut rng).await + let assignment_criteria = RealAssignmentCriteria {}; + let mut session_info_provider = RuntimeInfo::new_with_config(RuntimeInfoConfig { + keystore: None, + session_cache_lru_size: DISPUTE_WINDOW.get(), + }); + + self.run_inner( + ctx, + &mut state, + REPUTATION_CHANGE_INTERVAL, + &mut rng, + &assignment_criteria, + &mut session_info_provider, + ) + .await } /// Used for testing. @@ -2493,11 +2686,15 @@ impl ApprovalDistribution { state: &mut State, reputation_interval: Duration, rng: &mut (impl CryptoRng + Rng), + assignment_criteria: &impl AssignmentCriteria, + session_info_provider: &mut RuntimeInfo, ) { let new_reputation_delay = || futures_timer::Delay::new(reputation_interval).fuse(); let mut reputation_delay = new_reputation_delay(); let mut approval_voting_sender = ctx.sender().clone(); let mut network_sender = ctx.sender().clone(); + let mut runtime_api_sender = ctx.sender().clone(); + loop { select! { _ = reputation_delay => { @@ -2514,7 +2711,7 @@ impl ApprovalDistribution { }; - self.handle_from_orchestra(message, &mut approval_voting_sender, &mut network_sender, state, rng).await; + self.handle_from_orchestra(message, &mut approval_voting_sender, &mut network_sender, &mut runtime_api_sender, state, rng, assignment_criteria, session_info_provider).await; }, } @@ -2525,23 +2722,31 @@ impl ApprovalDistribution { pub async fn handle_from_orchestra< N: overseer::SubsystemSender, A: overseer::SubsystemSender, + RA: overseer::SubsystemSender, >( &self, message: FromOrchestra, approval_voting_sender: &mut A, network_sender: &mut N, + runtime_api_sender: &mut RA, state: &mut State, rng: &mut (impl CryptoRng + Rng), + assignment_criteria: &impl AssignmentCriteria, + session_info_provider: &mut RuntimeInfo, ) { match message { FromOrchestra::Communication { msg } => Self::handle_incoming( approval_voting_sender, network_sender, + runtime_api_sender, state, msg, &self.metrics, rng, + assignment_criteria, + self.clock.as_ref(), + session_info_provider, ) .await, FromOrchestra::Signal(OverseerSignal::ActiveLeaves(update)) => { @@ -2568,23 +2773,48 @@ impl ApprovalDistribution { async fn handle_incoming< N: overseer::SubsystemSender, A: overseer::SubsystemSender, + RA: overseer::SubsystemSender, >( approval_voting_sender: &mut A, network_sender: &mut N, + runtime_api_sender: &mut RA, state: &mut State, msg: ApprovalDistributionMessage, metrics: &Metrics, rng: &mut (impl CryptoRng + Rng), + assignment_criteria: &impl AssignmentCriteria, + clock: &(impl Clock + ?Sized), + session_info_provider: &mut RuntimeInfo, ) { match msg { ApprovalDistributionMessage::NetworkBridgeUpdate(event) => { state - .handle_network_msg(approval_voting_sender, network_sender, metrics, event, rng) + .handle_network_msg( + approval_voting_sender, + network_sender, + runtime_api_sender, + metrics, + event, + rng, + assignment_criteria, + clock, + session_info_provider, + ) .await; }, ApprovalDistributionMessage::NewBlocks(metas) => { state - .handle_new_blocks(approval_voting_sender, network_sender, metrics, metas, rng) + .handle_new_blocks( + approval_voting_sender, + network_sender, + runtime_api_sender, + metrics, + metas, + rng, + assignment_criteria, + clock, + session_info_provider, + ) .await; }, ApprovalDistributionMessage::DistributeAssignment(cert, candidate_indices) => { @@ -2608,11 +2838,15 @@ impl ApprovalDistribution { .import_and_circulate_assignment( approval_voting_sender, network_sender, + runtime_api_sender, &metrics, MessageSource::Local, cert, candidate_indices, rng, + assignment_criteria, + clock, + session_info_provider, ) .await; }, @@ -2628,9 +2862,11 @@ impl ApprovalDistribution { .import_and_circulate_approval( approval_voting_sender, network_sender, + runtime_api_sender, metrics, MessageSource::Local, vote, + session_info_provider, ) .await; }, 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/network/approval-distribution/src/tests.rs b/polkadot/node/network/approval-distribution/src/tests.rs index 2d08807f97b6..3f926746449d 100644 --- a/polkadot/node/network/approval-distribution/src/tests.rs +++ b/polkadot/node/network/approval-distribution/src/tests.rs @@ -16,7 +16,8 @@ use super::*; use assert_matches::assert_matches; -use futures::{executor, future, Future}; +use futures::{channel::oneshot, executor, future, Future}; +use polkadot_node_core_approval_voting::criteria; use polkadot_node_network_protocol::{ grid_topology::{SessionGridTopology, TopologyPeerInfo}, our_view, @@ -34,12 +35,17 @@ use polkadot_node_primitives::approval::{ }, }; use polkadot_node_subsystem::messages::{ - network_bridge_event, AllMessages, ApprovalCheckError, ReportPeerMessage, + network_bridge_event, AllMessages, ReportPeerMessage, RuntimeApiRequest, }; use polkadot_node_subsystem_util::{reputation::add_reputation, TimeoutExt as _}; -use polkadot_primitives::{AuthorityDiscoveryId, BlakeTwo256, CoreIndex, HashT}; +use polkadot_primitives::{ + ApprovalVoteMultipleCandidates, AuthorityDiscoveryId, BlakeTwo256, CoreIndex, ExecutorParams, + HashT, NodeFeatures, SessionInfo, ValidatorId, +}; use polkadot_primitives_test_helpers::dummy_signature; use rand::SeedableRng; +use sc_keystore::{Keystore, LocalKeystore}; +use sp_application_crypto::AppCrypto; use sp_authority_discovery::AuthorityPair as AuthorityDiscoveryPair; use sp_core::crypto::Pair as PairT; use std::time::Duration; @@ -47,6 +53,8 @@ type VirtualOverseer = polkadot_node_subsystem_test_helpers::TestSubsystemContextHandle; fn test_harness>( + assignment_criteria: &impl AssignmentCriteria, + clock: Box, mut state: State, test_fn: impl FnOnce(VirtualOverseer) -> T, ) -> State { @@ -59,12 +67,22 @@ fn test_harness>( let (context, virtual_overseer) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool.clone()); - let subsystem = ApprovalDistribution::new(Default::default()); + let subsystem = + ApprovalDistribution::new_with_clock(Metrics::default(), Default::default(), clock); { let mut rng = rand_chacha::ChaCha12Rng::seed_from_u64(12345); - - let subsystem = - subsystem.run_inner(context, &mut state, REPUTATION_CHANGE_TEST_INTERVAL, &mut rng); + let mut session_info_provider = RuntimeInfo::new_with_config(RuntimeInfoConfig { + keystore: None, + session_cache_lru_size: DISPUTE_WINDOW.get(), + }); + let subsystem = subsystem.run_inner( + context, + &mut state, + REPUTATION_CHANGE_TEST_INTERVAL, + &mut rng, + assignment_criteria, + &mut session_info_provider, + ); let test_fut = test_fn(virtual_overseer); @@ -118,6 +136,41 @@ async fn overseer_recv(overseer: &mut VirtualOverseer) -> AllMessages { msg } +async fn provide_session(virtual_overseer: &mut VirtualOverseer, session_info: SessionInfo) { + assert_matches!( + overseer_recv(virtual_overseer).await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request( + _, + RuntimeApiRequest::SessionInfo(_, si_tx), + ) + ) => { + si_tx.send(Ok(Some(session_info.clone()))).unwrap(); + } + ); + assert_matches!( + overseer_recv(virtual_overseer).await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request( + _, + RuntimeApiRequest::SessionExecutorParams(_, si_tx), + ) + ) => { + // Make sure all SessionExecutorParams calls are not made for the leaf (but for its relay parent) + si_tx.send(Ok(Some(ExecutorParams::default()))).unwrap(); + } + ); + + assert_matches!( + overseer_recv(virtual_overseer).await, + AllMessages::RuntimeApi( + RuntimeApiMessage::Request(_, RuntimeApiRequest::NodeFeatures(_, si_tx), ) + ) => { + si_tx.send(Ok(NodeFeatures::EMPTY)).unwrap(); + } + ); +} + fn make_peers_and_authority_ids(n: usize) -> Vec<(PeerId, AuthorityDiscoveryId)> { (0..n) .map(|_| { @@ -375,6 +428,86 @@ fn state_with_reputation_delay() -> State { State { reputation: ReputationAggregator::new(|_| false), ..Default::default() } } +fn dummy_session_info_valid( + index: SessionIndex, + keystore: &mut LocalKeystore, + num_validators: usize, +) -> SessionInfo { + let keys = (0..num_validators) + .map(|_| { + keystore + .sr25519_generate_new(ValidatorId::ID, Some("//Node")) + .expect("Insert key into keystore") + }) + .collect_vec(); + + SessionInfo { + validators: keys.clone().into_iter().map(|key| key.into()).collect(), + discovery_keys: keys.clone().into_iter().map(|key| key.into()).collect(), + assignment_keys: keys.clone().into_iter().map(|key| key.into()).collect(), + validator_groups: Default::default(), + n_cores: index as _, + zeroth_delay_tranche_width: index as _, + relay_vrf_modulo_samples: index as _, + n_delay_tranches: index as _, + no_show_slots: index as _, + needed_approvals: index as _, + active_validator_indices: Vec::new(), + dispute_period: 6, + random_seed: [0u8; 32], + } +} + +fn signature_for( + keystore: &LocalKeystore, + session: &SessionInfo, + candidate_hashes: Vec, + validator_index: ValidatorIndex, +) -> ValidatorSignature { + let payload = ApprovalVoteMultipleCandidates(&candidate_hashes).signing_payload(1); + let sign_key = session.validators.get(validator_index).unwrap().clone(); + let signature = keystore + .sr25519_sign(ValidatorId::ID, &sign_key.into(), &payload[..]) + .unwrap() + .unwrap(); + signature.into() +} + +struct MockAssignmentCriteria { + tranche: + Result, +} + +impl AssignmentCriteria for MockAssignmentCriteria { + fn compute_assignments( + &self, + _keystore: &LocalKeystore, + _relay_vrf_story: polkadot_node_primitives::approval::v1::RelayVRFStory, + _config: &criteria::Config, + _leaving_cores: Vec<( + CandidateHash, + polkadot_primitives::CoreIndex, + polkadot_primitives::GroupIndex, + )>, + _enable_assignments_v2: bool, + ) -> HashMap { + HashMap::new() + } + + fn check_assignment_cert( + &self, + _claimed_core_bitfield: polkadot_node_primitives::approval::v2::CoreBitfield, + _validator_index: polkadot_primitives::ValidatorIndex, + _config: &criteria::Config, + _relay_vrf_story: polkadot_node_primitives::approval::v1::RelayVRFStory, + _assignment: &polkadot_node_primitives::approval::v2::AssignmentCertV2, + _backing_groups: Vec, + ) -> Result + { + self.tranche + } +} + /// import an assignment /// connect a new peer /// the new peer sends us the same assignment @@ -388,89 +521,100 @@ fn try_import_the_same_assignment() { let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // setup peers - setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V1).await; - setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V1).await; - setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V1).await; - - // Set up a gossip topology, where a, b, c and d are topology neighbors to the node under - // testing. - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), - ) - .await; + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // setup peers + setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V1).await; + setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V1).await; + setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V1).await; + + // Set up a gossip topology, where a, b, c and d are topology neighbors to the node + // under testing. + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), + ) + .await; - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 2, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - // send the assignment related to `hash` - let validator_index = ValidatorIndex(0); - let cert = fake_assignment_cert(hash, validator_index); - let assignments = vec![(cert.clone(), 0u32)]; - - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); - send_message_from_peer(overseer, &peer_a, msg).await; - - expect_reputation_change(overseer, &peer_a, COST_UNEXPECTED_MESSAGE).await; - - // send an `Accept` message from the Approval Voting subsystem - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - assignment, - claimed_indices, - tx, - )) => { - assert_eq!(claimed_indices, 0u32.into()); - assert_eq!(assignment, cert.into()); - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 2, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + // send the assignment related to `hash` + let validator_index = ValidatorIndex(0); + let cert = fake_assignment_cert(hash, validator_index); + let assignments = vec![(cert.clone(), 0u32)]; - expect_reputation_change(overseer, &peer_a, BENEFIT_VALID_MESSAGE_FIRST).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 2); - assert_eq!(assignments.len(), 1); - } - ); + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + send_message_from_peer(overseer, &peer_a, msg).await; + + expect_reputation_change(overseer, &peer_a, COST_UNEXPECTED_MESSAGE).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; + // send an `Accept` message from the Approval Voting subsystem + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + assignment, + claimed_indices, + tranche, + _, + )) => { + assert_eq!(claimed_indices, 0u32.into()); + assert_eq!(assignment, cert.into()); + assert_eq!(tranche, 0); + } + ); - // setup new peer with V2 - setup_peer_with_view(overseer, &peer_d, view![], ValidationVersion::V3).await; + expect_reputation_change(overseer, &peer_a, BENEFIT_VALID_MESSAGE_FIRST).await; - // send the same assignment from peer_d - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments); - send_message_from_peer(overseer, &peer_d, msg).await; + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 2); + assert_eq!(assignments.len(), 1); + } + ); - expect_reputation_change(overseer, &peer_d, COST_UNEXPECTED_MESSAGE).await; - expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE).await; + // setup new peer with V2 + setup_peer_with_view(overseer, &peer_d, view![], ValidationVersion::V3).await; - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + // send the same assignment from peer_d + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments); + send_message_from_peer(overseer, &peer_d, msg).await; + + expect_reputation_change(overseer, &peer_d, COST_UNEXPECTED_MESSAGE).await; + expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE).await; + + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } /// Just like `try_import_the_same_assignment` but use `VRFModuloCompact` assignments for multiple @@ -485,97 +629,108 @@ fn try_import_the_same_assignment_v2() { let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // setup peers - setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V3).await; - setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V3).await; - setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V3).await; - - // Set up a gossip topology, where a, b, c and d are topology neighbors to the node under - // testing. - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), - ) - .await; + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // setup peers + setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V3).await; + setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V3).await; + setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V3).await; + + // Set up a gossip topology, where a, b, c and d are topology neighbors to the node + // under testing. + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), + ) + .await; - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 2, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - // send the assignment related to `hash` - let validator_index = ValidatorIndex(0); - let cores = vec![1, 2, 3, 4]; - let core_bitfield: CoreBitfield = cores - .iter() - .map(|index| CoreIndex(*index)) - .collect::>() - .try_into() - .unwrap(); - - let cert = fake_assignment_cert_v2(hash, validator_index, core_bitfield.clone()); - let assignments = vec![(cert.clone(), cores.clone().try_into().unwrap())]; - - let msg = protocol_v3::ApprovalDistributionMessage::Assignments(assignments.clone()); - send_message_from_peer_v3(overseer, &peer_a, msg).await; - - expect_reputation_change(overseer, &peer_a, COST_UNEXPECTED_MESSAGE).await; - - // send an `Accept` message from the Approval Voting subsystem - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - assignment, - claimed_indices, - tx, - )) => { - assert_eq!(claimed_indices, cores.try_into().unwrap()); - assert_eq!(assignment, cert.into()); - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 2, + candidates: vec![Default::default(); 4], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + // send the assignment related to `hash` + let validator_index = ValidatorIndex(0); + let cores = vec![1, 2, 3, 4]; + let core_bitfield: CoreBitfield = cores + .iter() + .map(|index| CoreIndex(*index)) + .collect::>() + .try_into() + .unwrap(); + + let cert = fake_assignment_cert_v2(hash, validator_index, core_bitfield.clone()); + let assignments = vec![(cert.clone(), cores.clone().try_into().unwrap())]; + + let msg = protocol_v3::ApprovalDistributionMessage::Assignments(assignments.clone()); + send_message_from_peer_v3(overseer, &peer_a, msg).await; + + expect_reputation_change(overseer, &peer_a, COST_UNEXPECTED_MESSAGE).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; + // send an `Accept` message from the Approval Voting subsystem + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + assignment, + claimed_indices, + tranche, + _, + )) => { + assert_eq!(claimed_indices, cores.try_into().unwrap()); + assert_eq!(assignment, cert.into()); + assert_eq!(tranche, 0); + } + ); - expect_reputation_change(overseer, &peer_a, BENEFIT_VALID_MESSAGE_FIRST).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 2); - assert_eq!(assignments.len(), 1); - } - ); + expect_reputation_change(overseer, &peer_a, BENEFIT_VALID_MESSAGE_FIRST).await; - // setup new peer - setup_peer_with_view(overseer, &peer_d, view![], ValidationVersion::V3).await; + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 2); + assert_eq!(assignments.len(), 1); + } + ); - // send the same assignment from peer_d - let msg = protocol_v3::ApprovalDistributionMessage::Assignments(assignments); - send_message_from_peer_v3(overseer, &peer_d, msg).await; + // setup new peer + setup_peer_with_view(overseer, &peer_d, view![], ValidationVersion::V3).await; - expect_reputation_change(overseer, &peer_d, COST_UNEXPECTED_MESSAGE).await; - expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE).await; + // send the same assignment from peer_d + let msg = protocol_v3::ApprovalDistributionMessage::Assignments(assignments); + send_message_from_peer_v3(overseer, &peer_d, msg).await; - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + expect_reputation_change(overseer, &peer_d, COST_UNEXPECTED_MESSAGE).await; + expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE).await; + + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } /// import an assignment @@ -587,55 +742,67 @@ fn delay_reputation_change() { let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); - let _ = test_harness(state_with_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_with_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + + // Setup peers + setup_peer_with_view(overseer, &peer, view![], ValidationVersion::V1).await; + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 2, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + // send the assignment related to `hash` + let validator_index = ValidatorIndex(0); + let cert = fake_assignment_cert(hash, validator_index); + let assignments = vec![(cert.clone(), 0u32)]; - // Setup peers - setup_peer_with_view(overseer, &peer, view![], ValidationVersion::V1).await; - - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 2, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - // send the assignment related to `hash` - let validator_index = ValidatorIndex(0); - let cert = fake_assignment_cert(hash, validator_index); - let assignments = vec![(cert.clone(), 0u32)]; - - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); - send_message_from_peer(overseer, &peer, msg).await; - - // send an `Accept` message from the Approval Voting subsystem - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - assignment, - claimed_candidates, - tx, - )) => { - assert_eq!(assignment.cert, cert.cert.into()); - assert_eq!(claimed_candidates, vec![0u32].try_into().unwrap()); - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); - expect_reputation_changes( - overseer, - &peer, - vec![COST_UNEXPECTED_MESSAGE, BENEFIT_VALID_MESSAGE_FIRST], - ) - .await; - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + send_message_from_peer(overseer, &peer, msg).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; - virtual_overseer - }); + // send an `Accept` message from the Approval Voting subsystem + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + assignment, + claimed_candidates, + tranche, + _, + )) => { + assert_eq!(assignment.cert, cert.cert.into()); + assert_eq!(claimed_candidates, vec![0u32].try_into().unwrap()); + assert_eq!(tranche, 0); + } + ); + expect_reputation_changes( + overseer, + &peer, + vec![COST_UNEXPECTED_MESSAGE, BENEFIT_VALID_MESSAGE_FIRST], + ) + .await; + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + + virtual_overseer + }, + ); } /// @@ -650,77 +817,90 @@ fn spam_attack_results_in_negative_reputation_change() { let peer_a = PeerId::random(); let hash_b = Hash::repeat_byte(0xBB); - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - let peer = &peer_a; - setup_peer_with_view(overseer, peer, view![], ValidationVersion::V1).await; - - // new block `hash_b` with 20 candidates - let candidates_count = 20; - let meta = BlockApprovalMeta { - hash: hash_b, - parent_hash, - number: 2, - candidates: vec![Default::default(); candidates_count], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - // send 20 assignments related to `hash_b` - // to populate our knowledge - let assignments: Vec<_> = (0..candidates_count) - .map(|candidate_index| { - let validator_index = ValidatorIndex(candidate_index as u32); - let cert = fake_assignment_cert(hash_b, validator_index); - (cert, candidate_index as u32) - }) - .collect(); - - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); - send_message_from_peer(overseer, peer, msg.clone()).await; - - for i in 0..candidates_count { - expect_reputation_change(overseer, peer, COST_UNEXPECTED_MESSAGE).await; + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + let peer = &peer_a; + setup_peer_with_view(overseer, peer, view![], ValidationVersion::V1).await; + + // new block `hash_b` with 20 candidates + let candidates_count = 20; + let meta = BlockApprovalMeta { + hash: hash_b, + parent_hash, + number: 2, + candidates: vec![Default::default(); candidates_count], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + // send 20 assignments related to `hash_b` + // to populate our knowledge + let assignments: Vec<_> = (0..candidates_count) + .map(|candidate_index| { + let validator_index = ValidatorIndex(candidate_index as u32); + let cert = fake_assignment_cert(hash_b, validator_index); + (cert, candidate_index as u32) + }) + .collect(); - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - assignment, - claimed_candidate_index, - tx, - )) => { - assert_eq!(assignment, assignments[i].0.clone().into()); - assert_eq!(claimed_candidate_index, assignments[i].1.into()); - tx.send(AssignmentCheckResult::Accepted).unwrap(); + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + send_message_from_peer(overseer, peer, msg.clone()).await; + + for i in 0..candidates_count { + expect_reputation_change(overseer, peer, COST_UNEXPECTED_MESSAGE).await; + if i == 0 { + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; } - ); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + assignment, + claimed_candidate_index, + tranche, + _, + )) => { + assert_eq!(assignment, assignments[i].0.clone().into()); + assert_eq!(claimed_candidate_index, assignments[i].1.into()); + assert_eq!(tranche, 0); + } + ); - expect_reputation_change(overseer, peer, BENEFIT_VALID_MESSAGE_FIRST).await; - } + expect_reputation_change(overseer, peer, BENEFIT_VALID_MESSAGE_FIRST).await; + } - // send a view update that removes block B from peer's view by bumping the finalized_number - overseer_send( - overseer, - ApprovalDistributionMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerViewChange( - *peer, - View::with_finalized(2), - )), - ) - .await; + // send a view update that removes block B from peer's view by bumping the + // finalized_number + overseer_send( + overseer, + ApprovalDistributionMessage::NetworkBridgeUpdate( + NetworkBridgeEvent::PeerViewChange(*peer, View::with_finalized(2)), + ), + ) + .await; - // send the assignments again - send_message_from_peer(overseer, peer, msg.clone()).await; + // send the assignments again + send_message_from_peer(overseer, peer, msg.clone()).await; - // each of them will incur `COST_UNEXPECTED_MESSAGE`, not only the first one - for _ in 0..candidates_count { - expect_reputation_change(overseer, peer, COST_UNEXPECTED_MESSAGE).await; - expect_reputation_change(overseer, peer, BENEFIT_VALID_MESSAGE).await; - } - virtual_overseer - }); + // each of them will incur `COST_UNEXPECTED_MESSAGE`, not only the first one + for _ in 0..candidates_count { + expect_reputation_change(overseer, peer, COST_UNEXPECTED_MESSAGE).await; + expect_reputation_change(overseer, peer, BENEFIT_VALID_MESSAGE).await; + } + virtual_overseer + }, + ); } /// Imagine we send a message to peer A and peer B. @@ -736,86 +916,94 @@ fn peer_sending_us_the_same_we_just_sent_them_is_ok() { let peers = make_peers_and_authority_ids(8); let peer_a = peers.first().unwrap().0; - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - let peer = &peer_a; - setup_peer_with_view(overseer, peer, view![], ValidationVersion::V1).await; - - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Setup a topology where peer_a is neighbor to current node. - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0], &[2], 1), - ) - .await; + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + let peer = &peer_a; + setup_peer_with_view(overseer, peer, view![], ValidationVersion::V1).await; + + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Setup a topology where peer_a is neighbor to current node. + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0], &[2], 1), + ) + .await; - // new block `hash` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - // import an assignment related to `hash` locally - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - let cert = fake_assignment_cert(hash, validator_index); - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment( - cert.clone().into(), - candidate_index.into(), - ), - ) - .await; + // new block `hash` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + // import an assignment related to `hash` locally + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; + let cert = fake_assignment_cert(hash, validator_index); + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment( + cert.clone().into(), + candidate_index.into(), + ), + ) + .await; - // update peer view to include the hash - overseer_send( - overseer, - ApprovalDistributionMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerViewChange( - *peer, - view![hash], - )), - ) - .await; + // update peer view to include the hash + overseer_send( + overseer, + ApprovalDistributionMessage::NetworkBridgeUpdate( + NetworkBridgeEvent::PeerViewChange(*peer, view![hash]), + ), + ) + .await; - // we should send them the assignment - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 1); - assert_eq!(assignments.len(), 1); - } - ); + // we should send them the assignment + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 1); + assert_eq!(assignments.len(), 1); + } + ); - // but if someone else is sending it the same assignment - // the peer could send us it as well - let assignments = vec![(cert, candidate_index)]; - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments); - send_message_from_peer(overseer, peer, msg.clone()).await; + // but if someone else is sending it the same assignment + // the peer could send us it as well + let assignments = vec![(cert, candidate_index)]; + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments); + send_message_from_peer(overseer, peer, msg.clone()).await; - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "we should not punish the peer"); + assert!( + overseer.recv().timeout(TIMEOUT).await.is_none(), + "we should not punish the peer" + ); - // send the assignments again - send_message_from_peer(overseer, peer, msg).await; + // send the assignments again + send_message_from_peer(overseer, peer, msg).await; - // now we should - expect_reputation_change(overseer, peer, COST_DUPLICATE_MESSAGE).await; - virtual_overseer - }); + // now we should + expect_reputation_change(overseer, peer, COST_DUPLICATE_MESSAGE).await; + virtual_overseer + }, + ); } #[test] @@ -827,116 +1015,134 @@ fn import_approval_happy_path_v1_v2_peers() { let peer_c = peers.get(2).unwrap().0; let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); + let candidate_hash = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xBB)); + + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // setup peers with V1 and V2 protocol versions + setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V1).await; + setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V3).await; + setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V1).await; + + let mut keystore = LocalKeystore::in_memory(); + let session = dummy_session_info_valid(1, &mut keystore, 1); + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![(candidate_hash, 0.into(), 0.into()); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Set up a gossip topology, where a, b, and c are topology neighbors to the node. + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), + ) + .await; - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // setup peers with V1 and V2 protocol versions - setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V1).await; - setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V3).await; - setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V1).await; - - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Set up a gossip topology, where a, b, and c are topology neighbors to the node. - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), - ) - .await; - - // import an assignment related to `hash` locally - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - let cert = fake_assignment_cert(hash, validator_index); - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment( - cert.clone().into(), - candidate_index.into(), - ), - ) - .await; + // import an assignment related to `hash` locally + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; + let cert = fake_assignment_cert(hash, validator_index); + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment( + cert.clone().into(), + candidate_index.into(), + ), + ) + .await; - // 1 peer is v1 - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 1); - assert_eq!(assignments.len(), 1); - } - ); + // 1 peer is v1 + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 1); + assert_eq!(assignments.len(), 1); + } + ); - // 1 peer is v2 - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 1); - assert_eq!(assignments.len(), 1); - } - ); + // 1 peer is v2 + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 1); + assert_eq!(assignments.len(), 1); + } + ); - // send the an approval from peer_b - let approval = IndirectSignedApprovalVoteV2 { - block_hash: hash, - candidate_indices: candidate_index.into(), - validator: validator_index, - signature: dummy_signature(), - }; - let msg: protocol_v3::ApprovalDistributionMessage = - protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); - send_message_from_peer_v3(overseer, &peer_b, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( - vote, - tx, - )) => { - assert_eq!(vote, approval); - tx.send(ApprovalCheckResult::Accepted).unwrap(); - } - ); + // send the an approval from peer_b + let approval = IndirectSignedApprovalVoteV2 { + block_hash: hash, + candidate_indices: candidate_index.into(), + validator: validator_index, + signature: signature_for( + &keystore, + &session, + vec![candidate_hash], + validator_index, + ), + }; + let msg: protocol_v3::ApprovalDistributionMessage = + protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); + send_message_from_peer_v3(overseer, &peer_b, msg).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; - expect_reputation_change(overseer, &peer_b, BENEFIT_VALID_MESSAGE_FIRST).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Approvals(approvals) - )) - )) => { - assert_eq!(peers.len(), 1); - assert_eq!(approvals.len(), 1); - } - ); - virtual_overseer - }); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( + vote, _, + )) => { + assert_eq!(vote, approval); + } + ); + + expect_reputation_change(overseer, &peer_b, BENEFIT_VALID_MESSAGE_FIRST).await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Approvals(approvals) + )) + )) => { + assert_eq!(peers.len(), 1); + assert_eq!(approvals.len(), 1); + } + ); + virtual_overseer + }, + ); } // Test a v2 approval that signs multiple candidate is correctly processed. @@ -949,103 +1155,123 @@ fn import_approval_happy_path_v2() { let peer_c = peers.get(2).unwrap().0; let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); + let candidate_hash_first = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xBB)); + let candidate_hash_second = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xCC)); + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // setup peers with V2 protocol versions + setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V3).await; + setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V3).await; + setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V3).await; + let mut keystore = LocalKeystore::in_memory(); + let session = dummy_session_info_valid(1, &mut keystore, 1); + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![ + (candidate_hash_first, 0.into(), 0.into()), + (candidate_hash_second, 1.into(), 1.into()), + ], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Set up a gossip topology, where a, b, and c are topology neighbors to the node. + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), + ) + .await; - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // setup peers with V2 protocol versions - setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V3).await; - setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V3).await; - setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V3).await; - - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 2], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Set up a gossip topology, where a, b, and c are topology neighbors to the node. - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), - ) - .await; + // import an assignment related to `hash` locally + let validator_index = ValidatorIndex(0); + let candidate_indices: CandidateBitfield = + vec![0 as CandidateIndex, 1 as CandidateIndex].try_into().unwrap(); + let candidate_bitfields = vec![CoreIndex(0), CoreIndex(1)].try_into().unwrap(); + let cert = fake_assignment_cert_v2(hash, validator_index, candidate_bitfields); + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment( + cert.clone().into(), + candidate_indices.clone(), + ), + ) + .await; - // import an assignment related to `hash` locally - let validator_index = ValidatorIndex(0); - let candidate_indices: CandidateBitfield = - vec![0 as CandidateIndex, 1 as CandidateIndex].try_into().unwrap(); - let candidate_bitfields = vec![CoreIndex(0), CoreIndex(1)].try_into().unwrap(); - let cert = fake_assignment_cert_v2(hash, validator_index, candidate_bitfields); - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment( - cert.clone().into(), - candidate_indices.clone(), - ), - ) - .await; + // 1 peer is v2 + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 2); + assert_eq!(assignments.len(), 1); + } + ); - // 1 peer is v2 - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 2); - assert_eq!(assignments.len(), 1); - } - ); + // send the an approval from peer_b + let approval = IndirectSignedApprovalVoteV2 { + block_hash: hash, + candidate_indices, + validator: validator_index, + signature: signature_for( + &keystore, + &session, + vec![candidate_hash_first, candidate_hash_second], + validator_index, + ), + }; + let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); + send_message_from_peer_v3(overseer, &peer_b, msg).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; - // send the an approval from peer_b - let approval = IndirectSignedApprovalVoteV2 { - block_hash: hash, - candidate_indices, - validator: validator_index, - signature: dummy_signature(), - }; - let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); - send_message_from_peer_v3(overseer, &peer_b, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( - vote, - tx, - )) => { - assert_eq!(vote, approval); - tx.send(ApprovalCheckResult::Accepted).unwrap(); - } - ); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( + vote, _, + )) => { + assert_eq!(vote, approval); + } + ); - expect_reputation_change(overseer, &peer_b, BENEFIT_VALID_MESSAGE_FIRST).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Approvals(approvals) - )) - )) => { - assert_eq!(peers.len(), 1); - assert_eq!(approvals.len(), 1); - } - ); - virtual_overseer - }); + expect_reputation_change(overseer, &peer_b, BENEFIT_VALID_MESSAGE_FIRST).await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Approvals(approvals) + )) + )) => { + assert_eq!(peers.len(), 1); + assert_eq!(approvals.len(), 1); + } + ); + virtual_overseer + }, + ); } // Tests that votes that cover multiple assignments candidates are correctly processed on importing @@ -1059,187 +1285,205 @@ fn multiple_assignments_covered_with_one_approval_vote() { let peer_d = peers.get(4).unwrap().0; let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); + let candidate_hash_first = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xBB)); + let candidate_hash_second = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xCC)); + + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // setup peers with V2 protocol versions + setup_peer_with_view(overseer, &peer_a, view![hash], ValidationVersion::V3).await; + setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V3).await; + setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V3).await; + setup_peer_with_view(overseer, &peer_d, view![hash], ValidationVersion::V3).await; + + let mut keystore = LocalKeystore::in_memory(); + let session = dummy_session_info_valid(1, &mut keystore, 5); + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![ + (candidate_hash_first, 0.into(), 0.into()), + (candidate_hash_second, 1.into(), 1.into()), + ], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Set up a gossip topology, where a, b, and c, d are topology neighbors to the node. + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), + ) + .await; - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // setup peers with V2 protocol versions - setup_peer_with_view(overseer, &peer_a, view![hash], ValidationVersion::V3).await; - setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V3).await; - setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V3).await; - setup_peer_with_view(overseer, &peer_d, view![hash], ValidationVersion::V3).await; - - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 2], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Set up a gossip topology, where a, b, and c, d are topology neighbors to the node. - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), - ) - .await; + // import an assignment related to `hash` locally + let validator_index = ValidatorIndex(2); // peer_c is the originator + let candidate_indices: CandidateBitfield = + vec![0 as CandidateIndex, 1 as CandidateIndex].try_into().unwrap(); - // import an assignment related to `hash` locally - let validator_index = ValidatorIndex(2); // peer_c is the originator - let candidate_indices: CandidateBitfield = - vec![0 as CandidateIndex, 1 as CandidateIndex].try_into().unwrap(); - - let core_bitfields = vec![CoreIndex(0)].try_into().unwrap(); - let cert = fake_assignment_cert_v2(hash, validator_index, core_bitfields); - - // send the candidate 0 assignment from peer_b - let assignment = IndirectAssignmentCertV2 { - block_hash: hash, - validator: validator_index, - cert: cert.cert, - }; - let msg = protocol_v3::ApprovalDistributionMessage::Assignments(vec![( - assignment, - (0 as CandidateIndex).into(), - )]); - send_message_from_peer_v3(overseer, &peer_d, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - _, _, - tx, - )) => { - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); - expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE_FIRST).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert!(peers.len() >= 2); - assert!(peers.contains(&peer_a)); - assert!(peers.contains(&peer_b)); - assert_eq!(assignments.len(), 1); - } - ); + let core_bitfields = vec![CoreIndex(0)].try_into().unwrap(); + let cert = fake_assignment_cert_v2(hash, validator_index, core_bitfields); - let candidate_bitfields = vec![CoreIndex(1)].try_into().unwrap(); - let cert = fake_assignment_cert_v2(hash, validator_index, candidate_bitfields); - - // send the candidate 1 assignment from peer_c - let assignment = IndirectAssignmentCertV2 { - block_hash: hash, - validator: validator_index, - cert: cert.cert, - }; - let msg = protocol_v3::ApprovalDistributionMessage::Assignments(vec![( - assignment, - (1 as CandidateIndex).into(), - )]); - - send_message_from_peer_v3(overseer, &peer_c, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - _, _, - tx, - )) => { - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); - expect_reputation_change(overseer, &peer_c, BENEFIT_VALID_MESSAGE_FIRST).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert!(peers.len() >= 2); - assert!(peers.contains(&peer_b)); - assert!(peers.contains(&peer_a)); - assert_eq!(assignments.len(), 1); - } - ); + // send the candidate 0 assignment from peer_b + let assignment = IndirectAssignmentCertV2 { + block_hash: hash, + validator: validator_index, + cert: cert.cert, + }; + let msg = protocol_v3::ApprovalDistributionMessage::Assignments(vec![( + assignment, + (0 as CandidateIndex).into(), + )]); + send_message_from_peer_v3(overseer, &peer_d, msg).await; + provide_session(overseer, session.clone()).await; - // send an approval from peer_b - let approval = IndirectSignedApprovalVoteV2 { - block_hash: hash, - candidate_indices, - validator: validator_index, - signature: dummy_signature(), - }; - let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); - send_message_from_peer_v3(overseer, &peer_d, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( - vote, - tx, - )) => { - assert_eq!(vote, approval); - tx.send(ApprovalCheckResult::Accepted).unwrap(); - } - ); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + _, _, + tranche, + _, + )) => { + assert_eq!(tranche, 0); + } + ); + expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE_FIRST).await; - expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE_FIRST).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Approvals(approvals) - )) - )) => { - assert!(peers.len() >= 2); - assert!(peers.contains(&peer_b)); - assert!(peers.contains(&peer_a)); - assert_eq!(approvals.len(), 1); - } - ); - for candidate_index in 0..1 { - let (tx_distribution, rx_distribution) = oneshot::channel(); - let mut candidates_requesting_signatures = HashSet::new(); - candidates_requesting_signatures.insert((hash, candidate_index)); - overseer_send( - overseer, - ApprovalDistributionMessage::GetApprovalSignatures( - candidates_requesting_signatures, - tx_distribution, + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert!(peers.len() >= 2); + assert!(peers.contains(&peer_a)); + assert!(peers.contains(&peer_b)); + assert_eq!(assignments.len(), 1); + } + ); + + let candidate_bitfields = vec![CoreIndex(1)].try_into().unwrap(); + let cert = fake_assignment_cert_v2(hash, validator_index, candidate_bitfields); + + // send the candidate 1 assignment from peer_c + let assignment = IndirectAssignmentCertV2 { + block_hash: hash, + validator: validator_index, + cert: cert.cert, + }; + let msg = protocol_v3::ApprovalDistributionMessage::Assignments(vec![( + assignment, + (1 as CandidateIndex).into(), + )]); + + send_message_from_peer_v3(overseer, &peer_c, msg).await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + _, _, + tranche, _, + )) => { + assert_eq!(tranche, 0); + } + ); + expect_reputation_change(overseer, &peer_c, BENEFIT_VALID_MESSAGE_FIRST).await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert!(peers.len() >= 2); + assert!(peers.contains(&peer_b)); + assert!(peers.contains(&peer_a)); + assert_eq!(assignments.len(), 1); + } + ); + + // send an approval from peer_b + let approval = IndirectSignedApprovalVoteV2 { + block_hash: hash, + candidate_indices, + validator: validator_index, + signature: signature_for( + &keystore, + &session, + vec![candidate_hash_first, candidate_hash_second], + validator_index, ), - ) - .await; - let signatures = rx_distribution.await.unwrap(); - - assert_eq!(signatures.len(), 1); - for (signing_validator, signature) in signatures { - assert_eq!(validator_index, signing_validator); - assert_eq!(signature.0, hash); - assert_eq!(signature.2, approval.signature); - assert_eq!(signature.1, vec![0, 1]); + }; + let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); + send_message_from_peer_v3(overseer, &peer_d, msg).await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( + vote, _, + )) => { + assert_eq!(vote, approval); + } + ); + + expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE_FIRST).await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Approvals(approvals) + )) + )) => { + assert!(peers.len() >= 2); + assert!(peers.contains(&peer_b)); + assert!(peers.contains(&peer_a)); + assert_eq!(approvals.len(), 1); + } + ); + for candidate_index in 0..1 { + let (tx_distribution, rx_distribution) = oneshot::channel(); + let mut candidates_requesting_signatures = HashSet::new(); + candidates_requesting_signatures.insert((hash, candidate_index)); + overseer_send( + overseer, + ApprovalDistributionMessage::GetApprovalSignatures( + candidates_requesting_signatures, + tx_distribution, + ), + ) + .await; + let signatures = rx_distribution.await.unwrap(); + + assert_eq!(signatures.len(), 1); + for (signing_validator, signature) in signatures { + assert_eq!(validator_index, signing_validator); + assert_eq!(signature.0, hash); + assert_eq!(signature.2, approval.signature); + assert_eq!(signature.1, vec![0, 1]); + } } - } - virtual_overseer - }); + virtual_overseer + }, + ); } // Tests that votes that cover multiple assignments candidates are correctly processed when unify @@ -1253,182 +1497,198 @@ fn unify_with_peer_multiple_assignments_covered_with_one_approval_vote() { let peer_d = peers.get(4).unwrap().0; let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); + let candidate_hash_first = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xBB)); + let candidate_hash_second = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xCC)); + + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + setup_peer_with_view(overseer, &peer_d, view![hash], ValidationVersion::V3).await; + let mut keystore = LocalKeystore::in_memory(); + let session = dummy_session_info_valid(1, &mut keystore, 5); + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![ + (candidate_hash_first, 0.into(), 0.into()), + (candidate_hash_second, 1.into(), 1.into()), + ], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Set up a gossip topology, where a, b, and c, d are topology neighbors to the node. + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), + ) + .await; - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - setup_peer_with_view(overseer, &peer_d, view![hash], ValidationVersion::V3).await; - - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 2], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Set up a gossip topology, where a, b, and c, d are topology neighbors to the node. - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), - ) - .await; + // import an assignment related to `hash` locally + let validator_index = ValidatorIndex(2); // peer_c is the originator + let candidate_indices: CandidateBitfield = + vec![0 as CandidateIndex, 1 as CandidateIndex].try_into().unwrap(); - // import an assignment related to `hash` locally - let validator_index = ValidatorIndex(2); // peer_c is the originator - let candidate_indices: CandidateBitfield = - vec![0 as CandidateIndex, 1 as CandidateIndex].try_into().unwrap(); - - let core_bitfields = vec![CoreIndex(0)].try_into().unwrap(); - let cert = fake_assignment_cert_v2(hash, validator_index, core_bitfields); - - // send the candidate 0 assignment from peer_b - let assignment = IndirectAssignmentCertV2 { - block_hash: hash, - validator: validator_index, - cert: cert.cert, - }; - let msg = protocol_v3::ApprovalDistributionMessage::Assignments(vec![( - assignment, - (0 as CandidateIndex).into(), - )]); - send_message_from_peer_v3(overseer, &peer_d, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - _, _, - tx, - )) => { - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); - expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE_FIRST).await; - - let candidate_bitfields = vec![CoreIndex(1)].try_into().unwrap(); - let cert = fake_assignment_cert_v2(hash, validator_index, candidate_bitfields); - - // send the candidate 1 assignment from peer_c - let assignment = IndirectAssignmentCertV2 { - block_hash: hash, - validator: validator_index, - cert: cert.cert, - }; - let msg = protocol_v3::ApprovalDistributionMessage::Assignments(vec![( - assignment, - (1 as CandidateIndex).into(), - )]); - - send_message_from_peer_v3(overseer, &peer_d, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - _, _, - tx, - )) => { - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); - expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE_FIRST).await; - - // send an approval from peer_b - let approval = IndirectSignedApprovalVoteV2 { - block_hash: hash, - candidate_indices, - validator: validator_index, - signature: dummy_signature(), - }; - let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); - send_message_from_peer_v3(overseer, &peer_d, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( - vote, - tx, - )) => { - assert_eq!(vote, approval); - tx.send(ApprovalCheckResult::Accepted).unwrap(); - } - ); + let core_bitfields = vec![CoreIndex(0)].try_into().unwrap(); + let cert = fake_assignment_cert_v2(hash, validator_index, core_bitfields); - expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE_FIRST).await; - - // setup peers with V2 protocol versions - setup_peer_with_view(overseer, &peer_a, view![hash], ValidationVersion::V3).await; - setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V3).await; - let mut expected_peers_assignments = vec![peer_a, peer_b]; - let mut expected_peers_approvals = vec![peer_a, peer_b]; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert!(peers.len() == 1); - assert!(expected_peers_assignments.contains(peers.first().unwrap())); - expected_peers_assignments.retain(|peer| peer != peers.first().unwrap()); - assert_eq!(assignments.len(), 2); - } - ); + // send the candidate 0 assignment from peer_b + let assignment = IndirectAssignmentCertV2 { + block_hash: hash, + validator: validator_index, + cert: cert.cert, + }; + let msg = protocol_v3::ApprovalDistributionMessage::Assignments(vec![( + assignment, + (0 as CandidateIndex).into(), + )]); + send_message_from_peer_v3(overseer, &peer_d, msg).await; + provide_session(overseer, session.clone()).await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Approvals(approvals) - )) - )) => { - assert!(peers.len() == 1); - assert!(expected_peers_approvals.contains(peers.first().unwrap())); - expected_peers_approvals.retain(|peer| peer != peers.first().unwrap()); - assert_eq!(approvals.len(), 1); - } - ); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + _, _, + tranche, _, + )) => { + assert_eq!(tranche, 0); + } + ); + expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE_FIRST).await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert!(peers.len() == 1); - assert!(expected_peers_assignments.contains(peers.first().unwrap())); - expected_peers_assignments.retain(|peer| peer != peers.first().unwrap()); - assert_eq!(assignments.len(), 2); - } - ); + let candidate_bitfields = vec![CoreIndex(1)].try_into().unwrap(); + let cert = fake_assignment_cert_v2(hash, validator_index, candidate_bitfields); - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Approvals(approvals) - )) - )) => { - assert!(peers.len() == 1); - assert!(expected_peers_approvals.contains(peers.first().unwrap())); - expected_peers_approvals.retain(|peer| peer != peers.first().unwrap()); - assert_eq!(approvals.len(), 1); - } - ); + // send the candidate 1 assignment from peer_c + let assignment = IndirectAssignmentCertV2 { + block_hash: hash, + validator: validator_index, + cert: cert.cert, + }; + let msg = protocol_v3::ApprovalDistributionMessage::Assignments(vec![( + assignment, + (1 as CandidateIndex).into(), + )]); - virtual_overseer - }); + send_message_from_peer_v3(overseer, &peer_d, msg).await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + _, _, + tranche, _, + )) => { + assert_eq!(tranche, 0); + } + ); + expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE_FIRST).await; + + // send an approval from peer_b + let approval = IndirectSignedApprovalVoteV2 { + block_hash: hash, + candidate_indices, + validator: validator_index, + signature: signature_for( + &keystore, + &session, + vec![candidate_hash_first, candidate_hash_second], + validator_index, + ), + }; + let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); + send_message_from_peer_v3(overseer, &peer_d, msg).await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( + vote, _, + )) => { + assert_eq!(vote, approval); + } + ); + + expect_reputation_change(overseer, &peer_d, BENEFIT_VALID_MESSAGE_FIRST).await; + + // setup peers with V2 protocol versions + setup_peer_with_view(overseer, &peer_a, view![hash], ValidationVersion::V3).await; + setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V3).await; + let mut expected_peers_assignments = vec![peer_a, peer_b]; + let mut expected_peers_approvals = vec![peer_a, peer_b]; + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert!(peers.len() == 1); + assert!(expected_peers_assignments.contains(peers.first().unwrap())); + expected_peers_assignments.retain(|peer| peer != peers.first().unwrap()); + assert_eq!(assignments.len(), 2); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Approvals(approvals) + )) + )) => { + assert!(peers.len() == 1); + assert!(expected_peers_approvals.contains(peers.first().unwrap())); + expected_peers_approvals.retain(|peer| peer != peers.first().unwrap()); + assert_eq!(approvals.len(), 1); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert!(peers.len() == 1); + assert!(expected_peers_assignments.contains(peers.first().unwrap())); + expected_peers_assignments.retain(|peer| peer != peers.first().unwrap()); + assert_eq!(assignments.len(), 2); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Approvals(approvals) + )) + )) => { + assert!(peers.len() == 1); + assert!(expected_peers_approvals.contains(peers.first().unwrap())); + expected_peers_approvals.retain(|peer| peer != peers.first().unwrap()); + assert_eq!(approvals.len(), 1); + } + ); + + virtual_overseer + }, + ); } #[test] @@ -1437,79 +1697,88 @@ fn import_approval_bad() { let peer_b = PeerId::random(); let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); + let candidate_hash = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xBB)); + + let diff_candidate_hash = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xCC)); + + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // setup peers + setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V1).await; + setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V1).await; + let mut keystore = LocalKeystore::in_memory(); + let session = dummy_session_info_valid(1, &mut keystore, 1); + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![(candidate_hash, 0.into(), 0.into()); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // setup peers - setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V1).await; - setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V1).await; - - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - let cert = fake_assignment_cert(hash, validator_index); - - // send the an approval from peer_b, we don't have an assignment yet - let approval = IndirectSignedApprovalVoteV2 { - block_hash: hash, - candidate_indices: candidate_index.into(), - validator: validator_index, - signature: dummy_signature(), - }; - let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); - send_message_from_peer_v3(overseer, &peer_b, msg).await; - - expect_reputation_change(overseer, &peer_b, COST_UNEXPECTED_MESSAGE).await; - - // now import an assignment from peer_b - let assignments = vec![(cert.clone(), candidate_index)]; - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments); - send_message_from_peer(overseer, &peer_b, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - assignment, - i, - tx, - )) => { - assert_eq!(assignment, cert.into()); - assert_eq!(i, candidate_index.into()); - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; + let cert = fake_assignment_cert(hash, validator_index); - expect_reputation_change(overseer, &peer_b, BENEFIT_VALID_MESSAGE_FIRST).await; + // Sign a different candidate hash. + let payload = + ApprovalVoteMultipleCandidates(&vec![diff_candidate_hash]).signing_payload(1); + let sign_key = session.validators.get(ValidatorIndex(0)).unwrap().clone(); + let signature = keystore + .sr25519_sign(ValidatorId::ID, &sign_key.into(), &payload[..]) + .unwrap() + .unwrap(); + + // send the an approval from peer_b, we don't have an assignment yet + let approval = IndirectSignedApprovalVoteV2 { + block_hash: hash, + candidate_indices: candidate_index.into(), + validator: validator_index, + signature: signature.into(), + }; + let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); + send_message_from_peer_v3(overseer, &peer_b, msg).await; - // and try again - let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); - send_message_from_peer_v3(overseer, &peer_b, msg).await; + expect_reputation_change(overseer, &peer_b, COST_UNEXPECTED_MESSAGE).await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( - vote, - tx, - )) => { - assert_eq!(vote, approval); - tx.send(ApprovalCheckResult::Bad(ApprovalCheckError::UnknownBlock(hash))).unwrap(); - } - ); + // now import an assignment from peer_b + let assignments = vec![(cert.clone(), candidate_index)]; + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments); + send_message_from_peer(overseer, &peer_b, msg).await; + provide_session(overseer, session.clone()).await; - expect_reputation_change(overseer, &peer_b, COST_INVALID_MESSAGE).await; - virtual_overseer - }); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + assignment, + i, + tranche, _, + )) => { + assert_eq!(assignment, cert.into()); + assert_eq!(i, candidate_index.into()); + assert_eq!(tranche, 0); + } + ); + + expect_reputation_change(overseer, &peer_b, BENEFIT_VALID_MESSAGE_FIRST).await; + + // and try again + let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); + send_message_from_peer_v3(overseer, &peer_b, msg).await; + + expect_reputation_change(overseer, &peer_b, COST_INVALID_MESSAGE).await; + virtual_overseer + }, + ); } /// make sure we clean up the state on block finalized @@ -1520,38 +1789,46 @@ fn update_our_view() { let hash_b = Hash::repeat_byte(0xBB); let hash_c = Hash::repeat_byte(0xCC); - let state = test_harness(State::default(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // new block `hash_a` with 1 candidates - let meta_a = BlockApprovalMeta { - hash: hash_a, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let meta_b = BlockApprovalMeta { - hash: hash_b, - parent_hash: hash_a, - number: 2, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let meta_c = BlockApprovalMeta { - hash: hash_c, - parent_hash: hash_b, - number: 3, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta_a, meta_b, meta_c]); - overseer_send(overseer, msg).await; - virtual_overseer - }); + let state = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + State::default(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // new block `hash_a` with 1 candidates + let meta_a = BlockApprovalMeta { + hash: hash_a, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let meta_b = BlockApprovalMeta { + hash: hash_b, + parent_hash: hash_a, + number: 2, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let meta_c = BlockApprovalMeta { + hash: hash_c, + parent_hash: hash_b, + number: 3, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta_a, meta_b, meta_c]); + overseer_send(overseer, msg).await; + virtual_overseer + }, + ); assert!(state.blocks_by_number.get(&1).is_some()); assert!(state.blocks_by_number.get(&2).is_some()); @@ -1560,12 +1837,17 @@ fn update_our_view() { assert!(state.blocks.get(&hash_b).is_some()); assert!(state.blocks.get(&hash_c).is_some()); - let state = test_harness(state, |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // finalize a block - overseer_signal_block_finalized(overseer, 2).await; - virtual_overseer - }); + let state = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state, + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // finalize a block + overseer_signal_block_finalized(overseer, 2).await; + virtual_overseer + }, + ); assert!(state.blocks_by_number.get(&1).is_none()); assert!(state.blocks_by_number.get(&2).is_none()); @@ -1574,12 +1856,17 @@ fn update_our_view() { assert!(state.blocks.get(&hash_b).is_none()); assert!(state.blocks.get(&hash_c).is_some()); - let state = test_harness(state, |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // finalize a very high block - overseer_signal_block_finalized(overseer, 4_000_000_000).await; - virtual_overseer - }); + let state = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state, + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // finalize a very high block + overseer_signal_block_finalized(overseer, 4_000_000_000).await; + virtual_overseer + }, + ); assert!(state.blocks_by_number.get(&3).is_none()); assert!(state.blocks.get(&hash_c).is_none()); @@ -1597,81 +1884,89 @@ fn update_peer_view() { let peer_a = peers.first().unwrap().0; let peer = &peer_a; - let state = test_harness(State::default(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // new block `hash_a` with 1 candidates - let meta_a = BlockApprovalMeta { - hash: hash_a, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let meta_b = BlockApprovalMeta { - hash: hash_b, - parent_hash: hash_a, - number: 2, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let meta_c = BlockApprovalMeta { - hash: hash_c, - parent_hash: hash_b, - number: 3, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta_a, meta_b, meta_c]); - overseer_send(overseer, msg).await; - - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Setup a topology where peer_a is neighbor to current node. - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0], &[2], 1), - ) - .await; + let state = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + State::default(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // new block `hash_a` with 1 candidates + let meta_a = BlockApprovalMeta { + hash: hash_a, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let meta_b = BlockApprovalMeta { + hash: hash_b, + parent_hash: hash_a, + number: 2, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let meta_c = BlockApprovalMeta { + hash: hash_c, + parent_hash: hash_b, + number: 3, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta_a, meta_b, meta_c]); + overseer_send(overseer, msg).await; + + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Setup a topology where peer_a is neighbor to current node. + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0], &[2], 1), + ) + .await; - let cert_a = fake_assignment_cert(hash_a, ValidatorIndex(0)); - let cert_b = fake_assignment_cert(hash_b, ValidatorIndex(0)); + let cert_a = fake_assignment_cert(hash_a, ValidatorIndex(0)); + let cert_b = fake_assignment_cert(hash_b, ValidatorIndex(0)); - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment(cert_a.into(), 0.into()), - ) - .await; + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment(cert_a.into(), 0.into()), + ) + .await; - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment(cert_b.into(), 0.into()), - ) - .await; + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment(cert_b.into(), 0.into()), + ) + .await; - // connect a peer - setup_peer_with_view(overseer, peer, view![hash_a], ValidationVersion::V1).await; - - // we should send relevant assignments to the peer - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 1); - assert_eq!(assignments.len(), 1); - } - ); - virtual_overseer - }); + // connect a peer + setup_peer_with_view(overseer, peer, view![hash_a], ValidationVersion::V1).await; + + // we should send relevant assignments to the peer + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 1); + assert_eq!(assignments.len(), 1); + } + ); + virtual_overseer + }, + ); assert_eq!(state.peer_views.get(peer).map(|v| v.view.finalized_number), Some(0)); assert_eq!( @@ -1688,42 +1983,49 @@ fn update_peer_view() { 1, ); - let state = test_harness(state, |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // update peer's view - overseer_send( - overseer, - ApprovalDistributionMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerViewChange( - *peer, - View::new(vec![hash_b, hash_c, hash_d], 2), - )), - ) - .await; + let state = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state, + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // update peer's view + overseer_send( + overseer, + ApprovalDistributionMessage::NetworkBridgeUpdate( + NetworkBridgeEvent::PeerViewChange( + *peer, + View::new(vec![hash_b, hash_c, hash_d], 2), + ), + ), + ) + .await; - let cert_c = fake_assignment_cert(hash_c, ValidatorIndex(0)); + let cert_c = fake_assignment_cert(hash_c, ValidatorIndex(0)); - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment(cert_c.clone().into(), 0.into()), - ) - .await; + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment(cert_c.clone().into(), 0.into()), + ) + .await; - // we should send relevant assignments to the peer - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 1); - assert_eq!(assignments.len(), 1); - assert_eq!(assignments[0].0, cert_c); - } - ); - virtual_overseer - }); + // we should send relevant assignments to the peer + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 1); + assert_eq!(assignments.len(), 1); + assert_eq!(assignments[0].0, cert_c); + } + ); + virtual_overseer + }, + ); assert_eq!(state.peer_views.get(peer).map(|v| v.view.finalized_number), Some(2)); assert_eq!( @@ -1741,19 +2043,26 @@ fn update_peer_view() { ); let finalized_number = 4_000_000_000; - let state = test_harness(state, |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // update peer's view - overseer_send( - overseer, - ApprovalDistributionMessage::NetworkBridgeUpdate(NetworkBridgeEvent::PeerViewChange( - *peer, - View::with_finalized(finalized_number), - )), - ) - .await; - virtual_overseer - }); + let state = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state, + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // update peer's view + overseer_send( + overseer, + ApprovalDistributionMessage::NetworkBridgeUpdate( + NetworkBridgeEvent::PeerViewChange( + *peer, + View::with_finalized(finalized_number), + ), + ), + ) + .await; + virtual_overseer + }, + ); assert_eq!(state.peer_views.get(peer).map(|v| v.view.finalized_number), Some(finalized_number)); assert!(state.blocks.get(&hash_c).unwrap().known_by.get(peer).is_none()); @@ -1776,164 +2085,176 @@ fn update_peer_authority_id() { // Y neighbour, we simulate that PeerId is not known in the beginning. let neighbour_y = peers.get(neighbour_y_index).unwrap().0; - let _state = test_harness(State::default(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // new block `hash_a` with 1 candidates - let meta_a = BlockApprovalMeta { - hash: hash_a, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let meta_b = BlockApprovalMeta { - hash: hash_b, - parent_hash: hash_a, - number: 2, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let meta_c = BlockApprovalMeta { - hash: hash_c, - parent_hash: hash_b, - number: 3, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta_a, meta_b, meta_c]); - overseer_send(overseer, msg).await; - - let peers_with_optional_peer_id = peers - .iter() - .enumerate() - .map(|(index, (peer_id, authority))| { - (if index == 0 { None } else { Some(*peer_id) }, authority.clone()) - }) - .collect_vec(); - - // Setup a topology where peer_a is neighbor to current node. - setup_gossip_topology( - overseer, - make_gossip_topology( - 1, - &peers_with_optional_peer_id, - &[neighbour_x_index], - &[neighbour_y_index], - local_index, - ), - ) - .await; + let _state = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + State::default(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // new block `hash_a` with 1 candidates + let meta_a = BlockApprovalMeta { + hash: hash_a, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let meta_b = BlockApprovalMeta { + hash: hash_b, + parent_hash: hash_a, + number: 2, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let meta_c = BlockApprovalMeta { + hash: hash_c, + parent_hash: hash_b, + number: 3, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta_a, meta_b, meta_c]); + overseer_send(overseer, msg).await; + + let peers_with_optional_peer_id = peers + .iter() + .enumerate() + .map(|(index, (peer_id, authority))| { + (if index == 0 { None } else { Some(*peer_id) }, authority.clone()) + }) + .collect_vec(); + + // Setup a topology where peer_a is neighbor to current node. + setup_gossip_topology( + overseer, + make_gossip_topology( + 1, + &peers_with_optional_peer_id, + &[neighbour_x_index], + &[neighbour_y_index], + local_index, + ), + ) + .await; - let cert_a = fake_assignment_cert(hash_a, ValidatorIndex(local_index as u32)); - let cert_b = fake_assignment_cert(hash_b, ValidatorIndex(local_index as u32)); + let cert_a = fake_assignment_cert(hash_a, ValidatorIndex(local_index as u32)); + let cert_b = fake_assignment_cert(hash_b, ValidatorIndex(local_index as u32)); - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment(cert_a.into(), 0.into()), - ) - .await; + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment(cert_a.into(), 0.into()), + ) + .await; - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment(cert_b.into(), 0.into()), - ) - .await; + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment(cert_b.into(), 0.into()), + ) + .await; - // connect a peer - setup_peer_with_view(overseer, &neighbour_x, view![hash_a], ValidationVersion::V1).await; - setup_peer_with_view(overseer, &neighbour_y, view![hash_a], ValidationVersion::V1).await; - - setup_peer_with_view(overseer, &neighbour_x, view![hash_b], ValidationVersion::V1).await; - setup_peer_with_view(overseer, &neighbour_y, view![hash_b], ValidationVersion::V1).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 1); - assert_eq!(assignments.len(), 1); - assert_eq!(peers.get(0), Some(&neighbour_y)); - } - ); + // connect a peer + setup_peer_with_view(overseer, &neighbour_x, view![hash_a], ValidationVersion::V1) + .await; + setup_peer_with_view(overseer, &neighbour_y, view![hash_a], ValidationVersion::V1) + .await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 1); - assert_eq!(assignments.len(), 1); - assert_eq!(peers.get(0), Some(&neighbour_y)); - } - ); + setup_peer_with_view(overseer, &neighbour_x, view![hash_b], ValidationVersion::V1) + .await; + setup_peer_with_view(overseer, &neighbour_y, view![hash_b], ValidationVersion::V1) + .await; - overseer_send( - overseer, - ApprovalDistributionMessage::NetworkBridgeUpdate( - NetworkBridgeEvent::UpdatedAuthorityIds( - peers[neighbour_x_index].0, - [peers[neighbour_x_index].1.clone()].into_iter().collect(), + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 1); + assert_eq!(assignments.len(), 1); + assert_eq!(peers.get(0), Some(&neighbour_y)); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 1); + assert_eq!(assignments.len(), 1); + assert_eq!(peers.get(0), Some(&neighbour_y)); + } + ); + + overseer_send( + overseer, + ApprovalDistributionMessage::NetworkBridgeUpdate( + NetworkBridgeEvent::UpdatedAuthorityIds( + peers[neighbour_x_index].0, + [peers[neighbour_x_index].1.clone()].into_iter().collect(), + ), ), - ), - ) - .await; + ) + .await; - // we should send relevant assignments to the peer, after we found it's peer id. - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - gum::info!(target: LOG_TARGET, ?peers, ?assignments); - assert_eq!(peers.len(), 1); - assert_eq!(assignments.len(), 2); - assert_eq!(assignments.get(0).unwrap().0.block_hash, hash_a); - assert_eq!(assignments.get(1).unwrap().0.block_hash, hash_b); - assert_eq!(peers.get(0), Some(&neighbour_x)); - } - ); + // we should send relevant assignments to the peer, after we found it's peer id. + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + gum::info!(target: LOG_TARGET, ?peers, ?assignments); + assert_eq!(peers.len(), 1); + assert_eq!(assignments.len(), 2); + assert_eq!(assignments.get(0).unwrap().0.block_hash, hash_a); + assert_eq!(assignments.get(1).unwrap().0.block_hash, hash_b); + assert_eq!(peers.get(0), Some(&neighbour_x)); + } + ); - overseer_send( - overseer, - ApprovalDistributionMessage::NetworkBridgeUpdate( - NetworkBridgeEvent::UpdatedAuthorityIds( - peers[neighbour_y_index].0, - [peers[neighbour_y_index].1.clone()].into_iter().collect(), + overseer_send( + overseer, + ApprovalDistributionMessage::NetworkBridgeUpdate( + NetworkBridgeEvent::UpdatedAuthorityIds( + peers[neighbour_y_index].0, + [peers[neighbour_y_index].1.clone()].into_iter().collect(), + ), ), - ), - ) - .await; - overseer_send( - overseer, - ApprovalDistributionMessage::NetworkBridgeUpdate( - NetworkBridgeEvent::UpdatedAuthorityIds( - peers[neighbour_x_index].0, - [peers[neighbour_x_index].1.clone()].into_iter().collect(), + ) + .await; + overseer_send( + overseer, + ApprovalDistributionMessage::NetworkBridgeUpdate( + NetworkBridgeEvent::UpdatedAuthorityIds( + peers[neighbour_x_index].0, + [peers[neighbour_x_index].1.clone()].into_iter().collect(), + ), ), - ), - ) - .await; - assert!( - overseer.recv().timeout(TIMEOUT).await.is_none(), - "no message should be sent peers are already known" - ); + ) + .await; + assert!( + overseer.recv().timeout(TIMEOUT).await.is_none(), + "no message should be sent peers are already known" + ); - virtual_overseer - }); + virtual_overseer + }, + ); } /// E.g. if someone copies the keys... @@ -1942,89 +2263,106 @@ fn import_remotely_then_locally() { let peer_a = PeerId::random(); let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); + let candidate_hash = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xBB)); let peer = &peer_a; - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // setup the peer - setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; - - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - // import the assignment remotely first - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - let cert = fake_assignment_cert(hash, validator_index); - let assignments = vec![(cert.clone(), candidate_index)]; - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); - send_message_from_peer(overseer, peer, msg).await; - - // send an `Accept` message from the Approval Voting subsystem - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - assignment, - i, - tx, - )) => { - assert_eq!(assignment, cert.clone().into()); - assert_eq!(i, candidate_index.into()); - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // setup the peer + setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; + let mut keystore = LocalKeystore::in_memory(); + + let session = dummy_session_info_valid(1, &mut keystore, 1); + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![(candidate_hash, 0.into(), 0.into()); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let payload = ApprovalVoteMultipleCandidates(&vec![candidate_hash]).signing_payload(1); + let sign_key = session.validators.get(ValidatorIndex(0)).unwrap().clone(); + let signature = keystore + .sr25519_sign(ValidatorId::ID, &sign_key.into(), &payload[..]) + .unwrap() + .unwrap(); + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + // import the assignment remotely first + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; + let cert = fake_assignment_cert(hash, validator_index); + let assignments = vec![(cert.clone(), candidate_index)]; + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + send_message_from_peer(overseer, peer, msg).await; + provide_session(overseer, session.clone()).await; + + // send an `Accept` message from the Approval Voting subsystem + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + assignment, + i, + tranche, _, + )) => { + assert_eq!(assignment, cert.clone().into()); + assert_eq!(i, candidate_index.into()); + assert_eq!(tranche, 0); + } + ); - expect_reputation_change(overseer, peer, BENEFIT_VALID_MESSAGE_FIRST).await; + expect_reputation_change(overseer, peer, BENEFIT_VALID_MESSAGE_FIRST).await; - // import the same assignment locally - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment( - cert.clone().into(), - candidate_index.into(), - ), - ) - .await; + // import the same assignment locally + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment( + cert.clone().into(), + candidate_index.into(), + ), + ) + .await; - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - - // send the approval remotely - let approval = IndirectSignedApprovalVoteV2 { - block_hash: hash, - candidate_indices: candidate_index.into(), - validator: validator_index, - signature: dummy_signature(), - }; - let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); - send_message_from_peer_v3(overseer, peer, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( - vote, - tx, - )) => { - assert_eq!(vote, approval); - tx.send(ApprovalCheckResult::Accepted).unwrap(); - } - ); - expect_reputation_change(overseer, peer, BENEFIT_VALID_MESSAGE_FIRST).await; + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - // import the same approval locally - overseer_send(overseer, ApprovalDistributionMessage::DistributeApproval(approval)).await; + // send the approval remotely + let approval = IndirectSignedApprovalVoteV2 { + block_hash: hash, + candidate_indices: candidate_index.into(), + validator: validator_index, + signature: signature.into(), + }; + let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); + send_message_from_peer_v3(overseer, peer, msg).await; - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( + vote, _, + )) => { + assert_eq!(vote, approval); + } + ); + expect_reputation_change(overseer, peer, BENEFIT_VALID_MESSAGE_FIRST).await; + + // import the same approval locally + overseer_send(overseer, ApprovalDistributionMessage::DistributeApproval(approval)) + .await; + + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } #[test] @@ -2035,94 +2373,100 @@ fn sends_assignments_even_when_state_is_approved() { let hash = Hash::repeat_byte(0xAA); let peer = &peer_a; - let _ = test_harness(State::default(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + State::default(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Setup a topology where peer_a is neighbor to current node. + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0], &[2], 1), + ) + .await; - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Setup a topology where peer_a is neighbor to current node. - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0], &[2], 1), - ) - .await; + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - - // import an assignment and approval locally. - let cert = fake_assignment_cert(hash, validator_index); - let approval = IndirectSignedApprovalVote { - block_hash: hash, - candidate_index, - validator: validator_index, - signature: dummy_signature(), - }; - - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment( - cert.clone().into(), - candidate_index.into(), - ), - ) - .await; + // import an assignment and approval locally. + let cert = fake_assignment_cert(hash, validator_index); + let approval = IndirectSignedApprovalVote { + block_hash: hash, + candidate_index, + validator: validator_index, + signature: dummy_signature(), + }; - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeApproval(approval.clone().into()), - ) - .await; + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment( + cert.clone().into(), + candidate_index.into(), + ), + ) + .await; - // connect the peer. - setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; - - let assignments = vec![(cert.clone(), candidate_index)]; - let approvals = vec![approval.clone()]; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) - )) - )) => { - assert_eq!(peers, vec![*peer]); - assert_eq!(sent_assignments, assignments); - } - ); + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeApproval(approval.clone().into()), + ) + .await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) - )) - )) => { - assert_eq!(peers, vec![*peer]); - assert_eq!(sent_approvals, approvals); - } - ); + // connect the peer. + setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + let assignments = vec![(cert.clone(), candidate_index)]; + let approvals = vec![approval.clone()]; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + )) + )) => { + assert_eq!(peers, vec![*peer]); + assert_eq!(sent_assignments, assignments); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) + )) + )) => { + assert_eq!(peers, vec![*peer]); + assert_eq!(sent_approvals, approvals); + } + ); + + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } /// Same as `sends_assignments_even_when_state_is_approved_v2` but with `VRFModuloCompact` @@ -2135,112 +2479,118 @@ fn sends_assignments_even_when_state_is_approved_v2() { let hash = Hash::repeat_byte(0xAA); let peer = &peer_a; - let _ = test_harness(State::default(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 4], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Setup a topology where peer_a is neighbor to current node. - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0], &[2], 1), - ) - .await; - - let validator_index = ValidatorIndex(0); - let cores = vec![0, 1, 2, 3]; - let candidate_bitfield: CandidateBitfield = cores.clone().try_into().unwrap(); - - let core_bitfield: CoreBitfield = cores - .iter() - .map(|index| CoreIndex(*index)) - .collect::>() - .try_into() - .unwrap(); - - let cert = fake_assignment_cert_v2(hash, validator_index, core_bitfield.clone()); + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + State::default(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 4], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Setup a topology where peer_a is neighbor to current node. + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0], &[2], 1), + ) + .await; - // Assumes candidate index == core index. - let approvals = cores - .iter() - .map(|core| IndirectSignedApprovalVoteV2 { - block_hash: hash, - candidate_indices: (*core).into(), - validator: validator_index, - signature: dummy_signature(), - }) - .collect::>(); - - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment( - cert.clone().into(), - candidate_bitfield.clone(), - ), - ) - .await; + let validator_index = ValidatorIndex(0); + let cores = vec![0, 1, 2, 3]; + let candidate_bitfield: CandidateBitfield = cores.clone().try_into().unwrap(); + + let core_bitfield: CoreBitfield = cores + .iter() + .map(|index| CoreIndex(*index)) + .collect::>() + .try_into() + .unwrap(); + + let cert = fake_assignment_cert_v2(hash, validator_index, core_bitfield.clone()); + + // Assumes candidate index == core index. + let approvals = cores + .iter() + .map(|core| IndirectSignedApprovalVoteV2 { + block_hash: hash, + candidate_indices: (*core).into(), + validator: validator_index, + signature: dummy_signature(), + }) + .collect::>(); - for approval in &approvals { overseer_send( overseer, - ApprovalDistributionMessage::DistributeApproval(approval.clone()), + ApprovalDistributionMessage::DistributeAssignment( + cert.clone().into(), + candidate_bitfield.clone(), + ), ) .await; - } - // connect the peer. - setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V3).await; - - let assignments = vec![(cert.clone(), candidate_bitfield.clone())]; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Assignments(sent_assignments) - )) - )) => { - assert_eq!(peers, vec![*peer]); - assert_eq!(sent_assignments, assignments); + for approval in &approvals { + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeApproval(approval.clone()), + ) + .await; } - ); - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( - protocol_v3::ApprovalDistributionMessage::Approvals(sent_approvals) - )) - )) => { - // Construct a hashmaps of approvals for comparison. Approval distribution reorders messages because they are kept in a - // hashmap as well. - let sent_approvals = sent_approvals.into_iter().map(|approval| (approval.candidate_indices.clone(), approval)).collect::>(); - let approvals = approvals.into_iter().map(|approval| (approval.candidate_indices.clone(), approval)).collect::>(); - - assert_eq!(peers, vec![*peer]); - assert_eq!(sent_approvals, approvals); - } - ); + // connect the peer. + setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V3).await; - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + let assignments = vec![(cert.clone(), candidate_bitfield.clone())]; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Assignments(sent_assignments) + )) + )) => { + assert_eq!(peers, vec![*peer]); + assert_eq!(sent_assignments, assignments); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V3(protocol_v3::ValidationProtocol::ApprovalDistribution( + protocol_v3::ApprovalDistributionMessage::Approvals(sent_approvals) + )) + )) => { + // Construct a hashmaps of approvals for comparison. Approval distribution reorders messages because they are kept in a + // hashmap as well. + let sent_approvals = sent_approvals.into_iter().map(|approval| (approval.candidate_indices.clone(), approval)).collect::>(); + let approvals = approvals.into_iter().map(|approval| (approval.candidate_indices.clone(), approval)).collect::>(); + + assert_eq!(peers, vec![*peer]); + assert_eq!(sent_approvals, approvals); + } + ); + + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } /// @@ -2255,71 +2605,83 @@ fn race_condition_in_local_vs_remote_view_update() { let peer_a = PeerId::random(); let hash_b = Hash::repeat_byte(0xBB); - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - let peer = &peer_a; - - // Test a small number of candidates - let candidates_count = 1; - let meta = BlockApprovalMeta { - hash: hash_b, - parent_hash, - number: 2, - candidates: vec![Default::default(); candidates_count], - slot: 1.into(), - session: 1, - }; - - // This will send a peer view that is ahead of our view - setup_peer_with_view(overseer, peer, view![hash_b], ValidationVersion::V1).await; - - // Send our view update to include a new head - overseer_send( - overseer, - ApprovalDistributionMessage::NetworkBridgeUpdate(NetworkBridgeEvent::OurViewChange( - our_view![hash_b], - )), - ) - .await; + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + let peer = &peer_a; + + // Test a small number of candidates + let candidates_count = 1; + let meta = BlockApprovalMeta { + hash: hash_b, + parent_hash, + number: 2, + candidates: vec![Default::default(); candidates_count], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + // This will send a peer view that is ahead of our view + setup_peer_with_view(overseer, peer, view![hash_b], ValidationVersion::V1).await; + + // Send our view update to include a new head + overseer_send( + overseer, + ApprovalDistributionMessage::NetworkBridgeUpdate( + NetworkBridgeEvent::OurViewChange(our_view![hash_b]), + ), + ) + .await; - // send assignments related to `hash_b` but they will come to the MessagesPending - let assignments: Vec<_> = (0..candidates_count) - .map(|candidate_index| { - let validator_index = ValidatorIndex(candidate_index as u32); - let cert = fake_assignment_cert(hash_b, validator_index); - (cert, candidate_index as u32) - }) - .collect(); + // send assignments related to `hash_b` but they will come to the MessagesPending + let assignments: Vec<_> = (0..candidates_count) + .map(|candidate_index| { + let validator_index = ValidatorIndex(candidate_index as u32); + let cert = fake_assignment_cert(hash_b, validator_index); + (cert, candidate_index as u32) + }) + .collect(); - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); - send_message_from_peer(overseer, peer, msg.clone()).await; + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + send_message_from_peer(overseer, peer, msg.clone()).await; - // This will handle pending messages being processed - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; + // This will handle pending messages being processed + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; - for i in 0..candidates_count { - // Previously, this has caused out-of-view assignments/approvals - //expect_reputation_change(overseer, peer, COST_UNEXPECTED_MESSAGE).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - assignment, - claimed_candidate_index, - tx, - )) => { - assert_eq!(assignment, assignments[i].0.clone().into()); - assert_eq!(claimed_candidate_index, assignments[i].1.into()); - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); + for i in 0..candidates_count { + // Previously, this has caused out-of-view assignments/approvals + //expect_reputation_change(overseer, peer, COST_UNEXPECTED_MESSAGE).await; - // Since we have a valid statement pending, this should always occur - expect_reputation_change(overseer, peer, BENEFIT_VALID_MESSAGE_FIRST).await; - } - virtual_overseer - }); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + assignment, + claimed_candidate_index, + tranche, _, + )) => { + assert_eq!(assignment, assignments[i].0.clone().into()); + assert_eq!(claimed_candidate_index, assignments[i].1.into()); + assert_eq!(tranche, 0); + } + ); + + // Since we have a valid statement pending, this should always occur + expect_reputation_change(overseer, peer, BENEFIT_VALID_MESSAGE_FIRST).await; + } + virtual_overseer + }, + ); } // Tests that local messages propagate to both dimensions. @@ -2330,198 +2692,86 @@ fn propagates_locally_generated_assignment_to_both_dimensions() { let peers = make_peers_and_authority_ids(100); - let _ = test_harness(State::default(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - - // Connect all peers. - for (peer, _) in &peers { - setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; - } - - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - - // Set up a gossip topology. - setup_gossip_topology( - overseer, - make_gossip_topology( - 1, - &peers_with_optional_peer_id, - &[0, 10, 20, 30, 40, 60, 70, 80], - &[50, 51, 52, 53, 54, 55, 56, 57], - 1, - ), - ) - .await; - - let expected_indices = [ - // Both dimensions in the gossip topology - 0, 10, 20, 30, 40, 60, 70, 80, 50, 51, 52, 53, 54, 55, 56, 57, - ]; - - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - - // import an assignment and approval locally. - let cert = fake_assignment_cert(hash, validator_index); - let approval = IndirectSignedApprovalVote { - block_hash: hash, - candidate_index, - validator: validator_index, - signature: dummy_signature(), - }; - - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment( - cert.clone().into(), - candidate_index.into(), - ), - ) - .await; - - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeApproval(approval.clone().into()), - ) - .await; - - let assignments = vec![(cert.clone(), candidate_index)]; - let approvals = vec![approval.clone()]; - - let mut assignment_sent_peers = assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - sent_peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) - )) - )) => { - assert_eq!(sent_peers.len(), expected_indices.len() + 4); - for &i in &expected_indices { - assert!( - sent_peers.contains(&peers[i].0), - "Message not sent to expected peer {}", - i, - ); - } - assert_eq!(sent_assignments, assignments); - sent_peers - } - ); + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + State::default(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - mut sent_peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) - )) - )) => { - // Random sampling is reused from the assignment. - sent_peers.sort(); - assignment_sent_peers.sort(); - assert_eq!(sent_peers, assignment_sent_peers); - assert_eq!(sent_approvals, approvals); + // Connect all peers. + for (peer, _) in &peers { + setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; } - ); - - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); -} - -// Tests that messages propagate to the unshared dimension. -#[test] -fn propagates_assignments_along_unshared_dimension() { - let parent_hash = Hash::repeat_byte(0xFF); - let hash = Hash::repeat_byte(0xAA); - - let peers = make_peers_and_authority_ids(100); - let _ = test_harness(state_without_reputation_delay(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); - // Connect all peers. - for (peer, _) in &peers { - setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; - } + // Set up a gossip topology. + setup_gossip_topology( + overseer, + make_gossip_topology( + 1, + &peers_with_optional_peer_id, + &[0, 10, 20, 30, 40, 60, 70, 80], + &[50, 51, 52, 53, 54, 55, 56, 57], + 1, + ), + ) + .await; - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - - // Set up a gossip topology. - setup_gossip_topology( - overseer, - make_gossip_topology( - 1, - &peers_with_optional_peer_id, - &[0, 10, 20, 30], - &[50, 51, 52, 53], - 1, - ), - ) - .await; + let expected_indices = [ + // Both dimensions in the gossip topology + 0, 10, 20, 30, 40, 60, 70, 80, 50, 51, 52, 53, 54, 55, 56, 57, + ]; + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - // Test messages from X direction go to Y peers - { let validator_index = ValidatorIndex(0); let candidate_index = 0u32; // import an assignment and approval locally. let cert = fake_assignment_cert(hash, validator_index); - let assignments = vec![(cert.clone(), candidate_index)]; + let approval = IndirectSignedApprovalVote { + block_hash: hash, + candidate_index, + validator: validator_index, + signature: dummy_signature(), + }; - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment( + cert.clone().into(), + candidate_index.into(), + ), + ) + .await; - // Issuer of the message is important, not the peer we receive from. - // 99 deliberately chosen because it's not in X or Y. - send_message_from_peer(overseer, &peers[99].0, msg).await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - _, - _, - tx, - )) => { - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); - expect_reputation_change(overseer, &peers[99].0, BENEFIT_VALID_MESSAGE_FIRST).await; + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeApproval(approval.clone().into()), + ) + .await; - let expected_y = [50, 51, 52, 53]; + let assignments = vec![(cert.clone(), candidate_index)]; + let approvals = vec![approval.clone()]; - assert_matches!( + let mut assignment_sent_peers = assert_matches!( overseer_recv(overseer).await, AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( sent_peers, @@ -2529,8 +2779,8 @@ fn propagates_assignments_along_unshared_dimension() { protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) )) )) => { - assert_eq!(sent_peers.len(), expected_y.len() + 4); - for &i in &expected_y { + assert_eq!(sent_peers.len(), expected_indices.len() + 4); + for &i in &expected_indices { assert!( sent_peers.contains(&peers[i].0), "Message not sent to expected peer {}", @@ -2538,189 +2788,285 @@ fn propagates_assignments_along_unshared_dimension() { ); } assert_eq!(sent_assignments, assignments); + sent_peers } ); - }; - - // Test messages from X direction go to Y peers - { - let validator_index = ValidatorIndex(50); - let candidate_index = 0u32; - - // import an assignment and approval locally. - let cert = fake_assignment_cert(hash, validator_index); - let assignments = vec![(cert.clone(), candidate_index)]; - - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); - - // Issuer of the message is important, not the peer we receive from. - // 99 deliberately chosen because it's not in X or Y. - send_message_from_peer(overseer, &peers[99].0, msg).await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - _, - _, - tx, - )) => { - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); - expect_reputation_change(overseer, &peers[99].0, BENEFIT_VALID_MESSAGE_FIRST).await; - - let expected_x = [0, 10, 20, 30]; assert_matches!( overseer_recv(overseer).await, AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - sent_peers, + mut sent_peers, Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) )) )) => { - assert_eq!(sent_peers.len(), expected_x.len() + 4); - for &i in &expected_x { - assert!( - sent_peers.contains(&peers[i].0), - "Message not sent to expected peer {}", - i, - ); - } - assert_eq!(sent_assignments, assignments); + // Random sampling is reused from the assignment. + sent_peers.sort(); + assignment_sent_peers.sort(); + assert_eq!(sent_peers, assignment_sent_peers); + assert_eq!(sent_approvals, approvals); } ); - }; - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } -// tests that messages are propagated to necessary peers after they connect +// Tests that messages propagate to the unshared dimension. #[test] -fn propagates_to_required_after_connect() { +fn propagates_assignments_along_unshared_dimension() { let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); let peers = make_peers_and_authority_ids(100); - let _ = test_harness(State::default(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - - let omitted = [0, 10, 50, 51]; + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; - // Connect all peers except omitted. - for (i, (peer, _)) in peers.iter().enumerate() { - if !omitted.contains(&i) { + // Connect all peers. + for (peer, _) in &peers { setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; } - } - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Set up a gossip topology. - setup_gossip_topology( - overseer, - make_gossip_topology( - 1, - &peers_with_optional_peer_id, - &[0, 10, 20, 30, 40, 60, 70, 80], - &[50, 51, 52, 53, 54, 55, 56, 57], - 1, - ), - ) - .await; - let expected_indices = [ - // Both dimensions in the gossip topology, minus omitted. - 20, 30, 40, 60, 70, 80, 52, 53, 54, 55, 56, 57, - ]; - - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - - // import an assignment and approval locally. - let cert = fake_assignment_cert(hash, validator_index); - let approval = IndirectSignedApprovalVote { - block_hash: hash, - candidate_index, - validator: validator_index, - signature: dummy_signature(), - }; - - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment( - cert.clone().into(), - candidate_index.into(), - ), - ) - .await; + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeApproval(approval.clone().into()), - ) - .await; + // Set up a gossip topology. + setup_gossip_topology( + overseer, + make_gossip_topology( + 1, + &peers_with_optional_peer_id, + &[0, 10, 20, 30], + &[50, 51, 52, 53], + 1, + ), + ) + .await; - let assignments = vec![(cert.clone(), candidate_index)]; - let approvals = vec![approval.clone()]; - - let mut assignment_sent_peers = assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - sent_peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) - )) - )) => { - assert_eq!(sent_peers.len(), expected_indices.len() + 4); - for &i in &expected_indices { - assert!( - sent_peers.contains(&peers[i].0), - "Message not sent to expected peer {}", - i, - ); + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + // Test messages from X direction go to Y peers + { + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; + + // import an assignment and approval locally. + let cert = fake_assignment_cert(hash, validator_index); + let assignments = vec![(cert.clone(), candidate_index)]; + + let msg = + protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + + // Issuer of the message is important, not the peer we receive from. + // 99 deliberately chosen because it's not in X or Y. + send_message_from_peer(overseer, &peers[99].0, msg).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + _, + _, + tranche, _, + )) => { + assert_eq!(tranche, 0); + } + ); + expect_reputation_change(overseer, &peers[99].0, BENEFIT_VALID_MESSAGE_FIRST).await; + + let expected_y = [50, 51, 52, 53]; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + sent_peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + )) + )) => { + assert_eq!(sent_peers.len(), expected_y.len() + 4); + for &i in &expected_y { + assert!( + sent_peers.contains(&peers[i].0), + "Message not sent to expected peer {}", + i, + ); + } + assert_eq!(sent_assignments, assignments); + } + ); + }; + + // Test messages from X direction go to Y peers + { + let validator_index = ValidatorIndex(50); + let candidate_index = 0u32; + + // import an assignment and approval locally. + let cert = fake_assignment_cert(hash, validator_index); + let assignments = vec![(cert.clone(), candidate_index)]; + + let msg = + protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + + // Issuer of the message is important, not the peer we receive from. + // 99 deliberately chosen because it's not in X or Y. + send_message_from_peer(overseer, &peers[99].0, msg).await; + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + _, + _, + tranche, _, + )) => { + assert_eq!(tranche, 0); + } + ); + expect_reputation_change(overseer, &peers[99].0, BENEFIT_VALID_MESSAGE_FIRST).await; + + let expected_x = [0, 10, 20, 30]; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + sent_peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + )) + )) => { + assert_eq!(sent_peers.len(), expected_x.len() + 4); + for &i in &expected_x { + assert!( + sent_peers.contains(&peers[i].0), + "Message not sent to expected peer {}", + i, + ); + } + assert_eq!(sent_assignments, assignments); + } + ); + }; + + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); +} + +// tests that messages are propagated to necessary peers after they connect +#[test] +fn propagates_to_required_after_connect() { + let parent_hash = Hash::repeat_byte(0xFF); + let hash = Hash::repeat_byte(0xAA); + + let peers = make_peers_and_authority_ids(100); + + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + State::default(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + + let omitted = [0, 10, 50, 51]; + + // Connect all peers except omitted. + for (i, (peer, _)) in peers.iter().enumerate() { + if !omitted.contains(&i) { + setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; } - assert_eq!(sent_assignments, assignments); - sent_peers } - ); + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Set up a gossip topology. + setup_gossip_topology( + overseer, + make_gossip_topology( + 1, + &peers_with_optional_peer_id, + &[0, 10, 20, 30, 40, 60, 70, 80], + &[50, 51, 52, 53, 54, 55, 56, 57], + 1, + ), + ) + .await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - mut sent_peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) - )) - )) => { - // Random sampling is reused from the assignment. - sent_peers.sort(); - assignment_sent_peers.sort(); - assert_eq!(sent_peers, assignment_sent_peers); - assert_eq!(sent_approvals, approvals); - } - ); + let expected_indices = [ + // Both dimensions in the gossip topology, minus omitted. + 20, 30, 40, 60, 70, 80, 52, 53, 54, 55, 56, 57, + ]; + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; - for i in omitted.iter().copied() { - setup_peer_with_view(overseer, &peers[i].0, view![hash], ValidationVersion::V1).await; + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; - assert_matches!( + // import an assignment and approval locally. + let cert = fake_assignment_cert(hash, validator_index); + let approval = IndirectSignedApprovalVote { + block_hash: hash, + candidate_index, + validator: validator_index, + signature: dummy_signature(), + }; + + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment( + cert.clone().into(), + candidate_index.into(), + ), + ) + .await; + + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeApproval(approval.clone().into()), + ) + .await; + + let assignments = vec![(cert.clone(), candidate_index)]; + let approvals = vec![approval.clone()]; + + let mut assignment_sent_peers = assert_matches!( overseer_recv(overseer).await, AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( sent_peers, @@ -2728,30 +3074,72 @@ fn propagates_to_required_after_connect() { protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) )) )) => { - assert_eq!(sent_peers.len(), 1); - assert_eq!(&sent_peers[0], &peers[i].0); + assert_eq!(sent_peers.len(), expected_indices.len() + 4); + for &i in &expected_indices { + assert!( + sent_peers.contains(&peers[i].0), + "Message not sent to expected peer {}", + i, + ); + } assert_eq!(sent_assignments, assignments); + sent_peers } ); assert_matches!( overseer_recv(overseer).await, AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - sent_peers, + mut sent_peers, Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) )) )) => { - assert_eq!(sent_peers.len(), 1); - assert_eq!(&sent_peers[0], &peers[i].0); + // Random sampling is reused from the assignment. + sent_peers.sort(); + assignment_sent_peers.sort(); + assert_eq!(sent_peers, assignment_sent_peers); assert_eq!(sent_approvals, approvals); } ); - } - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + for i in omitted.iter().copied() { + setup_peer_with_view(overseer, &peers[i].0, view![hash], ValidationVersion::V1) + .await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + sent_peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + )) + )) => { + assert_eq!(sent_peers.len(), 1); + assert_eq!(&sent_peers[0], &peers[i].0); + assert_eq!(sent_assignments, assignments); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + sent_peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) + )) + )) => { + assert_eq!(sent_peers.len(), 1); + assert_eq!(&sent_peers[0], &peers[i].0); + assert_eq!(sent_approvals, approvals); + } + ); + } + + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } // test that new gossip topology triggers send of messages. @@ -2762,124 +3150,130 @@ fn sends_to_more_peers_after_getting_topology() { let peers = make_peers_and_authority_ids(100); - let _ = test_harness(State::default(), |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + State::default(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; - // Connect all peers except omitted. - for (peer, _) in &peers { - setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; - } + // Connect all peers except omitted. + for (peer, _) in &peers { + setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; + } - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - - // import an assignment and approval locally. - let cert = fake_assignment_cert(hash, validator_index); - let approval = IndirectSignedApprovalVote { - block_hash: hash, - candidate_index, - validator: validator_index, - signature: dummy_signature(), - }; - - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment( - cert.clone().into(), - candidate_index.into(), - ), - ) - .await; + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeApproval(approval.clone().into()), - ) - .await; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; - let assignments = vec![(cert.clone(), candidate_index)]; - let approvals = vec![approval.clone()]; - - let expected_indices = vec![0, 10, 20, 30, 50, 51, 52, 53]; - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Set up a gossip topology. - setup_gossip_topology( - overseer, - make_gossip_topology( - 1, - &peers_with_optional_peer_id, - &[0, 10, 20, 30], - &[50, 51, 52, 53], - 1, - ), - ) - .await; + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; + + // import an assignment and approval locally. + let cert = fake_assignment_cert(hash, validator_index); + let approval = IndirectSignedApprovalVote { + block_hash: hash, + candidate_index, + validator: validator_index, + signature: dummy_signature(), + }; - let mut expected_indices_assignments = expected_indices.clone(); - let mut expected_indices_approvals = expected_indices.clone(); + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment( + cert.clone().into(), + candidate_index.into(), + ), + ) + .await; - for _ in 0..expected_indices_assignments.len() { - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - sent_peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) - )) - )) => { - // Sends to all expected peers. - assert_eq!(sent_peers.len(), 1); - assert_eq!(sent_assignments, assignments); + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeApproval(approval.clone().into()), + ) + .await; + + let assignments = vec![(cert.clone(), candidate_index)]; + let approvals = vec![approval.clone()]; + + let expected_indices = vec![0, 10, 20, 30, 50, 51, 52, 53]; + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Set up a gossip topology. + setup_gossip_topology( + overseer, + make_gossip_topology( + 1, + &peers_with_optional_peer_id, + &[0, 10, 20, 30], + &[50, 51, 52, 53], + 1, + ), + ) + .await; + + let mut expected_indices_assignments = expected_indices.clone(); + let mut expected_indices_approvals = expected_indices.clone(); + + for _ in 0..expected_indices_assignments.len() { + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + sent_peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + )) + )) => { + // Sends to all expected peers. + assert_eq!(sent_peers.len(), 1); + assert_eq!(sent_assignments, assignments); - let pos = expected_indices_assignments.iter() - .position(|i| &peers[*i].0 == &sent_peers[0]) - .unwrap(); - expected_indices_assignments.remove(pos); - } - ); - } + let pos = expected_indices_assignments.iter() + .position(|i| &peers[*i].0 == &sent_peers[0]) + .unwrap(); + expected_indices_assignments.remove(pos); + } + ); + } - for _ in 0..expected_indices_approvals.len() { - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - sent_peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) - )) - )) => { - // Sends to all expected peers. - assert_eq!(sent_peers.len(), 1); - assert_eq!(sent_approvals, approvals); + for _ in 0..expected_indices_approvals.len() { + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + sent_peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) + )) + )) => { + // Sends to all expected peers. + assert_eq!(sent_peers.len(), 1); + assert_eq!(sent_approvals, approvals); - let pos = expected_indices_approvals.iter() - .position(|i| &peers[*i].0 == &sent_peers[0]) - .unwrap(); + let pos = expected_indices_approvals.iter() + .position(|i| &peers[*i].0 == &sent_peers[0]) + .unwrap(); - expected_indices_approvals.remove(pos); - } - ); - } + expected_indices_approvals.remove(pos); + } + ); + } - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } // test aggression L1 @@ -2894,284 +3288,178 @@ fn originator_aggression_l1() { state.aggression_config.resend_unfinalized_period = None; let aggression_l1_threshold = state.aggression_config.l1_threshold.unwrap(); - let _ = test_harness(state, |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state, + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; - // Connect all peers except omitted. - for (peer, _) in &peers { - setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; - } + // Connect all peers except omitted. + for (peer, _) in &peers { + setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; + } - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - - // import an assignment and approval locally. - let cert = fake_assignment_cert(hash, validator_index); - let approval = IndirectSignedApprovalVote { - block_hash: hash, - candidate_index, - validator: validator_index, - signature: dummy_signature(), - }; - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Set up a gossip topology. - setup_gossip_topology( - overseer, - make_gossip_topology( - 1, - &peers_with_optional_peer_id, - &[0, 10, 20, 30], - &[50, 51, 52, 53], - 1, - ), - ) - .await; + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment( - cert.clone().into(), - candidate_index.into(), - ), - ) - .await; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeApproval(approval.clone().into()), - ) - .await; + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; - let assignments = vec![(cert.clone(), candidate_index)]; - let approvals = vec![approval.clone()]; - - let prev_sent_indices = assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - sent_peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(_) - )) - )) => { - sent_peers.into_iter() - .filter_map(|sp| peers.iter().position(|p| &p.0 == &sp)) - .collect::>() - } - ); + // import an assignment and approval locally. + let cert = fake_assignment_cert(hash, validator_index); + let approval = IndirectSignedApprovalVote { + block_hash: hash, + candidate_index, + validator: validator_index, + signature: dummy_signature(), + }; + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Set up a gossip topology. + setup_gossip_topology( + overseer, + make_gossip_topology( + 1, + &peers_with_optional_peer_id, + &[0, 10, 20, 30], + &[50, 51, 52, 53], + 1, + ), + ) + .await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - _, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Approvals(_) - )) - )) => { } - ); + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment( + cert.clone().into(), + candidate_index.into(), + ), + ) + .await; - // Add blocks until aggression L1 is triggered. - { - let mut parent_hash = hash; - for level in 0..aggression_l1_threshold { - let number = 1 + level + 1; // first block had number 1 - let hash = BlakeTwo256::hash_of(&(parent_hash, number)); - let meta = BlockApprovalMeta { - hash, - parent_hash, - number, - candidates: vec![], - slot: (level as u64).into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::ApprovalCheckingLagUpdate(level + 1); - overseer_send(overseer, msg).await; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - parent_hash = hash; - } - } + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeApproval(approval.clone().into()), + ) + .await; - let unsent_indices = - (0..peers.len()).filter(|i| !prev_sent_indices.contains(&i)).collect::>(); + let assignments = vec![(cert.clone(), candidate_index)]; + let approvals = vec![approval.clone()]; - for _ in 0..unsent_indices.len() { - assert_matches!( + let prev_sent_indices = assert_matches!( overseer_recv(overseer).await, AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( sent_peers, Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + protocol_v1::ApprovalDistributionMessage::Assignments(_) )) )) => { - // Sends to all expected peers. - assert_eq!(sent_peers.len(), 1); - assert_eq!(sent_assignments, assignments); - - assert!(unsent_indices.iter() - .any(|i| &peers[*i].0 == &sent_peers[0])); + sent_peers.into_iter() + .filter_map(|sp| peers.iter().position(|p| &p.0 == &sp)) + .collect::>() } ); - } - for _ in 0..unsent_indices.len() { assert_matches!( overseer_recv(overseer).await, AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - sent_peers, + _, Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) + protocol_v1::ApprovalDistributionMessage::Approvals(_) )) - )) => { - // Sends to all expected peers. - assert_eq!(sent_peers.len(), 1); - assert_eq!(sent_approvals, approvals); - - assert!(unsent_indices.iter() - .any(|i| &peers[*i].0 == &sent_peers[0])); - } + )) => { } ); - } - - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); -} - -// test aggression L1 -#[test] -fn non_originator_aggression_l1() { - let parent_hash = Hash::repeat_byte(0xFF); - let hash = Hash::repeat_byte(0xAA); - let peers = make_peers_and_authority_ids(100); + // Add blocks until aggression L1 is triggered. + { + let mut parent_hash = hash; + for level in 0..aggression_l1_threshold { + let number = 1 + level + 1; // first block had number 1 + let hash = BlakeTwo256::hash_of(&(parent_hash, number)); + let meta = BlockApprovalMeta { + hash, + parent_hash, + number, + candidates: vec![], + slot: (level as u64).into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; - let mut state = state_without_reputation_delay(); - state.aggression_config.resend_unfinalized_period = None; - let aggression_l1_threshold = state.aggression_config.l1_threshold.unwrap(); + let msg = ApprovalDistributionMessage::ApprovalCheckingLagUpdate(level + 1); + overseer_send(overseer, msg).await; - let _ = test_harness(state, |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; - // Connect all peers except omitted. - for (peer, _) in &peers { - setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; - } + parent_hash = hash; + } + } - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - - // import an assignment and approval locally. - let cert = fake_assignment_cert(hash, validator_index); - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Set up a gossip topology. - setup_gossip_topology( - overseer, - make_gossip_topology( - 1, - &peers_with_optional_peer_id, - &[0, 10, 20, 30], - &[50, 51, 52, 53], - 1, - ), - ) - .await; + let unsent_indices = + (0..peers.len()).filter(|i| !prev_sent_indices.contains(&i)).collect::>(); - let assignments = vec![(cert.clone().into(), candidate_index)]; - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + for _ in 0..unsent_indices.len() { + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + sent_peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + )) + )) => { + // Sends to all expected peers. + assert_eq!(sent_peers.len(), 1); + assert_eq!(sent_assignments, assignments); - // Issuer of the message is important, not the peer we receive from. - // 99 deliberately chosen because it's not in X or Y. - send_message_from_peer(overseer, &peers[99].0, msg).await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - _, - _, - tx, - )) => { - tx.send(AssignmentCheckResult::Accepted).unwrap(); + assert!(unsent_indices.iter() + .any(|i| &peers[*i].0 == &sent_peers[0])); + } + ); } - ); - - expect_reputation_change(overseer, &peers[99].0, BENEFIT_VALID_MESSAGE_FIRST).await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - _, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(_) - )) - )) => { } - ); + for _ in 0..unsent_indices.len() { + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + sent_peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Approvals(sent_approvals) + )) + )) => { + // Sends to all expected peers. + assert_eq!(sent_peers.len(), 1); + assert_eq!(sent_approvals, approvals); - // Add blocks until aggression L1 is triggered. - { - let mut parent_hash = hash; - for level in 0..aggression_l1_threshold { - let number = 1 + level + 1; // first block had number 1 - let hash = BlakeTwo256::hash_of(&(parent_hash, number)); - let meta = BlockApprovalMeta { - hash, - parent_hash, - number, - candidates: vec![], - slot: (level as u64).into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - parent_hash = hash; + assert!(unsent_indices.iter() + .any(|i| &peers[*i].0 == &sent_peers[0])); + } + ); } - } - - // No-op on non-originator - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } -// test aggression L2 on non-originator +// test aggression L1 #[test] -fn non_originator_aggression_l2() { +fn non_originator_aggression_l1() { let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); @@ -3179,275 +3467,256 @@ fn non_originator_aggression_l2() { let mut state = state_without_reputation_delay(); state.aggression_config.resend_unfinalized_period = None; - let aggression_l1_threshold = state.aggression_config.l1_threshold.unwrap(); - let aggression_l2_threshold = state.aggression_config.l2_threshold.unwrap(); - let _ = test_harness(state, |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // Connect all peers except omitted. - for (peer, _) in &peers { - setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; - } + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state, + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - - // import an assignment and approval locally. - let cert = fake_assignment_cert(hash, validator_index); - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Set up a gossip topology. - setup_gossip_topology( - overseer, - make_gossip_topology( - 1, - &peers_with_optional_peer_id, - &[0, 10, 20, 30], - &[50, 51, 52, 53], - 1, - ), - ) - .await; + // Connect all peers except omitted. + for (peer, _) in &peers { + setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; + } - let assignments = vec![(cert.clone(), candidate_index)]; - let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; - // Issuer of the message is important, not the peer we receive from. - // 99 deliberately chosen because it's not in X or Y. - send_message_from_peer(overseer, &peers[99].0, msg).await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( - _, - _, - tx, - )) => { - tx.send(AssignmentCheckResult::Accepted).unwrap(); - } - ); + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; - expect_reputation_change(overseer, &peers[99].0, BENEFIT_VALID_MESSAGE_FIRST).await; - - let prev_sent_indices = assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - sent_peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(_) - )) - )) => { - sent_peers.into_iter() - .filter_map(|sp| peers.iter().position(|p| &p.0 == &sp)) - .collect::>() - } - ); + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; - // Add blocks until aggression L1 is triggered. - let chain_head = { - let mut parent_hash = hash; - for level in 0..aggression_l1_threshold { - let number = 1 + level + 1; // first block had number 1 - let hash = BlakeTwo256::hash_of(&(parent_hash, number)); - let meta = BlockApprovalMeta { - hash, - parent_hash, - number, - candidates: vec![], - slot: (level as u64).into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::ApprovalCheckingLagUpdate(level + 1); - overseer_send(overseer, msg).await; - - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - parent_hash = hash; - } + // import an assignment and approval locally. + let cert = fake_assignment_cert(hash, validator_index); + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Set up a gossip topology. + setup_gossip_topology( + overseer, + make_gossip_topology( + 1, + &peers_with_optional_peer_id, + &[0, 10, 20, 30], + &[50, 51, 52, 53], + 1, + ), + ) + .await; - parent_hash - }; - - // No-op on non-originator - - // Add blocks until aggression L2 is triggered. - { - let mut parent_hash = chain_head; - for level in 0..aggression_l2_threshold - aggression_l1_threshold { - let number = aggression_l1_threshold + level + 1 + 1; // first block had number 1 - let hash = BlakeTwo256::hash_of(&(parent_hash, number)); - let meta = BlockApprovalMeta { - hash, - parent_hash, - number, - candidates: vec![], - slot: (level as u64).into(), - session: 1, - }; - - let msg = ApprovalDistributionMessage::ApprovalCheckingLagUpdate( - aggression_l1_threshold + level + 1, - ); - overseer_send(overseer, msg).await; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; + let assignments = vec![(cert.clone().into(), candidate_index)]; + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); - parent_hash = hash; - } - } + // Issuer of the message is important, not the peer we receive from. + // 99 deliberately chosen because it's not in X or Y. + send_message_from_peer(overseer, &peers[99].0, msg).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + _, + _, + tranche, _, + )) => { + assert_eq!(tranche, 0); + } + ); - // XY dimension - previously sent. - let unsent_indices = [0, 10, 20, 30, 50, 51, 52, 53] - .iter() - .cloned() - .filter(|i| !prev_sent_indices.contains(&i)) - .collect::>(); + expect_reputation_change(overseer, &peers[99].0, BENEFIT_VALID_MESSAGE_FIRST).await; - for _ in 0..unsent_indices.len() { assert_matches!( overseer_recv(overseer).await, AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - sent_peers, + _, Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + protocol_v1::ApprovalDistributionMessage::Assignments(_) )) - )) => { - // Sends to all expected peers. - assert_eq!(sent_peers.len(), 1); - assert_eq!(sent_assignments, assignments); + )) => { } + ); + + // Add blocks until aggression L1 is triggered. + { + let mut parent_hash = hash; + for level in 0..aggression_l1_threshold { + let number = 1 + level + 1; // first block had number 1 + let hash = BlakeTwo256::hash_of(&(parent_hash, number)); + let meta = BlockApprovalMeta { + hash, + parent_hash, + number, + candidates: vec![], + slot: (level as u64).into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; - assert!(unsent_indices.iter() - .any(|i| &peers[*i].0 == &sent_peers[0])); + parent_hash = hash; } - ); - } + } - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + // No-op on non-originator + + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } -// Tests that messages propagate to the unshared dimension. +// test aggression L2 on non-originator #[test] -fn resends_messages_periodically() { +fn non_originator_aggression_l2() { let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); let peers = make_peers_and_authority_ids(100); let mut state = state_without_reputation_delay(); - state.aggression_config.l1_threshold = None; - state.aggression_config.l2_threshold = None; - state.aggression_config.resend_unfinalized_period = Some(2); - let _ = test_harness(state, |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - - // Connect all peers. - for (peer, _) in &peers { - setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; - } - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - // Set up a gossip topology. - setup_gossip_topology( - overseer, - make_gossip_topology( - 1, - &peers_with_optional_peer_id, - &[0, 10, 20, 30], - &[50, 51, 52, 53], - 1, - ), - ) - .await; + state.aggression_config.resend_unfinalized_period = None; - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; + let aggression_l1_threshold = state.aggression_config.l1_threshold.unwrap(); + let aggression_l2_threshold = state.aggression_config.l2_threshold.unwrap(); + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state, + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + + // Connect all peers except omitted. + for (peer, _) in &peers { + setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; + } - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; - // import an assignment and approval locally. - let cert = fake_assignment_cert(hash, validator_index); - let assignments = vec![(cert.clone(), candidate_index)]; + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; + + // import an assignment and approval locally. + let cert = fake_assignment_cert(hash, validator_index); + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Set up a gossip topology. + setup_gossip_topology( + overseer, + make_gossip_topology( + 1, + &peers_with_optional_peer_id, + &[0, 10, 20, 30], + &[50, 51, 52, 53], + 1, + ), + ) + .await; - { + let assignments = vec![(cert.clone(), candidate_index)]; let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); // Issuer of the message is important, not the peer we receive from. // 99 deliberately chosen because it's not in X or Y. send_message_from_peer(overseer, &peers[99].0, msg).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; assert_matches!( overseer_recv(overseer).await, AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( _, _, - tx, + tranche, _, )) => { - tx.send(AssignmentCheckResult::Accepted).unwrap(); + assert_eq!(tranche, 0); } ); - expect_reputation_change(overseer, &peers[99].0, BENEFIT_VALID_MESSAGE_FIRST).await; - let expected_y = [50, 51, 52, 53]; + expect_reputation_change(overseer, &peers[99].0, BENEFIT_VALID_MESSAGE_FIRST).await; - assert_matches!( + let prev_sent_indices = assert_matches!( overseer_recv(overseer).await, AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( sent_peers, Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + protocol_v1::ApprovalDistributionMessage::Assignments(_) )) )) => { - assert_eq!(sent_peers.len(), expected_y.len() + 4); - for &i in &expected_y { - assert!( - sent_peers.contains(&peers[i].0), - "Message not sent to expected peer {}", - i, - ); - } - assert_eq!(sent_assignments, assignments); + sent_peers.into_iter() + .filter_map(|sp| peers.iter().position(|p| &p.0 == &sp)) + .collect::>() } ); - }; - let mut number = 1; - for _ in 0..10 { - // Add blocks until resend is done. - { + // Add blocks until aggression L1 is triggered. + let chain_head = { let mut parent_hash = hash; - for level in 0..2 { - number = number + 1; + for level in 0..aggression_l1_threshold { + let number = 1 + level + 1; // first block had number 1 + let hash = BlakeTwo256::hash_of(&(parent_hash, number)); + let meta = BlockApprovalMeta { + hash, + parent_hash, + number, + candidates: vec![], + slot: (level as u64).into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::ApprovalCheckingLagUpdate(level + 1); + overseer_send(overseer, msg).await; + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + parent_hash = hash; + } + + parent_hash + }; + + // No-op on non-originator + + // Add blocks until aggression L2 is triggered. + { + let mut parent_hash = chain_head; + for level in 0..aggression_l2_threshold - aggression_l1_threshold { + let number = aggression_l1_threshold + level + 1 + 1; // first block had number 1 let hash = BlakeTwo256::hash_of(&(parent_hash, number)); let meta = BlockApprovalMeta { hash, @@ -3456,9 +3725,12 @@ fn resends_messages_periodically() { candidates: vec![], slot: (level as u64).into(), session: 1, + vrf_story: RelayVRFStory(Default::default()), }; - let msg = ApprovalDistributionMessage::ApprovalCheckingLagUpdate(2); + let msg = ApprovalDistributionMessage::ApprovalCheckingLagUpdate( + aggression_l1_threshold + level + 1, + ); overseer_send(overseer, msg).await; let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); overseer_send(overseer, msg).await; @@ -3467,10 +3739,14 @@ fn resends_messages_periodically() { } } - let mut expected_y = vec![50, 51, 52, 53]; + // XY dimension - previously sent. + let unsent_indices = [0, 10, 20, 30, 50, 51, 52, 53] + .iter() + .cloned() + .filter(|i| !prev_sent_indices.contains(&i)) + .collect::>(); - // Expect messages sent only to topology peers, one by one. - for _ in 0..expected_y.len() { + for _ in 0..unsent_indices.len() { assert_matches!( overseer_recv(overseer).await, AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( @@ -3479,21 +3755,186 @@ fn resends_messages_periodically() { protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) )) )) => { + // Sends to all expected peers. assert_eq!(sent_peers.len(), 1); - let expected_pos = expected_y.iter() - .position(|&i| &peers[i].0 == &sent_peers[0]) - .unwrap(); + assert_eq!(sent_assignments, assignments); + + assert!(unsent_indices.iter() + .any(|i| &peers[*i].0 == &sent_peers[0])); + } + ); + } + + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); +} + +// Tests that messages propagate to the unshared dimension. +#[test] +fn resends_messages_periodically() { + let parent_hash = Hash::repeat_byte(0xFF); + let hash = Hash::repeat_byte(0xAA); + + let peers = make_peers_and_authority_ids(100); + + let mut state = state_without_reputation_delay(); + state.aggression_config.l1_threshold = None; + state.aggression_config.l2_threshold = None; + state.aggression_config.resend_unfinalized_period = Some(2); + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state, + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + + // Connect all peers. + for (peer, _) in &peers { + setup_peer_with_view(overseer, peer, view![hash], ValidationVersion::V1).await; + } + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + // Set up a gossip topology. + setup_gossip_topology( + overseer, + make_gossip_topology( + 1, + &peers_with_optional_peer_id, + &[0, 10, 20, 30], + &[50, 51, 52, 53], + 1, + ), + ) + .await; + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; + + // import an assignment and approval locally. + let cert = fake_assignment_cert(hash, validator_index); + let assignments = vec![(cert.clone(), candidate_index)]; + + { + let msg = + protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + + // Issuer of the message is important, not the peer we receive from. + // 99 deliberately chosen because it's not in X or Y. + send_message_from_peer(overseer, &peers[99].0, msg).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportAssignment( + _, + _, + tranche, _, + )) => { + assert_eq!(tranche, 0); + } + ); + expect_reputation_change(overseer, &peers[99].0, BENEFIT_VALID_MESSAGE_FIRST).await; + + let expected_y = [50, 51, 52, 53]; - expected_y.remove(expected_pos); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + sent_peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + )) + )) => { + assert_eq!(sent_peers.len(), expected_y.len() + 4); + for &i in &expected_y { + assert!( + sent_peers.contains(&peers[i].0), + "Message not sent to expected peer {}", + i, + ); + } assert_eq!(sent_assignments, assignments); } ); + }; + + let mut number = 1; + for _ in 0..10 { + // Add blocks until resend is done. + { + let mut parent_hash = hash; + for level in 0..2 { + number = number + 1; + let hash = BlakeTwo256::hash_of(&(parent_hash, number)); + let meta = BlockApprovalMeta { + hash, + parent_hash, + number, + candidates: vec![], + slot: (level as u64).into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + + let msg = ApprovalDistributionMessage::ApprovalCheckingLagUpdate(2); + overseer_send(overseer, msg).await; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + parent_hash = hash; + } + } + + let mut expected_y = vec![50, 51, 52, 53]; + + // Expect messages sent only to topology peers, one by one. + for _ in 0..expected_y.len() { + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + sent_peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(sent_assignments) + )) + )) => { + assert_eq!(sent_peers.len(), 1); + let expected_pos = expected_y.iter() + .position(|&i| &peers[i].0 == &sent_peers[0]) + .unwrap(); + + expected_y.remove(expected_pos); + assert_eq!(sent_assignments, assignments); + } + ); + } } - } - assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); - virtual_overseer - }); + assert!(overseer.recv().timeout(TIMEOUT).await.is_none(), "no message should be sent"); + virtual_overseer + }, + ); } /// Tests that peers correctly receive versioned messages. @@ -3507,154 +3948,170 @@ fn import_versioned_approval() { let parent_hash = Hash::repeat_byte(0xFF); let hash = Hash::repeat_byte(0xAA); let state = state_without_reputation_delay(); - let _ = test_harness(state, |mut virtual_overseer| async move { - let overseer = &mut virtual_overseer; - // All peers are aware of relay parent. - setup_peer_with_view(overseer, &peer_a, view![hash], ValidationVersion::V2).await; - setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V1).await; - setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V2).await; - - // Set up a gossip topology, where a, b, c and d are topology neighbors to the node under - // testing. - let peers_with_optional_peer_id = peers - .iter() - .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) - .collect_vec(); - setup_gossip_topology( - overseer, - make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), - ) - .await; + let candidate_hash = polkadot_primitives::CandidateHash(Hash::repeat_byte(0xBB)); + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(0) }, + Box::new(SystemClock {}), + state, + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // All peers are aware of relay parent. + setup_peer_with_view(overseer, &peer_a, view![hash], ValidationVersion::V2).await; + setup_peer_with_view(overseer, &peer_b, view![hash], ValidationVersion::V1).await; + setup_peer_with_view(overseer, &peer_c, view![hash], ValidationVersion::V2).await; + + // Set up a gossip topology, where a, b, c and d are topology neighbors to the node + // under testing. + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), + ) + .await; - // new block `hash_a` with 1 candidates - let meta = BlockApprovalMeta { - hash, - parent_hash, - number: 1, - candidates: vec![Default::default(); 1], - slot: 1.into(), - session: 1, - }; - let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); - overseer_send(overseer, msg).await; - - // import an assignment related to `hash` locally - let validator_index = ValidatorIndex(0); - let candidate_index = 0u32; - let cert = fake_assignment_cert(hash, validator_index); - overseer_send( - overseer, - ApprovalDistributionMessage::DistributeAssignment(cert.into(), candidate_index.into()), - ) - .await; + let mut keystore = LocalKeystore::in_memory(); + let session = dummy_session_info_valid(1, &mut keystore, 1); + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 1, + candidates: vec![(candidate_hash, 0.into(), 0.into()); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + // import an assignment related to `hash` locally + let validator_index = ValidatorIndex(0); + let candidate_index = 0u32; + let cert = fake_assignment_cert(hash, validator_index); + overseer_send( + overseer, + ApprovalDistributionMessage::DistributeAssignment( + cert.into(), + candidate_index.into(), + ), + ) + .await; - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers, vec![peer_b]); - assert_eq!(assignments.len(), 1); - } - ); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers, vec![peer_b]); + assert_eq!(assignments.len(), 1); + } + ); - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V2(protocol_v2::ValidationProtocol::ApprovalDistribution( - protocol_v2::ApprovalDistributionMessage::Assignments(assignments) - )) - )) => { - assert_eq!(peers.len(), 2); - assert!(peers.contains(&peer_a)); - assert!(peers.contains(&peer_c)); - - assert_eq!(assignments.len(), 1); - } - ); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V2(protocol_v2::ValidationProtocol::ApprovalDistribution( + protocol_v2::ApprovalDistributionMessage::Assignments(assignments) + )) + )) => { + assert_eq!(peers.len(), 2); + assert!(peers.contains(&peer_a)); + assert!(peers.contains(&peer_c)); - // send the an approval from peer_a - let approval = IndirectSignedApprovalVote { - block_hash: hash, - candidate_index, - validator: validator_index, - signature: dummy_signature(), - }; - let msg = protocol_v2::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); - send_message_from_peer_v2(overseer, &peer_a, msg).await; - - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( - vote, - tx, - )) => { - assert_eq!(vote, approval.into()); - tx.send(ApprovalCheckResult::Accepted).unwrap(); - } - ); + assert_eq!(assignments.len(), 1); + } + ); - expect_reputation_change(overseer, &peer_a, BENEFIT_VALID_MESSAGE_FIRST).await; - - // Peers b and c receive versioned approval messages. - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( - protocol_v1::ApprovalDistributionMessage::Approvals(approvals) - )) - )) => { - assert_eq!(peers, vec![peer_b]); - assert_eq!(approvals.len(), 1); - } - ); + // send the an approval from peer_a + let approval = IndirectSignedApprovalVote { + block_hash: hash, + candidate_index, + validator: validator_index, + signature: signature_for( + &keystore, + &session, + vec![candidate_hash], + validator_index, + ), + }; + let msg = protocol_v2::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); + send_message_from_peer_v2(overseer, &peer_a, msg).await; + provide_session(overseer, session).await; + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ApprovalVoting(ApprovalVotingMessage::CheckAndImportApproval( + vote, _, + )) => { + assert_eq!(vote, approval.into()); + } + ); - assert_matches!( - overseer_recv(overseer).await, - AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( - peers, - Versioned::V2(protocol_v2::ValidationProtocol::ApprovalDistribution( - protocol_v2::ApprovalDistributionMessage::Approvals(approvals) - )) - )) => { - assert_eq!(peers, vec![peer_c]); - assert_eq!(approvals.len(), 1); - } - ); + expect_reputation_change(overseer, &peer_a, BENEFIT_VALID_MESSAGE_FIRST).await; + + // Peers b and c receive versioned approval messages. + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V1(protocol_v1::ValidationProtocol::ApprovalDistribution( + protocol_v1::ApprovalDistributionMessage::Approvals(approvals) + )) + )) => { + assert_eq!(peers, vec![peer_b]); + assert_eq!(approvals.len(), 1); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::NetworkBridgeTx(NetworkBridgeTxMessage::SendValidationMessage( + peers, + Versioned::V2(protocol_v2::ValidationProtocol::ApprovalDistribution( + protocol_v2::ApprovalDistributionMessage::Approvals(approvals) + )) + )) => { + assert_eq!(peers, vec![peer_c]); + assert_eq!(approvals.len(), 1); + } + ); + + // send an obviously invalid approval + let approval = IndirectSignedApprovalVote { + block_hash: hash, + // Invalid candidate index, should not pass sanitization. + candidate_index: 16777284, + validator: validator_index, + signature: dummy_signature(), + }; + let msg = protocol_v2::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); + send_message_from_peer_v2(overseer, &peer_a, msg).await; - // send an obviously invalid approval - let approval = IndirectSignedApprovalVote { - block_hash: hash, - // Invalid candidate index, should not pass sanitization. - candidate_index: 16777284, - validator: validator_index, - signature: dummy_signature(), - }; - let msg = protocol_v2::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); - send_message_from_peer_v2(overseer, &peer_a, msg).await; - - expect_reputation_change(overseer, &peer_a, COST_OVERSIZED_BITFIELD).await; - - // send an obviously invalid approval - let approval = IndirectSignedApprovalVoteV2 { - block_hash: hash, - // Invalid candidates len, should not pass sanitization. - candidate_indices: 16777284.into(), - validator: validator_index, - signature: dummy_signature(), - }; - let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); - send_message_from_peer_v3(overseer, &peer_a, msg).await; - - expect_reputation_change(overseer, &peer_a, COST_OVERSIZED_BITFIELD).await; + expect_reputation_change(overseer, &peer_a, COST_OVERSIZED_BITFIELD).await; - virtual_overseer - }); + // send an obviously invalid approval + let approval = IndirectSignedApprovalVoteV2 { + block_hash: hash, + // Invalid candidates len, should not pass sanitization. + candidate_indices: 16777284.into(), + validator: validator_index, + signature: dummy_signature(), + }; + let msg = protocol_v3::ApprovalDistributionMessage::Approvals(vec![approval.clone()]); + send_message_from_peer_v3(overseer, &peer_a, msg).await; + + expect_reputation_change(overseer, &peer_a, COST_OVERSIZED_BITFIELD).await; + + virtual_overseer + }, + ); } fn batch_test_round(message_count: usize) { @@ -3664,11 +4121,26 @@ fn batch_test_round(message_count: usize) { let (mut context, mut virtual_overseer) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool.clone()); - let subsystem = ApprovalDistribution::new(Default::default()); + let subsystem = ApprovalDistribution::new_with_clock( + Default::default(), + Default::default(), + Box::new(SystemClock {}), + ); let mut rng = rand_chacha::ChaCha12Rng::seed_from_u64(12345); let mut sender = context.sender().clone(); - let subsystem = - subsystem.run_inner(context, &mut state, REPUTATION_CHANGE_TEST_INTERVAL, &mut rng); + let mut session_info_provider = RuntimeInfo::new_with_config(RuntimeInfoConfig { + keystore: None, + session_cache_lru_size: DISPUTE_WINDOW.get(), + }); + + let subsystem = subsystem.run_inner( + context, + &mut state, + REPUTATION_CHANGE_TEST_INTERVAL, + &mut rng, + &MockAssignmentCriteria { tranche: Ok(0) }, + &mut session_info_provider, + ); let test_fut = async move { let overseer = &mut virtual_overseer; @@ -3814,3 +4286,146 @@ fn const_ensure_size_not_zero() { crate::ensure_size_not_zero(super::MAX_ASSIGNMENT_BATCH_SIZE); crate::ensure_size_not_zero(super::MAX_APPROVAL_BATCH_SIZE); } + +struct DummyClock; +impl Clock for DummyClock { + fn tick_now(&self) -> polkadot_node_core_approval_voting::time::Tick { + 0 + } + + fn wait( + &self, + _tick: polkadot_node_core_approval_voting::time::Tick, + ) -> std::pin::Pin + Send + 'static>> { + todo!() + } +} + +/// Subsystem rejects assignments too far into the future. +#[test] +fn subsystem_rejects_assignment_in_future() { + let peers = make_peers_and_authority_ids(15); + let peer_a = peers.get(0).unwrap().0; + let parent_hash = Hash::repeat_byte(0xFF); + let hash = Hash::repeat_byte(0xAA); + + let _ = test_harness( + &MockAssignmentCriteria { tranche: Ok(89) }, + Box::new(DummyClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // setup peers + setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V1).await; + + // Set up a gossip topology, where a, b, c and d are topology neighbors to the node + // under testing. + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), + ) + .await; + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 2, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + // send the assignment related to `hash` + let validator_index = ValidatorIndex(0); + let cert = fake_assignment_cert(hash, validator_index); + let assignments = vec![(cert.clone(), 0u32)]; + setup_peer_with_view(overseer, &peer_a, view![hash], ValidationVersion::V3).await; + + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + send_message_from_peer(overseer, &peer_a, msg).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; + + expect_reputation_change(overseer, &peer_a, COST_ASSIGNMENT_TOO_FAR_IN_THE_FUTURE) + .await; + + virtual_overseer + }, + ); +} + +/// Subsystem rejects assignments too far into the future. +#[test] +fn subsystem_rejects_bad_assignments() { + let peers = make_peers_and_authority_ids(15); + let peer_a = peers.get(0).unwrap().0; + let parent_hash = Hash::repeat_byte(0xFF); + let hash = Hash::repeat_byte(0xAA); + + let _ = test_harness( + &MockAssignmentCriteria { + tranche: Err(InvalidAssignment(criteria::InvalidAssignmentReason::NullAssignment)), + }, + Box::new(DummyClock {}), + state_without_reputation_delay(), + |mut virtual_overseer| async move { + let overseer = &mut virtual_overseer; + // setup peers + setup_peer_with_view(overseer, &peer_a, view![], ValidationVersion::V1).await; + + // Set up a gossip topology, where a, b, c and d are topology neighbors to the node + // under testing. + let peers_with_optional_peer_id = peers + .iter() + .map(|(peer_id, authority)| (Some(*peer_id), authority.clone())) + .collect_vec(); + setup_gossip_topology( + overseer, + make_gossip_topology(1, &peers_with_optional_peer_id, &[0, 1], &[2, 4], 3), + ) + .await; + + // new block `hash_a` with 1 candidates + let meta = BlockApprovalMeta { + hash, + parent_hash, + number: 2, + candidates: vec![Default::default(); 1], + slot: 1.into(), + session: 1, + vrf_story: RelayVRFStory(Default::default()), + }; + let msg = ApprovalDistributionMessage::NewBlocks(vec![meta]); + overseer_send(overseer, msg).await; + + // send the assignment related to `hash` + let validator_index = ValidatorIndex(0); + let cert = fake_assignment_cert(hash, validator_index); + let assignments = vec![(cert.clone(), 0u32)]; + setup_peer_with_view(overseer, &peer_a, view![hash], ValidationVersion::V3).await; + + let msg = protocol_v1::ApprovalDistributionMessage::Assignments(assignments.clone()); + send_message_from_peer(overseer, &peer_a, msg).await; + provide_session( + overseer, + dummy_session_info_valid(1, &mut LocalKeystore::in_memory(), 1), + ) + .await; + + expect_reputation_change(overseer, &peer_a, COST_INVALID_MESSAGE).await; + + virtual_overseer + }, + ); +} diff --git a/polkadot/node/overseer/src/lib.rs b/polkadot/node/overseer/src/lib.rs index 24985a99913d..2c113f81c85f 100644 --- a/polkadot/node/overseer/src/lib.rs +++ b/polkadot/node/overseer/src/lib.rs @@ -580,6 +580,7 @@ pub struct Overseer { #[subsystem(blocking, message_capacity: 64000, ApprovalDistributionMessage, sends: [ NetworkBridgeTxMessage, ApprovalVotingMessage, + RuntimeApiMessage, ])] approval_distribution: ApprovalDistribution, diff --git a/polkadot/node/primitives/src/approval.rs b/polkadot/node/primitives/src/approval.rs index 66883b33367b..ec41647fc3d1 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, ValidatorIndex, ValidatorSignature, }; use sp_application_crypto::ByteArray; @@ -128,11 +128,13 @@ 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 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 5f4db99b00ef..eedc92572e08 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-bench/src/lib/approval/mod.rs b/polkadot/node/subsystem-bench/src/lib/approval/mod.rs index 4ac044ea3459..4d00b803033a 100644 --- a/polkadot/node/subsystem-bench/src/lib/approval/mod.rs +++ b/polkadot/node/subsystem-bench/src/lib/approval/mod.rs @@ -804,8 +804,11 @@ fn build_overseer( Box::new(system_clock.clone()), ); - let approval_distribution = - ApprovalDistribution::new(Metrics::register(Some(&dependencies.registry)).unwrap()); + let approval_distribution = ApprovalDistribution::new_with_clock( + Metrics::register(Some(&dependencies.registry)).unwrap(), + SLOT_DURATION_MILLIS, + Box::new(system_clock.clone()), + ); let mock_chain_api = MockChainApi::new(state.build_chain_api_state()); let mock_chain_selection = MockChainSelection { state: state.clone(), clock: system_clock }; let mock_runtime_api = MockRuntimeApi::new( diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index ee937bca05bf..e0c078de48b7 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, @@ -967,13 +967,17 @@ pub enum ApprovalVotingMessage { CheckAndImportAssignment( IndirectAssignmentCertV2, CandidateBitfield, - oneshot::Sender, + DelayTranche, + Option>, ), /// 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, + Option>, + ), /// 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