feat(preimage/common): Migrate to thiserror (#543)

anton-rs · Sep 21, 2024 · abbbc88 · abbbc88
1 parent 94df0c4
commit abbbc88
Show file tree

Hide file tree

Showing 25 changed files with 210 additions and 157 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bin/client/src/caching_oracle.rs b/bin/client/src/caching_oracle.rs
@@ -5,10 +5,11 @@
 //! [HintWriter]: kona_preimage::HintWriter
 
 use alloc::{boxed::Box, sync::Arc, vec::Vec};
-use anyhow::Result;
 use async_trait::async_trait;
 use core::num::NonZeroUsize;
-use kona_preimage::{HintWriterClient, PreimageKey, PreimageOracleClient};
+use kona_preimage::{
+    errors::PreimageOracleResult, HintWriterClient, PreimageKey, PreimageOracleClient,
+};
 use lru::LruCache;
 use spin::Mutex;
 
@@ -58,7 +59,7 @@ where
     OR: PreimageOracleClient + Sync,
     HW: HintWriterClient + Sync,
 {
-    async fn get(&self, key: PreimageKey) -> Result<Vec<u8>> {
+    async fn get(&self, key: PreimageKey) -> PreimageOracleResult<Vec<u8>> {
         let mut cache_lock = self.cache.lock();
         if let Some(value) = cache_lock.get(&key) {
             Ok(value.clone())
@@ -69,7 +70,7 @@ where
         }
     }
 
-    async fn get_exact(&self, key: PreimageKey, buf: &mut [u8]) -> Result<()> {
+    async fn get_exact(&self, key: PreimageKey, buf: &mut [u8]) -> PreimageOracleResult<()> {
         let mut cache_lock = self.cache.lock();
         if let Some(value) = cache_lock.get(&key) {
             // SAFETY: The value never enters the cache unless the preimage length matches the
@@ -90,7 +91,7 @@ where
     OR: PreimageOracleClient + Sync,
     HW: HintWriterClient + Sync,
 {
-    async fn write(&self, hint: &str) -> Result<()> {
+    async fn write(&self, hint: &str) -> PreimageOracleResult<()> {
         self.hint_writer.write(hint).await
     }
 }
diff --git a/bin/client/src/l1/chain_provider.rs b/bin/client/src/l1/chain_provider.rs
@@ -132,6 +132,7 @@ impl<T: CommsClient> TrieProvider for OracleL1ChainProvider<T> {
                 .get(PreimageKey::new(*key, PreimageKeyType::Keccak256))
                 .await
                 .map(Into::into)
+                .map_err(Into::into)
         })
     }
 

diff --git a/bin/client/src/l2/chain_provider.rs b/bin/client/src/l2/chain_provider.rs
@@ -131,6 +131,7 @@ impl<T: CommsClient> TrieProvider for OracleL2ChainProvider<T> {
                 .get(PreimageKey::new(*key, PreimageKeyType::Keccak256))
                 .await
                 .map(Into::into)
+                .map_err(Into::into)
         })
     }
 
@@ -143,6 +144,7 @@ impl<T: CommsClient> TrieProvider for OracleL2ChainProvider<T> {
                 .get(PreimageKey::new(*hash, PreimageKeyType::Keccak256))
                 .await
                 .map(Into::into)
+                .map_err(Into::into)
         })
     }
 
@@ -164,7 +166,10 @@ impl<T: CommsClient> TrieHinter for OracleL2ChainProvider<T> {
 
     fn hint_trie_node(&self, hash: B256) -> Result<()> {
         kona_common::block_on(async move {
-            self.oracle.write(&HintType::L2StateNode.encode_with(&[hash.as_slice()])).await
+            self.oracle
+                .write(&HintType::L2StateNode.encode_with(&[hash.as_slice()]))
+                .await
+                .map_err(Into::into)
         })
     }
 
@@ -176,6 +181,7 @@ impl<T: CommsClient> TrieHinter for OracleL2ChainProvider<T> {
                         .encode_with(&[block_number.to_be_bytes().as_ref(), address.as_slice()]),
                 )
                 .await
+                .map_err(Into::into)
         })
     }
 
@@ -193,6 +199,7 @@ impl<T: CommsClient> TrieHinter for OracleL2ChainProvider<T> {
                     slot.to_be_bytes::<32>().as_ref(),
                 ]))
                 .await
+                .map_err(Into::into)
         })
     }
 }
diff --git a/bin/host/src/preimage.rs b/bin/host/src/preimage.rs
@@ -1,9 +1,11 @@
 //! Contains the implementations of the [HintRouter] and [PreimageFetcher] traits.]
 
 use crate::{fetcher::Fetcher, kv::KeyValueStore};
-use anyhow::Result;
 use async_trait::async_trait;
-use kona_preimage::{HintRouter, PreimageFetcher, PreimageKey};
+use kona_preimage::{
+    errors::{PreimageOracleError, PreimageOracleResult},
+    HintRouter, PreimageFetcher, PreimageKey,
+};
 use std::sync::Arc;
 use tokio::sync::RwLock;
 
@@ -21,9 +23,12 @@ impl<KV> PreimageFetcher for OnlinePreimageFetcher<KV>
 where
     KV: KeyValueStore + Send + Sync + ?Sized,
 {
-    async fn get_preimage(&self, key: PreimageKey) -> Result<Vec<u8>> {
+    async fn get_preimage(&self, key: PreimageKey) -> PreimageOracleResult<Vec<u8>> {
         let fetcher = self.inner.read().await;
-        fetcher.get_preimage(key.into()).await
+        fetcher
+            .get_preimage(key.into())
+            .await
+            .map_err(|e| PreimageOracleError::Other(e.to_string()))
     }
 }
 
@@ -51,9 +56,9 @@ impl<KV> PreimageFetcher for OfflinePreimageFetcher<KV>
 where
     KV: KeyValueStore + Send + Sync + ?Sized,
 {
-    async fn get_preimage(&self, key: PreimageKey) -> Result<Vec<u8>> {
+    async fn get_preimage(&self, key: PreimageKey) -> PreimageOracleResult<Vec<u8>> {
         let kv_store = self.inner.read().await;
-        kv_store.get(key.into()).ok_or_else(|| anyhow::anyhow!("Key not found"))
+        kv_store.get(key.into()).ok_or(PreimageOracleError::KeyNotFound)
     }
 }
 
@@ -81,7 +86,7 @@ impl<KV> HintRouter for OnlineHintRouter<KV>
 where
     KV: KeyValueStore + Send + Sync + ?Sized,
 {
-    async fn route_hint(&self, hint: String) -> Result<()> {
+    async fn route_hint(&self, hint: String) -> PreimageOracleResult<()> {
         let mut fetcher = self.inner.write().await;
         fetcher.hint(&hint);
         Ok(())
@@ -104,7 +109,7 @@ pub struct OfflineHintRouter;
 
 #[async_trait]
 impl HintRouter for OfflineHintRouter {
-    async fn route_hint(&self, _hint: String) -> Result<()> {
+    async fn route_hint(&self, _hint: String) -> PreimageOracleResult<()> {
         Ok(())
     }
 }
diff --git a/bin/host/src/server.rs b/bin/host/src/server.rs
@@ -9,7 +9,8 @@ use crate::{
 };
 use anyhow::{anyhow, Result};
 use kona_preimage::{
-    HintReaderServer, HintRouter, PreimageFetcher, PreimageOracleServer, PreimageServerError,
+    errors::PreimageOracleError, HintReaderServer, HintRouter, PreimageFetcher,
+    PreimageOracleServer,
 };
 use std::sync::Arc;
 use tokio::sync::RwLock;
@@ -88,10 +89,10 @@ where
             loop {
                 match server.next_preimage_request(fetcher).await {
                     Ok(_) => (),
-                    Err(PreimageServerError::BrokenPipe(_)) => return Ok(()),
-                    Err(PreimageServerError::Other(e)) => {
-                        error!("Failed to serve preimage request: {e:?}");
-                        return Err(e);
+                    Err(PreimageOracleError::IOError(_)) => return Ok(()),
+                    Err(e) => {
+                        error!("Failed to serve preimage request: {e}");
+                        return Err(anyhow!("Failed to serve preimage request: {e}"));
                     }
                 }
             }
@@ -119,10 +120,10 @@ where
             loop {
                 match server.next_hint(router).await {
                     Ok(_) => (),
-                    Err(PreimageServerError::BrokenPipe(_)) => return Ok(()),
-                    Err(PreimageServerError::Other(e)) => {
-                        error!("Failed to serve preimage request: {e:?}");
-                        return Err(e);
+                    Err(PreimageOracleError::IOError(_)) => return Ok(()),
+                    Err(e) => {
+                        error!("Failed to serve route hint: {e}");
+                        return Err(anyhow!("Failed to route hint: {e}"));
                     }
                 }
             }

diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml
@@ -9,6 +9,6 @@ repository.workspace = true
 homepage.workspace = true
 
 [dependencies]
-anyhow.workspace = true
+thiserror.workspace = true
 cfg-if.workspace = true
 linked_list_allocator.workspace = true
diff --git a/crates/common/src/asterisc/io.rs b/crates/common/src/asterisc/io.rs
@@ -1,5 +1,4 @@
-use crate::{asterisc::syscall, BasicKernelInterface, FileDescriptor};
-use anyhow::Result;
+use crate::{asterisc::syscall, errors::IOResult, BasicKernelInterface, FileDescriptor};
 
 /// Concrete implementation of the [`KernelIO`] trait for the `riscv64` target architecture.
 #[derive(Debug)]
@@ -24,9 +23,9 @@ pub(crate) enum SyscallNumber {
 }
 
 impl BasicKernelInterface for AsteriscIO {
-    fn write(fd: FileDescriptor, buf: &[u8]) -> Result<usize> {
+    fn write(fd: FileDescriptor, buf: &[u8]) -> IOResult<usize> {
         unsafe {
-            Ok(syscall::syscall3(
+            crate::linux::from_ret(syscall::syscall3(
                 SyscallNumber::Write as usize,
                 fd.into(),
                 buf.as_ptr() as usize,
@@ -35,9 +34,9 @@ impl BasicKernelInterface for AsteriscIO {
         }
     }
 
-    fn read(fd: FileDescriptor, buf: &mut [u8]) -> Result<usize> {
+    fn read(fd: FileDescriptor, buf: &mut [u8]) -> IOResult<usize> {
         unsafe {
-            Ok(syscall::syscall3(
+            crate::linux::from_ret(syscall::syscall3(
                 SyscallNumber::Read as usize,
                 fd.into(),
                 buf.as_ptr() as usize,
@@ -48,7 +47,7 @@ impl BasicKernelInterface for AsteriscIO {
 
     fn exit(code: usize) -> ! {
         unsafe {
-            syscall::syscall1(SyscallNumber::Exit as usize, code);
+            let _ = syscall::syscall1(SyscallNumber::Exit as usize, code);
             panic!()
         }
     }

diff --git a/crates/common/src/cannon/io.rs b/crates/common/src/cannon/io.rs
@@ -1,5 +1,4 @@
-use crate::{cannon::syscall, BasicKernelInterface, FileDescriptor};
-use anyhow::{anyhow, Result};
+use crate::{cannon::syscall, errors::IOResult, BasicKernelInterface, FileDescriptor};
 
 /// Concrete implementation of the [BasicKernelInterface] trait for the `MIPS32rel1` target
 /// architecture. Exposes a safe interface for performing IO operations within the FPVM kernel.
@@ -25,33 +24,31 @@ pub(crate) enum SyscallNumber {
 }
 
 impl BasicKernelInterface for CannonIO {
-    fn write(fd: FileDescriptor, buf: &[u8]) -> Result<usize> {
+    fn write(fd: FileDescriptor, buf: &[u8]) -> IOResult<usize> {
         unsafe {
-            syscall::syscall3(
+            crate::linux::from_ret(syscall::syscall3(
                 SyscallNumber::Write as usize,
                 fd.into(),
                 buf.as_ptr() as usize,
                 buf.len(),
-            )
-            .map_err(|e| anyhow!("Syscall Error: {e}"))
+            ))
         }
     }
 
-    fn read(fd: FileDescriptor, buf: &mut [u8]) -> Result<usize> {
+    fn read(fd: FileDescriptor, buf: &mut [u8]) -> IOResult<usize> {
         unsafe {
-            syscall::syscall3(
+            crate::linux::from_ret(syscall::syscall3(
                 SyscallNumber::Read as usize,
                 fd.into(),
                 buf.as_ptr() as usize,
                 buf.len(),
-            )
-            .map_err(|e| anyhow!("Syscall Error: {e}"))
+            ))
         }
     }
 
     fn exit(code: usize) -> ! {
         unsafe {
-            syscall::syscall1(SyscallNumber::Exit as usize, code);
+            let _ = syscall::syscall1(SyscallNumber::Exit as usize, code);
             panic!()
         }
     }

diff --git a/crates/common/src/cannon/syscall.rs b/crates/common/src/cannon/syscall.rs
@@ -65,12 +65,7 @@ pub(crate) unsafe fn syscall1(n: usize, arg1: usize) -> usize {
 
 /// Issues a raw system call with 3 arguments. (e.g. read, write)
 #[inline]
-pub(crate) unsafe fn syscall3(
-    n: usize,
-    arg1: usize,
-    arg2: usize,
-    arg3: usize,
-) -> Result<usize, i32> {
+pub(crate) unsafe fn syscall3(n: usize, arg1: usize, arg2: usize, arg3: usize) -> usize {
     let mut err: usize;
     let mut ret: usize;
     asm!(
@@ -94,12 +89,5 @@ pub(crate) unsafe fn syscall3(
         options(nostack, preserves_flags)
     );
 
-    let value = (err == 0).then_some(ret).unwrap_or_else(|| ret.wrapping_neg());
-
-    (value <= -4096isize as usize).then_some(value).ok_or_else(|| {
-        // Truncation of the error value is guaranteed to never occur due to
-        // the above check. This is the same check that musl uses:
-        // https://git.musl-libc.org/cgit/musl/tree/src/internal/syscall_ret.c?h=v1.1.15
-        -(value as i32)
-    })
+    (err == 0).then_some(ret).unwrap_or_else(|| ret.wrapping_neg())
 }
diff --git a/crates/common/src/errors.rs b/crates/common/src/errors.rs
@@ -0,0 +1,11 @@
+//! Errors for the `kona-common` crate.
+
+use thiserror::Error;
+
+/// An error that can occur when reading from or writing to a file descriptor.
+#[derive(Error, Debug, PartialEq, Eq)]
+#[error("IO error (errno: {0})")]
+pub struct IOError(pub i32);
+
+/// A [Result] type for the [IOError].
+pub type IOResult<T> = Result<T, IOError>;