From 88eb17310d4268fe4aafff5f8f3c89b52777951e Mon Sep 17 00:00:00 2001
From: Hugo Rosenkranz-Costa <hugo.rosenkranz@cosmian.com>
Date: Fri, 12 Jan 2024 13:32:48 +0100
Subject: [PATCH] refacto: remove clones and clean function signatures

---
 src/core/api.rs               |  5 ++---
 src/core/primitives.rs        | 13 ++++++++-----
 src/data_struct/README.md     |  6 ------
 src/data_struct/dictionary.rs |  6 +++---
 src/test_utils/mod.rs         |  2 +-
 5 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/src/core/api.rs b/src/core/api.rs
index 0a7d5a23..09d5f028 100644
--- a/src/core/api.rs
+++ b/src/core/api.rs
@@ -100,7 +100,7 @@ impl Covercrypt {
     ///  - `access_policy`  : describe the keys to prune
     ///  - `policy`         : global policy
     ///  - `msk`            : master secret key
-    pub fn prune_master_keys(
+    pub fn prune_master_secret_key(
         &self,
         access_policy: &AccessPolicy,
         policy: &Policy,
@@ -141,8 +141,7 @@ impl Covercrypt {
     ///
     /// - `usk`                 : the user key to refresh
     /// - `msk`                 : master secret key
-    /// - `keep_old_accesses`   : whether access to old partitions (i.e. before
-    ///   rotation) should be kept
+    /// - `keep_old_rights`     : whether or not to keep old decryption rights
     pub fn refresh_user_secret_key(
         &self,
         usk: &mut UserSecretKey,
diff --git a/src/core/primitives.rs b/src/core/primitives.rs
index 3b0289a1..a0f3ff80 100644
--- a/src/core/primitives.rs
+++ b/src/core/primitives.rs
@@ -481,19 +481,22 @@ pub fn refresh(
                 let first_usk_subkey = usk_subkeys.next()?;
 
                 let mut new_usk_subkeys = LinkedList::new();
+                // Add new master secret subkeys
                 for msk_subkey in msk_subkeys.by_ref() {
-                    new_usk_subkeys.push_back(msk_subkey.clone());
                     if msk_subkey == &first_usk_subkey {
+                        new_usk_subkeys.push_back(first_usk_subkey);
                         break;
                     }
+                    new_usk_subkeys.push_back(msk_subkey.clone());
                 }
-                for next_usk_subkey in usk_subkeys {
-                    if Some(&next_usk_subkey) != msk_subkeys.next() {
+                // Keep old matching subkeys between the master and user subkeys
+                for subkey in usk_subkeys {
+                    if Some(&subkey) != msk_subkeys.next() {
                         break;
                     }
-                    new_usk_subkeys.push_back(next_usk_subkey);
+                    new_usk_subkeys.push_back(subkey);
                 }
-                Some((coordinate.clone(), new_usk_subkeys))
+                Some((coordinate, new_usk_subkeys))
             })
         })
         .collect::<RevisionVec<_, _>>();
diff --git a/src/data_struct/README.md b/src/data_struct/README.md
index a0792180..2c68c2c6 100644
--- a/src/data_struct/README.md
+++ b/src/data_struct/README.md
@@ -73,9 +73,3 @@ Cons:
 - following linked list pointers can be slower than iterating a regular vector
 
 - serialization requires following each linked list
-
-## Operations
-
-### Master Secret Key
-
-### User Secret Key
diff --git a/src/data_struct/dictionary.rs b/src/data_struct/dictionary.rs
index d491054a..a3577bae 100644
--- a/src/data_struct/dictionary.rs
+++ b/src/data_struct/dictionary.rs
@@ -4,6 +4,7 @@ use std::{
     fmt::{self, Debug},
     hash::Hash,
     marker::PhantomData,
+    mem::swap,
     usize,
 };
 
@@ -92,7 +93,7 @@ where
     }
 
     /// Updates the key for a given entry while retaining the current order.
-    pub fn update_key(&mut self, old_key: &K, new_key: K) -> Result<(), Error> {
+    pub fn update_key(&mut self, old_key: &K, mut new_key: K) -> Result<(), Error> {
         // Get index from old_key
         let index_entry = *self
             .indices
@@ -107,8 +108,7 @@ where
                 // Remove old key from indices
                 let _ = self.indices.remove(old_key);
                 // Replace old_key with new_key inside entries
-                let replaced_key = std::mem::replace(&mut self.entries[index_entry].0, new_key);
-                assert_eq!(&replaced_key, old_key);
+                swap(&mut self.entries[index_entry].0, &mut new_key);
                 Ok(())
             }
         }
diff --git a/src/test_utils/mod.rs b/src/test_utils/mod.rs
index 61673628..58e22fad 100644
--- a/src/test_utils/mod.rs
+++ b/src/test_utils/mod.rs
@@ -89,7 +89,7 @@ mod tests {
         assert_eq!(msk.subkeys.count_elements(), 25);
 
         // remove older subkeys for `Department::FIN`
-        cover_crypt.prune_master_keys(&rekey_access_policy, &policy, &mut msk)?;
+        cover_crypt.prune_master_secret_key(&rekey_access_policy, &policy, &mut msk)?;
         assert_eq!(mpk.subkeys.len(), 20);
         // we only keep the last subkeys in the secret key
         assert_eq!(msk.subkeys.count_elements(), 20);