Optimised delete jar method

sweatco · Oct 18, 2023 · a4c608d · a4c608d
1 parent 2e0a0eb
commit a4c608d
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 25 deletions.
diff --git a/contract/src/claim/api.rs b/contract/src/claim/api.rs
@@ -145,7 +145,7 @@ impl Contract {
 
                 if let Some(ref cache) = jar.cache {
                     if cache.interest == 0 && jar.principal == 0 {
-                        self.delete_jar(jar_before_transfer);
+                        self.delete_jar(&jar_before_transfer.account_id, jar_before_transfer.id);
                     }
                 }
             }

diff --git a/contract/src/jar/api.rs b/contract/src/jar/api.rs
@@ -203,7 +203,7 @@ impl JarApi for Contract {
         let (should_be_closed, withdraw_jar) = jar.withdrawn(product, principal, now);
 
         if should_be_closed {
-            self.delete_jar(withdraw_jar);
+            self.delete_jar(&withdraw_jar.account_id, withdraw_jar.id);
         } else {
             let jar_id = withdraw_jar.id;
             *self.get_jar_mut_internal(&account_id, jar_id) = withdraw_jar;

diff --git a/contract/src/jar/model.rs b/contract/src/jar/model.rs
@@ -278,40 +278,29 @@ impl Contract {
         U128(principal)
     }
 
-    pub(crate) fn delete_jar(&mut self, jar: Jar) {
-        let account = &jar.account_id;
-
+    pub(crate) fn delete_jar(&mut self, account_id: &AccountId, jar_id: JarId) {
         let jars = self
             .account_jars
-            .get_mut(account)
-            .unwrap_or_else(|| env::panic_str(&format!("Account '{account}' doesn't exist")));
-
-        require!(!jars.is_empty(), "Trying to delete jar from empty account");
-
-        if jars.len() == 1 {
-            jars.clear();
-            return;
-        }
+            .get_mut(account_id)
+            .unwrap_or_else(|| panic_str(&format!("Account '{account_id}' doesn't exist")));
 
-        // On jar deletion, we move the last jar in the vector in the deleted jar's place.
-        // This way we don't need to shift all jars to fill empty space in the vector.
+        require!(
+            !jars.is_empty(),
+            "Trying to delete a jar from account without any jars."
+        );
 
         let jar_position = jars
             .iter()
-            .position(|j| j.id == jar.id)
-            .unwrap_or_else(|| env::panic_str(&format!("Jar with id {} doesn't exist", jar.id)));
-
-        let last_jar = jars.pop().unwrap();
+            .position(|j| j.id == jar_id)
+            .unwrap_or_else(|| panic_str(&format!("Jar with id {jar_id} doesn't exist")));
 
-        if jar_position != jars.len() {
-            jars[jar_position] = last_jar;
-        }
+        jars.swap_remove(jar_position);
     }
 
     pub(crate) fn get_jar_mut_internal(&mut self, account: &AccountId, id: JarId) -> &mut Jar {
         self.account_jars
             .get_mut(account)
-            .unwrap_or_else(|| env::panic_str(&format!("Account '{account}' doesn't exist")))
+            .unwrap_or_else(|| panic_str(&format!("Account '{account}' doesn't exist")))
             .get_jar_mut(id)
     }
 

diff --git a/contract/src/withdraw/api.rs b/contract/src/withdraw/api.rs
@@ -91,7 +91,7 @@ impl Contract {
     ) -> WithdrawView {
         if is_promise_success {
             if close_jar {
-                self.delete_jar(original_jar.clone());
+                self.delete_jar(&original_jar.account_id, original_jar.id);
             } else {
                 self.get_jar_mut_internal(&original_jar.account_id, original_jar.id)
                     .unlock();