comments

pallets/api/src/messaging/deposits.rs

@@ -15,27 +15,58 @@ pub fn calculate_protocol_deposit<T: Config, ByteFee: Get<BalanceOf<T>>>(
 ) -> BalanceOf<T> {
 	let base: usize = match p {
 		ProtocolStorageDeposit::XcmQueries =>
-			KeyLenOf::<XcmQueries<T>>::get() as usize +
-				AccountIdOf::<T>::max_encoded_len() +
-				MessageId::max_encoded_len() +
-				Option::<Callback<T::AccountId>>::max_encoded_len(),
-		ProtocolStorageDeposit::IsmpRequests =>
-			KeyLenOf::<IsmpRequests<T>>::get() as usize +
-				AccountIdOf::<T>::max_encoded_len() +
-				MessageId::max_encoded_len(),
+			(KeyLenOf::<XcmQueries<T>>::get() as usize)
+				.saturating_add(AccountIdOf::<T>::max_encoded_len())
+				.saturating_add(MessageId::max_encoded_len())
+				.saturating_add(Option::<Callback<T::AccountId>>::max_encoded_len()),
+
+		ProtocolStorageDeposit::IsmpRequests => 
+			(KeyLenOf::<IsmpRequests<T>>::get() as usize)
+				.saturating_add(AccountIdOf::<T>::max_encoded_len())
+				.saturating_add(MessageId::max_encoded_len()),
 	};
-	ByteFee::get() * base.saturated_into()
+	ByteFee::get().saturating_mul(base.saturated_into())
 }
 
 /// Calculate the deposit for the storage used for the Message enum.
 pub fn calculate_message_deposit<T: Config, ByteFee: Get<BalanceOf<T>>>() -> BalanceOf<T> {
-	ByteFee::get() *
+	ByteFee::get().saturating_mul(
 		(KeyLenOf::<Messages<T>>::get() as usize + Message::<T>::max_encoded_len())
-			.saturated_into()
+			.saturated_into())
 }
 
 /// Blanket implementation of generating the deposit for a type that implements MaxEncodedLen.
 pub fn calculate_deposit_of<T: Config, ByteFee: Get<BalanceOf<T>>, U: MaxEncodedLen>(
 ) -> BalanceOf<T> {
-	ByteFee::get() * U::max_encoded_len().saturated_into()
+	ByteFee::get().saturating_mul(U::max_encoded_len().saturated_into())
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use crate::mock::*;
+	use frame_support::pallet_prelude::Get;
+
+	struct Two;
+	impl Get<u128> for Two {
+		fn get() -> u128 {
+			2
+		}
+	}
+
+	#[test]
+	fn calculate_deposit_of_works() {
+		new_test_ext().execute_with(|| {
+			// 4 + 4 bytes.
+			#[derive(Copy, Clone, Debug, Encode, Eq, Decode, MaxEncodedLen, PartialEq, TypeInfo)]
+			struct Data {
+				pub a: u32,
+				pub b: u32,
+			}
+
+			// 8 * 2 = 16 units
+			assert_eq!(calculate_deposit_of::<Test, Two, Data>(), 16);
+		})
+	}
+
 }

pallets/api/src/messaging/mod.rs

@@ -270,11 +270,8 @@ pub mod pallet {
 			T::Deposit::hold(&HoldReason::Messaging.into(), &origin, deposit)?;
 
 			// Process message by dispatching request via ISMP.
-			let maybe_commitment = T::IsmpDispatcher::default()
-				.dispatch_request(message.into(), FeeMetadata { payer: origin.clone(), fee });
-
-			match maybe_commitment {
-				Ok(commitment) => {
+			let commitment = T::IsmpDispatcher::default()
+				.dispatch_request(message.into(), FeeMetadata { payer: origin.clone(), fee }).map_err(|_|Error::<T>::IsmpDispatchFailed)?;
 					// Store commitment for lookup on response, message for querying,
 					// response/timeout handling.
 					IsmpRequests::<T>::insert(&commitment, (&origin, id));
@@ -295,22 +292,6 @@ pub mod pallet {
 						callback,
 					});
 					Ok(())
-				},
-				Err(_) => {
-					// Allow a caller to poll for the status still and retreive the message deposit.
-					Messages::<T>::insert(
-						&origin,
-						id,
-						Message::Ismp {
-							commitment: Default::default(),
-							callback,
-							deposit,
-							status: MessageStatus::Err(Error::<T>::IsmpDispatchFailed.into()),
-						},
-					);
-					Err(Error::<T>::IsmpDispatchFailed.into())
-				},
-			}
 		}
 
 		// TODO: does ismp allow querying to ensure that specified para id is supported?
@@ -334,11 +315,9 @@ pub mod pallet {
 			T::Deposit::hold(&HoldReason::Messaging.into(), &origin, deposit)?;
 
 			// Process message by dispatching request via ISMP.
-			let maybe_commitment = T::IsmpDispatcher::default()
-				.dispatch_request(message.into(), FeeMetadata { payer: origin.clone(), fee });
+			let commitment = T::IsmpDispatcher::default()
+				.dispatch_request(message.into(), FeeMetadata { payer: origin.clone(), fee }).map_err(|_| Error::<T>::IsmpDispatchFailed)?;
 
-			match maybe_commitment {
-				Ok(commitment) => {
 					// Store commitment for lookup on response, message for querying,
 					// response/timeout handling.
 					IsmpRequests::<T>::insert(&commitment, (&origin, id));
@@ -359,22 +338,6 @@ pub mod pallet {
 						callback,
 					});
 					Ok(())
-				},
-				Err(_) => {
-					// Allow a caller to poll for the status still and retreive the message deposit.
-					Messages::<T>::insert(
-						&origin,
-						id,
-						Message::Ismp {
-							commitment: Default::default(),
-							callback,
-							deposit,
-							status: MessageStatus::Err(Error::<T>::IsmpDispatchFailed.into()),
-						},
-					);
-					Err(Error::<T>::IsmpDispatchFailed.into())
-				},
-			}
 		}
 
 		#[pallet::call_index(3)]
@@ -479,8 +442,6 @@ pub mod pallet {
 		}
 
 		/// Try and remove a collection of messages.
-		/// Will revert if any one of the messages is erroneous.
-		#[frame_support::transactional]
 		#[pallet::call_index(5)]
 		#[pallet::weight(Weight::zero())]
 		pub fn remove(
@@ -689,7 +650,7 @@ impl<T: Config> Message<T> {
 	}
 
 	/// Remove a message from storage.
-	/// Does no check on wether a message should be removed.
+	/// Does no check on whether a message should be removed.
 	pub(crate) fn remove(&self, origin: &AccountIdOf<T>, id: &MessageId) {
 		Messages::<T>::remove(&origin, &id);
 		match self {
@@ -724,9 +685,9 @@ impl<T: Config> Message<T> {
 pub enum MessageStatus {
 	/// No errors have been recorded.
 	Ok,
-	/// An error has occurred with this message>
+	/// An error has occurred with this message.
 	Err(DispatchError),
-	/// A timeout has occurred
+	/// A timeout has occurred.
 	Timeout,
 }
 

pop-api/Cargo.toml

@@ -40,4 +40,4 @@ std = [
 	"pop-primitives/std",
 	"sp-io/std",
 ]
-messaging = [	]
+messaging = [ "default" ]