Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

263 add optimizations audit recommendations #392

Merged
merged 25 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3046062
SYS-04 Double Iterations can be merged into a single one
gianfra-t Aug 30, 2023
3194a81
SRL-02/SRW-02 limit parameter type optimization. get_latest_transacti…
gianfra-t Aug 30, 2023
e2c99ab
RPC-01/ Closure usage could simplify the codebase
gianfra-t Aug 31, 2023
640671c
PRO-02/PRF-02 Return Type Could be an Option
gianfra-t Aug 31, 2023
d05cfbc
LIV-01 Duplicated Helper Function Call
gianfra-t Aug 31, 2023
facd26b
LIP-04/LI5-04 Potential Unnecessary Computations
gianfra-t Aug 31, 2023
4c5bee2
LIP-03/LI5-03 Redundant Condition Check
gianfra-t Aug 31, 2023
b48b127
Rollback on LIV-01, try_deposit_collateral is also used in other pall…
gianfra-t Aug 31, 2023
090c314
LII-02/LBS-01 Duplicated Condition Check
gianfra-t Aug 31, 2023
c0abe50
4FC-06/9B2-06 Loops optimizations with iterators
gianfra-t Aug 31, 2023
d794f37
refactor for RPC-01
gianfra-t Sep 1, 2023
0cc62fa
Fix weird indentation within macro
ebma Sep 1, 2023
9298605
Replace `get_old_vault_replace_requests_filter`
ebma Sep 1, 2023
0f72231
Fix mock type declaration issues
ebma Sep 1, 2023
689e48f
Change signature of `get_vault_redeem_requests()`
ebma Sep 1, 2023
3c99b7b
RPC-01 fixed mock issue by adding constrained trait object, functions…
gianfra-t Sep 1, 2023
1ba4241
Fix (potential) bug introduced before in 4FC-06/9B2-06. mut requests …
gianfra-t Sep 4, 2023
0ccf8e5
Refactor RPC-01. Remove the need for 2 into_iter() calls
gianfra-t Sep 4, 2023
215c08d
Refactor PRO-02/PRF-02. Also return None when building fails.
gianfra-t Sep 4, 2023
ba506f0
FIX issue introduced by LII-02/LBS-01. Second check is needed.
gianfra-t Sep 4, 2023
28cbeb4
Refactor on LIP-04/LI5-04.
gianfra-t Sep 4, 2023
35dc5eb
Update pallets/vault-registry/src/lib.rs
gianfra-t Sep 4, 2023
8d70c49
Update pallets/vault-registry/src/lib.rs
gianfra-t Sep 4, 2023
5a6b56b
format
gianfra-t Sep 5, 2023
afdc423
clippy changes
gianfra-t Sep 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 36 additions & 23 deletions clients/runtime/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1179,24 +1179,29 @@ impl RedeemPallet for SpacewalkParachain {
.request("redeem_getVaultRedeemRequests", rpc_params![account_id, head])
.await?;

let redeem_requests: Result<Vec<(H256, SpacewalkRedeemRequest)>, Error> = join_all(
let redeem_requests = join_all(
result.into_iter().map(|key| async move {
self.get_redeem_request(key).await.map(|value| (key, value))
})
)
.await
.into_iter()
.collect();

match redeem_requests {
Ok(redeem_requests) => {
let filtered_results: Vec<_> = redeem_requests
.into_iter()
.filter_map(|item| filter(item))
.collect();
Ok(filtered_results)
}
Err(e) => Err(e),
.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),
}
}

Expand Down Expand Up @@ -1382,21 +1387,29 @@ impl ReplacePallet for SpacewalkParachain {
.request("replace_getOldVaultReplaceRequests", rpc_params![account_id, head])
.await?;

let redeem_requests: Result<Vec<(H256, SpacewalkReplaceRequest)>, Error> =
let replace_requests =
join_all(result.into_iter().map(|key| async move {
self.get_replace_request(key).await.map(|value| (key, value))
}))
.await
.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 redeem_requests {
Ok(redeem_requests) => {
let filtered_results: Vec<_> =
redeem_requests.into_iter().filter_map(|item| filter(item)).collect();
Ok(filtered_results)
},
Err(e) => Err(e),
match some_error {
Some(err) => Err(err),
None => Ok(filtered_results),
}

}
Expand Down
13 changes: 12 additions & 1 deletion clients/vault/src/cancellation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,18 @@ 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> {

requests.drain(..).filter(|active_req| current_height > active_req.parachain_deadline_height).collect()
let mut expired = Vec::new();
gianfra-t marked this conversation as resolved.
Show resolved Hide resolved

requests.retain(|req| {
if current_height > req.parachain_deadline_height {
expired.push(req.clone());
false
} else {
true
}
});

expired

}

Expand Down
3 changes: 1 addition & 2 deletions clients/vault/src/oracle/collector/proof_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ impl ScpMessageCollector {
slot: Slot,
sender: &StellarMessageSender,
) -> Option<UnlimitedVarArray<ScpEnvelope>> {
let empty = UnlimitedVarArray::new_empty();

if let Some(envelopes) = self.envelopes_map().get(&slot) {
// lacking envelopes
Expand All @@ -124,7 +123,7 @@ impl ScpMessageCollector {
"Proof Building for slot {slot}: not enough envelopes to build proof "
);
} else {
return Some(UnlimitedVarArray::new(envelopes.clone()).unwrap_or(empty))
return UnlimitedVarArray::new(envelopes.clone()).ok()
}
}

Expand Down
6 changes: 4 additions & 2 deletions pallets/issue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use currency::Amount;
pub use default_weights::{SubstrateWeight, WeightInfo};
pub use pallet::*;
use types::IssueRequestExt;
use vault_registry::{CurrencySource};
use vault_registry::{CurrencySource, VaultStatus};

use crate::types::{BalanceOf, CurrencyId, DefaultVaultId};
#[doc(inline)]
Expand Down Expand Up @@ -388,7 +388,9 @@ impl<T: Config> Pallet<T> {
Self::check_volume(amount_requested.clone())?;

// ensure that the vault is accepting new issues (vault is active)
let _vault = ext::vault_registry::get_active_vault_from_id::<T>(&vault_id).map_err(|_| Error::<T>::VaultNotAcceptingNewIssues)?;
let vault = ext::vault_registry::get_active_vault_from_id::<T>(&vault_id)?;

ensure!(vault.status == VaultStatus::Active(true), Error::<T>::VaultNotAcceptingNewIssues);

// Check that the vault is currently not banned
ext::vault_registry::ensure_not_banned::<T>(&vault_id)?;
Expand Down
2 changes: 0 additions & 2 deletions pallets/vault-registry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ pub mod pallet {
let vault = Self::get_active_rich_vault_from_id(&vault_id)?;

let amount = Amount::new(amount, currency_pair.collateral);

Self::try_deposit_collateral(&vault_id, &amount)?;

Self::deposit_event(Event::<T>::DepositCollateral {
Expand Down Expand Up @@ -911,7 +910,6 @@ impl<T: Config> Pallet<T> {
vault_id: &DefaultVaultId<T>,
amount: &Amount<T>,
) -> DispatchResult {

// ensure the vault is active
let _vault = Self::get_active_rich_vault_from_id(vault_id)?;

Expand Down
8 changes: 6 additions & 2 deletions primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -849,10 +849,14 @@ impl TransactionEnvelopeExt for TransactionEnvelope {
let tx_operations: Vec<Operation> = match self {
TransactionEnvelope::EnvelopeTypeTxV0(env) => env.tx.operations.get_vec().clone(),
TransactionEnvelope::EnvelopeTypeTx(env) => env.tx.operations.get_vec().clone(),
TransactionEnvelope::EnvelopeTypeTxFeeBump(_) => return BalanceConversion::unlookup(transferred_amount),
TransactionEnvelope::Default(_) => return BalanceConversion::unlookup(transferred_amount),
TransactionEnvelope::EnvelopeTypeTxFeeBump(_) => Vec::new(),
TransactionEnvelope::Default(_) => Vec::new(),
};

if tx_operations.len() == 0 {
return BalanceConversion::unlookup(transferred_amount);
}

transferred_amount = tx_operations
.iter()
.fold(0i64, |acc, x| {
Expand Down