diff --git a/polkadot/.config/lingua.dic b/polkadot/.config/lingua.dic index dcba9b0611..7460cc55d3 100644 --- a/polkadot/.config/lingua.dic +++ b/polkadot/.config/lingua.dic @@ -82,6 +82,7 @@ UDP UI union/MSG unservable/B +unfinalize/B validator/SM w3f/MS wasm/M diff --git a/polkadot/node/network/approval-distribution/src/lib.rs b/polkadot/node/network/approval-distribution/src/lib.rs index f1df7b1acd..1c53b74d26 100644 --- a/polkadot/node/network/approval-distribution/src/lib.rs +++ b/polkadot/node/network/approval-distribution/src/lib.rs @@ -186,11 +186,11 @@ impl State { self.blocks_by_number.entry(meta.number).or_default().push(meta.hash); } for (peer_id, view) in self.peer_views.iter() { - let intersection = view.heads.iter().filter(|h| new_hashes.contains(h)); - let view_intersection = View { - heads: intersection.cloned().collect(), - finalized_number: view.finalized_number, - }; + let intersection = view.iter().filter(|h| new_hashes.contains(h)); + let view_intersection = View::new( + intersection.cloned(), + view.finalized_number, + ); Self::unify_with_peer( &mut self.blocks, ctx, @@ -634,7 +634,7 @@ impl State { let mut to_send = HashSet::new(); let view_finalized_number = view.finalized_number; - for head in view.heads.into_iter() { + for head in view.into_iter() { let mut block = head; let interesting_blocks = std::iter::from_fn(|| { // step 2. diff --git a/polkadot/node/network/approval-distribution/src/tests.rs b/polkadot/node/network/approval-distribution/src/tests.rs index 5e0753749e..86395a92ad 100644 --- a/polkadot/node/network/approval-distribution/src/tests.rs +++ b/polkadot/node/network/approval-distribution/src/tests.rs @@ -330,7 +330,7 @@ fn spam_attack_results_in_negative_reputation_change() { overseer_send( overseer, ApprovalDistributionMessage::NetworkBridgeUpdateV1( - NetworkBridgeEvent::PeerViewChange(peer.clone(), View { heads: Default::default(), finalized_number: 2 }) + NetworkBridgeEvent::PeerViewChange(peer.clone(), View::with_finalized(2)) ) ).await; @@ -666,7 +666,7 @@ fn update_peer_view() { overseer_send( overseer, ApprovalDistributionMessage::NetworkBridgeUpdateV1( - NetworkBridgeEvent::PeerViewChange(peer.clone(), View { heads: vec![hash_b, hash_c, hash_d], finalized_number: 2 }) + NetworkBridgeEvent::PeerViewChange(peer.clone(), View::new(vec![hash_b, hash_c, hash_d], 2)) ) ).await; @@ -713,7 +713,7 @@ fn update_peer_view() { overseer_send( overseer, ApprovalDistributionMessage::NetworkBridgeUpdateV1( - NetworkBridgeEvent::PeerViewChange(peer.clone(), View { heads: vec![], finalized_number }) + NetworkBridgeEvent::PeerViewChange(peer.clone(), View::with_finalized(finalized_number)) ) ).await; }); diff --git a/polkadot/node/network/availability-distribution/src/lib.rs b/polkadot/node/network/availability-distribution/src/lib.rs index 798bc2570d..37b8dfdd11 100644 --- a/polkadot/node/network/availability-distribution/src/lib.rs +++ b/polkadot/node/network/availability-distribution/src/lib.rs @@ -417,7 +417,7 @@ where .filter(|(_peer, view)| { // collect all direct interests of a peer w/o ancestors state - .cached_live_candidates_unioned(view.heads.iter()) + .cached_live_candidates_unioned(view.iter()) .contains(&candidate_hash) }) .map(|(peer, _view)| peer.clone()) @@ -623,7 +623,7 @@ where let _timer = metrics.time_process_incoming_peer_message(); // obtain the set of candidates we are interested in based on our current view - let live_candidates = state.cached_live_candidates_unioned(state.view.heads.iter()); + let live_candidates = state.cached_live_candidates_unioned(state.view.iter()); // check if the candidate is of interest let candidate_hash = message.candidate_hash; @@ -764,7 +764,7 @@ where // peers view must contain the candidate hash too cached_live_candidates_unioned( per_relay_parent, - view.heads.iter(), + view.iter(), ).contains(&message.candidate_hash) }) .map(|(peer, _)| -> PeerId { peer.clone() }) diff --git a/polkadot/node/network/availability-distribution/src/tests.rs b/polkadot/node/network/availability-distribution/src/tests.rs index fc3f653e44..473e478994 100644 --- a/polkadot/node/network/availability-distribution/src/tests.rs +++ b/polkadot/node/network/availability-distribution/src/tests.rs @@ -37,7 +37,7 @@ use maplit::hashmap; macro_rules! view { ( $( $hash:expr ),* $(,)? ) => { // Finalized number unimportant for availability distribution. - View { heads: vec![ $( $hash.clone() ),* ], finalized_number: 0 } + View::new(vec![ $( $hash.clone() ),* ], 0) }; } diff --git a/polkadot/node/network/bitfield-distribution/src/lib.rs b/polkadot/node/network/bitfield-distribution/src/lib.rs index 9978b720e8..70748addd4 100644 --- a/polkadot/node/network/bitfield-distribution/src/lib.rs +++ b/polkadot/node/network/bitfield-distribution/src/lib.rs @@ -835,7 +835,7 @@ mod test { let validator = SyncCryptoStore::sr25519_generate_new(&*keystore, ValidatorId::ID, None) .expect("generating sr25519 key not to fail"); - state.per_relay_parent = view.heads.iter().map(|relay_parent| {( + state.per_relay_parent = view.iter().map(|relay_parent| {( relay_parent.clone(), PerRelayParentData { signing_context: signing_context.clone(), diff --git a/polkadot/node/network/bridge/src/lib.rs b/polkadot/node/network/bridge/src/lib.rs index 9c4414994e..e1dfa8fb5f 100644 --- a/polkadot/node/network/bridge/src/lib.rs +++ b/polkadot/node/network/bridge/src/lib.rs @@ -409,10 +409,10 @@ where } fn construct_view(live_heads: impl DoubleEndedIterator, finalized_number: BlockNumber) -> View { - View { - heads: live_heads.rev().take(MAX_VIEW_HEADS).collect(), + View::new( + live_heads.rev().take(MAX_VIEW_HEADS), finalized_number - } + ) } #[tracing::instrument(level = "trace", skip(net, ctx, validation_peers, collation_peers), fields(subsystem = LOG_TARGET))] @@ -427,8 +427,9 @@ async fn update_our_view( ) -> SubsystemResult<()> { let new_view = construct_view(live_heads.iter().map(|v| v.0), finalized_number); - // We only want to send a view update when the heads changed, not when only the finalized block changed. - if local_view.heads == new_view.heads { + // We only want to send a view update when the heads changed. + // A change in finalized block number only is _not_ sufficient. + if local_view.check_heads_eq(&new_view) { return Ok(()) } @@ -477,7 +478,7 @@ async fn handle_peer_messages( for message in messages { outgoing_messages.push(match message { WireMessage::ViewUpdate(new_view) => { - if new_view.heads.len() > MAX_VIEW_HEADS || + if new_view.len() > MAX_VIEW_HEADS || new_view.finalized_number < peer_data.view.finalized_number { net.report_peer( @@ -486,7 +487,7 @@ async fn handle_peer_messages( ).await?; continue - } else if new_view.heads.is_empty() { + } else if new_view.is_empty() { net.report_peer( peer.clone(), EMPTY_VIEW_COST, @@ -996,7 +997,7 @@ mod tests { let actions = network_handle.next_network_actions(4).await; let wire_message = WireMessage::::ViewUpdate( - View { heads: vec![hash_a], finalized_number: 5 } + View::new(vec![hash_a], 5) ).encode(); assert_network_actions_contains( @@ -1378,10 +1379,7 @@ mod tests { let actions = network_handle.next_network_actions(2).await; let wire_message = WireMessage::::ViewUpdate( - View { - heads: vec![hash_b], - finalized_number: 1, - } + View::new(vec![hash_b], 1) ).encode(); assert_network_actions_contains( @@ -1412,7 +1410,7 @@ mod tests { peer_a.clone(), PeerSet::Validation, WireMessage::::ViewUpdate( - View { heads: vec![Hash::repeat_byte(0x01)], finalized_number: 1 }, + View::new(vec![Hash::repeat_byte(0x01)], 1), ).encode(), ).await; @@ -1420,7 +1418,7 @@ mod tests { peer_a.clone(), PeerSet::Validation, WireMessage::::ViewUpdate( - View { heads: vec![], finalized_number: 0 }, + View::new(vec![], 0), ).encode(), ).await; diff --git a/polkadot/node/network/collator-protocol/src/collator_side.rs b/polkadot/node/network/collator-protocol/src/collator_side.rs index f69b5df15b..3036c927e2 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side.rs @@ -1342,7 +1342,7 @@ mod tests { overseer_send( virtual_overseer, CollatorProtocolMessage::NetworkBridgeUpdateV1( - NetworkBridgeEvent::PeerViewChange(peer.clone(), View { heads: hashes, finalized_number: 0 }), + NetworkBridgeEvent::PeerViewChange(peer.clone(), View::new(hashes, 0)), ), ).await; } diff --git a/polkadot/node/network/pov-distribution/src/lib.rs b/polkadot/node/network/pov-distribution/src/lib.rs index 137009d014..eda53777fd 100644 --- a/polkadot/node/network/pov-distribution/src/lib.rs +++ b/polkadot/node/network/pov-distribution/src/lib.rs @@ -684,7 +684,7 @@ async fn handle_network_update( peer_state.awaited.retain(|relay_parent, _| view.contains(&relay_parent)); // introduce things from the new view. - for relay_parent in view.heads.iter() { + for relay_parent in view.iter() { if let Entry::Vacant(entry) = peer_state.awaited.entry(*relay_parent) { entry.insert(HashSet::new()); diff --git a/polkadot/node/network/protocol/src/lib.rs b/polkadot/node/network/protocol/src/lib.rs index 2f7547f2de..107e3079f3 100644 --- a/polkadot/node/network/protocol/src/lib.rs +++ b/polkadot/node/network/protocol/src/lib.rs @@ -129,12 +129,12 @@ impl OurView { /// Creates a new instance. pub fn new(heads: impl IntoIterator)>, finalized_number: BlockNumber) -> Self { let state_per_head = heads.into_iter().collect::>(); - + let view = View::new( + state_per_head.keys().cloned(), + finalized_number, + ); Self { - view: View { - heads: state_per_head.keys().cloned().collect(), - finalized_number, - }, + view, span_per_head: state_per_head, } } @@ -189,7 +189,8 @@ macro_rules! our_view { #[derive(Default, Debug, Clone, PartialEq, Eq, Encode, Decode)] pub struct View { /// A bounded amount of chain heads. - pub heads: Vec, + /// Invariant: Sorted. + heads: Vec, /// The highest known finalized block number. pub finalized_number: BlockNumber, } @@ -208,11 +209,50 @@ pub struct View { #[macro_export] macro_rules! view { ( $( $hash:expr ),* $(,)? ) => { - $crate::View { heads: vec![ $( $hash.clone() ),* ], finalized_number: 0 } + $crate::View::new(vec![ $( $hash.clone() ),* ], 0) }; } impl View { + /// Construct a new view based on heads and a finalized block number. + pub fn new(heads: impl IntoIterator, finalized_number: BlockNumber) -> Self + { + let mut heads = heads.into_iter().collect::>(); + heads.sort(); + Self { + heads, + finalized_number, + } + } + + /// Start with no heads, but only a finalized block number. + pub fn with_finalized(finalized_number: BlockNumber) -> Self { + Self { + heads: Vec::new(), + finalized_number, + } + } + + /// Obtain the number of heads that are in view. + pub fn len(&self) -> usize { + self.heads.len() + } + + /// Check if the number of heads contained, is null. + pub fn is_empty(&self) -> bool { + self.heads.is_empty() + } + + /// Obtain an iterator over all heads. + pub fn iter<'a>(&'a self) -> impl Iterator { + self.heads.iter() + } + + /// Obtain an iterator over all heads. + pub fn into_iter(self) -> impl Iterator { + self.heads.into_iter() + } + /// Replace `self` with `new`. /// /// Returns an iterator that will yield all elements of `new` that were not part of `self`. @@ -236,6 +276,14 @@ impl View { pub fn contains(&self, hash: &Hash) -> bool { self.heads.contains(hash) } + + /// Check if two views have the same heads. + /// + /// Equivalent to the `PartialEq` fn, + /// but ignores the `finalized_number` field. + pub fn check_heads_eq(&self, other: &Self) -> bool { + self.heads == other.heads + } } /// v1 protocol types. diff --git a/polkadot/node/network/statement-distribution/src/lib.rs b/polkadot/node/network/statement-distribution/src/lib.rs index a5a6c1247f..3ae86eda59 100644 --- a/polkadot/node/network/statement-distribution/src/lib.rs +++ b/polkadot/node/network/statement-distribution/src/lib.rs @@ -1510,7 +1510,7 @@ mod tests { let peer_data_from_view = |view: View| PeerData { view: view.clone(), - view_knowledge: view.heads.iter().map(|v| (v.clone(), Default::default())).collect(), + view_knowledge: view.iter().map(|v| (v.clone(), Default::default())).collect(), }; let mut peer_data: HashMap<_, _> = vec![