feat/view: assure heads in a view are sorted (#2493)

* feat/view: assure heads in a view are sorted

Allows O(n) comparisons, adds an alternate equiv relation
which takes O(n^2) for integrity verification.

Ref #2133

* revert: remove custom PartialEq impl, there are no duplicates

* fix: do not sort the live_heads, that alters the local view

* refactor/view: heads should not be public

* chore/spellcheck: add unfinalized

* fix/view: add missing len() and is_empty() fns

* quirk

* vec is not view

* Update node/network/approval-distribution/src/tests.rs

Co-authored-by: Andronik Ordian <write@reusable.software>

* Update node/network/bridge/src/lib.rs

Co-authored-by: Andronik Ordian <write@reusable.software>

* Update node/network/protocol/src/lib.rs

Co-authored-by: Andronik Ordian <write@reusable.software>

* fixup comment

* fix botched test

Co-authored-by: Andronik Ordian <write@reusable.software>

polkadot/.config/lingua.dic

@@ -82,6 +82,7 @@ UDP
 UI
 union/MSG
 unservable/B
+unfinalize/B
 validator/SM
 w3f/MS
 wasm/M

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.

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;
 	});

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() })

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)
 	};
 }
 

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(),

polkadot/node/network/bridge/src/lib.rs

@@ -409,10 +409,10 @@ where
 }
 
 fn construct_view(live_heads: impl DoubleEndedIterator<Item = Hash>, 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<M>(
 	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<M>(
 					).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::<protocol_v1::ValidationProtocol>::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::<protocol_v1::ValidationProtocol>::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::<protocol_v1::ValidationProtocol>::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::<protocol_v1::ValidationProtocol>::ViewUpdate(
-					View { heads: vec![], finalized_number: 0 },
+					View::new(vec![], 0),
 				).encode(),
 			).await;
 

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;
 	}

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());
 

polkadot/node/network/protocol/src/lib.rs

@@ -129,12 +129,12 @@ impl OurView {
 	/// Creates a new instance.
 	pub fn new(heads: impl IntoIterator<Item = (Hash, Arc<jaeger::Span>)>, finalized_number: BlockNumber) -> Self {
 		let state_per_head = heads.into_iter().collect::<HashMap<_, _>>();
-
+		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<Hash>,
+	/// Invariant: Sorted.
+	heads: Vec<Hash>,
 	/// 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<Item=Hash>, finalized_number: BlockNumber) -> Self
+	{
+		let mut heads = heads.into_iter().collect::<Vec<Hash>>();
+		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<Item=&'a Hash> {
+		self.heads.iter()
+	}
+
+	/// Obtain an iterator over all heads.
+	pub fn into_iter(self) -> impl Iterator<Item=Hash> {
+		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.

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![