verify that GRANDPA pallet is not initialized before submitting initialization transaction (#1267)

* verify that GRANDPA pallet is not initialized before submitting initialization transaction

* spelling

bridges/modules/grandpa/src/lib.rs

@@ -626,7 +626,10 @@ mod tests {
 		JustificationGeneratorParams, ALICE, BOB,
 	};
 	use codec::Encode;
-	use frame_support::{assert_err, assert_noop, assert_ok, weights::PostDispatchInfo};
+	use frame_support::{
+		assert_err, assert_noop, assert_ok, storage::generator::StorageValue,
+		weights::PostDispatchInfo,
+	};
 	use sp_runtime::{Digest, DigestItem, DispatchError};
 
 	fn initialize_substrate_bridge() {
@@ -1145,4 +1148,12 @@ mod tests {
 			);
 		})
 	}
+
+	#[test]
+	fn storage_keys_computed_properly() {
+		assert_eq!(
+			BestFinalized::<TestRuntime>::storage_value_final_key().to_vec(),
+			bp_header_chain::storage_keys::best_finalized_hash_key("Grandpa").0,
+		);
+	}
 }

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

@@ -82,6 +82,8 @@ pub const EXISTENTIAL_DEPOSIT: Balance = 1_000_000_000_000 / 30_000;
 /// conditions.
 pub const SESSION_LENGTH: BlockNumber = time_units::HOURS;
 
+/// Name of the With-Kusama GRANDPA pallet instance that is deployed at bridged chains.
+pub const WITH_KUSAMA_GRANDPA_PALLET_NAME: &str = "BridgeKusamaGrandpa";
 /// Name of the With-Kusama messages pallet instance that is deployed at bridged chains.
 pub const WITH_KUSAMA_MESSAGES_PALLET_NAME: &str = "BridgeKusamaMessages";
 

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

@@ -257,6 +257,8 @@ frame_support::parameter_types! {
 		.build_or_panic();
 }
 
+/// Name of the With-Millau GRANDPA pallet instance that is deployed at bridged chains.
+pub const WITH_MILLAU_GRANDPA_PALLET_NAME: &str = "BridgeMillauGrandpa";
 /// Name of the With-Millau messages pallet instance that is deployed at bridged chains.
 pub const WITH_MILLAU_MESSAGES_PALLET_NAME: &str = "BridgeMillauMessages";
 

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

@@ -82,6 +82,8 @@ pub const EXISTENTIAL_DEPOSIT: Balance = 10_000_000_000;
 /// conditions.
 pub const SESSION_LENGTH: BlockNumber = 4 * time_units::HOURS;
 
+/// Name of the With-Polkadot GRANDPA pallet instance that is deployed at bridged chains.
+pub const WITH_POLKADOT_GRANDPA_PALLET_NAME: &str = "BridgePolkadotGrandpa";
 /// Name of the With-Polkadot messages pallet instance that is deployed at bridged chains.
 pub const WITH_POLKADOT_MESSAGES_PALLET_NAME: &str = "BridgePolkadotMessages";
 

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

@@ -226,6 +226,8 @@ frame_support::parameter_types! {
 		.build_or_panic();
 }
 
+/// Name of the With-Rialto GRANDPA pallet instance that is deployed at bridged chains.
+pub const WITH_RIALTO_GRANDPA_PALLET_NAME: &str = "BridgeRialtoGrandpa";
 /// Name of the With-Rialto messages pallet instance that is deployed at bridged chains.
 pub const WITH_RIALTO_MESSAGES_PALLET_NAME: &str = "BridgeRialtoMessages";
 

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

@@ -75,6 +75,8 @@ pub fn derive_account_from_wococo_id(id: bp_runtime::SourceAccount<AccountId>) -
 	AccountIdConverter::convert(encoded_id)
 }
 
+/// Name of the With-Rococo GRANDPA pallet instance that is deployed at bridged chains.
+pub const WITH_ROCOCO_GRANDPA_PALLET_NAME: &str = "BridgeRococoGrandpa";
 /// Name of the With-Rococo messages pallet instance that is deployed at bridged chains.
 pub const WITH_ROCOCO_MESSAGES_PALLET_NAME: &str = "BridgeRococoMessages";
 

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

@@ -87,6 +87,9 @@ pub fn derive_account_from_rococo_id(id: bp_runtime::SourceAccount<AccountId>) -
 	AccountIdConverter::convert(encoded_id)
 }
 
+/// Name of the With-Westend GRANDPA pallet instance that is deployed at bridged chains.
+pub const WITH_WESTEND_GRANDPA_PALLET_NAME: &str = "BridgeWestendGrandpa";
+
 /// Name of the `WestendFinalityApi::best_finalized` runtime method.
 pub const BEST_FINALIZED_WESTEND_HEADER_METHOD: &str = "WestendFinalityApi_best_finalized";
 

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

@@ -40,6 +40,8 @@ pub fn derive_account_from_rococo_id(id: bp_runtime::SourceAccount<AccountId>) -
 	AccountIdConverter::convert(encoded_id)
 }
 
+/// Name of the With-Wococo GRANDPA pallet instance that is deployed at bridged chains.
+pub const WITH_WOCOCO_GRANDPA_PALLET_NAME: &str = "BridgeWococoGrandpa";
 /// Name of the With-Wococo messages pallet instance that is deployed at bridged chains.
 pub const WITH_WOCOCO_MESSAGES_PALLET_NAME: &str = "BridgeWococoMessages";
 

bridges/primitives/header-chain/Cargo.toml

@@ -12,6 +12,10 @@ finality-grandpa = { version = "0.14.0", default-features = false }
 scale-info = { version = "1.0", default-features = false, features = ["derive"] }
 serde = { version = "1.0", optional = true }
 
+# Bridge dependencies
+
+bp-runtime = { path = "../runtime", default-features = false }
+
 # Substrate Dependencies
 
 frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
@@ -23,10 +27,13 @@ sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", d
 [dev-dependencies]
 assert_matches = "1.5"
 bp-test-utils = { path = "../test-utils" }
+hex = "0.4"
+hex-literal = "0.3"
 
 [features]
 default = ["std"]
 std = [
+	"bp-runtime/std",
 	"codec/std",
 	"finality-grandpa/std",
 	"serde/std",

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

@@ -29,6 +29,7 @@ use sp_runtime::{generic::OpaqueDigestItemId, traits::Header as HeaderT, Runtime
 use sp_std::boxed::Box;
 
 pub mod justification;
+pub mod storage_keys;
 
 /// A type that can be used as a parameter in a dispatchable function.
 ///

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

@@ -0,0 +1,52 @@
+// 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/>.
+
+//! Storage keys of bridge GRANDPA pallet.
+
+/// Name of the `BestFinalized` storage map.
+pub const BEST_FINALIZED_MAP_NAME: &str = "BestFinalized";
+
+use sp_core::storage::StorageKey;
+
+/// Storage key of the best finalized header hash value in the runtime storage.
+pub fn best_finalized_hash_key(pallet_prefix: &str) -> StorageKey {
+	StorageKey(
+		bp_runtime::storage_value_final_key(
+			pallet_prefix.as_bytes(),
+			BEST_FINALIZED_MAP_NAME.as_bytes(),
+		)
+		.to_vec(),
+	)
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use hex_literal::hex;
+
+	#[test]
+	fn best_finalized_hash_key_computed_properly() {
+		// If this test fails, then something has been changed in module storage that is breaking
+		// compatibility with previous pallet.
+		let storage_key = best_finalized_hash_key("BridgeGrandpa").0;
+		assert_eq!(
+			storage_key,
+			hex!("0b06f475eddb98cf933a12262e0388dea4ebafdd473c549fdb24c5c991c5591c").to_vec(),
+			"Unexpected storage key: {}",
+			hex::encode(&storage_key),
+		);
+	}
+}

bridges/relays/client-kusama/src/lib.rs

@@ -20,8 +20,8 @@ use bp_messages::MessageNonce;
 use codec::Encode;
 use frame_support::weights::Weight;
 use relay_substrate_client::{
-	Chain, ChainBase, ChainWithBalances, ChainWithMessages, SignParam, TransactionSignScheme,
-	UnsignedTransaction,
+	Chain, ChainBase, ChainWithBalances, ChainWithGrandpa, ChainWithMessages, SignParam,
+	TransactionSignScheme, UnsignedTransaction,
 };
 use sp_core::{storage::StorageKey, Pair};
 use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount};
@@ -70,6 +70,10 @@ impl Chain for Kusama {
 	type WeightToFee = bp_kusama::WeightToFee;
 }
 
+impl ChainWithGrandpa for Kusama {
+	const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str = bp_kusama::WITH_KUSAMA_GRANDPA_PALLET_NAME;
+}
+
 impl ChainWithMessages for Kusama {
 	const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str =
 		bp_kusama::WITH_KUSAMA_MESSAGES_PALLET_NAME;

bridges/relays/client-millau/src/lib.rs

@@ -20,8 +20,8 @@ use bp_messages::MessageNonce;
 use codec::{Compact, Decode, Encode};
 use frame_support::weights::Weight;
 use relay_substrate_client::{
-	BalanceOf, Chain, ChainBase, ChainWithBalances, ChainWithMessages, IndexOf, SignParam,
-	TransactionSignScheme, UnsignedTransaction,
+	BalanceOf, Chain, ChainBase, ChainWithBalances, ChainWithGrandpa, ChainWithMessages, IndexOf,
+	SignParam, TransactionSignScheme, UnsignedTransaction,
 };
 use sp_core::{storage::StorageKey, Pair};
 use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount};
@@ -54,6 +54,10 @@ impl ChainBase for Millau {
 	}
 }
 
+impl ChainWithGrandpa for Millau {
+	const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str = bp_millau::WITH_MILLAU_GRANDPA_PALLET_NAME;
+}
+
 impl ChainWithMessages for Millau {
 	const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str =
 		bp_millau::WITH_MILLAU_MESSAGES_PALLET_NAME;

bridges/relays/client-polkadot/src/lib.rs

@@ -20,8 +20,8 @@ use bp_messages::MessageNonce;
 use codec::Encode;
 use frame_support::weights::Weight;
 use relay_substrate_client::{
-	Chain, ChainBase, ChainWithBalances, ChainWithMessages, SignParam, TransactionSignScheme,
-	UnsignedTransaction,
+	Chain, ChainBase, ChainWithBalances, ChainWithGrandpa, ChainWithMessages, SignParam,
+	TransactionSignScheme, UnsignedTransaction,
 };
 use sp_core::{storage::StorageKey, Pair};
 use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount};
@@ -70,6 +70,11 @@ impl Chain for Polkadot {
 	type WeightToFee = bp_polkadot::WeightToFee;
 }
 
+impl ChainWithGrandpa for Polkadot {
+	const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str =
+		bp_polkadot::WITH_POLKADOT_GRANDPA_PALLET_NAME;
+}
+
 impl ChainWithMessages for Polkadot {
 	const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str =
 		bp_polkadot::WITH_POLKADOT_MESSAGES_PALLET_NAME;

bridges/relays/client-rialto/src/lib.rs

@@ -20,8 +20,8 @@ use bp_messages::MessageNonce;
 use codec::{Compact, Decode, Encode};
 use frame_support::weights::Weight;
 use relay_substrate_client::{
-	BalanceOf, Chain, ChainBase, ChainWithBalances, ChainWithMessages, IndexOf, SignParam,
-	TransactionSignScheme, UnsignedTransaction,
+	BalanceOf, Chain, ChainBase, ChainWithBalances, ChainWithGrandpa, ChainWithMessages, IndexOf,
+	SignParam, TransactionSignScheme, UnsignedTransaction,
 };
 use sp_core::{storage::StorageKey, Pair};
 use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount};
@@ -69,6 +69,10 @@ impl Chain for Rialto {
 	type WeightToFee = bp_rialto::WeightToFee;
 }
 
+impl ChainWithGrandpa for Rialto {
+	const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str = bp_rialto::WITH_RIALTO_GRANDPA_PALLET_NAME;
+}
+
 impl ChainWithMessages for Rialto {
 	const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str =
 		bp_rialto::WITH_RIALTO_MESSAGES_PALLET_NAME;

bridges/relays/client-rococo/src/lib.rs

@@ -20,8 +20,8 @@ use bp_messages::MessageNonce;
 use codec::Encode;
 use frame_support::weights::Weight;
 use relay_substrate_client::{
-	Chain, ChainBase, ChainWithBalances, ChainWithMessages, SignParam, TransactionSignScheme,
-	UnsignedTransaction,
+	Chain, ChainBase, ChainWithBalances, ChainWithGrandpa, ChainWithMessages, SignParam,
+	TransactionSignScheme, UnsignedTransaction,
 };
 use sp_core::{storage::StorageKey, Pair};
 use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount};
@@ -73,6 +73,10 @@ impl Chain for Rococo {
 	type WeightToFee = bp_rococo::WeightToFee;
 }
 
+impl ChainWithGrandpa for Rococo {
+	const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str = bp_rococo::WITH_ROCOCO_GRANDPA_PALLET_NAME;
+}
+
 impl ChainWithMessages for Rococo {
 	const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str =
 		bp_rococo::WITH_ROCOCO_MESSAGES_PALLET_NAME;

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

@@ -64,6 +64,20 @@ pub trait Chain: ChainBase + Clone {
 	type WeightToFee: WeightToFeePolynomial<Balance = Self::Balance>;
 }
 
+/// Substrate-based chain that is using direct GRANDPA finality from minimal relay-client point of
+/// view.
+///
+/// Keep in mind that parachains are relying on relay chain GRANDPA, so they should not implement
+/// this trait.
+pub trait ChainWithGrandpa: Chain {
+	/// Name of the bridge GRANDPA pallet (used in `construct_runtime` macro call) that is deployed
+	/// at some other chain to bridge with this `ChainWithGrandpa`.
+	///
+	/// We assume that all chains that are bridging with this `ChainWithGrandpa` are using
+	/// the same name.
+	const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str;
+}
+
 /// Substrate-based chain with messaging support from minimal relay-client point of view.
 pub trait ChainWithMessages: Chain {
 	/// Name of the bridge messages pallet (used in `construct_runtime` macro call) that is deployed

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

@@ -32,7 +32,7 @@ use std::time::Duration;
 pub use crate::{
 	chain::{
 		AccountKeyPairOf, BlockWithJustification, CallOf, Chain, ChainWithBalances,
-		ChainWithMessages, SignParam, TransactionSignScheme, TransactionStatusOf,
+		ChainWithGrandpa, ChainWithMessages, SignParam, TransactionSignScheme, TransactionStatusOf,
 		UnsignedTransaction, WeightToFeeOf,
 	},
 	client::{ChainRuntimeVersion, Client, OpaqueGrandpaAuthoritiesSet, Subscription},

bridges/relays/client-westend/src/lib.rs

@@ -17,7 +17,7 @@
 //! Types used to connect to the Westend chain.
 
 use frame_support::weights::Weight;
-use relay_substrate_client::{Chain, ChainBase, ChainWithBalances};
+use relay_substrate_client::{Chain, ChainBase, ChainWithBalances, ChainWithGrandpa};
 use sp_core::storage::StorageKey;
 use std::time::Duration;
 
@@ -65,6 +65,11 @@ impl Chain for Westend {
 	type WeightToFee = bp_westend::WeightToFee;
 }
 
+impl ChainWithGrandpa for Westend {
+	const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str =
+		bp_westend::WITH_WESTEND_GRANDPA_PALLET_NAME;
+}
+
 impl ChainWithBalances for Westend {
 	fn account_info_storage_key(account_id: &Self::AccountId) -> StorageKey {
 		StorageKey(bp_westend::account_info_storage_key(account_id))

bridges/relays/client-wococo/src/lib.rs

@@ -20,8 +20,8 @@ use bp_messages::MessageNonce;
 use codec::Encode;
 use frame_support::weights::Weight;
 use relay_substrate_client::{
-	Chain, ChainBase, ChainWithBalances, ChainWithMessages, SignParam, TransactionSignScheme,
-	UnsignedTransaction,
+	Chain, ChainBase, ChainWithBalances, ChainWithGrandpa, ChainWithMessages, SignParam,
+	TransactionSignScheme, UnsignedTransaction,
 };
 use sp_core::{storage::StorageKey, Pair};
 use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount};
@@ -73,6 +73,10 @@ impl Chain for Wococo {
 	type WeightToFee = bp_wococo::WeightToFee;
 }
 
+impl ChainWithGrandpa for Wococo {
+	const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str = bp_wococo::WITH_WOCOCO_GRANDPA_PALLET_NAME;
+}
+
 impl ChainWithMessages for Wococo {
 	const WITH_CHAIN_MESSAGES_PALLET_NAME: &'static str =
 		bp_wococo::WITH_WOCOCO_MESSAGES_PALLET_NAME;

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

@@ -55,4 +55,7 @@ pub enum Error<Hash: Debug + MaybeDisplay, HeaderNumber: Debug + MaybeDisplay> {
 	/// Failed to retrieve header by the hash from the source chain.
 	#[error("Failed to retrieve {0} header with hash {1}: {:?}")]
 	RetrieveHeader(&'static str, Hash, client::Error),
+	/// 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),
 }

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

@@ -31,13 +31,13 @@ use bp_header_chain::{
 use codec::Decode;
 use finality_grandpa::voter_set::VoterSet;
 use num_traits::{One, Zero};
-use relay_substrate_client::{Chain, Client};
+use relay_substrate_client::{BlockNumberOf, Chain, ChainWithGrandpa, Client, HashOf};
 use sp_core::Bytes;
 use sp_finality_grandpa::AuthorityList as GrandpaAuthoritiesSet;
 use sp_runtime::traits::Header as HeaderT;
 
 /// Submit headers-bridge initialization transaction.
-pub async fn initialize<SourceChain: Chain, TargetChain: Chain>(
+pub async fn initialize<SourceChain: ChainWithGrandpa, TargetChain: Chain>(
 	source_client: Client<SourceChain>,
 	target_client: Client<TargetChain>,
 	target_transactions_signer: TargetChain::AccountId,
@@ -54,13 +54,14 @@ pub async fn initialize<SourceChain: Chain, TargetChain: Chain>(
 	.await;
 
 	match result {
-		Ok(tx_hash) => log::info!(
+		Ok(Some(tx_hash)) => log::info!(
 			target: "bridge",
 			"Successfully submitted {}-headers bridge initialization transaction to {}: {:?}",
 			SourceChain::NAME,
 			TargetChain::NAME,
 			tx_hash,
 		),
+		Ok(None) => (),
 		Err(err) => log::error!(
 			target: "bridge",
 			"Failed to submit {}-headers bridge initialization transaction to {}: {:?}",
@@ -72,14 +73,28 @@ pub async fn initialize<SourceChain: Chain, TargetChain: Chain>(
 }
 
 /// Craft and submit initialization transaction, returning any error that may occur.
-async fn do_initialize<SourceChain: Chain, TargetChain: Chain>(
+async fn do_initialize<SourceChain: ChainWithGrandpa, TargetChain: Chain>(
 	source_client: Client<SourceChain>,
 	target_client: Client<TargetChain>,
 	target_transactions_signer: TargetChain::AccountId,
 	prepare_initialize_transaction: impl FnOnce(TargetChain::Index, InitializationData<SourceChain::Header>) -> Bytes
 		+ Send
 		+ 'static,
-) -> Result<TargetChain::Hash, Error<SourceChain::Hash, <SourceChain::Header as HeaderT>::Number>> {
+) -> Result<
+	Option<TargetChain::Hash>,
+	Error<SourceChain::Hash, <SourceChain::Header as HeaderT>::Number>,
+> {
+	let is_initialized = is_initialized::<SourceChain, TargetChain>(&target_client).await?;
+	if is_initialized {
+		log::info!(
+			target: "bridge",
+			"{}-headers bridge at {} is already initialized. Skipping",
+			SourceChain::NAME,
+			TargetChain::NAME,
+		);
+		return Ok(None)
+	}
+
 	let initialization_data = prepare_initialization_data(source_client).await?;
 	log::info!(
 		target: "bridge",
@@ -95,7 +110,23 @@ async fn do_initialize<SourceChain: Chain, TargetChain: Chain>(
 		})
 		.await
 		.map_err(|err| Error::SubmitTransaction(TargetChain::NAME, err))?;
-	Ok(initialization_tx_hash)
+	Ok(Some(initialization_tx_hash))
+}
+
+/// Returns `Ok(true)` if bridge has already been initialized.
+async fn is_initialized<SourceChain: ChainWithGrandpa, TargetChain: Chain>(
+	target_client: &Client<TargetChain>,
+) -> Result<bool, Error<HashOf<SourceChain>, BlockNumberOf<SourceChain>>> {
+	Ok(target_client
+		.raw_storage_value(
+			bp_header_chain::storage_keys::best_finalized_hash_key(
+				SourceChain::WITH_CHAIN_GRANDPA_PALLET_NAME,
+			),
+			None,
+		)
+		.await
+		.map_err(|err| Error::RetrieveBestFinalizedHeaderHash(SourceChain::NAME, err))?
+		.is_some())
 }
 
 /// Prepare initialization data for the GRANDPA verifier pallet.