Repository: paradigmxyz/reth (language: rust)

Issue:
Remove weird ProviderError variants
### Describe the feature

due to some circular dep and no-std constraints we have these:

https://github.com/paradigmxyz/reth/blob/b7dfd5447f521937bc8c9c82b70a6b841c358940/crates/storage/errors/src/provider.rs#L30-L35

for these we should introduce a new error struct where this is initialized

https://github.com/paradigmxyz/reth/blob/b7dfd5447f521937bc8c9c82b70a6b841c358940/crates/storage/errors/src/provider.rs#L87-L89


https://github.com/paradigmxyz/reth/blob/b7dfd5447f521937bc8c9c82b70a6b841c358940/crates/storage/errors/src/provider.rs#L114-L117

## TODO
* remove these error variants and use:

https://github.com/paradigmxyz/reth/blob/b7dfd5447f521937bc8c9c82b70a6b841c358940/crates/storage/errors/src/provider.rs#L154-L161

instead, e.g.

https://github.com/paradigmxyz/reth/blob/b7dfd5447f521937bc8c9c82b70a6b841c358940/crates/storage/db/src/static_file/cursor.rs#L22-L22

would become `ProviderError::other`

these changes can be done 1 variant at a time

### Additional context

_No response_


Discussion / hints:
Hey @mattsse can i work on this one?

Fix patch:
diff --git a/crates/storage/db/src/static_file/cursor.rs b/crates/storage/db/src/static_file/cursor.rs
index a9eadd9c12a6..814da063adf3 100644
--- a/crates/storage/db/src/static_file/cursor.rs
+++ b/crates/storage/db/src/static_file/cursor.rs
@@ -17,10 +17,7 @@ type ColumnResult<T> = ProviderResult<Option<T>>;
 impl<'a> StaticFileCursor<'a> {
     /// Returns a new [`StaticFileCursor`].
     pub fn new(jar: &'a NippyJar<SegmentHeader>, reader: Arc<DataReader>) -> ProviderResult<Self> {
-        Ok(Self(
-            NippyJarCursor::with_reader(jar, reader)
-                .map_err(|err| ProviderError::NippyJar(err.to_string()))?,
-        ))
+        Ok(Self(NippyJarCursor::with_reader(jar, reader).map_err(ProviderError::other)?))
     }
 
     /// Returns the current `BlockNumber` or `TxNumber` of the cursor depending on the kind of
diff --git a/crates/storage/errors/src/provider.rs b/crates/storage/errors/src/provider.rs
index 7b312a95988a..c2fac985c79a 100644
--- a/crates/storage/errors/src/provider.rs
+++ b/crates/storage/errors/src/provider.rs
@@ -24,9 +24,6 @@ pub enum ProviderError {
     /// RLP error.
     #[error("{_0}")]
     Rlp(alloy_rlp::Error),
-    /// Nippy jar error.
-    #[error("nippy jar error: {_0}")]
-    NippyJar(String),
     /// Trie witness error.
     #[error("trie witness error: {_0}")]
     TrieWitnessError(String),
@@ -204,6 +201,22 @@ pub struct RootMismatch {
     pub block_hash: BlockHash,
 }
 
+/// A Static File Write Error.
+#[derive(Debug, thiserror::Error)]
+#[error("{message}")]
+pub struct StaticFileWriterError {
+    /// The error message.
+    pub message: String,
+}
+
+impl StaticFileWriterError {
+    /// Creates a new [`StaticFileWriterError`] with the given message.
+    #[allow(dead_code)]
+    pub fn new(message: impl Into<String>) -> Self {
+        Self { message: message.into() }
+    }
+}
+
 /// Consistent database view error.
 #[derive(Clone, Debug, PartialEq, Eq, Display)]
 pub enum ConsistentViewError {
diff --git a/crates/storage/provider/src/providers/static_file/manager.rs b/crates/storage/provider/src/providers/static_file/manager.rs
index 1c959e4ec056..f0e5f05d47b1 100644
--- a/crates/storage/provider/src/providers/static_file/manager.rs
+++ b/crates/storage/provider/src/providers/static_file/manager.rs
@@ -303,8 +303,7 @@ impl<N: NodePrimitives> StaticFileProvider<N> {
     pub fn report_metrics(&self) -> ProviderResult<()> {
         let Some(metrics) = &self.metrics else { return Ok(()) };
 
-        let static_files =
-            iter_static_files(&self.path).map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+        let static_files = iter_static_files(&self.path).map_err(ProviderError::other)?;
         for (segment, ranges) in static_files {
             let mut entries = 0;
             let mut size = 0;
@@ -429,10 +428,10 @@ impl<N: NodePrimitives> StaticFileProvider<N> {
             jar.jar
         } else {
             NippyJar::<SegmentHeader>::load(&self.path.join(segment.filename(&fixed_block_range)))
-                .map_err(|e| ProviderError::NippyJar(e.to_string()))?
+                .map_err(ProviderError::other)?
         };
 
-        jar.delete().map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+        jar.delete().map_err(ProviderError::other)?;
 
         let mut segment_max_block = None;
         if fixed_block_range.start() > 0 {
@@ -461,7 +460,7 @@ impl<N: NodePrimitives> StaticFileProvider<N> {
         } else {
             trace!(target: "provider::static_file", ?segment, ?fixed_block_range, "Creating jar from scratch");
             let path = self.path.join(segment.filename(fixed_block_range));
-            let jar = NippyJar::load(&path).map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+            let jar = NippyJar::load(&path).map_err(ProviderError::other)?;
             self.map.entry(key).insert(LoadedJar::new(jar)?).downgrade().into()
         };
 
@@ -535,7 +534,7 @@ impl<N: NodePrimitives> StaticFileProvider<N> {
                 let jar = NippyJar::<SegmentHeader>::load(
                     &self.path.join(segment.filename(&fixed_range)),
                 )
-                .map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+                .map_err(ProviderError::other)?;
 
                 // Updates the tx index by first removing all entries which have a higher
                 // block_start than our current static file.
@@ -600,9 +599,7 @@ impl<N: NodePrimitives> StaticFileProvider<N> {
         max_block.clear();
         tx_index.clear();
 
-        for (segment, ranges) in
-            iter_static_files(&self.path).map_err(|e| ProviderError::NippyJar(e.to_string()))?
-        {
+        for (segment, ranges) in iter_static_files(&self.path).map_err(ProviderError::other)? {
             // Update last block for each segment
             if let Some((block_range, _)) = ranges.last() {
                 max_block.insert(segment, block_range.end());
@@ -819,12 +816,9 @@ impl<N: NodePrimitives> StaticFileProvider<N> {
             let file_path =
                 self.directory().join(segment.filename(&self.find_fixed_range(latest_block)));
 
-            let jar = NippyJar::<SegmentHeader>::load(&file_path)
-                .map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+            let jar = NippyJar::<SegmentHeader>::load(&file_path).map_err(ProviderError::other)?;
 
-            NippyJarChecker::new(jar)
-                .check_consistency()
-                .map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+            NippyJarChecker::new(jar).check_consistency().map_err(ProviderError::other)?;
         }
         Ok(())
     }
diff --git a/crates/storage/provider/src/providers/static_file/mod.rs b/crates/storage/provider/src/providers/static_file/mod.rs
index b7965d63114c..6ea76e4b3915 100644
--- a/crates/storage/provider/src/providers/static_file/mod.rs
+++ b/crates/storage/provider/src/providers/static_file/mod.rs
@@ -31,7 +31,7 @@ impl LoadedJar {
                 let mmap_handle = Arc::new(data_reader);
                 Ok(Self { jar, mmap_handle })
             }
-            Err(e) => Err(ProviderError::NippyJar(e.to_string())),
+            Err(e) => Err(ProviderError::other(e)),
         }
     }
 
diff --git a/crates/storage/provider/src/providers/static_file/writer.rs b/crates/storage/provider/src/providers/static_file/writer.rs
index c558f43c7e58..d106745fb3fc 100644
--- a/crates/storage/provider/src/providers/static_file/writer.rs
+++ b/crates/storage/provider/src/providers/static_file/writer.rs
@@ -14,7 +14,7 @@ use reth_primitives::{
     static_file::{SegmentHeader, SegmentRangeInclusive},
     StaticFileSegment,
 };
-use reth_storage_errors::provider::{ProviderError, ProviderResult};
+use reth_storage_errors::provider::{ProviderError, ProviderResult, StaticFileWriterError};
 use std::{
     borrow::Borrow,
     fmt::Debug,
@@ -163,8 +163,7 @@ impl<N: NodePrimitives> StaticFileProviderRW<N> {
             None,
         ) {
             Ok(provider) => (
-                NippyJar::load(provider.data_path())
-                    .map_err(|e| ProviderError::NippyJar(e.to_string()))?,
+                NippyJar::load(provider.data_path()).map_err(ProviderError::other)?,
                 provider.data_path().into(),
             ),
             Err(ProviderError::MissingStaticFileBlock(_, _)) => {
@@ -180,7 +179,7 @@ impl<N: NodePrimitives> StaticFileProviderRW<N> {
                 // This static file has been frozen, so we should
                 Err(ProviderError::FinalizedStaticFile(segment, block))
             }
-            Err(e) => Err(ProviderError::NippyJar(e.to_string())),
+            Err(e) => Err(ProviderError::other(e)),
         }?;
 
         if let Some(metrics) = &metrics {
@@ -214,7 +213,7 @@ impl<N: NodePrimitives> StaticFileProviderRW<N> {
             self.user_header_mut().prune(pruned_rows);
         }
 
-        self.writer.commit().map_err(|error| ProviderError::NippyJar(error.to_string()))?;
+        self.writer.commit().map_err(ProviderError::other)?;
 
         // Updates the [SnapshotProvider] manager
         self.update_index()?;
@@ -240,7 +239,7 @@ impl<N: NodePrimitives> StaticFileProviderRW<N> {
 
         if self.writer.is_dirty() {
             // Commits offsets and new user_header to disk
-            self.writer.commit().map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+            self.writer.commit().map_err(ProviderError::other)?;
 
             if let Some(metrics) = &self.metrics {
                 metrics.record_segment_operation(
@@ -272,9 +271,7 @@ impl<N: NodePrimitives> StaticFileProviderRW<N> {
         let start = Instant::now();
 
         // Commits offsets and new user_header to disk
-        self.writer
-            .commit_without_sync_all()
-            .map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+        self.writer.commit_without_sync_all().map_err(ProviderError::other)?;
 
         if let Some(metrics) = &self.metrics {
             metrics.record_segment_operation(
@@ -421,9 +418,7 @@ impl<N: NodePrimitives> StaticFileProviderRW<N> {
                 } else {
                     // Update `SegmentHeader`
                     self.writer.user_header_mut().prune(len);
-                    self.writer
-                        .prune_rows(len as usize)
-                        .map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+                    self.writer.prune_rows(len as usize).map_err(ProviderError::other)?;
                     break
                 }
 
@@ -433,9 +428,7 @@ impl<N: NodePrimitives> StaticFileProviderRW<N> {
                 self.writer.user_header_mut().prune(remaining_rows);
 
                 // Truncate data
-                self.writer
-                    .prune_rows(remaining_rows as usize)
-                    .map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+                self.writer.prune_rows(remaining_rows as usize).map_err(ProviderError::other)?;
                 remaining_rows = 0;
             }
         }
@@ -476,9 +469,9 @@ impl<N: NodePrimitives> StaticFileProviderRW<N> {
         self.writer.set_dirty();
         self.data_path = data_path;
         NippyJar::<SegmentHeader>::load(&current_path)
-            .map_err(|e| ProviderError::NippyJar(e.to_string()))?
+            .map_err(ProviderError::other)?
             .delete()
-            .map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+            .map_err(ProviderError::other)?;
         Ok(())
     }
 
@@ -487,9 +480,7 @@ impl<N: NodePrimitives> StaticFileProviderRW<N> {
         self.buf.clear();
         column.to_compact(&mut self.buf);
 
-        self.writer
-            .append_column(Some(Ok(&self.buf)))
-            .map_err(|e| ProviderError::NippyJar(e.to_string()))?;
+        self.writer.append_column(Some(Ok(&self.buf))).map_err(ProviderError::other)?;
         Ok(())
     }
 
@@ -762,9 +753,9 @@ impl<N: NodePrimitives> StaticFileProviderRW<N> {
     /// Returns Error if there is a pruning instruction that needs to be applied.
     fn ensure_no_queued_prune(&self) -> ProviderResult<()> {
         if self.prune_on_commit.is_some() {
-            return Err(ProviderError::NippyJar(
-                "Pruning should be committed before appending or pruning more data".to_string(),
-            ))
+            return Err(ProviderError::other(StaticFileWriterError::new(
+                "Pruning should be committed before appending or pruning more data",
+            )));
         }
         Ok(())
     }


Test patch: