perf: reduce list_proposals instructions (#153)

This PR includes the first set of improvements for the `list_proposals`,
notable changes are:

- Adds the `SelectionFilter` trait with default implementations for
`And` and `Or` logic
- Changes some of the indexes to user `cbor` serialization instead
`candid`
- Instead of loading all proposals to memory, only the set that wll be
shown in the limit is loaded, the entire selection works with ids

Still to follow in a separate PR:

- Improve the performance of the sorting (created_dt, expiration_dt,
modification_dt)
- Improve the performance of the access control checks
- Update the remaining models to use cbor as well

As noted by the benchmarks by changing the load of proposals to only ids
and some of the common indexes to use cbor the total instructions
reduced by `66%`.

.vscode/extensions.json

@@ -1,3 +1,3 @@
 {
-  "recommendations": ["Vue.volar", "Vue.vscode-typescript-vue-plugin", "esbenp.prettier-vscode"]
+  "recommendations": ["Vue.volar", "esbenp.prettier-vscode"]
 }

Cargo.toml

@@ -55,5 +55,5 @@ sha2 = "0.10"
 syn = { version = "2.0", features = ["extra-traits", "full"] }
 thiserror = "1.0.48"
 time = { version = "0.3", features = ["formatting", "parsing"] }
-tokio = { version = "1.33.0", features = ["full"] }
+tokio = { version = "1.33.0" }
 uuid = { version = "1.4.1", features = ["serde", "v4"] }

canisters/wallet/api/src/proposal.rs

@@ -30,14 +30,14 @@ pub enum ProposalStatusDTO {
 
 #[derive(CandidType, Deserialize, Debug, Clone, PartialEq, Eq, Hash)]
 pub enum ProposalStatusCodeDTO {
-    Created,
-    Adopted,
-    Rejected,
-    Cancelled,
-    Scheduled,
-    Processing,
-    Completed,
-    Failed,
+    Created = 0,
+    Adopted = 1,
+    Rejected = 2,
+    Cancelled = 3,
+    Scheduled = 4,
+    Processing = 5,
+    Completed = 6,
+    Failed = 7,
 }
 
 #[derive(CandidType, Deserialize, Debug, Clone)]

canisters/wallet/impl/Cargo.toml

@@ -13,6 +13,9 @@ repository = "https://github.com/dfinity/orbit-wallet"
 crate-type = ["cdylib"]
 bench = false
 
+[features]
+canbench = ["canbench-rs"]
+
 [dependencies]
 anyhow = { workspace = true }
 async-trait = { workspace = true }
@@ -32,7 +35,7 @@ lazy_static = { workspace = true }
 num-bigint = { workspace = true }
 num-traits = { workspace = true }
 prometheus = { workspace = true }
-serde = { workspace = true }
+serde = { workspace = true, features = ["derive"] }
 serde_bytes = { workspace = true }
 serde_cbor = { workspace = true }
 sha2 = { workspace = true }

canisters/wallet/impl/canbench.yml

@@ -1,3 +1,3 @@
-build_cmd: cargo build --locked --release --target wasm32-unknown-unknown --features canbench-rs
+build_cmd: cargo build --locked --release --target wasm32-unknown-unknown --features canbench
 wasm_path: ../../../target/wasm32-unknown-unknown/release/wallet.wasm
 results_path: results.yml

canisters/wallet/impl/results.yml

@@ -1,20 +1,26 @@
 benches:
-  batch_insert_100_proposals:
+  repository_batch_insert_100_proposals:
     total:
-      instructions: 3051795057
+      instructions: 3107967048
       heap_increase: 2
-      stable_memory_increase: 898
+      stable_memory_increase: 1410
     scopes: {}
-  filter_all_proposals_by_default_filters:
+  repository_filter_all_proposal_ids_by_default_filters:
     total:
-      instructions: 4690513262
-      heap_increase: 10
+      instructions: 489667791
+      heap_increase: 0
       stable_memory_increase: 0
     scopes: {}
-  list_all_proposals:
+  repository_list_all_proposals:
     total:
-      instructions: 3308284274
+      instructions: 3300020748
       heap_increase: 8
       stable_memory_increase: 0
     scopes: {}
+  service_filter_all_proposals_with_default_filters:
+    total:
+      instructions: 336959888
+      heap_increase: 0
+      stable_memory_increase: 0
+    scopes: {}
 version: 0.1.1

canisters/wallet/impl/src/controllers/wallet.rs

@@ -17,7 +17,7 @@ use wallet_api::{
 };
 
 // Canister entrypoints for the controller.
-#[cfg(any(not(feature = "canbench-rs"), test))]
+#[cfg(any(not(feature = "canbench"), test))]
 #[ic_cdk_macros::init]
 async fn initialize(input: Option<WalletInstall>) {
     match input {
@@ -29,7 +29,7 @@ async fn initialize(input: Option<WalletInstall>) {
 /// The init is overriden for benchmarking purposes.
 ///
 /// This is only used for benchmarking and is not included in the final canister.
-#[cfg(feature = "canbench-rs")]
+#[cfg(all(feature = "canbench", not(test)))]
 #[ic_cdk_macros::init]
 pub async fn mock_init() {
     // Initialize the random number generator with a fixed seed to ensure deterministic
@@ -79,7 +79,7 @@ impl WalletController {
         Self { wallet_service }
     }
 
-    #[cfg(any(not(feature = "canbench-rs"), test))]
+    #[cfg(any(not(feature = "canbench"), test))]
     async fn initialize(&self, input: wallet_api::WalletInit) {
         let ctx = &call_context();
         self.wallet_service

canisters/wallet/impl/src/core/memory.rs

@@ -34,6 +34,10 @@ pub const ADDRESS_BOOK_MEMORY_ID: MemoryId = MemoryId::new(22);
 pub const ADDRESS_BOOK_INDEX_MEMORY_ID: MemoryId = MemoryId::new(23);
 pub const PROPOSAL_PROPOSER_INDEX_MEMORY_ID: MemoryId = MemoryId::new(24);
 pub const PROPOSAL_CREATION_TIME_INDEX_MEMORY_ID: MemoryId = MemoryId::new(25);
+pub const PROPOSAL_KEY_CREATION_TIME_INDEX_MEMORY_ID: MemoryId = MemoryId::new(26);
+pub const PROPOSAL_KEY_EXPIRATION_TIME_INDEX_MEMORY_ID: MemoryId = MemoryId::new(27);
+pub const PROPOSAL_SORT_INDEX_MEMORY_ID: MemoryId = MemoryId::new(28);
+pub const PROPOSAL_STATUS_MODIFICATION_INDEX_MEMORY_ID: MemoryId = MemoryId::new(29);
 
 thread_local! {
   /// Static configuration of the canister.

canisters/wallet/impl/src/core/utils.rs

@@ -81,18 +81,6 @@ pub fn calculate_minimum_threshold(percentage: &Percentage, total_value: &usize)
     }
 }
 
-/// Matches a date against a date range.
-///
-/// If the provided range is `None`, then the date is considered to be within the range.
-pub(crate) fn match_date_range(date: &u64, start_dt: &Option<u64>, to_dt: &Option<u64>) -> bool {
-    match (start_dt, to_dt) {
-        (Some(start_dt), Some(to_dt)) => date >= start_dt && date <= to_dt,
-        (Some(start_dt), None) => date >= start_dt,
-        (None, Some(to_dt)) => date <= to_dt,
-        (None, None) => true,
-    }
-}
-
 /// Retains items based on the result of an access control evaluation.
 ///
 /// This function will evaluate the access control for each item in the list and retain only the

canisters/wallet/impl/src/jobs/cancel_expired_proposals.rs

@@ -1,4 +1,8 @@
-use crate::{core::ic_cdk::api::time, models::ProposalStatus, repositories::ProposalRepository};
+use crate::{
+    core::ic_cdk::api::time,
+    models::{ProposalStatus, ProposalStatusCode},
+    repositories::ProposalRepository,
+};
 use async_trait::async_trait;
 use ic_canister_core::repository::Repository;
 
@@ -27,7 +31,7 @@ impl Job {
         let mut proposals = self.proposal_repository.find_by_expiration_dt_and_status(
             None,
             Some(current_time),
-            ProposalStatus::Created.to_string(),
+            ProposalStatusCode::Created.to_string(),
         );
 
         for proposal in proposals.iter_mut() {

canisters/wallet/impl/src/jobs/schedule_adopted_proposals.rs

@@ -1,6 +1,6 @@
 use crate::{
     core::ic_cdk::api::time,
-    models::{ProposalExecutionPlan, ProposalStatus},
+    models::{ProposalExecutionPlan, ProposalStatus, ProposalStatusCode},
     repositories::ProposalRepository,
 };
 use async_trait::async_trait;
@@ -34,7 +34,7 @@ impl Job {
     async fn process_adopted_proposals(&self) {
         let current_time = time();
         let mut proposals = self.proposal_repository.find_by_status(
-            ProposalStatus::Adopted.to_string(),
+            ProposalStatusCode::Adopted,
             None,
             Some(current_time),
         );

canisters/wallet/impl/src/mappers/account.rs

@@ -7,7 +7,7 @@ use crate::{
     },
     repositories::policy::PROPOSAL_POLICY_REPOSITORY,
 };
-use ic_canister_core::{repository::Repository, types::UUID, utils::timestamp_to_rfc3339};
+use ic_canister_core::{repository::Repository, utils::timestamp_to_rfc3339};
 use uuid::Uuid;
 use wallet_api::{AccountBalanceDTO, AccountBalanceInfoDTO, AccountDTO, CriteriaDTO};
 
@@ -47,7 +47,7 @@ impl AccountMapper {
 
     pub fn from_create_input(
         input: AddAccountOperationInput,
-        account_id: UUID,
+        account_id: AccountId,
         address: Option<String>,
     ) -> Result<Account, MapperError> {
         if !input

canisters/wallet/impl/src/mappers/proposal_status.rs

@@ -1,4 +1,4 @@
-use crate::models::ProposalStatus;
+use crate::models::{ProposalStatus, ProposalStatusCode};
 use ic_canister_core::utils::{rfc3339_to_timestamp, timestamp_to_rfc3339};
 use wallet_api::{ProposalStatusCodeDTO, ProposalStatusDTO};
 
@@ -59,6 +59,21 @@ impl From<ProposalStatusDTO> for ProposalStatus {
     }
 }
 
+impl From<ProposalStatusCodeDTO> for ProposalStatusCode {
+    fn from(status: ProposalStatusCodeDTO) -> Self {
+        match status {
+            ProposalStatusCodeDTO::Created => ProposalStatusCode::Created,
+            ProposalStatusCodeDTO::Adopted => ProposalStatusCode::Adopted,
+            ProposalStatusCodeDTO::Rejected => ProposalStatusCode::Rejected,
+            ProposalStatusCodeDTO::Completed => ProposalStatusCode::Completed,
+            ProposalStatusCodeDTO::Failed => ProposalStatusCode::Failed,
+            ProposalStatusCodeDTO::Processing => ProposalStatusCode::Processing,
+            ProposalStatusCodeDTO::Scheduled => ProposalStatusCode::Scheduled,
+            ProposalStatusCodeDTO::Cancelled => ProposalStatusCode::Cancelled,
+        }
+    }
+}
+
 #[derive(Debug)]
 pub struct ProposalStatusMapper;
 

canisters/wallet/impl/src/models/access_control.rs

@@ -414,13 +414,14 @@ mod tests {
     }
 }
 
-#[cfg(test)]
+#[cfg(any(test, feature = "canbench"))]
 pub mod access_control_test_utils {
     use super::*;
+    use uuid::Uuid;
 
     pub fn mock_access_policy() -> AccessControlPolicy {
         AccessControlPolicy {
-            id: [0; 16],
+            id: *Uuid::new_v4().as_bytes(),
             user: UserSpecifier::Any,
             resource: ResourceSpecifier::Common(
                 ResourceType::Account,

canisters/wallet/impl/src/models/account.rs

@@ -291,10 +291,11 @@ pub mod account_test_utils {
     use super::*;
     use crate::repositories::ACCOUNT_REPOSITORY;
     use ic_canister_core::repository::Repository;
+    use uuid::Uuid;
 
     pub fn mock_account() -> Account {
         Account {
-            id: [0; 16],
+            id: *Uuid::new_v4().as_bytes(),
             address: "0x1234".to_string(),
             balance: None,
             blockchain: Blockchain::InternetComputer,

canisters/wallet/impl/src/models/indexes/mod.rs

@@ -6,9 +6,13 @@ pub mod notification_user_index;
 pub mod proposal_account_index;
 pub mod proposal_creation_time_index;
 pub mod proposal_expiration_time_index;
+pub mod proposal_key_creation_time_index;
+pub mod proposal_key_expiration_time_index;
 pub mod proposal_proposer_index;
 pub mod proposal_scheduled_index;
+pub mod proposal_sort_index;
 pub mod proposal_status_index;
+pub mod proposal_status_modification_index;
 pub mod proposal_voter_index;
 pub mod transfer_account_index;
 pub mod transfer_status_index;

canisters/wallet/impl/src/models/indexes/proposal_account_index.rs

@@ -1,6 +1,5 @@
 use crate::models::{AccountId, Proposal, ProposalId, ProposalOperation};
 use candid::{CandidType, Deserialize};
-use ic_canister_core::types::Timestamp;
 use ic_canister_macros::stable_object;
 
 /// Index of proposals by account id.
@@ -9,25 +8,20 @@ use ic_canister_macros::stable_object;
 pub struct ProposalAccountIndex {
     /// The account id that is associated with this proposal.
     pub account_id: AccountId,
-    /// The time when the proposal was created.
-    pub created_at: Timestamp,
     /// The proposal id, which is a UUID.
     pub proposal_id: ProposalId,
 }
 
 #[derive(Clone, Debug)]
 pub struct ProposalAccountIndexCriteria {
     pub account_id: AccountId,
-    pub from_dt: Option<Timestamp>,
-    pub to_dt: Option<Timestamp>,
 }
 
 impl Proposal {
     pub fn to_index_for_account(&self) -> Option<ProposalAccountIndex> {
         if let ProposalOperation::Transfer(ctx) = &self.operation {
             return Some(ProposalAccountIndex {
                 proposal_id: self.id.to_owned(),
-                created_at: self.created_timestamp.to_owned(),
                 account_id: ctx.input.from_account_id.to_owned(),
             });
         }
@@ -50,9 +44,8 @@ mod tests {
         let account_id = [0; 16];
         let proposal_id = [1; 16];
         let model = ProposalAccountIndex {
-            proposal_id,
             account_id,
-            created_at: 0,
+            proposal_id,
         };
 
         let serialized_model = model.to_bytes();

canisters/wallet/impl/src/models/indexes/proposal_creation_time_index.rs

@@ -1,12 +1,11 @@
 use crate::models::Proposal;
-use candid::{CandidType, Deserialize};
 use ic_canister_core::types::{Timestamp, UUID};
-use ic_canister_macros::stable_object;
+use ic_canister_macros::storable;
 use std::hash::Hash;
 
-/// Represents a proposal index by execution time.
-#[stable_object]
-#[derive(CandidType, Deserialize, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
+/// Represents a proposal index by creation time.
+#[storable]
+#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct ProposalCreationTimeIndex {
     /// The time the proposal was created.
     pub created_at: Timestamp,

canisters/wallet/impl/src/models/indexes/proposal_expiration_time_index.rs

@@ -4,11 +4,11 @@ use ic_canister_core::types::{Timestamp, UUID};
 use ic_canister_macros::stable_object;
 use std::hash::Hash;
 
-/// Represents a proposal index by execution time.
+/// Represents a proposal index by expiration time.
 #[stable_object]
 #[derive(CandidType, Deserialize, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct ProposalExpirationTimeIndex {
-    /// The time the proposal is scheduled to be set as expired if not executed.
+    /// The time the proposal is scheduled to be set as expired if still pending.
     pub expiration_dt: Timestamp,
     /// The proposal id, which is a UUID.
     pub proposal_id: UUID,