diff --git a/bridges/bin/runtime-common/src/integrity.rs b/bridges/bin/runtime-common/src/integrity.rs index 1292a4fbaa3..a0af3b981f3 100644 --- a/bridges/bin/runtime-common/src/integrity.rs +++ b/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>); diff --git a/bridges/bin/runtime-common/src/mock.rs b/bridges/bin/runtime-common/src/mock.rs index f7e5fc7daa3..6b5edabc886 100644 --- a/bridges/bin/runtime-common/src/mock.rs +++ b/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; diff --git a/bridges/bin/runtime-common/src/parachains_benchmarking.rs b/bridges/bin/runtime-common/src/parachains_benchmarking.rs index 53095784cb1..aad53673c3a 100644 --- a/bridges/bin/runtime-common/src/parachains_benchmarking.rs +++ b/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]); diff --git a/bridges/bin/runtime-common/src/refund_relayer_extension.rs b/bridges/bin/runtime-common/src/refund_relayer_extension.rs index 1beacd981ee..c5419837316 100644 --- a/bridges/bin/runtime-common/src/refund_relayer_extension.rs +++ b/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; diff --git a/bridges/modules/grandpa/src/call_ext.rs b/bridges/modules/grandpa/src/call_ext.rs index 868b6626955..c83e88b7b56 100644 --- a/bridges/modules/grandpa/src/call_ext.rs +++ b/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) diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index 8ff128ef20f..eb49849ac88 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/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}; diff --git a/bridges/modules/grandpa/src/mock.rs b/bridges/modules/grandpa/src/mock.rs index 819875b870a..bd305dfef9d 100644 --- a/bridges/modules/grandpa/src/mock.rs +++ b/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; diff --git a/bridges/modules/parachains/src/benchmarking.rs b/bridges/modules/parachains/src/benchmarking.rs index f89fbb0f361..59c4642cde9 100644 --- a/bridges/modules/parachains/src/benchmarking.rs +++ b/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, >, diff --git a/bridges/modules/parachains/src/lib.rs b/bridges/modules/parachains/src/lib.rs index 6f28bfc1b08..4f78a45d4b7 100644 --- a/bridges/modules/parachains/src/lib.rs +++ b/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, ); diff --git a/bridges/modules/parachains/src/mock.rs b/bridges/modules/parachains/src/mock.rs index c72298a5e1d..a7030f0ae03 100644 --- a/bridges/modules/parachains/src/mock.rs +++ b/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; diff --git a/bridges/modules/relayers/src/mock.rs b/bridges/modules/relayers/src/mock.rs index e9ba058bc4c..b3fcb24cdd2 100644 --- a/bridges/modules/relayers/src/mock.rs +++ b/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 = (); diff --git a/bridges/primitives/chain-bridge-hub-cumulus/src/lib.rs b/bridges/primitives/chain-bridge-hub-cumulus/src/lib.rs index f0826952209..525b2e62cea 100644 --- a/bridges/primitives/chain-bridge-hub-cumulus/src/lib.rs +++ b/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. diff --git a/bridges/primitives/chain-bridge-hub-kusama/src/lib.rs b/bridges/primitives/chain-bridge-hub-kusama/src/lib.rs index ac899256343..7405f561fb2 100644 --- a/bridges/primitives/chain-bridge-hub-kusama/src/lib.rs +++ b/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; diff --git a/bridges/primitives/chain-bridge-hub-polkadot/src/lib.rs b/bridges/primitives/chain-bridge-hub-polkadot/src/lib.rs index 76af4b943c4..e1fc0d7bc47 100644 --- a/bridges/primitives/chain-bridge-hub-polkadot/src/lib.rs +++ b/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; diff --git a/bridges/primitives/chain-bridge-hub-rococo/src/lib.rs b/bridges/primitives/chain-bridge-hub-rococo/src/lib.rs index a11bc6b8e1f..50206c8e6b3 100644 --- a/bridges/primitives/chain-bridge-hub-rococo/src/lib.rs +++ b/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; diff --git a/bridges/primitives/chain-bridge-hub-wococo/src/lib.rs b/bridges/primitives/chain-bridge-hub-wococo/src/lib.rs index 71010695337..7d14460c737 100644 --- a/bridges/primitives/chain-bridge-hub-wococo/src/lib.rs +++ b/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; diff --git a/bridges/primitives/chain-kusama/src/lib.rs b/bridges/primitives/chain-kusama/src/lib.rs index e4b5d330354..229905a3d4a 100644 --- a/bridges/primitives/chain-kusama/src/lib.rs +++ b/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; diff --git a/bridges/primitives/chain-polkadot/src/lib.rs b/bridges/primitives/chain-polkadot/src/lib.rs index b57486916d2..628634bb46f 100644 --- a/bridges/primitives/chain-polkadot/src/lib.rs +++ b/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; diff --git a/bridges/primitives/chain-rococo/src/lib.rs b/bridges/primitives/chain-rococo/src/lib.rs index b8a6b47b423..a825c8b3978 100644 --- a/bridges/primitives/chain-rococo/src/lib.rs +++ b/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; diff --git a/bridges/primitives/chain-wococo/src/lib.rs b/bridges/primitives/chain-wococo/src/lib.rs index 00653267e70..fb63613427d 100644 --- a/bridges/primitives/chain-wococo/src/lib.rs +++ b/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; diff --git a/bridges/primitives/header-chain/src/justification.rs b/bridges/primitives/header-chain/src/justification.rs index a90f4bab94a..714546a42ef 100644 --- a/bridges/primitives/header-chain/src/justification.rs +++ b/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>); - -impl OptimizationCallbacks { - fn optimize<Header: HeaderT>( - self, - mut justification: GrandpaJustification<Header>, - ) -> GrandpaJustification<Header> { - for invalid_precommit_idx in self.0.into_iter().rev() { +struct OptimizationCallbacks<Header: HeaderT> { + extra_precommits: Vec<usize>, + redundant_votes_ancestries: BTreeSet<Header::Hash>, +} + +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 there are no extra headers in the justification - if !chain.unvisited.is_empty() { - return Err(Error::ExtraHeadersInVotesAncestries) + // 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 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) + // check that there are no extra headers in the justification + if !chain.is_fully_visited() { + callbacks.on_redundant_votes_ancestries(chain.unvisited)?; } + + 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(¤t_hash); current_hash = match self.parents.get(¤t_hash) { Some(parent_hash) => { + let is_visited_before = self.unvisited.get(¤t_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() } } diff --git a/bridges/primitives/header-chain/src/lib.rs b/bridges/primitives/header-chain/src/lib.rs index cf08a936234..5268e7d5c5f 100644 --- a/bridges/primitives/header-chain/src/lib.rs +++ b/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() } } diff --git a/bridges/primitives/header-chain/tests/implementation_match.rs b/bridges/primitives/header-chain/tests/implementation_match.rs index 22f690b8cb2..d5e42e21497 100644 --- a/bridges/primitives/header-chain/tests/implementation_match.rs +++ b/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(¤t_hash).cloned() { + match self.0.parents.get(¤t_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(); diff --git a/bridges/primitives/header-chain/tests/justification.rs b/bridges/primitives/header-chain/tests/justification.rs index 3cd63b935d0..26ed67fa65f 100644 --- a/bridges/primitives/header-chain/tests/justification.rs +++ b/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); +} diff --git a/bridges/primitives/messages/src/lib.rs b/bridges/primitives/messages/src/lib.rs index 3df039d7eb8..cb3a14572d7 100644 --- a/bridges/primitives/messages/src/lib.rs +++ b/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) diff --git a/bridges/primitives/polkadot-core/src/lib.rs b/bridges/primitives/polkadot-core/src/lib.rs index 2f812a79538..ffd48d8a389 100644 --- a/bridges/primitives/polkadot-core/src/lib.rs +++ b/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; diff --git a/bridges/primitives/runtime/src/chain.rs b/bridges/primitives/runtime/src/chain.rs index fa0d82311e3..8c47662a7c1 100644 --- a/bridges/primitives/runtime/src/chain.rs +++ b/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. diff --git a/bridges/primitives/runtime/src/lib.rs b/bridges/primitives/runtime/src/lib.rs index 7ba20e11e22..c394af37fa4 100644 --- a/bridges/primitives/runtime/src/lib.rs +++ b/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.