From 2c02e8910a7b619dabd7d153c8d3612b7a5900cf Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 16 Aug 2024 11:54:03 +0200 Subject: [PATCH 1/4] maybe consideration --- .../balances/src/tests/fungible_tests.rs | 12 ++++++-- substrate/frame/support/src/traits.rs | 5 ++-- substrate/frame/support/src/traits/storage.rs | 16 ++++++++++ .../support/src/traits/tokens/fungible/mod.rs | 29 ++++++++++++++++++- 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/substrate/frame/balances/src/tests/fungible_tests.rs b/substrate/frame/balances/src/tests/fungible_tests.rs index e1c1be9b1335a..ac373a351d343 100644 --- a/substrate/frame/balances/src/tests/fungible_tests.rs +++ b/substrate/frame/balances/src/tests/fungible_tests.rs @@ -25,7 +25,7 @@ use frame_support::traits::{ Preservation::{Expendable, Preserve, Protect}, Restriction::Free, }, - Consideration, Footprint, LinearStoragePrice, + Consideration, Footprint, LinearStoragePrice, MaybeConsideration, }; use fungible::{ FreezeConsideration, HoldConsideration, Inspect, InspectFreeze, InspectHold, @@ -519,6 +519,7 @@ fn freeze_consideration_works() { // freeze amount taken somewhere outside of our (Consideration) scope. let extend_freeze = 15; let zero_ticket = Consideration::new(&who, Footprint::from_parts(0, 0)).unwrap(); + assert!(zero_ticket.is_none()); assert_eq!(Balances::balance_frozen(&TestId::Foo, &who), 0); let ticket = Consideration::new(&who, Footprint::from_parts(10, 1)).unwrap(); @@ -533,7 +534,9 @@ fn freeze_consideration_works() { let ticket = ticket.update(&who, Footprint::from_parts(8, 1)).unwrap(); assert_eq!(Balances::balance_frozen(&TestId::Foo, &who), 8 + extend_freeze); - assert_eq!(ticket.update(&who, Footprint::from_parts(0, 0)).unwrap(), zero_ticket); + let ticket = ticket.update(&who, Footprint::from_parts(0, 0)).unwrap(); + assert!(ticket.is_none()); + assert_eq!(ticket, zero_ticket); assert_eq!(Balances::balance_frozen(&TestId::Foo, &who), 0 + extend_freeze); let ticket = Consideration::new(&who, Footprint::from_parts(10, 1)).unwrap(); @@ -563,6 +566,7 @@ fn hold_consideration_works() { let extend_hold = 15; let zero_ticket = Consideration::new(&who, Footprint::from_parts(0, 0)).unwrap(); + assert!(zero_ticket.is_none()); assert_eq!(Balances::balance_on_hold(&TestId::Foo, &who), 0); let ticket = Consideration::new(&who, Footprint::from_parts(10, 1)).unwrap(); @@ -577,7 +581,9 @@ fn hold_consideration_works() { let ticket = ticket.update(&who, Footprint::from_parts(8, 1)).unwrap(); assert_eq!(Balances::balance_on_hold(&TestId::Foo, &who), 8 + extend_hold); - assert_eq!(ticket.update(&who, Footprint::from_parts(0, 0)).unwrap(), zero_ticket); + let ticket = ticket.update(&who, Footprint::from_parts(0, 0)).unwrap(); + assert!(ticket.is_none()); + assert_eq!(ticket, zero_ticket); assert_eq!(Balances::balance_on_hold(&TestId::Foo, &who), 0 + extend_hold); let ticket = Consideration::new(&who, Footprint::from_parts(10, 1)).unwrap(); diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index a423656c394f2..5e572b18f09c2 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -93,8 +93,9 @@ pub use hooks::{ pub mod schedule; mod storage; pub use storage::{ - Consideration, Footprint, Incrementable, Instance, LinearStoragePrice, PartialStorageInfoTrait, - StorageInfo, StorageInfoTrait, StorageInstance, TrackedStorageKey, WhitelistedStorageKeys, + Consideration, Footprint, Incrementable, Instance, LinearStoragePrice, MaybeConsideration, + PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, TrackedStorageKey, + WhitelistedStorageKeys, }; mod dispatch; diff --git a/substrate/frame/support/src/traits/storage.rs b/substrate/frame/support/src/traits/storage.rs index eb63ea59f6cb8..f88f199e6d967 100644 --- a/substrate/frame/support/src/traits/storage.rs +++ b/substrate/frame/support/src/traits/storage.rs @@ -257,6 +257,22 @@ impl Consideration for () { fn ensure_successful(_: &A, _: F) {} } +/// An extension of the [Consideration] trait that allows for the management of tickets that may +/// represent no cost. While a `MaybeConsideration` still requires proper handling, it introduces +/// the ability to determine if a ticket has no associated cost and can be forgotten without any +/// side effects. +pub trait MaybeConsideration: Consideration { + /// Returns `true` if this `Consideration` represents a "no-cost" ticket and can be forgotten + /// without any side effects. + fn is_none(&self) -> bool; +} + +impl MaybeConsideration for () { + fn is_none(&self) -> bool { + true + } +} + macro_rules! impl_incrementable { ($($type:ty),+) => { $( diff --git a/substrate/frame/support/src/traits/tokens/fungible/mod.rs b/substrate/frame/support/src/traits/tokens/fungible/mod.rs index 9b7c86971bb88..ad8e9f2cf1c96 100644 --- a/substrate/frame/support/src/traits/tokens/fungible/mod.rs +++ b/substrate/frame/support/src/traits/tokens/fungible/mod.rs @@ -188,7 +188,7 @@ pub use union_of::{NativeFromLeft, NativeOrWithId, UnionOf}; use crate::{ ensure, - traits::{Consideration, Footprint}, + traits::{Consideration, Footprint, MaybeConsideration}, }; /// Consideration method using a `fungible` balance frozen as the cost exacted for the footprint. @@ -242,6 +242,20 @@ impl< } } +impl< + A: 'static + Eq, + #[cfg(not(feature = "runtime-benchmarks"))] F: 'static + MutateFreeze, + #[cfg(feature = "runtime-benchmarks")] F: 'static + MutateFreeze + Mutate, + R: 'static + Get, + D: 'static + Convert, + Fp: 'static, + > MaybeConsideration for FreezeConsideration +{ + fn is_none(&self) -> bool { + self.0.is_zero() + } +} + /// Consideration method using a `fungible` balance frozen as the cost exacted for the footprint. #[derive( CloneNoBound, @@ -295,6 +309,19 @@ impl< let _ = F::mint_into(who, F::minimum_balance().saturating_add(D::convert(fp))); } } +impl< + A: 'static + Eq, + #[cfg(not(feature = "runtime-benchmarks"))] F: 'static + MutateHold, + #[cfg(feature = "runtime-benchmarks")] F: 'static + MutateHold + Mutate, + R: 'static + Get, + D: 'static + Convert, + Fp: 'static, + > MaybeConsideration for HoldConsideration +{ + fn is_none(&self) -> bool { + self.0.is_zero() + } +} /// Basic consideration method using a `fungible` balance frozen as the cost exacted for the /// footprint. From cc6495f7fa3e9de0e53efee845e700d2622ed400 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 16 Aug 2024 12:00:01 +0200 Subject: [PATCH 2/4] docs --- substrate/frame/support/src/traits/storage.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/substrate/frame/support/src/traits/storage.rs b/substrate/frame/support/src/traits/storage.rs index f88f199e6d967..15540d82ee32c 100644 --- a/substrate/frame/support/src/traits/storage.rs +++ b/substrate/frame/support/src/traits/storage.rs @@ -257,12 +257,12 @@ impl Consideration for () { fn ensure_successful(_: &A, _: F) {} } -/// An extension of the [Consideration] trait that allows for the management of tickets that may -/// represent no cost. While a `MaybeConsideration` still requires proper handling, it introduces -/// the ability to determine if a ticket has no associated cost and can be forgotten without any -/// side effects. +/// An extension of the [`Consideration`] trait that allows for the management of tickets that may +/// represent no cost. While the [`MaybeConsideration`] still requires proper handling, it +/// introduces the ability to determine if a ticket represents no cost and can be safely forgotten +/// without any side effects. pub trait MaybeConsideration: Consideration { - /// Returns `true` if this `Consideration` represents a "no-cost" ticket and can be forgotten + /// Returns `true` if this [`Consideration`] represents a no-cost ticket and can be forgotten /// without any side effects. fn is_none(&self) -> bool; } From b339a008fa00345bbc27f066c5abfa5459cfb040 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 16 Aug 2024 14:24:25 +0200 Subject: [PATCH 3/4] experimental --- substrate/frame/support/src/traits.rs | 7 ++++--- substrate/frame/support/src/traits/storage.rs | 2 ++ substrate/frame/support/src/traits/tokens/fungible/mod.rs | 7 +++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 5e572b18f09c2..f635ed32a1248 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -92,10 +92,11 @@ pub use hooks::{ pub mod schedule; mod storage; +#[cfg(feature = "experimental")] +pub use storage::MaybeConsideration; pub use storage::{ - Consideration, Footprint, Incrementable, Instance, LinearStoragePrice, MaybeConsideration, - PartialStorageInfoTrait, StorageInfo, StorageInfoTrait, StorageInstance, TrackedStorageKey, - WhitelistedStorageKeys, + Consideration, Footprint, Incrementable, Instance, LinearStoragePrice, PartialStorageInfoTrait, + StorageInfo, StorageInfoTrait, StorageInstance, TrackedStorageKey, WhitelistedStorageKeys, }; mod dispatch; diff --git a/substrate/frame/support/src/traits/storage.rs b/substrate/frame/support/src/traits/storage.rs index 15540d82ee32c..2b8e437073894 100644 --- a/substrate/frame/support/src/traits/storage.rs +++ b/substrate/frame/support/src/traits/storage.rs @@ -257,6 +257,7 @@ impl Consideration for () { fn ensure_successful(_: &A, _: F) {} } +#[cfg(feature = "experimental")] /// An extension of the [`Consideration`] trait that allows for the management of tickets that may /// represent no cost. While the [`MaybeConsideration`] still requires proper handling, it /// introduces the ability to determine if a ticket represents no cost and can be safely forgotten @@ -267,6 +268,7 @@ pub trait MaybeConsideration: Consideration bool; } +#[cfg(feature = "experimental")] impl MaybeConsideration for () { fn is_none(&self) -> bool { true diff --git a/substrate/frame/support/src/traits/tokens/fungible/mod.rs b/substrate/frame/support/src/traits/tokens/fungible/mod.rs index ad8e9f2cf1c96..c67755e133bf4 100644 --- a/substrate/frame/support/src/traits/tokens/fungible/mod.rs +++ b/substrate/frame/support/src/traits/tokens/fungible/mod.rs @@ -186,9 +186,11 @@ use sp_core::Get; use sp_runtime::{traits::Convert, DispatchError}; pub use union_of::{NativeFromLeft, NativeOrWithId, UnionOf}; +#[cfg(feature = "experimental")] +use crate::traits::MaybeConsideration; use crate::{ ensure, - traits::{Consideration, Footprint, MaybeConsideration}, + traits::{Consideration, Footprint}, }; /// Consideration method using a `fungible` balance frozen as the cost exacted for the footprint. @@ -241,7 +243,7 @@ impl< let _ = F::mint_into(who, F::minimum_balance().saturating_add(D::convert(fp))); } } - +#[cfg(feature = "experimental")] impl< A: 'static + Eq, #[cfg(not(feature = "runtime-benchmarks"))] F: 'static + MutateFreeze, @@ -309,6 +311,7 @@ impl< let _ = F::mint_into(who, F::minimum_balance().saturating_add(D::convert(fp))); } } +#[cfg(feature = "experimental")] impl< A: 'static + Eq, #[cfg(not(feature = "runtime-benchmarks"))] F: 'static + MutateHold, From 4d124e89d62b9f5a43dba589a5dfb640569cf87c Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 16 Aug 2024 14:44:35 +0200 Subject: [PATCH 4/4] prdoc --- prdoc/pr_5384.prdoc | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 prdoc/pr_5384.prdoc diff --git a/prdoc/pr_5384.prdoc b/prdoc/pr_5384.prdoc new file mode 100644 index 0000000000000..74d477f8e153d --- /dev/null +++ b/prdoc/pr_5384.prdoc @@ -0,0 +1,25 @@ +# 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: "`MaybeConsideration` extension trait for `Consideration`" + +doc: + - audience: Runtime Dev + description: | + The trait allows for the management of tickets that may represent no cost. While + the `MaybeConsideration` still requires proper handling, it introduces the ability + to determine if a ticket represents no cost and can be safely forgotten without any + side effects. + + The new trait is particularly useful when a consumer expects the cost to be zero under + certain conditions (e.g., when the proposal count is below a threshold N) and does not want + to store such consideration tickets in storage. The extension approach allows us to avoid + breaking changes to the existing trait and to continue using it as a non-optional version + for migrating pallets that utilize the `Currency` and `fungible` traits for `holds` and + `freezes`, without requiring any storage migration. + +crates: + - name: frame-support + bump: minor + - name: pallet-balances + bump: patch