Apply suggestions from code review

Co-authored-by: Phoebe Goldman <phoebe@clockworklabs.io>
Signed-off-by: Mario Montoya <mamcx@elmalabarista.com>

Author: mamcx

crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module#FFI.verified.cs

@@ -23,7 +23,7 @@ public sealed record ReducerContext : DbContext<Local>, Internal.IReducerContext
         public readonly Timestamp Timestamp;
         public readonly AuthCtx SenderAuth;
 
-        // **Note:** must be 0..=i32::MAX
+        // **Note:** must be 0..=u32::MAX
         internal int CounterUuid;
 
         // We need this property to be non-static for parity with client SDK.

crates/bindings-csharp/Codegen.Tests/fixtures/server/snapshots/Module#FFI.verified.cs

@@ -1738,7 +1738,7 @@ public sealed record ReducerContext : DbContext<Local>, Internal.IReducerContext
                             public readonly Random Rng;
                             public readonly Timestamp Timestamp;
                             public readonly AuthCtx SenderAuth;
-                            // **Note:** must be 0..=i32::MAX
+                            // **Note:** must be 0..=u32::MAX
                             internal int CounterUuid;
                             // We need this property to be non-static for parity with client SDK.
                             public Identity Identity => Internal.IReducerContext.GetIdentity();

crates/bindings-csharp/Codegen/Module.cs

@@ -52,7 +52,7 @@ Timestamp time
     public Timestamp Timestamp { get; private set; } = time;
     public AuthCtx SenderAuth { get; } = AuthCtx.BuildFromSystemTables(connectionId, sender);
 
-    // **Note:** must be 0..=i32::MAX
+    // **Note:** must be 0..=u32::MAX
     protected int CounterUuid = 0;
     private Internal.TxContext? txContext;
     private ProcedureTxContextBase? cachedUserTxContext;

crates/bindings-csharp/Runtime/ProcedureContext.cs

@@ -119,7 +119,7 @@ export type ReducerCtx<SchemaDef extends UntypedSchemaDef> = Readonly<{
   identity: Identity;
   timestamp: Timestamp;
   connectionId: ConnectionId | null;
-  // **Note:** must be 0..=i32::MAX
+  // **Note:** must be 0..=u32::MAX
   counter_uuid: { value: number };
   db: DbView<SchemaDef>;
   senderAuth: AuthCtx;

crates/bindings-typescript/src/lib/reducers.ts

@@ -35,7 +35,7 @@ export function callProcedure(
     timestamp,
     connectionId,
     http: httpClient,
-    // **Note:** must be 0..=i32::MAX
+    // **Note:** must be 0..=u32::MAX
     counter_uuid: { value: Number(0) },
     get identity() {
       return new Identity(sys.identity().__identity__);

crates/bindings-typescript/src/server/procedures.ts

@@ -1,7 +1,7 @@
 #![doc = include_str!("../README.md")]
 // ^ if you are working on docs, go read the top comment of README.md please.
 
-use core::cell::{LazyCell, OnceCell, RefCell};
+use core::cell::{Cell, LazyCell, OnceCell, RefCell};
 use core::ops::Deref;
 use spacetimedb_lib::bsatn;
 use std::rc::Rc;
@@ -978,8 +978,9 @@ pub struct ReducerContext {
     #[cfg(feature = "rand08")]
     rng: std::cell::OnceCell<StdbRng>,
     /// A counter used for generating UUIDv7 values.
-    /// **Note:** must be 0..=i32::MAX
-    counter_uuid: RefCell<i32>,
+    /// **Note:** must be 0..=u32::MAX
+    #[cfg(feature = "rand")]
+    counter_uuid: Cell<u32>,
 }
 
 impl ReducerContext {
@@ -993,7 +994,8 @@ impl ReducerContext {
             sender_auth: AuthCtx::internal(),
             #[cfg(feature = "rand08")]
             rng: std::cell::OnceCell::new(),
-            counter_uuid: RefCell::new(0),
+            #[cfg(feature = "rand")]
+            counter_uuid: Cell::new(0),
         }
     }
 
@@ -1007,7 +1009,8 @@ impl ReducerContext {
             sender_auth: AuthCtx::from_connection_id_opt(connection_id),
             #[cfg(feature = "rand08")]
             rng: std::cell::OnceCell::new(),
-            counter_uuid: RefCell::new(0),
+            #[cfg(feature = "rand")]
+            counter_uuid: Cell::new(0),
         }
     }
 
@@ -1073,6 +1076,7 @@ impl ReducerContext {
     /// }
     /// # }
     /// ```
+    #[cfg(feature = "rand")]
     pub fn new_uuid_v7(&self) -> anyhow::Result<Uuid> {
         let mut random_bytes = [0u8; 4];
         self.rng().try_fill_bytes(&mut random_bytes)?;
@@ -1138,8 +1142,10 @@ pub struct ProcedureContext {
     #[cfg(feature = "rand08")]
     rng: std::cell::OnceCell<StdbRng>,
     /// A counter used for generating UUIDv7 values.
-    /// **Note:** must be 0..=i32::MAX
-    counter_uuid: RefCell<i32>,
+    /// **Note:** must be 0..=u32::MAX
+    // Disabled when compiling without `rand`, as both v4 and v7 UUIDs have random components.
+    #[cfg(feature = "rand")]
+    counter_uuid: Cell<u32>,
 }
 
 #[cfg(feature = "unstable")]
@@ -1152,7 +1158,8 @@ impl ProcedureContext {
             http: http::HttpClient {},
             #[cfg(feature = "rand08")]
             rng: std::cell::OnceCell::new(),
-            counter_uuid: RefCell::new(0),
+            #[cfg(feature = "rand")]
+            counter_uuid: Cell::new(0),
         }
     }
     /// Read the current module's [`Identity`].
@@ -1355,7 +1362,7 @@ impl ProcedureContext {
     /// }
     /// # }
     /// ```
-    #[cfg(feature = "unstable")]
+    #[cfg(all(feature = "unstable", feature = "rand"))]
     pub fn new_uuid_v7(&self) -> anyhow::Result<Uuid> {
         let mut random_bytes = [0u8; 4];
         self.rng().try_fill_bytes(&mut random_bytes)?;

crates/bindings/src/lib.rs

@@ -47,7 +47,7 @@ bytestring = "1"
 tokio-tungstenite.workspace = true
 itoa.workspace = true
 derive_more = "0.99.17"
-uuid = { workspace = true, features = ["v4"] }
+uuid.workspace = true
 jsonwebtoken.workspace = true
 scopeguard.workspace = true
 serde_with.workspace = true

crates/client-api/Cargo.toml

@@ -1,4 +1,4 @@
-use std::cell::RefCell;
+use std::cell::Cell;
 use std::fmt;
 
 use crate::timestamp::Timestamp;
@@ -123,17 +123,10 @@ impl Uuid {
     ///     uuid.to_string(),
     /// );
     /// ```
-    pub fn from_counter_v7(counter: &RefCell<i32>, now: Timestamp, random_bytes: &[u8; 4]) -> anyhow::Result<Self> {
+    pub fn from_counter_v7(counter: &Cell<u32>, now: Timestamp, random_bytes: &[u8; 4]) -> anyhow::Result<Self> {
         // Monotonic counter value (31 bits)
-        let mut current = counter.borrow_mut();
-        // The counter is constructed from Timestamp::UNIX_EPOCH by callers
-        assert!(*current >= 0, "uuid counter must be non-negative");
-
-        let counter_val = {
-            let next = *current;
-            *current = next.wrapping_add(1) & 0x7FFF_FFFF;
-            next
-        };
+        let counter_val = counter.get();
+        counter.set(counter_val.wrapping_add(1) & 0x7FFF_FFFF);
 
         let ts_ms = now
             .to_duration_since_unix_epoch()
@@ -312,7 +305,7 @@ mod test {
         let u_v4 = Uuid::from_random_bytes_v4([0u8; 16]);
         assert_eq!(u_v4.get_version(), Some(Version::V4));
 
-        let counter = RefCell::new(0);
+        let counter = Cell::new(0);
         let ts = Timestamp::from_micros_since_unix_epoch(1_686_000_000_000);
         let u_v7 = Uuid::from_counter_v7(&counter, ts, &[0u8; 4]).unwrap();
         assert_eq!(u_v7.get_version(), Some(Version::V7));
@@ -321,16 +314,16 @@ mod test {
     #[test]
     fn wrap_around() {
         // Check wraparound behavior
-        let counter = RefCell::new(i32::MAX);
+        let counter = Cell::new(u32::MAX);
         let ts = Timestamp::now();
         let _u1 = Uuid::from_counter_v7(&counter, ts, &[0u8; 4]).unwrap();
-        assert_eq!(0, counter.borrow().to_owned());
+        assert_eq!(0, counter.get());
     }
 
     #[test]
     #[should_panic(expected = "timestamp before unix epoch")]
     fn negative_timestamp_panics() {
-        let counter = RefCell::new(0);
+        let counter = Cell::new(0);
         let ts = Timestamp::from_micros_since_unix_epoch(-1);
         let _u = Uuid::from_counter_v7(&counter, ts, &[0u8; 4]).unwrap();
     }
@@ -344,13 +337,13 @@ mod test {
         assert_eq!(u1, u1);
         assert_ne!(u1, u2);
         // Check we start from zero
-        let counter = RefCell::new(0);
+        let counter = Cell::new(0);
         let ts = Timestamp::now();
         let u_start = Uuid::from_counter_v7(&counter, ts, &[0u8; 4]).unwrap();
         assert_eq!(u_start.get_counter(), 0);
         // Check ordering over many UUIDs up to the max counter value
         let total = 10_000_000;
-        let counter = RefCell::new(i32::MAX - total);
+        let counter = Cell::new(u32::MAX - total);
         let ts = Timestamp::now();
         let uuids = (0..total)
             .map(|_| {