Fixes and docs

substrate/frame/democracy/src/tests.rs

@@ -113,6 +113,7 @@ impl pallet_preimage::Config for Test {
 	type ManagerOrigin = EnsureRoot<u64>;
 	type BaseDeposit = ConstU64<0>;
 	type ByteDeposit = ConstU64<0>;
+	type Consideration = ();
 }
 
 impl pallet_scheduler::Config for Test {

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

@@ -152,21 +152,46 @@ impl Footprint {
 	}
 }
 
+/// Some sort of cost taken from account temporarily in order to offset the cost to the chain of
+/// holding some data `Footprint` in state.
+///
+/// The cost may be increased, reduced or dropped entirely as the footprint changes.
 pub trait Consideration<AccountId> {
+	/// A single ticket corresponding to some particular datum held in storage. This is an opaque
+	/// type, but must itself be stored and generally it should be placed alongside whatever data
+	/// the ticket was created for.
+	///
+	/// While not technically a linear type owing to the need for `FullCodec`, *this should be
+	/// treated as one*. Don't type to duplicate it, and remember to drop it when you're done with
+	/// it.
 	type Ticket: Member + FullCodec + TypeInfo + MaxEncodedLen + Default;
 
+	/// Optionally consume an old ticket and alter the footprint, enforcing the new cost to `who`
+	/// and returning the new ticket (or an error if there was an issue).
+	///
+	/// For creating tickets and dropping them, you can use the simpler `new` and `drop` instead.
 	fn update(
 		who: &AccountId,
 		old: Option<Self::Ticket>,
 		new: Option<Footprint>,
 	) -> Result<Self::Ticket, DispatchError>;
 
+	/// Create a ticket for the `new` footprint attributable to `who`. This ticket *must* be
+	/// consumed (through either `drop` or `update`) if the footprint changes or is removed.
 	fn new(who: &AccountId, new: Footprint) -> Result<Self::Ticket, DispatchError> {
 		Self::update(who, None, Some(new))
 	}
 
-	fn drop(who: &AccountId, old: Self::Ticket) -> Result<Self::Ticket, DispatchError> {
-		Self::update(who, Some(old), None)
+	/// Consume a ticket for some `old` footprint attributable to `who` which has now been freed.
+	fn drop(who: &AccountId, old: Self::Ticket) -> Result<(), DispatchError> {
+		Self::update(who, Some(old), None).map(|_| ())
+	}
+}
+
+impl<A> Consideration<A> for () {
+	type Ticket = ();
+	fn update(_: &A, _: Option<()>, _: Option<Footprint>) -> Result<(), DispatchError> {
+		Ok(())
 	}
 }
 

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

@@ -56,10 +56,14 @@ pub use item_of::ItemOf;
 pub use regular::{
 	Balanced, DecreaseIssuance, Dust, IncreaseIssuance, Inspect, Mutate, Unbalanced,
 };
+use sp_arithmetic::traits::Zero;
 use sp_core::Get;
-use sp_runtime::traits::Convert;
+use sp_runtime::{traits::Convert, DispatchError};
 
-use crate::traits::{Consideration, Footprint};
+use crate::{
+	ensure,
+	traits::{Consideration, Footprint},
+};
 
 /// Consideration method using a `fungible` balance frozen as the cost exacted for the footprint.
 ///
@@ -150,7 +154,8 @@ impl<A, F: MutateFreeze<A>, R: Get<F::Id>, D: Convert<Footprint, F::Balance>> Co
 		who: &A,
 		_old: Option<Self::Ticket>,
 		new: Option<Footprint>,
-	) -> Result<Self::Ticket, sp_runtime::DispatchError> {
+	) -> Result<Self::Ticket, DispatchError> {
+		ensure!(F::balance_frozen(&R::get(), who).is_zero(), DispatchError::Unavailable);
 		match new {
 			Some(footprint) => F::set_freeze(&R::get(), who, D::convert(footprint)),
 			None => F::thaw(&R::get(), who),
@@ -175,6 +180,7 @@ impl<A, F: MutateHold<A>, R: Get<F::Reason>, D: Convert<Footprint, F::Balance>>
 		_old: Option<Self::Ticket>,
 		new: Option<Footprint>,
 	) -> Result<Self::Ticket, sp_runtime::DispatchError> {
+		ensure!(F::balance_on_hold(&R::get(), who).is_zero(), DispatchError::Unavailable);
 		match new {
 			Some(footprint) => F::set_on_hold(&R::get(), who, D::convert(footprint)),
 			None => F::release_all(&R::get(), who, super::Precision::BestEffort).map(|_| ()),

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

@@ -17,10 +17,13 @@
 
 //! The traits for putting freezes within a single fungible token class.
 
+use crate::{ensure, traits::tokens::Fortitude};
 use scale_info::TypeInfo;
-use sp_arithmetic::{ArithmeticError, traits::{CheckedAdd, CheckedSub}};
+use sp_arithmetic::{
+	traits::{CheckedAdd, CheckedSub},
+	ArithmeticError,
+};
 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
@@ -90,7 +93,10 @@ pub trait Mutate<AccountId>: Inspect<AccountId> {
 		fortitude: Fortitude,
 	) -> DispatchResult {
 		let force = fortitude == Fortitude::Force;
-		ensure!(force || Self::balance_freezable(asset.clone(), who) >= amount, TokenError::FundsUnavailable);
+		ensure!(
+			force || Self::balance_freezable(asset.clone(), who) >= amount,
+			TokenError::FundsUnavailable
+		);
 		Self::set_freeze(asset, id, who, amount)
 	}
 
@@ -107,7 +113,10 @@ pub trait Mutate<AccountId>: Inspect<AccountId> {
 		fortitude: Fortitude,
 	) -> DispatchResult {
 		let force = fortitude == Fortitude::Force;
-		ensure!(force || Self::balance_freezable(asset.clone(), who) >= amount, TokenError::FundsUnavailable);
+		ensure!(
+			force || Self::balance_freezable(asset.clone(), who) >= amount,
+			TokenError::FundsUnavailable
+		);
 		Self::extend_freeze(asset, id, who, amount)
 	}