From 2813c92505497ff32fddedff12efc78181791a25 Mon Sep 17 00:00:00 2001
From: hinto-janai <hinto.janai@protonmail.com>
Date: Wed, 3 Apr 2024 17:44:25 -0400
Subject: [PATCH] database: make `open_db_rw()` take `&TxRw` (#104)

* env: take `&TxRw` in `open_db_rw()`

* heed: use `UnsafeCell` for write transactions

* backend: update tests

* add `trait DatabaseIter<T: Table>`

* heed: impl `trait DatabaseIter`, inline shared iter `fn`s

* env: make `open_db_ro()` return `impl DatabaseIter`

* heed: use full path for transaction fn calls

* tests: fix tx/table tests

* backend: fix redb

* heed: `UnsafeCell` -> `RefCell`

* docs

* remove unneeded `// SAFETY`

* Update database/src/backend/heed/env.rs

Co-authored-by: Boog900 <boog900@tutanota.com>

---------

Co-authored-by: Boog900 <boog900@tutanota.com>
---
 database/src/backend/heed/database.rs    | 129 ++++++-----------------
 database/src/backend/heed/env.rs         |  43 ++++++--
 database/src/backend/heed/transaction.rs |  14 +--
 database/src/backend/redb/database.rs    | 122 +++++----------------
 database/src/backend/redb/env.rs         |   6 +-
 database/src/backend/tests.rs            |  82 ++++++++------
 database/src/database.rs                 |  45 +++++---
 database/src/env.rs                      |   9 +-
 database/src/lib.rs                      |   2 +-
 9 files changed, 191 insertions(+), 261 deletions(-)

diff --git a/database/src/backend/heed/database.rs b/database/src/backend/heed/database.rs
index 778ffd56..9b3745c5 100644
--- a/database/src/backend/heed/database.rs
+++ b/database/src/backend/heed/database.rs
@@ -3,6 +3,7 @@
 //---------------------------------------------------------------------------------------------------- Import
 use std::{
     borrow::{Borrow, Cow},
+    cell::RefCell,
     fmt::Debug,
     ops::RangeBounds,
     sync::RwLockReadGuard,
@@ -10,7 +11,7 @@ use std::{
 
 use crate::{
     backend::heed::{storable::StorableHeed, types::HeedDb},
-    database::{DatabaseRo, DatabaseRw},
+    database::{DatabaseIter, DatabaseRo, DatabaseRw},
     error::RuntimeError,
     table::Table,
 };
@@ -45,7 +46,7 @@ pub(super) struct HeedTableRw<'env, 'tx, T: Table> {
     /// An already opened database table.
     pub(super) db: HeedDb<T::Key, T::Value>,
     /// The associated read/write transaction that opened this table.
-    pub(super) tx_rw: &'tx mut heed::RwTxn<'env>,
+    pub(super) tx_rw: &'tx RefCell<heed::RwTxn<'env>>,
 }
 
 //---------------------------------------------------------------------------------------------------- Shared functions
@@ -63,46 +64,6 @@ fn get<T: Table>(
     db.get(tx_ro, key)?.ok_or(RuntimeError::KeyNotFound)
 }
 
-/// Shared [`DatabaseRo::get_range()`].
-#[inline]
-fn get_range<'a, T: Table, Range>(
-    db: &'a HeedDb<T::Key, T::Value>,
-    tx_ro: &'a heed::RoTxn<'_>,
-    range: Range,
-) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + 'a, RuntimeError>
-where
-    Range: RangeBounds<T::Key> + 'a,
-{
-    Ok(db.range(tx_ro, &range)?.map(|res| Ok(res?.1)))
-}
-
-/// Shared [`DatabaseRo::iter()`].
-#[inline]
-fn iter<'a, T: Table>(
-    db: &'a HeedDb<T::Key, T::Value>,
-    tx_ro: &'a heed::RoTxn<'_>,
-) -> Result<impl Iterator<Item = Result<(T::Key, T::Value), RuntimeError>> + 'a, RuntimeError> {
-    Ok(db.iter(tx_ro)?.map(|res| Ok(res?)))
-}
-
-/// Shared [`DatabaseRo::keys()`].
-#[inline]
-fn keys<'a, T: Table>(
-    db: &'a HeedDb<T::Key, T::Value>,
-    tx_ro: &'a heed::RoTxn<'_>,
-) -> Result<impl Iterator<Item = Result<T::Key, RuntimeError>> + 'a, RuntimeError> {
-    Ok(db.iter(tx_ro)?.map(|res| Ok(res?.0)))
-}
-
-/// Shared [`DatabaseRo::values()`].
-#[inline]
-fn values<'a, T: Table>(
-    db: &'a HeedDb<T::Key, T::Value>,
-    tx_ro: &'a heed::RoTxn<'_>,
-) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + 'a, RuntimeError> {
-    Ok(db.iter(tx_ro)?.map(|res| Ok(res?.1)))
-}
-
 /// Shared [`DatabaseRo::len()`].
 #[inline]
 fn len<T: Table>(
@@ -139,13 +100,8 @@ fn is_empty<T: Table>(
     Ok(db.is_empty(tx_ro)?)
 }
 
-//---------------------------------------------------------------------------------------------------- DatabaseRo Impl
-impl<T: Table> DatabaseRo<T> for HeedTableRo<'_, T> {
-    #[inline]
-    fn get(&self, key: &T::Key) -> Result<T::Value, RuntimeError> {
-        get::<T>(&self.db, self.tx_ro, key)
-    }
-
+//---------------------------------------------------------------------------------------------------- DatabaseIter Impl
+impl<T: Table> DatabaseIter<T> for HeedTableRo<'_, T> {
     #[inline]
     fn get_range<'a, Range>(
         &'a self,
@@ -154,7 +110,7 @@ impl<T: Table> DatabaseRo<T> for HeedTableRo<'_, T> {
     where
         Range: RangeBounds<T::Key> + 'a,
     {
-        get_range::<T, Range>(&self.db, self.tx_ro, range)
+        Ok(self.db.range(self.tx_ro, &range)?.map(|res| Ok(res?.1)))
     }
 
     #[inline]
@@ -162,21 +118,29 @@ impl<T: Table> DatabaseRo<T> for HeedTableRo<'_, T> {
         &self,
     ) -> Result<impl Iterator<Item = Result<(T::Key, T::Value), RuntimeError>> + '_, RuntimeError>
     {
-        iter::<T>(&self.db, self.tx_ro)
+        Ok(self.db.iter(self.tx_ro)?.map(|res| Ok(res?)))
     }
 
     #[inline]
     fn keys(
         &self,
     ) -> Result<impl Iterator<Item = Result<T::Key, RuntimeError>> + '_, RuntimeError> {
-        keys::<T>(&self.db, self.tx_ro)
+        Ok(self.db.iter(self.tx_ro)?.map(|res| Ok(res?.0)))
     }
 
     #[inline]
     fn values(
         &self,
     ) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + '_, RuntimeError> {
-        values::<T>(&self.db, self.tx_ro)
+        Ok(self.db.iter(self.tx_ro)?.map(|res| Ok(res?.1)))
+    }
+}
+
+//---------------------------------------------------------------------------------------------------- DatabaseRo Impl
+impl<T: Table> DatabaseRo<T> for HeedTableRo<'_, T> {
+    #[inline]
+    fn get(&self, key: &T::Key) -> Result<T::Value, RuntimeError> {
+        get::<T>(&self.db, self.tx_ro, key)
     }
 
     #[inline]
@@ -204,79 +168,48 @@ impl<T: Table> DatabaseRo<T> for HeedTableRo<'_, T> {
 impl<T: Table> DatabaseRo<T> for HeedTableRw<'_, '_, T> {
     #[inline]
     fn get(&self, key: &T::Key) -> Result<T::Value, RuntimeError> {
-        get::<T>(&self.db, self.tx_rw, key)
-    }
-
-    #[inline]
-    fn get_range<'a, Range>(
-        &'a self,
-        range: Range,
-    ) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + 'a, RuntimeError>
-    where
-        Range: RangeBounds<T::Key> + 'a,
-    {
-        get_range::<T, Range>(&self.db, self.tx_rw, range)
-    }
-
-    #[inline]
-    fn iter(
-        &self,
-    ) -> Result<impl Iterator<Item = Result<(T::Key, T::Value), RuntimeError>> + '_, RuntimeError>
-    {
-        iter::<T>(&self.db, self.tx_rw)
-    }
-
-    #[inline]
-    fn keys(
-        &self,
-    ) -> Result<impl Iterator<Item = Result<T::Key, RuntimeError>> + '_, RuntimeError> {
-        keys::<T>(&self.db, self.tx_rw)
-    }
-
-    #[inline]
-    fn values(
-        &self,
-    ) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + '_, RuntimeError> {
-        values::<T>(&self.db, self.tx_rw)
+        get::<T>(&self.db, &self.tx_rw.borrow(), key)
     }
 
     #[inline]
     fn len(&self) -> Result<u64, RuntimeError> {
-        len::<T>(&self.db, self.tx_rw)
+        len::<T>(&self.db, &self.tx_rw.borrow())
     }
 
     #[inline]
     fn first(&self) -> Result<(T::Key, T::Value), RuntimeError> {
-        first::<T>(&self.db, self.tx_rw)
+        first::<T>(&self.db, &self.tx_rw.borrow())
     }
 
     #[inline]
     fn last(&self) -> Result<(T::Key, T::Value), RuntimeError> {
-        last::<T>(&self.db, self.tx_rw)
+        last::<T>(&self.db, &self.tx_rw.borrow())
     }
 
     #[inline]
     fn is_empty(&self) -> Result<bool, RuntimeError> {
-        is_empty::<T>(&self.db, self.tx_rw)
+        is_empty::<T>(&self.db, &self.tx_rw.borrow())
     }
 }
 
 impl<T: Table> DatabaseRw<T> for HeedTableRw<'_, '_, T> {
     #[inline]
     fn put(&mut self, key: &T::Key, value: &T::Value) -> Result<(), RuntimeError> {
-        Ok(self.db.put(self.tx_rw, key, value)?)
+        Ok(self.db.put(&mut self.tx_rw.borrow_mut(), key, value)?)
     }
 
     #[inline]
     fn delete(&mut self, key: &T::Key) -> Result<(), RuntimeError> {
-        self.db.delete(self.tx_rw, key)?;
+        self.db.delete(&mut self.tx_rw.borrow_mut(), key)?;
         Ok(())
     }
 
     #[inline]
     fn pop_first(&mut self) -> Result<(T::Key, T::Value), RuntimeError> {
+        let tx_rw = &mut self.tx_rw.borrow_mut();
+
         // Get the first value first...
-        let Some(first) = self.db.first(self.tx_rw)? else {
+        let Some(first) = self.db.first(tx_rw)? else {
             return Err(RuntimeError::KeyNotFound);
         };
 
@@ -286,7 +219,7 @@ impl<T: Table> DatabaseRw<T> for HeedTableRw<'_, '_, T> {
         // remove the _first_ and only the first `(key, value)`.
         // `delete()` removes all keys including duplicates which
         // is slightly different behavior.
-        let mut iter = self.db.iter_mut(self.tx_rw)?;
+        let mut iter = self.db.iter_mut(tx_rw)?;
 
         // SAFETY:
         // It is undefined behavior to keep a reference of
@@ -302,11 +235,13 @@ impl<T: Table> DatabaseRw<T> for HeedTableRw<'_, '_, T> {
 
     #[inline]
     fn pop_last(&mut self) -> Result<(T::Key, T::Value), RuntimeError> {
-        let Some(first) = self.db.last(self.tx_rw)? else {
+        let tx_rw = &mut self.tx_rw.borrow_mut();
+
+        let Some(first) = self.db.last(tx_rw)? else {
             return Err(RuntimeError::KeyNotFound);
         };
 
-        let mut iter = self.db.rev_iter_mut(self.tx_rw)?;
+        let mut iter = self.db.rev_iter_mut(tx_rw)?;
 
         // SAFETY:
         // It is undefined behavior to keep a reference of
diff --git a/database/src/backend/heed/env.rs b/database/src/backend/heed/env.rs
index dcf0ddf7..d9e3fdc2 100644
--- a/database/src/backend/heed/env.rs
+++ b/database/src/backend/heed/env.rs
@@ -2,6 +2,7 @@
 
 //---------------------------------------------------------------------------------------------------- Import
 use std::{
+    cell::RefCell,
     fmt::Debug,
     ops::Deref,
     sync::{RwLock, RwLockReadGuard, RwLockWriteGuard},
@@ -16,7 +17,7 @@ use crate::{
         types::HeedDb,
     },
     config::{Config, SyncMode},
-    database::{DatabaseRo, DatabaseRw},
+    database::{DatabaseIter, DatabaseRo, DatabaseRw},
     env::{Env, EnvInner},
     error::{InitError, RuntimeError},
     resize::ResizeAlgorithm,
@@ -96,7 +97,23 @@ impl Env for ConcreteEnv {
     const SYNCS_PER_TX: bool = false;
     type EnvInner<'env> = RwLockReadGuard<'env, heed::Env>;
     type TxRo<'tx> = heed::RoTxn<'tx>;
-    type TxRw<'tx> = heed::RwTxn<'tx>;
+
+    /// HACK:
+    /// `heed::RwTxn` is wrapped in `RefCell` to allow:
+    /// - opening a database with only a `&` to it
+    /// - allowing 1 write tx to open multiple tables
+    ///
+    /// Our mutable accesses are safe and will not panic as:
+    /// - Write transactions are `!Sync`
+    /// - A table operation does not hold a reference to the inner cell
+    ///   once the call is over
+    /// - The function to manipulate the table takes the same type
+    ///   of reference that the `RefCell` gets for that function
+    ///
+    /// Also see:
+    /// - <https://github.com/Cuprate/cuprate/pull/102#discussion_r1548695610>
+    /// - <https://github.com/Cuprate/cuprate/pull/104>
+    type TxRw<'tx> = RefCell<heed::RwTxn<'tx>>;
 
     #[cold]
     #[inline(never)] // called once.
@@ -263,7 +280,8 @@ impl Env for ConcreteEnv {
 }
 
 //---------------------------------------------------------------------------------------------------- EnvInner Impl
-impl<'env> EnvInner<'env, heed::RoTxn<'env>, heed::RwTxn<'env>> for RwLockReadGuard<'env, heed::Env>
+impl<'env> EnvInner<'env, heed::RoTxn<'env>, RefCell<heed::RwTxn<'env>>>
+    for RwLockReadGuard<'env, heed::Env>
 where
     Self: 'env,
 {
@@ -273,15 +291,15 @@ where
     }
 
     #[inline]
-    fn tx_rw(&'env self) -> Result<heed::RwTxn<'env>, RuntimeError> {
-        Ok(self.write_txn()?)
+    fn tx_rw(&'env self) -> Result<RefCell<heed::RwTxn<'env>>, RuntimeError> {
+        Ok(RefCell::new(self.write_txn()?))
     }
 
     #[inline]
     fn open_db_ro<T: Table>(
         &self,
         tx_ro: &heed::RoTxn<'env>,
-    ) -> Result<impl DatabaseRo<T>, RuntimeError> {
+    ) -> Result<impl DatabaseRo<T> + DatabaseIter<T>, RuntimeError> {
         // Open up a read-only database using our table's const metadata.
         Ok(HeedTableRo {
             db: self
@@ -294,19 +312,26 @@ where
     #[inline]
     fn open_db_rw<T: Table>(
         &self,
-        tx_rw: &mut heed::RwTxn<'env>,
+        tx_rw: &RefCell<heed::RwTxn<'env>>,
     ) -> Result<impl DatabaseRw<T>, RuntimeError> {
+        let tx_ro = tx_rw.borrow();
+
         // Open up a read/write database using our table's const metadata.
         Ok(HeedTableRw {
             db: self
-                .open_database(tx_rw, Some(T::NAME))?
+                .open_database(&tx_ro, Some(T::NAME))?
                 .expect(PANIC_MSG_MISSING_TABLE),
             tx_rw,
         })
     }
 
     #[inline]
-    fn clear_db<T: Table>(&self, tx_rw: &mut heed::RwTxn<'env>) -> Result<(), RuntimeError> {
+    fn clear_db<T: Table>(
+        &self,
+        tx_rw: &mut RefCell<heed::RwTxn<'env>>,
+    ) -> Result<(), RuntimeError> {
+        let tx_rw = tx_rw.get_mut();
+
         // Open the table first...
         let db: HeedDb<T::Key, T::Value> = self
             .open_database(tx_rw, Some(T::NAME))?
diff --git a/database/src/backend/heed/transaction.rs b/database/src/backend/heed/transaction.rs
index 123328ee..43096980 100644
--- a/database/src/backend/heed/transaction.rs
+++ b/database/src/backend/heed/transaction.rs
@@ -1,6 +1,6 @@
 //! Implementation of `trait TxRo/TxRw` for `heed`.
 
-use std::{ops::Deref, sync::RwLockReadGuard};
+use std::{cell::RefCell, ops::Deref, sync::RwLockReadGuard};
 
 //---------------------------------------------------------------------------------------------------- Import
 use crate::{
@@ -11,25 +11,25 @@ use crate::{
 //---------------------------------------------------------------------------------------------------- TxRo
 impl TxRo<'_> for heed::RoTxn<'_> {
     fn commit(self) -> Result<(), RuntimeError> {
-        Ok(self.commit()?)
+        Ok(heed::RoTxn::commit(self)?)
     }
 }
 
 //---------------------------------------------------------------------------------------------------- TxRw
-impl TxRo<'_> for heed::RwTxn<'_> {
+impl TxRo<'_> for RefCell<heed::RwTxn<'_>> {
     fn commit(self) -> Result<(), RuntimeError> {
-        Ok(self.commit()?)
+        TxRw::commit(self)
     }
 }
 
-impl TxRw<'_> for heed::RwTxn<'_> {
+impl TxRw<'_> for RefCell<heed::RwTxn<'_>> {
     fn commit(self) -> Result<(), RuntimeError> {
-        Ok(self.commit()?)
+        Ok(heed::RwTxn::commit(self.into_inner())?)
     }
 
     /// This function is infallible.
     fn abort(self) -> Result<(), RuntimeError> {
-        self.abort();
+        heed::RwTxn::abort(self.into_inner());
         Ok(())
     }
 }
diff --git a/database/src/backend/redb/database.rs b/database/src/backend/redb/database.rs
index 853a91ce..43f1a3ac 100644
--- a/database/src/backend/redb/database.rs
+++ b/database/src/backend/redb/database.rs
@@ -8,12 +8,14 @@ use std::{
     ops::{Bound, Deref, RangeBounds},
 };
 
+use redb::ReadableTable;
+
 use crate::{
     backend::redb::{
         storable::StorableRedb,
         types::{RedbTableRo, RedbTableRw},
     },
-    database::{DatabaseRo, DatabaseRw},
+    database::{DatabaseIter, DatabaseRo, DatabaseRw},
     error::RuntimeError,
     storable::Storable,
     table::Table,
@@ -33,54 +35,6 @@ fn get<T: Table + 'static>(
     Ok(db.get(key)?.ok_or(RuntimeError::KeyNotFound)?.value())
 }
 
-/// Shared [`DatabaseRo::get_range()`].
-#[inline]
-fn get_range<'a, T: Table, Range>(
-    db: &'a impl redb::ReadableTable<StorableRedb<T::Key>, StorableRedb<T::Value>>,
-    range: Range,
-) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + 'a, RuntimeError>
-where
-    Range: RangeBounds<T::Key> + 'a,
-{
-    Ok(db.range(range)?.map(|result| {
-        let (_key, value) = result?;
-        Ok(value.value())
-    }))
-}
-
-/// Shared [`DatabaseRo::iter()`].
-#[inline]
-fn iter<T: Table>(
-    db: &impl redb::ReadableTable<StorableRedb<T::Key>, StorableRedb<T::Value>>,
-) -> Result<impl Iterator<Item = Result<(T::Key, T::Value), RuntimeError>> + '_, RuntimeError> {
-    Ok(db.iter()?.map(|result| {
-        let (key, value) = result?;
-        Ok((key.value(), value.value()))
-    }))
-}
-
-/// Shared [`DatabaseRo::iter()`].
-#[inline]
-fn keys<T: Table>(
-    db: &impl redb::ReadableTable<StorableRedb<T::Key>, StorableRedb<T::Value>>,
-) -> Result<impl Iterator<Item = Result<T::Key, RuntimeError>> + '_, RuntimeError> {
-    Ok(db.iter()?.map(|result| {
-        let (key, _value) = result?;
-        Ok(key.value())
-    }))
-}
-
-/// Shared [`DatabaseRo::values()`].
-#[inline]
-fn values<T: Table>(
-    db: &impl redb::ReadableTable<StorableRedb<T::Key>, StorableRedb<T::Value>>,
-) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + '_, RuntimeError> {
-    Ok(db.iter()?.map(|result| {
-        let (_key, value) = result?;
-        Ok(value.value())
-    }))
-}
-
 /// Shared [`DatabaseRo::len()`].
 #[inline]
 fn len<T: Table>(
@@ -115,13 +69,8 @@ fn is_empty<T: Table>(
     Ok(db.is_empty()?)
 }
 
-//---------------------------------------------------------------------------------------------------- DatabaseRo
-impl<T: Table + 'static> DatabaseRo<T> for RedbTableRo<T::Key, T::Value> {
-    #[inline]
-    fn get(&self, key: &T::Key) -> Result<T::Value, RuntimeError> {
-        get::<T>(self, key)
-    }
-
+//---------------------------------------------------------------------------------------------------- DatabaseIter
+impl<T: Table + 'static> DatabaseIter<T> for RedbTableRo<T::Key, T::Value> {
     #[inline]
     fn get_range<'a, Range>(
         &'a self,
@@ -130,7 +79,10 @@ impl<T: Table + 'static> DatabaseRo<T> for RedbTableRo<T::Key, T::Value> {
     where
         Range: RangeBounds<T::Key> + 'a,
     {
-        get_range::<T, Range>(self, range)
+        Ok(ReadableTable::range(self, range)?.map(|result| {
+            let (_key, value) = result?;
+            Ok(value.value())
+        }))
     }
 
     #[inline]
@@ -138,21 +90,38 @@ impl<T: Table + 'static> DatabaseRo<T> for RedbTableRo<T::Key, T::Value> {
         &self,
     ) -> Result<impl Iterator<Item = Result<(T::Key, T::Value), RuntimeError>> + '_, RuntimeError>
     {
-        iter::<T>(self)
+        Ok(ReadableTable::iter(self)?.map(|result| {
+            let (key, value) = result?;
+            Ok((key.value(), value.value()))
+        }))
     }
 
     #[inline]
     fn keys(
         &self,
     ) -> Result<impl Iterator<Item = Result<T::Key, RuntimeError>> + '_, RuntimeError> {
-        keys::<T>(self)
+        Ok(ReadableTable::iter(self)?.map(|result| {
+            let (key, _value) = result?;
+            Ok(key.value())
+        }))
     }
 
     #[inline]
     fn values(
         &self,
     ) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + '_, RuntimeError> {
-        values::<T>(self)
+        Ok(ReadableTable::iter(self)?.map(|result| {
+            let (_key, value) = result?;
+            Ok(value.value())
+        }))
+    }
+}
+
+//---------------------------------------------------------------------------------------------------- DatabaseRo
+impl<T: Table + 'static> DatabaseRo<T> for RedbTableRo<T::Key, T::Value> {
+    #[inline]
+    fn get(&self, key: &T::Key) -> Result<T::Value, RuntimeError> {
+        get::<T>(self, key)
     }
 
     #[inline]
@@ -183,39 +152,6 @@ impl<T: Table + 'static> DatabaseRo<T> for RedbTableRw<'_, T::Key, T::Value> {
         get::<T>(self, key)
     }
 
-    #[inline]
-    fn get_range<'a, Range>(
-        &'a self,
-        range: Range,
-    ) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + 'a, RuntimeError>
-    where
-        Range: RangeBounds<T::Key> + 'a,
-    {
-        get_range::<T, Range>(self, range)
-    }
-
-    #[inline]
-    fn iter(
-        &self,
-    ) -> Result<impl Iterator<Item = Result<(T::Key, T::Value), RuntimeError>> + '_, RuntimeError>
-    {
-        iter::<T>(self)
-    }
-
-    #[inline]
-    fn keys(
-        &self,
-    ) -> Result<impl Iterator<Item = Result<T::Key, RuntimeError>> + '_, RuntimeError> {
-        keys::<T>(self)
-    }
-
-    #[inline]
-    fn values(
-        &self,
-    ) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + '_, RuntimeError> {
-        values::<T>(self)
-    }
-
     #[inline]
     fn len(&self) -> Result<u64, RuntimeError> {
         len::<T>(self)
diff --git a/database/src/backend/redb/env.rs b/database/src/backend/redb/env.rs
index 74e55b4b..6525de62 100644
--- a/database/src/backend/redb/env.rs
+++ b/database/src/backend/redb/env.rs
@@ -9,7 +9,7 @@ use crate::{
         types::{RedbTableRo, RedbTableRw},
     },
     config::{Config, SyncMode},
-    database::{DatabaseRo, DatabaseRw},
+    database::{DatabaseIter, DatabaseRo, DatabaseRw},
     env::{Env, EnvInner},
     error::{InitError, RuntimeError},
     table::Table,
@@ -186,7 +186,7 @@ where
     fn open_db_ro<T: Table>(
         &self,
         tx_ro: &redb::ReadTransaction,
-    ) -> Result<impl DatabaseRo<T>, RuntimeError> {
+    ) -> Result<impl DatabaseRo<T> + DatabaseIter<T>, RuntimeError> {
         // Open up a read-only database using our `T: Table`'s const metadata.
         let table: redb::TableDefinition<'static, StorableRedb<T::Key>, StorableRedb<T::Value>> =
             redb::TableDefinition::new(T::NAME);
@@ -198,7 +198,7 @@ where
     #[inline]
     fn open_db_rw<T: Table>(
         &self,
-        tx_rw: &mut redb::WriteTransaction,
+        tx_rw: &redb::WriteTransaction,
     ) -> Result<impl DatabaseRw<T>, RuntimeError> {
         // Open up a read/write database using our `T: Table`'s const metadata.
         let table: redb::TableDefinition<'static, StorableRedb<T::Key>, StorableRedb<T::Value>> =
diff --git a/database/src/backend/tests.rs b/database/src/backend/tests.rs
index cf3bf4bc..50f92644 100644
--- a/database/src/backend/tests.rs
+++ b/database/src/backend/tests.rs
@@ -24,7 +24,7 @@ use std::borrow::{Borrow, Cow};
 
 use crate::{
     config::{Config, SyncMode},
-    database::{DatabaseRo, DatabaseRw},
+    database::{DatabaseIter, DatabaseRo, DatabaseRw},
     env::{Env, EnvInner},
     error::{InitError, RuntimeError},
     resize::ResizeAlgorithm,
@@ -81,7 +81,7 @@ fn open_db() {
     let (env, _tempdir) = tmp_concrete_env();
     let env_inner = env.env_inner();
     let tx_ro = env_inner.tx_ro().unwrap();
-    let mut tx_rw = env_inner.tx_rw().unwrap();
+    let tx_rw = env_inner.tx_rw().unwrap();
 
     // Open all tables in read-only mode.
     // This should be updated when tables are modified.
@@ -103,21 +103,21 @@ fn open_db() {
     TxRo::commit(tx_ro).unwrap();
 
     // Open all tables in read/write mode.
-    env_inner.open_db_rw::<BlockBlobs>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<BlockHeights>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<BlockInfoV1s>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<BlockInfoV2s>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<BlockInfoV3s>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<KeyImages>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<NumOutputs>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<Outputs>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<PrunableHashes>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<PrunableTxBlobs>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<PrunedTxBlobs>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<RctOutputs>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<TxHeights>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<TxIds>(&mut tx_rw).unwrap();
-    env_inner.open_db_rw::<TxUnlockTime>(&mut tx_rw).unwrap();
+    env_inner.open_db_rw::<BlockBlobs>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<BlockHeights>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<BlockInfoV1s>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<BlockInfoV2s>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<BlockInfoV3s>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<KeyImages>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<NumOutputs>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<Outputs>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<PrunableHashes>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<PrunableTxBlobs>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<PrunedTxBlobs>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<RctOutputs>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<TxHeights>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<TxIds>(&tx_rw).unwrap();
+    env_inner.open_db_rw::<TxUnlockTime>(&tx_rw).unwrap();
     TxRw::commit(tx_rw).unwrap();
 }
 
@@ -166,11 +166,12 @@ fn non_manual_resize_2() {
 
 /// Test all `DatabaseR{o,w}` operations.
 #[test]
+#[allow(clippy::too_many_lines)]
 fn db_read_write() {
     let (env, _tempdir) = tmp_concrete_env();
     let env_inner = env.env_inner();
-    let mut tx_rw = env_inner.tx_rw().unwrap();
-    let mut table = env_inner.open_db_rw::<Outputs>(&mut tx_rw).unwrap();
+    let tx_rw = env_inner.tx_rw().unwrap();
+    let mut table = env_inner.open_db_rw::<Outputs>(&tx_rw).unwrap();
 
     /// The (1st) key.
     const KEY: PreRctOutputId = PreRctOutputId {
@@ -221,9 +222,17 @@ fn db_read_write() {
         assert_same(last);
     }
 
+    // Commit transactions, create new ones.
+    drop(table);
+    TxRw::commit(tx_rw).unwrap();
+    let tx_ro = env_inner.tx_ro().unwrap();
+    let table_ro = env_inner.open_db_ro::<Outputs>(&tx_ro).unwrap();
+    let tx_rw = env_inner.tx_rw().unwrap();
+    let mut table = env_inner.open_db_rw::<Outputs>(&tx_rw).unwrap();
+
     // Assert the whole range is there.
     {
-        let range = table.get_range(..).unwrap();
+        let range = table_ro.get_range(..).unwrap();
         let mut i = 0;
         for result in range {
             let value: Output = result.unwrap();
@@ -240,11 +249,14 @@ fn db_read_write() {
     let range = KEY..key;
 
     // Assert count is correct.
-    assert_eq!(N as usize, table.get_range(range.clone()).unwrap().count());
+    assert_eq!(
+        N as usize,
+        table_ro.get_range(range.clone()).unwrap().count()
+    );
 
     // Assert each returned value from the iterator is owned.
     {
-        let mut iter = table.get_range(range.clone()).unwrap();
+        let mut iter = table_ro.get_range(range.clone()).unwrap();
         let value: Output = iter.next().unwrap().unwrap(); // 1. take value out
         drop(iter); // 2. drop the `impl Iterator + 'a`
         assert_same(value); // 3. assert even without the iterator, the value is alive
@@ -252,7 +264,7 @@ fn db_read_write() {
 
     // Assert each value is the same.
     {
-        let mut iter = table.get_range(range).unwrap();
+        let mut iter = table_ro.get_range(range).unwrap();
         for _ in 0..N {
             let value: Output = iter.next().unwrap().unwrap();
             assert_same(value);
@@ -273,17 +285,13 @@ fn db_read_write() {
     }
 
     drop(table);
-    tx_rw.commit().unwrap();
-    let mut tx_rw = env_inner.tx_rw().unwrap();
+    TxRw::commit(tx_rw).unwrap();
 
     // Assert `clear_db()` works.
     {
-        // Make sure this works even if readers have the table open.
-        let tx_ro = env_inner.tx_ro().unwrap();
-        let reader_table = env_inner.open_db_ro::<Outputs>(&tx_ro).unwrap();
-
+        let mut tx_rw = env_inner.tx_rw().unwrap();
         env_inner.clear_db::<Outputs>(&mut tx_rw).unwrap();
-        let table = env_inner.open_db_rw::<Outputs>(&mut tx_rw).unwrap();
+        let table = env_inner.open_db_rw::<Outputs>(&tx_rw).unwrap();
         assert!(table.is_empty().unwrap());
         for n in 0..N {
             let mut key = KEY;
@@ -294,7 +302,7 @@ fn db_read_write() {
         }
 
         // Reader still sees old value.
-        assert!(!reader_table.is_empty().unwrap());
+        assert!(!table_ro.is_empty().unwrap());
 
         // Writer sees updated value (nothing).
         assert!(table.is_empty().unwrap());
@@ -345,11 +353,19 @@ macro_rules! test_tables {
             assert!(table.contains(&KEY).unwrap());
             assert_eq!(table.len().unwrap(), 1);
 
+            // Commit transactions, create new ones.
+            drop(table);
+            TxRw::commit(tx_rw).unwrap();
+            let mut tx_rw = env_inner.tx_rw().unwrap();
+            let tx_ro = env_inner.tx_ro().unwrap();
+            let mut table = env_inner.open_db_rw::<$table>(&tx_rw).unwrap();
+            let table_ro = env_inner.open_db_ro::<$table>(&tx_ro).unwrap();
+
             // Assert `get_range()` works.
             {
                 let range = KEY..;
-                assert_eq!(1, table.get_range(range.clone()).unwrap().count());
-                let mut iter = table.get_range(range).unwrap();
+                assert_eq!(1, table_ro.get_range(range.clone()).unwrap().count());
+                let mut iter = table_ro.get_range(range).unwrap();
                 let value = iter.next().unwrap().unwrap();
                 assert_eq(&value);
             }
diff --git a/database/src/database.rs b/database/src/database.rs
index 6b711988..df62414b 100644
--- a/database/src/database.rs
+++ b/database/src/database.rs
@@ -13,22 +13,20 @@ use crate::{
     transaction::{TxRo, TxRw},
 };
 
-//---------------------------------------------------------------------------------------------------- DatabaseRo
-/// Database (key-value store) read abstraction.
+//---------------------------------------------------------------------------------------------------- DatabaseRoIter
+/// Database (key-value store) read-only iteration abstraction.
 ///
-/// This is a read-only database table,
-/// write operations are defined in [`DatabaseRw`].
-pub trait DatabaseRo<T: Table> {
-    /// Get the value corresponding to a key.
-    ///
-    /// The returned value is _owned_.
-    ///
-    /// # Errors
-    /// This will return [`RuntimeError::KeyNotFound`] wrapped in [`Err`] if `key` does not exist.
-    ///
-    /// It will return other [`RuntimeError`]'s on things like IO errors as well.
-    fn get(&self, key: &T::Key) -> Result<T::Value, RuntimeError>;
-
+/// These are read-only iteration-related operations that
+/// can only be called from [`DatabaseRo`] objects.
+///
+/// # Hack
+/// This is a HACK to get around the fact our read/write tables
+/// cannot safely return values returning lifetimes, as such,
+/// only read-only tables implement this trait.
+///
+/// - <https://github.com/Cuprate/cuprate/pull/102#discussion_r1548695610>
+/// - <https://github.com/Cuprate/cuprate/pull/104>
+pub trait DatabaseIter<T: Table> {
     /// Get an iterator of value's corresponding to a range of keys.
     ///
     /// For example:
@@ -77,6 +75,23 @@ pub trait DatabaseRo<T: Table> {
     fn values(
         &self,
     ) -> Result<impl Iterator<Item = Result<T::Value, RuntimeError>> + '_, RuntimeError>;
+}
+
+//---------------------------------------------------------------------------------------------------- DatabaseRo
+/// Database (key-value store) read abstraction.
+///
+/// This is a read-only database table,
+/// write operations are defined in [`DatabaseRw`].
+pub trait DatabaseRo<T: Table> {
+    /// Get the value corresponding to a key.
+    ///
+    /// The returned value is _owned_.
+    ///
+    /// # Errors
+    /// This will return [`RuntimeError::KeyNotFound`] wrapped in [`Err`] if `key` does not exist.
+    ///
+    /// It will return other [`RuntimeError`]'s on things like IO errors as well.
+    fn get(&self, key: &T::Key) -> Result<T::Value, RuntimeError>;
 
     /// TODO
     ///
diff --git a/database/src/env.rs b/database/src/env.rs
index 65f1c479..26adc975 100644
--- a/database/src/env.rs
+++ b/database/src/env.rs
@@ -5,7 +5,7 @@ use std::{fmt::Debug, ops::Deref};
 
 use crate::{
     config::Config,
-    database::{DatabaseRo, DatabaseRw},
+    database::{DatabaseIter, DatabaseRo, DatabaseRw},
     error::{InitError, RuntimeError},
     resize::ResizeAlgorithm,
     table::Table,
@@ -201,7 +201,10 @@ where
     /// As [`Table`] is `Sealed`, and all tables are created
     /// upon [`Env::open`], this function will never error because
     /// a table doesn't exist.
-    fn open_db_ro<T: Table>(&self, tx_ro: &Ro) -> Result<impl DatabaseRo<T>, RuntimeError>;
+    fn open_db_ro<T: Table>(
+        &self,
+        tx_ro: &Ro,
+    ) -> Result<impl DatabaseRo<T> + DatabaseIter<T>, RuntimeError>;
 
     /// Open a database in read/write mode.
     ///
@@ -217,7 +220,7 @@ where
     /// As [`Table`] is `Sealed`, and all tables are created
     /// upon [`Env::open`], this function will never error because
     /// a table doesn't exist.
-    fn open_db_rw<T: Table>(&self, tx_rw: &mut Rw) -> Result<impl DatabaseRw<T>, RuntimeError>;
+    fn open_db_rw<T: Table>(&self, tx_rw: &Rw) -> Result<impl DatabaseRw<T>, RuntimeError>;
 
     /// Clear all `(key, value)`'s from a database table.
     ///
diff --git a/database/src/lib.rs b/database/src/lib.rs
index 51315f20..a80647f0 100644
--- a/database/src/lib.rs
+++ b/database/src/lib.rs
@@ -222,7 +222,7 @@ pub use constants::{
 };
 
 mod database;
-pub use database::{DatabaseRo, DatabaseRw};
+pub use database::{DatabaseIter, DatabaseRo, DatabaseRw};
 
 mod env;
 pub use env::{Env, EnvInner};