chore: add ConflictPoolOp trait for common pool operations

Signed-off-by: bsbds <[email protected]>

crates/curp-external-api/src/conflict.rs

@@ -1,21 +1,10 @@
 #![allow(clippy::module_name_repetitions)]
 
-/// Insert into speculative pool
-pub trait SpeculativePoolOp {
+/// Common operations for conflict pools
+pub trait ConflictPoolOp {
     /// Entry of the pool
     type Entry;
 
-    /// Inserts a command in to the pool
-    ///
-    /// Returns the entry if a conflict is detected
-    fn insert_if_not_conflict(&mut self, entry: Self::Entry) -> Option<Self::Entry>;
-
-    /// Returns the number of commands in the pool
-    fn len(&self) -> usize;
-
-    /// Checks if the pool contains some commands that will conflict with all other commands
-    fn is_empty(&self) -> bool;
-
     /// Removes a command from the pool
     fn remove(&mut self, entry: Self::Entry);
 
@@ -24,32 +13,29 @@ pub trait SpeculativePoolOp {
 
     /// Clears all entries in the pool
     fn clear(&mut self);
+
+    /// Returns the number of commands in the pool
+    fn len(&self) -> usize;
+
+    /// Checks if the pool contains some commands that will conflict with all other commands
+    fn is_empty(&self) -> bool;
+}
+
+/// Insert into speculative pool
+pub trait SpeculativePoolOp: ConflictPoolOp {
+    /// Inserts a command in to the pool
+    ///
+    /// Returns the entry if a conflict is detected
+    fn insert_if_not_conflict(&mut self, entry: Self::Entry) -> Option<Self::Entry>;
 }
 
 /// Insert into uncommitted pool
-pub trait UncommittedPoolOp {
-    /// Entry of the pool
-    type Entry;
+pub trait UncommittedPoolOp: ConflictPoolOp {
     /// Inserts a command in to the pool
     ///
     /// Returns `true` if a conflict is detected
     fn insert(&mut self, entry: Self::Entry) -> bool;
 
     /// Returns all commands in the pool that conflicts with the given command
     fn all_conflict(&self, entry: &Self::Entry) -> Vec<Self::Entry>;
-
-    /// Returns all commands in the pool
-    fn all(&self) -> Vec<Self::Entry>;
-
-    /// Returns the number of commands in the pool
-    fn len(&self) -> usize;
-
-    /// Checks if the pool will conflict with all commands
-    fn is_empty(&self) -> bool;
-
-    /// Removes a command from the pool
-    fn remove(&mut self, entry: Self::Entry);
-
-    /// Clears all entries in the pool
-    fn clear(&mut self);
 }

crates/curp/src/server/conflict/spec_pool_new.rs

@@ -1,4 +1,4 @@
-use curp_external_api::conflict::SpeculativePoolOp;
+use curp_external_api::conflict::{ConflictPoolOp, SpeculativePoolOp};
 
 use crate::rpc::PoolEntry;
 
@@ -43,7 +43,7 @@ impl<C> SpeculativePool<C> {
                     .command_sps
                     .iter()
                     .map(AsRef::as_ref)
-                    .all(SpeculativePoolOp::is_empty)
+                    .all(ConflictPoolOp::is_empty)
                 {
                     return Some(c.into());
                 }
@@ -96,17 +96,9 @@ struct ConfChangeSp {
     change: Option<ConfChangeEntry>,
 }
 
-impl SpeculativePoolOp for ConfChangeSp {
+impl ConflictPoolOp for ConfChangeSp {
     type Entry = ConfChangeEntry;
 
-    fn insert_if_not_conflict(&mut self, entry: Self::Entry) -> Option<Self::Entry> {
-        if self.change.is_some() {
-            return Some(entry);
-        }
-        self.change = Some(entry);
-        None
-    }
-
     fn is_empty(&self) -> bool {
         self.change.is_none()
     }
@@ -127,3 +119,13 @@ impl SpeculativePoolOp for ConfChangeSp {
         self.change.iter().count()
     }
 }
+
+impl SpeculativePoolOp for ConfChangeSp {
+    fn insert_if_not_conflict(&mut self, entry: Self::Entry) -> Option<Self::Entry> {
+        if self.change.is_some() {
+            return Some(entry);
+        }
+        self.change = Some(entry);
+        None
+    }
+}

crates/curp/src/server/conflict/tests.rs

@@ -1,6 +1,6 @@
 use std::{cmp::Ordering, sync::Arc};
 
-use curp_external_api::conflict::{SpeculativePoolOp, UncommittedPoolOp};
+use curp_external_api::conflict::{ConflictPoolOp, SpeculativePoolOp, UncommittedPoolOp};
 
 use crate::{
     rpc::{ConfChange, PoolEntry, PoolEntryInner, ProposeId},
@@ -14,17 +14,9 @@ struct TestSp {
     entries: Vec<CommandEntry<i32>>,
 }
 
-impl SpeculativePoolOp for TestSp {
+impl ConflictPoolOp for TestSp {
     type Entry = CommandEntry<i32>;
 
-    fn insert_if_not_conflict(&mut self, entry: Self::Entry) -> Option<Self::Entry> {
-        if self.entries.iter().any(|e| e.as_ref() == entry.as_ref()) {
-            return Some(entry);
-        }
-        self.entries.push(entry);
-        None
-    }
-
     fn len(&self) -> usize {
         self.entries.len()
     }
@@ -52,27 +44,24 @@ impl SpeculativePoolOp for TestSp {
     }
 }
 
+impl SpeculativePoolOp for TestSp {
+    fn insert_if_not_conflict(&mut self, entry: Self::Entry) -> Option<Self::Entry> {
+        if self.entries.iter().any(|e| e.as_ref() == entry.as_ref()) {
+            return Some(entry);
+        }
+        self.entries.push(entry);
+        None
+    }
+}
+
 #[derive(Debug, Default)]
 struct TestUcp {
     entries: Vec<CommandEntry<i32>>,
 }
 
-impl UncommittedPoolOp for TestUcp {
+impl ConflictPoolOp for TestUcp {
     type Entry = CommandEntry<i32>;
 
-    fn insert(&mut self, entry: Self::Entry) -> bool {
-        let conflict = self.entries.iter().any(|e| e.as_ref() == entry.as_ref());
-        self.entries.push(entry);
-        conflict
-    }
-
-    fn all_conflict(&self, entry: &Self::Entry) -> Vec<Self::Entry> {
-        self.entries
-            .iter()
-            .filter_map(|e| (e.as_ref() == entry.as_ref()).then_some(e.clone()))
-            .collect()
-    }
-
     fn all(&self) -> Vec<Self::Entry> {
         self.entries.clone()
     }
@@ -100,6 +89,21 @@ impl UncommittedPoolOp for TestUcp {
     }
 }
 
+impl UncommittedPoolOp for TestUcp {
+    fn insert(&mut self, entry: Self::Entry) -> bool {
+        let conflict = self.entries.iter().any(|e| e.as_ref() == entry.as_ref());
+        self.entries.push(entry);
+        conflict
+    }
+
+    fn all_conflict(&self, entry: &Self::Entry) -> Vec<Self::Entry> {
+        self.entries
+            .iter()
+            .filter_map(|e| (e.as_ref() == entry.as_ref()).then_some(e.clone()))
+            .collect()
+    }
+}
+
 impl Eq for PoolEntry<i32> {}
 
 impl PartialOrd for PoolEntry<i32> {

crates/curp/src/server/conflict/uncommitted_pool.rs

@@ -1,4 +1,4 @@
-use curp_external_api::conflict::UncommittedPoolOp;
+use curp_external_api::conflict::{ConflictPoolOp, UncommittedPoolOp};
 
 use crate::rpc::PoolEntry;
 
@@ -42,7 +42,7 @@ impl<C> UncommittedPool<C> {
                     .command_ucps
                     .iter()
                     .map(AsRef::as_ref)
-                    .all(UncommittedPoolOp::is_empty);
+                    .all(ConflictPoolOp::is_empty);
             }
         }
 
@@ -89,7 +89,7 @@ impl<C> UncommittedPool<C> {
                     self.command_ucps
                         .iter()
                         .map(AsRef::as_ref)
-                        .flat_map(UncommittedPoolOp::all)
+                        .flat_map(ConflictPoolOp::all)
                         .map(Into::into),
                 )
                 .collect(),
@@ -129,15 +129,9 @@ struct ConfChangeUcp {
     conf_changes: Vec<ConfChangeEntry>,
 }
 
-impl UncommittedPoolOp for ConfChangeUcp {
+impl ConflictPoolOp for ConfChangeUcp {
     type Entry = ConfChangeEntry;
 
-    fn insert(&mut self, entry: Self::Entry) -> bool {
-        let conflict = !self.conf_changes.is_empty();
-        self.conf_changes.push(entry);
-        conflict
-    }
-
     fn is_empty(&self) -> bool {
         self.conf_changes.is_empty()
     }
@@ -148,10 +142,6 @@ impl UncommittedPoolOp for ConfChangeUcp {
         }
     }
 
-    fn all_conflict(&self, _entry: &Self::Entry) -> Vec<Self::Entry> {
-        self.conf_changes.clone()
-    }
-
     fn all(&self) -> Vec<Self::Entry> {
         self.conf_changes.clone()
     }
@@ -164,3 +154,15 @@ impl UncommittedPoolOp for ConfChangeUcp {
         self.conf_changes.len()
     }
 }
+
+impl UncommittedPoolOp for ConfChangeUcp {
+    fn insert(&mut self, entry: Self::Entry) -> bool {
+        let conflict = !self.conf_changes.is_empty();
+        self.conf_changes.push(entry);
+        conflict
+    }
+
+    fn all_conflict(&self, _entry: &Self::Entry) -> Vec<Self::Entry> {
+        self.conf_changes.clone()
+    }
+}