Address some bugs.

Author: MathieuDutSik

linera-storage-service/src/client.rs

@@ -265,20 +265,22 @@ impl ReadableKeyValueStore for StorageServiceStoreInternal {
             is_finished,
         } = response;
         let prefix_len = self.start_key.len();
-        if num_chunks == 0 {
-            let keys = keys
-                .into_iter()
-                .map(|key| key[prefix_len..].to_vec())
-                .collect();
-            Ok((keys, is_finished))
+        let keys = if num_chunks == 0 {
+            keys
         } else {
-            let keys: Vec<Vec<u8>> = self.read_entries(message_index, num_chunks).await?;
-            let keys = keys
-                .into_iter()
-                .map(|key| key[prefix_len..].to_vec())
-                .collect();
-            Ok((keys, is_finished))
-        }
+            self.read_entries(message_index, num_chunks).await?
+        };
+        let keys = keys
+            .into_iter()
+            .map(|key| {
+                ensure!(
+                    key.starts_with(&self.start_key),
+                    StorageServiceStoreError::KeyOutsidePartition
+                );
+                Ok::<_, StorageServiceStoreError>(key[prefix_len..].to_vec())
+            })
+            .collect::<Result<Vec<_>, _>>()?;
+        Ok((keys, is_finished))
     }
 
     async fn find_key_values_in_interval(
@@ -333,21 +335,22 @@ impl ReadableKeyValueStore for StorageServiceStoreInternal {
             is_finished,
         } = response;
         let prefix_len = self.start_key.len();
-        if num_chunks == 0 {
-            let key_values = key_values
-                .into_iter()
-                .map(|x| (x.key[prefix_len..].to_vec(), x.value))
-                .collect::<Vec<_>>();
-            Ok((key_values, is_finished))
+        let key_values: Vec<(Vec<u8>, Vec<u8>)> = if num_chunks == 0 {
+            key_values.into_iter().map(|x| (x.key, x.value)).collect()
         } else {
-            let key_values: Vec<(Vec<u8>, Vec<u8>)> =
-                self.read_entries(message_index, num_chunks).await?;
-            let key_values = key_values
-                .into_iter()
-                .map(|(key, value)| (key[prefix_len..].to_vec(), value))
-                .collect();
-            Ok((key_values, is_finished))
-        }
+            self.read_entries(message_index, num_chunks).await?
+        };
+        let key_values = key_values
+            .into_iter()
+            .map(|(key, value)| {
+                ensure!(
+                    key.starts_with(&self.start_key),
+                    StorageServiceStoreError::KeyOutsidePartition
+                );
+                Ok::<_, StorageServiceStoreError>((key[prefix_len..].to_vec(), value))
+            })
+            .collect::<Result<Vec<_>, _>>()?;
+        Ok((key_values, is_finished))
     }
 }
 

linera-storage-service/src/common.rs

@@ -51,6 +51,13 @@ pub enum StorageServiceStoreError {
     #[error("The key size must be at most 1 MB")]
     KeyTooLong,
 
+    /// The server returned a key that does not lie within the requesting
+    /// client's partition. This indicates either a malformed request (e.g. an
+    /// `Unbounded` end with an all-`0xFF` `start_key`, which leaves the
+    /// underlying scan unbounded) or a server-side bug.
+    #[error("server returned a key outside this client's partition")]
+    KeyOutsidePartition,
+
     /// Transport error
     #[error(transparent)]
     TransportError(#[from] tonic::transport::Error),

linera-views/src/backends/lru_caching.rs

@@ -11,37 +11,12 @@ use serde::{Deserialize, Serialize};
 use crate::memory::MemoryDatabase;
 #[cfg(with_testing)]
 use crate::store::TestKeyValueDatabase;
-use std::ops::Bound;
-
 use crate::{
     batch::{Batch, WriteOperation},
     lru_prefix_cache::{LruPrefixCache, StorageCacheConfig},
-    store::{
-        KeyInterval, KeyIntervalStart, KeyValueDatabase, ReadableKeyValueStore, WithError,
-        WritableKeyValueStore,
-    },
+    store::{KeyInterval, KeyValueDatabase, ReadableKeyValueStore, WithError, WritableKeyValueStore},
 };
 
-/// Returns the longest common byte prefix shared by `start` and `end`. Any key
-/// inside `[start, end]` (any inclusivity) must start with this prefix. For
-/// `Unbounded` ends the LCP is empty (only an empty-prefix cache entry could
-/// cover the request).
-fn lcp_of_interval(key_interval: &KeyInterval) -> Vec<u8> {
-    let start = match &key_interval.start {
-        KeyIntervalStart::Included(k) | KeyIntervalStart::Excluded(k) => k.as_slice(),
-    };
-    let end = match &key_interval.end {
-        Bound::Included(k) | Bound::Excluded(k) => k.as_slice(),
-        Bound::Unbounded => return Vec::new(),
-    };
-    let n = start
-        .iter()
-        .zip(end.iter())
-        .take_while(|(a, b)| a == b)
-        .count();
-    start[..n].to_vec()
-}
-
 #[cfg(with_metrics)]
 mod metrics {
     use std::sync::LazyLock;
@@ -368,7 +343,7 @@ where
         // Any key in the user interval starts with `lcp(start, end)`. The
         // prefix cache walks back from this prefix to any shorter cached
         // prefix that covers it, so a single probe is sufficient.
-        let lcp = lcp_of_interval(&key_interval);
+        let lcp = key_interval.common_prefix();
         let cached = {
             let mut cache = cache.lock().unwrap();
             cache.query_find_keys(&lcp)
@@ -438,7 +413,7 @@ where
         let Some(cache) = self.get_exclusive_cache() else {
             return self.store.find_key_values_in_interval(key_interval).await;
         };
-        let lcp = lcp_of_interval(&key_interval);
+        let lcp = key_interval.common_prefix();
         let cached = {
             let mut cache = cache.lock().unwrap();
             cache.query_find_key_values(&lcp)

linera-views/src/store.rs

@@ -82,6 +82,26 @@ impl KeyInterval {
         }
     }
 
+    /// Returns the longest common byte prefix shared by the start and end
+    /// keys of this interval. Any key inside the interval (any inclusivity)
+    /// must start with this prefix. Returns the empty vector when the end
+    /// is `Unbounded` (only an empty-prefix scan can cover such a request).
+    pub fn common_prefix(&self) -> Vec<u8> {
+        let start = match &self.start {
+            KeyIntervalStart::Included(k) | KeyIntervalStart::Excluded(k) => k.as_slice(),
+        };
+        let end = match &self.end {
+            Included(k) | Excluded(k) => k.as_slice(),
+            Unbounded => return Vec::new(),
+        };
+        let n = start
+            .iter()
+            .zip(end.iter())
+            .take_while(|(a, b)| a == b)
+            .count();
+        start[..n].to_vec()
+    }
+
     /// Returns `true` if `key` falls within this interval's bounds. The
     /// `limit` field is ignored.
     pub fn contains(&self, key: &[u8]) -> bool {

linera-views/src/views/key_value_store_view.rs

@@ -25,7 +25,7 @@ use crate::{
     batch::{Batch, WriteOperation},
     common::{
         from_bytes_option, from_bytes_option_or_default, get_key_range_for_prefix, get_upper_bound,
-        key_matches_interval, DeletionSet, HasherOutput, SuffixClosedSetIterator, Update,
+        DeletionSet, HasherOutput, SuffixClosedSetIterator, Update,
     },
     context::Context,
     hashable_wrapper::WrappedHashableContainerView,
@@ -1012,42 +1012,63 @@ impl<C: Context> KeyValueStoreView<C> {
         &self,
         key_interval: KeyInterval,
     ) -> Result<(Vec<Vec<u8>>, bool), ViewError> {
-        let key_values = self.find_key_values_by_prefix(&[]).await?;
+        if key_interval.is_empty() {
+            return Ok((Vec::new(), true));
+        }
+        // Any key in the interval starts with the longest common prefix of
+        // its bounds, so the prefix-scan path (which already merges in-memory
+        // updates with the underlying store and respects the deletion set)
+        // is a sufficient superset. We then trim it to the interval and limit.
+        let prefix = key_interval.common_prefix();
+        let prefix_keys = self.find_keys_by_prefix(&prefix).await?;
         let mut keys = Vec::new();
-        for (key, _value) in key_values {
-            if key_matches_interval(&key, key_interval.start_bound(), key_interval.end_bound()) {
-                keys.push(key);
-                if key_interval.limit.is_some_and(|limit| keys.len() >= limit) {
-                    break;
-                }
+        let mut more_after_limit = false;
+        for stripped in prefix_keys {
+            let mut full = prefix.clone();
+            full.extend(&stripped);
+            if !key_interval.contains(&full) {
+                continue;
+            }
+            if key_interval
+                .limit
+                .is_some_and(|limit| keys.len() >= limit)
+            {
+                more_after_limit = true;
+                break;
             }
+            keys.push(full);
         }
-        let is_finished = key_interval.limit.is_none_or(|limit| keys.len() < limit);
-        Ok((keys, is_finished))
+        Ok((keys, !more_after_limit))
     }
 
     /// Iterates over all key-value pairs matching the given interval.
     pub async fn find_key_values_in_interval(
         &self,
         key_interval: KeyInterval,
     ) -> Result<(Vec<(Vec<u8>, Vec<u8>)>, bool), ViewError> {
-        let entries = self.find_key_values_by_prefix(&[]).await?;
+        if key_interval.is_empty() {
+            return Ok((Vec::new(), true));
+        }
+        let prefix = key_interval.common_prefix();
+        let prefix_entries = self.find_key_values_by_prefix(&prefix).await?;
         let mut key_values = Vec::new();
-        for (key, value) in entries {
-            if key_matches_interval(&key, key_interval.start_bound(), key_interval.end_bound()) {
-                key_values.push((key, value));
-                if key_interval
-                    .limit
-                    .is_some_and(|limit| key_values.len() >= limit)
-                {
-                    break;
-                }
+        let mut more_after_limit = false;
+        for (stripped, value) in prefix_entries {
+            let mut full = prefix.clone();
+            full.extend(&stripped);
+            if !key_interval.contains(&full) {
+                continue;
+            }
+            if key_interval
+                .limit
+                .is_some_and(|limit| key_values.len() >= limit)
+            {
+                more_after_limit = true;
+                break;
             }
+            key_values.push((full, value));
         }
-        let is_finished = key_interval
-            .limit
-            .is_none_or(|limit| key_values.len() < limit);
-        Ok((key_values, is_finished))
+        Ok((key_values, !more_after_limit))
     }
 
     /// Iterates over all the keys matching the given prefix. The prefix is not included in the returned keys.
@@ -1330,43 +1351,60 @@ impl<C: Context> ReadableKeyValueStore for ViewContainer<C> {
         &self,
         key_interval: KeyInterval,
     ) -> Result<(Vec<Vec<u8>>, bool), ViewContainerError> {
+        if key_interval.is_empty() {
+            return Ok((Vec::new(), true));
+        }
         let view = self.view.read().await;
-        let key_values = view.find_key_values_by_prefix(&[]).await?;
+        let prefix = key_interval.common_prefix();
+        let prefix_keys = view.find_keys_by_prefix(&prefix).await?;
         let mut keys = Vec::new();
-        for (key, _value) in key_values {
-            if key_matches_interval(&key, key_interval.start_bound(), key_interval.end_bound()) {
-                keys.push(key);
-                if key_interval.limit.is_some_and(|limit| keys.len() >= limit) {
-                    break;
-                }
+        let mut more_after_limit = false;
+        for stripped in prefix_keys {
+            let mut full = prefix.clone();
+            full.extend(&stripped);
+            if !key_interval.contains(&full) {
+                continue;
+            }
+            if key_interval
+                .limit
+                .is_some_and(|limit| keys.len() >= limit)
+            {
+                more_after_limit = true;
+                break;
             }
+            keys.push(full);
         }
-        let is_finished = key_interval.limit.is_none_or(|limit| keys.len() < limit);
-        Ok((keys, is_finished))
+        Ok((keys, !more_after_limit))
     }
 
     async fn find_key_values_in_interval(
         &self,
         key_interval: KeyInterval,
     ) -> Result<(Vec<(Vec<u8>, Vec<u8>)>, bool), ViewContainerError> {
+        if key_interval.is_empty() {
+            return Ok((Vec::new(), true));
+        }
         let view = self.view.read().await;
-        let entries = view.find_key_values_by_prefix(&[]).await?;
+        let prefix = key_interval.common_prefix();
+        let prefix_entries = view.find_key_values_by_prefix(&prefix).await?;
         let mut key_values = Vec::new();
-        for (key, value) in entries {
-            if key_matches_interval(&key, key_interval.start_bound(), key_interval.end_bound()) {
-                key_values.push((key, value));
-                if key_interval
-                    .limit
-                    .is_some_and(|limit| key_values.len() >= limit)
-                {
-                    break;
-                }
+        let mut more_after_limit = false;
+        for (stripped, value) in prefix_entries {
+            let mut full = prefix.clone();
+            full.extend(&stripped);
+            if !key_interval.contains(&full) {
+                continue;
+            }
+            if key_interval
+                .limit
+                .is_some_and(|limit| key_values.len() >= limit)
+            {
+                more_after_limit = true;
+                break;
             }
+            key_values.push((full, value));
         }
-        let is_finished = key_interval
-            .limit
-            .is_none_or(|limit| key_values.len() < limit);
-        Ok((key_values, is_finished))
+        Ok((key_values, !more_after_limit))
     }
 }