From 1805f5051685380cb1162f0f318fefcca0a4e568 Mon Sep 17 00:00:00 2001
From: Joseph Livesey <joseph.livesey@btp.works>
Date: Wed, 22 Feb 2023 12:34:41 -0500
Subject: [PATCH] fix(commit_store): fix bug in get_batch_by_transaction

Signed-off-by: Joseph Livesey <joseph.livesey@btp.works>
---
 validator/src/journal/commit_store.rs | 95 ++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 1 deletion(-)

diff --git a/validator/src/journal/commit_store.rs b/validator/src/journal/commit_store.rs
index bd92f28ad7..a6957985e2 100644
--- a/validator/src/journal/commit_store.rs
+++ b/validator/src/journal/commit_store.rs
@@ -336,7 +336,7 @@ impl CommitStore {
                         batch
                             .transaction_ids
                             .iter()
-                            .any(|txn_id| txn_id == transaction_id)
+                            .all(|txn_id| txn_id != transaction_id)
                     })
                     .next()
                     .ok_or_else(|| {
@@ -577,3 +577,96 @@ impl ChainReader for CommitStore {
             .map_err(|err| ChainReadError::GeneralReadError(format!("{:?}", err)))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use sawdb::error::DatabaseError;
+
+    #[derive(Clone, Debug, PartialEq)]
+    pub struct TestBatch {
+        // pub header_signature: String,
+        // pub transactions: Vec<Transaction>,
+        // pub signer_public_key: String,
+        pub transaction_ids: Vec<String>,
+        // pub trace: bool,
+
+        // pub header_bytes: Vec<u8>,
+    }
+
+    pub struct TestBlock {
+        // pub header_signature: String,
+        pub batches: Vec<TestBatch>,
+        // pub state_root_hash: String,
+        // pub consensus: Vec<u8>,
+        // pub batch_ids: Vec<String>,
+        // pub signer_public_key: String,
+        // pub previous_block_id: String,
+        // pub block_num: u64,
+
+        // pub header_bytes: Vec<u8>,
+    }
+
+    // Copies flawed logic of `sawtooth_validator::journal::commit_store::CommitStore::get_batch_by_transaction`
+    fn mock_previous_get_batch_by_transaction(
+        transaction_id: &str,
+        block: TestBlock,
+    ) -> Result<TestBatch, DatabaseError> {
+        // self.get_by_transaction_id(transaction_id)
+        // .and_then(|block| {
+        block
+            .batches
+            .into_iter()
+            .skip_while(|batch| {
+                batch
+                    .transaction_ids
+                    .iter()
+                    .any(|txn_id| txn_id == transaction_id)
+            })
+            .next()
+            .ok_or_else(|| DatabaseError::CorruptionError("Transaction index corrupted".into()))
+        // })
+    }
+
+    #[test]
+    fn test_previous_get_batch_by_transaction_has_a_bug() {
+        let block = TestBlock {
+            batches: vec![TestBatch {
+                transaction_ids: vec!["1".to_string()],
+            }],
+        };
+        let transaction_id = "1";
+        assert!(mock_previous_get_batch_by_transaction(transaction_id, block).is_err());
+    }
+
+    // Copies logic of new `sawtooth_validator::journal::commit_store::CommitStore::get_batch_by_transaction`
+    fn mock_get_batch_by_transaction(
+        transaction_id: &str,
+        block: TestBlock,
+    ) -> Result<TestBatch, DatabaseError> {
+        // self.get_by_transaction_id(transaction_id)
+        // .and_then(|block| {
+        block
+            .batches
+            .into_iter()
+            .skip_while(|batch| {
+                batch
+                    .transaction_ids
+                    .iter()
+                    .all(|txn_id| txn_id != transaction_id)
+            })
+            .next()
+            .ok_or_else(|| DatabaseError::CorruptionError("Transaction index corrupted".into()))
+        // })
+    }
+
+    #[test]
+    fn test_get_batch_by_transaction() {
+        let block = TestBlock {
+            batches: vec![TestBatch {
+                transaction_ids: vec!["1".to_string()],
+            }],
+        };
+        let transaction_id = "1";
+        assert!(mock_get_batch_by_transaction(transaction_id, block).is_ok());
+    }
+}