Parameters pallet #169

ggwpez · 2023-06-02T13:56:21Z

          This probably should be alterable via governance. Maybe it's time we introduced a generic Parameters pallet to give a place for all these parameters to sit?

Originally posted by @gavofyork in paritytech/cumulus#2607 (comment)

A pallet that can implement dynamic Get<…> and have configurable origins for modifying those would be nice.
Otherwise we are restricted to either use pub storage which can only be modified by root, or doing in the pallet itself.
For naming: the word Parameter is currently used as trait for function params, so maybe not best.

The text was updated successfully, but these errors were encountered:

xlc · 2023-06-14T00:54:49Z

I am going to implement this in ORML

xlc · 2023-06-19T09:15:11Z

WIP PR open-web3-stack/open-runtime-module-library#927

kianenigma · 2023-06-19T11:12:00Z

don't have anything against a central pallet per se, but this could have also been something like

#[pallet::storage(configurable)] or [pallet::storage(configurable(T::CustomOrigin))] that would auto generate setters with the given EnsureOrigin impl, and EnsureRoot if left blank.

If named #[pallet::setter], could have made the story around getters and setters also more consistent, although I am personally still not a fan of getters to begin with (see paritytech/substrate#13365).

So all in all, something like:

#[pallet::storage]
#[pallet::getter(fn foo)]
#[pallet::setter(T::FooOrigin)] // `T::FooOrigin::ensure_origin(o)?` is enough to set `Foo` via `set_foo`. 
type Foo<T: Config> = StorageValue<Value = u32>

UPDATE: call index of something like above would also be hard to manage 👎

ggwpez · 2023-06-19T11:25:56Z

UPDATE: call index of something like above would also be hard to manage -1

Yes, unless we only have one set_parameter function. It could be made type-safe by using an Enum over all parameters and still checking the origin.
But i agree that we can just start with a pallet as well.

xlc · 2023-06-19T22:53:50Z

A central pallet allows a single place for all the parameters. This is super useful for governance / audit / review. No need to check every single pallet for governance calls, which each of them may have different naming convention etc.

xlc · 2023-06-19T23:12:00Z

One thing about variable Get is that it could make metadata variable, which is unexpected #1139

muharem · 2023-07-10T09:53:49Z

how about having these features:

custom origin pre key (we could use a different tracks to set different values)
default value for a key (might be useful and existing pallets do not have to be adjusted to expect the None)
a new attribute and trait to replace #[pallet::constant], where the trait impl can return the full key for the value in the storage. The full key can be included to the metadata, and clients can fetch the value from storage or use default if the storage value is None.

what you think?

xlc · 2023-07-10T10:59:31Z

Every feature request must come with some use cases otherwise we will be wasting time implementing things no one is using, which we already have a few instances in Substrate.

muharem · 2023-07-13T10:49:49Z

I believe we already have the use cases for those features,

for example, fellowship salary budget will be controlled by Treasurer origin/track, staking related configs by StakingAdmin;
if we can not set a default value, the pallets has to be adjusted to handle None variant;
if no new attribute with the full key to the storage value, there is no way for clients to know the value behind the key (now they get it from metadata as constant).

xlc · 2023-07-13T11:16:18Z

https://github.com/open-web3-stack/open-runtime-module-library/blob/7ac9410a8e61fc74b025354d88a0beff61152dca/parameters/src/mock.rs#L75

You have access to the key and can do whatever logics you want to control the permission.

https://github.com/open-web3-stack/open-runtime-module-library/blob/7ac9410a8e61fc74b025354d88a0beff61152dca/traits/src/parameters.rs#L14

this returns an option so user will just add unwrap_or_else to support default value

if no new attribute with the full key to the storage value, there is no way for clients to know the value behind the key (now they get it from metadata as constant).

I don't 100% get what you mean but I will just say my implementation don't have this issue.

muharem · 2023-07-14T09:03:42Z

true, solved with EnsureOriginWithArg

regarding the last point, let me give you an example.

pallet_salary::Config{ 
   ...
   #[pallet::constant]
   Budget: Get<Balance>
}

now with this configuration and static value for Budget clients can get this value from the metadata.
what if I replace that static value by a getter like

impl<Default, ParameterStore> Get<..> for Budget<..> {
  fn get() -> Balance {
      ParameterStore::get(key).unwrap_or_else(Default::get())
  }
}

how clients will retrieve the budget?

xlc · 2023-07-14T09:18:14Z

The key type defined by the macro contains all the keys and client can use them to query for parameters.

Basically replace api.const.salary.budge with api.query.parameters.parameters({ Salary: { Budget: undefiled } })

bkchr · 2023-07-14T18:44:23Z

Basically replace api.const.salary.budge with api.query.parameters.parameters({ Salary: { Budget: undefiled } })

I like :)

xlc · 2023-09-05T01:53:21Z

orml-parameters is ready open-web3-stack/open-runtime-module-library#927
Example usage at Acala AcalaNetwork/Acala#2605

kianenigma · 2023-09-18T14:45:50Z

Here's how I would think about this, in terms of the final API:

// in some ./runtime/lib.rs
// A u32 parameter
pub struct Parameter1; 
impl FRAMEParameter<u32> for Parameter1 {
    // Something that has to implement `EnsureOrigin`
    type AdminOrigin = ...
    /// Something that is Get<u32> the default value for this parameter if not present in storage. 
    /// (if you update this after the parameter has already been set in storage, it would probably have no effect, but that's an implementation detail)
    type Default = ...
}


// A Perbill parameter.
pub InflationRate; 
impl FRAMEParameter<Perbill> for InflationRate {
    type AdminOrigin = ...
    type Default = ...
}

impl pallet_parameters::Config for Runtime {
    type Parameters = (Parameter1, InflationRate);
}

This will give you a custom origin per parameter.

EDIT: the implementaiton by @xlc also supports this with EnsureOriginWithArg.

muharem · 2023-10-20T10:42:45Z

@xlc nice
I will try it in Collectives and share here with you

ggwpez · 2023-10-26T14:58:34Z

orml-parameters is ready open-web3-stack/open-runtime-module-library#927 Example usage at Acala AcalaNetwork/Acala#2605

I think we can start using this in Rococ/Westend to try it out (eg. pallet NIS). With Kusama/Polkadot it needs audit first.
I will prepare something.

PS: It seems like the ORML repo is not exposing a Rust workspace, so we will have to copy&paste it.

@xlc

Closes #169 Fork of the `orml-parameters-pallet` as introduced by open-web3-stack/open-runtime-module-library#927 (cc @xlc) It greatly changes how the macros work, but keeps the pallet the same. The downside of my code is now that it does only support constant keys in the form of types, not value-bearing keys. I think this is an acceptable trade off, give that it can be used by *any* pallet without any changes. The pallet allows to dynamically set parameters that can be used in pallet configs while also restricting the updating on a per-key basis. The rust-docs contains a complete example. Changes: - Add `parameters-pallet` - Use in the kitchensink as demonstration - Add experimental attribute to define dynamic params in the runtime. - Adding a bunch of traits to `frame_support::traits::dynamic_params` that can be re-used by the ORML macros ## Example First to define the parameters in the runtime file. The syntax is very explicit about the codec index and errors if there is no. ```rust #[dynamic_params(RuntimeParameters, pallet_parameters::Parameters::<Runtime>))] pub mod dynamic_params { use super::*; #[dynamic_pallet_params] #[codec(index = 0)] pub mod storage { /// Configures the base deposit of storing some data. #[codec(index = 0)] pub static BaseDeposit: Balance = 1 * DOLLARS; /// Configures the per-byte deposit of storing some data. #[codec(index = 1)] pub static ByteDeposit: Balance = 1 * CENTS; } #[dynamic_pallet_params] #[codec(index = 1)] pub mod contracts { #[codec(index = 0)] pub static DepositPerItem: Balance = deposit(1, 0); #[codec(index = 1)] pub static DepositPerByte: Balance = deposit(0, 1); } } ``` Then the pallet is configured with the aggregate: ```rust impl pallet_parameters::Config for Runtime { type AggregratedKeyValue = RuntimeParameters; type AdminOrigin = EnsureRootWithSuccess<AccountId, ConstBool<true>>; ... } ``` And then the parameters can be used in a pallet config: ```rust impl pallet_preimage::Config for Runtime { type DepositBase = dynamic_params::storage::DepositBase; } ``` A custom origin an be defined like this: ```rust pub struct DynamicParametersManagerOrigin; impl EnsureOriginWithArg<RuntimeOrigin, RuntimeParametersKey> for DynamicParametersManagerOrigin { type Success = (); fn try_origin( origin: RuntimeOrigin, key: &RuntimeParametersKey, ) -> Result<Self::Success, RuntimeOrigin> { match key { RuntimeParametersKey::Storage(_) => { frame_system::ensure_root(origin.clone()).map_err(|_| origin)?; return Ok(()) }, RuntimeParametersKey::Contract(_) => { frame_system::ensure_root(origin.clone()).map_err(|_| origin)?; return Ok(()) }, } } #[cfg(feature = "runtime-benchmarks")] fn try_successful_origin(_key: &RuntimeParametersKey) -> Result<RuntimeOrigin, ()> { Ok(RuntimeOrigin::Root) } } ``` --------- Signed-off-by: Oliver Tale-Yazdi <[email protected]> Co-authored-by: Nikhil Gupta <[email protected]> Co-authored-by: Kian Paimani <[email protected]> Co-authored-by: command-bot <>

@xlc

Closes paritytech#169 Fork of the `orml-parameters-pallet` as introduced by open-web3-stack/open-runtime-module-library#927 (cc @xlc) It greatly changes how the macros work, but keeps the pallet the same. The downside of my code is now that it does only support constant keys in the form of types, not value-bearing keys. I think this is an acceptable trade off, give that it can be used by *any* pallet without any changes. The pallet allows to dynamically set parameters that can be used in pallet configs while also restricting the updating on a per-key basis. The rust-docs contains a complete example. Changes: - Add `parameters-pallet` - Use in the kitchensink as demonstration - Add experimental attribute to define dynamic params in the runtime. - Adding a bunch of traits to `frame_support::traits::dynamic_params` that can be re-used by the ORML macros ## Example First to define the parameters in the runtime file. The syntax is very explicit about the codec index and errors if there is no. ```rust #[dynamic_params(RuntimeParameters, pallet_parameters::Parameters::<Runtime>))] pub mod dynamic_params { use super::*; #[dynamic_pallet_params] #[codec(index = 0)] pub mod storage { /// Configures the base deposit of storing some data. #[codec(index = 0)] pub static BaseDeposit: Balance = 1 * DOLLARS; /// Configures the per-byte deposit of storing some data. #[codec(index = 1)] pub static ByteDeposit: Balance = 1 * CENTS; } #[dynamic_pallet_params] #[codec(index = 1)] pub mod contracts { #[codec(index = 0)] pub static DepositPerItem: Balance = deposit(1, 0); #[codec(index = 1)] pub static DepositPerByte: Balance = deposit(0, 1); } } ``` Then the pallet is configured with the aggregate: ```rust impl pallet_parameters::Config for Runtime { type AggregratedKeyValue = RuntimeParameters; type AdminOrigin = EnsureRootWithSuccess<AccountId, ConstBool<true>>; ... } ``` And then the parameters can be used in a pallet config: ```rust impl pallet_preimage::Config for Runtime { type DepositBase = dynamic_params::storage::DepositBase; } ``` A custom origin an be defined like this: ```rust pub struct DynamicParametersManagerOrigin; impl EnsureOriginWithArg<RuntimeOrigin, RuntimeParametersKey> for DynamicParametersManagerOrigin { type Success = (); fn try_origin( origin: RuntimeOrigin, key: &RuntimeParametersKey, ) -> Result<Self::Success, RuntimeOrigin> { match key { RuntimeParametersKey::Storage(_) => { frame_system::ensure_root(origin.clone()).map_err(|_| origin)?; return Ok(()) }, RuntimeParametersKey::Contract(_) => { frame_system::ensure_root(origin.clone()).map_err(|_| origin)?; return Ok(()) }, } } #[cfg(feature = "runtime-benchmarks")] fn try_successful_origin(_key: &RuntimeParametersKey) -> Result<RuntimeOrigin, ()> { Ok(RuntimeOrigin::Root) } } ``` --------- Signed-off-by: Oliver Tale-Yazdi <[email protected]> Co-authored-by: Nikhil Gupta <[email protected]> Co-authored-by: Kian Paimani <[email protected]> Co-authored-by: command-bot <>

ggwpez added T1-runtime and removed U3-nice_to_have labels Jun 2, 2023

ggwpez mentioned this issue Jun 4, 2023

Runtime: Polkadot Fellowship promotion/demotion periods, members activity and salaries paritytech/cumulus#2607

Merged

2 tasks

xlc mentioned this issue Jun 19, 2023

orml-parameters open-web3-stack/open-runtime-module-library#927

Merged

2 tasks

ggwpez assigned xlc Jun 19, 2023

juangirini transferred this issue from paritytech/substrate Aug 24, 2023

the-right-joyce added T1-FRAME This PR/Issue is related to core FRAME, the framework. and removed T1-runtime labels Aug 25, 2023

ggwpez mentioned this issue Nov 1, 2023

[FRAME] Parameters pallet #2061

Merged

ggwpez closed this as completed in #2061 Feb 8, 2024

github-project-automation bot moved this from In Progress to Done in Runtime / FRAME Feb 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parameters pallet #169

Parameters pallet #169

ggwpez commented Jun 2, 2023 •

edited

Loading

xlc commented Jun 14, 2023

xlc commented Jun 19, 2023 •

edited

Loading

kianenigma commented Jun 19, 2023 •

edited

Loading

ggwpez commented Jun 19, 2023 •

edited

Loading

xlc commented Jun 19, 2023

xlc commented Jun 19, 2023

muharem commented Jul 10, 2023

xlc commented Jul 10, 2023

muharem commented Jul 13, 2023

xlc commented Jul 13, 2023

muharem commented Jul 14, 2023

xlc commented Jul 14, 2023

bkchr commented Jul 14, 2023

xlc commented Sep 5, 2023

kianenigma commented Sep 18, 2023 •

edited

Loading

muharem commented Oct 20, 2023

ggwpez commented Oct 26, 2023 •

edited

Loading

Parameters pallet #169

Parameters pallet #169

Comments

ggwpez commented Jun 2, 2023 • edited Loading

xlc commented Jun 14, 2023

xlc commented Jun 19, 2023 • edited Loading

kianenigma commented Jun 19, 2023 • edited Loading

ggwpez commented Jun 19, 2023 • edited Loading

xlc commented Jun 19, 2023

xlc commented Jun 19, 2023

muharem commented Jul 10, 2023

xlc commented Jul 10, 2023

muharem commented Jul 13, 2023

xlc commented Jul 13, 2023

muharem commented Jul 14, 2023

xlc commented Jul 14, 2023

bkchr commented Jul 14, 2023

xlc commented Sep 5, 2023

kianenigma commented Sep 18, 2023 • edited Loading

muharem commented Oct 20, 2023

ggwpez commented Oct 26, 2023 • edited Loading

ggwpez commented Jun 2, 2023 •

edited

Loading

xlc commented Jun 19, 2023 •

edited

Loading

kianenigma commented Jun 19, 2023 •

edited

Loading

ggwpez commented Jun 19, 2023 •

edited

Loading

kianenigma commented Sep 18, 2023 •

edited

Loading

ggwpez commented Oct 26, 2023 •

edited

Loading