Avoid associated type in Consideration

Author: gavofyork

substrate/frame/preimage/src/lib.rs

@@ -85,8 +85,6 @@ pub enum RequestStatus<AccountId, Ticket> {
 
 type BalanceOf<T> =
 	<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
-type TicketOf<T> =
-	<<T as Config>::Consideration as Consideration<<T as frame_system::Config>::AccountId>>::Ticket;
 
 /// Maximum size of preimage we can store is 4mb.
 const MAX_SIZE: u32 = 4 * 1024 * 1024;
@@ -164,7 +162,7 @@ pub mod pallet {
 	/// The request status of a given hash.
 	#[pallet::storage]
 	pub(super) type RequestStatusFor<T: Config> =
-		StorageMap<_, Identity, T::Hash, RequestStatus<T::AccountId, TicketOf<T>>>;
+		StorageMap<_, Identity, T::Hash, RequestStatus<T::AccountId, T::Consideration>>;
 
 	#[pallet::storage]
 	pub(super) type PreimageFor<T: Config> =
@@ -239,20 +237,21 @@ impl<T: Config> Pallet<T> {
 				// unreserve deposit
 				T::Currency::unreserve(&who, amount);
 				// take consideration
-				let ticket = T::Consideration::new(&who, Footprint::from_parts(1, len as usize))
-					.unwrap_or_default();
-				RequestStatus::Unrequested { ticket: (who, ticket), len }
+				let footprint = Footprint::from_parts(1, len as usize);
+				let maybe_consider = T::Consideration::new(&who, footprint);
+				let Ok(consider) = maybe_consider else { return false };
+				RequestStatus::Unrequested { ticket: (who, consider), len }
 			},
 			OldRequestStatus::Requested { deposit: maybe_deposit, count, len: maybe_len } => {
 				let maybe_ticket = if let Some((who, deposit)) = maybe_deposit {
 					// unreserve deposit
 					T::Currency::unreserve(&who, deposit);
 					// take consideration
 					if let Some(len) = maybe_len {
-						let ticket =
-							T::Consideration::new(&who, Footprint::from_parts(1, len as usize))
-								.unwrap_or_default();
-						Some((who, ticket))
+						let footprint = Footprint::from_parts(1, len as usize);
+						let maybe_consider = T::Consideration::new(&who, footprint);
+						let Ok(consider) = maybe_consider else { return false };
+						Some((who, consider))
 					} else {
 						None
 					}
@@ -308,9 +307,9 @@ impl<T: Config> Pallet<T> {
 			(None, None) =>
 				RequestStatus::Requested { maybe_ticket: None, count: 1, maybe_len: Some(len) },
 			(None, Some(depositor)) => {
-				let ticket =
+				let consider =
 					T::Consideration::new(depositor, Footprint::from_parts(1, len as usize))?;
-				RequestStatus::Unrequested { ticket: (depositor.clone(), ticket), len }
+				RequestStatus::Unrequested { ticket: (depositor.clone(), consider), len }
 			},
 		};
 		let was_requested = matches!(status, RequestStatus::Requested { .. });
@@ -359,9 +358,13 @@ impl<T: Config> Pallet<T> {
 	) -> DispatchResult {
 		Self::ensure_updated(&hash);
 		match RequestStatusFor::<T>::get(hash).ok_or(Error::<T>::NotNoted)? {
-			RequestStatus::Requested { maybe_ticket: Some((owner, ticket)), count, maybe_len } => {
+			RequestStatus::Requested {
+				maybe_ticket: Some((owner, consider)),
+				count,
+				maybe_len,
+			} => {
 				ensure!(maybe_check_owner.map_or(true, |c| c == owner), Error::<T>::NotAuthorized);
-				let _ = T::Consideration::drop(&owner, ticket);
+				let _ = consider.drop(&owner);
 				RequestStatusFor::<T>::insert(
 					hash,
 					RequestStatus::Requested { maybe_ticket: None, count, maybe_len },
@@ -372,9 +375,9 @@ impl<T: Config> Pallet<T> {
 				ensure!(maybe_check_owner.is_none(), Error::<T>::NotAuthorized);
 				Self::do_unrequest_preimage(hash)
 			},
-			RequestStatus::Unrequested { ticket: (owner, ticket), len } => {
+			RequestStatus::Unrequested { ticket: (owner, consider), len } => {
 				ensure!(maybe_check_owner.map_or(true, |c| c == owner), Error::<T>::NotAuthorized);
-				let _ = T::Consideration::drop(&owner, ticket);
+				let _ = consider.drop(&owner);
 				RequestStatusFor::<T>::remove(hash);
 
 				Self::remove(hash, len);

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

@@ -156,41 +156,38 @@ impl Footprint {
 /// 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;
+///
+/// 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.
+#[must_use]
+pub trait Consideration<AccountId>: 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>;
 
 	/// 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 update(self, who: &AccountId, new: Footprint) -> Result<Self, DispatchError>;
 
 	/// 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(|_| ())
-	}
+	fn drop(self, who: &AccountId) -> Result<(), DispatchError>;
 }
 
 impl<A> Consideration<A> for () {
-	type Ticket = ();
-	fn update(_: &A, _: Option<()>, _: Option<Footprint>) -> Result<(), DispatchError> {
+	fn new(_: &A, _: Footprint) -> Result<Self, DispatchError> {
+		Ok(())
+	}
+	fn update(self, _: &A, _: Footprint) -> Result<(), DispatchError> {
+		Ok(())
+	}
+	fn drop(self, _: &A) -> Result<(), DispatchError> {
 		Ok(())
 	}
 }