review & cleanup

pallets/asset-index/src/lib.rs

@@ -34,7 +34,6 @@ pub mod pallet {
 	use primitives::traits::AssetRecorderBenchmarks;
 
 	use frame_support::{
-		dispatch::DispatchResultWithPostInfo,
 		pallet_prelude::*,
 		sp_runtime::{
 			traits::{AccountIdConversion, AtLeast32BitUnsigned, CheckedAdd, CheckedDiv, CheckedSub, Saturating, Zero},
@@ -361,7 +360,7 @@ pub mod pallet {
 			asset_id: T::AssetId,
 			units: T::Balance,
 			amount: T::Balance,
-		) -> DispatchResultWithPostInfo {
+		) -> DispatchResult {
 			Self::do_add_asset(T::AdminOrigin::ensure_origin(origin)?, asset_id, units, amount)
 		}
 
@@ -373,7 +372,7 @@ pub mod pallet {
 			units: T::Balance,
 			amount: T::Balance,
 			recipient: T::AccountId,
-		) -> DispatchResultWithPostInfo {
+		) -> DispatchResult {
 			ensure_root(origin)?;
 			Self::do_add_asset(recipient, asset_id, units, amount)
 		}
@@ -394,7 +393,7 @@ pub mod pallet {
 			asset_id: T::AssetId,
 			units: T::Balance,
 			recipient: Option<T::AccountId>,
-		) -> DispatchResultWithPostInfo {
+		) -> DispatchResult {
 			Self::do_remove_asset(T::AdminOrigin::ensure_origin(origin)?, asset_id, units, recipient)
 		}
 
@@ -406,7 +405,7 @@ pub mod pallet {
 			asset_id: T::AssetId,
 			units: T::Balance,
 			recipient: Option<T::AccountId>,
-		) -> DispatchResultWithPostInfo {
+		) -> DispatchResult {
 			ensure_root(origin)?;
 			Self::do_remove_asset(who, asset_id, units, recipient)
 		}
@@ -447,7 +446,7 @@ pub mod pallet {
 			T::AdminOrigin::ensure_origin(origin)?;
 			ensure!(!new_range.minimum.is_zero(), Error::<T>::InvalidDepositRange);
 			ensure!(new_range.maximum > new_range.minimum, Error::<T>::InvalidDepositRange);
-			IndexTokenDepositRange::<T>::put(new_range.clone());
+			IndexTokenDepositRange::<T>::put(&new_range);
 			Self::deposit_event(Event::<T>::IndexTokenDepositRangeUpdated(new_range));
 			Ok(())
 		}
@@ -486,12 +485,9 @@ pub mod pallet {
 			let bounded_symbol: BoundedVec<u8, T::StringLimit> =
 				symbol.clone().try_into().map_err(|_| Error::<T>::BadMetadata)?;
 
-			Metadata::<T>::try_mutate_exists(id, |metadata| {
-				*metadata = Some(AssetMetadata { name: bounded_name, symbol: bounded_symbol, decimals });
-
-				Self::deposit_event(Event::MetadataSet(id, name, symbol, decimals));
-				Ok(())
-			})
+			Metadata::<T>::insert(id, AssetMetadata { name: bounded_name, symbol: bounded_symbol, decimals });
+			Self::deposit_event(Event::MetadataSet(id, name, symbol, decimals));
+			Ok(())
 		}
 
 		/// Initiate a transfer from the user's sovereign account into the
@@ -501,6 +497,7 @@ pub mod pallet {
 		/// account and mints PINT proportionally using the latest
 		/// available price pairs
 		#[pallet::weight(T::WeightInfo::deposit())]
+		#[transactional]
 		pub fn deposit(origin: OriginFor<T>, asset_id: T::AssetId, units: T::Balance) -> DispatchResult {
 			let caller = T::AdminOrigin::ensure_origin(origin)?;
 			if units.is_zero() {
@@ -533,12 +530,8 @@ pub mod pallet {
 			T::RemoteAssetManager::deposit(asset_id, units);
 
 			// insert new deposit
-			<Deposits<T>>::try_mutate(&caller, |deposits| -> DispatchResult {
-				deposits
-					.try_push((index_tokens, frame_system::Pallet::<T>::block_number()))
-					.map_err(|_| Error::<T>::TooManyDeposits)?;
-				Ok(())
-			})?;
+			Deposits::<T>::try_append(&caller, (index_tokens, frame_system::Pallet::<T>::block_number()))
+				.map_err(|_| Error::<T>::TooManyDeposits)?;
 
 			Self::deposit_event(Event::Deposited(asset_id, units, caller, index_tokens));
 			Ok(())
@@ -557,7 +550,7 @@ pub mod pallet {
 		/// ratio of the liquid assets in the index.
 		#[pallet::weight(T::WeightInfo::withdraw())]
 		#[transactional]
-		pub fn withdraw(origin: OriginFor<T>, amount: T::Balance) -> DispatchResultWithPostInfo {
+		pub fn withdraw(origin: OriginFor<T>, amount: T::Balance) -> DispatchResult {
 			let caller = T::AdminOrigin::ensure_origin(origin.clone())?;
 			ensure!(amount >= T::MinimumRedemption::get(), Error::<T>::MinimumRedemption);
 
@@ -616,13 +609,10 @@ pub mod pallet {
 			// state
 			let end_block = frame_system::Pallet::<T>::block_number().saturating_add(T::WithdrawalPeriod::get());
 			// lock the assets for the withdrawal period starting at current block
-			PendingWithdrawals::<T>::mutate(&caller, |maybe_redemption| {
-				let redemption = maybe_redemption.get_or_insert_with(|| Vec::with_capacity(1));
-				redemption.push(PendingRedemption { end_block, assets })
-			});
+			PendingWithdrawals::<T>::append(&caller, PendingRedemption { end_block, assets });
 
 			Self::deposit_event(Event::WithdrawalInitiated(caller, effectively_withdrawn));
-			Ok(().into())
+			Ok(())
 		}
 
 		/// Attempts to complete all currently pending redemption processes
@@ -643,8 +633,8 @@ pub mod pallet {
 		/// as soon as the aforementioned conditions are met, regardless of
 		/// whether the other `AssetWithdrawal`s in the same `PendingWithdrawal` set
 		/// can also be closed successfully.
-		#[transactional]
 		#[pallet::weight(T::WeightInfo::complete_withdraw())]
+		#[transactional]
 		pub fn complete_withdraw(origin: OriginFor<T>) -> DispatchResult {
 			let caller = T::AdminOrigin::ensure_origin(origin.clone())?;
 			let current_block = frame_system::Pallet::<T>::block_number();
@@ -766,18 +756,18 @@ pub mod pallet {
 			asset_id: T::AssetId,
 			units: T::Balance,
 			amount: T::Balance,
-		) -> DispatchResultWithPostInfo {
-			Assets::<T>::get(&asset_id).ok_or(Error::<T>::AssetNotExists)?;
+		) -> DispatchResult {
+			ensure!(Assets::<T>::contains_key(&asset_id), Error::<T>::AssetNotExists);
 
 			if units.is_zero() {
-				return Ok(().into());
+				return Ok(());
 			}
 
 			// transfer the caller's fund into the treasury account
 			Self::add_liquid(&recipient, asset_id, units, amount)?;
 
 			Self::deposit_event(Event::AssetAdded(asset_id, units, recipient, amount));
-			Ok(().into())
+			Ok(())
 		}
 
 		/// Removes liquid assets
@@ -786,9 +776,9 @@ pub mod pallet {
 			asset_id: T::AssetId,
 			units: T::Balance,
 			recipient: Option<T::AccountId>,
-		) -> DispatchResultWithPostInfo {
+		) -> DispatchResult {
 			if units.is_zero() {
-				return Ok(().into());
+				return Ok(());
 			}
 			Self::ensure_not_native_asset(&asset_id)?;
 
@@ -989,7 +979,7 @@ pub mod pallet {
 		fn do_update_index_token_locks(user: &T::AccountId) {
 			let locks = IndexTokenLocks::<T>::get(user);
 			if !locks.is_empty() {
-				Self::do_insert_index_token_locks(user, IndexTokenLocks::<T>::get(user))
+				Self::do_insert_index_token_locks(user, locks)
 			}
 		}
 
@@ -1148,7 +1138,7 @@ pub mod pallet {
 			units: T::Balance,
 			location: MultiLocation,
 			amount: T::Balance,
-		) -> DispatchResultWithPostInfo {
+		) -> DispatchResult {
 			let origin = T::AdminOrigin::successful_origin();
 			let origin_account_id = T::AdminOrigin::ensure_origin(origin.clone()).unwrap();
 

pallets/committee/src/lib.rs

@@ -110,7 +110,7 @@ pub mod pallet {
 
 	/// Store a mapping (hash) -> Proposal for all existing proposals.
 	#[pallet::storage]
-	pub type Proposals<T: Config> = StorageMap<_, Blake2_128Concat, HashFor<T>, Proposal<T>, OptionQuery>;
+	pub type Proposals<T: Config> = StorageMap<_, Identity, HashFor<T>, Proposal<T>, OptionQuery>;
 
 	/// Store a mapping (hash) -> () for all proposals that have been executed
 	#[pallet::storage]
@@ -247,14 +247,11 @@ pub mod pallet {
 		/// store Returns an error if the data type used for the nonce
 		/// exceeds is maximum value
 		fn take_and_increment_nonce() -> Result<T::ProposalNonce, Error<T>> {
-			let nonce = ProposalCount::<T>::get();
-			match nonce.checked_add(&T::ProposalNonce::one()) {
-				Some(next) => {
-					ProposalCount::<T>::set(next);
-					Ok(nonce)
-				}
-				None => Err(Error::ProposalNonceExhausted),
-			}
+			ProposalCount::<T>::try_mutate(|nonce| -> Result<T::ProposalNonce, Error<T>> {
+				let current = nonce.clone();
+				*nonce = current.checked_add(&One::one()).ok_or(Error::ProposalNonceExhausted)?;
+				Ok(current)
+			})
 		}
 
 		pub fn active_proposals() -> Vec<HashFor<T>> {
@@ -294,25 +291,26 @@ pub mod pallet {
 		/// `Storage: ActiveProposals (r:1 w:1) + Votes (r1) * len(proposals)`
 		fn upkeep(n: BlockNumberFor<T>) -> Weight {
 			// ActiveProposals.retain (r:1 w:1)
-			let mut consumed = T::DbWeight::get().reads_writes(1, 1);
+			let mut reads: Weight = 1;
+			let writes: Weight = 1;
 
 			// clear out proposals that are no longer active
 			ActiveProposals::<T>::mutate(|proposals| {
 				// consumed weight for all `Storage: Votes (r1)` lookups
-				let read_vote = T::DbWeight::get().reads(1);
-				consumed = consumed.saturating_add(read_vote.saturating_mul(proposals.len() as Weight));
+				reads = reads.saturating_add(proposals.len() as Weight);
 
 				proposals.retain(|hash| if let Some(votes) = Self::get_votes_for(hash) { votes.end > n } else { false })
 			});
 
-			consumed
+			T::DbWeight::get().reads_writes(reads, writes)
 		}
 
 		/// Used to check if an origin is signed and the signer is a member of
 		/// the committee
 		pub fn ensure_member(origin: OriginFor<T>) -> Result<CommitteeMember<AccountIdFor<T>>, DispatchError> {
 			let who = ensure_signed(origin)?;
-			Ok(CommitteeMember::new(who.clone(), Members::<T>::get(who).ok_or(Error::<T>::NotMember)?))
+			let members = Members::<T>::get(&who).ok_or(Error::<T>::NotMember)?;
+			Ok(CommitteeMember::new(who, members))
 		}
 	}
 

pallets/remote-asset-manager/src/lib.rs

@@ -474,7 +474,6 @@ pub mod pallet {
 			value: T::Balance,
 			payee: RewardDestination<AccountIdFor<T>>,
 		) -> DispatchResultWithPostInfo {
-			let _ = ensure_signed(origin.clone())?;
 			T::AdminOrigin::ensure_origin(origin)?;
 			if value.is_zero() {
 				return Ok(().into());