From 410314b7807c416cc76c585488e54154ae5bf8f3 Mon Sep 17 00:00:00 2001
From: Julien Cretin <cretin@google.com>
Date: Tue, 3 Nov 2020 12:54:30 +0100
Subject: [PATCH] Move transaction validity check to Format

---
 libraries/persistent_store/src/format.rs | 26 ++++++++++++++++++++++++
 libraries/persistent_store/src/model.rs  | 23 +++------------------
 libraries/persistent_store/src/store.rs  | 25 +++++------------------
 3 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/libraries/persistent_store/src/format.rs b/libraries/persistent_store/src/format.rs
index 7f79328..1f87ef3 100644
--- a/libraries/persistent_store/src/format.rs
+++ b/libraries/persistent_store/src/format.rs
@@ -520,6 +520,32 @@ impl Format {
         1 + self.bytes_to_words(usize_to_nat(value.len()))
     }
 
+    /// Checks if a transaction is valid and returns its sorted keys.
+    ///
+    /// Returns `None` if the transaction is invalid.
+    pub fn transaction_valid(&self, updates: &[StoreUpdate]) -> Option<Vec<Nat>> {
+        if usize_to_nat(updates.len()) > self.max_updates() {
+            return None;
+        }
+        let mut sorted_keys = Vec::with_capacity(updates.len());
+        for update in updates {
+            let key = usize_to_nat(update.key());
+            if key > self.max_key() {
+                return None;
+            }
+            if let Some(value) = update.value() {
+                if usize_to_nat(value.len()) > self.max_value_len() {
+                    return None;
+                }
+            }
+            match sorted_keys.binary_search(&key) {
+                Ok(_) => return None,
+                Err(pos) => sorted_keys.insert(pos, key),
+            }
+        }
+        Some(sorted_keys)
+    }
+
     /// Returns the minimum number of words to represent a given number of bytes.
     ///
     /// # Preconditions
diff --git a/libraries/persistent_store/src/model.rs b/libraries/persistent_store/src/model.rs
index b558f99..c509b03 100644
--- a/libraries/persistent_store/src/model.rs
+++ b/libraries/persistent_store/src/model.rs
@@ -14,7 +14,7 @@
 
 use crate::format::Format;
 use crate::{usize_to_nat, StoreError, StoreRatio, StoreResult, StoreUpdate};
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 
 /// Models the mutable operations of a store.
 ///
@@ -90,17 +90,8 @@ impl StoreModel {
 
     /// Applies a transaction.
     fn transaction(&mut self, updates: Vec<StoreUpdate>) -> StoreResult<()> {
-        // Fail if too many updates.
-        if updates.len() > self.format.max_updates() as usize {
-            return Err(StoreError::InvalidArgument);
-        }
-        // Fail if an update is invalid.
-        if !updates.iter().all(|x| self.update_valid(x)) {
-            return Err(StoreError::InvalidArgument);
-        }
-        // Fail if updates are not disjoint, i.e. there are duplicate keys.
-        let keys: HashSet<_> = updates.iter().map(|x| x.key()).collect();
-        if keys.len() != updates.len() {
+        // Fail if the transaction is invalid.
+        if self.format.transaction_valid(&updates).is_none() {
             return Err(StoreError::InvalidArgument);
         }
         // Fail if there is not enough capacity.
@@ -138,12 +129,4 @@ impl StoreModel {
         }
         Ok(())
     }
-
-    /// Returns whether an update is valid.
-    fn update_valid(&self, update: &StoreUpdate) -> bool {
-        update.key() <= self.format.max_key() as usize
-            && update
-                .value()
-                .map_or(true, |x| x.len() <= self.format.max_value_len() as usize)
-    }
 }
diff --git a/libraries/persistent_store/src/store.rs b/libraries/persistent_store/src/store.rs
index b7c034a..3d38b9c 100644
--- a/libraries/persistent_store/src/store.rs
+++ b/libraries/persistent_store/src/store.rs
@@ -259,26 +259,11 @@ impl<S: Storage> Store<S> {
                 StoreUpdate::Remove { key } => return self.remove(key),
             }
         }
-        if count > self.format.max_updates() {
-            return Err(StoreError::InvalidArgument);
-        }
-        // Check that the updates are valid.
-        let mut sorted_keys = Vec::with_capacity(count as usize);
-        for update in updates {
-            let key = usize_to_nat(update.key());
-            if key > self.format.max_key() {
-                return Err(StoreError::InvalidArgument);
-            }
-            if let Some(value) = update.value() {
-                if usize_to_nat(value.len()) > self.format.max_value_len() {
-                    return Err(StoreError::InvalidArgument);
-                }
-            }
-            match sorted_keys.binary_search(&key) {
-                Ok(_) => return Err(StoreError::InvalidArgument),
-                Err(pos) => sorted_keys.insert(pos, key),
-            }
-        }
+        // Get the sorted keys. Fail if the transaction is invalid.
+        let sorted_keys = match self.format.transaction_valid(updates) {
+            None => return Err(StoreError::InvalidArgument),
+            Some(x) => x,
+        };
         // Reserve the capacity.
         self.reserve(self.format.transaction_capacity(updates))?;
         // Write the marker entry.