Bound uses of `Call`
Polkadot companion: paritytech/polkadot#5729 Related: https://github.com/paritytech/substrate/issues/12070
Introduces a new utility type Bounded<T> and two new traits QueryPreimage and StorePreimage which are meant to be used instead of the old (and probably soon to be deprecated) PreimageProvider and PreimageRecipient. A new Scheduler API is also introduced (under traits::scheduler::v3) which uses the Bounded<Call> type for being passed the dispatchable (rather than the previous ValueOrHash<Call>).
As a result, Referenda, Democracy, Scheduler and Preimage pallets are all now bounded in storage access footprint. Preimage pallet takes a complexity parameter in the note API allowing the expected PoV weight to be properly profiled without using the impractically pessimistic MEL for the storage item which is degeneratively large. The QueryPreimage and StorePreimage trait APIs also ensure that a length is retained to allow any users of them to similarly profile and bound their weight. This is in use in Scheduler pallet, but will need similar logic introducing for any other pallets which expect to do hash lookups of wildly varying preimage sizes.
Specific Changes
Runtime Primitives
General
- Introduce
DispatchErrorvariantsExhausted,Corruption,Unavailable.
Bounded
- Implement
DefaultforBoundedSlice - Introduce
BoundedSlice::truncate_from - In
BoundedVec,try_insert&try_pushreturn the item to be introduced if the case of error. (NOTE: This has resulted in some minor alterations throughout the codebase.) - Similarly
BoundedVec::try_fromreturns the sourceVecin case of error.
Frame Support
Schedule
Introduce v3 which removes unbounded components from the previous v2 API.
- The
callparam is nowBounded<Call>. - The
idparam is nowTaskName=[u8; 32]
Preimages
Introduce preimages.rs containing:
- New traits
StorePreimageandQueryPreimagewith more succinct language in use for the operations; - New type
Bounded<T>which takes a MEL-unbounded typeTand provides a MEL-bounded wrapper which can yield the originalTthrough a preimage-based API.Bounded<T>may be created with aStorePreimage::into_bounded(T) -> Result<Bounded<T>>, or alternatively withBounded<T>::unrequested(hash: Hash, len: u32)orBounded<T>::requested(hash: Hash, len: u32)and efficiently placed in storage. If the latter API is used, then it cannot be retrieved until the hash's preimage is introduced into the preimages backend (e.g. the Preimage pallet). This makes it quite useful for governance use-cases like Referenda where only the hash is supplied initially.- When the original
Tvalue is needed, it can be retrieved withQueryPreimage::into_value(b: &Bounded<T>) -> Result<T>.
CallerTrait
- Introduce a new trait for the
Origin::Callerwhich allows attempting conversion into theRawOrigin(referenced or consuming). - Introduce convenience functions
into_caller()andas_system_reftoOriginTraitwhich make use ofCallerTrait. - Reimplement
OriginTrait::as_signedto use these functions.
Frame System
frame_system::Config::Call is now bounded sensibly (Parameter and Dispatchable with the right Origin) to avoid the need to re-bound in other pallets which may need to use it.
Scheduler
- Functionally decompose
on_initializeinto four calls which can be weighted in isolationservice_agendas,service_agenda,service_task_*andexecute_dispatch_*. This is primarily due to the two-stage weight check (first we check PoV-weight for the possible lookup and bail if too much, then we check overall weight for dispatch and bail if too much). It's also quite a lot easier to comprehend. - Remove
on_initializeweight/benchmark combination in favour of individual benchmarks for the decomposed functions. - Refactor tests.
- For named tasks, use
[u8; 32]rather than an (unbounded)Vec. - Introduce
IncompleteSinceallowing O(1) postponing of agenda items. - Introduce migrations.
- Some other refactoring (e.g. introduce
place_task).
Referenda
Refactor to ensure well-bounded in PoV footprint.
- Use scheduler API
v3withBounded<Call>instead of using call or its hash directly.
Preimage
- Implements the two new preimages traits.
- Remove the
MaxSizeparameter and just handle all reasonable sizes. - Index preimage by hash and length.
- Store the length (if known) in the hash status.
- This allows for operations using preimage-lookups to have their PoV-footprint benchmarked and known ahead of time.
- Do tests and migrations.
- Slight change to API:
note_preimagerecords the preimage and also requests it if it is coming from a superuser (i.e. if there is no depositor). The assumption being that if you're the superuser and are noting a preimage then you probably want it to stick around until you're done with it.
Democracy
Refactor to ensure well-bounded in PoV footprint.
- Use
Bounded<Call>instead of aHashof the proposal throughout except when the preimage is unneeded for the function (e.g. for blacklisting). - Introduce
MELto types throughout. - Refactor tests & benchmarks accordingly.
- Completely remove any preimage handling code.
- Remove unused complexity parameters from benchmark/weight functions.
- Introduce
EncodeInto(though this should probably be moved toparity_scale_codecin due course). - Rearrange the config trait items to keep system-level stuff at the top, then constants, then origins & imbalance handlers.
Migration notes
Runtime storage migrations must be run for:
- Preimage pallet
- Scheduler pallet
- Democracy pallet
Democracy
Three items in the Config trait are removed Proposal, PreimageByteDeposit, OperationalPreimageOrigin.
Three items have been added Preimages, MaxDeposits, MaxBlacklisted; assuming your runtime already uses the Preimages pallet, here are example changes:
impl pallet_democracy::Config for Runtime {
// snip
type Proposal = /* snip */;
type PreimageByteDeposit = /* snip */;
type OperationalPreimageOrigin = /* snip */;
}
becomes:
impl pallet_democracy::Config for Runtime {
// snip
type Preimages = Preimage;
type MaxDeposits = ConstU32<100>;
type MaxBlacklisted = ConstU32<100>;
}
There is migration code which moves existing proposals and referenda over to the new format. However IT DOES NOT MIGRATE EVERYTHING:
- Preimages are NOT migrated. Any registered preimages in Democracy at the time of migration are dropped. Their balance is NOT UNRESERVED.
- The re-dispatcher used in the old Democracy implementation is removed. Any proposals scheduled for dispatch by Democracy WILL NOT EXECUTE.
This means you SHOULD ensure that:
- the preimage for the runtime upgrade is placed as an imminent preimage, not with a deposit;
- no other preimages are in place at the time of upgrade;
- there are no other proposals scheduled for dispatch by Democracy at the time of upgrade.
The Democracy pallet will be marked as deprecated immediately once Referenda is considered production-ready. ALL TEAMS ARE RECOMMENDED TO SWITCH SWAY FROM DEMOCRACY PALLET TO REFERENDA/CONVICTION-VOTING PALLETS ASAP
Preimage
Preimage pallet has had MaxSize type removed from its Config trait. Example code changes:
impl pallet_preimage::Config for Runtime {
// snip
type MaxSize = /* snip */;
}
becomes
impl pallet_preimage::Config for Runtime {
// snip
}
Scheduler
In Scheduler's Config trait, PreimageProvider has been renamed to Preimages and NoPreimagePostponement has been removed (since its corresponding feature is also removed), so:
impl pallet_scheduler::Config for Runtime {
// snip
type PreimageProvider = Preimage;
type NoPreimagePostponement = NoPreimagePostponement;
}
becomes
impl pallet_scheduler::Config for Runtime {
// snip
type Preimages = Preimage;
}
If you find tests are breaking, and especially where Scheduler appears not to be executing scheduled items check the MaximumWeight config parameter of Scheduler and the weight of whatever function is being called. Scheduler has been changed to remove the concept of a "hard deadline" or weight-override priority and no longer guarantees that at least one scheduled item will be executed per block (since these are both dangerous to parachains which have a strict need of weight limits). This means you must ensure that scheduled items are below the MaximumWeight or they will not be executed.
TODO
- [x] Remove the different Preimage maps; just use real benchmarks for PoV-weight.
- [x] Consider fixing Democracy pallet's dispatch
- [x] Migration in Democracy for
ReferendumInfoOf,NextExternal,PublicProps. - [x] Tests in Preimage pallet should use new preimage trait APIs
- [x] Tests in Scheduler pallet for changed functionality
- [x] Test Democracy's migration
- [x] Scheduler migration test needs a closer look @ggwpez
- [ ] Re-generate weights and use use new weight functions
- [ ] Discussions resolved
@ggwpez Should be ready for a full review now.
ill try to do a second pass tmrw focusing on tests, benchmarks, and migration
This pull request has been mentioned on Polkadot Forum. There might be relevant details there:
https://forum.polkadot.network/t/parachain-technical-summit-next-steps/51/12
This pull request has been mentioned on Polkadot Forum. There might be relevant details there:
https://forum.polkadot.network/t/parachain-technical-summit-next-steps/51/15
bot rebase
Rebased
bot rebase
Rebased
CI blocked by https://github.com/paritytech/polkadot/pull/6061
/cmd queue -c bench-bot $ pallet dev pallet_democracy /cmd queue -c bench-bot $ pallet dev pallet_preimage /cmd queue -c bench-bot $ pallet dev pallet_scheduler
@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909327 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_democracy. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.
Comment /cmd cancel 2-becafb42-6f03-4678-ad49-75c7c8dbfdcd to cancel this command or /cmd cancel to cancel all commands in this pull request.
@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909328 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_preimage. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.
Comment /cmd cancel 3-4b0e470b-5a39-4245-8e37-0db8a5380e70 to cancel this command or /cmd cancel to cancel all commands in this pull request.
@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909329 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_scheduler. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.
Comment /cmd cancel 4-18602fea-b27e-4dd8-9581-7720746f57cf to cancel this command or /cmd cancel to cancel all commands in this pull request.
@ggwpez Command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_preimage has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909328 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909328/artifacts/download.
@ggwpez Command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_democracy has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909327 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909327/artifacts/download.
@ggwpez Command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_scheduler has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909329 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909329/artifacts/download.
/cmd queue -c bench-bot $ pallet dev pallet_preimage
@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909723 was started for your command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_preimage. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.
Comment /cmd cancel 5-53e84f22-e73b-44a3-a7b2-e91d5da5f6bf to cancel this command or /cmd cancel to cancel all commands in this pull request.
@ggwpez Command "$PIPELINE_SCRIPTS_DIR/bench-bot.sh" pallet dev pallet_preimage has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909723 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1909723/artifacts/download.
bot merge
Error: "Check reviews" status is not passing for https://github.com/paritytech/polkadot/pull/5729
This pull request has been mentioned on Polkadot Forum. There might be relevant details there:
https://forum.polkadot.network/t/polkadot-release-analysis/1026/2
This pull request has been mentioned on Polkadot Forum. There might be relevant details there:
https://forum.polkadot.network/t/polkadot-digest-11-jan-2023/1654/1
This pull request has been mentioned on Polkadot Forum. There might be relevant details there:
https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-34/1432/8
This pull request has been mentioned on Polkadot Forum. There might be relevant details there:
https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-38/2122/1