Fix bug in StateMap::get_mut (#343)

See the changelog for more details.

concordium-std/CHANGELOG.md

@@ -3,6 +3,8 @@
 ## Unreleased changes
 
 - Set minimum Rust version to 1.66.
+- Fix bug in `StateMap::get_mut`, which allowed multiple mutable references to the state to coexist.
+  - The signature has changed using `&self` to using `&mut self`.
 
 ## concordium-std 8.0.0 (2023-08-21)
 

concordium-std/src/impls.rs

@@ -908,7 +908,8 @@ where
 
     /// Lookup a mutable reference to the value with the given key. Return
     /// [None] if there is no value with the given key.
-    pub fn get_mut(&self, key: &K) -> Option<StateRefMut<V, S>> {
+
+    pub fn get_mut(&mut self, key: &K) -> Option<StateRefMut<V, S>> {
         let k = self.key_with_map_prefix(key);
         let entry = self.state_api.lookup_entry(&k)?;
         Some(StateRefMut::new(entry, self.state_api.clone()))
@@ -2992,3 +2993,21 @@ impl Deserial for MetadataUrl {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+
+    /// Check that you cannot have multiple active entries from a statemap at
+    /// the same time. See the test file for details.
+    #[test]
+    fn statemap_multiple_entries_not_allowed() {
+        let t = trybuild::TestCases::new();
+        t.compile_fail("tests/state/map-multiple-entries.rs");
+    }
+
+    #[test]
+    fn statemap_multiple_state_ref_mut_not_allowed() {
+        let t = trybuild::TestCases::new();
+        t.compile_fail("tests/state/map-multiple-state-ref-mut.rs");
+    }
+}

concordium-std/src/test_infrastructure.rs

@@ -2431,17 +2431,6 @@ mod test {
         assert!(iter.nth(1).is_none());
     }
 
-    #[test]
-    fn multiple_entries_not_allowed() {
-        let mut state_builder = TestStateBuilder::new();
-        let mut map = state_builder.new_map();
-        map.insert(0u8, 1u8);
-        let e1 = map.entry(0u8);
-        // Uncommenting this line should give a borrow-check error.
-        // let e2 = map.entry(1u8);
-        e1.and_modify(|v| *v += 1);
-    }
-
     #[test]
     fn occupied_entry_truncates_leftover_data() {
         let mut state_builder = TestStateBuilder::new();

concordium-std/tests/state/map-multiple-entries.rs

@@ -0,0 +1,16 @@
+//! This test checks that you cannot have multiple entries from a `StateMap`
+//! alive at the same time.
+//!
+//! When compiling it, the borrow-checker is supposed to throw an error.
+use concordium_std::*;
+
+pub fn main() {
+    let mut state_builder = StateBuilder::open(ExternStateApi::open());
+    let mut map: StateMap<u8, u8, _> = state_builder.new_map();
+    // Get two entries.
+    let e1 = map.entry(0u8);
+    let e2 = map.entry(1u8);
+    // Use them, so we are certain that their lifetimes overlap.
+    e1.or_insert(1);
+    e2.or_insert(2);
+}

concordium-std/tests/state/map-multiple-entries.stderr

@@ -0,0 +1,10 @@
+error[E0499]: cannot borrow `map` as mutable more than once at a time
+  --> tests/state/map-multiple-entries.rs:12:14
+   |
+11 |     let e1 = map.entry(0u8);
+   |              -------------- first mutable borrow occurs here
+12 |     let e2 = map.entry(1u8);
+   |              ^^^^^^^^^^^^^^ second mutable borrow occurs here
+13 |     // Use them, so we are certain that their lifetimes overlap.
+14 |     e1.or_insert(1);
+   |     -- first borrow later used here

concordium-std/tests/state/map-multiple-state-ref-mut.rs

@@ -0,0 +1,17 @@
+//! This test checks that you cannot have multiple `StateRefMut` from a
+//! `StateMap` alive at the same time.
+//!
+//! When compiling it, the borrow-checker is supposed to throw an error.
+use concordium_std::*;
+
+pub fn main() {
+    let mut state_builder = StateBuilder::open(ExternStateApi::open());
+    let mut map: StateMap<u8, u8, _> = state_builder.new_map();
+    map.insert(0, 1);
+    map.insert(1, 2);
+    // Get two mutable references and unwrap the options.
+    let e1 = map.get_mut(&0u8).unwrap();
+    let e2 = map.get_mut(&1u8).unwrap();
+    // Use them, so we are certain that their lifetimes overlap.
+    assert_eq!(*e1, *e2);
+}

concordium-std/tests/state/map-multiple-state-ref-mut.stderr

@@ -0,0 +1,10 @@
+error[E0499]: cannot borrow `map` as mutable more than once at a time
+  --> tests/state/map-multiple-state-ref-mut.rs:14:14
+   |
+13 |     let e1 = map.get_mut(&0u8).unwrap();
+   |              ----------------- first mutable borrow occurs here
+14 |     let e2 = map.get_mut(&1u8).unwrap();
+   |              ^^^^^^^^^^^^^^^^^ second mutable borrow occurs here
+15 |     // Use them, so we are certain that their lifetimes overlap.
+16 |     assert_eq!(*e1, *e2);
+   |                 -- first borrow later used here