Pallet parameters account for all parameters read weight #8
Conversation
TarekkMA
changed the title
TarekkMA
changed the title
| #[pallet::hooks] | ||
| impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> { | ||
| fn on_initialize(_: BlockNumberFor<T>) -> Weight { | ||
| let items = Parameters::<T>::iter().count() as u64; |
There was a problem hiding this comment.
Looking at the support/procedural/src/dynamic_params.rs in polkadot-sdk, I do not see any storage being set for the initial values (it fallbacks to the defaults when storage is not present). Instead we should get the length from T::RuntimeParameters::default(), because the storage will be empty in the benchmarks.
There was a problem hiding this comment.
I do not see any insert for the initial values
We don't need to insert initial values in production, only in benchmark setup (for the on_initialize benchmark scenario that still need to be added in this PR)
| fn on_initialize(_: BlockNumberFor<T>) -> Weight { | ||
| let proof_size_before: u64 = get_proof_size().unwrap_or(0); | ||
|
|
||
| let items = Parameters::<T>::iter().count() as u64; |
There was a problem hiding this comment.
I still do not agree with the need for a on_initialize hook here. Also Parameters::<T>::iter() will be empty until set_parameter is called.
There was a problem hiding this comment.
Not all parameter usage is captured by our benchmarks. For instance, some parameters may be read within xcm or in other unaccounted areas. As a result, we can't rely solely on benchmark coverage to determine parameter impact.
To mitigate potential issues, we should assume that all parameters stored in each block will be read.
This approach helps us avoid hitting the proof size limit and prevents our chain from stalling, which would otherwise require recovery through relay chain governance.
There was a problem hiding this comment.
Parameters::<T>::iter() initially will be empty, but as we add more and more parameters it will report all the stored ones
Co-authored-by: Rodrigo Quelhas <[email protected]>
|
The only thing missing is the storage whitelist |
Account for all parameters read weight on every block, and whitelist the parameters storage so the weight is not double counted.