Merge branch 'main' into rkuris/simple-range-proof-verification

Author: rkuris

.github/workflows/ci.yaml

@@ -35,6 +35,8 @@ jobs:
             os: macos-latest
           - profile-key: debug-ethhash-logger
             profile-args: "--features ethhash,logger"
+          - profile-key: debug-all-features
+            profile-args: "--all-features"
           - profile-key: maxperf-ethhash-logger
             profile-args: "--profile maxperf --features ethhash,logger"
     runs-on: ${{ matrix.os || 'ubuntu-latest' }}
@@ -89,6 +91,8 @@ jobs:
             os: macos-latest
           - profile-key: debug-ethhash-logger
             profile-args: "--features ethhash,logger"
+          - profile-key: debug-all-features
+            profile-args: "--all-features"
     runs-on: ${{ matrix.os || 'ubuntu-latest' }}
     steps:
       - uses: actions/checkout@v4
@@ -121,6 +125,8 @@ jobs:
             os: macos-latest
           - profile-key: debug-ethhash-logger
             profile-args: "--features ethhash,logger"
+          - profile-key: debug-all-features
+            profile-args: "--all-features"
     runs-on: ${{ matrix.os || 'ubuntu-latest' }}
     steps:
       - uses: actions/checkout@v4
@@ -150,6 +156,8 @@ jobs:
             os: macos-latest
           - profile-key: debug-ethhash-logger
             profile-args: "--features ethhash,logger"
+          - profile-key: debug-all-features
+            profile-args: "--all-features"
           - profile-key: maxperf-ethhash-logger
             profile-args: "--cargo-profile maxperf --features ethhash,logger"
     runs-on: ${{ matrix.os || 'ubuntu-latest' }}
@@ -178,6 +186,8 @@ jobs:
             os: macos-latest
           - profile-key: debug-ethhash-logger
             profile-args: "--features ethhash,logger"
+          - profile-key: debug-all-features
+            profile-args: "--all-features"
           - profile-key: maxperf-ethhash-logger
             profile-args: "--profile maxperf --features ethhash,logger"
     runs-on: ${{ matrix.os || 'ubuntu-latest' }}

clippy.toml

@@ -11,6 +11,7 @@ doc-valid-idents = [
 
 disallowed-methods = [
   { path = "rand::rng", replacement = "firewood_storage::SeededRng::from_env_or_random", reason = "use a prng with a user-defined seed instead", allow-invalid = true },
+  { path = "std::os::unix::fs::FileExt::write_at", replacement = "write_all_at", reason = "use write_all_at instead of write_at to handle short writes ensuring the entire buffer is written", allow-invalid = true },
 ]
 
 disallowed-types = [

firewood/src/db.rs

@@ -224,7 +224,8 @@ impl Db {
     /// Check the database for consistency
     pub fn check(&self, opt: CheckOpt) -> CheckerReport {
         let latest_rev_nodestore = self.manager.current_revision();
-        latest_rev_nodestore.check(opt)
+        let header = self.manager.locked_header();
+        latest_rev_nodestore.check(&header, opt)
     }
 
     /// Create a proposal with a specified parent. A proposal is created in parallel if `use_parallel`
@@ -1284,7 +1285,7 @@ mod test {
         assert_eq!(&version, b"firewood-v1\0\0\0\0\0");
 
         // overwrite the magic string to simulate an older version
-        file.write_at(b"firewood 0.0.18\0", 0).unwrap();
+        file.write_all_at(b"firewood 0.0.18\0", 0).unwrap();
         drop(file);
 
         let testdb = testdb.reopen();

firewood/src/manager.rs

@@ -10,7 +10,7 @@
     reason = "Found 3 occurrences after enabling the lint."
 )]
 
-use parking_lot::{Mutex, RwLock};
+use parking_lot::{Mutex, MutexGuard, RwLock};
 use std::collections::{HashMap, VecDeque};
 use std::io;
 use std::num::NonZero;
@@ -29,7 +29,7 @@ use firewood_metrics::{firewood_increment, firewood_set};
 pub use firewood_storage::CacheReadStrategy;
 use firewood_storage::{
     BranchNode, Committed, FileBacked, FileIoError, HashedNodeReader, ImmutableProposal,
-    NodeHashAlgorithm, NodeStore, TrieHash,
+    NodeHashAlgorithm, NodeStore, NodeStoreHeader, TrieHash,
 };
 
 pub(crate) const DB_FILE_NAME: &str = "firewood.db";
@@ -89,6 +89,12 @@ pub(crate) struct RevisionManager {
     /// is preserved on disk for historical queries.
     max_revisions: usize,
 
+    /// Persisted metadata for the database.
+    ///
+    /// Loaded from disk on startup and updated during commits when nodes
+    /// are persisted or deleted nodes are added to the free lists.
+    header: Mutex<NodeStoreHeader>,
+
     /// FIFO queue of committed revisions kept in memory. The queue always
     /// contains at least one revision.
     in_memory_revisions: RwLock<VecDeque<CommittedRevision>>,
@@ -157,7 +163,15 @@ impl RevisionManager {
         fb.lock()?;
 
         let storage = Arc::new(fb);
-        let nodestore = Arc::new(NodeStore::open(storage.clone())?);
+        let header = match NodeStoreHeader::read_from_storage(storage.as_ref()) {
+            Ok(header) => header,
+            Err(err) if err.kind() == io::ErrorKind::UnexpectedEof => {
+                // Empty file - create a new header for a fresh database
+                NodeStoreHeader::new(config.node_hash_algorithm)
+            }
+            Err(err) => return Err(err.into()),
+        };
+        let nodestore = Arc::new(NodeStore::open(&header, storage.clone())?);
         let root_store = config
             .root_store
             .then(|| {
@@ -167,23 +181,26 @@ impl RevisionManager {
             })
             .transpose()?;
 
+        if config.truncate {
+            header.flush_to(storage.as_ref())?;
+            storage.set_len(NodeStoreHeader::SIZE)?;
+        }
+
+        let mut by_hash = HashMap::new();
+        if let Some(hash) = nodestore.root_hash().or_default_root_hash() {
+            by_hash.insert(hash, nodestore.clone());
+        }
+
         let manager = Self {
             max_revisions: config.manager.max_revisions,
+            header: Mutex::new(header),
             in_memory_revisions: RwLock::new(VecDeque::from([nodestore.clone()])),
-            by_hash: RwLock::new(Default::default()),
+            by_hash: RwLock::new(by_hash),
             proposals: Mutex::new(Default::default()),
             threadpool: OnceLock::new(),
             root_store,
         };
 
-        if let Some(hash) = nodestore.root_hash().or_default_root_hash() {
-            manager.by_hash.write().insert(hash, nodestore.clone());
-        }
-
-        if config.truncate {
-            nodestore.flush_header_with_padding()?;
-        }
-
         // On startup, we always write the latest revision to RootStore
         if let Some(root_hash) = manager.current_revision().root_hash() {
             let root_address = manager.current_revision().root_address().ok_or(
@@ -231,7 +248,10 @@ impl RevisionManager {
             });
         }
 
-        let mut committed = proposal.as_committed(&current_revision);
+        let committed = proposal.as_committed();
+
+        // Lock header for the duration of reaping and persistence
+        let mut header = self.header.lock();
 
         // 2. Revision reaping
         // When we exceed max_revisions, remove the oldest revision from memory.
@@ -257,7 +277,7 @@ impl RevisionManager {
                 // This guarantee is there because we have a `&mut self` reference to the manager, so
                 // the compiler guarantees we are the only one using this manager.
                 match Arc::try_unwrap(oldest) {
-                    Ok(oldest) => oldest.reap_deleted(&mut committed)?,
+                    Ok(oldest) => oldest.reap_deleted(&mut header)?,
                     Err(original) => {
                         warn!("Oldest revision could not be reaped; still referenced");
                         self.in_memory_revisions.write().push_front(original);
@@ -273,9 +293,7 @@ impl RevisionManager {
         }
 
         // 3. Persist to disk.
-        // TODO: We can probably do this in another thread, but it requires that
-        // we move the header out of NodeStore, which is in a future PR.
-        committed.persist()?;
+        committed.persist(&mut header)?;
 
         // 4. Persist revision to root store
         if let Some(store) = &self.root_store
@@ -298,6 +316,10 @@ impl RevisionManager {
             self.by_hash.write().insert(hash, committed.clone());
         }
 
+        // At this point, we can release the lock on the header as the header
+        // and the last committed revision are up-to-date.
+        drop(header);
+
         // 6. Proposal Cleanup
         // Free proposal that is being committed as well as any proposals no longer
         // referenced by anyone else. Track how many were discarded (dropped without commit).
@@ -405,6 +427,11 @@ impl RevisionManager {
             .clone()
     }
 
+    /// Acquires a lock on the header and returns a guard.
+    pub(crate) fn locked_header(&self) -> MutexGuard<'_, NodeStoreHeader> {
+        self.header.lock()
+    }
+
     /// Gets or creates a threadpool associated with the revision manager.
     ///
     /// # Panics

firewood/src/merkle/tests/ethhash.rs

@@ -149,7 +149,7 @@ fn test_root_hash_random_deletions() {
         items_ordered.sort_unstable();
         items_ordered.shuffle(&mut &rng);
 
-        let mut committed_merkle = init_merkle(&items);
+        let (mut committed_merkle, mut header) = init_merkle_with_header(&items);
 
         for (k, v) in items_ordered {
             let mut merkle = committed_merkle.fork().unwrap();
@@ -166,7 +166,7 @@ fn test_root_hash_random_deletions() {
                 assert_eq!(merkle.get_value(k).unwrap().as_deref(), Some(v.as_ref()));
             }
 
-            committed_merkle = into_committed(merkle.hash(), committed_merkle.nodestore());
+            committed_merkle = into_committed(merkle.hash(), &mut header);
 
             let h: TrieHash = KeccakHasher::trie_root(&items).to_fixed_bytes().into();
 

firewood/src/merkle/tests/mod.rs

@@ -16,7 +16,8 @@ use std::fmt::Write;
 
 use super::*;
 use firewood_storage::{
-    Committed, MemStore, MutableProposal, NodeHashAlgorithm, NodeStore, RootReader, TrieHash,
+    Committed, MemStore, MutableProposal, NodeHashAlgorithm, NodeStore, NodeStoreHeader,
+    RootReader, TrieHash,
 };
 
 // Returns n random key-value pairs.
@@ -37,14 +38,26 @@ fn generate_random_kvs(rng: &firewood_storage::SeededRng, n: usize) -> Vec<(Vec<
 
 fn into_committed(
     merkle: Merkle<NodeStore<Arc<ImmutableProposal>, MemStore>>,
-    parent: &NodeStore<Committed, MemStore>,
+    header: &mut NodeStoreHeader,
 ) -> Merkle<NodeStore<Committed, MemStore>> {
-    let mut ns = merkle.into_inner().as_committed(parent);
-    ns.persist().unwrap();
+    let ns = merkle.into_inner().as_committed();
+    ns.persist(header).unwrap();
     ns.into()
 }
 
 pub(crate) fn init_merkle<I, K, V>(iter: I) -> Merkle<NodeStore<Committed, MemStore>>
+where
+    I: Clone + IntoIterator<Item = (K, V)>,
+    K: AsRef<[u8]>,
+    V: AsRef<[u8]>,
+{
+    let (merkle, _header) = init_merkle_with_header(iter);
+    merkle
+}
+
+pub(crate) fn init_merkle_with_header<I, K, V>(
+    iter: I,
+) -> (Merkle<NodeStore<Committed, MemStore>>, NodeStoreHeader)
 where
     I: Clone + IntoIterator<Item = (K, V)>,
     K: AsRef<[u8]>,
@@ -54,6 +67,7 @@ where
         Vec::with_capacity(64 * 1024),
         NodeHashAlgorithm::compile_option(),
     ));
+    let mut header = NodeStoreHeader::new(NodeHashAlgorithm::compile_option());
     let base = Merkle::from(NodeStore::new_empty_committed(memstore.clone()));
     let mut merkle = base.fork().unwrap();
 
@@ -65,7 +79,7 @@ where
     }
 
     let merkle = merkle.hash();
-    let merkle = into_committed(merkle, base.nodestore());
+    let merkle = into_committed(merkle, &mut header);
 
     // Single verification pass at the end to ensure correctness
     for (k, v) in iter {
@@ -79,7 +93,7 @@ where
         );
     }
 
-    merkle
+    (merkle, header)
 }
 
 // generate pseudorandom data, but prefix it with some known data

fwdctl/src/check.rs

@@ -8,7 +8,9 @@ use std::sync::Arc;
 use askama::Template;
 use clap::Args;
 use firewood::v2::api;
-use firewood_storage::{CacheReadStrategy, CheckOpt, DBStats, FileBacked, NodeStore};
+use firewood_storage::{
+    CacheReadStrategy, CheckOpt, DBStats, FileBacked, NodeStore, NodeStoreHeader,
+};
 use indicatif::{ProgressBar, ProgressFinish, ProgressStyle};
 use nonzero_ext::nonzero;
 use num_format::{Locale, ToFormattedString};
@@ -69,8 +71,9 @@ pub(super) fn run(opts: &Options) -> Result<(), api::Error> {
         progress_bar: Some(progress_bar),
     };
 
-    let nodestore = NodeStore::open(storage)?;
-    let check_report = nodestore.check(check_ops);
+    let mut header = NodeStoreHeader::read_from_storage(storage.as_ref())?;
+    let nodestore = NodeStore::open(&header, storage)?;
+    let check_report = nodestore.check(&header, check_ops);
 
     println!("Errors ({}): ", check_report.errors.len());
     for error in &check_report.errors {
@@ -79,7 +82,7 @@ pub(super) fn run(opts: &Options) -> Result<(), api::Error> {
 
     let mut db_stats = check_report.db_stats.clone();
     if opts.fix {
-        match nodestore.fix(check_report) {
+        match nodestore.fix(&mut header, check_report) {
             Ok(report) => {
                 println!("Fixed Errors ({}):", report.fixed.len());
                 for error in report.fixed {