Fix on-different-forks metrics during initialization (paritytech#1468)

* fix on-different-forks metrics during initialization

* "initialize" parachain finality pallet in on-demand parachains relay

* decrease converstion rate requests count

* more error logging

* fix compilation

* clippy

bridges/README.md

@@ -216,7 +216,7 @@ To run a Rialto node for example, you can use the following command:
 
 ```bash
 docker run -p 30333:30333 -p 9933:9933 -p 9944:9944 \
-  -it paritytech/rialto-bridge-node --dev --tmp \
+  -it local/rialto-bridge-node --dev --tmp \
   --rpc-cors=all --unsafe-rpc-external --unsafe-ws-external
 ```
 

bridges/bin/millau/runtime/src/lib.rs

@@ -795,33 +795,28 @@ impl_runtime_apis! {
 	}
 
 	impl bp_rialto::RialtoFinalityApi<Block> for Runtime {
-		fn best_finalized() -> (bp_rialto::BlockNumber, bp_rialto::Hash) {
-			let header = BridgeRialtoGrandpa::best_finalized();
-			(header.number, header.hash())
+		fn best_finalized() -> Option<(bp_rialto::BlockNumber, bp_rialto::Hash)> {
+			BridgeRialtoGrandpa::best_finalized().map(|header| (header.number, header.hash()))
 		}
 	}
 
 	impl bp_westend::WestendFinalityApi<Block> for Runtime {
-		fn best_finalized() -> (bp_westend::BlockNumber, bp_westend::Hash) {
-			let header = BridgeWestendGrandpa::best_finalized();
-			(header.number, header.hash())
+		fn best_finalized() -> Option<(bp_westend::BlockNumber, bp_westend::Hash)> {
+			BridgeWestendGrandpa::best_finalized().map(|header| (header.number, header.hash()))
 		}
 	}
 
 	impl bp_rialto_parachain::RialtoParachainFinalityApi<Block> for Runtime {
-		fn best_finalized() -> (bp_rialto::BlockNumber, bp_rialto::Hash) {
+		fn best_finalized() -> Option<(bp_rialto::BlockNumber, bp_rialto::Hash)> {
 			// the parachains finality pallet is never decoding parachain heads, so it is
 			// only done in the integration code
 			use bp_rialto_parachain::RIALTO_PARACHAIN_ID;
-			let best_rialto_parachain_head = pallet_bridge_parachains::Pallet::<
+			let encoded_head = pallet_bridge_parachains::Pallet::<
 				Runtime,
 				WithRialtoParachainsInstance,
-			>::best_parachain_head(RIALTO_PARACHAIN_ID.into())
-				.and_then(|encoded_header| bp_rialto_parachain::Header::decode(&mut &encoded_header.0[..]).ok());
-			match best_rialto_parachain_head {
-				Some(head) => (*head.number(), head.hash()),
-				None => (Default::default(), Default::default()),
-			}
+			>::best_parachain_head(RIALTO_PARACHAIN_ID.into())?;
+			let head = bp_rialto_parachain::Header::decode(&mut &encoded_head.0[..]).ok()?;
+			Some((*head.number(), head.hash()))
 		}
 	}
 

bridges/bin/rialto-parachain/runtime/src/lib.rs

@@ -683,9 +683,8 @@ impl_runtime_apis! {
 	}
 
 	impl bp_millau::MillauFinalityApi<Block> for Runtime {
-		fn best_finalized() -> (bp_millau::BlockNumber, bp_millau::Hash) {
-			let header = BridgeMillauGrandpa::best_finalized();
-			(header.number, header.hash())
+		fn best_finalized() -> Option<(bp_millau::BlockNumber, bp_millau::Hash)> {
+			BridgeMillauGrandpa::best_finalized().map(|header| (header.number, header.hash()))
 		}
 	}
 

bridges/bin/rialto/runtime/src/lib.rs

@@ -640,9 +640,8 @@ impl_runtime_apis! {
 	}
 
 	impl bp_millau::MillauFinalityApi<Block> for Runtime {
-		fn best_finalized() -> (bp_millau::BlockNumber, bp_millau::Hash) {
-			let header = BridgeMillauGrandpa::best_finalized();
-			(header.number, header.hash())
+		fn best_finalized() -> Option<(bp_millau::BlockNumber, bp_millau::Hash)> {
+			BridgeMillauGrandpa::best_finalized().map(|header| (header.number, header.hash()))
 		}
 	}
 

bridges/modules/grandpa/src/lib.rs

@@ -537,17 +537,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	///
 	/// Returns a dummy header if there is no best header. This can only happen
 	/// if the pallet has not been initialized yet.
-	pub fn best_finalized() -> BridgedHeader<T, I> {
+	pub fn best_finalized() -> Option<BridgedHeader<T, I>> {
 		let hash = <BestFinalized<T, I>>::get();
-		<ImportedHeaders<T, I>>::get(hash).unwrap_or_else(|| {
-			<BridgedHeader<T, I>>::new(
-				Default::default(),
-				Default::default(),
-				Default::default(),
-				Default::default(),
-				Default::default(),
-			)
-		})
+		<ImportedHeaders<T, I>>::get(hash)
 	}
 
 	/// Check if a particular header is known to the bridge pallet.
@@ -718,7 +710,7 @@ mod tests {
 				BestFinalized::<TestRuntime>::get(),
 				BridgedBlockHash::<TestRuntime, ()>::default()
 			);
-			assert_eq!(Pallet::<TestRuntime>::best_finalized(), test_header(0));
+			assert_eq!(Pallet::<TestRuntime>::best_finalized(), None);
 
 			let init_data = init_with_origin(Origin::root()).unwrap();
 
@@ -1131,7 +1123,7 @@ mod tests {
 		run_test(|| {
 			initialize_substrate_bridge();
 			assert_ok!(submit_finality_proof(1));
-			let first_header = Pallet::<TestRuntime>::best_finalized();
+			let first_header = Pallet::<TestRuntime>::best_finalized().unwrap();
 			next_block();
 
 			assert_ok!(submit_finality_proof(2));

bridges/primitives/chain-kusama/src/lib.rs

@@ -117,7 +117,7 @@ sp_api::decl_runtime_apis! {
 	/// Kusama runtime itself.
 	pub trait KusamaFinalityApi {
 		/// Returns number and hash of the best finalized header known to the bridge module.
-		fn best_finalized() -> (BlockNumber, Hash);
+		fn best_finalized() -> Option<(BlockNumber, Hash)>;
 	}
 
 	/// Outbound message lane API for messages that are sent to Kusama chain.

bridges/primitives/chain-millau/src/lib.rs

@@ -314,7 +314,7 @@ sp_api::decl_runtime_apis! {
 	/// Millau runtime itself.
 	pub trait MillauFinalityApi {
 		/// Returns number and hash of the best finalized header known to the bridge module.
-		fn best_finalized() -> (BlockNumber, Hash);
+		fn best_finalized() -> Option<(BlockNumber, Hash)>;
 	}
 
 	/// Outbound message lane API for messages that are sent to Millau chain.

bridges/primitives/chain-polkadot/src/lib.rs

@@ -118,7 +118,7 @@ sp_api::decl_runtime_apis! {
 	/// Polkadot runtime itself.
 	pub trait PolkadotFinalityApi {
 		/// Returns number and hash of the best finalized header known to the bridge module.
-		fn best_finalized() -> (BlockNumber, Hash);
+		fn best_finalized() -> Option<(BlockNumber, Hash)>;
 	}
 
 	/// Outbound message lane API for messages that are sent to Polkadot chain.

bridges/primitives/chain-rialto-parachain/src/lib.rs

@@ -241,7 +241,7 @@ sp_api::decl_runtime_apis! {
 	/// RialtoParachain runtime itself.
 	pub trait RialtoParachainFinalityApi {
 		/// Returns number and hash of the best finalized header known to the bridge module.
-		fn best_finalized() -> (BlockNumber, Hash);
+		fn best_finalized() -> Option<(BlockNumber, Hash)>;
 	}
 
 	/// Outbound message lane API for messages that are sent to RialtoParachain chain.

bridges/primitives/chain-rialto/src/lib.rs

@@ -262,7 +262,7 @@ sp_api::decl_runtime_apis! {
 	/// Rialto runtime itself.
 	pub trait RialtoFinalityApi {
 		/// Returns number and hash of the best finalized header known to the bridge module.
-		fn best_finalized() -> (BlockNumber, Hash);
+		fn best_finalized() -> Option<(BlockNumber, Hash)>;
 	}
 
 	/// Outbound message lane API for messages that are sent to Rialto chain.

bridges/primitives/chain-rococo/src/lib.rs

@@ -113,7 +113,7 @@ sp_api::decl_runtime_apis! {
 	/// Rococo runtime itself.
 	pub trait RococoFinalityApi {
 		/// Returns number and hash of the best finalized header known to the bridge module.
-		fn best_finalized() -> (BlockNumber, Hash);
+		fn best_finalized() -> Option<(BlockNumber, Hash)>;
 	}
 
 	/// Outbound message lane API for messages that are sent to Rococo chain.

bridges/primitives/chain-westend/src/lib.rs

@@ -106,6 +106,6 @@ sp_api::decl_runtime_apis! {
 	/// Westend runtime itself.
 	pub trait WestendFinalityApi {
 		/// Returns number and hash of the best finalized header known to the bridge module.
-		fn best_finalized() -> (BlockNumber, Hash);
+		fn best_finalized() -> Option<(BlockNumber, Hash)>;
 	}
 }

bridges/primitives/chain-wococo/src/lib.rs

@@ -70,7 +70,7 @@ sp_api::decl_runtime_apis! {
 	/// Wococo runtime itself.
 	pub trait WococoFinalityApi {
 		/// Returns number and hash of the best finalized header known to the bridge module.
-		fn best_finalized() -> (BlockNumber, Hash);
+		fn best_finalized() -> Option<(BlockNumber, Hash)>;
 	}
 
 	/// Outbound message lane API for messages that are sent to Wococo chain.

bridges/primitives/header-chain/src/lib.rs

@@ -88,7 +88,7 @@ pub trait InclusionProofVerifier {
 /// A trait for pallets which want to keep track of finalized headers from a bridged chain.
 pub trait HeaderChain<H, E> {
 	/// Get the best finalized header known to the header chain.
-	fn best_finalized() -> H;
+	fn best_finalized() -> Option<H>;
 
 	/// Get the best authority set known to the header chain.
 	fn authority_set() -> AuthoritySet;
@@ -98,8 +98,8 @@ pub trait HeaderChain<H, E> {
 }
 
 impl<H: Default, E> HeaderChain<H, E> for () {
-	fn best_finalized() -> H {
-		H::default()
+	fn best_finalized() -> Option<H> {
+		None
 	}
 
 	fn authority_set() -> AuthoritySet {

bridges/relays/client-substrate/src/client.rs

@@ -456,8 +456,12 @@ impl<C: Chain> Client<C> {
 				IndexOf<C>,
 				C::SignedBlock,
 			>::author_submit_extrinsic(&*client, transaction)
-			.await?;
-			log::trace!(target: "bridge", "Sent transaction to Substrate node: {:?}", tx_hash);
+			.await
+			.map_err(|e| {
+				log::error!(target: "bridge", "Failed to send transaction to {} node: {:?}", C::NAME, e);
+				e
+			})?;
+			log::trace!(target: "bridge", "Sent transaction to {} node: {:?}", C::NAME, tx_hash);
 			Ok(tx_hash)
 		})
 		.await
@@ -499,7 +503,11 @@ impl<C: Chain> Client<C> {
 				IndexOf<C>,
 				C::SignedBlock,
 			>::author_submit_extrinsic(&*client, extrinsic)
-			.await?;
+			.await
+			.map_err(|e| {
+				log::error!(target: "bridge", "Failed to send transaction to {} node: {:?}", C::NAME, e);
+				e
+			})?;
 			log::trace!(target: "bridge", "Sent transaction to {} node: {:?}", C::NAME, tx_hash);
 			Ok(tx_hash)
 		})
@@ -528,7 +536,11 @@ impl<C: Chain> Client<C> {
 							.map_err(|e| Error::RpcError(e.into()))?])),
 						"author_unwatchExtrinsic",
 					)
-					.await?;
+					.await
+					.map_err(|e| {
+						log::error!(target: "bridge", "Failed to send transaction to {} node: {:?}", C::NAME, e);
+						e
+					})?;
 				log::trace!(target: "bridge", "Sent transaction to {} node: {:?}", C::NAME, tx_hash);
 				Ok(subscription)
 			})

bridges/relays/lib-substrate-relay/src/error.rs

@@ -58,4 +58,9 @@ pub enum Error<Hash: Debug + MaybeDisplay, HeaderNumber: Debug + MaybeDisplay> {
 	/// Failed to retrieve best finalized source header hash from the target chain.
 	#[error("Failed to retrieve best finalized {0} header from the target chain: {1}")]
 	RetrieveBestFinalizedHeaderHash(&'static str, client::Error),
+	/// Failed to submit signed extrinsic from to the target chain.
+	#[error(
+		"Failed to retrieve `is_initialized` flag of the with-{0} finality pallet at {1}: {2:?}"
+	)]
+	IsInitializedRetrieve(&'static str, &'static str, client::Error),
 }

bridges/relays/lib-substrate-relay/src/finality/engine.rs

@@ -37,7 +37,7 @@ use std::marker::PhantomData;
 
 /// Finality enfine, used by the Substrate chain.
 #[async_trait]
-pub trait Engine<C: Chain> {
+pub trait Engine<C: Chain>: Send {
 	/// Unique consensus engine identifier.
 	const ID: ConsensusEngineId;
 	/// Type of finality proofs, used by consensus engine.
@@ -59,6 +59,23 @@ pub trait Engine<C: Chain> {
 	async fn prepare_initialization_data(
 		client: Client<C>,
 	) -> Result<Self::InitializationData, Error<HashOf<C>, BlockNumberOf<C>>>;
+
+	/// Returns `Ok(true)` if finality pallet at the bridged chain has already been initialized.
+	async fn is_initialized<TargetChain: Chain>(
+		target_client: &Client<TargetChain>,
+	) -> Result<bool, SubstrateError> {
+		Ok(target_client
+			.raw_storage_value(Self::is_initialized_key(), None)
+			.await?
+			.is_some())
+	}
+
+	/// Returns `Ok(true)` if finality pallet at the bridged chain is halted.
+	async fn is_halted<TargetChain: Chain>(
+		target_client: &Client<TargetChain>,
+	) -> Result<bool, SubstrateError> {
+		Ok(target_client.storage_value(Self::is_halted_key(), None).await?.unwrap_or(false))
+	}
 }
 
 /// GRANDPA finality engine.

bridges/relays/lib-substrate-relay/src/finality/initialize.rs

@@ -23,7 +23,7 @@
 
 use crate::{error::Error, finality::engine::Engine};
 
-use relay_substrate_client::{BlockNumberOf, Chain, Client, Error as SubstrateError, HashOf};
+use relay_substrate_client::{Chain, Client, Error as SubstrateError};
 use sp_core::Bytes;
 use sp_runtime::traits::Header as HeaderT;
 
@@ -80,7 +80,9 @@ where
 		+ Send
 		+ 'static,
 {
-	let is_initialized = is_initialized::<E, SourceChain, TargetChain>(&target_client).await?;
+	let is_initialized = E::is_initialized(&target_client)
+		.await
+		.map_err(|e| Error::IsInitializedRetrieve(SourceChain::NAME, TargetChain::NAME, e))?;
 	if is_initialized {
 		log::info!(
 			target: "bridge",
@@ -108,18 +110,3 @@ where
 		.map_err(|err| Error::SubmitTransaction(TargetChain::NAME, err))?;
 	Ok(Some(initialization_tx_hash))
 }
-
-/// Returns `Ok(true)` if bridge has already been initialized.
-pub(crate) async fn is_initialized<
-	E: Engine<SourceChain>,
-	SourceChain: Chain,
-	TargetChain: Chain,
->(
-	target_client: &Client<TargetChain>,
-) -> Result<bool, Error<HashOf<SourceChain>, BlockNumberOf<SourceChain>>> {
-	Ok(target_client
-		.raw_storage_value(E::is_initialized_key(), None)
-		.await
-		.map_err(|err| Error::RetrieveBestFinalizedHeaderHash(SourceChain::NAME, err))?
-		.is_some())
-}

bridges/relays/lib-substrate-relay/src/finality/target.rs

@@ -51,15 +51,12 @@ impl<P: SubstrateFinalitySyncPipeline> SubstrateFinalityTarget<P> {
 
 	/// Ensure that the bridge pallet at target chain is active.
 	pub async fn ensure_pallet_active(&self) -> Result<(), Error> {
-		let is_halted = self.client.storage_value(P::FinalityEngine::is_halted_key(), None).await?;
-		if is_halted.unwrap_or(false) {
+		let is_halted = P::FinalityEngine::is_halted(&self.client).await?;
+		if is_halted {
 			return Err(Error::BridgePalletIsHalted)
 		}
 
-		let is_initialized =
-			super::initialize::is_initialized::<P::FinalityEngine, P::SourceChain, P::TargetChain>(
-				&self.client,
-			)
+		let is_initialized = P::FinalityEngine::is_initialized(&self.client)
 			.await
 			.map_err(|e| Error::Custom(e.to_string()))?;
 		if !is_initialized {

bridges/relays/lib-substrate-relay/src/messages_source.rs

@@ -146,7 +146,11 @@ where
 	async fn state(&self) -> Result<SourceClientState<MessageLaneAdapter<P>>, SubstrateError> {
 		// we can't continue to deliver confirmations if source node is out of sync, because
 		// it may have already received confirmations that we're going to deliver
+		//
+		// we can't continue to deliver messages if target node is out of sync, because
+		// it may have already received (some of) messages that we're going to deliver
 		self.source_client.ensure_synced().await?;
+		self.target_client.ensure_synced().await?;
 		// we can't relay confirmations if messages pallet at source chain is halted
 		self.ensure_pallet_active().await?;
 
@@ -562,9 +566,13 @@ where
 			Some(at_self_hash),
 		)
 		.await?;
-	let decoded_best_finalized_peer_on_self: (BlockNumberOf<PeerChain>, HashOf<PeerChain>) =
-		Decode::decode(&mut &encoded_best_finalized_peer_on_self.0[..])
-			.map_err(SubstrateError::ResponseParseFailed)?;
+	let decoded_best_finalized_peer_on_self =
+		Option::<(BlockNumberOf<PeerChain>, HashOf<PeerChain>)>::decode(
+			&mut &encoded_best_finalized_peer_on_self.0[..],
+		)
+		.map_err(SubstrateError::ResponseParseFailed)?
+		.map(Ok)
+		.unwrap_or(Err(SubstrateError::BridgePalletIsNotInitialized))?;
 	let peer_on_self_best_finalized_id =
 		HeaderId(decoded_best_finalized_peer_on_self.0, decoded_best_finalized_peer_on_self.1);
 

bridges/relays/lib-substrate-relay/src/messages_target.rs

@@ -146,8 +146,12 @@ where
 	BalanceOf<P::SourceChain>: TryFrom<BalanceOf<P::TargetChain>>,
 {
 	async fn state(&self) -> Result<TargetClientState<MessageLaneAdapter<P>>, SubstrateError> {
+		// we can't continue to deliver confirmations if source node is out of sync, because
+		// it may have already received confirmations that we're going to deliver
+		//
 		// we can't continue to deliver messages if target node is out of sync, because
 		// it may have already received (some of) messages that we're going to deliver
+		self.source_client.ensure_synced().await?;
 		self.target_client.ensure_synced().await?;
 		// we can't relay messages if messages pallet at target chain is halted
 		self.ensure_pallet_active().await?;

bridges/relays/lib-substrate-relay/src/on_demand/headers.rs

@@ -253,7 +253,7 @@ async fn background_task<P: SubstrateFinalitySyncPipeline>(
 
 			log::info!(
 				target: "bridge",
-				"[{}] Starting on-demand relay task\n\t\
+				"[{}] Starting on-demand headers relay task\n\t\
 					Only mandatory headers: {}\n\t\
 					Tx mortality: {:?} (~{}m)\n\t\
 					Stall timeout: {:?}",