Skip to content

Commit

Permalink
Box large arguments of GRANDPA pallet (paritytech#1154)
Browse files Browse the repository at this point in the history
* box large arguments

* benchmarks

* fix
  • Loading branch information
svyatonik authored and serban300 committed Apr 10, 2024
1 parent 23072a0 commit cc25369
Show file tree
Hide file tree
Showing 18 changed files with 67 additions and 33 deletions.
8 changes: 8 additions & 0 deletions bridges/bin/millau/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<pallet_bridge_grandpa::Call<Runtime>>() <= MAX_CALL_SIZE);
assert!(core::mem::size_of::<pallet_bridge_messages::Call<Runtime>>() <= MAX_CALL_SIZE);
}
}
8 changes: 8 additions & 0 deletions bridges/bin/rialto/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<pallet_bridge_grandpa::Call<Runtime>>() <= MAX_CALL_SIZE);
assert!(core::mem::size_of::<pallet_bridge_messages::Call<Runtime>>() <= MAX_CALL_SIZE);
}
}
4 changes: 2 additions & 2 deletions bridges/modules/grandpa/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fn prepare_benchmark_data<T: Config<I>, I: 'static>(
.collect::<Vec<_>>();

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,
Expand Down Expand Up @@ -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::<T, I>(p, v);
}: submit_finality_proof(RawOrigin::Signed(caller), header, justification)
}: submit_finality_proof(RawOrigin::Signed(caller), Box::new(header), justification)
verify {
let header: BridgedHeader<T, I> = bp_test_utils::test_header(header_number::<T, I, _>());
let expected_hash = header.hash();
Expand Down
34 changes: 19 additions & 15 deletions bridges/modules/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -130,7 +130,7 @@ pub mod pallet {
))]
pub fn submit_finality_proof(
origin: OriginFor<T>,
finality_target: BridgedHeader<T, I>,
finality_target: Box<BridgedHeader<T, I>>,
justification: GrandpaJustification<BridgedHeader<T, I>>,
) -> DispatchResultWithPostInfo {
ensure_operational::<T, I>()?;
Expand Down Expand Up @@ -166,7 +166,7 @@ pub mod pallet {

let is_authorities_change_enacted = try_enact_authority_change::<T, I>(&finality_target, set_id)?;
<RequestCount<T, I>>::mutate(|count| *count += 1);
insert_header::<T, I>(finality_target, hash);
insert_header::<T, I>(*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
Expand Down Expand Up @@ -471,7 +471,7 @@ pub mod pallet {
let initial_hash = header.hash();
<InitialHash<T, I>>::put(initial_hash);
<ImportedHashesPointer<T, I>>::put(0);
insert_header::<T, I>(header, initial_hash);
insert_header::<T, I>(*header, initial_hash);

let authority_set = bp_header_chain::AuthoritySet::new(authority_list, set_id);
<CurrentAuthoritySet<T, I>>::put(authority_set);
Expand Down Expand Up @@ -598,7 +598,7 @@ pub(crate) fn find_forced_change<H: HeaderT>(
#[cfg(feature = "runtime-benchmarks")]
pub fn initialize_for_benchmarks<T: Config<I>, I: 'static>(header: BridgedHeader<T, I>) {
initialize_bridge::<T, I>(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,
Expand Down Expand Up @@ -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,
Expand All @@ -641,7 +641,7 @@ mod tests {
let header = test_header(header.into());
let justification = make_default_justification(&header);

Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), header, justification)
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), Box::new(header), justification)
}

fn next_block() {
Expand Down Expand Up @@ -828,7 +828,7 @@ mod tests {
let justification = make_justification_for_header(params);

assert_err!(
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), header, justification,),
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), Box::new(header), justification,),
<Error<TestRuntime>>::InvalidJustification
);
})
Expand All @@ -844,7 +844,7 @@ mod tests {
justification.round = 42;

assert_err!(
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), header, justification,),
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), Box::new(header), justification,),
<Error<TestRuntime>>::InvalidJustification
);
})
Expand All @@ -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,
Expand All @@ -869,7 +869,7 @@ mod tests {
let justification = make_default_justification(&header);

assert_err!(
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), header, justification,),
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), Box::new(header), justification,),
<Error<TestRuntime>>::InvalidAuthoritySet
);
})
Expand Down Expand Up @@ -904,7 +904,11 @@ mod tests {

// Let's import our test header
assert_ok!(
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), header.clone(), justification),
Pallet::<TestRuntime>::submit_finality_proof(
Origin::signed(1),
Box::new(header.clone()),
justification
),
PostDispatchInfo {
actual_weight: None,
pays_fee: frame_support::weights::Pays::No,
Expand Down Expand Up @@ -938,7 +942,7 @@ mod tests {

// Should not be allowed to import this header
assert_err!(
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), header, justification),
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), Box::new(header), justification),
<Error<TestRuntime>>::UnsupportedScheduledChange
);
})
Expand All @@ -959,7 +963,7 @@ mod tests {

// Should not be allowed to import this header
assert_err!(
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), header, justification),
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), Box::new(header), justification),
<Error<TestRuntime>>::UnsupportedScheduledChange
);
})
Expand Down Expand Up @@ -1017,7 +1021,7 @@ mod tests {
let mut invalid_justification = make_default_justification(&header);
invalid_justification.round = 42;

Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), header, invalid_justification)
Pallet::<TestRuntime>::submit_finality_proof(Origin::signed(1), Box::new(header), invalid_justification)
};

initialize_substrate_bridge();
Expand Down
3 changes: 2 additions & 1 deletion bridges/primitives/header-chain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -62,7 +63,7 @@ impl AuthoritySet {
#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
pub struct InitializationData<H: HeaderT> {
/// The header from which we should start syncing.
pub header: H,
pub header: Box<H>,
/// The initial authorities of the pallet.
pub authority_list: AuthorityList,
/// The ID of the initial authority set.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ impl SubstrateFinalitySyncPipeline for KusamaFinalityToPolkadot {
proof: GrandpaJustification<bp_kusama::Header>,
) -> 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ impl SubstrateFinalitySyncPipeline for MillauFinalityToRialto {
header: MillauSyncHeader,
proof: GrandpaJustification<bp_millau::Header>,
) -> 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(
Expand Down
6 changes: 3 additions & 3 deletions bridges/relays/bin-substrate/src/chains/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<millau_runtime::Runtime>::submit_finality_proof(
header,
Box::new(header),
justification,
);

Expand Down Expand Up @@ -327,7 +327,7 @@ mod westend_tests {

let actual = bp_westend::BridgeGrandpaRococoCall::submit_finality_proof(header.clone(), justification.clone());
let expected = millau_runtime::BridgeGrandpaRialtoCall::<millau_runtime::Runtime>::submit_finality_proof(
header,
Box::new(header),
justification,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ impl SubstrateFinalitySyncPipeline for PolkadotFinalityToKusama {
proof: GrandpaJustification<bp_polkadot::Header>,
) -> 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ impl SubstrateFinalitySyncPipeline for RococoFinalityToWococo {
proof: GrandpaJustification<bp_rococo::Header>,
) -> 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ impl SubstrateFinalitySyncPipeline for WococoFinalityToRococo {
proof: GrandpaJustification<bp_wococo::Header>,
) -> 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(
Expand Down
2 changes: 1 addition & 1 deletion bridges/relays/client-kusama/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub enum BalancesCall {
pub enum BridgePolkadotGrandpaCall {
#[codec(index = 0)]
submit_finality_proof(
<PolkadotLike as Chain>::Header,
Box<<PolkadotLike as Chain>::Header>,
bp_header_chain::justification::GrandpaJustification<<PolkadotLike as Chain>::Header>,
),
#[codec(index = 1)]
Expand Down
2 changes: 1 addition & 1 deletion bridges/relays/client-polkadot/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub enum BalancesCall {
pub enum BridgeKusamaGrandpaCall {
#[codec(index = 0)]
submit_finality_proof(
<PolkadotLike as Chain>::Header,
Box<<PolkadotLike as Chain>::Header>,
bp_header_chain::justification::GrandpaJustification<<PolkadotLike as Chain>::Header>,
),
#[codec(index = 1)]
Expand Down
2 changes: 1 addition & 1 deletion bridges/relays/client-rococo/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub enum SystemCall {
pub enum BridgeGrandpaWococoCall {
#[codec(index = 0)]
submit_finality_proof(
<PolkadotLike as Chain>::Header,
Box<<PolkadotLike as Chain>::Header>,
bp_header_chain::justification::GrandpaJustification<<PolkadotLike as Chain>::Header>,
),
#[codec(index = 1)]
Expand Down
2 changes: 1 addition & 1 deletion bridges/relays/client-wococo/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub enum SystemCall {
pub enum BridgeGrandpaRococoCall {
#[codec(index = 0)]
submit_finality_proof(
<PolkadotLike as Chain>::Header,
Box<<PolkadotLike as Chain>::Header>,
bp_header_chain::justification::GrandpaJustification<<PolkadotLike as Chain>::Header>,
),
#[codec(index = 1)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ async fn prepare_initialization_data<SourceChain: Chain>(
}

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
Expand Down

0 comments on commit cc25369

Please sign in to comment.