Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MaxValues limit for storage maps in the pallet-bridge-grandpa #1861

Merged
merged 2 commits into from
Feb 8, 2023
Merged

MaxValues limit for storage maps in the pallet-bridge-grandpa #1861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbd93c7
MaxValues limit for storage maps in the pallet-bridge-grandpa
svyatonik Feb 7, 2023
7469718
remove use from the future PR
svyatonik Feb 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions modules/grandpa/src/benchmarking.rs
View file
Open in desktop
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 modules/grandpa/src/lib.rs
View file
Open in desktop
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 modules/grandpa/src/weights.rs
View file
Open in desktop
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