263 add optimizations audit recommendations

clients/runtime/src/rpc.rs

@@ -1109,10 +1109,11 @@ pub trait RedeemPallet {
 	async fn get_redeem_request(&self, redeem_id: H256) -> Result<SpacewalkRedeemRequest, Error>;
 
 	/// Get all redeem requests requested of the given vault
-	async fn get_vault_redeem_requests(
+	async fn get_vault_redeem_requests<T>(
 		&self,
 		account_id: AccountId,
-	) -> Result<Vec<(H256, SpacewalkRedeemRequest)>, Error>;
+		filter: Box<dyn Fn((H256, SpacewalkRedeemRequest)) -> Option<T> + Send>,
+	) -> Result<Vec<T>, Error>;
 
 	async fn get_redeem_period(&self) -> Result<BlockNumber, Error>;
 }
@@ -1165,24 +1166,38 @@ impl RedeemPallet for SpacewalkParachain {
 			.await
 	}
 
-	async fn get_vault_redeem_requests(
+	async fn get_vault_redeem_requests<T>(
 		&self,
 		account_id: AccountId,
-	) -> Result<Vec<(H256, SpacewalkRedeemRequest)>, Error> {
+		filter: Box<dyn Fn((H256, SpacewalkRedeemRequest)) -> Option<T> + Send>,
+	) -> Result<Vec<T>, Error> {
 		let head = self.get_finalized_block_hash().await?;
 		let result: Vec<H256> = self
 			.api
 			.rpc()
 			.request("redeem_getVaultRedeemRequests", rpc_params![account_id, head])
 			.await?;
-		join_all(
-			result.into_iter().map(|key| async move {
-				self.get_redeem_request(key).await.map(|value| (key, value))
-			}),
-		)
-		.await
-		.into_iter()
-		.collect()
+
+		let redeem_requests = join_all(result.into_iter().map(|key| async move {
+			self.get_redeem_request(key).await.map(|value| (key, value))
+		}))
+		.await;
+
+		let mut some_error: Option<Error> = None;
+		let filtered_results: Vec<_> = redeem_requests
+			.into_iter()
+			.filter_map(|item_maybe| match item_maybe {
+				Ok(item) => filter(item),
+				Err(e) => {
+					some_error.get_or_insert(e);
+					None
+				},
+			})
+			.collect();
+		match some_error {
+			Some(err) => Err(err),
+			None => Ok(filtered_results),
+		}
 	}
 
 	async fn get_redeem_period(&self) -> Result<BlockNumber, Error> {
@@ -1256,10 +1271,11 @@ pub trait ReplacePallet {
 	) -> Result<Vec<(H256, SpacewalkReplaceRequest)>, Error>;
 
 	/// Get all replace requests made by the given vault
-	async fn get_old_vault_replace_requests(
+	async fn get_old_vault_replace_requests<T: 'static>(
 		&self,
 		account_id: AccountId,
-	) -> Result<Vec<(H256, SpacewalkReplaceRequest)>, Error>;
+		filter: Box<dyn Fn((H256, SpacewalkReplaceRequest)) -> Option<T> + Send>,
+	) -> Result<Vec<T>, Error>;
 
 	/// Get the time difference in number of blocks between when a replace
 	/// request is created and required completion time by a vault
@@ -1352,22 +1368,39 @@ impl ReplacePallet for SpacewalkParachain {
 	}
 
 	/// Get all replace requests made by the given vault
-	async fn get_old_vault_replace_requests(
+	async fn get_old_vault_replace_requests<T>(
 		&self,
 		account_id: AccountId,
-	) -> Result<Vec<(H256, SpacewalkReplaceRequest)>, Error> {
+		filter: Box<dyn Fn((H256, SpacewalkReplaceRequest)) -> Option<T> + Send>,
+	) -> Result<Vec<T>, Error> {
 		let head = self.get_finalized_block_hash().await?;
 		let result: Vec<H256> = self
 			.api
 			.rpc()
 			.request("replace_getOldVaultReplaceRequests", rpc_params![account_id, head])
 			.await?;
-		join_all(result.into_iter().map(|key| async move {
+
+		let replace_requests = join_all(result.into_iter().map(|key| async move {
 			self.get_replace_request(key).await.map(|value| (key, value))
 		}))
-		.await
-		.into_iter()
-		.collect()
+		.await;
+
+		let mut some_error: Option<Error> = None;
+		let filtered_results: Vec<_> = replace_requests
+			.into_iter()
+			.filter_map(|item_maybe| match item_maybe {
+				Ok(item) => filter(item),
+				Err(e) => {
+					some_error.get_or_insert(e);
+					None
+				},
+			})
+			.collect();
+
+		match some_error {
+			Some(err) => Err(err),
+			None => Ok(filtered_results),
+		}
 	}
 
 	async fn get_replace_period(&self) -> Result<u32, Error> {

clients/vault/src/cancellation.rs

@@ -169,16 +169,16 @@ impl<P: IssuePallet + ReplacePallet + Send + Sync> Canceller<P> for ReplaceCance
 // verbose drain_filter
 fn drain_expired(requests: &mut Vec<ActiveRequest>, current_height: u32) -> Vec<ActiveRequest> {
 	let mut expired = Vec::new();
-	let has_expired = |request: &ActiveRequest| current_height > request.parachain_deadline_height;
-	let mut i = 0;
-	while i != requests.len() {
-		if has_expired(&requests[i]) {
-			let req = requests.remove(i);
-			expired.push(req);
+
+	requests.retain(|req| {
+		if current_height > req.parachain_deadline_height {
+			expired.push(*req);
+			false
 		} else {
-			i += 1;
+			true
 		}
-	}
+	});
+
 	expired
 }
 

clients/vault/src/execution.rs

@@ -374,29 +374,38 @@ pub async fn execute_open_requests(
 	let parachain_rpc = &parachain_rpc;
 	let vault_id = parachain_rpc.get_account_id().clone();
 
-	// get all redeem and replace requests
-	let (redeem_requests, replace_requests) = try_join!(
-		parachain_rpc.get_vault_redeem_requests(vault_id.clone()),
-		parachain_rpc.get_old_vault_replace_requests(vault_id.clone()),
-	)?;
-
-	let open_redeems = redeem_requests
-		.into_iter()
-		.filter(|(_, request)| request.status == RedeemRequestStatus::Pending)
-		.filter_map(|(hash, request)| {
+	//closure to filter and transform redeem_requests
+	let filter_redeem_reqs = move |(hash, request): (H256, SpacewalkRedeemRequest)| {
+		if request.status == RedeemRequestStatus::Pending {
 			Request::from_redeem_request(hash, request, payment_margin).ok()
-		});
+		} else {
+			None
+		}
+	};
 
-	let open_replaces = replace_requests
-		.into_iter()
-		.filter(|(_, request)| request.status == ReplaceRequestStatus::Pending)
-		.filter_map(|(hash, request)| {
+	//closure to filter and transform replace_requests
+	let filter_replace_reqs = move |(hash, request): (H256, SpacewalkReplaceRequest)| {
+		if request.status == ReplaceRequestStatus::Pending {
 			Request::from_replace_request(hash, request, payment_margin).ok()
-		});
+		} else {
+			None
+		}
+	};
+
+	// get all redeem and replace requests
+	let (open_redeems, open_replaces) = try_join!(
+		parachain_rpc
+			.get_vault_redeem_requests::<Request>(vault_id.clone(), Box::new(filter_redeem_reqs)),
+		parachain_rpc.get_old_vault_replace_requests::<Request>(
+			vault_id.clone(),
+			Box::new(filter_replace_reqs)
+		),
+	)?;
 
 	// collect all requests into a hashmap, indexed by their id
 	let mut open_requests = open_redeems
-		.chain(open_replaces)
+		.into_iter()
+		.chain(open_replaces.into_iter())
 		.map(|x| (derive_shortened_request_id(&x.hash.0), x))
 		.collect::<HashMap<_, _>>();
 
@@ -461,7 +470,7 @@ pub async fn execute_open_requests(
 
 	// All requests remaining in the hashmap did not have a Stellar payment yet, so pay
 	// and execute all of these
-	for (_, request) in open_requests {
+	for (_, request) in open_requests.into_iter() {
 		// there are potentially a large number of open requests - pay and execute each
 		// in a separate task to ensure that awaiting confirmations does not significantly
 		// delay other requests

clients/vault/src/metrics.rs

@@ -15,7 +15,7 @@ use runtime::{
 	types::currency_id::CurrencyIdExt,
 	AggregateUpdatedEvent, CollateralBalancesPallet, CurrencyId, Error as RuntimeError, FixedU128,
 	IssuePallet, IssueRequestStatus, OracleKey, RedeemPallet, RedeemRequestStatus, SecurityPallet,
-	SpacewalkParachain, UtilFuncs, VaultId, VaultRegistryPallet, H256,
+	SpacewalkParachain, SpacewalkRedeemRequest, UtilFuncs, VaultId, VaultRegistryPallet, H256,
 };
 use service::{
 	warp::{Rejection, Reply},
@@ -469,8 +469,14 @@ pub async fn poll_metrics<
 	loop {
 		publish_native_currency_balance(parachain_rpc).await?;
 		publish_issue_count(parachain_rpc, vault_id_manager).await;
+
+		let pass_all_filter = |item: (H256, SpacewalkRedeemRequest)| Some(item);
+
 		if let Ok(redeems) = parachain_rpc
-			.get_vault_redeem_requests(parachain_rpc.get_account_id().clone())
+			.get_vault_redeem_requests::<(H256, SpacewalkRedeemRequest)>(
+				parachain_rpc.get_account_id().clone(),
+				Box::new(pass_all_filter),
+			)
 			.await
 		{
 			publish_redeem_count(vault_id_manager, &redeems).await;

clients/vault/src/oracle/collector/proof_builder.rs

@@ -114,24 +114,22 @@ impl ScpMessageCollector {
 		&self,
 		slot: Slot,
 		sender: &StellarMessageSender,
-	) -> UnlimitedVarArray<ScpEnvelope> {
-		let empty = UnlimitedVarArray::new_empty();
-
+	) -> Option<UnlimitedVarArray<ScpEnvelope>> {
 		if let Some(envelopes) = self.envelopes_map().get(&slot) {
 			// lacking envelopes
 			if envelopes.len() < get_min_externalized_messages(self.is_public()) {
 				tracing::debug!(
 					"Proof Building for slot {slot}: not enough envelopes to build proof "
 				);
 			} else {
-				return UnlimitedVarArray::new(envelopes.clone()).unwrap_or(empty)
+				return UnlimitedVarArray::new(envelopes.clone()).ok()
 			}
 		}
 
 		// forcefully retrieve envelopes
 		self.fetch_missing_envelopes(slot, sender).await;
 
-		empty
+		return None
 	}
 
 	/// Returns either a TransactionSet or a ProofStatus saying it failed to retrieve the set.
@@ -179,15 +177,15 @@ impl ScpMessageCollector {
 	/// * `slot` - the slot where the txset is  to get.
 	/// * `sender` - used to send messages to Stellar Node
 	pub async fn build_proof(&self, slot: Slot, sender: &StellarMessageSender) -> Option<Proof> {
-		let envelopes = self.get_envelopes(slot, sender).await;
+		let envelopes_maybe = self.get_envelopes(slot, sender).await;
 		// return early if we don't have enough envelopes or the tx_set
-		if envelopes.len() == 0 {
-			tracing::debug!("Couldn't built proof for slot {slot} due to missing envelopes");
+		if let Some(envelopes) = envelopes_maybe {
+			let tx_set = self.get_txset(slot, sender).await?;
+			return Some(Proof { slot, envelopes, tx_set })
+		} else {
+			tracing::debug!("Couldn't build proof for slot {slot} due to missing envelopes");
 			return None
 		}
-
-		let tx_set = self.get_txset(slot, sender).await?;
-		Some(Proof { slot, envelopes, tx_set })
 	}
 
 	/// Insert envelopes fetched from the archive to the map

clients/vault/src/replace.rs

@@ -248,47 +248,54 @@ mod tests {
 	}
 
 	mockall::mock! {
-	Provider {}
-
-	#[async_trait]
-	pub trait VaultRegistryPallet {
-		async fn get_vault(&self, vault_id: &VaultId) -> Result<SpacewalkVault, RuntimeError>;
-		async fn get_vaults_by_account_id(&self, account_id: &AccountId) -> Result<Vec<VaultId>, RuntimeError>;
-		async fn get_all_vaults(&self) -> Result<Vec<SpacewalkVault>, RuntimeError>;
-		async fn register_vault(&self, vault_id: &VaultId, collateral: u128) -> Result<(), RuntimeError>;
-		async fn deposit_collateral(&self, vault_id: &VaultId, amount: u128) -> Result<(), RuntimeError>;
-		async fn withdraw_collateral(&self, vault_id: &VaultId, amount: u128) -> Result<(), RuntimeError>;
-		async fn get_public_key(&self) -> Result<Option<StellarPublicKeyRaw>, RuntimeError>;
-		async fn register_public_key(&self, public_key: StellarPublicKeyRaw) -> Result<(), RuntimeError>;
-		async fn get_required_collateral_for_wrapped(&self, amount: u128, wrapped_currency: CurrencyId, collateral_currency: CurrencyId) -> Result<u128, RuntimeError>;
-		async fn get_required_collateral_for_vault(&self, vault_id: VaultId) -> Result<u128, RuntimeError>;
-		async fn get_vault_total_collateral(&self, vault_id: VaultId) -> Result<u128, RuntimeError>;
-		async fn get_collateralization_from_vault(&self, vault_id: VaultId, only_issued: bool) -> Result<u128, RuntimeError>;
-	}
+		Provider {}
+
+		#[async_trait]
+		pub trait VaultRegistryPallet {
+			async fn get_vault(&self, vault_id: &VaultId) -> Result<SpacewalkVault, RuntimeError>;
+			async fn get_vaults_by_account_id(&self, account_id: &AccountId) -> Result<Vec<VaultId>, RuntimeError>;
+			async fn get_all_vaults(&self) -> Result<Vec<SpacewalkVault>, RuntimeError>;
+			async fn register_vault(&self, vault_id: &VaultId, collateral: u128) -> Result<(), RuntimeError>;
+			async fn deposit_collateral(&self, vault_id: &VaultId, amount: u128) -> Result<(), RuntimeError>;
+			async fn withdraw_collateral(&self, vault_id: &VaultId, amount: u128) -> Result<(), RuntimeError>;
+			async fn get_public_key(&self) -> Result<Option<StellarPublicKeyRaw>, RuntimeError>;
+			async fn register_public_key(&self, public_key: StellarPublicKeyRaw) -> Result<(), RuntimeError>;
+			async fn get_required_collateral_for_wrapped(&self, amount: u128, wrapped_currency: CurrencyId, collateral_currency: CurrencyId) -> Result<u128, RuntimeError>;
+			async fn get_required_collateral_for_vault(&self, vault_id: VaultId) -> Result<u128, RuntimeError>;
+			async fn get_vault_total_collateral(&self, vault_id: VaultId) -> Result<u128, RuntimeError>;
+			async fn get_collateralization_from_vault(&self, vault_id: VaultId, only_issued: bool) -> Result<u128, RuntimeError>;
+		}
 
-	#[async_trait]
-	pub trait ReplacePallet {
-		async fn request_replace(&self, vault_id: &VaultId, amount: u128) -> Result<(), RuntimeError>;
-		async fn withdraw_replace(&self, vault_id: &VaultId, amount: u128) -> Result<(), RuntimeError>;
-		async fn accept_replace(&self, new_vault: &VaultId, old_vault: &VaultId, amount: u128, collateral: u128, stellar_address: StellarPublicKeyRaw) -> Result<(), RuntimeError>;
-		async fn execute_replace(&self, replace_id: H256, tx_env: &[u8], scp_envs: &[u8], tx_set: &[u8]) -> Result<(), RuntimeError>;
-		async fn cancel_replace(&self, replace_id: H256) -> Result<(), RuntimeError>;
-		async fn get_new_vault_replace_requests(&self, account_id: AccountId) -> Result<Vec<(H256, SpacewalkReplaceRequest)>, RuntimeError>;
-		async fn get_old_vault_replace_requests(&self, account_id: AccountId) -> Result<Vec<(H256, SpacewalkReplaceRequest)>, RuntimeError>;
-		async fn get_replace_period(&self) -> Result<u32, RuntimeError>;
-		async fn get_replace_request(&self, replace_id: H256) -> Result<SpacewalkReplaceRequest, RuntimeError>;
-		async fn get_replace_dust_amount(&self) -> Result<u128, RuntimeError>;
-	}
 
 
-	#[async_trait]
-	pub trait CollateralBalancesPallet {
-		async fn get_free_balance(&self, currency_id: CurrencyId) -> Result<Balance, RuntimeError>;
-		async fn get_native_balance_for_id(&self, id: &AccountId) -> Result<Balance, RuntimeError>;
-		async fn get_free_balance_for_id(&self, id: AccountId, currency_id: CurrencyId) -> Result<Balance, RuntimeError>;
-		async fn get_reserved_balance(&self, currency_id: CurrencyId) -> Result<Balance, RuntimeError>;
-		async fn get_reserved_balance_for_id(&self, id: AccountId, currency_id: CurrencyId) -> Result<Balance, RuntimeError>;
-		async fn transfer_to(&self, recipient: &AccountId, amount: u128, currency_id: CurrencyId) -> Result<(), RuntimeError>;         }
+		#[async_trait]
+		pub trait ReplacePallet {
+			async fn request_replace(&self, vault_id: &VaultId, amount: u128) -> Result<(), RuntimeError>;
+			async fn withdraw_replace(&self, vault_id: &VaultId, amount: u128) -> Result<(), RuntimeError>;
+			async fn accept_replace(&self, new_vault: &VaultId, old_vault: &VaultId, amount: u128, collateral: u128, stellar_address: StellarPublicKeyRaw) -> Result<(), RuntimeError>;
+			async fn execute_replace(&self, replace_id: H256, tx_env: &[u8], scp_envs: &[u8], tx_set: &[u8]) -> Result<(), RuntimeError>;
+			async fn cancel_replace(&self, replace_id: H256) -> Result<(), RuntimeError>;
+			async fn get_new_vault_replace_requests(&self, account_id: AccountId) -> Result<Vec<(H256, SpacewalkReplaceRequest)>, RuntimeError>;
+			async fn get_old_vault_replace_requests<T: 'static>(
+				&self,
+				account_id: AccountId,
+				filter: Box<dyn Fn((H256, SpacewalkReplaceRequest)) -> Option<T> + Send>,
+			) -> Result<Vec<T>, RuntimeError>;
+			async fn get_replace_period(&self) -> Result<u32, RuntimeError>;
+			async fn get_replace_request(&self, replace_id: H256) -> Result<SpacewalkReplaceRequest, RuntimeError>;
+			async fn get_replace_dust_amount(&self) -> Result<u128, RuntimeError>;
+		}
+
+
+		#[async_trait]
+		pub trait CollateralBalancesPallet {
+			async fn get_free_balance(&self, currency_id: CurrencyId) -> Result<Balance, RuntimeError>;
+			async fn get_native_balance_for_id(&self, id: &AccountId) -> Result<Balance, RuntimeError>;
+			async fn get_free_balance_for_id(&self, id: AccountId, currency_id: CurrencyId) -> Result<Balance, RuntimeError>;
+			async fn get_reserved_balance(&self, currency_id: CurrencyId) -> Result<Balance, RuntimeError>;
+			async fn get_reserved_balance_for_id(&self, id: AccountId, currency_id: CurrencyId) -> Result<Balance, RuntimeError>;
+			async fn transfer_to(&self, recipient: &AccountId, amount: u128, currency_id: CurrencyId) -> Result<(), RuntimeError>;
+		}
 	}
 
 	impl Clone for MockProvider {