Fix on demand parachains relay when no parachain head at target (#1834)

* `best_finalized_peer_at_best_self` in messages relay is now Option<> - before it was an error, which effectively blocked the lane * unnecessary mut * clone on return
paritytech · Jan 31, 2023 · 45a68ad · 45a68ad
1 parent 6dbce72
commit 45a68ad
Show file tree

Hide file tree

Showing 6 changed files with 150 additions and 98 deletions.
diff --git a/relays/lib-substrate-relay/src/finality/target.rs b/relays/lib-substrate-relay/src/finality/target.rs
@@ -102,7 +102,8 @@ where
 			None,
 		)
 		.await?
-		.best_finalized_peer_at_best_self)
+		.best_finalized_peer_at_best_self
+		.ok_or(Error::BridgePalletIsNotInitialized)?)
 	}
 
 	async fn submit_finality_proof(

diff --git a/relays/lib-substrate-relay/src/messages_source.rs b/relays/lib-substrate-relay/src/messages_source.rs
@@ -421,14 +421,15 @@ where
 		.await?;
 
 	// read actual header, matching the `peer_on_self_best_finalized_id` from the peer chain
-	let actual_peer_on_self_best_finalized_id = match peer_client {
-		Some(peer_client) => {
-			let actual_peer_on_self_best_finalized =
-				peer_client.header_by_number(peer_on_self_best_finalized_id.number()).await?;
-			actual_peer_on_self_best_finalized.id()
-		},
-		None => peer_on_self_best_finalized_id,
-	};
+	let actual_peer_on_self_best_finalized_id =
+		match (peer_client, peer_on_self_best_finalized_id.as_ref()) {
+			(Some(peer_client), Some(peer_on_self_best_finalized_id)) => {
+				let actual_peer_on_self_best_finalized =
+					peer_client.header_by_number(peer_on_self_best_finalized_id.number()).await?;
+				Some(actual_peer_on_self_best_finalized.id())
+			},
+			_ => peer_on_self_best_finalized_id,
+		};
 
 	Ok(ClientState {
 		best_self: self_best_id,
@@ -444,7 +445,7 @@ where
 pub async fn best_finalized_peer_header_at_self<SelfChain, PeerChain>(
 	self_client: &Client<SelfChain>,
 	at_self_hash: HashOf<SelfChain>,
-) -> Result<HeaderIdOf<PeerChain>, SubstrateError>
+) -> Result<Option<HeaderIdOf<PeerChain>>, SubstrateError>
 where
 	SelfChain: Chain,
 	PeerChain: Chain,
@@ -456,8 +457,7 @@ where
 			(),
 			Some(at_self_hash),
 		)
-		.await?
-		.ok_or(SubstrateError::BridgePalletIsNotInitialized)
+		.await
 }
 
 fn validate_out_msgs_details<C: Chain>(

diff --git a/relays/lib-substrate-relay/src/on_demand/parachains.rs b/relays/lib-substrate-relay/src/on_demand/parachains.rs
@@ -422,11 +422,14 @@ struct RelayData<ParaHash, ParaNumber, RelayNumber> {
 	pub para_header_at_source: Option<HeaderId<ParaHash, ParaNumber>>,
 	/// Parachain header, that is available at the source relay chain at `relay_header_at_target`
 	/// block.
+	///
+	/// May be `None` if there's no `relay_header_at_target` yet, or if the
+	/// `relay_header_at_target` is too old and we think its state has been pruned.
 	pub para_header_at_relay_header_at_target: Option<HeaderId<ParaHash, ParaNumber>>,
 	/// Relay header number at the source chain.
 	pub relay_header_at_source: RelayNumber,
 	/// Relay header number at the target chain.
-	pub relay_header_at_target: RelayNumber,
+	pub relay_header_at_target: Option<RelayNumber>,
 }
 
 /// Read required data from source and target clients.
@@ -477,9 +480,8 @@ where
 	// submit at least one. Otherwise the pallet will be treated as uninitialized and messages
 	// sync will stall.
 	let para_header_at_target = match para_header_at_target {
-		Ok(para_header_at_target) => Some(para_header_at_target.0),
-		Err(SubstrateError::BridgePalletIsNotInitialized) |
-		Err(SubstrateError::NoParachainHeadAtTarget(_, _)) => None,
+		Ok(Some(para_header_at_target)) => Some(para_header_at_target.0),
+		Ok(None) => None,
 		Err(e) => return Err(map_target_err(e)),
 	};
 
@@ -502,51 +504,70 @@ where
 	.await
 	.map_err(map_target_err)?;
 
-	// if relay header at target is too old, then its state may already be discarded at the source
+	// if relay header at target is too old then its state may already be discarded at the source
 	// => just use `None` in this case
-	let is_relay_header_at_target_ancient =
-		is_ancient_block(relay_header_at_target.number(), relay_header_at_source);
-	let para_header_at_relay_header_at_target = if is_relay_header_at_target_ancient {
-		None
-	} else {
-		source
-			.on_chain_para_head_id(relay_header_at_target, P::SourceParachain::PARACHAIN_ID.into())
-			.await
-			.map_err(map_source_err)?
-	};
+	//
+	// the same is for case when there's no relay header at target at all
+	let available_relay_header_at_target =
+		relay_header_at_target.filter(|relay_header_at_target| {
+			!is_ancient_block(relay_header_at_target.number(), relay_header_at_source)
+		});
+	let para_header_at_relay_header_at_target =
+		if let Some(available_relay_header_at_target) = available_relay_header_at_target {
+			source
+				.on_chain_para_head_id(
+					available_relay_header_at_target,
+					P::SourceParachain::PARACHAIN_ID.into(),
+				)
+				.await
+				.map_err(map_source_err)?
+		} else {
+			None
+		};
 
 	Ok(RelayData {
 		required_para_header: required_header_number,
 		para_header_at_target,
 		para_header_at_source,
 		relay_header_at_source,
-		relay_header_at_target: relay_header_at_target.0,
+		relay_header_at_target: relay_header_at_target
+			.map(|relay_header_at_target| relay_header_at_target.0),
 		para_header_at_relay_header_at_target,
 	})
 }
 
 /// Select relay and parachain headers that need to be relayed.
 fn select_headers_to_relay<ParaHash, ParaNumber, RelayNumber>(
 	data: &RelayData<ParaHash, ParaNumber, RelayNumber>,
-	mut state: RelayState<ParaHash, ParaNumber, RelayNumber>,
+	state: RelayState<ParaHash, ParaNumber, RelayNumber>,
 ) -> RelayState<ParaHash, ParaNumber, RelayNumber>
 where
 	ParaHash: Clone,
 	ParaNumber: Copy + PartialOrd + Zero,
 	RelayNumber: Copy + Debug + Ord,
 {
+	// we can't do anything until **relay chain** bridge GRANDPA pallet is not initialized at the
+	// target chain
+	let relay_header_at_target = match data.relay_header_at_target {
+		Some(relay_header_at_target) => relay_header_at_target,
+		None => return RelayState::Idle,
+	};
+
 	// Process the `RelayingRelayHeader` state.
 	if let &RelayState::RelayingRelayHeader(relay_header_number) = &state {
-		if data.relay_header_at_target < relay_header_number {
+		if relay_header_at_target < relay_header_number {
 			// The required relay header hasn't yet been relayed. Ask / wait for it.
 			return state
 		}
 
 		// We may switch to `RelayingParaHeader` if parachain head is available.
-		state = data
-			.para_header_at_relay_header_at_target
-			.clone()
-			.map_or(RelayState::Idle, RelayState::RelayingParaHeader);
+		if let Some(para_header_at_relay_header_at_target) =
+			data.para_header_at_relay_header_at_target.as_ref()
+		{
+			return RelayState::RelayingParaHeader(para_header_at_relay_header_at_target.clone())
+		}
+
+		// else use the regular process - e.g. we may require to deliver new relay header first
 	}
 
 	// Process the `RelayingParaHeader` state.
@@ -585,7 +606,7 @@ where
 	// its ancestor
 
 	// we need relay chain header first
-	if data.relay_header_at_target < data.relay_header_at_source {
+	if relay_header_at_target < data.relay_header_at_source {
 		return RelayState::RelayingRelayHeader(data.relay_header_at_source)
 	}
 
@@ -640,7 +661,8 @@ impl<'a, P: SubstrateParachainsPipeline>
 			None,
 		)
 		.await?
-		.best_finalized_peer_at_best_self)
+		.best_finalized_peer_at_best_self
+		.ok_or(SubstrateError::BridgePalletIsNotInitialized)?)
 	}
 
 	async fn best_finalized_para_block_at_source(
@@ -726,7 +748,7 @@ mod tests {
 					para_header_at_target: Some(50),
 					para_header_at_source: Some(HeaderId(110, 110)),
 					relay_header_at_source: 800,
-					relay_header_at_target: 700,
+					relay_header_at_target: Some(700),
 					para_header_at_relay_header_at_target: Some(HeaderId(100, 100)),
 				},
 				RelayState::RelayingRelayHeader(750),
@@ -744,7 +766,7 @@ mod tests {
 					para_header_at_target: Some(50),
 					para_header_at_source: Some(HeaderId(110, 110)),
 					relay_header_at_source: 800,
-					relay_header_at_target: 750,
+					relay_header_at_target: Some(750),
 					para_header_at_relay_header_at_target: Some(HeaderId(100, 100)),
 				},
 				RelayState::RelayingRelayHeader(750),
@@ -762,7 +784,7 @@ mod tests {
 					para_header_at_target: Some(50),
 					para_header_at_source: Some(HeaderId(110, 110)),
 					relay_header_at_source: 800,
-					relay_header_at_target: 780,
+					relay_header_at_target: Some(780),
 					para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
 				},
 				RelayState::RelayingRelayHeader(750),
@@ -779,7 +801,7 @@ mod tests {
 					para_header_at_target: Some(50),
 					para_header_at_source: Some(HeaderId(110, 110)),
 					relay_header_at_source: 800,
-					relay_header_at_target: 780,
+					relay_header_at_target: Some(780),
 					para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
 				},
 				RelayState::RelayingParaHeader(HeaderId(105, 105)),
@@ -797,7 +819,7 @@ mod tests {
 					para_header_at_target: Some(105),
 					para_header_at_source: Some(HeaderId(110, 110)),
 					relay_header_at_source: 800,
-					relay_header_at_target: 780,
+					relay_header_at_target: Some(780),
 					para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
 				},
 				RelayState::Idle,
@@ -815,7 +837,7 @@ mod tests {
 					para_header_at_target: Some(105),
 					para_header_at_source: None,
 					relay_header_at_source: 800,
-					relay_header_at_target: 780,
+					relay_header_at_target: Some(780),
 					para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
 				},
 				RelayState::Idle,
@@ -833,7 +855,7 @@ mod tests {
 					para_header_at_target: Some(105),
 					para_header_at_source: Some(HeaderId(110, 110)),
 					relay_header_at_source: 800,
-					relay_header_at_target: 780,
+					relay_header_at_target: Some(780),
 					para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
 				},
 				RelayState::Idle,
@@ -851,7 +873,7 @@ mod tests {
 					para_header_at_target: Some(105),
 					para_header_at_source: Some(HeaderId(125, 125)),
 					relay_header_at_source: 800,
-					relay_header_at_target: 780,
+					relay_header_at_target: Some(780),
 					para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
 				},
 				RelayState::Idle,
@@ -869,7 +891,7 @@ mod tests {
 					para_header_at_target: Some(105),
 					para_header_at_source: Some(HeaderId(125, 125)),
 					relay_header_at_source: 800,
-					relay_header_at_target: 800,
+					relay_header_at_target: Some(800),
 					para_header_at_relay_header_at_target: Some(HeaderId(125, 125)),
 				},
 				RelayState::Idle,
@@ -887,7 +909,7 @@ mod tests {
 					para_header_at_target: Some(105),
 					para_header_at_source: None,
 					relay_header_at_source: 800,
-					relay_header_at_target: 800,
+					relay_header_at_target: Some(800),
 					para_header_at_relay_header_at_target: None,
 				},
 				RelayState::RelayingRelayHeader(800),
@@ -905,7 +927,7 @@ mod tests {
 					para_header_at_target: None,
 					para_header_at_source: Some(HeaderId(125, 125)),
 					relay_header_at_source: 800,
-					relay_header_at_target: 800,
+					relay_header_at_target: Some(800),
 					para_header_at_relay_header_at_target: Some(HeaderId(125, 125)),
 				},
 				RelayState::Idle,
@@ -923,7 +945,7 @@ mod tests {
 					para_header_at_target: None,
 					para_header_at_source: Some(HeaderId(125, 125)),
 					relay_header_at_source: 800,
-					relay_header_at_target: 700,
+					relay_header_at_target: Some(700),
 					para_header_at_relay_header_at_target: Some(HeaderId(125, 125)),
 				},
 				RelayState::Idle,