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.

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

Add set_partial_params dispatchable function #3843

Merged
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
6e388f0
Add set_partial_params dispatchable function
chungquantin Mar 26, 2024
64ca32d
Optimize set_partial_params and assert ParamsChanged event
chungquantin Mar 27, 2024
3cf3103
Update documentation and add prdoc
chungquantin Mar 28, 2024
68fe4ff
Reformat the prdoc
chungquantin Mar 28, 2024
1859790
Refractor partial_params method to avoid unwrap()
chungquantin Apr 18, 2024
e7c3307
Add weight and benchmark for set_partial_params
chungquantin Apr 18, 2024
efe9457
Merge branch 'master' into chungquantin/fellowship_core_set_params
chungquantin Apr 18, 2024
cdab265
Config set_partial_params in westend collectives
chungquantin Apr 18, 2024
0c558c6
Merge branch 'chungquantin/fellowship_core_set_params' of https://git…
chungquantin Apr 18, 2024
3238144
Merge branch 'master' into chungquantin/fellowship_core_set_params
chungquantin May 27, 2024
c3d9474
Update lib.rs
chungquantin May 27, 2024
d22c388
Migrate to BoundedVec with configured MaxRank
chungquantin Jun 3, 2024
cc4f5b5
Merge branch 'master' into chungquantin/fellowship_core_set_params
chungquantin Jun 3, 2024
a85b371
Update substrate/frame/core-fellowship/src/lib.rs
chungquantin Jun 3, 2024
2f8c071
Fix benchmarking code for the set_partial_params
chungquantin Jun 4, 2024
a77b512
Merge branch 'chungquantin/fellowship_core_set_params' of https://git…
chungquantin Jun 4, 2024
149dd53
hotfix benchmarking for set_partial_params
chungquantin Jun 15, 2024
71f9dee
Update prdoc/pr_3843.prdoc
chungquantin Jun 16, 2024
6f02d9a
Merge branch 'master' into chungquantin/fellowship_core_set_params
chungquantin Jun 16, 2024
488b532
fix prdoc style
chungquantin Jun 16, 2024
229c87b
add collectives-westend-runtime to prdoc
chungquantin Jun 16, 2024
087d00d
Update substrate/frame/core-fellowship/src/lib.rs
chungquantin Jun 18, 2024
0a00ddd
Format benchmarking.rs
chungquantin Jun 18, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ impl<T: frame_system::Config> pallet_core_fellowship::WeightInfo for WeightInfo<
.saturating_add(Weight::from_parts(0, 0))
.saturating_add(T::DbWeight::get().writes(1))
}
/// Storage: `AmbassadorCore::Params` (r:0 w:1)
/// Proof: `AmbassadorCore::Params` (`max_values`: Some(1), `max_size`: Some(364), added: 859, mode: `MaxEncodedLen`)
fn set_partial_params() -> Weight {
// Proof Size summary in bytes:
// Measured: `0`
// Estimated: `0`
// Minimum execution time: 11_000_000 picoseconds.
Weight::from_parts(11_000_000, 0)
.saturating_add(Weight::from_parts(0, 0))
.saturating_add(T::DbWeight::get().writes(1))
}
/// Storage: `AmbassadorCore::Member` (r:1 w:1)
/// Proof: `AmbassadorCore::Member` (`max_values`: None, `max_size`: Some(49), added: 2524, mode: `MaxEncodedLen`)
/// Storage: `AmbassadorCollective::Members` (r:1 w:1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,17 @@ impl<T: frame_system::Config> pallet_core_fellowship::WeightInfo for WeightInfo<
.saturating_add(Weight::from_parts(0, 0))
.saturating_add(T::DbWeight::get().writes(1))
}
/// Storage: `FellowshipCore::Params` (r:0 w:1)
/// Proof: `FellowshipCore::Params` (`max_values`: Some(1), `max_size`: Some(364), added: 859, mode: `MaxEncodedLen`)
fn set_partial_params() -> Weight {
// Proof Size summary in bytes:
// Measured: `0`
// Estimated: `0`
// Minimum execution time: 11_000_000 picoseconds.
Weight::from_parts(12_000_000, 0)
.saturating_add(Weight::from_parts(0, 0))
.saturating_add(T::DbWeight::get().writes(1))
}
/// Storage: `FellowshipCore::Member` (r:1 w:1)
/// Proof: `FellowshipCore::Member` (`max_values`: None, `max_size`: Some(49), added: 2524, mode: `MaxEncodedLen`)
/// Storage: `FellowshipCollective::Members` (r:1 w:1)
Expand Down
17 changes: 17 additions & 0 deletions prdoc/pr_3843.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Introduce a new dispatchable function `set_partial_params` in `pallet-core-fellowship`

doc:
- audience: Runtime Dev
description: |
This PR adds a new dispatchable function `set_partial_params`
to update config with multiple arguments without duplicating the
fields that does not need to update.

crates:
- name: pallet-core-fellowship
chungquantin marked this conversation as resolved.
Show resolved Hide resolved
bump: major
- name: collectives-westend-runtime
bump: patch
39 changes: 39 additions & 0 deletions substrate/frame/core-fellowship/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,45 @@ mod benchmarks {
Ok(())
}

#[benchmark]
fn set_partial_params() -> Result<(), BenchmarkError> {
let max_rank = T::MaxRank::get().try_into().unwrap();

// Set up the initial default state for the Params storage
let params = ParamsType {
active_salary: BoundedVec::try_from(vec![100u32.into(); max_rank]).unwrap(),
passive_salary: BoundedVec::try_from(vec![10u32.into(); max_rank]).unwrap(),
demotion_period: BoundedVec::try_from(vec![100u32.into(); max_rank]).unwrap(),
min_promotion_period: BoundedVec::try_from(vec![100u32.into(); max_rank]).unwrap(),
offboard_timeout: 1u32.into(),
};
CoreFellowship::<T, I>::set_params(RawOrigin::Root.into(), Box::new(params))?;

let default_params = Params::<T, I>::get();
let expected_params = ParamsType {
active_salary: default_params.active_salary,
passive_salary: BoundedVec::try_from(vec![10u32.into(); max_rank]).unwrap(),
demotion_period: default_params.demotion_period,
min_promotion_period: BoundedVec::try_from(vec![100u32.into(); max_rank]).unwrap(),
offboard_timeout: 1u32.into(),
};

let params_payload = ParamsType {
active_salary: BoundedVec::try_from(vec![None; max_rank]).unwrap(),
passive_salary: BoundedVec::try_from(vec![Some(10u32.into()); max_rank]).unwrap(),
demotion_period: BoundedVec::try_from(vec![None; max_rank]).unwrap(),
min_promotion_period: BoundedVec::try_from(vec![Some(100u32.into()); max_rank])
.unwrap(),
offboard_timeout: None,
};

#[extrinsic_call]
_(RawOrigin::Root, Box::new(params_payload.clone()));

assert_eq!(Params::<T, I>::get(), expected_params);
chungquantin marked this conversation as resolved.
Show resolved Hide resolved
Ok(())
}

#[benchmark]
fn bump_offboard() -> Result<(), BenchmarkError> {
set_benchmark_params::<T, I>()?;
Expand Down
55 changes: 55 additions & 0 deletions substrate/frame/core-fellowship/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,11 @@ pub mod pallet {

pub type ParamsOf<T, I> =
ParamsType<<T as Config<I>>::Balance, BlockNumberFor<T>, <T as Config<I>>::MaxRank>;
pub type PartialParamsOf<T, I> = ParamsType<
Option<<T as Config<I>>::Balance>,
Option<BlockNumberFor<T>>,
<T as Config<I>>::MaxRank,
>;
pub type MemberStatusOf<T> = MemberStatus<BlockNumberFor<T>>;
pub type RankOf<T, I> = <<T as Config<I>>::Members as RankedMembers>::Rank;

Expand Down Expand Up @@ -558,9 +563,59 @@ pub mod pallet {

Ok(Pays::No.into())
}

/// Set the parameters partially.
///
/// - `origin`: An origin complying with `ParamsOrigin` or root.
/// - `partial_params`: The new parameters for the pallet.
///
/// This update config with multiple arguments without duplicating
/// the fields that does not need to update (set to None).
#[pallet::weight(T::WeightInfo::set_partial_params())]
#[pallet::call_index(9)]
pub fn set_partial_params(
origin: OriginFor<T>,
partial_params: Box<PartialParamsOf<T, I>>,
) -> DispatchResult {
T::ParamsOrigin::ensure_origin_or_root(origin)?;
let params = Params::<T, I>::mutate(|p| {
Self::set_partial_params_slice(&mut p.active_salary, partial_params.active_salary);
Self::set_partial_params_slice(
&mut p.passive_salary,
partial_params.passive_salary,
);
Self::set_partial_params_slice(
&mut p.demotion_period,
partial_params.demotion_period,
);
Self::set_partial_params_slice(
&mut p.min_promotion_period,
partial_params.min_promotion_period,
);
if let Some(new_offboard_timeout) = partial_params.offboard_timeout {
p.offboard_timeout = new_offboard_timeout;
}
return p.clone();
chungquantin marked this conversation as resolved.
Show resolved Hide resolved
});
Self::deposit_event(Event::<T, I>::ParamsChanged { params });
Ok(())
}
}

impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// Partially update the base slice with a new slice
///
/// Only elements in the base slice which has a new value in the new slice will be updated.
pub(crate) fn set_partial_params_slice<S>(
base_slice: &mut BoundedVec<S, <T as Config<I>>::MaxRank>,
new_slice: BoundedVec<Option<S>, <T as Config<I>>::MaxRank>,
) {
for (base_element, new_element) in base_slice.iter_mut().zip(new_slice) {
if let Some(element) = new_element {
*base_element = element;
}
}
}
/// Convert a rank into a `0..RANK_COUNT` index suitable for the arrays in Params.
///
/// Rank 1 becomes index 0, rank `RANK_COUNT` becomes index `RANK_COUNT - 1`. Any rank not
Expand Down
34 changes: 34 additions & 0 deletions substrate/frame/core-fellowship/src/tests/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,40 @@ fn set_params_works() {
});
}

#[test]
fn set_partial_params_works() {
new_test_ext().execute_with(|| {
let params = ParamsType {
active_salary: bounded_vec![None; 9],
passive_salary: bounded_vec![None; 9],
demotion_period: bounded_vec![None, Some(10), None, None, None, None, None, None, None],
min_promotion_period: bounded_vec![None; 9],
offboard_timeout: Some(2),
};
assert_noop!(
CoreFellowship::set_partial_params(signed(2), Box::new(params.clone())),
DispatchError::BadOrigin
);
assert_ok!(CoreFellowship::set_partial_params(signed(1), Box::new(params)));
chungquantin marked this conversation as resolved.
Show resolved Hide resolved

// Update params from the base params value declared in `new_test_ext`
let raw_updated_params = ParamsType {
active_salary: bounded_vec![10, 20, 30, 40, 50, 60, 70, 80, 90],
passive_salary: bounded_vec![1, 2, 3, 4, 5, 6, 7, 8, 9],
demotion_period: bounded_vec![2, 10, 6, 8, 10, 12, 14, 16, 18],
min_promotion_period: bounded_vec![3, 6, 9, 12, 15, 18, 21, 24, 27],
offboard_timeout: 2,
};
// Updated params stored in Params storage value
let updated_params = Params::<Test>::get();
assert_eq!(raw_updated_params, updated_params);

System::assert_last_event(
Event::<Test, _>::ParamsChanged { params: updated_params }.into(),
);
});
}

#[test]
fn induct_works() {
new_test_ext().execute_with(|| {
Expand Down
21 changes: 21 additions & 0 deletions substrate/frame/core-fellowship/src/weights.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading