Frame: Consideration trait generic over Footprint and indicates zero cost

prdoc/pr_4596.prdoc

@@ -0,0 +1,16 @@
+title: "Frame: `Consideration` trait generic over `Footprint` and handles zero cost"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      `Consideration` trait generic over `Footprint` and can handle zero cost for a give footprint.
+
+      `Consideration` trait is generic over `Footprint` (currently defined over the type with the same name). This makes it possible to setup a custom footprint (e.g. current number of proposals in the storage).
+
+      `Consideration::new` and `Consideration::update` return an `Option<Self>` instead `Self`, this make it possible to define no cost for a specific footprint (e.g. current number of proposals in the storage < max_proposal_count / 2).
+
+crates:
+  - name: frame-support
+    bump: major
+  - name: pallet-preimage
+    bump: major

substrate/frame/preimage/src/benchmarking.rs

@@ -116,7 +116,7 @@ benchmarks! {
 		T::ManagerOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?,
 		hash
 	) verify {
-		let ticket = TicketOf::<T>::new(&noter, Footprint { count: 1, size: MAX_SIZE as u64 }).unwrap();
+		let ticket = TicketOf::<T>::new(&noter, Footprint { count: 1, size: MAX_SIZE as u64 }).unwrap().unwrap();
 		let s = RequestStatus::Requested { maybe_ticket: Some((noter, ticket)), count: 1, maybe_len: Some(MAX_SIZE) };
 		assert_eq!(RequestStatusFor::<T>::get(&hash), Some(s));
 	}

substrate/frame/preimage/src/lib.rs

@@ -122,7 +122,9 @@ pub mod pallet {
 		type ManagerOrigin: EnsureOrigin<Self::RuntimeOrigin>;
 
 		/// A means of providing some cost while data is stored on-chain.
-		type Consideration: Consideration<Self::AccountId>;
+		///
+		/// Should never return a `None`, implying no cost for a non-empty preimage.
+		type Consideration: Consideration<Self::AccountId, Footprint>;
 	}
 
 	#[pallet::pallet]
@@ -158,6 +160,8 @@ pub mod pallet {
 		TooMany,
 		/// Too few hashes were requested to be upgraded (i.e. zero).
 		TooFew,
+		/// No ticket with a cost was returned by [`Config::Consideration`] to store the preimage.
+		NoCost,
 	}
 
 	/// A reason for this pallet placing a hold on funds.
@@ -268,10 +272,10 @@ impl<T: Config> Pallet<T> {
 				// unreserve deposit
 				T::Currency::unreserve(&who, amount);
 				// take consideration
-				let Ok(ticket) =
+				let Ok(Some(ticket)) =
 					T::Consideration::new(&who, Footprint::from_parts(1, len as usize))
-						.defensive_proof("Unexpected inability to take deposit after unreserved")
 				else {
+					defensive!("None ticket or inability to take deposit after unreserved");
 					return true
 				};
 				RequestStatus::Unrequested { ticket: (who, ticket), len }
@@ -282,12 +286,10 @@ impl<T: Config> Pallet<T> {
 					T::Currency::unreserve(&who, deposit);
 					// take consideration
 					if let Some(len) = maybe_len {
-						let Ok(ticket) =
+						let Ok(Some(ticket)) =
 							T::Consideration::new(&who, Footprint::from_parts(1, len as usize))
-								.defensive_proof(
-									"Unexpected inability to take deposit after unreserved",
-								)
 						else {
+							defensive!("None ticket or inability to take deposit after unreserved");
 							return true
 						};
 						Some((who, ticket))
@@ -347,7 +349,8 @@ impl<T: Config> Pallet<T> {
 				RequestStatus::Requested { maybe_ticket: None, count: 1, maybe_len: Some(len) },
 			(None, Some(depositor)) => {
 				let ticket =
-					T::Consideration::new(depositor, Footprint::from_parts(1, len as usize))?;
+					T::Consideration::new(depositor, Footprint::from_parts(1, len as usize))?
+						.ok_or(Error::<T>::NoCost)?;
 				RequestStatus::Unrequested { ticket: (depositor.clone(), ticket), len }
 			},
 		};

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

@@ -194,7 +194,7 @@ where
 }
 
 /// Some sort of cost taken from account temporarily in order to offset the cost to the chain of
-/// holding some data [`Footprint`] in state.
+/// holding some data `Footprint` (e.g. [`Footprint`]) in state.
 ///
 /// The cost may be increased, reduced or dropped entirely as the footprint changes.
 ///
@@ -206,16 +206,20 @@ where
 /// treated as one*. Don't type to duplicate it, and remember to drop it when you're done with
 /// it.
 #[must_use]
-pub trait Consideration<AccountId>: Member + FullCodec + TypeInfo + MaxEncodedLen {
+pub trait Consideration<AccountId, Footprint>:
+	Member + FullCodec + TypeInfo + MaxEncodedLen
+{
 	/// Create a ticket for the `new` footprint attributable to `who`. This ticket *must* ultimately
-	/// be consumed through `update` or `drop` once the footprint changes or is removed.
-	fn new(who: &AccountId, new: Footprint) -> Result<Self, DispatchError>;
+	/// be consumed through `update` or `drop` once the footprint changes or is removed. `None`
+	/// implies no cost for a given footprint.
+	fn new(who: &AccountId, new: Footprint) -> Result<Option<Self>, DispatchError>;
 
 	/// 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).
+	/// and returning the new ticket (or an error if there was an issue). `None` implies no cost for
+	/// a given footprint.
 	///
 	/// For creating tickets and dropping them, you can use the simpler `new` and `drop` instead.
-	fn update(self, who: &AccountId, new: Footprint) -> Result<Self, DispatchError>;
+	fn update(self, who: &AccountId, new: Footprint) -> Result<Option<Self>, DispatchError>;
 
 	/// Consume a ticket for some `old` footprint attributable to `who` which should now been freed.
 	fn drop(self, who: &AccountId) -> Result<(), DispatchError>;
@@ -230,12 +234,12 @@ pub trait Consideration<AccountId>: Member + FullCodec + TypeInfo + MaxEncodedLe
 	}
 }
 
-impl<A> Consideration<A> for () {
-	fn new(_: &A, _: Footprint) -> Result<Self, DispatchError> {
-		Ok(())
+impl<A, F> Consideration<A, F> for () {
+	fn new(_: &A, _: F) -> Result<Option<Self>, DispatchError> {
+		Ok(Some(()))
 	}
-	fn update(self, _: &A, _: Footprint) -> Result<(), DispatchError> {
-		Ok(())
+	fn update(self, _: &A, _: F) -> Result<Option<Self>, DispatchError> {
+		Ok(Some(()))
 	}
 	fn drop(self, _: &A) -> Result<(), DispatchError> {
 		Ok(())

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

@@ -198,31 +198,40 @@ use crate::{
 	MaxEncodedLen,
 	RuntimeDebugNoBound,
 )]
-#[scale_info(skip_type_params(A, F, R, D))]
+#[scale_info(skip_type_params(A, F, R, D, Fp))]
 #[codec(mel_bound())]
-pub struct FreezeConsideration<A, F, R, D>(F::Balance, PhantomData<fn() -> (A, R, D)>)
+pub struct FreezeConsideration<A, F, R, D, Fp>(F::Balance, PhantomData<fn() -> (A, R, D, Fp)>)
 where
 	F: MutateFreeze<A>;
 impl<
 		A: 'static,
 		F: 'static + MutateFreeze<A>,
 		R: 'static + Get<F::Id>,
-		D: 'static + Convert<Footprint, F::Balance>,
-	> Consideration<A> for FreezeConsideration<A, F, R, D>
+		D: 'static + Convert<Fp, F::Balance>,
+		Fp: 'static,
+	> Consideration<A, Fp> for FreezeConsideration<A, F, R, D, Fp>
 {
-	fn new(who: &A, footprint: Footprint) -> Result<Self, DispatchError> {
+	fn new(who: &A, footprint: Fp) -> Result<Option<Self>, DispatchError> {
 		let new = D::convert(footprint);
-		F::increase_frozen(&R::get(), who, new)?;
-		Ok(Self(new, PhantomData))
+		if new.is_zero() {
+			Ok(None)
+		} else {
+			F::increase_frozen(&R::get(), who, new)?;
+			Ok(Some(Self(new, PhantomData)))
+		}
 	}
-	fn update(self, who: &A, footprint: Footprint) -> Result<Self, DispatchError> {
+	fn update(self, who: &A, footprint: Fp) -> Result<Option<Self>, DispatchError> {
 		let new = D::convert(footprint);
 		if self.0 > new {
 			F::decrease_frozen(&R::get(), who, self.0 - new)?;
 		} else if new > self.0 {
 			F::increase_frozen(&R::get(), who, new - self.0)?;
 		}
-		Ok(Self(new, PhantomData))
+		if new.is_zero() {
+			Ok(None)
+		} else {
+			Ok(Some(Self(new, PhantomData)))
+		}
 	}
 	fn drop(self, who: &A) -> Result<(), DispatchError> {
 		F::decrease_frozen(&R::get(), who, self.0).map(|_| ())
@@ -240,31 +249,43 @@ impl<
 	MaxEncodedLen,
 	RuntimeDebugNoBound,
 )]
-#[scale_info(skip_type_params(A, F, R, D))]
+#[scale_info(skip_type_params(A, F, R, D, Fp))]
 #[codec(mel_bound())]
-pub struct HoldConsideration<A, F, R, D>(F::Balance, PhantomData<fn() -> (A, R, D)>)
+pub struct HoldConsideration<A, F, R, D, Fp = Footprint>(
+	F::Balance,
+	PhantomData<fn() -> (A, R, D, Fp)>,
+)
 where
 	F: MutateHold<A>;
 impl<
 		A: 'static,
 		F: 'static + MutateHold<A>,
 		R: 'static + Get<F::Reason>,
-		D: 'static + Convert<Footprint, F::Balance>,
-	> Consideration<A> for HoldConsideration<A, F, R, D>
+		D: 'static + Convert<Fp, F::Balance>,
+		Fp: 'static,
+	> Consideration<A, Fp> for HoldConsideration<A, F, R, D, Fp>
 {
-	fn new(who: &A, footprint: Footprint) -> Result<Self, DispatchError> {
+	fn new(who: &A, footprint: Fp) -> Result<Option<Self>, DispatchError> {
 		let new = D::convert(footprint);
-		F::hold(&R::get(), who, new)?;
-		Ok(Self(new, PhantomData))
+		if new.is_zero() {
+			Ok(None)
+		} else {
+			F::hold(&R::get(), who, new)?;
+			Ok(Some(Self(new, PhantomData)))
+		}
 	}
-	fn update(self, who: &A, footprint: Footprint) -> Result<Self, DispatchError> {
+	fn update(self, who: &A, footprint: Fp) -> Result<Option<Self>, DispatchError> {
 		let new = D::convert(footprint);
 		if self.0 > new {
 			F::release(&R::get(), who, self.0 - new, BestEffort)?;
 		} else if new > self.0 {
 			F::hold(&R::get(), who, new - self.0)?;
 		}
-		Ok(Self(new, PhantomData))
+		if new.is_zero() {
+			Ok(None)
+		} else {
+			Ok(Some(Self(new, PhantomData)))
+		}
 	}
 	fn drop(self, who: &A) -> Result<(), DispatchError> {
 		F::release(&R::get(), who, self.0, BestEffort).map(|_| ())
@@ -291,22 +312,34 @@ impl<
 	MaxEncodedLen,
 	RuntimeDebugNoBound,
 )]
-#[scale_info(skip_type_params(A, Fx, Rx, D))]
+#[scale_info(skip_type_params(A, Fx, Rx, D, Fp))]
 #[codec(mel_bound())]
-pub struct LoneFreezeConsideration<A, Fx, Rx, D>(PhantomData<fn() -> (A, Fx, Rx, D)>);
+pub struct LoneFreezeConsideration<A, Fx, Rx, D, Fp>(PhantomData<fn() -> (A, Fx, Rx, D, Fp)>);
 impl<
 		A: 'static,
 		Fx: 'static + MutateFreeze<A>,
 		Rx: 'static + Get<Fx::Id>,
-		D: 'static + Convert<Footprint, Fx::Balance>,
-	> Consideration<A> for LoneFreezeConsideration<A, Fx, Rx, D>
+		D: 'static + Convert<Fp, Fx::Balance>,
+		Fp: 'static,
+	> Consideration<A, Fp> for LoneFreezeConsideration<A, Fx, Rx, D, Fp>
 {
-	fn new(who: &A, footprint: Footprint) -> Result<Self, DispatchError> {
+	fn new(who: &A, footprint: Fp) -> Result<Option<Self>, DispatchError> {
 		ensure!(Fx::balance_frozen(&Rx::get(), who).is_zero(), DispatchError::Unavailable);
-		Fx::set_frozen(&Rx::get(), who, D::convert(footprint), Polite).map(|_| Self(PhantomData))
+		let new = D::convert(footprint);
+		if new.is_zero() {
+			Ok(None)
+		} else {
+			Fx::set_frozen(&Rx::get(), who, new, Polite).map(|_| Some(Self(PhantomData)))
+		}
 	}
-	fn update(self, who: &A, footprint: Footprint) -> Result<Self, DispatchError> {
-		Fx::set_frozen(&Rx::get(), who, D::convert(footprint), Polite).map(|_| Self(PhantomData))
+	fn update(self, who: &A, footprint: Fp) -> Result<Option<Self>, DispatchError> {
+		let new = D::convert(footprint);
+		let _ = Fx::set_frozen(&Rx::get(), who, new, Polite)?;
+		if new.is_zero() {
+			Ok(None)
+		} else {
+			Ok(Some(Self(PhantomData)))
+		}
 	}
 	fn drop(self, who: &A) -> Result<(), DispatchError> {
 		Fx::thaw(&Rx::get(), who).map(|_| ())
@@ -330,22 +363,34 @@ impl<
 	MaxEncodedLen,
 	RuntimeDebugNoBound,
 )]
-#[scale_info(skip_type_params(A, Fx, Rx, D))]
+#[scale_info(skip_type_params(A, Fx, Rx, D, Fp))]
 #[codec(mel_bound())]
-pub struct LoneHoldConsideration<A, Fx, Rx, D>(PhantomData<fn() -> (A, Fx, Rx, D)>);
+pub struct LoneHoldConsideration<A, Fx, Rx, D, Fp>(PhantomData<fn() -> (A, Fx, Rx, D, Fp)>);
 impl<
 		A: 'static,
 		F: 'static + MutateHold<A>,
 		R: 'static + Get<F::Reason>,
-		D: 'static + Convert<Footprint, F::Balance>,
-	> Consideration<A> for LoneHoldConsideration<A, F, R, D>
+		D: 'static + Convert<Fp, F::Balance>,
+		Fp: 'static,
+	> Consideration<A, Fp> for LoneHoldConsideration<A, F, R, D, Fp>
 {
-	fn new(who: &A, footprint: Footprint) -> Result<Self, DispatchError> {
+	fn new(who: &A, footprint: Fp) -> Result<Option<Self>, DispatchError> {
 		ensure!(F::balance_on_hold(&R::get(), who).is_zero(), DispatchError::Unavailable);
-		F::set_on_hold(&R::get(), who, D::convert(footprint)).map(|_| Self(PhantomData))
+		let new = D::convert(footprint);
+		if new.is_zero() {
+			Ok(None)
+		} else {
+			F::set_on_hold(&R::get(), who, new).map(|_| Some(Self(PhantomData)))
+		}
 	}
-	fn update(self, who: &A, footprint: Footprint) -> Result<Self, DispatchError> {
-		F::set_on_hold(&R::get(), who, D::convert(footprint)).map(|_| Self(PhantomData))
+	fn update(self, who: &A, footprint: Fp) -> Result<Option<Self>, DispatchError> {
+		let new = D::convert(footprint);
+		let _ = F::set_on_hold(&R::get(), who, new)?;
+		if new.is_zero() {
+			Ok(None)
+		} else {
+			Ok(Some(Self(PhantomData)))
+		}
 	}
 	fn drop(self, who: &A) -> Result<(), DispatchError> {
 		F::release_all(&R::get(), who, BestEffort).map(|_| ())