From cc253696b1ddc6e1e62eaec455ef6c8b9d4114b7 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 22 Sep 2021 13:20:10 +0300 Subject: [PATCH] Box large arguments of GRANDPA pallet (#1154) * box large arguments * benchmarks * fix --- bridges/bin/millau/runtime/src/lib.rs | 8 +++++ bridges/bin/rialto/runtime/src/lib.rs | 8 +++++ bridges/modules/grandpa/src/benchmarking.rs | 4 +-- bridges/modules/grandpa/src/lib.rs | 34 +++++++++++-------- bridges/primitives/header-chain/src/lib.rs | 3 +- .../src/chains/kusama_headers_to_polkadot.rs | 5 ++- .../src/chains/millau_headers_to_rialto.rs | 3 +- .../relays/bin-substrate/src/chains/mod.rs | 6 ++-- .../src/chains/polkadot_headers_to_kusama.rs | 5 ++- .../src/chains/rialto_headers_to_millau.rs | 2 +- .../src/chains/rococo_headers_to_wococo.rs | 5 ++- .../src/chains/westend_headers_to_millau.rs | 2 +- .../src/chains/wococo_headers_to_rococo.rs | 5 ++- bridges/relays/client-kusama/src/runtime.rs | 2 +- bridges/relays/client-polkadot/src/runtime.rs | 2 +- bridges/relays/client-rococo/src/runtime.rs | 2 +- bridges/relays/client-wococo/src/runtime.rs | 2 +- .../src/headers_initialize.rs | 2 +- 18 files changed, 67 insertions(+), 33 deletions(-) diff --git a/bridges/bin/millau/runtime/src/lib.rs b/bridges/bin/millau/runtime/src/lib.rs index 69e993b7bb78d..e395c378125af 100644 --- a/bridges/bin/millau/runtime/src/lib.rs +++ b/bridges/bin/millau/runtime/src/lib.rs @@ -750,4 +750,12 @@ mod tests { DbWeight::get(), ); } + + #[test] + fn call_size() { + // pallets that are (to be) used by polkadot runtime + const MAX_CALL_SIZE: usize = 230; // value from polkadot-runtime tests + assert!(core::mem::size_of::>() <= MAX_CALL_SIZE); + assert!(core::mem::size_of::>() <= MAX_CALL_SIZE); + } } diff --git a/bridges/bin/rialto/runtime/src/lib.rs b/bridges/bin/rialto/runtime/src/lib.rs index 52206a8cf1693..bdbea8438b11b 100644 --- a/bridges/bin/rialto/runtime/src/lib.rs +++ b/bridges/bin/rialto/runtime/src/lib.rs @@ -1377,4 +1377,12 @@ mod tests { additional_amount }); } + + #[test] + fn call_size() { + // pallets that are (to be) used by polkadot runtime + const MAX_CALL_SIZE: usize = 230; // value from polkadot-runtime tests + assert!(core::mem::size_of::>() <= MAX_CALL_SIZE); + assert!(core::mem::size_of::>() <= MAX_CALL_SIZE); + } } diff --git a/bridges/modules/grandpa/src/benchmarking.rs b/bridges/modules/grandpa/src/benchmarking.rs index d2e73c529d0aa..1714024928dc5 100644 --- a/bridges/modules/grandpa/src/benchmarking.rs +++ b/bridges/modules/grandpa/src/benchmarking.rs @@ -80,7 +80,7 @@ fn prepare_benchmark_data, I: 'static>( .collect::>(); let init_data = InitializationData { - header: bp_test_utils::test_header(Zero::zero()), + header: Box::new(bp_test_utils::test_header(Zero::zero())), authority_list, set_id: TEST_GRANDPA_SET_ID, is_halted: false, @@ -109,7 +109,7 @@ benchmarks_instance_pallet! { let v in 1..MAX_VOTE_ANCESTRIES; let caller: T::AccountId = whitelisted_caller(); let (header, justification) = prepare_benchmark_data::(p, v); - }: submit_finality_proof(RawOrigin::Signed(caller), header, justification) + }: submit_finality_proof(RawOrigin::Signed(caller), Box::new(header), justification) verify { let header: BridgedHeader = bp_test_utils::test_header(header_number::()); let expected_hash = header.hash(); diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index 49a775077c4c3..f1599a5ebfbaf 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -46,7 +46,7 @@ use frame_support::{ensure, fail}; use frame_system::{ensure_signed, RawOrigin}; use sp_finality_grandpa::{ConsensusLog, GRANDPA_ENGINE_ID}; use sp_runtime::traits::{BadOrigin, Header as HeaderT, Zero}; -use sp_std::convert::TryInto; +use sp_std::{boxed::Box, convert::TryInto}; #[cfg(test)] mod mock; @@ -130,7 +130,7 @@ pub mod pallet { ))] pub fn submit_finality_proof( origin: OriginFor, - finality_target: BridgedHeader, + finality_target: Box>, justification: GrandpaJustification>, ) -> DispatchResultWithPostInfo { ensure_operational::()?; @@ -166,7 +166,7 @@ pub mod pallet { let is_authorities_change_enacted = try_enact_authority_change::(&finality_target, set_id)?; >::mutate(|count| *count += 1); - insert_header::(finality_target, hash); + insert_header::(*finality_target, hash); log::info!(target: "runtime::bridge-grandpa", "Succesfully imported finalized header with hash {:?}!", hash); // mandatory header is a header that changes authorities set. The pallet can't go further @@ -471,7 +471,7 @@ pub mod pallet { let initial_hash = header.hash(); >::put(initial_hash); >::put(0); - insert_header::(header, initial_hash); + insert_header::(*header, initial_hash); let authority_set = bp_header_chain::AuthoritySet::new(authority_list, set_id); >::put(authority_set); @@ -598,7 +598,7 @@ pub(crate) fn find_forced_change( #[cfg(feature = "runtime-benchmarks")] pub fn initialize_for_benchmarks, I: 'static>(header: BridgedHeader) { initialize_bridge::(InitializationData { - header, + header: Box::new(header), authority_list: sp_std::vec::Vec::new(), // we don't verify any proofs in external benchmarks set_id: 0, is_halted: false, @@ -628,7 +628,7 @@ mod tests { let genesis = test_header(0); let init_data = InitializationData { - header: genesis, + header: Box::new(genesis), authority_list: authority_list(), set_id: 1, is_halted: false, @@ -641,7 +641,7 @@ mod tests { let header = test_header(header.into()); let justification = make_default_justification(&header); - Pallet::::submit_finality_proof(Origin::signed(1), header, justification) + Pallet::::submit_finality_proof(Origin::signed(1), Box::new(header), justification) } fn next_block() { @@ -828,7 +828,7 @@ mod tests { let justification = make_justification_for_header(params); assert_err!( - Pallet::::submit_finality_proof(Origin::signed(1), header, justification,), + Pallet::::submit_finality_proof(Origin::signed(1), Box::new(header), justification,), >::InvalidJustification ); }) @@ -844,7 +844,7 @@ mod tests { justification.round = 42; assert_err!( - Pallet::::submit_finality_proof(Origin::signed(1), header, justification,), + Pallet::::submit_finality_proof(Origin::signed(1), Box::new(header), justification,), >::InvalidJustification ); }) @@ -857,7 +857,7 @@ mod tests { let invalid_authority_list = vec![(ALICE.into(), u64::MAX), (BOB.into(), u64::MAX)]; let init_data = InitializationData { - header: genesis, + header: Box::new(genesis), authority_list: invalid_authority_list, set_id: 1, is_halted: false, @@ -869,7 +869,7 @@ mod tests { let justification = make_default_justification(&header); assert_err!( - Pallet::::submit_finality_proof(Origin::signed(1), header, justification,), + Pallet::::submit_finality_proof(Origin::signed(1), Box::new(header), justification,), >::InvalidAuthoritySet ); }) @@ -904,7 +904,11 @@ mod tests { // Let's import our test header assert_ok!( - Pallet::::submit_finality_proof(Origin::signed(1), header.clone(), justification), + Pallet::::submit_finality_proof( + Origin::signed(1), + Box::new(header.clone()), + justification + ), PostDispatchInfo { actual_weight: None, pays_fee: frame_support::weights::Pays::No, @@ -938,7 +942,7 @@ mod tests { // Should not be allowed to import this header assert_err!( - Pallet::::submit_finality_proof(Origin::signed(1), header, justification), + Pallet::::submit_finality_proof(Origin::signed(1), Box::new(header), justification), >::UnsupportedScheduledChange ); }) @@ -959,7 +963,7 @@ mod tests { // Should not be allowed to import this header assert_err!( - Pallet::::submit_finality_proof(Origin::signed(1), header, justification), + Pallet::::submit_finality_proof(Origin::signed(1), Box::new(header), justification), >::UnsupportedScheduledChange ); }) @@ -1017,7 +1021,7 @@ mod tests { let mut invalid_justification = make_default_justification(&header); invalid_justification.round = 42; - Pallet::::submit_finality_proof(Origin::signed(1), header, invalid_justification) + Pallet::::submit_finality_proof(Origin::signed(1), Box::new(header), invalid_justification) }; initialize_substrate_bridge(); diff --git a/bridges/primitives/header-chain/src/lib.rs b/bridges/primitives/header-chain/src/lib.rs index adac6eb268802..f1eaa28d071f0 100644 --- a/bridges/primitives/header-chain/src/lib.rs +++ b/bridges/primitives/header-chain/src/lib.rs @@ -29,6 +29,7 @@ use serde::{Deserialize, Serialize}; use sp_finality_grandpa::{AuthorityList, ConsensusLog, SetId, GRANDPA_ENGINE_ID}; use sp_runtime::RuntimeDebug; use sp_runtime::{generic::OpaqueDigestItemId, traits::Header as HeaderT}; +use sp_std::boxed::Box; pub mod justification; @@ -62,7 +63,7 @@ impl AuthoritySet { #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] pub struct InitializationData { /// The header from which we should start syncing. - pub header: H, + pub header: Box, /// The initial authorities of the pallet. pub authority_list: AuthorityList, /// The ID of the initial authority set. diff --git a/bridges/relays/bin-substrate/src/chains/kusama_headers_to_polkadot.rs b/bridges/relays/bin-substrate/src/chains/kusama_headers_to_polkadot.rs index 4e7703529e32d..b3b7d956bcc51 100644 --- a/bridges/relays/bin-substrate/src/chains/kusama_headers_to_polkadot.rs +++ b/bridges/relays/bin-substrate/src/chains/kusama_headers_to_polkadot.rs @@ -88,7 +88,10 @@ impl SubstrateFinalitySyncPipeline for KusamaFinalityToPolkadot { proof: GrandpaJustification, ) -> Bytes { let call = relay_polkadot_client::runtime::Call::BridgeKusamaGrandpa( - relay_polkadot_client::runtime::BridgeKusamaGrandpaCall::submit_finality_proof(header.into_inner(), proof), + relay_polkadot_client::runtime::BridgeKusamaGrandpaCall::submit_finality_proof( + Box::new(header.into_inner()), + proof, + ), ); let genesis_hash = *self.finality_pipeline.target_client.genesis_hash(); let transaction = Polkadot::sign_transaction( diff --git a/bridges/relays/bin-substrate/src/chains/millau_headers_to_rialto.rs b/bridges/relays/bin-substrate/src/chains/millau_headers_to_rialto.rs index fa51200b6f879..f0ea225485bd4 100644 --- a/bridges/relays/bin-substrate/src/chains/millau_headers_to_rialto.rs +++ b/bridges/relays/bin-substrate/src/chains/millau_headers_to_rialto.rs @@ -59,7 +59,8 @@ impl SubstrateFinalitySyncPipeline for MillauFinalityToRialto { header: MillauSyncHeader, proof: GrandpaJustification, ) -> Bytes { - let call = rialto_runtime::BridgeGrandpaMillauCall::submit_finality_proof(header.into_inner(), proof).into(); + let call = + rialto_runtime::BridgeGrandpaMillauCall::submit_finality_proof(Box::new(header.into_inner()), proof).into(); let genesis_hash = *self.finality_pipeline.target_client.genesis_hash(); let transaction = Rialto::sign_transaction( diff --git a/bridges/relays/bin-substrate/src/chains/mod.rs b/bridges/relays/bin-substrate/src/chains/mod.rs index 4c02e9b1c98b2..bdc4f2628ee56 100644 --- a/bridges/relays/bin-substrate/src/chains/mod.rs +++ b/bridges/relays/bin-substrate/src/chains/mod.rs @@ -278,11 +278,11 @@ mod rococo_tests { }; let actual = relay_rococo_client::runtime::BridgeGrandpaWococoCall::submit_finality_proof( - header.clone(), + Box::new(header.clone()), justification.clone(), ); let expected = millau_runtime::BridgeGrandpaRialtoCall::::submit_finality_proof( - header, + Box::new(header), justification, ); @@ -327,7 +327,7 @@ mod westend_tests { let actual = bp_westend::BridgeGrandpaRococoCall::submit_finality_proof(header.clone(), justification.clone()); let expected = millau_runtime::BridgeGrandpaRialtoCall::::submit_finality_proof( - header, + Box::new(header), justification, ); diff --git a/bridges/relays/bin-substrate/src/chains/polkadot_headers_to_kusama.rs b/bridges/relays/bin-substrate/src/chains/polkadot_headers_to_kusama.rs index aee66b8a3f270..c225b77d546aa 100644 --- a/bridges/relays/bin-substrate/src/chains/polkadot_headers_to_kusama.rs +++ b/bridges/relays/bin-substrate/src/chains/polkadot_headers_to_kusama.rs @@ -88,7 +88,10 @@ impl SubstrateFinalitySyncPipeline for PolkadotFinalityToKusama { proof: GrandpaJustification, ) -> Bytes { let call = relay_kusama_client::runtime::Call::BridgePolkadotGrandpa( - relay_kusama_client::runtime::BridgePolkadotGrandpaCall::submit_finality_proof(header.into_inner(), proof), + relay_kusama_client::runtime::BridgePolkadotGrandpaCall::submit_finality_proof( + Box::new(header.into_inner()), + proof, + ), ); let genesis_hash = *self.finality_pipeline.target_client.genesis_hash(); let transaction = Kusama::sign_transaction( diff --git a/bridges/relays/bin-substrate/src/chains/rialto_headers_to_millau.rs b/bridges/relays/bin-substrate/src/chains/rialto_headers_to_millau.rs index 4f649702b7a3c..df0f17d94431a 100644 --- a/bridges/relays/bin-substrate/src/chains/rialto_headers_to_millau.rs +++ b/bridges/relays/bin-substrate/src/chains/rialto_headers_to_millau.rs @@ -63,7 +63,7 @@ impl SubstrateFinalitySyncPipeline for RialtoFinalityToMillau { let call = millau_runtime::BridgeGrandpaRialtoCall::< millau_runtime::Runtime, millau_runtime::RialtoGrandpaInstance, - >::submit_finality_proof(header.into_inner(), proof) + >::submit_finality_proof(Box::new(header.into_inner()), proof) .into(); let genesis_hash = *self.finality_pipeline.target_client.genesis_hash(); diff --git a/bridges/relays/bin-substrate/src/chains/rococo_headers_to_wococo.rs b/bridges/relays/bin-substrate/src/chains/rococo_headers_to_wococo.rs index 5d8cf255398c5..269dead8dc231 100644 --- a/bridges/relays/bin-substrate/src/chains/rococo_headers_to_wococo.rs +++ b/bridges/relays/bin-substrate/src/chains/rococo_headers_to_wococo.rs @@ -83,7 +83,10 @@ impl SubstrateFinalitySyncPipeline for RococoFinalityToWococo { proof: GrandpaJustification, ) -> Bytes { let call = relay_wococo_client::runtime::Call::BridgeGrandpaRococo( - relay_wococo_client::runtime::BridgeGrandpaRococoCall::submit_finality_proof(header.into_inner(), proof), + relay_wococo_client::runtime::BridgeGrandpaRococoCall::submit_finality_proof( + Box::new(header.into_inner()), + proof, + ), ); let genesis_hash = *self.finality_pipeline.target_client.genesis_hash(); let transaction = Wococo::sign_transaction( diff --git a/bridges/relays/bin-substrate/src/chains/westend_headers_to_millau.rs b/bridges/relays/bin-substrate/src/chains/westend_headers_to_millau.rs index 8501ff41868a0..4a5e48b51eeab 100644 --- a/bridges/relays/bin-substrate/src/chains/westend_headers_to_millau.rs +++ b/bridges/relays/bin-substrate/src/chains/westend_headers_to_millau.rs @@ -71,7 +71,7 @@ impl SubstrateFinalitySyncPipeline for WestendFinalityToMillau { let call = millau_runtime::BridgeGrandpaWestendCall::< millau_runtime::Runtime, millau_runtime::WestendGrandpaInstance, - >::submit_finality_proof(header.into_inner(), proof) + >::submit_finality_proof(Box::new(header.into_inner()), proof) .into(); let genesis_hash = *self.finality_pipeline.target_client.genesis_hash(); diff --git a/bridges/relays/bin-substrate/src/chains/wococo_headers_to_rococo.rs b/bridges/relays/bin-substrate/src/chains/wococo_headers_to_rococo.rs index b06921bfd9cec..74152cdff6d41 100644 --- a/bridges/relays/bin-substrate/src/chains/wococo_headers_to_rococo.rs +++ b/bridges/relays/bin-substrate/src/chains/wococo_headers_to_rococo.rs @@ -88,7 +88,10 @@ impl SubstrateFinalitySyncPipeline for WococoFinalityToRococo { proof: GrandpaJustification, ) -> Bytes { let call = relay_rococo_client::runtime::Call::BridgeGrandpaWococo( - relay_rococo_client::runtime::BridgeGrandpaWococoCall::submit_finality_proof(header.into_inner(), proof), + relay_rococo_client::runtime::BridgeGrandpaWococoCall::submit_finality_proof( + Box::new(header.into_inner()), + proof, + ), ); let genesis_hash = *self.finality_pipeline.target_client.genesis_hash(); let transaction = Rococo::sign_transaction( diff --git a/bridges/relays/client-kusama/src/runtime.rs b/bridges/relays/client-kusama/src/runtime.rs index 4b490b2799b7e..f2145f5b02ff1 100644 --- a/bridges/relays/client-kusama/src/runtime.rs +++ b/bridges/relays/client-kusama/src/runtime.rs @@ -96,7 +96,7 @@ pub enum BalancesCall { pub enum BridgePolkadotGrandpaCall { #[codec(index = 0)] submit_finality_proof( - ::Header, + Box<::Header>, bp_header_chain::justification::GrandpaJustification<::Header>, ), #[codec(index = 1)] diff --git a/bridges/relays/client-polkadot/src/runtime.rs b/bridges/relays/client-polkadot/src/runtime.rs index 63d0987d32315..1c5d42a00c222 100644 --- a/bridges/relays/client-polkadot/src/runtime.rs +++ b/bridges/relays/client-polkadot/src/runtime.rs @@ -96,7 +96,7 @@ pub enum BalancesCall { pub enum BridgeKusamaGrandpaCall { #[codec(index = 0)] submit_finality_proof( - ::Header, + Box<::Header>, bp_header_chain::justification::GrandpaJustification<::Header>, ), #[codec(index = 1)] diff --git a/bridges/relays/client-rococo/src/runtime.rs b/bridges/relays/client-rococo/src/runtime.rs index d01f8a5dfd0f6..b68aea985e772 100644 --- a/bridges/relays/client-rococo/src/runtime.rs +++ b/bridges/relays/client-rococo/src/runtime.rs @@ -85,7 +85,7 @@ pub enum SystemCall { pub enum BridgeGrandpaWococoCall { #[codec(index = 0)] submit_finality_proof( - ::Header, + Box<::Header>, bp_header_chain::justification::GrandpaJustification<::Header>, ), #[codec(index = 1)] diff --git a/bridges/relays/client-wococo/src/runtime.rs b/bridges/relays/client-wococo/src/runtime.rs index 18fde4629a933..daf20156d69e8 100644 --- a/bridges/relays/client-wococo/src/runtime.rs +++ b/bridges/relays/client-wococo/src/runtime.rs @@ -85,7 +85,7 @@ pub enum SystemCall { pub enum BridgeGrandpaRococoCall { #[codec(index = 0)] submit_finality_proof( - ::Header, + Box<::Header>, bp_header_chain::justification::GrandpaJustification<::Header>, ), #[codec(index = 1)] diff --git a/bridges/relays/lib-substrate-relay/src/headers_initialize.rs b/bridges/relays/lib-substrate-relay/src/headers_initialize.rs index 4a7a0798bbf62..4a3a16bbe1775 100644 --- a/bridges/relays/lib-substrate-relay/src/headers_initialize.rs +++ b/bridges/relays/lib-substrate-relay/src/headers_initialize.rs @@ -213,7 +213,7 @@ async fn prepare_initialization_data( } Ok(InitializationData { - header: initial_header, + header: Box::new(initial_header), authority_list: initial_authorities_set, set_id: if schedules_change { initial_authorities_set_id + 1