From 390e836db42ceb80ace8f776ac52dbc5a301ef9b Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 6 Dec 2023 15:16:02 +0300 Subject: [PATCH] Select header that will be fully refunded in on-demand batch finality relay (#2729) * select header that will be fully refunded for submission in on-demand **batch** finality relay * added the only possible test * spelling * nl * updated comment --- Cargo.lock | 1 + bin/runtime-common/src/mock.rs | 5 +- modules/grandpa/src/call_ext.rs | 29 ++--- modules/grandpa/src/mock.rs | 5 +- modules/parachains/src/mock.rs | 10 +- primitives/chain-kusama/src/lib.rs | 5 +- primitives/chain-polkadot-bulletin/src/lib.rs | 10 +- primitives/chain-polkadot/src/lib.rs | 5 +- primitives/chain-rococo/src/lib.rs | 5 +- primitives/chain-westend/src/lib.rs | 5 +- .../header-chain/src/justification/mod.rs | 4 +- primitives/header-chain/src/lib.rs | 106 +++++++++++++----- primitives/polkadot-core/src/lib.rs | 25 ++--- relays/bin-substrate/Cargo.toml | 1 + .../src/finality_base/engine.rs | 46 +++++++- .../src/on_demand/headers.rs | 76 +++++++++---- 16 files changed, 223 insertions(+), 115 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dc4725aa2aaa7..1c50614043b16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9491,6 +9491,7 @@ dependencies = [ "bp-runtime", "bp-test-utils", "bridge-runtime-common", + "env_logger", "finality-grandpa", "frame-support", "futures", diff --git a/bin/runtime-common/src/mock.rs b/bin/runtime-common/src/mock.rs index 2d34dc272c728..bd47d37fc07d0 100644 --- a/bin/runtime-common/src/mock.rs +++ b/bin/runtime-common/src/mock.rs @@ -376,9 +376,8 @@ impl ChainWithGrandpa for BridgedUnderlyingChain { const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str = ""; const MAX_AUTHORITIES_COUNT: u32 = 16; const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = 8; - const MAX_HEADER_SIZE: u32 = 256; - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = 64; - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = 64; + const MAX_MANDATORY_HEADER_SIZE: u32 = 256; + const AVERAGE_HEADER_SIZE: u32 = 64; } impl Chain for BridgedUnderlyingParachain { diff --git a/modules/grandpa/src/call_ext.rs b/modules/grandpa/src/call_ext.rs index f238064f92bca..c1585020be13c 100644 --- a/modules/grandpa/src/call_ext.rs +++ b/modules/grandpa/src/call_ext.rs @@ -15,7 +15,10 @@ // along with Parity Bridges Common. If not, see . use crate::{weights::WeightInfo, BridgedBlockNumber, BridgedHeader, Config, Error, Pallet}; -use bp_header_chain::{justification::GrandpaJustification, ChainWithGrandpa}; +use bp_header_chain::{ + justification::GrandpaJustification, max_expected_submit_finality_proof_arguments_size, + ChainWithGrandpa, GrandpaConsensusLogReader, +}; use bp_runtime::{BlockNumberOf, OwnedBridgeModule}; use codec::Encode; use frame_support::{dispatch::CallableCallFor, traits::IsSubType, weights::Weight}; @@ -169,28 +172,28 @@ pub(crate) fn submit_finality_proof_info_from_args, I: 'static>( Weight::zero() }; + // check if the `finality_target` is a mandatory header. If so, we are ready to refund larger + // size + let is_mandatory_finality_target = + GrandpaConsensusLogReader::>::find_scheduled_change( + finality_target.digest(), + ) + .is_some(); + // we can estimate extra call size easily, without any additional significant overhead let actual_call_size: u32 = finality_target .encoded_size() .saturating_add(justification.encoded_size()) .saturated_into(); - let max_expected_call_size = max_expected_call_size::(required_precommits); + let max_expected_call_size = max_expected_submit_finality_proof_arguments_size::( + is_mandatory_finality_target, + required_precommits, + ); let extra_size = actual_call_size.saturating_sub(max_expected_call_size); SubmitFinalityProofInfo { block_number, extra_weight, extra_size } } -/// Returns maximal expected size of `submit_finality_proof` call arguments. -fn max_expected_call_size, I: 'static>(required_precommits: u32) -> u32 { - let max_expected_justification_size = - GrandpaJustification::>::max_reasonable_size::( - required_precommits, - ); - - // call arguments are header and justification - T::BridgedChain::MAX_HEADER_SIZE.saturating_add(max_expected_justification_size) -} - #[cfg(test)] mod tests { use crate::{ diff --git a/modules/grandpa/src/mock.rs b/modules/grandpa/src/mock.rs index ad61e7498bd14..a54f56c4a6249 100644 --- a/modules/grandpa/src/mock.rs +++ b/modules/grandpa/src/mock.rs @@ -86,9 +86,8 @@ impl ChainWithGrandpa for TestBridgedChain { const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str = ""; const MAX_AUTHORITIES_COUNT: u32 = MAX_BRIDGED_AUTHORITIES; const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = 8; - const MAX_HEADER_SIZE: u32 = 256; - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = 64; - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = 64; + const MAX_MANDATORY_HEADER_SIZE: u32 = 256; + const AVERAGE_HEADER_SIZE: u32 = 64; } /// Return test externalities to use in tests. diff --git a/modules/parachains/src/mock.rs b/modules/parachains/src/mock.rs index 8e7a0d7e40028..1c7851364d1c0 100644 --- a/modules/parachains/src/mock.rs +++ b/modules/parachains/src/mock.rs @@ -252,9 +252,8 @@ impl ChainWithGrandpa for TestBridgedChain { const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str = ""; const MAX_AUTHORITIES_COUNT: u32 = 16; const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = 8; - const MAX_HEADER_SIZE: u32 = 256; - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = 64; - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = 64; + const MAX_MANDATORY_HEADER_SIZE: u32 = 256; + const AVERAGE_HEADER_SIZE: u32 = 64; } #[derive(Debug)] @@ -284,9 +283,8 @@ impl ChainWithGrandpa for OtherBridgedChain { const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str = ""; const MAX_AUTHORITIES_COUNT: u32 = 16; const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = 8; - const MAX_HEADER_SIZE: u32 = 256; - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = 64; - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = 64; + const MAX_MANDATORY_HEADER_SIZE: u32 = 256; + const AVERAGE_HEADER_SIZE: u32 = 64; } /// Return test externalities to use in tests. diff --git a/primitives/chain-kusama/src/lib.rs b/primitives/chain-kusama/src/lib.rs index 7f11daadf1af7..5f089fbc589f6 100644 --- a/primitives/chain-kusama/src/lib.rs +++ b/primitives/chain-kusama/src/lib.rs @@ -52,9 +52,8 @@ impl ChainWithGrandpa for Kusama { const MAX_AUTHORITIES_COUNT: u32 = MAX_AUTHORITIES_COUNT; const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY; - const MAX_HEADER_SIZE: u32 = MAX_HEADER_SIZE; - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = AVERAGE_HEADER_SIZE_IN_JUSTIFICATION; - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = WORST_HEADER_SIZE_IN_JUSTIFICATION; + const MAX_MANDATORY_HEADER_SIZE: u32 = MAX_MANDATORY_HEADER_SIZE; + const AVERAGE_HEADER_SIZE: u32 = AVERAGE_HEADER_SIZE; } // The SignedExtension used by Kusama. diff --git a/primitives/chain-polkadot-bulletin/src/lib.rs b/primitives/chain-polkadot-bulletin/src/lib.rs index 49461ab8fbfa3..fe82c9644b673 100644 --- a/primitives/chain-polkadot-bulletin/src/lib.rs +++ b/primitives/chain-polkadot-bulletin/src/lib.rs @@ -42,9 +42,8 @@ use sp_runtime::{traits::DispatchInfoOf, transaction_validity::TransactionValidi // This chain reuses most of Polkadot primitives. pub use bp_polkadot_core::{ AccountAddress, AccountId, Balance, Block, BlockNumber, Hash, Hasher, Header, Nonce, Signature, - SignedBlock, UncheckedExtrinsic, AVERAGE_HEADER_SIZE_IN_JUSTIFICATION, - EXTRA_STORAGE_PROOF_SIZE, MAX_HEADER_SIZE, REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY, - WORST_HEADER_SIZE_IN_JUSTIFICATION, + SignedBlock, UncheckedExtrinsic, AVERAGE_HEADER_SIZE, EXTRA_STORAGE_PROOF_SIZE, + MAX_MANDATORY_HEADER_SIZE, REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY, }; /// Maximal number of GRANDPA authorities at Polkadot Bulletin chain. @@ -208,9 +207,8 @@ impl ChainWithGrandpa for PolkadotBulletin { const MAX_AUTHORITIES_COUNT: u32 = MAX_AUTHORITIES_COUNT; const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY; - const MAX_HEADER_SIZE: u32 = MAX_HEADER_SIZE; - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = AVERAGE_HEADER_SIZE_IN_JUSTIFICATION; - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = WORST_HEADER_SIZE_IN_JUSTIFICATION; + const MAX_MANDATORY_HEADER_SIZE: u32 = MAX_MANDATORY_HEADER_SIZE; + const AVERAGE_HEADER_SIZE: u32 = AVERAGE_HEADER_SIZE; } decl_bridge_finality_runtime_apis!(polkadot_bulletin, grandpa); diff --git a/primitives/chain-polkadot/src/lib.rs b/primitives/chain-polkadot/src/lib.rs index 3f9e2bbc078d2..9a5b8970accb2 100644 --- a/primitives/chain-polkadot/src/lib.rs +++ b/primitives/chain-polkadot/src/lib.rs @@ -52,9 +52,8 @@ impl ChainWithGrandpa for Polkadot { const MAX_AUTHORITIES_COUNT: u32 = MAX_AUTHORITIES_COUNT; const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY; - const MAX_HEADER_SIZE: u32 = MAX_HEADER_SIZE; - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = AVERAGE_HEADER_SIZE_IN_JUSTIFICATION; - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = WORST_HEADER_SIZE_IN_JUSTIFICATION; + const MAX_MANDATORY_HEADER_SIZE: u32 = MAX_MANDATORY_HEADER_SIZE; + const AVERAGE_HEADER_SIZE: u32 = AVERAGE_HEADER_SIZE; } /// The SignedExtension used by Polkadot. diff --git a/primitives/chain-rococo/src/lib.rs b/primitives/chain-rococo/src/lib.rs index 4c2d5f41f5f93..7f3e762715f32 100644 --- a/primitives/chain-rococo/src/lib.rs +++ b/primitives/chain-rococo/src/lib.rs @@ -52,9 +52,8 @@ impl ChainWithGrandpa for Rococo { const MAX_AUTHORITIES_COUNT: u32 = MAX_AUTHORITIES_COUNT; const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY; - const MAX_HEADER_SIZE: u32 = MAX_HEADER_SIZE; - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = AVERAGE_HEADER_SIZE_IN_JUSTIFICATION; - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = WORST_HEADER_SIZE_IN_JUSTIFICATION; + const MAX_MANDATORY_HEADER_SIZE: u32 = MAX_MANDATORY_HEADER_SIZE; + const AVERAGE_HEADER_SIZE: u32 = AVERAGE_HEADER_SIZE; } parameter_types! { diff --git a/primitives/chain-westend/src/lib.rs b/primitives/chain-westend/src/lib.rs index fdf1abe6e07b9..7fa5e140d5707 100644 --- a/primitives/chain-westend/src/lib.rs +++ b/primitives/chain-westend/src/lib.rs @@ -52,9 +52,8 @@ impl ChainWithGrandpa for Westend { const MAX_AUTHORITIES_COUNT: u32 = MAX_AUTHORITIES_COUNT; const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY; - const MAX_HEADER_SIZE: u32 = MAX_HEADER_SIZE; - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = AVERAGE_HEADER_SIZE_IN_JUSTIFICATION; - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = WORST_HEADER_SIZE_IN_JUSTIFICATION; + const MAX_MANDATORY_HEADER_SIZE: u32 = MAX_MANDATORY_HEADER_SIZE; + const AVERAGE_HEADER_SIZE: u32 = AVERAGE_HEADER_SIZE; } parameter_types! { diff --git a/primitives/header-chain/src/justification/mod.rs b/primitives/header-chain/src/justification/mod.rs index 72a5f68918d97..b32d8bdb5f1d8 100644 --- a/primitives/header-chain/src/justification/mod.rs +++ b/primitives/header-chain/src/justification/mod.rs @@ -82,8 +82,8 @@ impl GrandpaJustification { .saturating_add(BlockNumberOf::::max_encoded_len().saturated_into()) .saturating_add(HashOf::::max_encoded_len().saturated_into()); - let max_expected_votes_ancestries_size = C::REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY - .saturating_mul(C::AVERAGE_HEADER_SIZE_IN_JUSTIFICATION); + let max_expected_votes_ancestries_size = + C::REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY.saturating_mul(C::AVERAGE_HEADER_SIZE); // justification is round number (u64=8b), a signed GRANDPA commit and the // `votes_ancestries` vector diff --git a/primitives/header-chain/src/lib.rs b/primitives/header-chain/src/lib.rs index 4e004b28b1946..1459b1c1994bc 100644 --- a/primitives/header-chain/src/lib.rs +++ b/primitives/header-chain/src/lib.rs @@ -266,36 +266,28 @@ pub trait ChainWithGrandpa: Chain { /// to submitter. const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32; - /// Maximal size of the chain header. The header may be the header that enacts new GRANDPA - /// authorities set (so it has large digest inside). + /// Maximal size of the mandatory chain header. Mandatory header is the header that enacts new + /// GRANDPA authorities set (so it has large digest inside). /// /// This isn't a strict limit. The relay may submit larger headers and the pallet will accept /// the call. The limit is only used to compute maximal refund amount and doing calls which /// exceed the limit, may be costly to submitter. - const MAX_HEADER_SIZE: u32; + const MAX_MANDATORY_HEADER_SIZE: u32; - /// Average size of the chain header from justification ancestry. We don't expect to see there - /// headers that change GRANDPA authorities set (GRANDPA will probably be able to finalize at - /// least one additional header per session on non test chains), so this is average size of - /// headers that aren't changing the set. + /// Average size of the chain header. We don't expect to see there headers that change GRANDPA + /// authorities set (GRANDPA will probably be able to finalize at least one additional header + /// per session on non test chains), so this is average size of headers that aren't changing the + /// set. /// - /// This isn't a strict limit. The relay may submit justifications with larger headers in its - /// ancestry and the pallet will accept the call. The limit is only used to compute fee, paid - /// by the user at the sending chain. It covers most of cases, but if the actual header, - /// submitted with the messages transaction will be larger than the - /// `AVERAGE_HEADER_SIZE_IN_JUSTIFICATION`, the difference (`WORST_HEADER_SIZE_IN_JUSTIFICATION` - /// - `AVERAGE_HEADER_SIZE_IN_JUSTIFICATION`) must be covered by the sending chain. - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32; - - /// Worst-case size of the chain header from justification ancestry. We don't expect to see - /// there headers that change GRANDPA authorities set (GRANDPA will probably be able to finalize - /// at least one additional header per session on non test chains), so this is the worst-case - /// size of headers that aren't changing the set. + /// This isn't a strict limit. The relay may submit justifications with larger headers and the + /// pallet will accept the call. However, if the total size of all `submit_finality_proof` + /// arguments exceeds the maximal size, computed using this average size, relayer will only get + /// partial refund. /// - /// This isn't a strict limit. The relay may submit justifications with larger headers in its - /// ancestry and the pallet will accept the call. The limit is only used to compute maximal - /// refund amount and doing calls which exceed the limit, may be costly to submitter. - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32; + /// We expect some headers on production chains that are above this size. But they are rare and + /// if rellayer cares about its profitability, we expect it'll select other headers for + /// submission. + const AVERAGE_HEADER_SIZE: u32; } impl ChainWithGrandpa for T @@ -308,9 +300,67 @@ where const MAX_AUTHORITIES_COUNT: u32 = ::MAX_AUTHORITIES_COUNT; const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = ::REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY; - const MAX_HEADER_SIZE: u32 = ::MAX_HEADER_SIZE; - const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = - ::AVERAGE_HEADER_SIZE_IN_JUSTIFICATION; - const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = - ::WORST_HEADER_SIZE_IN_JUSTIFICATION; + const MAX_MANDATORY_HEADER_SIZE: u32 = + ::MAX_MANDATORY_HEADER_SIZE; + const AVERAGE_HEADER_SIZE: u32 = ::AVERAGE_HEADER_SIZE; +} + +/// Returns maximal expected size of `submit_finality_proof` call arguments. +pub fn max_expected_submit_finality_proof_arguments_size( + is_mandatory_finality_target: bool, + precommits: u32, +) -> u32 { + let max_expected_justification_size = + GrandpaJustification::>::max_reasonable_size::(precommits); + + // call arguments are header and justification + let max_expected_finality_target_size = if is_mandatory_finality_target { + C::MAX_MANDATORY_HEADER_SIZE + } else { + C::AVERAGE_HEADER_SIZE + }; + max_expected_finality_target_size.saturating_add(max_expected_justification_size) +} + +#[cfg(test)] +mod tests { + use super::*; + use frame_support::weights::Weight; + use sp_runtime::{testing::H256, traits::BlakeTwo256, MultiSignature}; + + struct TestChain; + + impl Chain for TestChain { + type BlockNumber = u32; + type Hash = H256; + type Hasher = BlakeTwo256; + type Header = sp_runtime::generic::Header; + type AccountId = u64; + type Balance = u64; + type Nonce = u64; + type Signature = MultiSignature; + + fn max_extrinsic_size() -> u32 { + 0 + } + fn max_extrinsic_weight() -> Weight { + Weight::zero() + } + } + + impl ChainWithGrandpa for TestChain { + const WITH_CHAIN_GRANDPA_PALLET_NAME: &'static str = "Test"; + const MAX_AUTHORITIES_COUNT: u32 = 128; + const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = 2; + const MAX_MANDATORY_HEADER_SIZE: u32 = 100_000; + const AVERAGE_HEADER_SIZE: u32 = 1_024; + } + + #[test] + fn max_expected_submit_finality_proof_arguments_size_respects_mandatory_argument() { + assert!( + max_expected_submit_finality_proof_arguments_size::(true, 100) > + max_expected_submit_finality_proof_arguments_size::(false, 100), + ); + } } diff --git a/primitives/polkadot-core/src/lib.rs b/primitives/polkadot-core/src/lib.rs index 9ebff80ce4d54..586cbf8cb9b47 100644 --- a/primitives/polkadot-core/src/lib.rs +++ b/primitives/polkadot-core/src/lib.rs @@ -64,7 +64,7 @@ pub const MAX_AUTHORITIES_COUNT: u32 = 1_256; /// /// See [`bp-header-chain::ChainWithGrandpa`] for more details. /// -/// This value comes from recent (February, 2023) Kusama and Polkadot headers. There are no +/// This value comes from recent (December, 2023) Kusama and Polkadot headers. There are no /// justifications with any additional headers in votes ancestry, so reasonable headers may /// be set to zero. But we assume that there may be small GRANDPA lags, so we're leaving some /// reserve here. @@ -75,28 +75,17 @@ pub const REASONABLE_HEADERS_IN_JUSTIFICATON_ANCESTRY: u32 = 2; /// /// See [`bp-header-chain::ChainWithGrandpa`] for more details. /// -/// This value comes from recent (February, 2023) Kusama headers. Average is `336` there, but let's -/// have some reserve and make it 1024. -pub const AVERAGE_HEADER_SIZE_IN_JUSTIFICATION: u32 = 1024; - -/// Worst-case header size in `votes_ancestries` field of justification on Polkadot-like -/// chains. -/// -/// See [`bp-header-chain::ChainWithGrandpa`] for more details. -/// -/// This value comes from recent (February, 2023) Kusama headers. Average is `336` there, but some -/// non-mandatory headers has size `40kb` (they contain the BABE epoch descriptor with all -/// authorities - just like our mandatory header). Since we assume `2` headers in justification -/// votes ancestry, let's set average header to `40kb / 2`. -pub const WORST_HEADER_SIZE_IN_JUSTIFICATION: u32 = 20 * 1024; +/// This value comes from recent (December, 2023) Kusama headers. Most of headers are `327` bytes +/// there, but let's have some reserve and make it 1024. +pub const AVERAGE_HEADER_SIZE: u32 = 1024; /// Approximate maximal header size on Polkadot-like chains. /// /// See [`bp-header-chain::ChainWithGrandpa`] for more details. /// -/// This value comes from recent (February, 2023) Kusama headers. Maximal header is a mandatory -/// header. In its SCALE-encoded form it is `80348` bytes. Let's have some reserve here. -pub const MAX_HEADER_SIZE: u32 = 90_000; +/// This value comes from recent (December, 2023) Kusama headers. Maximal header is a mandatory +/// header. In its SCALE-encoded form it is `113407` bytes. Let's have some reserve here. +pub const MAX_MANDATORY_HEADER_SIZE: u32 = 120 * 1024; /// Number of extra bytes (excluding size of storage value itself) of storage proof, built at /// Polkadot-like chain. This mostly depends on number of entries in the storage trie. diff --git a/relays/bin-substrate/Cargo.toml b/relays/bin-substrate/Cargo.toml index df2fc2ad169e1..a850f15ed4673 100644 --- a/relays/bin-substrate/Cargo.toml +++ b/relays/bin-substrate/Cargo.toml @@ -10,6 +10,7 @@ anyhow = "1.0" async-std = "1.9.0" async-trait = "0.1" codec = { package = "parity-scale-codec", version = "3.1.5" } +env_logger = "0.10" futures = "0.3.29" hex = "0.4" log = "0.4.20" diff --git a/relays/lib-substrate-relay/src/finality_base/engine.rs b/relays/lib-substrate-relay/src/finality_base/engine.rs index fb4515beeaaca..831c1e7ad5b50 100644 --- a/relays/lib-substrate-relay/src/finality_base/engine.rs +++ b/relays/lib-substrate-relay/src/finality_base/engine.rs @@ -23,8 +23,9 @@ use bp_header_chain::{ verify_and_optimize_justification, GrandpaEquivocationsFinder, GrandpaJustification, JustificationVerificationContext, }, - AuthoritySet, ConsensusLogReader, FinalityProof, FindEquivocations, GrandpaConsensusLogReader, - HeaderFinalityInfo, HeaderGrandpaInfo, StoredHeaderGrandpaInfo, + max_expected_submit_finality_proof_arguments_size, AuthoritySet, ConsensusLogReader, + FinalityProof, FindEquivocations, GrandpaConsensusLogReader, HeaderFinalityInfo, + HeaderGrandpaInfo, StoredHeaderGrandpaInfo, }; use bp_runtime::{BasicOperatingMode, HeaderIdProvider, OperatingMode}; use codec::{Decode, Encode}; @@ -35,9 +36,22 @@ use relay_substrate_client::{ }; use sp_consensus_grandpa::{AuthorityList as GrandpaAuthoritiesSet, GRANDPA_ENGINE_ID}; use sp_core::{storage::StorageKey, Bytes}; -use sp_runtime::{scale_info::TypeInfo, traits::Header, ConsensusEngineId}; +use sp_runtime::{scale_info::TypeInfo, traits::Header, ConsensusEngineId, SaturatedConversion}; use std::{fmt::Debug, marker::PhantomData}; +/// Result of checking maximal expected call size. +pub enum MaxExpectedCallSizeCheck { + /// Size is ok and call will be refunded. + Ok, + /// The call size exceeds the maximal expected and relayer will only get partial refund. + Exceeds { + /// Actual call size. + call_size: u32, + /// Maximal expected call size. + max_call_size: u32, + }, +} + /// Finality engine, used by the Substrate chain. #[async_trait] pub trait Engine: Send { @@ -111,6 +125,14 @@ pub trait Engine: Send { proof: &mut Self::FinalityProof, ) -> Result<(), SubstrateError>; + /// Checks whether the given `header` and its finality `proof` fit the maximal expected + /// call size limit. If result is `MaxExpectedCallSizeCheck::Exceeds { .. }`, this + /// submission won't be fully refunded and relayer will spend its own funds on that. + fn check_max_expected_call_size( + header: &C::Header, + proof: &Self::FinalityProof, + ) -> MaxExpectedCallSizeCheck; + /// Prepare initialization data for the finality bridge pallet. async fn prepare_initialization_data( client: Client, @@ -219,6 +241,24 @@ impl Engine for Grandpa { }) } + fn check_max_expected_call_size( + header: &C::Header, + proof: &Self::FinalityProof, + ) -> MaxExpectedCallSizeCheck { + let is_mandatory = Self::ConsensusLogReader::schedules_authorities_change(header.digest()); + let call_size: u32 = + header.encoded_size().saturating_add(proof.encoded_size()).saturated_into(); + let max_call_size = max_expected_submit_finality_proof_arguments_size::( + is_mandatory, + proof.commit.precommits.len().saturated_into(), + ); + if call_size > max_call_size { + MaxExpectedCallSizeCheck::Exceeds { call_size, max_call_size } + } else { + MaxExpectedCallSizeCheck::Ok + } + } + /// Prepare initialization data for the GRANDPA verifier pallet. async fn prepare_initialization_data( source_client: Client, diff --git a/relays/lib-substrate-relay/src/on_demand/headers.rs b/relays/lib-substrate-relay/src/on_demand/headers.rs index 5014851a5b2f3..0090dee3a03c8 100644 --- a/relays/lib-substrate-relay/src/on_demand/headers.rs +++ b/relays/lib-substrate-relay/src/on_demand/headers.rs @@ -16,14 +16,16 @@ //! On-demand Substrate -> Substrate header finality relay. -use crate::finality::SubmitFinalityProofCallBuilder; +use crate::{ + finality::SubmitFinalityProofCallBuilder, finality_base::engine::MaxExpectedCallSizeCheck, +}; use async_std::sync::{Arc, Mutex}; use async_trait::async_trait; use bp_header_chain::ConsensusLogReader; use bp_runtime::HeaderIdProvider; use futures::{select, FutureExt}; -use num_traits::{One, Zero}; +use num_traits::{One, Saturating, Zero}; use sp_runtime::traits::Header; use finality_relay::{FinalitySyncParams, TargetClient as FinalityTargetClient}; @@ -133,29 +135,61 @@ impl OnDemandRelay, ) -> Result<(HeaderIdOf, Vec>), SubstrateError> { - // first find proper header (either `required_header`) or its descendant - let finality_source = SubstrateFinalitySource::

::new(self.source_client.clone(), None); - let (header, mut proof) = finality_source.prove_block_finality(required_header).await?; - let header_id = header.id(); + const MAX_ITERATIONS: u32 = 4; + let mut iterations = 0; + let mut current_required_header = required_header; + loop { + // first find proper header (either `current_required_header`) or its descendant + let finality_source = + SubstrateFinalitySource::

::new(self.source_client.clone(), None); + let (header, mut proof) = + finality_source.prove_block_finality(current_required_header).await?; + let header_id = header.id(); + + // optimize justification before including it into the call + P::FinalityEngine::optimize_proof(&self.target_client, &header, &mut proof).await?; + + // now we have the header and its proof, but we want to minimize our losses, so let's + // check if we'll get the full refund for submitting this header + let check_result = P::FinalityEngine::check_max_expected_call_size(&header, &proof); + if let MaxExpectedCallSizeCheck::Exceeds { call_size, max_call_size } = check_result { + iterations += 1; + current_required_header = header_id.number().saturating_add(One::one()); + if iterations < MAX_ITERATIONS { + log::debug!( + target: "bridge", + "[{}] Requested to prove {} head {:?}. Selected to prove {} head {:?}. But it is too large: {} vs {}. \ + Going to select next header", + self.relay_task_name, + P::SourceChain::NAME, + required_header, + P::SourceChain::NAME, + header_id, + call_size, + max_call_size, + ); - // optimize justification before including it into the call - P::FinalityEngine::optimize_proof(&self.target_client, &header, &mut proof).await?; + continue; + } + } - log::debug!( - target: "bridge", - "[{}] Requested to prove {} head {:?}. Selected to prove {} head {:?}", - self.relay_task_name, - P::SourceChain::NAME, - required_header, - P::SourceChain::NAME, - header_id, - ); + log::debug!( + target: "bridge", + "[{}] Requested to prove {} head {:?}. Selected to prove {} head {:?} (after {} iterations)", + self.relay_task_name, + P::SourceChain::NAME, + required_header, + P::SourceChain::NAME, + header_id, + iterations, + ); - // and then craft the submit-proof call - let call = - P::SubmitFinalityProofCallBuilder::build_submit_finality_proof_call(header, proof); + // and then craft the submit-proof call + let call = + P::SubmitFinalityProofCallBuilder::build_submit_finality_proof_call(header, proof); - Ok((header_id, vec![call])) + return Ok((header_id, vec![call])); + } } }