From d6149f20e4525c32c9d00972994e4c63d201c125 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Tue, 21 Feb 2023 09:33:07 +0100 Subject: [PATCH] Squashed 'bridges/' changes from d39f563be..78e3357c0 78e3357c0 RefundRelayerForMessagesFromParachain improvements (#1895) 131b17359 optimize justification before submit (#1887) 5bc279ebb use complex transactions on RBH/WBH bridge hubs (#1893) 8f0c09ab9 Bump clap from 4.1.4 to 4.1.6 66429b06a Bump sysinfo from 0.27.7 to 0.28.0 8b329ee8f Bump trie-db from 0.25.0 to 0.25.1 635cfccfd Bump time from 0.3.17 to 0.3.19 git-subtree-dir: bridges git-subtree-split: 78e3357c0387c95317b8c3e5c4d9316f3a9f3ef4 --- Cargo.lock | 36 +- bin/millau/node/Cargo.toml | 2 +- bin/millau/runtime/src/lib.rs | 28 +- bin/rialto-parachain/node/Cargo.toml | 2 +- bin/rialto/node/Cargo.toml | 2 +- bin/runtime-common/src/messages_call_ext.rs | 6 +- .../src/refund_relayer_extension.rs | 347 ++++++++++-------- modules/grandpa/src/lib.rs | 5 + modules/parachains/src/call_ext.rs | 9 +- primitives/header-chain/src/justification.rs | 121 +++++- primitives/header-chain/src/storage_keys.rs | 26 ++ .../header-chain/tests/justification.rs | 86 ++++- primitives/runtime/Cargo.toml | 2 +- primitives/runtime/src/lib.rs | 24 ++ primitives/test-utils/src/lib.rs | 5 +- ...ub_rococo_messages_to_bridge_hub_wococo.rs | 6 +- ...ub_wococo_messages_to_bridge_hub_rococo.rs | 6 +- relays/client-bridge-hub-rococo/src/lib.rs | 9 +- .../src/runtime_wrapper.rs | 11 +- relays/client-bridge-hub-wococo/src/lib.rs | 9 +- .../src/runtime_wrapper.rs | 11 +- relays/client-millau/src/lib.rs | 2 +- relays/client-substrate/src/calls.rs | 9 + relays/client-substrate/src/chain.rs | 17 + relays/client-substrate/src/lib.rs | 4 +- .../src/finality/engine.rs | 51 ++- .../src/finality/target.rs | 4 + relays/utils/Cargo.toml | 2 +- 28 files changed, 628 insertions(+), 214 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a5298107be1..da7247b1c52 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1034,7 +1034,7 @@ dependencies = [ "sp-state-machine 0.13.0", "sp-std 5.0.0", "sp-trie 7.0.0", - "trie-db 0.25.0", + "trie-db 0.25.1", ] [[package]] @@ -1392,9 +1392,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.1.4" +version = "4.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f13b9c79b5d1dd500d20ef541215a6423c75829ef43117e1b4d17fd8af0b5d76" +checksum = "ec0b0588d44d4d63a87dbd75c136c166bbfd9a86a31cb89e09906521c7d3f5e3" dependencies = [ "bitflags", "clap_derive", @@ -1818,7 +1818,7 @@ name = "cumulus-client-cli" version = "0.1.0" source = "git+https://github.com/paritytech/cumulus?branch=master#445f9277ab55b4d930ced4fbbb38d27c617c6658" dependencies = [ - "clap 4.1.4", + "clap 4.1.6", "parity-scale-codec", "sc-chain-spec", "sc-cli", @@ -3151,7 +3151,7 @@ dependencies = [ "Inflector", "array-bytes 4.2.0", "chrono", - "clap 4.1.4", + "clap 4.1.6", "comfy-table", "frame-benchmarking", "frame-support", @@ -5451,7 +5451,7 @@ version = "0.1.0" dependencies = [ "beefy-gadget", "beefy-gadget-rpc", - "clap 4.1.4", + "clap 4.1.6", "frame-benchmarking", "frame-benchmarking-cli", "jsonrpsee 0.16.2", @@ -5859,7 +5859,7 @@ name = "node-inspect" version = "0.9.0-dev" source = "git+https://github.com/paritytech/substrate?branch=master#51cf2e790f118f6cfe8d94fb9ca96bc580bd7c98" dependencies = [ - "clap 4.1.4", + "clap 4.1.6", "parity-scale-codec", "sc-cli", "sc-client-api", @@ -7554,7 +7554,7 @@ name = "polkadot-cli" version = "0.9.37" source = "git+https://github.com/paritytech/polkadot?branch=master#4f331d74c3004d9765b735ec66acc92edea62c7f" dependencies = [ - "clap 4.1.4", + "clap 4.1.6", "frame-benchmarking-cli", "futures", "log", @@ -9432,7 +9432,7 @@ dependencies = [ name = "rialto-bridge-node" version = "0.1.0" dependencies = [ - "clap 4.1.4", + "clap 4.1.6", "frame-benchmarking", "frame-benchmarking-cli", "frame-support", @@ -9459,7 +9459,7 @@ dependencies = [ name = "rialto-parachain-collator" version = "0.1.0" dependencies = [ - "clap 4.1.4", + "clap 4.1.6", "cumulus-client-cli", "cumulus-client-consensus-aura", "cumulus-client-consensus-common", @@ -9999,7 +9999,7 @@ source = "git+https://github.com/paritytech/substrate?branch=master#51cf2e790f11 dependencies = [ "array-bytes 4.2.0", "chrono", - "clap 4.1.4", + "clap 4.1.6", "fdlimit", "futures", "libp2p", @@ -10793,7 +10793,7 @@ name = "sc-storage-monitor" version = "0.1.0" source = "git+https://github.com/paritytech/substrate?branch=master#51cf2e790f118f6cfe8d94fb9ca96bc580bd7c98" dependencies = [ - "clap 4.1.4", + "clap 4.1.6", "futures", "log", "nix 0.26.2", @@ -13015,9 +13015,9 @@ dependencies = [ [[package]] name = "sysinfo" -version = "0.27.7" +version = "0.28.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "975fe381e0ecba475d4acff52466906d95b153a40324956552e027b2a9eaa89e" +checksum = "727220a596b4ca0af040a07091e49f5c105ec8f2592674339a5bf35be592f76e" dependencies = [ "cfg-if 1.0.0", "core-foundation-sys", @@ -13532,9 +13532,9 @@ dependencies = [ [[package]] name = "trie-db" -version = "0.25.0" +version = "0.25.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9bcd4e7fbb3bcab17b02596b53876e36eb39663ddd87884bfd88c69cdeb2ebd6" +checksum = "3390c0409daaa6027d6681393316f4ccd3ff82e1590a1e4725014e3ae2bf1920" dependencies = [ "hash-db", "hashbrown 0.13.2", @@ -13609,7 +13609,7 @@ name = "try-runtime-cli" version = "0.10.0-dev" source = "git+https://github.com/paritytech/substrate?branch=master#51cf2e790f118f6cfe8d94fb9ca96bc580bd7c98" dependencies = [ - "clap 4.1.4", + "clap 4.1.6", "frame-remote-externalities", "hex", "log", @@ -13665,7 +13665,7 @@ version = "1.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "97fee6b57c6a41524a810daee9286c02d7752c4253064d0b05472833a438f675" dependencies = [ - "cfg-if 0.1.10", + "cfg-if 1.0.0", "digest 0.10.6", "rand 0.8.5", "static_assertions", diff --git a/bin/millau/node/Cargo.toml b/bin/millau/node/Cargo.toml index fea96c9b11f..3d3d238943d 100644 --- a/bin/millau/node/Cargo.toml +++ b/bin/millau/node/Cargo.toml @@ -9,7 +9,7 @@ repository = "https://github.com/paritytech/parity-bridges-common/" license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] -clap = { version = "4.1.4", features = ["derive"] } +clap = { version = "4.1.6", features = ["derive"] } jsonrpsee = { version = "0.16.2", features = ["server"] } serde_json = "1.0.93" diff --git a/bin/millau/runtime/src/lib.rs b/bin/millau/runtime/src/lib.rs index 5a18f8b12c0..369bea2b1fa 100644 --- a/bin/millau/runtime/src/lib.rs +++ b/bin/millau/runtime/src/lib.rs @@ -80,7 +80,13 @@ pub use pallet_sudo::Call as SudoCall; pub use pallet_timestamp::Call as TimestampCall; pub use pallet_xcm::Call as XcmCall; -use bridge_runtime_common::generate_bridge_reject_obsolete_headers_and_messages; +use bridge_runtime_common::{ + generate_bridge_reject_obsolete_headers_and_messages, + refund_relayer_extension::{ + ActualFeeRefund, RefundBridgedParachainMessages, RefundableMessagesLane, + RefundableParachain, + }, +}; #[cfg(any(feature = "std", test))] pub use sp_runtime::BuildStorage; pub use sp_runtime::{Perbill, Permill}; @@ -582,17 +588,15 @@ generate_bridge_reject_obsolete_headers_and_messages! { BridgeRialtoMessages, BridgeRialtoParachainMessages } +bp_runtime::generate_static_str_provider!(BridgeRefundRialtoPara2000Lane0Msgs); /// Signed extension that refunds relayers that are delivering messages from the Rialto parachain. -pub type BridgeRefundRialtoParachainRelayers = - bridge_runtime_common::refund_relayer_extension::RefundRelayerForMessagesFromParachain< - Runtime, - RialtoGrandpaInstance, - WithRialtoParachainsInstance, - WithRialtoParachainMessagesInstance, - RialtoParachainId, - RialtoParachainMessagesLane, - Runtime, - >; +pub type BridgeRefundRialtoParachainMessages = RefundBridgedParachainMessages< + Runtime, + RefundableParachain, + RefundableMessagesLane, + ActualFeeRefund, + StrBridgeRefundRialtoPara2000Lane0Msgs, +>; /// The address format for describing accounts. pub type Address = AccountId; @@ -615,7 +619,7 @@ pub type SignedExtra = ( frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, BridgeRejectObsoleteHeadersAndMessages, - BridgeRefundRialtoParachainRelayers, + BridgeRefundRialtoParachainMessages, ); /// The payload being signed in transactions. pub type SignedPayload = generic::SignedPayload; diff --git a/bin/rialto-parachain/node/Cargo.toml b/bin/rialto-parachain/node/Cargo.toml index 5e1eba1fee5..c20b7f248cc 100644 --- a/bin/rialto-parachain/node/Cargo.toml +++ b/bin/rialto-parachain/node/Cargo.toml @@ -17,7 +17,7 @@ default = [] runtime-benchmarks = ['rialto-parachain-runtime/runtime-benchmarks'] [dependencies] -clap = { version = "4.1.4", features = ["derive"] } +clap = { version = "4.1.6", features = ["derive"] } log = '0.4.17' codec = { package = 'parity-scale-codec', version = '3.1.5' } serde = { version = '1.0', features = ['derive'] } diff --git a/bin/rialto/node/Cargo.toml b/bin/rialto/node/Cargo.toml index d03deaaabef..556088498f7 100644 --- a/bin/rialto/node/Cargo.toml +++ b/bin/rialto/node/Cargo.toml @@ -9,7 +9,7 @@ repository = "https://github.com/paritytech/parity-bridges-common/" license = "GPL-3.0-or-later WITH Classpath-exception-2.0" [dependencies] -clap = { version = "4.1.4", features = ["derive"] } +clap = { version = "4.1.6", features = ["derive"] } serde_json = "1.0.93" # Bridge dependencies diff --git a/bin/runtime-common/src/messages_call_ext.rs b/bin/runtime-common/src/messages_call_ext.rs index 20e604142d9..740d17129c8 100644 --- a/bin/runtime-common/src/messages_call_ext.rs +++ b/bin/runtime-common/src/messages_call_ext.rs @@ -18,12 +18,12 @@ use crate::messages::{ source::FromBridgedChainMessagesDeliveryProof, target::FromBridgedChainMessagesProof, }; use bp_messages::{LaneId, MessageNonce}; -use frame_support::{dispatch::CallableCallFor, traits::IsSubType}; +use frame_support::{dispatch::CallableCallFor, traits::IsSubType, RuntimeDebug}; use pallet_bridge_messages::{Config, Pallet}; -use sp_runtime::{transaction_validity::TransactionValidity, RuntimeDebug}; +use sp_runtime::transaction_validity::TransactionValidity; /// Info about a `ReceiveMessagesProof` call which tries to update a single lane. -#[derive(Copy, Clone, PartialEq, RuntimeDebug)] +#[derive(PartialEq, RuntimeDebug)] pub struct ReceiveMessagesProofInfo { pub lane_id: LaneId, pub best_proof_nonce: MessageNonce, diff --git a/bin/runtime-common/src/refund_relayer_extension.rs b/bin/runtime-common/src/refund_relayer_extension.rs index ac65617483d..9efeedf24dd 100644 --- a/bin/runtime-common/src/refund_relayer_extension.rs +++ b/bin/runtime-common/src/refund_relayer_extension.rs @@ -23,6 +23,7 @@ use crate::messages_call_ext::{ MessagesCallSubType, ReceiveMessagesProofHelper, ReceiveMessagesProofInfo, }; use bp_messages::LaneId; +use bp_runtime::StaticStrProvider; use codec::{Decode, Encode}; use frame_support::{ dispatch::{CallableCallFor, DispatchInfo, Dispatchable, PostDispatchInfo}, @@ -46,28 +47,77 @@ use sp_runtime::{ }; use sp_std::{marker::PhantomData, vec, vec::Vec}; -// TODO (https://github.com/paritytech/parity-bridges-common/issues/1667): -// support multiple bridges in this extension +// without this typedef rustfmt fails with internal err +type BalanceOf = + <::OnChargeTransaction as OnChargeTransaction>::Balance; +type CallOf = ::RuntimeCall; + +/// Trait identifying a bridged parachain. A relayer might be refunded for delivering messages +/// coming from this parachain. +trait RefundableParachainId { + /// The instance of the bridge parachains pallet. + type Instance; + /// The parachain Id. + type Id: Get; +} -/// Transaction fee calculation. -pub trait TransactionFeeCalculation { - /// Compute fee that is paid for given transaction. The fee is later refunded to relayer. - fn compute_fee( +/// Default implementation of `RefundableParachainId`. +pub struct RefundableParachain(PhantomData<(Instance, Id)>); + +impl RefundableParachainId for RefundableParachain +where + Id: Get, +{ + type Instance = Instance; + type Id = Id; +} + +/// Trait identifying a bridged messages lane. A relayer might be refunded for delivering messages +/// coming from this lane. +trait RefundableMessagesLaneId { + /// The instance of the bridge messages pallet. + type Instance; + /// The messages lane id. + type Id: Get; +} + +/// Default implementation of `RefundableMessagesLaneId`. +pub struct RefundableMessagesLane(PhantomData<(Instance, Id)>); + +impl RefundableMessagesLaneId for RefundableMessagesLane +where + Id: Get, +{ + type Instance = Instance; + type Id = Id; +} + +/// Refund calculator. +pub trait RefundCalculator { + // The underlying integer type in which the refund is calculated. + type Balance; + + /// Compute refund for given transaction. + fn compute_refund( info: &DispatchInfo, post_info: &PostDispatchInfo, len: usize, - tip: Balance, - ) -> Balance; + tip: Self::Balance, + ) -> Self::Balance; } -impl TransactionFeeCalculation> for R +/// `RefundCalculator` implementation which refunds the actual transaction fee. +pub struct ActualFeeRefund(PhantomData); + +impl RefundCalculator for ActualFeeRefund where R: TransactionPaymentConfig, - ::RuntimeCall: - Dispatchable, + CallOf: Dispatchable, BalanceOf: FixedPointOperand, { - fn compute_fee( + type Balance = BalanceOf; + + fn compute_refund( info: &DispatchInfo, post_info: &PostDispatchInfo, len: usize, @@ -77,7 +127,55 @@ where } } -/// Signed extension that refunds relayer for new messages coming from the parachain. +/// Data that is crafted in `pre_dispatch` method and used at `post_dispatch`. +#[cfg_attr(test, derive(Debug, PartialEq))] +pub struct PreDispatchData { + /// Transaction submitter (relayer) account. + relayer: AccountId, + /// Type of the call. + call_info: CallInfo, +} + +/// Type of the call that the extension recognizes. +#[derive(RuntimeDebugNoBound, PartialEq)] +pub enum CallInfo { + /// Relay chain finality + parachain finality + message delivery calls. + AllFinalityAndDelivery(RelayBlockNumber, SubmitParachainHeadsInfo, ReceiveMessagesProofInfo), + /// Parachain finality + message delivery calls. + ParachainFinalityAndDelivery(SubmitParachainHeadsInfo, ReceiveMessagesProofInfo), + /// Standalone message delivery call. + Delivery(ReceiveMessagesProofInfo), +} + +impl CallInfo { + /// Returns the pre-dispatch `finality_target` sent to the `SubmitFinalityProof` call. + fn submit_finality_proof_info(&self) -> Option { + match *self { + Self::AllFinalityAndDelivery(info, _, _) => Some(info), + _ => None, + } + } + + /// Returns the pre-dispatch `SubmitParachainHeadsInfo`. + fn submit_parachain_heads_info(&self) -> Option<&SubmitParachainHeadsInfo> { + match self { + Self::AllFinalityAndDelivery(_, info, _) => Some(info), + Self::ParachainFinalityAndDelivery(info, _) => Some(info), + _ => None, + } + } + + /// Returns the pre-dispatch `ReceiveMessagesProofInfo`. + fn receive_messages_proof_info(&self) -> &ReceiveMessagesProofInfo { + match self { + Self::AllFinalityAndDelivery(_, _, info) => info, + Self::ParachainFinalityAndDelivery(_, info) => info, + Self::Delivery(info) => info, + } + } +} + +/// Signed extension that refunds a relayer for new messages coming from a parachain. /// /// Also refunds relayer for successful finality delivery if it comes in batch (`utility.batchAll`) /// with message delivery transaction. Batch may deliver either both relay chain header and @@ -86,29 +184,29 @@ where /// /// Extension does not refund transaction tip due to security reasons. #[derive( + DefaultNoBound, CloneNoBound, Decode, - DefaultNoBound, Encode, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, TypeInfo, )] -#[scale_info(skip_type_params(RT, GI, PI, MI, PID, LID, FEE))] -pub struct RefundRelayerForMessagesFromParachain( - PhantomData<(RT, GI, PI, MI, PID, LID, FEE)>, +#[scale_info(skip_type_params(Runtime, Para, Msgs, Refund, Id))] +pub struct RefundBridgedParachainMessages( + PhantomData<(Runtime, Para, Msgs, Refund, Id)>, ); -impl - RefundRelayerForMessagesFromParachain +impl + RefundBridgedParachainMessages where - R: UtilityConfig>, - CallOf: IsSubType, R>>, + Runtime: UtilityConfig>, + CallOf: IsSubType, Runtime>>, { - fn expand_call<'a>(&self, call: &'a CallOf) -> Option>> { + fn expand_call<'a>(&self, call: &'a CallOf) -> Option>> { let calls = match call.is_sub_type() { - Some(UtilityCall::::batch_all { ref calls }) => { + Some(UtilityCall::::batch_all { ref calls }) => { if calls.len() > 3 { return None } @@ -123,71 +221,30 @@ where } } -/// Data that is crafted in `pre_dispatch` method and used at `post_dispatch`. -#[derive(PartialEq)] -#[cfg_attr(test, derive(Debug))] -pub struct PreDispatchData { - /// Transaction submitter (relayer) account. - relayer: AccountId, - /// Type of the call. - pub call_type: CallType, -} - -/// Type of the call that the extension recognizes. -#[derive(Clone, Copy, PartialEq, RuntimeDebugNoBound)] -pub enum CallType { - /// Relay chain finality + parachain finality + message delivery calls. - AllFinalityAndDelivery(RelayBlockNumber, SubmitParachainHeadsInfo, ReceiveMessagesProofInfo), - /// Parachain finality + message delivery calls. - ParachainFinalityAndDelivery(SubmitParachainHeadsInfo, ReceiveMessagesProofInfo), - /// Standalone message delivery call. - Delivery(ReceiveMessagesProofInfo), -} - -impl CallType { - /// Returns the pre-dispatch messages pallet state. - fn receive_messages_proof_info(&self) -> ReceiveMessagesProofInfo { - match *self { - Self::AllFinalityAndDelivery(_, _, info) => info, - Self::ParachainFinalityAndDelivery(_, info) => info, - Self::Delivery(info) => info, - } - } -} - -// without this typedef rustfmt fails with internal err -type BalanceOf = - <::OnChargeTransaction as OnChargeTransaction>::Balance; -type CallOf = ::RuntimeCall; - -impl SignedExtension - for RefundRelayerForMessagesFromParachain +impl SignedExtension + for RefundBridgedParachainMessages where - R: 'static - + Send - + Sync - + UtilityConfig> - + BoundedBridgeGrandpaConfig - + ParachainsConfig - + MessagesConfig + Self: 'static + Send + Sync, + Runtime: UtilityConfig> + + BoundedBridgeGrandpaConfig + + ParachainsConfig + + MessagesConfig + RelayersConfig, - GI: 'static + Send + Sync, - PI: 'static + Send + Sync, - MI: 'static + Send + Sync, - PID: 'static + Send + Sync + Get, - LID: 'static + Send + Sync + Get, - FEE: 'static + Send + Sync + TransactionFeeCalculation<::Reward>, - CallOf: Dispatchable - + IsSubType, R>> - + GrandpaCallSubType - + ParachainsCallSubType - + MessagesCallSubType, + Para: RefundableParachainId, + Msgs: RefundableMessagesLaneId, + Refund: RefundCalculator, + Id: StaticStrProvider, + CallOf: Dispatchable + + IsSubType, Runtime>> + + GrandpaCallSubType + + ParachainsCallSubType + + MessagesCallSubType, { - const IDENTIFIER: &'static str = "RefundRelayerForMessagesFromParachain"; - type AccountId = R::AccountId; - type Call = CallOf; + const IDENTIFIER: &'static str = Id::STR; + type AccountId = Runtime::AccountId; + type Call = CallOf; type AdditionalSigned = (); - type Pre = Option>; + type Pre = Option>; fn additional_signed(&self) -> Result<(), TransactionValidityError> { Ok(()) @@ -200,15 +257,12 @@ where _info: &DispatchInfoOf, _len: usize, ) -> TransactionValidity { - let calls = match self.expand_call(call) { - Some(calls) => calls, - None => return Ok(ValidTransaction::default()), - }; - - for call in calls { - call.check_obsolete_submit_finality_proof()?; - call.check_obsolete_submit_parachain_heads()?; - call.check_obsolete_receive_messages_proof()?; + if let Some(calls) = self.expand_call(call) { + for nested_call in calls { + nested_call.check_obsolete_submit_finality_proof()?; + nested_call.check_obsolete_submit_parachain_heads()?; + nested_call.check_obsolete_receive_messages_proof()?; + } } Ok(ValidTransaction::default()) @@ -225,35 +279,35 @@ where self.validate(who, call, info, len).map(drop)?; // Try to check if the tx matches one of types we support. - let parse_call_type = || { + let parse_call = || { let mut calls = self.expand_call(call)?.into_iter(); match calls.len() { - 3 => Some(CallType::AllFinalityAndDelivery( + 3 => Some(CallInfo::AllFinalityAndDelivery( calls.next()?.submit_finality_proof_info()?, - calls.next()?.submit_parachain_heads_info_for(PID::get())?, - calls.next()?.receive_messages_proof_info_for(LID::get())?, + calls.next()?.submit_parachain_heads_info_for(Para::Id::get())?, + calls.next()?.receive_messages_proof_info_for(Msgs::Id::get())?, )), - 2 => Some(CallType::ParachainFinalityAndDelivery( - calls.next()?.submit_parachain_heads_info_for(PID::get())?, - calls.next()?.receive_messages_proof_info_for(LID::get())?, + 2 => Some(CallInfo::ParachainFinalityAndDelivery( + calls.next()?.submit_parachain_heads_info_for(Para::Id::get())?, + calls.next()?.receive_messages_proof_info_for(Msgs::Id::get())?, )), - 1 => Some(CallType::Delivery( - calls.next()?.receive_messages_proof_info_for(LID::get())?, + 1 => Some(CallInfo::Delivery( + calls.next()?.receive_messages_proof_info_for(Msgs::Id::get())?, )), _ => None, } }; - Ok(parse_call_type().map(|call_type| { + Ok(parse_call().map(|call_info| { log::trace!( target: "runtime::bridge", - "RefundRelayerForMessagesFromParachain from parachain {} via {:?} \ - parsed bridge transaction in pre-dispatch: {:?}", - PID::get(), - LID::get(), - call_type, + "{} from parachain {} via {:?} parsed bridge transaction in pre-dispatch: {:?}", + Self::IDENTIFIER, + Para::Id::get(), + Msgs::Id::get(), + call_info, ); - PreDispatchData { relayer: who.clone(), call_type } + PreDispatchData { relayer: who.clone(), call_info } })) } @@ -270,14 +324,14 @@ where } // We don't refund anything for transactions that we don't support. - let (relayer, call_type) = match pre { - Some(Some(pre)) => (pre.relayer, pre.call_type), + let (relayer, call_info) = match pre { + Some(Some(pre)) => (pre.relayer, pre.call_info), _ => return Ok(()), }; // check if relay chain state has been updated - if let CallType::AllFinalityAndDelivery(relay_block_number, _, _) = call_type { - if !SubmitFinalityProofHelper::::was_successful(relay_block_number) { + if let Some(relay_block_number) = call_info.submit_finality_proof_info() { + if !SubmitFinalityProofHelper::::was_successful(relay_block_number) { // we only refund relayer if all calls have updated chain state return Ok(()) } @@ -291,44 +345,44 @@ where } // check if parachain state has been updated - match call_type { - CallType::AllFinalityAndDelivery(_, parachain_heads_info, _) | - CallType::ParachainFinalityAndDelivery(parachain_heads_info, _) => { - if !SubmitParachainHeadsHelper::::was_successful(¶chain_heads_info) { - // we only refund relayer if all calls have updated chain state - return Ok(()) - } - }, - _ => (), + if let Some(para_proof_info) = call_info.submit_parachain_heads_info() { + if !SubmitParachainHeadsHelper::::was_successful( + para_proof_info, + ) { + // we only refund relayer if all calls have updated chain state + return Ok(()) + } } // Check if the `ReceiveMessagesProof` call delivered at least some of the messages that // it contained. If this happens, we consider the transaction "helpful" and refund it. - let messages_proof_info = call_type.receive_messages_proof_info(); - if !ReceiveMessagesProofHelper::::was_partially_successful(&messages_proof_info) { + let msgs_proof_info = call_info.receive_messages_proof_info(); + if !ReceiveMessagesProofHelper::::was_partially_successful( + msgs_proof_info, + ) { return Ok(()) } // regarding the tip - refund that happens here (at this side of the bridge) isn't the whole // relayer compensation. He'll receive some amount at the other side of the bridge. It shall - // (in theory) cover the tip here. Otherwise, if we'll be compensating tip here, some + // (in theory) cover the tip there. Otherwise, if we'll be compensating tip here, some // malicious relayer may use huge tips, effectively depleting account that pay rewards. The // cost of this attack is nothing. Hence we use zero as tip here. let tip = Zero::zero(); - // compute the relayer reward - let reward = FEE::compute_fee(info, post_info, len, tip); - - // finally - register reward in relayers pallet - RelayersPallet::::register_relayer_reward(LID::get(), &relayer, reward); + // compute the relayer refund + let refund = Refund::compute_refund(info, post_info, len, tip); + // finally - register refund in relayers pallet + RelayersPallet::::register_relayer_reward(Msgs::Id::get(), &relayer, refund); log::trace!( target: "runtime::bridge", - "RefundRelayerForMessagesFromParachain from parachain {} via {:?} has registered {:?} reward: {:?}", - PID::get(), - LID::get(), + "{} from parachain {} via {:?} has registered reward: {:?} for {:?}", + Self::IDENTIFIER, + Para::Id::get(), + Msgs::Id::get(), + refund, relayer, - reward, ); Ok(()) @@ -353,18 +407,17 @@ mod tests { }; parameter_types! { - pub TestParachain: u32 = 1000; + TestParachain: u32 = 1000; pub TestLaneId: LaneId = TEST_LANE_ID; } - type TestExtension = RefundRelayerForMessagesFromParachain< - TestRuntime, - (), - (), - (), - TestParachain, - TestLaneId, + bp_runtime::generate_static_str_provider!(TestExtension); + type TestExtension = RefundBridgedParachainMessages< TestRuntime, + RefundableParachain<(), TestParachain>, + RefundableMessagesLane<(), TestLaneId>, + ActualFeeRefund, + StrTestExtension, >; fn relayer_account_at_this_chain() -> ThisChainAccountId { @@ -470,7 +523,7 @@ mod tests { fn all_finality_pre_dispatch_data() -> PreDispatchData { PreDispatchData { relayer: relayer_account_at_this_chain(), - call_type: CallType::AllFinalityAndDelivery( + call_info: CallInfo::AllFinalityAndDelivery( 200, SubmitParachainHeadsInfo { at_relay_block_number: 200, @@ -489,7 +542,7 @@ mod tests { fn parachain_finality_pre_dispatch_data() -> PreDispatchData { PreDispatchData { relayer: relayer_account_at_this_chain(), - call_type: CallType::ParachainFinalityAndDelivery( + call_info: CallInfo::ParachainFinalityAndDelivery( SubmitParachainHeadsInfo { at_relay_block_number: 200, para_id: ParaId(TestParachain::get()), @@ -507,7 +560,7 @@ mod tests { fn delivery_pre_dispatch_data() -> PreDispatchData { PreDispatchData { relayer: relayer_account_at_this_chain(), - call_type: CallType::Delivery(ReceiveMessagesProofInfo { + call_info: CallInfo::Delivery(ReceiveMessagesProofInfo { lane_id: TEST_LANE_ID, best_proof_nonce: 200, best_stored_nonce: 100, @@ -520,14 +573,14 @@ mod tests { } fn run_validate(call: RuntimeCall) -> TransactionValidity { - let extension: TestExtension = RefundRelayerForMessagesFromParachain(PhantomData); + let extension: TestExtension = RefundBridgedParachainMessages(PhantomData); extension.validate(&relayer_account_at_this_chain(), &call, &DispatchInfo::default(), 0) } fn run_pre_dispatch( call: RuntimeCall, ) -> Result>, TransactionValidityError> { - let extension: TestExtension = RefundRelayerForMessagesFromParachain(PhantomData); + let extension: TestExtension = RefundBridgedParachainMessages(PhantomData); extension.pre_dispatch(&relayer_account_at_this_chain(), &call, &DispatchInfo::default(), 0) } diff --git a/modules/grandpa/src/lib.rs b/modules/grandpa/src/lib.rs index 197726dd47a..b11da53f7b3 100644 --- a/modules/grandpa/src/lib.rs +++ b/modules/grandpa/src/lib.rs @@ -1187,6 +1187,11 @@ mod tests { bp_header_chain::storage_keys::pallet_operating_mode_key("Grandpa").0, ); + assert_eq!( + CurrentAuthoritySet::::storage_value_final_key().to_vec(), + bp_header_chain::storage_keys::current_authority_set_key("Grandpa").0, + ); + assert_eq!( BestFinalized::::storage_value_final_key().to_vec(), bp_header_chain::storage_keys::best_finalized_key("Grandpa").0, diff --git a/modules/parachains/src/call_ext.rs b/modules/parachains/src/call_ext.rs index 41649336579..5aed9e80ba0 100644 --- a/modules/parachains/src/call_ext.rs +++ b/modules/parachains/src/call_ext.rs @@ -17,14 +17,11 @@ use crate::{Config, Pallet, RelayBlockNumber}; use bp_parachains::BestParaHeadHash; use bp_polkadot_core::parachains::{ParaHash, ParaId}; -use frame_support::{dispatch::CallableCallFor, traits::IsSubType}; -use sp_runtime::{ - transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction}, - RuntimeDebug, -}; +use frame_support::{dispatch::CallableCallFor, traits::IsSubType, RuntimeDebug}; +use sp_runtime::transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction}; /// Info about a `SubmitParachainHeads` call which tries to update a single parachain. -#[derive(Copy, Clone, PartialEq, RuntimeDebug)] +#[derive(PartialEq, RuntimeDebug)] pub struct SubmitParachainHeadsInfo { pub at_relay_block_number: RelayBlockNumber, pub para_id: ParaId, diff --git a/primitives/header-chain/src/justification.rs b/primitives/header-chain/src/justification.rs index dadd48a4850..43adc801254 100644 --- a/primitives/header-chain/src/justification.rs +++ b/primitives/header-chain/src/justification.rs @@ -71,6 +71,14 @@ pub enum Error { ExtraHeadersInVotesAncestries, } +/// Given GRANDPA authorities set size, return number of valid authorities votes that the +/// justification must have to be valid. +/// +/// This function assumes that all authorities have the same vote weight. +pub fn required_justification_precommits(authorities_set_length: u32) -> u32 { + authorities_set_length - authorities_set_length.saturating_sub(1) / 3 +} + /// Decode justification target. pub fn decode_justification_target( raw_justification: &[u8], @@ -80,6 +88,27 @@ pub fn decode_justification_target( .map_err(|_| Error::JustificationDecode) } +/// Verify and optimize given justification by removing unknown and duplicate votes. +pub fn optimize_justification( + finalized_target: (Header::Hash, Header::Number), + authorities_set_id: SetId, + authorities_set: &VoterSet, + justification: GrandpaJustification
, +) -> Result, Error> +where + Header::Number: finality_grandpa::BlockNumberOps, +{ + let mut optimizer = OptimizationCallbacks(Vec::new()); + verify_justification_with_callbacks( + finalized_target, + authorities_set_id, + authorities_set, + &justification, + &mut optimizer, + )?; + Ok(optimizer.optimize(justification)) +} + /// Verify that justification, that is generated by given authority set, finalizes given header. pub fn verify_justification( finalized_target: (Header::Hash, Header::Number), @@ -87,6 +116,83 @@ pub fn verify_justification( authorities_set: &VoterSet, justification: &GrandpaJustification
, ) -> Result<(), Error> +where + Header::Number: finality_grandpa::BlockNumberOps, +{ + verify_justification_with_callbacks( + finalized_target, + authorities_set_id, + authorities_set, + justification, + &mut (), + ) +} + +/// Verification callbacks. +trait VerificationCallbacks { + /// 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 after we've collected enough votes from authorities. + fn on_redundant_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error>; +} + +/// Verification callbacks for justification optimization. +struct OptimizationCallbacks(Vec); + +impl OptimizationCallbacks { + fn optimize( + self, + mut justification: GrandpaJustification
, + ) -> GrandpaJustification
{ + for invalid_precommit_idx in self.0.into_iter().rev() { + justification.commit.precommits.remove(invalid_precommit_idx); + } + justification + } +} + +impl VerificationCallbacks for OptimizationCallbacks { + fn on_unkown_authority(&mut self, precommit_idx: usize) -> Result<(), Error> { + self.0.push(precommit_idx); + Ok(()) + } + + fn on_duplicate_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error> { + self.0.push(precommit_idx); + Ok(()) + } + + fn on_redundant_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error> { + self.0.push(precommit_idx); + Ok(()) + } +} + +// this implementation will be removed in https://github.com/paritytech/parity-bridges-common/pull/1882 +impl VerificationCallbacks for () { + fn on_unkown_authority(&mut self, _precommit_idx: usize) -> Result<(), Error> { + Ok(()) + } + + fn on_duplicate_authority_vote(&mut self, _precommit_idx: usize) -> Result<(), Error> { + Ok(()) + } + + fn on_redundant_authority_vote(&mut self, _precommit_idx: usize) -> Result<(), Error> { + Ok(()) + } +} + +/// Verify that justification, that is generated by given authority set, finalizes given header. +fn verify_justification_with_callbacks( + finalized_target: (Header::Hash, Header::Number), + authorities_set_id: SetId, + authorities_set: &VoterSet, + justification: &GrandpaJustification
, + callbacks: &mut C, +) -> Result<(), Error> where Header::Number: finality_grandpa::BlockNumberOps, { @@ -95,17 +201,23 @@ where return Err(Error::InvalidJustificationTarget) } + let threshold = authorities_set.threshold().0.into(); let mut chain = AncestryChain::new(&justification.votes_ancestries); let mut signature_buffer = Vec::new(); let mut votes = BTreeSet::new(); let mut cumulative_weight = 0u64; - for signed in &justification.commit.precommits { + for (precommit_idx, signed) in justification.commit.precommits.iter().enumerate() { + // if we have collected enough precommits, we probabably want to fail/remove extra + // precommits + if cumulative_weight > threshold { + callbacks.on_redundant_authority_vote(precommit_idx)?; + } + // authority must be in the set let authority_info = match authorities_set.get(&signed.id) { Some(authority_info) => authority_info, None => { - // just ignore precommit from unknown authority as - // `finality_grandpa::import_precommit` does + callbacks.on_unkown_authority(precommit_idx)?; continue }, }; @@ -116,6 +228,7 @@ where // `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()) { + callbacks.on_duplicate_authority_vote(precommit_idx)?; continue } @@ -142,6 +255,7 @@ where thus we'll never overflow the u64::MAX;\ qed", ); + // verify authority signature if !sp_finality_grandpa::check_message_signature_with_buffer( &finality_grandpa::Message::Precommit(signed.precommit.clone()), @@ -162,7 +276,6 @@ where // check that the cumulative weight of validators voted for the justification target (or one // of its descendents) is larger than required threshold. - let threshold = authorities_set.threshold().0.into(); if cumulative_weight >= threshold { Ok(()) } else { diff --git a/primitives/header-chain/src/storage_keys.rs b/primitives/header-chain/src/storage_keys.rs index bb642b1817f..c4dbe53bd9a 100644 --- a/primitives/header-chain/src/storage_keys.rs +++ b/primitives/header-chain/src/storage_keys.rs @@ -20,6 +20,8 @@ pub const PALLET_OPERATING_MODE_VALUE_NAME: &str = "PalletOperatingMode"; /// Name of the `BestFinalized` storage value. pub const BEST_FINALIZED_VALUE_NAME: &str = "BestFinalized"; +/// Name of the `CurrentAuthoritySet` storage value. +pub const CURRENT_AUTHORITY_SET_VALUE_NAME: &str = "CurrentAuthoritySet"; use sp_core::storage::StorageKey; @@ -34,6 +36,17 @@ pub fn pallet_operating_mode_key(pallet_prefix: &str) -> StorageKey { ) } +/// Storage key of the `CurrentAuthoritySet` variable in the runtime storage. +pub fn current_authority_set_key(pallet_prefix: &str) -> StorageKey { + StorageKey( + bp_runtime::storage_value_final_key( + pallet_prefix.as_bytes(), + CURRENT_AUTHORITY_SET_VALUE_NAME.as_bytes(), + ) + .to_vec(), + ) +} + /// Storage key of the best finalized header number and hash value in the runtime storage. pub fn best_finalized_key(pallet_prefix: &str) -> StorageKey { StorageKey( @@ -63,6 +76,19 @@ mod tests { ); } + #[test] + fn current_authority_set_key_computed_properly() { + // If this test fails, then something has been changed in module storage that is breaking + // compatibility with previous pallet. + let storage_key = current_authority_set_key("BridgeGrandpa").0; + assert_eq!( + storage_key, + hex!("0b06f475eddb98cf933a12262e0388de24a7b8b5717ea33346fa595a66ccbcb0").to_vec(), + "Unexpected storage key: {}", + hex::encode(&storage_key), + ); + } + #[test] fn best_finalized_key_computed_properly() { // If this test fails, then something has been changed in module storage that is breaking diff --git a/primitives/header-chain/tests/justification.rs b/primitives/header-chain/tests/justification.rs index 5b4981a0f69..e18163313c9 100644 --- a/primitives/header-chain/tests/justification.rs +++ b/primitives/header-chain/tests/justification.rs @@ -16,7 +16,7 @@ //! Tests for Grandpa Justification code. -use bp_header_chain::justification::{verify_justification, Error}; +use bp_header_chain::justification::{optimize_justification, verify_justification, Error}; use bp_test_utils::*; type TestHeader = sp_runtime::testing::Header; @@ -190,3 +190,87 @@ fn justification_is_invalid_if_we_dont_meet_threshold() { Err(Error::TooLowCumulativeWeight), ); } + +#[test] +fn optimizer_does_noting_with_minimal_justification() { + let justification = make_default_justification::(&test_header(1)); + + let num_precommits_before = justification.commit.precommits.len(); + let justification = optimize_justification::( + header_id::(1), + TEST_GRANDPA_SET_ID, + &voter_set(), + justification, + ) + .unwrap(); + let num_precommits_after = justification.commit.precommits.len(); + + assert_eq!(num_precommits_before, num_precommits_after); +} + +#[test] +fn unknown_authority_votes_are_removed_by_optimizer() { + let mut justification = make_default_justification::(&test_header(1)); + justification.commit.precommits.push(signed_precommit::( + &bp_test_utils::Account(42), + header_id::(1), + justification.round, + TEST_GRANDPA_SET_ID, + )); + + let num_precommits_before = justification.commit.precommits.len(); + let justification = optimize_justification::( + header_id::(1), + TEST_GRANDPA_SET_ID, + &voter_set(), + justification, + ) + .unwrap(); + let num_precommits_after = justification.commit.precommits.len(); + + assert_eq!(num_precommits_before - 1, num_precommits_after); +} + +#[test] +fn duplicate_authority_votes_are_removed_by_optimizer() { + let mut justification = make_default_justification::(&test_header(1)); + justification + .commit + .precommits + .push(justification.commit.precommits.first().cloned().unwrap()); + + let num_precommits_before = justification.commit.precommits.len(); + let justification = optimize_justification::( + header_id::(1), + TEST_GRANDPA_SET_ID, + &voter_set(), + justification, + ) + .unwrap(); + let num_precommits_after = justification.commit.precommits.len(); + + assert_eq!(num_precommits_before - 1, num_precommits_after); +} + +#[test] +fn redundant_authority_votes_are_removed_by_optimizer() { + let mut justification = make_default_justification::(&test_header(1)); + justification.commit.precommits.push(signed_precommit::( + &EVE, + header_id::(1), + justification.round, + TEST_GRANDPA_SET_ID, + )); + + let num_precommits_before = justification.commit.precommits.len(); + let justification = optimize_justification::( + header_id::(1), + TEST_GRANDPA_SET_ID, + &voter_set(), + justification, + ) + .unwrap(); + let num_precommits_after = justification.commit.precommits.len(); + + assert_eq!(num_precommits_before - 1, num_precommits_after); +} diff --git a/primitives/runtime/Cargo.toml b/primitives/runtime/Cargo.toml index 515d6e1a6c4..c8829a508d9 100644 --- a/primitives/runtime/Cargo.toml +++ b/primitives/runtime/Cargo.toml @@ -24,7 +24,7 @@ sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master sp-state-machine = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } sp-trie = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } -trie-db = { version = "0.25.0", default-features = false } +trie-db = { version = "0.25.1", default-features = false } [dev-dependencies] hex-literal = "0.3" diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 75151ccb723..0121b4ab84e 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -507,6 +507,24 @@ impl WeightExtraOps for Weight { } } +/// Trait that provides a static `str`. +pub trait StaticStrProvider { + const STR: &'static str; +} + +#[macro_export] +macro_rules! generate_static_str_provider { + ($str:expr) => { + $crate::paste::item! { + pub struct []; + + impl $crate::StaticStrProvider for [] { + const STR: &'static str = stringify!($str); + } + } + }; +} + #[cfg(test)] mod tests { use super::*; @@ -531,4 +549,10 @@ mod tests { ), ); } + + #[test] + fn generate_static_str_provider_works() { + generate_static_str_provider!(Test); + assert_eq!(StrTest::STR, "Test"); + } } diff --git a/primitives/test-utils/src/lib.rs b/primitives/test-utils/src/lib.rs index c1e95ec6fef..a6bb3d9b8fe 100644 --- a/primitives/test-utils/src/lib.rs +++ b/primitives/test-utils/src/lib.rs @@ -18,7 +18,7 @@ #![cfg_attr(not(feature = "std"), no_std)] -use bp_header_chain::justification::GrandpaJustification; +use bp_header_chain::justification::{required_justification_precommits, GrandpaJustification}; use codec::Encode; use sp_finality_grandpa::{AuthorityId, AuthoritySignature, AuthorityWeight, SetId}; use sp_runtime::traits::{Header as HeaderT, One, Zero}; @@ -57,11 +57,12 @@ pub struct JustificationGeneratorParams { impl Default for JustificationGeneratorParams { fn default() -> Self { + let required_signatures = required_justification_precommits(test_keyring().len() as _); Self { header: test_header(One::one()), round: TEST_GRANDPA_ROUND, set_id: TEST_GRANDPA_SET_ID, - authorities: test_keyring(), + authorities: test_keyring().into_iter().take(required_signatures as _).collect(), ancestors: 2, forks: 1, } diff --git a/relays/bin-substrate/src/chains/bridge_hub_rococo_messages_to_bridge_hub_wococo.rs b/relays/bin-substrate/src/chains/bridge_hub_rococo_messages_to_bridge_hub_wococo.rs index b48ace58987..9e4b5b15fd6 100644 --- a/relays/bin-substrate/src/chains/bridge_hub_rococo_messages_to_bridge_hub_wococo.rs +++ b/relays/bin-substrate/src/chains/bridge_hub_rococo_messages_to_bridge_hub_wococo.rs @@ -19,7 +19,7 @@ use crate::cli::bridge::{CliBridgeBase, MessagesCliBridge}; use relay_bridge_hub_rococo_client::BridgeHubRococo; use relay_bridge_hub_wococo_client::BridgeHubWococo; -use substrate_relay_helper::messages_lane::SubstrateMessageLane; +use substrate_relay_helper::{messages_lane::SubstrateMessageLane, UtilityPalletBatchCallBuilder}; pub struct BridgeHubRococoToBridgeHubWococoMessagesCliBridge {} @@ -59,6 +59,6 @@ impl SubstrateMessageLane for BridgeHubRococoMessagesToBridgeHubWococoMessageLan type ReceiveMessagesDeliveryProofCallBuilder = BridgeHubRococoMessagesToBridgeHubWococoMessageLaneReceiveMessagesDeliveryProofCallBuilder; - type SourceBatchCallBuilder = (); - type TargetBatchCallBuilder = (); + type SourceBatchCallBuilder = UtilityPalletBatchCallBuilder; + type TargetBatchCallBuilder = UtilityPalletBatchCallBuilder; } diff --git a/relays/bin-substrate/src/chains/bridge_hub_wococo_messages_to_bridge_hub_rococo.rs b/relays/bin-substrate/src/chains/bridge_hub_wococo_messages_to_bridge_hub_rococo.rs index 3bcf62de333..fb5a81c021e 100644 --- a/relays/bin-substrate/src/chains/bridge_hub_wococo_messages_to_bridge_hub_rococo.rs +++ b/relays/bin-substrate/src/chains/bridge_hub_wococo_messages_to_bridge_hub_rococo.rs @@ -19,7 +19,7 @@ use crate::cli::bridge::{CliBridgeBase, MessagesCliBridge}; use relay_bridge_hub_rococo_client::BridgeHubRococo; use relay_bridge_hub_wococo_client::BridgeHubWococo; -use substrate_relay_helper::messages_lane::SubstrateMessageLane; +use substrate_relay_helper::{messages_lane::SubstrateMessageLane, UtilityPalletBatchCallBuilder}; pub struct BridgeHubWococoToBridgeHubRococoMessagesCliBridge {} @@ -59,6 +59,6 @@ impl SubstrateMessageLane for BridgeHubWococoMessagesToBridgeHubRococoMessageLan type ReceiveMessagesDeliveryProofCallBuilder = BridgeHubWococoMessagesToBridgeHubRococoMessageLaneReceiveMessagesDeliveryProofCallBuilder; - type SourceBatchCallBuilder = (); - type TargetBatchCallBuilder = (); + type SourceBatchCallBuilder = UtilityPalletBatchCallBuilder; + type TargetBatchCallBuilder = UtilityPalletBatchCallBuilder; } diff --git a/relays/client-bridge-hub-rococo/src/lib.rs b/relays/client-bridge-hub-rococo/src/lib.rs index 80c075cd0d5..d2e421423a5 100644 --- a/relays/client-bridge-hub-rococo/src/lib.rs +++ b/relays/client-bridge-hub-rococo/src/lib.rs @@ -21,8 +21,9 @@ use bp_bridge_hub_wococo::PolkadotSignedExtension; use bp_messages::MessageNonce; use codec::Encode; use relay_substrate_client::{ - Chain, ChainWithBalances, ChainWithMessages, ChainWithTransactions, Error as SubstrateError, - SignParam, UnderlyingChainProvider, UnsignedTransaction, + Chain, ChainWithBalances, ChainWithMessages, ChainWithTransactions, ChainWithUtilityPallet, + Error as SubstrateError, MockedRuntimeUtilityPallet, SignParam, UnderlyingChainProvider, + UnsignedTransaction, }; use sp_core::{storage::StorageKey, Pair}; use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount}; @@ -57,6 +58,10 @@ impl ChainWithBalances for BridgeHubRococo { } } +impl ChainWithUtilityPallet for BridgeHubRococo { + type UtilityPallet = MockedRuntimeUtilityPallet; +} + impl ChainWithTransactions for BridgeHubRococo { type AccountKeyPair = sp_core::sr25519::Pair; type SignedTransaction = runtime::UncheckedExtrinsic; diff --git a/relays/client-bridge-hub-rococo/src/runtime_wrapper.rs b/relays/client-bridge-hub-rococo/src/runtime_wrapper.rs index 7f526a35aa9..fc945d8c950 100644 --- a/relays/client-bridge-hub-rococo/src/runtime_wrapper.rs +++ b/relays/client-bridge-hub-rococo/src/runtime_wrapper.rs @@ -25,7 +25,7 @@ use bp_bridge_hub_rococo::SignedExtension; pub use bp_header_chain::BridgeGrandpaCallOf; pub use bp_parachains::BridgeParachainCall; pub use bridge_runtime_common::messages::BridgeMessagesCallOf; -pub use relay_substrate_client::calls::SystemCall; +pub use relay_substrate_client::calls::{SystemCall, UtilityCall}; /// Unchecked BridgeHubRococo extrinsic. pub type UncheckedExtrinsic = bp_bridge_hub_rococo::UncheckedExtrinsic; @@ -49,6 +49,9 @@ pub enum Call { #[cfg(test)] #[codec(index = 0)] System(SystemCall), + /// Utility pallet. + #[codec(index = 40)] + Utility(UtilityCall), /// Wococo bridge pallet. #[codec(index = 41)] @@ -60,3 +63,9 @@ pub enum Call { #[codec(index = 46)] BridgeWococoMessages(BridgeWococoMessagesCall), } + +impl From> for Call { + fn from(call: UtilityCall) -> Call { + Call::Utility(call) + } +} diff --git a/relays/client-bridge-hub-wococo/src/lib.rs b/relays/client-bridge-hub-wococo/src/lib.rs index 7af310bfd41..2c211ae86cf 100644 --- a/relays/client-bridge-hub-wococo/src/lib.rs +++ b/relays/client-bridge-hub-wococo/src/lib.rs @@ -20,8 +20,9 @@ use bp_bridge_hub_wococo::{PolkadotSignedExtension, AVERAGE_BLOCK_INTERVAL}; use bp_messages::MessageNonce; use codec::Encode; use relay_substrate_client::{ - Chain, ChainWithBalances, ChainWithMessages, ChainWithTransactions, Error as SubstrateError, - SignParam, UnderlyingChainProvider, UnsignedTransaction, + Chain, ChainWithBalances, ChainWithMessages, ChainWithTransactions, ChainWithUtilityPallet, + Error as SubstrateError, MockedRuntimeUtilityPallet, SignParam, UnderlyingChainProvider, + UnsignedTransaction, }; use sp_core::{storage::StorageKey, Pair}; use sp_runtime::{generic::SignedPayload, traits::IdentifyAccount}; @@ -56,6 +57,10 @@ impl ChainWithBalances for BridgeHubWococo { } } +impl ChainWithUtilityPallet for BridgeHubWococo { + type UtilityPallet = MockedRuntimeUtilityPallet; +} + impl ChainWithTransactions for BridgeHubWococo { type AccountKeyPair = sp_core::sr25519::Pair; type SignedTransaction = runtime::UncheckedExtrinsic; diff --git a/relays/client-bridge-hub-wococo/src/runtime_wrapper.rs b/relays/client-bridge-hub-wococo/src/runtime_wrapper.rs index 17cc4cbd4e8..c16e7d1a45b 100644 --- a/relays/client-bridge-hub-wococo/src/runtime_wrapper.rs +++ b/relays/client-bridge-hub-wococo/src/runtime_wrapper.rs @@ -23,7 +23,7 @@ use bp_bridge_hub_wococo::SignedExtension; pub use bp_header_chain::BridgeGrandpaCallOf; pub use bp_parachains::BridgeParachainCall; pub use bridge_runtime_common::messages::BridgeMessagesCallOf; -pub use relay_substrate_client::calls::SystemCall; +pub use relay_substrate_client::calls::{SystemCall, UtilityCall}; /// Unchecked BridgeHubWococo extrinsic. pub type UncheckedExtrinsic = bp_bridge_hub_wococo::UncheckedExtrinsic; @@ -47,6 +47,9 @@ pub enum Call { #[cfg(test)] #[codec(index = 0)] System(SystemCall), + /// Utility pallet. + #[codec(index = 40)] + Utility(UtilityCall), /// Rococo bridge pallet. #[codec(index = 43)] @@ -59,6 +62,12 @@ pub enum Call { BridgeRococoMessages(BridgeRococoMessagesCall), } +impl From> for Call { + fn from(call: UtilityCall) -> Call { + Call::Utility(call) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/relays/client-millau/src/lib.rs b/relays/client-millau/src/lib.rs index 711b4f32cc6..4c4c1370a6a 100644 --- a/relays/client-millau/src/lib.rs +++ b/relays/client-millau/src/lib.rs @@ -95,7 +95,7 @@ impl ChainWithTransactions for Millau { frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(unsigned.tip), millau_runtime::BridgeRejectObsoleteHeadersAndMessages, - millau_runtime::BridgeRefundRialtoParachainRelayers::default(), + millau_runtime::BridgeRefundRialtoParachainMessages::default(), ), ( (), diff --git a/relays/client-substrate/src/calls.rs b/relays/client-substrate/src/calls.rs index 89fc49a209a..4e0ae9d99d2 100644 --- a/relays/client-substrate/src/calls.rs +++ b/relays/client-substrate/src/calls.rs @@ -31,6 +31,15 @@ pub enum SystemCall { remark(Vec), } +/// A minimized version of `pallet-utility::Call` that can be used without a runtime. +#[derive(Encode, Decode, Debug, PartialEq, Eq, Clone, TypeInfo)] +#[allow(non_camel_case_types)] +pub enum UtilityCall { + /// `pallet-utility::Call::batch_all` + #[codec(index = 2)] + batch_all(Vec), +} + /// A minimized version of `pallet-sudo::Call` that can be used without a runtime. #[derive(Encode, Decode, Debug, PartialEq, Eq, Clone, TypeInfo)] #[allow(non_camel_case_types)] diff --git a/relays/client-substrate/src/chain.rs b/relays/client-substrate/src/chain.rs index b9c5793842e..ebd9e7172d8 100644 --- a/relays/client-substrate/src/chain.rs +++ b/relays/client-substrate/src/chain.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . +use crate::calls::UtilityCall; + use bp_messages::MessageNonce; use bp_runtime::{ Chain as ChainBase, EncodedOrDecodedCall, HashOf, Parachain as ParachainBase, TransactionEra, @@ -276,6 +278,21 @@ where } } +/// Structure that implements `UtilityPalletProvider` based on a call conversion. +pub struct MockedRuntimeUtilityPallet { + _phantom: std::marker::PhantomData, +} + +impl UtilityPallet for MockedRuntimeUtilityPallet +where + C: Chain, + C::Call: From>, +{ + fn build_batch_call(calls: Vec) -> C::Call { + UtilityCall::batch_all(calls).into() + } +} + /// Substrate-based chain that uses `pallet-utility`. pub trait ChainWithUtilityPallet: Chain { /// The utility pallet provider. diff --git a/relays/client-substrate/src/lib.rs b/relays/client-substrate/src/lib.rs index c1a96c487c4..c8d8b6f8129 100644 --- a/relays/client-substrate/src/lib.rs +++ b/relays/client-substrate/src/lib.rs @@ -36,8 +36,8 @@ pub use crate::{ chain::{ AccountKeyPairOf, BlockWithJustification, CallOf, Chain, ChainWithBalances, ChainWithGrandpa, ChainWithMessages, ChainWithTransactions, ChainWithUtilityPallet, - FullRuntimeUtilityPallet, Parachain, RelayChain, SignParam, TransactionStatusOf, - UnsignedTransaction, UtilityPallet, + FullRuntimeUtilityPallet, MockedRuntimeUtilityPallet, Parachain, RelayChain, SignParam, + TransactionStatusOf, UnsignedTransaction, UtilityPallet, }, client::{ is_ancient_block, ChainRuntimeVersion, Client, OpaqueGrandpaAuthoritiesSet, diff --git a/relays/lib-substrate-relay/src/finality/engine.rs b/relays/lib-substrate-relay/src/finality/engine.rs index 4c2da5a5319..f5ac8539a68 100644 --- a/relays/lib-substrate-relay/src/finality/engine.rs +++ b/relays/lib-substrate-relay/src/finality/engine.rs @@ -22,7 +22,7 @@ use bp_header_chain::{ justification::{verify_justification, GrandpaJustification}, ConsensusLogReader, FinalityProof, GrandpaConsensusLogReader, }; -use bp_runtime::{BasicOperatingMode, OperatingMode}; +use bp_runtime::{BasicOperatingMode, HeaderIdProvider, OperatingMode}; use codec::{Decode, Encode}; use finality_grandpa::voter_set::VoterSet; use num_traits::{One, Zero}; @@ -87,6 +87,13 @@ pub trait Engine: Send { client.subscribe_finality_justifications::().await } + /// Optimize finality proof before sending it to the target node. + async fn optimize_proof( + target_client: &Client, + header: &C::Header, + proof: Self::FinalityProof, + ) -> Result; + /// Prepare initialization data for the finality bridge pallet. async fn prepare_initialization_data( client: Client, @@ -139,6 +146,48 @@ impl Engine for Grandpa { bp_header_chain::storage_keys::pallet_operating_mode_key(C::WITH_CHAIN_GRANDPA_PALLET_NAME) } + async fn optimize_proof( + target_client: &Client, + header: &C::Header, + proof: Self::FinalityProof, + ) -> Result { + let current_authority_set_key = bp_header_chain::storage_keys::current_authority_set_key( + C::WITH_CHAIN_GRANDPA_PALLET_NAME, + ); + let (authority_set, authority_set_id): ( + sp_finality_grandpa::AuthorityList, + sp_finality_grandpa::SetId, + ) = target_client + .storage_value(current_authority_set_key, None) + .await? + .map(Ok) + .unwrap_or(Err(SubstrateError::Custom(format!( + "{} `CurrentAuthoritySet` is missing from the {} storage", + C::NAME, + TargetChain::NAME, + ))))?; + let authority_set = + finality_grandpa::voter_set::VoterSet::new(authority_set).expect("TODO"); + // we're risking with race here - we have decided to submit justification some time ago and + // actual authorities set (which we have read now) may have changed, so this + // `optimize_justification` may fail. But if target chain is configured properly, it'll fail + // anyway, after we submit transaction and failing earlier is better. So - it is fine + bp_header_chain::justification::optimize_justification( + (header.hash(), *header.number()), + authority_set_id, + &authority_set, + proof, + ) + .map_err(|e| { + SubstrateError::Custom(format!( + "Failed to optimize {} GRANDPA jutification for header {:?}: {:?}", + C::NAME, + header.id(), + e, + )) + }) + } + /// Prepare initialization data for the GRANDPA verifier pallet. async fn prepare_initialization_data( source_client: Client, diff --git a/relays/lib-substrate-relay/src/finality/target.rs b/relays/lib-substrate-relay/src/finality/target.rs index 9c6ec7c3055..81a22520fa9 100644 --- a/relays/lib-substrate-relay/src/finality/target.rs +++ b/relays/lib-substrate-relay/src/finality/target.rs @@ -111,6 +111,10 @@ where header: SyncHeader>, proof: SubstrateFinalityProof

, ) -> Result { + // runtime module at target chain may require optimized finality proof + let proof = P::FinalityEngine::optimize_proof(&self.client, &header, proof).await?; + + // now we may submit optimized finality proof let transaction_params = self.transaction_params.clone(); let call = P::SubmitFinalityProofCallBuilder::build_submit_finality_proof_call(header, proof); diff --git a/relays/utils/Cargo.toml b/relays/utils/Cargo.toml index 57eeb7fbc8c..1d7422d5a8c 100644 --- a/relays/utils/Cargo.toml +++ b/relays/utils/Cargo.toml @@ -18,7 +18,7 @@ jsonpath_lib = "0.3" log = "0.4.17" num-traits = "0.2" serde_json = "1.0" -sysinfo = "0.27" +sysinfo = "0.28" time = { version = "0.3", features = ["formatting", "local-offset", "std"] } tokio = { version = "1.25", features = ["rt"] } thiserror = "1.0.26"