Skip to content

Commit

Permalink
MaxValues limit for storage maps in the pallet-bridge-grandpa (parity…
Browse files Browse the repository at this point in the history 
…tech#1861)

* MaxValues limit for storage maps in the pallet-bridge-grandpa

* remove use from the future PR
  • Loading branch information
@svyatonik @serban300
svyatonik authored and serban300 committed Apr 9, 2024
1 parent 34c5d02 commit 4caed27
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 92 deletions.
24 changes: 12 additions & 12 deletions bridges/modules/grandpa/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use frame_benchmarking::{benchmarks_instance_pallet, whitelisted_caller};
use frame_support::traits::Get;
use frame_system::RawOrigin;
use sp_finality_grandpa::AuthorityId;
use sp_runtime::traits::Zero;
use sp_runtime::traits::{One, Zero};
use sp_std::vec::Vec;

/// The maximum number of vote ancestries to include in a justification.
Expand All @@ -76,14 +76,6 @@ fn validator_set_range_end<T: Config<I>, I: 'static>() -> u32 {
}
}

/// Returns number of first header to be imported.
///
/// Since we bootstrap the pallet with `HeadersToKeep` already imported headers,
/// this function computes the next expected header number to import.
fn header_number<T: Config<I>, I: 'static, N: From<u32>>() -> N {
(T::HeadersToKeep::get() + 1).into()
}

/// Prepare header and its justification to submit using `submit_finality_proof`.
fn prepare_benchmark_data<T: Config<I>, I: 'static>(
precommits: u32,
Expand All @@ -94,16 +86,19 @@ fn prepare_benchmark_data<T: Config<I>, I: 'static>(
.map(|id| (AuthorityId::from(*id), 1))
.collect::<Vec<_>>();

let genesis_header: BridgedHeader<T, I> = bp_test_utils::test_header(Zero::zero());
let genesis_hash = genesis_header.hash();
let init_data = InitializationData {
header: Box::new(bp_test_utils::test_header(Zero::zero())),
header: Box::new(genesis_header),
authority_list,
set_id: TEST_GRANDPA_SET_ID,
operating_mode: BasicOperatingMode::Normal,
};

bootstrap_bridge::<T, I>(init_data);
assert!(<ImportedHeaders<T, I>>::contains_key(genesis_hash));

let header: BridgedHeader<T, I> = bp_test_utils::test_header(header_number::<T, I, _>());
let header: BridgedHeader<T, I> = bp_test_utils::test_header(One::one());
let params = JustificationGeneratorParams {
header: header.clone(),
round: TEST_GRANDPA_ROUND,
Expand All @@ -126,10 +121,15 @@ benchmarks_instance_pallet! {
let (header, justification) = prepare_benchmark_data::<T, I>(p, v);
}: 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 genesis_header: BridgedHeader<T, I> = bp_test_utils::test_header(Zero::zero());
let header: BridgedHeader<T, I> = bp_test_utils::test_header(One::one());
let expected_hash = header.hash();

// check that the header#1 has been inserted
assert_eq!(<BestFinalized<T, I>>::get().unwrap().1, expected_hash);
assert!(<ImportedHeaders<T, I>>::contains_key(expected_hash));

// check that the header#0 has been pruned
assert!(!<ImportedHeaders<T, I>>::contains_key(genesis_header.hash()));
}
}
105 changes: 50 additions & 55 deletions bridges/modules/grandpa/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,6 @@ pub mod pallet {
#[pallet::constant]
type MaxRequests: Get<u32>;

// Avoid using `HeadersToKeep` directly in the pallet code. Use `headers_to_keep` function
// instead.

/// Maximal number of finalized headers to keep in the storage.
///
/// The setting is there to prevent growing the on-chain state indefinitely. Note
Expand Down Expand Up @@ -292,8 +289,14 @@ pub mod pallet {

/// A ring buffer of imported hashes. Ordered by the insertion time.
#[pallet::storage]
pub(super) type ImportedHashes<T: Config<I>, I: 'static = ()> =
StorageMap<_, Identity, u32, BridgedBlockHash<T, I>>;
pub(super) type ImportedHashes<T: Config<I>, I: 'static = ()> = StorageMap<
Hasher = Identity,
Key = u32,
Value = BridgedBlockHash<T, I>,
QueryKind = OptionQuery,
OnEmpty = GetDefault,
MaxValues = MaybeHeadersToKeep<T, I>,
>;

/// Current ring buffer position.
#[pallet::storage]
Expand All @@ -302,8 +305,14 @@ pub mod pallet {

/// Relevant fields of imported headers.
#[pallet::storage]
pub type ImportedHeaders<T: Config<I>, I: 'static = ()> =
StorageMap<_, Identity, BridgedBlockHash<T, I>, BridgedStoredHeaderData<T, I>>;
pub type ImportedHeaders<T: Config<I>, I: 'static = ()> = StorageMap<
Hasher = Identity,
Key = BridgedBlockHash<T, I>,
Value = BridgedStoredHeaderData<T, I>,
QueryKind = OptionQuery,
OnEmpty = GetDefault,
MaxValues = MaybeHeadersToKeep<T, I>,
>;

/// The current GRANDPA Authority set.
#[pallet::storage]
Expand Down Expand Up @@ -467,20 +476,6 @@ pub mod pallet {
})?)
}

/// Return number of headers to keep in the runtime storage.
#[cfg(not(feature = "runtime-benchmarks"))]
pub(crate) fn headers_to_keep<T: Config<I>, I: 'static>() -> u32 {
T::HeadersToKeep::get()
}

/// Return number of headers to keep in the runtime storage.
#[cfg(feature = "runtime-benchmarks")]
pub(crate) fn headers_to_keep<T: Config<I>, I: 'static>() -> u32 {
// every db operation (significantly) slows down benchmarks, so let's keep as min as
// possible
2
}

/// Import a previously verified header to the storage.
///
/// Note this function solely takes care of updating the storage and pruning old entries,
Expand All @@ -496,7 +491,7 @@ pub mod pallet {
<ImportedHashes<T, I>>::insert(index, hash);

// Update ring buffer pointer and remove old header.
<ImportedHashesPointer<T, I>>::put((index + 1) % headers_to_keep::<T, I>());
<ImportedHashesPointer<T, I>>::put((index + 1) % T::HeadersToKeep::get());
if let Ok(hash) = pruning {
log::debug!(target: LOG_TARGET, "Pruning old header: {:?}.", hash);
<ImportedHeaders<T, I>>::remove(hash);
Expand Down Expand Up @@ -535,27 +530,35 @@ pub mod pallet {
Ok(())
}

/// Adapter for using `Config::HeadersToKeep` as `MaxValues` bound in our storage maps.
pub struct MaybeHeadersToKeep<T, I>(PhantomData<(T, I)>);

// this implementation is required to use the struct as `MaxValues`
impl<T: Config<I>, I: 'static> Get<Option<u32>> for MaybeHeadersToKeep<T, I> {
fn get() -> Option<u32> {
Some(T::HeadersToKeep::get())
}
}

/// Initialize pallet so that it is ready for inserting new header.
///
/// The function makes sure that the new insertion will cause the pruning of some old header.
///
/// Returns parent header for the new header.
#[cfg(feature = "runtime-benchmarks")]
pub(crate) fn bootstrap_bridge<T: Config<I>, I: 'static>(
init_params: super::InitializationData<BridgedHeader<T, I>>,
) {
let start_number = *init_params.header.number();
let end_number = start_number + headers_to_keep::<T, I>().into();
) -> BridgedHeader<T, I> {
let start_header = init_params.header.clone();
initialize_bridge::<T, I>(init_params).expect("benchmarks are correct");

let mut number = start_number;
while number < end_number {
number = number + sp_runtime::traits::One::one();
let header = <BridgedHeader<T, I>>::new(
number,
Default::default(),
Default::default(),
Default::default(),
Default::default(),
);
let hash = header.hash();
insert_header::<T, I>(header, hash);
}
// the most obvious way to cause pruning during next insertion would be to insert
// `HeadersToKeep` headers. But it'll make our benchmarks slow. So we will just play with
// our pruning ring-buffer.
assert_eq!(ImportedHashesPointer::<T, I>::get(), 1);
ImportedHashesPointer::<T, I>::put(0);

*start_header
}
}

Expand Down Expand Up @@ -816,13 +819,9 @@ mod tests {
fn succesfully_imports_header_with_valid_finality() {
run_test(|| {
initialize_substrate_bridge();
assert_ok!(
submit_finality_proof(1),
PostDispatchInfo {
actual_weight: None,
pays_fee: frame_support::dispatch::Pays::Yes,
},
);
let result = submit_finality_proof(1);
assert_ok!(result);
assert_eq!(result.unwrap().pays_fee, frame_support::dispatch::Pays::Yes);

let header = test_header(1);
assert_eq!(<BestFinalized<TestRuntime>>::get().unwrap().1, header.hash());
Expand Down Expand Up @@ -929,17 +928,13 @@ mod tests {
let justification = make_default_justification(&header);

// Let's import our test header
assert_ok!(
Pallet::<TestRuntime>::submit_finality_proof(
RuntimeOrigin::signed(1),
Box::new(header.clone()),
justification
),
PostDispatchInfo {
actual_weight: None,
pays_fee: frame_support::dispatch::Pays::No,
},
let result = Pallet::<TestRuntime>::submit_finality_proof(
RuntimeOrigin::signed(1),
Box::new(header.clone()),
justification,
);
assert_ok!(result);
assert_eq!(result.unwrap().pays_fee, frame_support::dispatch::Pays::No);

// Make sure that our header is the best finalized
assert_eq!(<BestFinalized<TestRuntime>>::get().unwrap().1, header.hash());
Expand Down
50 changes: 25 additions & 25 deletions bridges/modules/grandpa/src/weights.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
//! Autogenerated weights for pallet_bridge_grandpa
//!
//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
//! DATE: 2023-02-06, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! DATE: 2023-02-07, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]`
//! WORST CASE MAP SIZE: `1000000`
//! HOSTNAME: `covid`, CPU: `11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz`
//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
Expand Down Expand Up @@ -85,27 +85,27 @@ impl<T: frame_system::Config> WeightInfo for BridgeWeight<T> {
///
/// Storage: BridgeRialtoGrandpa ImportedHashes (r:1 w:1)
///
/// Proof: BridgeRialtoGrandpa ImportedHashes (max_values: None, max_size: Some(36), added:
/// 2511, mode: MaxEncodedLen)
/// Proof: BridgeRialtoGrandpa ImportedHashes (max_values: Some(14400), max_size: Some(36),
/// added: 2016, mode: MaxEncodedLen)
///
/// Storage: BridgeRialtoGrandpa ImportedHeaders (r:0 w:2)
///
/// Proof: BridgeRialtoGrandpa ImportedHeaders (max_values: None, max_size: Some(68), added:
/// 2543, mode: MaxEncodedLen)
/// Proof: BridgeRialtoGrandpa ImportedHeaders (max_values: Some(14400), max_size: Some(68),
/// added: 2048, mode: MaxEncodedLen)
///
/// The range of component `p` is `[1, 5]`.
///
/// The range of component `v` is `[50, 100]`.
fn submit_finality_proof(p: u32, v: u32) -> Weight {
// Proof Size summary in bytes:
// Measured: `459 + p * (40 ±0)`
// Estimated: `5240`
// Minimum execution time: 368_734 nanoseconds.
Weight::from_parts(64_214_587, 5240)
// Standard Error: 226_504
.saturating_add(Weight::from_ref_time(41_231_918).saturating_mul(p.into()))
// Standard Error: 20_667
.saturating_add(Weight::from_ref_time(2_770_962).saturating_mul(v.into()))
// Measured: `416 + p * (40 ±0)`
// Estimated: `4745`
// Minimum execution time: 221_703 nanoseconds.
Weight::from_parts(39_358_497, 4745)
// Standard Error: 85_573
.saturating_add(Weight::from_ref_time(40_593_280).saturating_mul(p.into()))
// Standard Error: 7_808
.saturating_add(Weight::from_ref_time(1_529_400).saturating_mul(v.into()))
.saturating_add(T::DbWeight::get().reads(6_u64))
.saturating_add(T::DbWeight::get().writes(6_u64))
}
Expand Down Expand Up @@ -140,27 +140,27 @@ impl WeightInfo for () {
///
/// Storage: BridgeRialtoGrandpa ImportedHashes (r:1 w:1)
///
/// Proof: BridgeRialtoGrandpa ImportedHashes (max_values: None, max_size: Some(36), added:
/// 2511, mode: MaxEncodedLen)
/// Proof: BridgeRialtoGrandpa ImportedHashes (max_values: Some(14400), max_size: Some(36),
/// added: 2016, mode: MaxEncodedLen)
///
/// Storage: BridgeRialtoGrandpa ImportedHeaders (r:0 w:2)
///
/// Proof: BridgeRialtoGrandpa ImportedHeaders (max_values: None, max_size: Some(68), added:
/// 2543, mode: MaxEncodedLen)
/// Proof: BridgeRialtoGrandpa ImportedHeaders (max_values: Some(14400), max_size: Some(68),
/// added: 2048, mode: MaxEncodedLen)
///
/// The range of component `p` is `[1, 5]`.
///
/// The range of component `v` is `[50, 100]`.
fn submit_finality_proof(p: u32, v: u32) -> Weight {
// Proof Size summary in bytes:
// Measured: `459 + p * (40 ±0)`
// Estimated: `5240`
// Minimum execution time: 368_734 nanoseconds.
Weight::from_parts(64_214_587, 5240)
// Standard Error: 226_504
.saturating_add(Weight::from_ref_time(41_231_918).saturating_mul(p.into()))
// Standard Error: 20_667
.saturating_add(Weight::from_ref_time(2_770_962).saturating_mul(v.into()))
// Measured: `416 + p * (40 ±0)`
// Estimated: `4745`
// Minimum execution time: 221_703 nanoseconds.
Weight::from_parts(39_358_497, 4745)
// Standard Error: 85_573
.saturating_add(Weight::from_ref_time(40_593_280).saturating_mul(p.into()))
// Standard Error: 7_808
.saturating_add(Weight::from_ref_time(1_529_400).saturating_mul(v.into()))
.saturating_add(RocksDbWeight::get().reads(6_u64))
.saturating_add(RocksDbWeight::get().writes(6_u64))
}
Expand Down

0 comments on commit 4caed27

Please sign in to comment.