Docs and fixes

substrate/frame/preimage/src/lib.rs

@@ -120,6 +120,7 @@ pub mod pallet {
 		/// The per-byte deposit for placing a preimage on chain.
 		type ByteDeposit: Get<BalanceOf<Self>>;
 
+		/// A means of providing some cost while data is stored on-chain.
 		type Consideration: Consideration<Self::AccountId>;
 	}
 

substrate/frame/preimage/src/mock.rs

@@ -22,7 +22,7 @@ use super::*;
 use crate as pallet_preimage;
 use frame_support::{
 	ord_parameter_types,
-	traits::{fungible::FreezeMultiConsideration, ConstU32, ConstU64, Everything},
+	traits::{fungible::HoldConsideration, ConstU32, ConstU64, Everything},
 	weights::constants::RocksDbWeight,
 };
 use frame_system::EnsureSignedBy;
@@ -82,7 +82,7 @@ impl pallet_balances::Config for Test {
 	type FreezeIdentifier = ();
 	type MaxFreezes = ConstU32<1>;
 	type RuntimeHoldReason = ();
-	type MaxHolds = ();
+	type MaxHolds = ConstU32<2>;
 }
 
 ord_parameter_types! {
@@ -103,7 +103,7 @@ impl Config for Test {
 	type ManagerOrigin = EnsureSignedBy<One, u64>;
 	type BaseDeposit = ConstU64<2>;
 	type ByteDeposit = ConstU64<1>;
-	type Consideration = FreezeMultiConsideration<u64, Balances, (), ConvertDeposit>;
+	type Consideration = HoldConsideration<u64, Balances, (), ConvertDeposit>;
 }
 
 pub fn new_test_ext() -> sp_io::TestExternalities {

substrate/frame/preimage/src/tests.rs

@@ -24,7 +24,7 @@ use crate::mock::*;
 
 use frame_support::{
 	assert_err, assert_noop, assert_ok, assert_storage_noop,
-	traits::{fungible::InspectFreeze, Bounded, BoundedInline, Hash as PreimageHash},
+	traits::{fungible::InspectHold, Bounded, BoundedInline, Hash as PreimageHash},
 	StorageNoopGuard,
 };
 use sp_core::{blake2_256, H256};
@@ -51,8 +51,8 @@ pub fn make_bounded_values() -> (Bounded<Vec<u8>>, Bounded<Vec<u8>>, Bounded<Vec
 fn user_note_preimage_works() {
 	new_test_ext().execute_with(|| {
 		assert_ok!(Preimage::note_preimage(RuntimeOrigin::signed(2), vec![1]));
-		assert_eq!(Balances::balance_frozen(&(), &2), 3);
-		assert_eq!(Balances::free_balance(2), 100);
+		assert_eq!(Balances::balance_on_hold(&(), &2), 3);
+		assert_eq!(Balances::free_balance(2), 97);
 
 		let h = hashed([1]);
 		assert!(Preimage::have_preimage(&h));
@@ -248,14 +248,14 @@ fn unrequest_preimage_works() {
 fn user_noted_then_requested_preimage_is_refunded_once_only() {
 	new_test_ext().execute_with(|| {
 		assert_ok!(Preimage::note_preimage(RuntimeOrigin::signed(2), vec![1; 3]));
-		assert_eq!(Balances::balance_frozen(&(), &2), 5);
+		assert_eq!(Balances::balance_on_hold(&(), &2), 5);
 		assert_ok!(Preimage::note_preimage(RuntimeOrigin::signed(2), vec![1]));
-		assert_eq!(Balances::balance_frozen(&(), &2), 8);
+		assert_eq!(Balances::balance_on_hold(&(), &2), 8);
 		assert_ok!(Preimage::request_preimage(RuntimeOrigin::signed(1), hashed([1])));
 		assert_ok!(Preimage::unrequest_preimage(RuntimeOrigin::signed(1), hashed([1])));
 		assert_ok!(Preimage::unnote_preimage(RuntimeOrigin::signed(2), hashed([1])));
 		// Still have freeze from `vec[1; 3]`.
-		assert_eq!(Balances::balance_frozen(&(), &2), 5);
+		assert_eq!(Balances::balance_on_hold(&(), &2), 5);
 	});
 }
 

substrate/frame/support/src/traits/storage.rs

@@ -170,9 +170,6 @@ pub trait Consideration<AccountId> {
 	}
 }
 
-#[test]
-fn it_builds() {}
-
 macro_rules! impl_incrementable {
 	($($type:ty),+) => {
 		$(

substrate/frame/support/src/traits/tokens/fungible/freeze.rs

@@ -24,7 +24,7 @@ use sp_arithmetic::{
 };
 use sp_runtime::{DispatchResult, TokenError};
 
-use crate::ensure;
+use crate::{ensure, traits::tokens::Fortitude};
 
 /// Trait for inspecting a fungible asset which can be frozen. Freezing is essentially setting a
 /// minimum balance bellow which the total balance (inclusive of any funds placed on hold) may not
@@ -72,20 +72,52 @@ pub trait Mutate<AccountId>: Inspect<AccountId> {
 	/// Remove an existing lock.
 	fn thaw(id: &Self::Id, who: &AccountId) -> DispatchResult;
 
-	/// Reduce an existing lock.
+	/// Attempt to alter the amount frozen under the given `id` to `amount`.
+	///
+	/// Fail if the account of `who` has fewer freezable funds than `amount`, unless `fortitude` is
+	/// `Fortitude::Force`.
+	fn set_frozen(
+		id: &Self::Id,
+		who: &AccountId,
+		amount: Self::Balance,
+		fortitude: Fortitude,
+	) -> DispatchResult {
+		let force = fortitude == Fortitude::Force;
+		ensure!(force || Self::balance_freezable(who) >= amount, TokenError::FundsUnavailable);
+		Self::set_freeze(id, who, amount)
+	}
+
+	/// Attempt to set the amount frozen under the given `id` to `amount`, iff this would increase
+	/// the amount frozen under `id`. Do nothing otherwise.
+	///
+	/// Fail if the account of `who` has fewer freezable funds than `amount`, unless `fortitude` is
+	/// `Fortitude::Force`.
+	fn ensure_frozen(
+		id: &Self::Id,
+		who: &AccountId,
+		amount: Self::Balance,
+		fortitude: Fortitude,
+	) -> DispatchResult {
+		let force = fortitude == Fortitude::Force;
+		ensure!(force || Self::balance_freezable(who) >= amount, TokenError::FundsUnavailable);
+		Self::extend_freeze(id, who, amount)
+	}
+
+	/// Decrease the amount which is being frozen for a particular lock, failing in the case of
+	/// underflow.
 	fn decrease_frozen(id: &Self::Id, who: &AccountId, amount: Self::Balance) -> DispatchResult {
 		let a = Self::balance_frozen(id, who)
 			.checked_sub(&amount)
 			.ok_or(ArithmeticError::Underflow)?;
 		Self::set_freeze(id, who, a)
 	}
 
-	/// Reduce an existing lock.
+	/// Increase the amount which is being frozen for a particular lock, failing in the case that
+	/// too little balance is available for being frozen.
 	fn increase_frozen(id: &Self::Id, who: &AccountId, amount: Self::Balance) -> DispatchResult {
 		let a = Self::balance_frozen(id, who)
 			.checked_add(&amount)
 			.ok_or(ArithmeticError::Overflow)?;
-		ensure!(Self::balance_freezable(who) >= a, TokenError::FundsUnavailable);
-		Self::set_freeze(id, who, a)
+		Self::set_frozen(id, who, a, Fortitude::Polite)
 	}
 }

substrate/frame/support/src/traits/tokens/fungible/hold.rs

@@ -92,11 +92,11 @@ pub trait Inspect<AccountId>: super::Inspect<AccountId> {
 		who: &AccountId,
 		amount: Self::Balance,
 	) -> DispatchResult {
-		ensure!(Self::hold_available(reason, who), TokenError::CannotCreateHold);
 		ensure!(
 			amount <= Self::reducible_balance(who, Protect, Force),
 			TokenError::FundsUnavailable
 		);
+		ensure!(Self::hold_available(reason, who), TokenError::CannotCreateHold);
 		Ok(())
 	}
 

substrate/frame/support/src/traits/tokens/fungible/mod.rs

@@ -45,6 +45,7 @@ mod imbalance;
 mod item_of;
 mod regular;
 
+use super::Precision::BestEffort;
 pub use freeze::{Inspect as InspectFreeze, Mutate as MutateFreeze};
 pub use hold::{
 	Balanced as BalancedHold, Inspect as InspectHold, Mutate as MutateHold,
@@ -60,26 +61,48 @@ use sp_runtime::traits::Convert;
 
 use crate::traits::{Consideration, Footprint};
 
+/// Consideration method using a `fungible` balance frozen as the cost exacted for the footprint.
+///
+/// The aggregate amount frozen under `R::get()` for any account which has multiple tickets,
+/// is the *cumulative* amounts of each ticket's footprint (each individually determined by `D`).
 pub struct FreezeConsideration<A, F, R, D>(sp_std::marker::PhantomData<(A, F, R, D)>);
 impl<A, F: MutateFreeze<A>, R: Get<F::Id>, D: Convert<Footprint, F::Balance>> Consideration<A>
 	for FreezeConsideration<A, F, R, D>
 {
-	type Ticket = ();
+	type Ticket = F::Balance;
 	fn update(
 		who: &A,
-		_old: Option<Self::Ticket>,
+		old: Option<Self::Ticket>,
 		new: Option<Footprint>,
 	) -> Result<Self::Ticket, sp_runtime::DispatchError> {
-		match new {
-			Some(footprint) => F::set_freeze(&R::get(), who, D::convert(footprint)),
-			None => F::thaw(&R::get(), who),
+		match (old, new) {
+			(None, Some(footprint)) => {
+				let new = D::convert(footprint);
+				F::increase_frozen(&R::get(), who, new)?;
+				Ok(new)
+			},
+			(Some(old), Some(footprint)) => {
+				let new = D::convert(footprint);
+				if old > new {
+					F::decrease_frozen(&R::get(), who, old - new)?;
+				} else if new > old {
+					F::increase_frozen(&R::get(), who, new - old)?;
+				}
+				Ok(new)
+			},
+			(Some(old), None) => {
+				F::decrease_frozen(&R::get(), who, old)?;
+				Ok(Default::default())
+			},
+			(None, None) => Ok(Default::default()),
 		}
 	}
 }
 
-pub struct FreezeMultiConsideration<A, F, R, D>(sp_std::marker::PhantomData<(A, F, R, D)>);
-impl<A, F: MutateFreeze<A>, R: Get<F::Id>, D: Convert<Footprint, F::Balance>> Consideration<A>
-	for FreezeMultiConsideration<A, F, R, D>
+/// Consideration method using a `fungible` balance frozen as the cost exacted for the footprint.
+pub struct HoldConsideration<A, F, R, D>(sp_std::marker::PhantomData<(A, F, R, D)>);
+impl<A, F: MutateHold<A>, R: Get<F::Reason>, D: Convert<Footprint, F::Balance>> Consideration<A>
+	for HoldConsideration<A, F, R, D>
 {
 	type Ticket = F::Balance;
 	fn update(
@@ -90,30 +113,61 @@ impl<A, F: MutateFreeze<A>, R: Get<F::Id>, D: Convert<Footprint, F::Balance>> Co
 		match (old, new) {
 			(None, Some(footprint)) => {
 				let new = D::convert(footprint);
-				F::increase_frozen(&R::get(), who, new)?;
+				F::hold(&R::get(), who, new)?;
 				Ok(new)
 			},
 			(Some(old), Some(footprint)) => {
 				let new = D::convert(footprint);
 				if old > new {
-					F::decrease_frozen(&R::get(), who, old - new)?;
+					F::release(&R::get(), who, old - new, BestEffort)?;
 				} else if new > old {
-					F::extend_freeze(&R::get(), who, new - old)?;
+					F::hold(&R::get(), who, new - old)?;
 				}
 				Ok(new)
 			},
 			(Some(old), None) => {
-				F::decrease_frozen(&R::get(), who, old)?;
+				F::release(&R::get(), who, old, BestEffort)?;
 				Ok(Default::default())
 			},
 			(None, None) => Ok(Default::default()),
 		}
 	}
 }
 
-pub struct HoldConsideration<A, F, R, D>(sp_std::marker::PhantomData<(A, F, R, D)>);
+/// Basic consideration method using a `fungible` balance frozen as the cost exacted for the
+/// footprint.
+///
+/// NOTE: This is an optimized implementation, which can only be used for systems where each
+/// account has only a single active ticket associated with it since individual tickets do not
+/// track the specific balance which is frozen. If you are uncertain then use `FreezeConsideration`
+/// instead, since this works in all circumstances.
+pub struct SingletonFreezeConsideration<A, F, R, D>(sp_std::marker::PhantomData<(A, F, R, D)>);
+impl<A, F: MutateFreeze<A>, R: Get<F::Id>, D: Convert<Footprint, F::Balance>> Consideration<A>
+	for SingletonFreezeConsideration<A, F, R, D>
+{
+	type Ticket = ();
+	fn update(
+		who: &A,
+		_old: Option<Self::Ticket>,
+		new: Option<Footprint>,
+	) -> Result<Self::Ticket, sp_runtime::DispatchError> {
+		match new {
+			Some(footprint) => F::set_freeze(&R::get(), who, D::convert(footprint)),
+			None => F::thaw(&R::get(), who),
+		}
+	}
+}
+
+/// Basic consideration method using a `fungible` balance placed on hold as the cost exacted for the
+/// footprint.
+///
+/// NOTE: This is an optimized implementation, which can only be used for systems where each
+/// account has only a single active ticket associated with it since individual tickets do not
+/// track the specific balance which is frozen. If you are uncertain then use `FreezeConsideration`
+/// instead, since this works in all circumstances.
+pub struct SingletonHoldConsideration<A, F, R, D>(sp_std::marker::PhantomData<(A, F, R, D)>);
 impl<A, F: MutateHold<A>, R: Get<F::Reason>, D: Convert<Footprint, F::Balance>> Consideration<A>
-	for HoldConsideration<A, F, R, D>
+	for SingletonHoldConsideration<A, F, R, D>
 {
 	type Ticket = ();
 	fn update(

substrate/frame/support/src/traits/tokens/fungibles/freeze.rs

@@ -18,7 +18,9 @@
 //! The traits for putting freezes within a single fungible token class.
 
 use scale_info::TypeInfo;
-use sp_runtime::DispatchResult;
+use sp_arithmetic::{ArithmeticError, traits::{CheckedAdd, CheckedSub}};
+use sp_runtime::{DispatchResult, TokenError};
+use crate::{ensure, traits::tokens::Fortitude};
 
 /// Trait for inspecting a fungible asset which can be frozen. Freezing is essentially setting a
 /// minimum balance below which the total balance (inclusive of any funds placed on hold) may not
@@ -75,4 +77,65 @@ pub trait Mutate<AccountId>: Inspect<AccountId> {
 
 	/// Remove an existing lock.
 	fn thaw(asset: Self::AssetId, id: &Self::Id, who: &AccountId) -> DispatchResult;
+
+	/// Attempt to alter the amount frozen under the given `id` to `amount`.
+	///
+	/// Fail if the account of `who` has fewer freezable funds than `amount`, unless `fortitude` is
+	/// `Fortitude::Force`.
+	fn set_frozen(
+		asset: Self::AssetId,
+		id: &Self::Id,
+		who: &AccountId,
+		amount: Self::Balance,
+		fortitude: Fortitude,
+	) -> DispatchResult {
+		let force = fortitude == Fortitude::Force;
+		ensure!(force || Self::balance_freezable(asset.clone(), who) >= amount, TokenError::FundsUnavailable);
+		Self::set_freeze(asset, id, who, amount)
+	}
+
+	/// Attempt to set the amount frozen under the given `id` to `amount`, iff this would increase
+	/// the amount frozen under `id`. Do nothing otherwise.
+	///
+	/// Fail if the account of `who` has fewer freezable funds than `amount`, unless `fortitude` is
+	/// `Fortitude::Force`.
+	fn ensure_frozen(
+		asset: Self::AssetId,
+		id: &Self::Id,
+		who: &AccountId,
+		amount: Self::Balance,
+		fortitude: Fortitude,
+	) -> DispatchResult {
+		let force = fortitude == Fortitude::Force;
+		ensure!(force || Self::balance_freezable(asset.clone(), who) >= amount, TokenError::FundsUnavailable);
+		Self::extend_freeze(asset, id, who, amount)
+	}
+
+	/// Decrease the amount which is being frozen for a particular lock, failing in the case of
+	/// underflow.
+	fn decrease_frozen(
+		asset: Self::AssetId,
+		id: &Self::Id,
+		who: &AccountId,
+		amount: Self::Balance,
+	) -> DispatchResult {
+		let a = Self::balance_frozen(asset.clone(), id, who)
+			.checked_sub(&amount)
+			.ok_or(ArithmeticError::Underflow)?;
+		Self::set_frozen(asset, id, who, a, Fortitude::Polite)
+	}
+
+	/// Increase the amount which is being frozen for a particular lock, failing in the case that
+	/// too little balance is available for being frozen.
+	fn increase_frozen(
+		asset: Self::AssetId,
+		id: &Self::Id,
+		who: &AccountId,
+		amount: Self::Balance,
+	) -> DispatchResult {
+		let a = Self::balance_frozen(asset.clone(), id, who)
+			.checked_add(&amount)
+			.ok_or(ArithmeticError::Overflow)?;
+		Self::set_frozen(asset, id, who, a, Fortitude::Polite)
+	}
 }