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

MaybeConsideration extension trait for Consideration #5384

Merged
merged 5 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 25 additions & 0 deletions prdoc/pr_5384.prdoc
Original file line number Diff line number Diff line change
@@ -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
12 changes: 9 additions & 3 deletions substrate/frame/balances/src/tests/fungible_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions substrate/frame/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ pub use hooks::{

pub mod schedule;
mod storage;
#[cfg(feature = "experimental")]
pub use storage::MaybeConsideration;
pub use storage::{
Consideration, Footprint, Incrementable, Instance, LinearStoragePrice, PartialStorageInfoTrait,
StorageInfo, StorageInfoTrait, StorageInstance, TrackedStorageKey, WhitelistedStorageKeys,
Expand Down
18 changes: 18 additions & 0 deletions substrate/frame/support/src/traits/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,24 @@ impl<A, F> Consideration<A, F> 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
/// without any side effects.
pub trait MaybeConsideration<AccountId, Footprint>: Consideration<AccountId, Footprint> {
/// Returns `true` if this [`Consideration`] represents a no-cost ticket and can be forgotten
/// without any side effects.
fn is_none(&self) -> bool;
}

#[cfg(feature = "experimental")]
impl<A, F> MaybeConsideration<A, F> for () {
fn is_none(&self) -> bool {
true
}
}

macro_rules! impl_incrementable {
($($type:ty),+) => {
$(
Expand Down
30 changes: 30 additions & 0 deletions substrate/frame/support/src/traits/tokens/fungible/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ 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},
Expand Down Expand Up @@ -241,6 +243,20 @@ 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<A>,
#[cfg(feature = "runtime-benchmarks")] F: 'static + MutateFreeze<A> + Mutate<A>,
R: 'static + Get<F::Id>,
D: 'static + Convert<Fp, F::Balance>,
Fp: 'static,
> MaybeConsideration<A, Fp> for FreezeConsideration<A, F, R, D, Fp>
{
fn is_none(&self) -> bool {
self.0.is_zero()
}
}

/// Consideration method using a `fungible` balance frozen as the cost exacted for the footprint.
#[derive(
Expand Down Expand Up @@ -295,6 +311,20 @@ 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<A>,
#[cfg(feature = "runtime-benchmarks")] F: 'static + MutateHold<A> + Mutate<A>,
R: 'static + Get<F::Reason>,
D: 'static + Convert<Fp, F::Balance>,
Fp: 'static,
> MaybeConsideration<A, Fp> for HoldConsideration<A, F, R, D, Fp>
{
fn is_none(&self) -> bool {
self.0.is_zero()
}
}

/// Basic consideration method using a `fungible` balance frozen as the cost exacted for the
/// footprint.
Expand Down
Loading