Fix mempool size bug in FiFoStrictBatchBuilder (#897)

* Add failing test Signed-off-by: Filippo Costa <[email protected]> * Doc fix * Fix bug --------- Signed-off-by: Filippo Costa <[email protected]>
Sovereign-Labs · Sep 19, 2023 · 0259c64 · 0259c64
1 parent 9b38ce0
commit 0259c64
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 3 deletions.
diff --git a/full-node/sov-stf-runner/src/batch_builder.rs b/full-node/sov-stf-runner/src/batch_builder.rs
@@ -43,7 +43,7 @@ where
 {
     /// Transaction can only be declined only mempool is full
     fn accept_tx(&mut self, tx: Vec<u8>) -> anyhow::Result<()> {
-        if self.mempool.len() > self.mempool_max_txs_count {
+        if self.mempool.len() >= self.mempool_max_txs_count {
             bail!("Mempool is full")
         }
         self.mempool.push_back(tx);
@@ -261,7 +261,7 @@ mod tests {
             let tmpdir = tempfile::tempdir().unwrap();
             let (mut batch_builder, _) = create_batch_builder(10, &tmpdir);
 
-            for _ in 0..=MAX_TX_POOL_SIZE {
+            for _ in 0..MAX_TX_POOL_SIZE {
                 let tx = generate_random_valid_tx();
                 batch_builder.accept_tx(tx).unwrap();
             }
@@ -272,6 +272,19 @@ mod tests {
             assert!(accept_result.is_err());
             assert_eq!("Mempool is full", accept_result.unwrap_err().to_string());
         }
+
+        #[test]
+        fn zero_sized_mempool_cant_accept_tx() {
+            let tmpdir = tempfile::tempdir().unwrap();
+            let (mut batch_builder, _) = create_batch_builder(10, &tmpdir);
+            batch_builder.mempool_max_txs_count = 0;
+
+            let tx = generate_random_valid_tx();
+            let accept_result = batch_builder.accept_tx(tx);
+
+            assert!(accept_result.is_err());
+            assert_eq!("Mempool is full", accept_result.unwrap_err().to_string());
+        }
     }
 
     mod build_batch {

diff --git a/rollup-interface/src/node/services/da.rs b/rollup-interface/src/node/services/da.rs
@@ -87,7 +87,7 @@ pub trait DaService: Send + Sync + 'static {
 
 /// `SlotData` is the subset of a DA layer block which is stored in the rollup's database.
 /// At the very least, the rollup needs access to the hashes and headers of all DA layer blocks, but rollups
-/// may choose to partial (or full) block data as well.
+/// may choose to store partial (or full) block data as well.
 pub trait SlotData:
     Serialize + DeserializeOwned + PartialEq + core::fmt::Debug + Clone + Send + Sync
 {