do not start spec_version guard when version mode is set to auto (#1278)

bridges/relays/bin-substrate/Cargo.toml

@@ -8,6 +8,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 [dependencies]
 anyhow = "1.0"
 async-std = "1.9.0"
+async-trait = "0.1.42"
 codec = { package = "parity-scale-codec", version = "2.2.0" }
 futures = "0.3.12"
 hex = "0.4"

bridges/relays/bin-substrate/src/chains/kusama_headers_to_polkadot.rs

@@ -16,7 +16,8 @@
 
 //! Kusama-to-Polkadot headers sync entrypoint.
 
-use sp_core::Pair;
+use async_trait::async_trait;
+use relay_polkadot_client::Polkadot;
 use substrate_relay_helper::{finality_pipeline::SubstrateFinalitySyncPipeline, TransactionParams};
 
 /// Maximal saturating difference between `balance(now)` and `balance(now-24h)` to treat
@@ -36,26 +37,26 @@ substrate_relay_helper::generate_mocked_submit_finality_proof_call_builder!(
 	relay_polkadot_client::runtime::BridgeKusamaGrandpaCall::submit_finality_proof
 );
 
+#[async_trait]
 impl SubstrateFinalitySyncPipeline for KusamaFinalityToPolkadot {
 	type SourceChain = relay_kusama_client::Kusama;
-	type TargetChain = relay_polkadot_client::Polkadot;
+	type TargetChain = Polkadot;
 
 	type SubmitFinalityProofCallBuilder = KusamaFinalityToPolkadotCallBuilder;
-	type TransactionSignScheme = relay_polkadot_client::Polkadot;
+	type TransactionSignScheme = Polkadot;
 
-	fn start_relay_guards(
-		target_client: &relay_substrate_client::Client<relay_polkadot_client::Polkadot>,
+	async fn start_relay_guards(
+		target_client: &relay_substrate_client::Client<Polkadot>,
 		transaction_params: &TransactionParams<sp_core::sr25519::Pair>,
-	) {
-		relay_substrate_client::guard::abort_on_spec_version_change(
-			target_client.clone(),
-			bp_polkadot::VERSION.spec_version,
-		);
-		relay_substrate_client::guard::abort_when_account_balance_decreased(
-			target_client.clone(),
-			transaction_params.signer.public().into(),
+		enable_version_guard: bool,
+	) -> relay_substrate_client::Result<()> {
+		substrate_relay_helper::finality_guards::start::<Polkadot, Polkadot>(
+			target_client,
+			transaction_params,
+			enable_version_guard,
 			MAXIMAL_BALANCE_DECREASE_PER_DAY,
-		);
+		)
+		.await
 	}
 }
 

bridges/relays/bin-substrate/src/chains/polkadot_headers_to_kusama.rs

@@ -16,7 +16,8 @@
 
 //! Polkadot-to-Kusama headers sync entrypoint.
 
-use sp_core::Pair;
+use async_trait::async_trait;
+use relay_kusama_client::Kusama;
 use substrate_relay_helper::{finality_pipeline::SubstrateFinalitySyncPipeline, TransactionParams};
 
 /// Maximal saturating difference between `balance(now)` and `balance(now-24h)` to treat
@@ -36,26 +37,26 @@ substrate_relay_helper::generate_mocked_submit_finality_proof_call_builder!(
 	relay_kusama_client::runtime::BridgePolkadotGrandpaCall::submit_finality_proof
 );
 
+#[async_trait]
 impl SubstrateFinalitySyncPipeline for PolkadotFinalityToKusama {
 	type SourceChain = relay_polkadot_client::Polkadot;
-	type TargetChain = relay_kusama_client::Kusama;
+	type TargetChain = Kusama;
 
 	type SubmitFinalityProofCallBuilder = PolkadotFinalityToKusamaCallBuilder;
-	type TransactionSignScheme = relay_kusama_client::Kusama;
+	type TransactionSignScheme = Kusama;
 
-	fn start_relay_guards(
-		target_client: &relay_substrate_client::Client<relay_kusama_client::Kusama>,
+	async fn start_relay_guards(
+		target_client: &relay_substrate_client::Client<Kusama>,
 		transaction_params: &TransactionParams<sp_core::sr25519::Pair>,
-	) {
-		relay_substrate_client::guard::abort_on_spec_version_change(
-			target_client.clone(),
-			bp_kusama::VERSION.spec_version,
-		);
-		relay_substrate_client::guard::abort_when_account_balance_decreased(
-			target_client.clone(),
-			transaction_params.signer.public().into(),
+		enable_version_guard: bool,
+	) -> relay_substrate_client::Result<()> {
+		substrate_relay_helper::finality_guards::start::<Kusama, Kusama>(
+			target_client,
+			transaction_params,
+			enable_version_guard,
 			MAXIMAL_BALANCE_DECREASE_PER_DAY,
-		);
+		)
+		.await
 	}
 }
 

bridges/relays/bin-substrate/src/chains/rococo_headers_to_wococo.rs

@@ -18,7 +18,8 @@
 
 use crate::chains::wococo_headers_to_rococo::MAXIMAL_BALANCE_DECREASE_PER_DAY;
 
-use sp_core::Pair;
+use async_trait::async_trait;
+use relay_wococo_client::Wococo;
 use substrate_relay_helper::{finality_pipeline::SubstrateFinalitySyncPipeline, TransactionParams};
 
 /// Description of Rococo -> Wococo finalized headers bridge.
@@ -31,25 +32,25 @@ substrate_relay_helper::generate_mocked_submit_finality_proof_call_builder!(
 	relay_wococo_client::runtime::BridgeGrandpaRococoCall::submit_finality_proof
 );
 
+#[async_trait]
 impl SubstrateFinalitySyncPipeline for RococoFinalityToWococo {
 	type SourceChain = relay_rococo_client::Rococo;
-	type TargetChain = relay_wococo_client::Wococo;
+	type TargetChain = Wococo;
 
 	type SubmitFinalityProofCallBuilder = RococoFinalityToWococoCallBuilder;
-	type TransactionSignScheme = relay_wococo_client::Wococo;
+	type TransactionSignScheme = Wococo;
 
-	fn start_relay_guards(
-		target_client: &relay_substrate_client::Client<relay_wococo_client::Wococo>,
+	async fn start_relay_guards(
+		target_client: &relay_substrate_client::Client<Wococo>,
 		transaction_params: &TransactionParams<sp_core::sr25519::Pair>,
-	) {
-		relay_substrate_client::guard::abort_on_spec_version_change(
-			target_client.clone(),
-			bp_wococo::VERSION.spec_version,
-		);
-		relay_substrate_client::guard::abort_when_account_balance_decreased(
-			target_client.clone(),
-			transaction_params.signer.public().into(),
+		enable_version_guard: bool,
+	) -> relay_substrate_client::Result<()> {
+		substrate_relay_helper::finality_guards::start::<Wococo, Wococo>(
+			target_client,
+			transaction_params,
+			enable_version_guard,
 			MAXIMAL_BALANCE_DECREASE_PER_DAY,
-		);
+		)
+		.await
 	}
 }

bridges/relays/bin-substrate/src/chains/wococo_headers_to_rococo.rs

@@ -16,7 +16,8 @@
 
 //! Wococo-to-Rococo headers sync entrypoint.
 
-use sp_core::Pair;
+use async_trait::async_trait;
+use relay_rococo_client::Rococo;
 use substrate_relay_helper::{finality_pipeline::SubstrateFinalitySyncPipeline, TransactionParams};
 
 /// Maximal saturating difference between `balance(now)` and `balance(now-24h)` to treat
@@ -36,26 +37,26 @@ substrate_relay_helper::generate_mocked_submit_finality_proof_call_builder!(
 	relay_rococo_client::runtime::BridgeGrandpaWococoCall::submit_finality_proof
 );
 
+#[async_trait]
 impl SubstrateFinalitySyncPipeline for WococoFinalityToRococo {
 	type SourceChain = relay_wococo_client::Wococo;
-	type TargetChain = relay_rococo_client::Rococo;
+	type TargetChain = Rococo;
 
 	type SubmitFinalityProofCallBuilder = WococoFinalityToRococoCallBuilder;
-	type TransactionSignScheme = relay_rococo_client::Rococo;
+	type TransactionSignScheme = Rococo;
 
-	fn start_relay_guards(
-		target_client: &relay_substrate_client::Client<relay_rococo_client::Rococo>,
+	async fn start_relay_guards(
+		target_client: &relay_substrate_client::Client<Rococo>,
 		transaction_params: &TransactionParams<sp_core::sr25519::Pair>,
-	) {
-		relay_substrate_client::guard::abort_on_spec_version_change(
-			target_client.clone(),
-			bp_rococo::VERSION.spec_version,
-		);
-		relay_substrate_client::guard::abort_when_account_balance_decreased(
-			target_client.clone(),
-			transaction_params.signer.public().into(),
+		enable_version_guard: bool,
+	) -> relay_substrate_client::Result<()> {
+		substrate_relay_helper::finality_guards::start::<Rococo, Rococo>(
+			target_client,
+			transaction_params,
+			enable_version_guard,
 			MAXIMAL_BALANCE_DECREASE_PER_DAY,
-		);
+		)
+		.await
 	}
 }
 

bridges/relays/bin-substrate/src/cli/mod.rs

@@ -528,6 +528,16 @@ macro_rules! declare_chain_options {
 			}
 
 			impl [<$chain ConnectionParams>] {
+				/// Returns `true` if version guard can be started.
+				///
+				/// There's no reason to run version guard when version mode is set to `Auto`. It can
+				/// lead to relay shutdown when chain is upgraded, even though we have explicitly
+				/// said that we don't want to shutdown.
+				#[allow(dead_code)]
+				pub fn can_start_version_guard(&self) -> bool {
+					self.[<$chain_prefix _runtime_version>].[<$chain_prefix _version_mode>] != RuntimeVersionType::Auto
+				}
+
 				/// Convert connection params into Substrate client.
 				pub async fn to_client<Chain: CliChain>(
 					&self,

bridges/relays/bin-substrate/src/cli/relay_headers.rs

@@ -157,7 +157,12 @@ impl RelayHeaders {
 				signer: target_sign,
 				mortality: target_transactions_mortality,
 			};
-			Finality::start_relay_guards(&target_client, &target_transactions_params);
+			Finality::start_relay_guards(
+				&target_client,
+				&target_transactions_params,
+				self.target.can_start_version_guard(),
+			)
+			.await?;
 
 			substrate_relay_helper::finality_pipeline::run::<Finality>(
 				source_client,

bridges/relays/bin-substrate/src/cli/relay_headers_and_messages.rs

@@ -512,11 +512,15 @@ impl RelayHeadersAndMessages {
 			LeftToRightFinality::start_relay_guards(
 				&right_client,
 				&left_to_right_transaction_params,
-			);
+				params.right.can_start_version_guard(),
+			)
+			.await?;
 			RightToLeftFinality::start_relay_guards(
 				&left_client,
 				&right_to_left_transaction_params,
-			);
+				params.left.can_start_version_guard(),
+			)
+			.await?;
 			let left_to_right_on_demand_headers = OnDemandHeadersRelay::new::<LeftToRightFinality>(
 				left_client.clone(),
 				right_client.clone(),

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

@@ -64,6 +64,13 @@ pub fn abort_on_spec_version_change<C: ChainWithBalances>(
 	expected_spec_version: u32,
 ) {
 	async_std::task::spawn(async move {
+		log::info!(
+			target: "bridge-guard",
+			"Starting spec_version guard for {}. Expected spec_version: {}",
+			C::NAME,
+			expected_spec_version,
+		);
+
 		loop {
 			let actual_spec_version = env.runtime_version().await;
 			match actual_spec_version {
@@ -103,6 +110,14 @@ pub fn abort_when_account_balance_decreased<C: ChainWithBalances>(
 	const DAY: Duration = Duration::from_secs(60 * 60 * 24);
 
 	async_std::task::spawn(async move {
+		log::info!(
+			target: "bridge-guard",
+			"Starting balance guard for {}/{:?}. Maximal decrease: {:?}",
+			C::NAME,
+			account_id,
+			maximal_decrease,
+		);
+
 		let mut balances = VecDeque::new();
 
 		loop {

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

@@ -0,0 +1,48 @@
+// Copyright 2019-2021 Parity Technologies (UK) Ltd.
+// This file is part of Parity Bridges Common.
+
+// Parity Bridges Common is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity Bridges Common is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity Bridges Common.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Tools for starting guards of finality relays.
+
+use crate::TransactionParams;
+
+use relay_substrate_client::{
+	AccountIdOf, AccountKeyPairOf, ChainWithBalances, TransactionSignScheme,
+};
+use sp_core::Pair;
+
+/// Start finality relay guards.
+pub async fn start<C: ChainWithBalances, S: TransactionSignScheme<Chain = C>>(
+	target_client: &relay_substrate_client::Client<C>,
+	transaction_params: &TransactionParams<S::AccountKeyPair>,
+	enable_version_guard: bool,
+	maximal_balance_decrease_per_day: C::Balance,
+) -> relay_substrate_client::Result<()>
+where
+	AccountIdOf<C>: From<<AccountKeyPairOf<S> as Pair>::Public>,
+{
+	if enable_version_guard {
+		relay_substrate_client::guard::abort_on_spec_version_change(
+			target_client.clone(),
+			target_client.simple_runtime_version().await?.0,
+		);
+	}
+	relay_substrate_client::guard::abort_when_account_balance_decreased(
+		target_client.clone(),
+		transaction_params.signer.public().into(),
+		maximal_balance_decrease_per_day,
+	);
+	Ok(())
+}

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

@@ -22,6 +22,7 @@ use crate::{
 	TransactionParams,
 };
 
+use async_trait::async_trait;
 use bp_header_chain::justification::GrandpaJustification;
 use finality_relay::FinalitySyncPipeline;
 use pallet_bridge_grandpa::{Call as BridgeGrandpaCall, Config as BridgeGrandpaConfig};
@@ -40,6 +41,7 @@ use std::{fmt::Debug, marker::PhantomData};
 pub(crate) const RECENT_FINALITY_PROOFS_LIMIT: usize = 4096;
 
 /// Substrate -> Substrate finality proofs synchronization pipeline.
+#[async_trait]
 pub trait SubstrateFinalitySyncPipeline: 'static + Clone + Debug + Send + Sync {
 	/// Headers of this chain are submitted to the `TargetChain`.
 	type SourceChain: Chain;
@@ -52,10 +54,12 @@ pub trait SubstrateFinalitySyncPipeline: 'static + Clone + Debug + Send + Sync {
 	type TransactionSignScheme: TransactionSignScheme;
 
 	/// Add relay guards if required.
-	fn start_relay_guards(
+	async fn start_relay_guards(
 		_target_client: &Client<Self::TargetChain>,
 		_transaction_params: &TransactionParams<AccountKeyPairOf<Self::TransactionSignScheme>>,
-	) {
+		_enable_version_guard: bool,
+	) -> relay_substrate_client::Result<()> {
+		Ok(())
 	}
 }
 

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

@@ -22,6 +22,7 @@ use std::time::Duration;
 
 pub mod conversion_rate_update;
 pub mod error;
+pub mod finality_guards;
 pub mod finality_pipeline;
 pub mod finality_source;
 pub mod finality_target;