feat(reconstruction): Mutable->reconstruted conversions

Also adds tests

Author: rkuris

firewood/src/db.rs

@@ -499,18 +499,60 @@ impl api::Reconstructible for Reconstructed {
     where
         Self: Sized,
     {
-        reconstruct_with_parent(&*self.nodestore, batch)
+        reconstruct_with_reconstructed(self.nodestore, batch)
     }
 }
 
+/// Reconstruction path for a `Reconstructed` view.
+///
+/// Converts `Reconstructed` → `MutableProposal` using the optimized `TryFrom` impl
+/// that moves the root Node when possible and clones otherwise, then applies the batch.
+fn reconstruct_with_reconstructed(
+    nodestore: Arc<NodeStore<StorageReconstructed, FileBacked>>,
+    batch: impl IntoBatchIter,
+) -> Result<Reconstructed, api::Error> {
+    // The TryFrom<Arc<NodeStore<Reconstructed>>> impl extracts and moves the root
+    // Node when uniquely owned, or clones when shared.
+    let proposal =
+        NodeStore::<MutableProposal, _>::try_from(nodestore).map_err(api::Error::from)?;
+
+    let mut merkle = Merkle::from(proposal);
+
+    for res in batch.into_iter().into_batch_iter::<api::Error>() {
+        match res? {
+            BatchOp::Put { key, value } => {
+                merkle.insert(key.as_ref(), value.as_ref().into())?;
+            }
+            BatchOp::Delete { key } => {
+                merkle.remove(key.as_ref())?;
+            }
+            BatchOp::DeleteRange { prefix } => {
+                merkle.remove_prefix(prefix.as_ref())?;
+            }
+        }
+    }
+
+    let nodestore = merkle.into_inner();
+    Ok(Reconstructed {
+        nodestore: Arc::new(nodestore.try_into()?),
+    })
+}
+
+/// Reconstruction path for a `DbView` (or other parent-backed view).
+///
+/// Creates a new `NodeStore` from the parent, which requires cloning the root node.
+/// This is significantly slower than the reconstructed path due to the clone.
 fn reconstruct_with_parent<P>(
     parent: &NodeStore<P, FileBacked>,
     batch: impl IntoBatchIter,
 ) -> Result<Reconstructed, api::Error>
 where
+    P: Parentable,
     NodeStore<P, FileBacked>: TrieReader + HashedNodeReader,
 {
-    let proposal = NodeStore::<MutableProposal, _>::new_for_reconstruction(parent)?;
+    // This clones the root node when creating a mutable nodestore from a parent view.
+    let proposal = NodeStore::<MutableProposal, _>::new(parent)?;
+
     let mut merkle = Merkle::from(proposal);
 
     for res in batch.into_iter().into_batch_iter::<api::Error>() {
@@ -526,11 +568,9 @@ where
             }
         }
     }
-
     let nodestore = merkle.into_inner();
-    let reconstructed = Arc::new(nodestore.try_into()?);
     Ok(Reconstructed {
-        nodestore: reconstructed,
+        nodestore: Arc::new(nodestore.try_into()?),
     })
 }
 
@@ -553,7 +593,7 @@ mod test {
 
     use crate::db::{Db, Proposal, UseParallel};
     use crate::manager::RevisionManagerConfig;
-    use crate::v2::api::{Db as _, DbView, HashKeyExt, Proposal as _};
+    use crate::v2::api::{Db as _, DbView, HashKeyExt, Proposal as _, Reconstructible as _};
 
     use super::{BatchOp, DbConfig};
 
@@ -625,6 +665,41 @@ mod test {
         assert_eq!(&*historical.val(b"k").unwrap().unwrap(), b"v");
     }
 
+    #[test]
+    fn test_reconstruct_reads_and_chains() {
+        let db = TestDb::new();
+
+        let initial = db
+            .propose(vec![BatchOp::Put {
+                key: b"base",
+                value: b"v0",
+            }])
+            .unwrap();
+        initial.commit().unwrap();
+
+        let historical_hash = db.root_hash().unwrap();
+        let historical = db.revision(historical_hash).unwrap();
+
+        let reconstructed = historical
+            .as_ref()
+            .reconstruct(vec![BatchOp::Put {
+                key: b"base",
+                value: b"v1",
+            }])
+            .unwrap();
+        assert_eq!(&*reconstructed.val(b"base").unwrap().unwrap(), b"v1");
+
+        let reconstructed = reconstructed
+            .reconstruct(vec![BatchOp::Put {
+                key: b"next",
+                value: b"v2",
+            }])
+            .unwrap();
+
+        assert_eq!(&*reconstructed.val(b"base").unwrap().unwrap(), b"v1");
+        assert_eq!(&*reconstructed.val(b"next").unwrap().unwrap(), b"v2");
+    }
+
     #[test]
     fn reopen_test() {
         let db = TestDb::new();

firewood/src/v2/api.rs

@@ -418,7 +418,16 @@ pub trait Reconstructible: DbView {
     type Reconstructed: DbView + Reconstructible<Reconstructed = Self::Reconstructed>;
 
     /// Reconstruct a new view from this one by applying `data`.
-    #[expect(clippy::missing_errors_doc)]
+    ///
+    /// If the current view is shared (for example, by holding a `view()` clone),
+    /// reconstruction may need to clone the root node or read it from storage.
+    /// For best performance, drop other references to the view before calling
+    /// `reconstruct` in a chain.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if applying the batch fails or if the underlying storage
+    /// cannot be read while resolving nodes.
     fn reconstruct(self, data: impl IntoBatchIter) -> Result<Self::Reconstructed, Error>
     where
         Self: Sized;

storage/src/node/persist.rs

@@ -2,9 +2,9 @@
 // See the file LICENSE.md for licensing terms.
 
 use parking_lot::Mutex;
-use std::{fmt::Display, sync::Arc};
+use std::{fmt::Display, ops::Deref, sync::Arc};
 
-use crate::{FileIoError, LinearAddress, NodeReader, SharedNode};
+use crate::{FileIoError, LinearAddress, Node, NodeReader, SharedNode};
 
 /// A node that is either in memory or on disk.
 ///
@@ -95,6 +95,40 @@ impl MaybePersistedNode {
         }
     }
 
+    /// Resolve this node into a concrete `Node`, performing I/O when needed.
+    ///
+    /// Attempts to move the in-memory node without cloning when this is the sole
+    /// reference. If multiple references exist, it falls back to cloning the
+    /// node (similar to `reconstruct_with_parent`). If the node is persisted,
+    /// it reads from storage.
+    pub fn try_into_node<S: NodeReader>(self, storage: &S) -> Result<Node, FileIoError> {
+        fn resolve_state<S: NodeReader>(
+            state: MaybePersisted,
+            storage: &S,
+        ) -> Result<Node, FileIoError> {
+            match state {
+                MaybePersisted::Unpersisted(shared_node)
+                | MaybePersisted::Allocated(_, shared_node) => {
+                    match triomphe::Arc::try_unwrap(shared_node) {
+                        Ok(node) => Ok(node),
+                        Err(shared) => Ok(shared.deref().clone()),
+                    }
+                }
+                MaybePersisted::Persisted(address) => {
+                    let shared = storage.read_node(address)?;
+                    Ok(shared.deref().clone())
+                }
+            }
+        }
+
+        match Arc::try_unwrap(self.0) {
+            Ok(mutex) => resolve_state(mutex.into_inner(), storage),
+            Err(arc) => {
+                let shared = Self(arc).as_shared_node(storage)?;
+                Ok(shared.deref().clone())
+            }
+        }
+    }
     /// Returns the linear address of the node if it is persisted on disk.
     ///
     /// # Returns

storage/src/nodestore/mod.rs

@@ -562,6 +562,79 @@ impl<T: Into<NodeStoreParent>, S: ReadableStorage> From<NodeStore<T, S>>
     }
 }
 
+fn resolve_reconstructed_root<R: NodeReader>(
+    root_child: Option<Child>,
+    reader: &R,
+) -> Result<Option<Node>, FileIoError> {
+    match root_child {
+        Some(Child::Node(node)) => Ok(Some(node)),
+        Some(Child::MaybePersisted(maybe_persisted, _hash)) => {
+            Ok(Some(maybe_persisted.try_into_node(reader)?))
+        }
+        Some(Child::AddressWithHash(address, _hash)) => {
+            let shared = reader.read_node(address)?;
+            Ok(Some(shared.deref().clone()))
+        }
+        None => Ok(None),
+    }
+}
+
+impl<S: ReadableStorage> TryFrom<NodeStore<Reconstructed, S>> for NodeStore<MutableProposal, S> {
+    type Error = FileIoError;
+
+    fn try_from(val: NodeStore<Reconstructed, S>) -> Result<Self, Self::Error> {
+        // Extract the root without cloning - just move it!
+        let root_child = val.kind.root;
+        let reader = NodeStore {
+            kind: Reconstructed { root: None },
+            storage: val.storage.clone(),
+        };
+        let root = resolve_reconstructed_root(root_child, &reader)?;
+
+        Ok(NodeStore {
+            kind: MutableProposal {
+                root,
+                // Reconstructed chains never persist, so we don't need to track deletions
+                deleted: Vec::default(),
+                parent: NodeStoreParent::Committed(None),
+            },
+            storage: val.storage,
+        })
+    }
+}
+
+impl<S: ReadableStorage> TryFrom<&NodeStore<Reconstructed, S>> for NodeStore<MutableProposal, S> {
+    type Error = FileIoError;
+
+    fn try_from(val: &NodeStore<Reconstructed, S>) -> Result<Self, Self::Error> {
+        let root_child = val.kind.root.clone();
+        let root = resolve_reconstructed_root(root_child, val)?;
+
+        Ok(NodeStore {
+            kind: MutableProposal {
+                root,
+                // Reconstructed chains never persist, so we don't need to track deletions
+                deleted: Vec::default(),
+                parent: NodeStoreParent::Committed(None),
+            },
+            storage: val.storage.clone(),
+        })
+    }
+}
+
+impl<S: ReadableStorage> TryFrom<Arc<NodeStore<Reconstructed, S>>>
+    for NodeStore<MutableProposal, S>
+{
+    type Error = FileIoError;
+
+    fn try_from(val: Arc<NodeStore<Reconstructed, S>>) -> Result<Self, Self::Error> {
+        match Arc::try_unwrap(val) {
+            Ok(owned) => Self::try_from(owned),
+            Err(shared) => Self::try_from(&*shared),
+        }
+    }
+}
+
 /// Commit a proposal to a new revision of the trie
 impl<S: WritableStorage> From<NodeStore<ImmutableProposal, S>> for NodeStore<Committed, S> {
     fn from(val: NodeStore<ImmutableProposal, S>) -> Self {
@@ -642,11 +715,26 @@ impl<S: ReadableStorage> TryFrom<NodeStore<MutableProposal, S>> for NodeStore<Re
     type Error = FileIoError;
 
     fn try_from(val: NodeStore<MutableProposal, S>) -> Result<Self, Self::Error> {
-        let NodeStore {
-            kind: _kind,
-            storage: _storage,
-        } = val;
-        todo!("convert mutable proposal into reconstructed nodestore")
+        let NodeStore { kind, storage } = val;
+
+        let mut nodestore = NodeStore {
+            kind: Reconstructed { root: None },
+            storage,
+        };
+
+        let Some(root) = kind.root else {
+            return Ok(nodestore);
+        };
+
+        // Hash the trie with an empty path and keep the reconstructed root in-memory.
+        #[cfg(feature = "ethhash")]
+        let (root, root_hash) = nodestore.hash_helper(root, Path::new())?;
+        #[cfg(not(feature = "ethhash"))]
+        let (root, root_hash) = NodeStore::<MutableProposal, S>::hash_helper(root, Path::new())?;
+
+        nodestore.kind.root = Some(Child::MaybePersisted(root, root_hash));
+
+        Ok(nodestore)
     }
 }