Update bridges subtree (#2903)

* Squashed 'bridges/' changes from 0417308a48..3c4ada921b

3c4ada921b Update dependecies (#2277) (#2281)
3e195c9e76 GRANDPA: optimize votes_ancestries when needed (#2262) (#2264)
7065bbabc6 Implement RuntimeDebug for GrandpaJustification (#2254)
8c9e59bcbc Define generate_grandpa_key_ownership_proof() (#2247) (#2248)
0b46956df7 Deduplicate Grandpa consensus log reading logic (#2245) (#2246)
96c9701710 Fix deps from Cumulus (#2244)

git-subtree-dir: bridges
git-subtree-split: 3c4ada921bbdbdba945c3aa85d76ce316f7baab3

* removed extra files

* post-merge fixes

* also post-merge fixes

cumulus/bridges/bin/runtime-common/src/integrity.rs

@@ -38,7 +38,7 @@ macro_rules! assert_chain_types(
 			// if one of asserts fail, then either bridge isn't configured properly (or alternatively - non-standard
 			// configuration is used), or something has broke existing configuration (meaning that all bridged chains
 			// and relays will stop functioning)
-			use frame_system::{Config as SystemConfig, pallet_prelude::*};
+			use frame_system::{Config as SystemConfig, pallet_prelude::{BlockNumberFor, HeaderFor}};
 			use static_assertions::assert_type_eq_all;
 
 			assert_type_eq_all!(<$r as SystemConfig>::Nonce, bp_runtime::NonceOf<$this>);

cumulus/bridges/bin/runtime-common/src/mock.rs

@@ -63,7 +63,9 @@ pub type ThisChainHasher = BlakeTwo256;
 pub type ThisChainRuntimeCall = RuntimeCall;
 /// Runtime call origin at `ThisChain`.
 pub type ThisChainCallOrigin = RuntimeOrigin;
-// Block of `ThisChain`.
+/// Header of `ThisChain`.
+pub type ThisChainHeader = sp_runtime::generic::Header<ThisChainBlockNumber, ThisChainHasher>;
+/// Block of `ThisChain`.
 pub type ThisChainBlock = frame_system::mocking::MockBlockU32<TestRuntime>;
 
 /// Account identifier at the `BridgedChain`.
@@ -79,8 +81,6 @@ pub type BridgedChainHasher = BlakeTwo256;
 /// Header of the `BridgedChain`.
 pub type BridgedChainHeader =
 	sp_runtime::generic::Header<BridgedChainBlockNumber, BridgedChainHasher>;
-/// Block of the `BridgedChain`.
-pub type BridgedChainBlock = frame_system::mocking::MockBlockU32<TestRuntime>;
 
 /// Rewards payment procedure.
 pub type TestPaymentProcedure = PayRewardFromAccount<Balances, ThisChainAccountId>;
@@ -312,9 +312,10 @@ impl From<BridgedChainOrigin>
 pub struct ThisUnderlyingChain;
 
 impl Chain for ThisUnderlyingChain {
-	type Block = ThisChainBlock;
+	type BlockNumber = ThisChainBlockNumber;
 	type Hash = ThisChainHash;
 	type Hasher = ThisChainHasher;
+	type Header = ThisChainHeader;
 	type AccountId = ThisChainAccountId;
 	type Balance = ThisChainBalance;
 	type Nonce = u32;
@@ -351,9 +352,10 @@ pub struct BridgedUnderlyingParachain;
 pub struct BridgedChainCall;
 
 impl Chain for BridgedUnderlyingChain {
-	type Block = BridgedChainBlock;
+	type BlockNumber = BridgedChainBlockNumber;
 	type Hash = BridgedChainHash;
 	type Hasher = BridgedChainHasher;
+	type Header = BridgedChainHeader;
 	type AccountId = BridgedChainAccountId;
 	type Balance = BridgedChainBalance;
 	type Nonce = u32;
@@ -376,9 +378,10 @@ impl ChainWithGrandpa for BridgedUnderlyingChain {
 }
 
 impl Chain for BridgedUnderlyingParachain {
-	type Block = BridgedChainBlock;
+	type BlockNumber = BridgedChainBlockNumber;
 	type Hash = BridgedChainHash;
 	type Hasher = BridgedChainHasher;
+	type Header = BridgedChainHeader;
 	type AccountId = BridgedChainAccountId;
 	type Balance = BridgedChainBalance;
 	type Nonce = u32;

cumulus/bridges/bin/runtime-common/src/parachains_benchmarking.rs

@@ -46,7 +46,7 @@ where
 		+ pallet_bridge_grandpa::Config<R::BridgesGrandpaPalletInstance>,
 	PI: 'static,
 	<R as pallet_bridge_grandpa::Config<R::BridgesGrandpaPalletInstance>>::BridgedChain:
-		bp_runtime::Chain<Block = pallet_bridge_parachains::RelayBlock, Hash = RelayBlockHash>,
+		bp_runtime::Chain<BlockNumber = RelayBlockNumber, Hash = RelayBlockHash>,
 {
 	let parachain_head = ParaHead(vec![0u8; parachain_head_size as usize]);
 

cumulus/bridges/bin/runtime-common/src/refund_relayer_extension.rs

@@ -24,7 +24,7 @@ use crate::messages_call_ext::{
 };
 use bp_messages::{LaneId, MessageNonce};
 use bp_relayers::{RewardsAccountOwner, RewardsAccountParams};
-use bp_runtime::{Chain, Parachain, ParachainIdOf, RangeInclusiveExt, StaticStrProvider};
+use bp_runtime::{Parachain, ParachainIdOf, RangeInclusiveExt, StaticStrProvider};
 use codec::{Decode, Encode};
 use frame_support::{
 	dispatch::{CallableCallFor, DispatchInfo, Dispatchable, PostDispatchInfo},
@@ -47,10 +47,7 @@ use pallet_transaction_payment::{Config as TransactionPaymentConfig, OnChargeTra
 use pallet_utility::{Call as UtilityCall, Config as UtilityConfig, Pallet as UtilityPallet};
 use scale_info::TypeInfo;
 use sp_runtime::{
-	traits::{
-		Block as BlockT, DispatchInfoOf, Get, Header as HeaderT, PostDispatchInfoOf,
-		SignedExtension, Zero,
-	},
+	traits::{DispatchInfoOf, Get, PostDispatchInfoOf, SignedExtension, Zero},
 	transaction_validity::{
 		TransactionPriority, TransactionValidity, TransactionValidityError, ValidTransactionBuilder,
 	},
@@ -281,7 +278,6 @@ where
 		+ GrandpaCallSubType<Runtime, Runtime::BridgesGrandpaPalletInstance>
 		+ ParachainsCallSubType<Runtime, Para::Instance>
 		+ MessagesCallSubType<Runtime, Msgs::Instance>,
-	<<<Runtime as BoundedBridgeGrandpaConfig<Runtime::BridgesGrandpaPalletInstance>>::BridgedRelayChain as Chain>::Block as BlockT>::Header: HeaderT<Number = RelayBlockNumber>
 {
 	fn expand_call<'a>(&self, call: &'a CallOf<Runtime>) -> Vec<&'a CallOf<Runtime>> {
 		match call.is_sub_type() {
@@ -529,7 +525,6 @@ where
 		+ GrandpaCallSubType<Runtime, Runtime::BridgesGrandpaPalletInstance>
 		+ ParachainsCallSubType<Runtime, Para::Instance>
 		+ MessagesCallSubType<Runtime, Msgs::Instance>,
-	<<<Runtime as BoundedBridgeGrandpaConfig<Runtime::BridgesGrandpaPalletInstance>>::BridgedRelayChain as Chain>::Block as BlockT>::Header: HeaderT<Number = RelayBlockNumber>
 {
 	const IDENTIFIER: &'static str = Id::STR;
 	type AccountId = Runtime::AccountId;

cumulus/bridges/modules/grandpa/src/call_ext.rs

@@ -19,7 +19,6 @@ use bp_header_chain::{justification::GrandpaJustification, ChainWithGrandpa};
 use bp_runtime::BlockNumberOf;
 use codec::Encode;
 use frame_support::{dispatch::CallableCallFor, traits::IsSubType, weights::Weight, RuntimeDebug};
-use frame_system::pallet_prelude::HeaderFor;
 use sp_runtime::{
 	traits::{Header, Zero},
 	transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction},
@@ -179,9 +178,10 @@ pub(crate) fn submit_finality_proof_info_from_args<T: Config<I>, I: 'static>(
 
 /// Returns maximal expected size of `submit_finality_proof` call arguments.
 fn max_expected_call_size<T: Config<I>, I: 'static>(required_precommits: u32) -> u32 {
-	let max_expected_justification_size = GrandpaJustification::<HeaderFor<T>>::max_reasonable_size::<
-		T::BridgedChain,
-	>(required_precommits);
+	let max_expected_justification_size =
+		GrandpaJustification::<BridgedHeader<T, I>>::max_reasonable_size::<T::BridgedChain>(
+			required_precommits,
+		);
 
 	// call arguments are header and justification
 	T::BridgedChain::MAX_HEADER_SIZE.saturating_add(max_expected_justification_size)

cumulus/bridges/modules/grandpa/src/lib.rs

@@ -39,13 +39,12 @@
 pub use storage_types::StoredAuthoritySet;
 
 use bp_header_chain::{
-	justification::GrandpaJustification, ChainWithGrandpa, HeaderChain, InitializationData,
-	StoredHeaderData, StoredHeaderDataBuilder,
+	justification::GrandpaJustification, ChainWithGrandpa, GrandpaConsensusLogReader, HeaderChain,
+	InitializationData, StoredHeaderData, StoredHeaderDataBuilder,
 };
 use bp_runtime::{BlockNumberOf, HashOf, HasherOf, HeaderId, HeaderOf, OwnedBridgeModule};
 use finality_grandpa::voter_set::VoterSet;
 use frame_support::{dispatch::PostDispatchInfo, ensure, DefaultNoBound};
-use sp_consensus_grandpa::{ConsensusLog, GRANDPA_ENGINE_ID};
 use sp_runtime::{
 	traits::{Header as HeaderT, Zero},
 	SaturatedConversion,
@@ -443,11 +442,17 @@ pub mod pallet {
 
 		// We don't support forced changes - at that point governance intervention is required.
 		ensure!(
-			super::find_forced_change(header).is_none(),
+			GrandpaConsensusLogReader::<BridgedBlockNumber<T, I>>::find_forced_change(
+				header.digest()
+			)
+			.is_none(),
 			<Error<T, I>>::UnsupportedScheduledChange
 		);
 
-		if let Some(change) = super::find_scheduled_change(header) {
+		if let Some(change) =
+			GrandpaConsensusLogReader::<BridgedBlockNumber<T, I>>::find_scheduled_change(
+				header.digest(),
+			) {
 			// GRANDPA only includes a `delay` for forced changes, so this isn't valid.
 			ensure!(change.delay == Zero::zero(), <Error<T, I>>::UnsupportedScheduledChange);
 
@@ -616,42 +621,6 @@ impl<T: Config<I>, I: 'static> HeaderChain<BridgedChain<T, I>> for GrandpaChainH
 	}
 }
 
-pub(crate) fn find_scheduled_change<H: HeaderT>(
-	header: &H,
-) -> Option<sp_consensus_grandpa::ScheduledChange<H::Number>> {
-	use sp_runtime::generic::OpaqueDigestItemId;
-
-	let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID);
-
-	let filter_log = |log: ConsensusLog<H::Number>| match log {
-		ConsensusLog::ScheduledChange(change) => Some(change),
-		_ => None,
-	};
-
-	// find the first consensus digest with the right ID which converts to
-	// the right kind of consensus log.
-	header.digest().convert_first(|l| l.try_to(id).and_then(filter_log))
-}
-
-/// Checks the given header for a consensus digest signaling a **forced** scheduled change and
-/// extracts it.
-pub(crate) fn find_forced_change<H: HeaderT>(
-	header: &H,
-) -> Option<(H::Number, sp_consensus_grandpa::ScheduledChange<H::Number>)> {
-	use sp_runtime::generic::OpaqueDigestItemId;
-
-	let id = OpaqueDigestItemId::Consensus(&GRANDPA_ENGINE_ID);
-
-	let filter_log = |log: ConsensusLog<H::Number>| match log {
-		ConsensusLog::ForcedChange(delay, change) => Some((delay, change)),
-		_ => None,
-	};
-
-	// find the first consensus digest with the right ID which converts to
-	// the right kind of consensus log.
-	header.digest().convert_first(|l| l.try_to(id).and_then(filter_log))
-}
-
 /// (Re)initialize bridge with given header for using it in `pallet-bridge-messages` benchmarks.
 #[cfg(feature = "runtime-benchmarks")]
 pub fn initialize_for_benchmarks<T: Config<I>, I: 'static>(header: BridgedHeader<T, I>) {
@@ -685,6 +654,7 @@ mod tests {
 		storage::generator::StorageValue,
 	};
 	use frame_system::{EventRecord, Phase};
+	use sp_consensus_grandpa::{ConsensusLog, GRANDPA_ENGINE_ID};
 	use sp_core::Get;
 	use sp_runtime::{Digest, DigestItem, DispatchError};
 

cumulus/bridges/modules/grandpa/src/mock.rs

@@ -32,8 +32,8 @@ use sp_runtime::{
 };
 
 pub type AccountId = u64;
-pub type TestHeader = crate::BridgedHeader<TestRuntime, ()>;
-pub type TestNumber = crate::BridgedBlockNumber<TestRuntime, ()>;
+pub type TestHeader = sp_runtime::testing::Header;
+pub type TestNumber = u64;
 
 type Block = frame_system::mocking::MockBlock<TestRuntime>;
 
@@ -100,9 +100,10 @@ impl grandpa::Config for TestRuntime {
 pub struct TestBridgedChain;
 
 impl Chain for TestBridgedChain {
-	type Block = Block;
+	type BlockNumber = TestNumber;
 	type Hash = <TestRuntime as frame_system::Config>::Hash;
 	type Hasher = <TestRuntime as frame_system::Config>::Hashing;
+	type Header = TestHeader;
 
 	type AccountId = AccountId;
 	type Balance = u64;

cumulus/bridges/modules/parachains/src/benchmarking.rs

@@ -47,7 +47,7 @@ benchmarks_instance_pallet! {
 		where
 			<T as pallet_bridge_grandpa::Config<T::BridgesGrandpaPalletInstance>>::BridgedChain:
 				bp_runtime::Chain<
-					Block = crate::RelayBlock,
+					BlockNumber = RelayBlockNumber,
 					Hash = RelayBlockHash,
 					Hasher = RelayBlockHasher,
 				>,

cumulus/bridges/modules/parachains/src/lib.rs

@@ -63,8 +63,6 @@ pub type RelayBlockHash = bp_polkadot_core::Hash;
 pub type RelayBlockNumber = bp_polkadot_core::BlockNumber;
 /// Hasher of the bridged relay chain.
 pub type RelayBlockHasher = bp_polkadot_core::Hasher;
-/// Block type of the bridged relay chain.
-pub type RelayBlock = bp_polkadot_core::Block;
 
 /// Artifacts of the parachains head update.
 struct UpdateParachainHeadArtifacts {
@@ -139,15 +137,18 @@ pub mod pallet {
 	pub trait BoundedBridgeGrandpaConfig<I: 'static>:
 		pallet_bridge_grandpa::Config<I, BridgedChain = Self::BridgedRelayChain>
 	{
-		type BridgedRelayChain: Chain<Hash = RelayBlockHash, Hasher = RelayBlockHasher>;
+		type BridgedRelayChain: Chain<
+			BlockNumber = RelayBlockNumber,
+			Hash = RelayBlockHash,
+			Hasher = RelayBlockHasher,
+		>;
 	}
 
 	impl<T, I: 'static> BoundedBridgeGrandpaConfig<I> for T
 	where
 		T: pallet_bridge_grandpa::Config<I>,
-		T::BridgedChain: Chain<Hash = RelayBlockHash, Hasher = RelayBlockHasher>,
-		<<T::BridgedChain as Chain>::Block as sp_runtime::traits::Block>::Header:
-			sp_runtime::traits::Header<Number = RelayBlockNumber>,
+		T::BridgedChain:
+			Chain<BlockNumber = RelayBlockNumber, Hash = RelayBlockHash, Hasher = RelayBlockHasher>,
 	{
 		type BridgedRelayChain = T::BridgedChain;
 	}
@@ -322,7 +323,7 @@ pub mod pallet {
 			>::get(relay_block_hash)
 			.ok_or(Error::<T, I>::UnknownRelayChainBlock)?;
 			ensure!(
-				relay_block.number == relay_block_number.into(),
+				relay_block.number == relay_block_number,
 				Error::<T, I>::InvalidRelayChainBlockNumber,
 			);
 

cumulus/bridges/modules/parachains/src/mock.rs

@@ -20,7 +20,7 @@ use bp_runtime::{Chain, Parachain};
 use frame_support::{construct_runtime, parameter_types, traits::ConstU32, weights::Weight};
 use sp_runtime::{
 	testing::H256,
-	traits::{BlakeTwo256, Header, IdentityLookup},
+	traits::{BlakeTwo256, Header as HeaderT, IdentityLookup},
 	MultiSignature, Perbill,
 };
 
@@ -48,9 +48,10 @@ pub type BigParachainHeader = sp_runtime::generic::Header<u128, BlakeTwo256>;
 pub struct Parachain1;
 
 impl Chain for Parachain1 {
-	type Block = Block;
+	type BlockNumber = u64;
 	type Hash = H256;
 	type Hasher = RegularParachainHasher;
+	type Header = RegularParachainHeader;
 	type AccountId = u64;
 	type Balance = u64;
 	type Nonce = u64;
@@ -71,9 +72,10 @@ impl Parachain for Parachain1 {
 pub struct Parachain2;
 
 impl Chain for Parachain2 {
-	type Block = Block;
+	type BlockNumber = u64;
 	type Hash = H256;
 	type Hasher = RegularParachainHasher;
+	type Header = RegularParachainHeader;
 	type AccountId = u64;
 	type Balance = u64;
 	type Nonce = u64;
@@ -94,9 +96,10 @@ impl Parachain for Parachain2 {
 pub struct Parachain3;
 
 impl Chain for Parachain3 {
-	type Block = Block;
+	type BlockNumber = u64;
 	type Hash = H256;
 	type Hasher = RegularParachainHasher;
+	type Header = RegularParachainHeader;
 	type AccountId = u64;
 	type Balance = u64;
 	type Nonce = u64;
@@ -117,12 +120,11 @@ impl Parachain for Parachain3 {
 // this parachain is using u128 as block number and stored head data size exceeds limit
 pub struct BigParachain;
 
-type BigBlock = frame_system::mocking::MockBlockU128<TestRuntime>;
-
 impl Chain for BigParachain {
-	type Block = BigBlock;
+	type BlockNumber = u128;
 	type Hash = H256;
 	type Hasher = RegularParachainHasher;
+	type Header = BigParachainHeader;
 	type AccountId = u64;
 	type Balance = u64;
 	type Nonce = u64;
@@ -161,11 +163,11 @@ impl frame_system::Config for TestRuntime {
 	type RuntimeOrigin = RuntimeOrigin;
 	type Nonce = u64;
 	type RuntimeCall = RuntimeCall;
+	type Block = Block;
 	type Hash = H256;
 	type Hashing = RegularParachainHasher;
 	type AccountId = AccountId;
 	type Lookup = IdentityLookup<Self::AccountId>;
-	type Block = Block;
 	type RuntimeEvent = RuntimeEvent;
 	type BlockHashCount = BlockHashCount;
 	type Version = ();
@@ -256,9 +258,10 @@ impl pallet_bridge_parachains::benchmarking::Config<()> for TestRuntime {
 pub struct TestBridgedChain;
 
 impl Chain for TestBridgedChain {
-	type Block = crate::RelayBlock;
+	type BlockNumber = crate::RelayBlockNumber;
 	type Hash = crate::RelayBlockHash;
 	type Hasher = crate::RelayBlockHasher;
+	type Header = RelayBlockHeader;
 
 	type AccountId = AccountId;
 	type Balance = u32;
@@ -286,9 +289,10 @@ impl ChainWithGrandpa for TestBridgedChain {
 pub struct OtherBridgedChain;
 
 impl Chain for OtherBridgedChain {
-	type Block = Block;
+	type BlockNumber = u64;
 	type Hash = crate::RelayBlockHash;
 	type Hasher = crate::RelayBlockHasher;
+	type Header = sp_runtime::generic::Header<u64, crate::RelayBlockHasher>;
 
 	type AccountId = AccountId;
 	type Balance = u32;

cumulus/bridges/modules/relayers/src/mock.rs

@@ -65,11 +65,11 @@ impl frame_system::Config for TestRuntime {
 	type RuntimeOrigin = RuntimeOrigin;
 	type Nonce = u64;
 	type RuntimeCall = RuntimeCall;
+	type Block = Block;
 	type Hash = H256;
 	type Hashing = BlakeTwo256;
 	type AccountId = AccountId;
 	type Lookup = IdentityLookup<Self::AccountId>;
-	type Block = Block;
 	type RuntimeEvent = RuntimeEvent;
 	type BlockHashCount = frame_support::traits::ConstU64<250>;
 	type Version = ();

cumulus/bridges/primitives/chain-bridge-hub-cumulus/src/lib.rs

@@ -17,8 +17,8 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
 pub use bp_polkadot_core::{
-	AccountId, AccountInfoStorageMapKeyProvider, AccountPublic, Balance, Block, BlockNumber, Hash,
-	Hasher, Hashing, Header, Nonce, Perbill, Signature, SignedBlock, UncheckedExtrinsic,
+	AccountId, AccountInfoStorageMapKeyProvider, AccountPublic, Balance, BlockNumber, Hash, Hasher,
+	Hashing, Header, Nonce, Perbill, Signature, SignedBlock, UncheckedExtrinsic,
 	EXTRA_STORAGE_PROOF_SIZE, TX_EXTRA_BYTES,
 };
 
@@ -53,7 +53,7 @@ pub const NORMAL_DISPATCH_RATIO: Perbill = Perbill::from_percent(75);
 /// This is a copy-paste from the cumulus repo's `parachains-common` crate.
 const MAXIMUM_BLOCK_WEIGHT: Weight = Weight::from_parts(constants::WEIGHT_REF_TIME_PER_SECOND, 0)
 	.saturating_div(2)
-	.set_proof_size(polkadot_primitives::MAX_POV_SIZE as u64);
+	.set_proof_size(polkadot_primitives::v5::MAX_POV_SIZE as u64);
 
 /// All cumulus bridge hubs assume that about 5 percent of the block weight is consumed by
 /// `on_initialize` handlers. This is used to limit the maximal weight of a single extrinsic.

cumulus/bridges/primitives/chain-bridge-hub-kusama/src/lib.rs

@@ -36,9 +36,11 @@ use sp_std::prelude::*;
 pub struct BridgeHubKusama;
 
 impl Chain for BridgeHubKusama {
+	type BlockNumber = BlockNumber;
 	type Hash = Hash;
 	type Hasher = Hasher;
-	type Block = Block;
+	type Header = Header;
+
 	type AccountId = AccountId;
 	type Balance = Balance;
 	type Nonce = Nonce;

cumulus/bridges/primitives/chain-bridge-hub-polkadot/src/lib.rs

@@ -32,9 +32,11 @@ use sp_std::prelude::*;
 pub struct BridgeHubPolkadot;
 
 impl Chain for BridgeHubPolkadot {
+	type BlockNumber = BlockNumber;
 	type Hash = Hash;
 	type Hasher = Hasher;
-	type Block = Block;
+	type Header = Header;
+
 	type AccountId = AccountId;
 	type Balance = Balance;
 	type Nonce = Nonce;

cumulus/bridges/primitives/chain-bridge-hub-rococo/src/lib.rs

@@ -36,9 +36,11 @@ use sp_std::prelude::*;
 pub struct BridgeHubRococo;
 
 impl Chain for BridgeHubRococo {
+	type BlockNumber = BlockNumber;
 	type Hash = Hash;
 	type Hasher = Hasher;
-	type Block = Block;
+	type Header = Header;
+
 	type AccountId = AccountId;
 	type Balance = Balance;
 	type Nonce = Nonce;

cumulus/bridges/primitives/chain-bridge-hub-wococo/src/lib.rs

@@ -32,9 +32,11 @@ use sp_std::prelude::*;
 pub struct BridgeHubWococo;
 
 impl Chain for BridgeHubWococo {
+	type BlockNumber = BlockNumber;
 	type Hash = Hash;
 	type Hasher = Hasher;
-	type Block = Block;
+	type Header = Header;
+
 	type AccountId = AccountId;
 	type Balance = Balance;
 	type Nonce = Nonce;

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

@@ -28,9 +28,10 @@ use frame_support::weights::Weight;
 pub struct Kusama;
 
 impl Chain for Kusama {
-	type Block = <PolkadotLike as Chain>::Block;
+	type BlockNumber = <PolkadotLike as Chain>::BlockNumber;
 	type Hash = <PolkadotLike as Chain>::Hash;
 	type Hasher = <PolkadotLike as Chain>::Hasher;
+	type Header = <PolkadotLike as Chain>::Header;
 
 	type AccountId = <PolkadotLike as Chain>::AccountId;
 	type Balance = <PolkadotLike as Chain>::Balance;

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

@@ -28,9 +28,10 @@ use frame_support::weights::Weight;
 pub struct Polkadot;
 
 impl Chain for Polkadot {
-	type Block = <PolkadotLike as Chain>::Block;
+	type BlockNumber = <PolkadotLike as Chain>::BlockNumber;
 	type Hash = <PolkadotLike as Chain>::Hash;
 	type Hasher = <PolkadotLike as Chain>::Hasher;
+	type Header = <PolkadotLike as Chain>::Header;
 
 	type AccountId = <PolkadotLike as Chain>::AccountId;
 	type Balance = <PolkadotLike as Chain>::Balance;

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

@@ -28,9 +28,11 @@ use frame_support::{parameter_types, weights::Weight};
 pub struct Rococo;
 
 impl Chain for Rococo {
-	type Block = <PolkadotLike as Chain>::Block;
+	type BlockNumber = <PolkadotLike as Chain>::BlockNumber;
 	type Hash = <PolkadotLike as Chain>::Hash;
 	type Hasher = <PolkadotLike as Chain>::Hasher;
+	type Header = <PolkadotLike as Chain>::Header;
+
 	type AccountId = <PolkadotLike as Chain>::AccountId;
 	type Balance = <PolkadotLike as Chain>::Balance;
 	type Nonce = <PolkadotLike as Chain>::Nonce;

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

@@ -31,9 +31,11 @@ use frame_support::weights::Weight;
 pub struct Wococo;
 
 impl Chain for Wococo {
-	type Block = <PolkadotLike as Chain>::Block;
+	type BlockNumber = <PolkadotLike as Chain>::BlockNumber;
 	type Hash = <PolkadotLike as Chain>::Hash;
 	type Hasher = <PolkadotLike as Chain>::Hasher;
+	type Header = <PolkadotLike as Chain>::Header;
+
 	type AccountId = <PolkadotLike as Chain>::AccountId;
 	type Balance = <PolkadotLike as Chain>::Balance;
 	type Nonce = <PolkadotLike as Chain>::Nonce;

cumulus/bridges/primitives/header-chain/src/justification.rs

@@ -21,10 +21,10 @@
 
 use crate::ChainWithGrandpa;
 
-use bp_runtime::{BlockNumberOf, Chain, HashOf};
+use bp_runtime::{BlockNumberOf, Chain, HashOf, HeaderId};
 use codec::{Decode, Encode, MaxEncodedLen};
 use finality_grandpa::voter_set::VoterSet;
-use frame_support::RuntimeDebug;
+use frame_support::{RuntimeDebug, RuntimeDebugNoBound};
 use scale_info::TypeInfo;
 use sp_consensus_grandpa::{AuthorityId, AuthoritySignature, SetId};
 use sp_runtime::{traits::Header as HeaderT, SaturatedConversion};
@@ -38,7 +38,7 @@ use sp_std::{
 ///
 /// This particular proof is used to prove that headers on a bridged chain
 /// (so not our chain) have been finalized correctly.
-#[derive(Encode, Decode, Clone, PartialEq, Eq, TypeInfo)]
+#[derive(Encode, Decode, Clone, PartialEq, Eq, TypeInfo, RuntimeDebugNoBound)]
 pub struct GrandpaJustification<Header: HeaderT> {
 	/// The round (voting period) this justification is valid for.
 	pub round: u64,
@@ -49,25 +49,6 @@ pub struct GrandpaJustification<Header: HeaderT> {
 	pub votes_ancestries: Vec<Header>,
 }
 
-// TODO: remove and use `RuntimeDebug` (https://github.com/paritytech/parity-bridges-common/issues/2136)
-impl<Header: HeaderT> sp_std::fmt::Debug for GrandpaJustification<Header> {
-	fn fmt(&self, fmt: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
-		#[cfg(feature = "std")]
-		{
-			fmt.debug_struct("GrandpaJustification")
-				.field("round", &self.round)
-				.field("commit", &self.commit)
-				.field("votes_ancestries", &self.votes_ancestries)
-				.finish()
-		}
-
-		#[cfg(not(feature = "std"))]
-		{
-			fmt.write_str("<stripped>")
-		}
-	}
-}
-
 impl<H: HeaderT> GrandpaJustification<H> {
 	/// Returns reasonable size of justification using constants from the provided chain.
 	///
@@ -103,6 +84,10 @@ impl<H: HeaderT> GrandpaJustification<H> {
 		8u32.saturating_add(max_expected_signed_commit_size)
 			.saturating_add(max_expected_votes_ancestries_size)
 	}
+
+	pub fn commit_target_id(&self) -> HeaderId<H::Hash, H::Number> {
+		HeaderId(self.commit.target_number, self.commit.target_hash)
+	}
 }
 
 impl<H: HeaderT> crate::FinalityProof<H::Number> for GrandpaJustification<H> {
@@ -128,12 +113,12 @@ pub enum Error {
 	InvalidAuthoritySignature,
 	/// The justification contains precommit for header that is not a descendant of the commit
 	/// header.
-	PrecommitIsNotCommitDescendant,
+	UnrelatedAncestryVote,
 	/// The cumulative weight of all votes in the justification is not enough to justify commit
 	/// header finalization.
 	TooLowCumulativeWeight,
 	/// The justification contains extra (unused) headers in its `votes_ancestries` field.
-	ExtraHeadersInVotesAncestries,
+	RedundantVotesAncestries,
 }
 
 /// Given GRANDPA authorities set size, return number of valid authorities votes that the
@@ -158,17 +143,22 @@ pub fn verify_and_optimize_justification<Header: HeaderT>(
 	finalized_target: (Header::Hash, Header::Number),
 	authorities_set_id: SetId,
 	authorities_set: &VoterSet<AuthorityId>,
-	justification: GrandpaJustification<Header>,
-) -> Result<GrandpaJustification<Header>, Error> {
-	let mut optimizer = OptimizationCallbacks(Vec::new());
+	justification: &mut GrandpaJustification<Header>,
+) -> Result<(), Error> {
+	let mut optimizer = OptimizationCallbacks {
+		extra_precommits: vec![],
+		redundant_votes_ancestries: Default::default(),
+	};
 	verify_justification_with_callbacks(
 		finalized_target,
 		authorities_set_id,
 		authorities_set,
-		&justification,
+		justification,
 		&mut optimizer,
 	)?;
-	Ok(optimizer.optimize(justification))
+	optimizer.optimize(justification);
+
+	Ok(())
 }
 
 /// Verify that justification, that is generated by given authority set, finalizes given header.
@@ -188,19 +178,28 @@ pub fn verify_justification<Header: HeaderT>(
 }
 
 /// Verification callbacks.
-trait VerificationCallbacks {
+trait VerificationCallbacks<Header: HeaderT> {
 	/// Called when we see a precommit from unknown authority.
 	fn on_unkown_authority(&mut self, precommit_idx: usize) -> Result<(), Error>;
 	/// Called when we see a precommit with duplicate vote from known authority.
 	fn on_duplicate_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error>;
+	/// Called when we see a precommit with an invalid signature.
+	fn on_invalid_authority_signature(&mut self, precommit_idx: usize) -> Result<(), Error>;
 	/// Called when we see a precommit after we've collected enough votes from authorities.
 	fn on_redundant_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error>;
+	/// Called when we see a precommit that is not a descendant of the commit target.
+	fn on_unrelated_ancestry_vote(&mut self, precommit_idx: usize) -> Result<(), Error>;
+	/// Called when there are redundant headers in the votes ancestries.
+	fn on_redundant_votes_ancestries(
+		&mut self,
+		redundant_votes_ancestries: BTreeSet<Header::Hash>,
+	) -> Result<(), Error>;
 }
 
 /// Verification callbacks that reject all unknown, duplicate or redundant votes.
 struct StrictVerificationCallbacks;
 
-impl VerificationCallbacks for StrictVerificationCallbacks {
+impl<Header: HeaderT> VerificationCallbacks<Header> for StrictVerificationCallbacks {
 	fn on_unkown_authority(&mut self, _precommit_idx: usize) -> Result<(), Error> {
 		Err(Error::UnknownAuthorityVote)
 	}
@@ -209,45 +208,82 @@ impl VerificationCallbacks for StrictVerificationCallbacks {
 		Err(Error::DuplicateAuthorityVote)
 	}
 
+	fn on_invalid_authority_signature(&mut self, _precommit_idx: usize) -> Result<(), Error> {
+		Err(Error::InvalidAuthoritySignature)
+	}
+
 	fn on_redundant_authority_vote(&mut self, _precommit_idx: usize) -> Result<(), Error> {
 		Err(Error::RedundantVotesInJustification)
 	}
+
+	fn on_unrelated_ancestry_vote(&mut self, _precommit_idx: usize) -> Result<(), Error> {
+		Err(Error::UnrelatedAncestryVote)
+	}
+
+	fn on_redundant_votes_ancestries(
+		&mut self,
+		_redundant_votes_ancestries: BTreeSet<Header::Hash>,
+	) -> Result<(), Error> {
+		Err(Error::RedundantVotesAncestries)
+	}
 }
 
 /// Verification callbacks for justification optimization.
-struct OptimizationCallbacks(Vec<usize>);
+struct OptimizationCallbacks<Header: HeaderT> {
+	extra_precommits: Vec<usize>,
+	redundant_votes_ancestries: BTreeSet<Header::Hash>,
+}
 
-impl OptimizationCallbacks {
-	fn optimize<Header: HeaderT>(
-		self,
-		mut justification: GrandpaJustification<Header>,
-	) -> GrandpaJustification<Header> {
-		for invalid_precommit_idx in self.0.into_iter().rev() {
+impl<Header: HeaderT> OptimizationCallbacks<Header> {
+	fn optimize(self, justification: &mut GrandpaJustification<Header>) {
+		for invalid_precommit_idx in self.extra_precommits.into_iter().rev() {
 			justification.commit.precommits.remove(invalid_precommit_idx);
 		}
-		justification
+		if !self.redundant_votes_ancestries.is_empty() {
+			justification
+				.votes_ancestries
+				.retain(|header| !self.redundant_votes_ancestries.contains(&header.hash()))
+		}
 	}
 }
 
-impl VerificationCallbacks for OptimizationCallbacks {
+impl<Header: HeaderT> VerificationCallbacks<Header> for OptimizationCallbacks<Header> {
 	fn on_unkown_authority(&mut self, precommit_idx: usize) -> Result<(), Error> {
-		self.0.push(precommit_idx);
+		self.extra_precommits.push(precommit_idx);
 		Ok(())
 	}
 
 	fn on_duplicate_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error> {
-		self.0.push(precommit_idx);
+		self.extra_precommits.push(precommit_idx);
+		Ok(())
+	}
+
+	fn on_invalid_authority_signature(&mut self, precommit_idx: usize) -> Result<(), Error> {
+		self.extra_precommits.push(precommit_idx);
 		Ok(())
 	}
 
 	fn on_redundant_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error> {
-		self.0.push(precommit_idx);
+		self.extra_precommits.push(precommit_idx);
+		Ok(())
+	}
+
+	fn on_unrelated_ancestry_vote(&mut self, precommit_idx: usize) -> Result<(), Error> {
+		self.extra_precommits.push(precommit_idx);
+		Ok(())
+	}
+
+	fn on_redundant_votes_ancestries(
+		&mut self,
+		redundant_votes_ancestries: BTreeSet<Header::Hash>,
+	) -> Result<(), Error> {
+		self.redundant_votes_ancestries = redundant_votes_ancestries;
 		Ok(())
 	}
 }
 
 /// Verify that justification, that is generated by given authority set, finalizes given header.
-fn verify_justification_with_callbacks<Header: HeaderT, C: VerificationCallbacks>(
+fn verify_justification_with_callbacks<Header: HeaderT, C: VerificationCallbacks<Header>>(
 	finalized_target: (Header::Hash, Header::Number),
 	authorities_set_id: SetId,
 	authorities_set: &VoterSet<AuthorityId>,
@@ -259,8 +295,8 @@ fn verify_justification_with_callbacks<Header: HeaderT, C: VerificationCallbacks
 		return Err(Error::InvalidJustificationTarget)
 	}
 
-	let threshold = authorities_set.threshold().0.into();
-	let mut chain = AncestryChain::new(&justification.votes_ancestries);
+	let threshold = authorities_set.threshold().get();
+	let mut chain = AncestryChain::new(justification);
 	let mut signature_buffer = Vec::new();
 	let mut votes = BTreeSet::new();
 	let mut cumulative_weight = 0u64;
@@ -287,34 +323,20 @@ fn verify_justification_with_callbacks<Header: HeaderT, C: VerificationCallbacks
 		// there's a lot of code in `validate_commit` and `import_precommit` functions inside
 		// `finality-grandpa` crate (mostly related to reporting equivocations). But the only thing
 		// that we care about is that only first vote from the authority is accepted
-		if !votes.insert(signed.id.clone()) {
+		if votes.contains(&signed.id) {
 			callbacks.on_duplicate_authority_vote(precommit_idx)?;
 			continue
 		}
 
-		// everything below this line can't just `continue`, because state is already altered
-
-		// precommits aren't allowed for block lower than the target
-		if signed.precommit.target_number < justification.commit.target_number {
-			return Err(Error::PrecommitIsNotCommitDescendant)
-		}
-		// all precommits must be descendants of target block
-		chain = chain
-			.ensure_descendant(&justification.commit.target_hash, &signed.precommit.target_hash)?;
-		// since we know now that the precommit target is the descendant of the justification
-		// target, we may increase 'weight' of the justification target
-		//
-		// there's a lot of code in the `VoteGraph::insert` method inside `finality-grandpa` crate,
-		// but in the end it is only used to find GHOST, which we don't care about. The only thing
-		// that we care about is that the justification target has enough weight
-		cumulative_weight = cumulative_weight.checked_add(authority_info.weight().0.into()).expect(
-			"sum of weights of ALL authorities is expected not to overflow - this is guaranteed by\
-				existence of VoterSet;\
-				the order of loop conditions guarantees that we can account vote from same authority\
-				multiple times;\
-				thus we'll never overflow the u64::MAX;\
-				qed",
-		);
+		// all precommits must be descendants of the target block
+		let route =
+			match chain.ancestry(&signed.precommit.target_hash, &signed.precommit.target_number) {
+				Some(route) => route,
+				None => {
+					callbacks.on_unrelated_ancestry_vote(precommit_idx)?;
+					continue
+				},
+			};
 
 		// verify authority signature
 		if !sp_consensus_grandpa::check_message_signature_with_buffer(
@@ -325,76 +347,98 @@ fn verify_justification_with_callbacks<Header: HeaderT, C: VerificationCallbacks
 			authorities_set_id,
 			&mut signature_buffer,
 		) {
-			return Err(Error::InvalidAuthoritySignature)
+			callbacks.on_invalid_authority_signature(precommit_idx)?;
+			continue
 		}
+
+		// now we can count the vote since we know that it is valid
+		votes.insert(signed.id.clone());
+		chain.mark_route_as_visited(route);
+		cumulative_weight = cumulative_weight.saturating_add(authority_info.weight().get());
+	}
+
+	// check that the cumulative weight of validators that voted for the justification target (or
+	// one of its descendents) is larger than the required threshold.
+	if cumulative_weight < threshold {
+		return Err(Error::TooLowCumulativeWeight)
 	}
 
 	// check that there are no extra headers in the justification
-	if !chain.unvisited.is_empty() {
-		return Err(Error::ExtraHeadersInVotesAncestries)
+	if !chain.is_fully_visited() {
+		callbacks.on_redundant_votes_ancestries(chain.unvisited)?;
 	}
 
-	// check that the cumulative weight of validators voted for the justification target (or one
-	// of its descendents) is larger than required threshold.
-	if cumulative_weight >= threshold {
-		Ok(())
-	} else {
-		Err(Error::TooLowCumulativeWeight)
-	}
+	Ok(())
 }
 
 /// Votes ancestries with useful methods.
 #[derive(RuntimeDebug)]
 pub struct AncestryChain<Header: HeaderT> {
+	/// We expect all forks in the ancestry chain to be descendants of base.
+	base: HeaderId<Header::Hash, Header::Number>,
 	/// Header hash => parent header hash mapping.
 	pub parents: BTreeMap<Header::Hash, Header::Hash>,
-	/// Hashes of headers that were not visited by `is_ancestor` method.
+	/// Hashes of headers that were not visited by `ancestry()`.
 	pub unvisited: BTreeSet<Header::Hash>,
 }
 
 impl<Header: HeaderT> AncestryChain<Header> {
 	/// Create new ancestry chain.
-	pub fn new(ancestry: &[Header]) -> AncestryChain<Header> {
+	pub fn new(justification: &GrandpaJustification<Header>) -> AncestryChain<Header> {
 		let mut parents = BTreeMap::new();
 		let mut unvisited = BTreeSet::new();
-		for ancestor in ancestry {
+		for ancestor in &justification.votes_ancestries {
 			let hash = ancestor.hash();
 			let parent_hash = *ancestor.parent_hash();
 			parents.insert(hash, parent_hash);
 			unvisited.insert(hash);
 		}
-		AncestryChain { parents, unvisited }
+		AncestryChain { base: justification.commit_target_id(), parents, unvisited }
 	}
 
-	/// Returns `Ok(_)` if `precommit_target` is a descendant of the `commit_target` block and
-	/// `Err(_)` otherwise.
-	pub fn ensure_descendant(
-		mut self,
-		commit_target: &Header::Hash,
-		precommit_target: &Header::Hash,
-	) -> Result<Self, Error> {
-		let mut current_hash = *precommit_target;
+	/// Returns a route if the precommit target block is a descendant of the `base` block.
+	pub fn ancestry(
+		&self,
+		precommit_target_hash: &Header::Hash,
+		precommit_target_number: &Header::Number,
+	) -> Option<Vec<Header::Hash>> {
+		if precommit_target_number < &self.base.number() {
+			return None
+		}
+
+		let mut route = vec![];
+		let mut current_hash = *precommit_target_hash;
 		loop {
-			if current_hash == *commit_target {
+			if current_hash == self.base.hash() {
 				break
 			}
 
-			let is_visited_before = !self.unvisited.remove(&current_hash);
 			current_hash = match self.parents.get(&current_hash) {
 				Some(parent_hash) => {
+					let is_visited_before = self.unvisited.get(&current_hash).is_none();
 					if is_visited_before {
-						// `Some(parent_hash)` means that the `current_hash` is in the `parents`
-						// container `is_visited_before` means that it has been visited before in
-						// some of previous calls => since we assume that previous call has finished
-						// with `true`, this also will be finished with `true`
-						return Ok(self)
+						// If the current header has been visited in a previous call, it is a
+						// descendent of `base` (we assume that the previous call was successful).
+						return Some(route)
 					}
+					route.push(current_hash);
 
 					*parent_hash
 				},
-				None => return Err(Error::PrecommitIsNotCommitDescendant),
+				None => return None,
 			};
 		}
-		Ok(self)
+
+		Some(route)
+	}
+
+	fn mark_route_as_visited(&mut self, route: Vec<Header::Hash>) {
+		for hash in route {
+			self.unvisited.remove(&hash);
+		}
+	}
+
+	fn is_fully_visited(&self) -> bool {
+		self.unvisited.is_empty()
 	}
 }

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

@@ -139,7 +139,7 @@ pub trait ConsensusLogReader {
 pub struct GrandpaConsensusLogReader<Number>(sp_std::marker::PhantomData<Number>);
 
 impl<Number: Codec> GrandpaConsensusLogReader<Number> {
-	pub fn find_authorities_change(
+	pub fn find_scheduled_change(
 		digest: &Digest,
 	) -> Option<sp_consensus_grandpa::ScheduledChange<Number>> {
 		// find the first consensus digest with the right ID which converts to
@@ -151,11 +151,24 @@ impl<Number: Codec> GrandpaConsensusLogReader<Number> {
 				_ => None,
 			})
 	}
+
+	pub fn find_forced_change(
+		digest: &Digest,
+	) -> Option<(Number, sp_consensus_grandpa::ScheduledChange<Number>)> {
+		// find the first consensus digest with the right ID which converts to
+		// the right kind of consensus log.
+		digest
+			.convert_first(|log| log.consensus_try_to(&GRANDPA_ENGINE_ID))
+			.and_then(|log| match log {
+				ConsensusLog::ForcedChange(delay, change) => Some((delay, change)),
+				_ => None,
+			})
+	}
 }
 
 impl<Number: Codec> ConsensusLogReader for GrandpaConsensusLogReader<Number> {
 	fn schedules_authorities_change(digest: &Digest) -> bool {
-		GrandpaConsensusLogReader::<Number>::find_authorities_change(digest).is_some()
+		GrandpaConsensusLogReader::<Number>::find_scheduled_change(digest).is_some()
 	}
 }
 

cumulus/bridges/primitives/header-chain/tests/implementation_match.rs

@@ -38,8 +38,8 @@ type TestNumber = <TestHeader as HeaderT>::Number;
 struct AncestryChain(bp_header_chain::justification::AncestryChain<TestHeader>);
 
 impl AncestryChain {
-	fn new(ancestry: &[TestHeader]) -> Self {
-		Self(bp_header_chain::justification::AncestryChain::new(ancestry))
+	fn new(justification: &GrandpaJustification<TestHeader>) -> Self {
+		Self(bp_header_chain::justification::AncestryChain::new(justification))
 	}
 }
 
@@ -55,9 +55,9 @@ impl finality_grandpa::Chain<TestHash, TestNumber> for AncestryChain {
 			if current_hash == base {
 				break
 			}
-			match self.0.parents.get(&current_hash).cloned() {
+			match self.0.parents.get(&current_hash) {
 				Some(parent_hash) => {
-					current_hash = parent_hash;
+					current_hash = *parent_hash;
 					route.push(current_hash);
 				},
 				_ => return Err(finality_grandpa::Error::NotDescendent),
@@ -124,7 +124,7 @@ fn same_result_when_precommit_target_has_lower_number_than_commit_target() {
 			&full_voter_set(),
 			&justification,
 		),
-		Err(Error::PrecommitIsNotCommitDescendant),
+		Err(Error::UnrelatedAncestryVote),
 	);
 
 	// original implementation returns `Ok(validation_result)`
@@ -132,7 +132,7 @@ fn same_result_when_precommit_target_has_lower_number_than_commit_target() {
 	let result = finality_grandpa::validate_commit(
 		&justification.commit,
 		&full_voter_set(),
-		&AncestryChain::new(&justification.votes_ancestries),
+		&AncestryChain::new(&justification),
 	)
 	.unwrap();
 
@@ -157,7 +157,7 @@ fn same_result_when_precommit_target_is_not_descendant_of_commit_target() {
 			&full_voter_set(),
 			&justification,
 		),
-		Err(Error::PrecommitIsNotCommitDescendant),
+		Err(Error::UnrelatedAncestryVote),
 	);
 
 	// original implementation returns `Ok(validation_result)`
@@ -165,7 +165,7 @@ fn same_result_when_precommit_target_is_not_descendant_of_commit_target() {
 	let result = finality_grandpa::validate_commit(
 		&justification.commit,
 		&full_voter_set(),
-		&AncestryChain::new(&justification.votes_ancestries),
+		&AncestryChain::new(&justification),
 	)
 	.unwrap();
 
@@ -198,7 +198,7 @@ fn same_result_when_there_are_not_enough_cumulative_weight_to_finalize_commit_ta
 	let result = finality_grandpa::validate_commit(
 		&justification.commit,
 		&full_voter_set(),
-		&AncestryChain::new(&justification.votes_ancestries),
+		&AncestryChain::new(&justification),
 	)
 	.unwrap();
 
@@ -236,7 +236,7 @@ fn different_result_when_justification_contains_duplicate_vote() {
 	let result = finality_grandpa::validate_commit(
 		&justification.commit,
 		&full_voter_set(),
-		&AncestryChain::new(&justification.votes_ancestries),
+		&AncestryChain::new(&justification),
 	)
 	.unwrap();
 
@@ -277,7 +277,7 @@ fn different_results_when_authority_equivocates_once_in_a_round() {
 	let result = finality_grandpa::validate_commit(
 		&justification.commit,
 		&full_voter_set(),
-		&AncestryChain::new(&justification.votes_ancestries),
+		&AncestryChain::new(&justification),
 	)
 	.unwrap();
 
@@ -330,7 +330,7 @@ fn different_results_when_authority_equivocates_twice_in_a_round() {
 	let result = finality_grandpa::validate_commit(
 		&justification.commit,
 		&full_voter_set(),
-		&AncestryChain::new(&justification.votes_ancestries),
+		&AncestryChain::new(&justification),
 	)
 	.unwrap();
 
@@ -369,7 +369,7 @@ fn different_results_when_there_are_more_than_enough_votes() {
 	let result = finality_grandpa::validate_commit(
 		&justification.commit,
 		&full_voter_set(),
-		&AncestryChain::new(&justification.votes_ancestries),
+		&AncestryChain::new(&justification),
 	)
 	.unwrap();
 
@@ -410,7 +410,7 @@ fn different_results_when_there_is_a_vote_of_unknown_authority() {
 	let result = finality_grandpa::validate_commit(
 		&justification.commit,
 		&full_voter_set(),
-		&AncestryChain::new(&justification.votes_ancestries),
+		&AncestryChain::new(&justification),
 	)
 	.unwrap();
 

cumulus/bridges/primitives/header-chain/tests/justification.rs

@@ -21,6 +21,8 @@ use bp_header_chain::justification::{
 	Error,
 };
 use bp_test_utils::*;
+use finality_grandpa::SignedPrecommit;
+use sp_consensus_grandpa::AuthoritySignature;
 
 type TestHeader = sp_runtime::testing::Header;
 
@@ -133,7 +135,7 @@ fn justification_with_invalid_commit_rejected() {
 			&voter_set(),
 			&justification,
 		),
-		Err(Error::ExtraHeadersInVotesAncestries),
+		Err(Error::TooLowCumulativeWeight),
 	);
 }
 
@@ -166,7 +168,7 @@ fn justification_with_invalid_precommit_ancestry() {
 			&voter_set(),
 			&justification,
 		),
-		Err(Error::ExtraHeadersInVotesAncestries),
+		Err(Error::RedundantVotesAncestries),
 	);
 }
 
@@ -197,14 +199,14 @@ fn justification_is_invalid_if_we_dont_meet_threshold() {
 
 #[test]
 fn optimizer_does_noting_with_minimal_justification() {
-	let justification = make_default_justification::<TestHeader>(&test_header(1));
+	let mut justification = make_default_justification::<TestHeader>(&test_header(1));
 
 	let num_precommits_before = justification.commit.precommits.len();
-	let justification = verify_and_optimize_justification::<TestHeader>(
+	verify_and_optimize_justification::<TestHeader>(
 		header_id::<TestHeader>(1),
 		TEST_GRANDPA_SET_ID,
 		&voter_set(),
-		justification,
+		&mut justification,
 	)
 	.unwrap();
 	let num_precommits_after = justification.commit.precommits.len();
@@ -223,11 +225,11 @@ fn unknown_authority_votes_are_removed_by_optimizer() {
 	));
 
 	let num_precommits_before = justification.commit.precommits.len();
-	let justification = verify_and_optimize_justification::<TestHeader>(
+	verify_and_optimize_justification::<TestHeader>(
 		header_id::<TestHeader>(1),
 		TEST_GRANDPA_SET_ID,
 		&voter_set(),
-		justification,
+		&mut justification,
 	)
 	.unwrap();
 	let num_precommits_after = justification.commit.precommits.len();
@@ -244,11 +246,42 @@ fn duplicate_authority_votes_are_removed_by_optimizer() {
 		.push(justification.commit.precommits.first().cloned().unwrap());
 
 	let num_precommits_before = justification.commit.precommits.len();
-	let justification = verify_and_optimize_justification::<TestHeader>(
+	verify_and_optimize_justification::<TestHeader>(
 		header_id::<TestHeader>(1),
 		TEST_GRANDPA_SET_ID,
 		&voter_set(),
-		justification,
+		&mut justification,
+	)
+	.unwrap();
+	let num_precommits_after = justification.commit.precommits.len();
+
+	assert_eq!(num_precommits_before - 1, num_precommits_after);
+}
+
+#[test]
+fn invalid_authority_signatures_are_removed_by_optimizer() {
+	let mut justification = make_default_justification::<TestHeader>(&test_header(1));
+
+	let target = header_id::<TestHeader>(1);
+	let invalid_raw_signature: Vec<u8> = ALICE.sign(b"").to_bytes().into();
+	justification.commit.precommits.insert(
+		0,
+		SignedPrecommit {
+			precommit: finality_grandpa::Precommit {
+				target_hash: target.0,
+				target_number: target.1,
+			},
+			signature: AuthoritySignature::try_from(invalid_raw_signature).unwrap(),
+			id: ALICE.into(),
+		},
+	);
+
+	let num_precommits_before = justification.commit.precommits.len();
+	verify_and_optimize_justification::<TestHeader>(
+		header_id::<TestHeader>(1),
+		TEST_GRANDPA_SET_ID,
+		&voter_set(),
+		&mut justification,
 	)
 	.unwrap();
 	let num_precommits_after = justification.commit.precommits.len();
@@ -267,14 +300,58 @@ fn redundant_authority_votes_are_removed_by_optimizer() {
 	));
 
 	let num_precommits_before = justification.commit.precommits.len();
-	let justification = verify_and_optimize_justification::<TestHeader>(
+	verify_and_optimize_justification::<TestHeader>(
 		header_id::<TestHeader>(1),
 		TEST_GRANDPA_SET_ID,
 		&voter_set(),
-		justification,
+		&mut justification,
 	)
 	.unwrap();
 	let num_precommits_after = justification.commit.precommits.len();
 
 	assert_eq!(num_precommits_before - 1, num_precommits_after);
 }
+
+#[test]
+fn unrelated_ancestry_votes_are_removed_by_optimizer() {
+	let mut justification = make_default_justification::<TestHeader>(&test_header(2));
+	justification.commit.precommits.insert(
+		0,
+		signed_precommit::<TestHeader>(
+			&ALICE,
+			header_id::<TestHeader>(1),
+			justification.round,
+			TEST_GRANDPA_SET_ID,
+		),
+	);
+
+	let num_precommits_before = justification.commit.precommits.len();
+	verify_and_optimize_justification::<TestHeader>(
+		header_id::<TestHeader>(2),
+		TEST_GRANDPA_SET_ID,
+		&voter_set(),
+		&mut justification,
+	)
+	.unwrap();
+	let num_precommits_after = justification.commit.precommits.len();
+
+	assert_eq!(num_precommits_before - 1, num_precommits_after);
+}
+
+#[test]
+fn redundant_votes_ancestries_are_removed_by_optimizer() {
+	let mut justification = make_default_justification::<TestHeader>(&test_header(1));
+	justification.votes_ancestries.push(test_header(100));
+
+	let num_votes_ancestries_before = justification.votes_ancestries.len();
+	verify_and_optimize_justification::<TestHeader>(
+		header_id::<TestHeader>(1),
+		TEST_GRANDPA_SET_ID,
+		&voter_set(),
+		&mut justification,
+	)
+	.unwrap();
+	let num_votes_ancestries_after = justification.votes_ancestries.len();
+
+	assert_eq!(num_votes_ancestries_before - 1, num_votes_ancestries_after);
+}

cumulus/bridges/primitives/messages/src/lib.rs

@@ -30,6 +30,7 @@ use frame_support::{PalletError, RuntimeDebug};
 // Weight is reexported to avoid additional frame-support dependencies in related crates.
 pub use frame_support::weights::Weight;
 use scale_info::TypeInfo;
+use serde::{Deserialize, Serialize};
 use source_chain::RelayersRewards;
 use sp_core::TypeId;
 use sp_std::{collections::vec_deque::VecDeque, ops::RangeInclusive, prelude::*};
@@ -49,8 +50,8 @@ pub mod target_chain;
 	RuntimeDebug,
 	TypeInfo,
 	MaxEncodedLen,
-	serde::Serialize,
-	serde::Deserialize,
+	Serialize,
+	Deserialize,
 )]
 pub enum MessagesOperatingMode {
 	/// Basic operating mode (Normal/Halted)

cumulus/bridges/primitives/polkadot-core/src/lib.rs

@@ -202,7 +202,7 @@ pub type AccountId = <AccountPublic as IdentifyAccount>::AccountId;
 /// Address of account on Polkadot-like chains.
 pub type AccountAddress = MultiAddress<AccountId, ()>;
 
-/// Index of a transaction on the Polkadot-like chains.
+/// Nonce of a transaction on the Polkadot-like chains.
 pub type Nonce = u32;
 
 /// Block type of Polkadot-like chains.
@@ -226,9 +226,11 @@ pub type Address = MultiAddress<AccountId, ()>;
 pub struct PolkadotLike;
 
 impl Chain for PolkadotLike {
+	type BlockNumber = BlockNumber;
 	type Hash = Hash;
 	type Hasher = Hasher;
-	type Block = Block;
+	type Header = Header;
+
 	type AccountId = AccountId;
 	type Balance = Balance;
 	type Nonce = Nonce;

cumulus/bridges/primitives/runtime/src/chain.rs

@@ -14,18 +14,18 @@
 // 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/>.
 
+use crate::HeaderIdProvider;
 use codec::{Decode, Encode, MaxEncodedLen};
 use frame_support::{weights::Weight, Parameter};
-use num_traits::{Bounded, CheckedSub, SaturatingAdd, Zero};
+use num_traits::{AsPrimitive, Bounded, CheckedSub, Saturating, SaturatingAdd, Zero};
 use sp_runtime::{
 	traits::{
-		AtLeast32Bit, AtLeast32BitUnsigned, Block as BlockT, Hash as HashT, Header as HeaderT,
-		HeaderProvider, MaybeDisplay, MaybeSerialize, MaybeSerializeDeserialize, Member,
-		SimpleBitOps, Verify,
+		AtLeast32Bit, AtLeast32BitUnsigned, Hash as HashT, Header as HeaderT, MaybeDisplay,
+		MaybeSerialize, MaybeSerializeDeserialize, Member, SimpleBitOps, Verify,
 	},
 	FixedPointOperand,
 };
-use sp_std::{convert::TryFrom, fmt::Debug, hash::Hash, vec, vec::Vec};
+use sp_std::{convert::TryFrom, fmt::Debug, hash::Hash, str::FromStr, vec, vec::Vec};
 
 /// Chain call, that is either SCALE-encoded, or decoded.
 #[derive(Debug, Clone, PartialEq)]
@@ -91,6 +91,27 @@ impl<ChainCall: Encode> Encode for EncodedOrDecodedCall<ChainCall> {
 
 /// Minimal Substrate-based chain representation that may be used from no_std environment.
 pub trait Chain: Send + Sync + 'static {
+	/// A type that fulfills the abstract idea of what a Substrate block number is.
+	// Constraits come from the associated Number type of `sp_runtime::traits::Header`
+	// See here for more info:
+	// https://crates.parity.io/sp_runtime/traits/trait.Header.html#associatedtype.Number
+	//
+	// Note that the `AsPrimitive<usize>` trait is required by the GRANDPA justification
+	// verifier, and is not usually part of a Substrate Header's Number type.
+	type BlockNumber: Parameter
+		+ Member
+		+ MaybeSerializeDeserialize
+		+ Hash
+		+ Copy
+		+ Default
+		+ MaybeDisplay
+		+ AtLeast32BitUnsigned
+		+ FromStr
+		+ AsPrimitive<usize>
+		+ Default
+		+ Saturating
+		+ MaxEncodedLen;
+
 	/// A type that fulfills the abstract idea of what a Substrate hash is.
 	// Constraits come from the associated Hash type of `sp_runtime::traits::Header`
 	// See here for more info:
@@ -115,10 +136,13 @@ pub trait Chain: Send + Sync + 'static {
 	// https://crates.parity.io/sp_runtime/traits/trait.Header.html#associatedtype.Hashing
 	type Hasher: HashT<Output = Self::Hash>;
 
-	/// A type that fulfills the abstract idea of what a Substrate block is.
+	/// A type that fulfills the abstract idea of what a Substrate header is.
 	// See here for more info:
-	// https://crates.parity.io/sp_runtime/traits/trait.Block.html
-	type Block: Parameter + BlockT<Hash = Self::Hash> + MaybeSerialize;
+	// https://crates.parity.io/sp_runtime/traits/trait.Header.html
+	type Header: Parameter
+		+ HeaderT<Number = Self::BlockNumber, Hash = Self::Hash>
+		+ HeaderIdProvider<Self::Header>
+		+ MaybeSerializeDeserialize;
 
 	/// The user account identifier type for the runtime.
 	type AccountId: Parameter
@@ -146,7 +170,7 @@ pub trait Chain: Send + Sync + 'static {
 		+ Zero
 		+ TryFrom<sp_core::U256>
 		+ MaxEncodedLen;
-	/// Index of a transaction used by the chain.
+	/// Nonce of a transaction used by the chain.
 	type Nonce: Parameter
 		+ Member
 		+ MaybeSerialize
@@ -176,9 +200,10 @@ impl<T> Chain for T
 where
 	T: Send + Sync + 'static + UnderlyingChainProvider,
 {
+	type BlockNumber = <T::Chain as Chain>::BlockNumber;
 	type Hash = <T::Chain as Chain>::Hash;
 	type Hasher = <T::Chain as Chain>::Hasher;
-	type Block = <T::Chain as Chain>::Block;
+	type Header = <T::Chain as Chain>::Header;
 	type AccountId = <T::Chain as Chain>::AccountId;
 	type Balance = <T::Chain as Chain>::Balance;
 	type Nonce = <T::Chain as Chain>::Nonce;
@@ -219,7 +244,7 @@ impl<Para: Parachain> frame_support::traits::Get<u32> for ParachainIdOf<Para> {
 pub type UnderlyingChainOf<C> = <C as UnderlyingChainProvider>::Chain;
 
 /// Block number used by the chain.
-pub type BlockNumberOf<C> = <<<C as Chain>::Block as HeaderProvider>::HeaderT as HeaderT>::Number;
+pub type BlockNumberOf<C> = <C as Chain>::BlockNumber;
 
 /// Hash type used by the chain.
 pub type HashOf<C> = <C as Chain>::Hash;
@@ -228,7 +253,7 @@ pub type HashOf<C> = <C as Chain>::Hash;
 pub type HasherOf<C> = <C as Chain>::Hasher;
 
 /// Header type used by the chain.
-pub type HeaderOf<C> = <<C as Chain>::Block as HeaderProvider>::HeaderT;
+pub type HeaderOf<C> = <C as Chain>::Header;
 
 /// Account id type used by the chain.
 pub type AccountIdOf<C> = <C as Chain>::AccountId;
@@ -236,7 +261,7 @@ pub type AccountIdOf<C> = <C as Chain>::AccountId;
 /// Balance type used by the chain.
 pub type BalanceOf<C> = <C as Chain>::Balance;
 
-/// Transaction index type used by the chain.
+/// Transaction nonce type used by the chain.
 pub type NonceOf<C> = <C as Chain>::Nonce;
 
 /// Signature type used by the chain.

cumulus/bridges/primitives/runtime/src/lib.rs

@@ -25,6 +25,7 @@ use frame_support::{
 };
 use frame_system::RawOrigin;
 use scale_info::TypeInfo;
+use serde::{Deserialize, Serialize};
 use sp_core::storage::StorageKey;
 use sp_runtime::traits::{BadOrigin, Header as HeaderT, UniqueSaturatedInto};
 use sp_std::{convert::TryFrom, fmt::Debug, ops::RangeInclusive, vec, vec::Vec};
@@ -383,8 +384,8 @@ pub trait OperatingMode: Send + Copy + Debug + FullCodec {
 	RuntimeDebug,
 	TypeInfo,
 	MaxEncodedLen,
-	serde::Serialize,
-	serde::Deserialize,
+	Serialize,
+	Deserialize,
 )]
 pub enum BasicOperatingMode {
 	/// Normal mode, when all operations are allowed.