convert fvm and update type bounds accordingly

Author: dignifiedquire

fvm/src/blockstore/buffered.rs

@@ -1,7 +1,6 @@
 // Copyright 2019-2022 ChainSafe Systems
 // SPDX-License-Identifier: Apache-2.0, MIT
 
-use anyhow::{anyhow, Result};
 use byteorder::{BigEndian, ByteOrder, ReadBytesExt};
 use cid::Cid;
 use fvm_ipld_blockstore::{Blockstore, Buffered};
@@ -38,33 +37,72 @@ where
     }
 }
 
+#[derive(thiserror::Error, Debug)]
+pub enum Error<E> {
+    #[error("flush: {0}")]
+    Flush(#[from] FlushError),
+    #[error("blockstore: {0}")]
+    Blockstore(E),
+}
+
 impl<BS> Buffered for BufferedBlockstore<BS>
 where
     BS: Blockstore,
 {
     /// Flushes the buffered cache based on the root node.
     /// This will recursively traverse the cache and write all data connected by links to this
     /// root Cid.
-    fn flush(&self, root: &Cid) -> Result<()> {
+    fn flush(&self, root: &Cid) -> Result<(), Error<BS::Error>> {
         let mut buffer = Vec::new();
         let mut s = self.write.borrow_mut();
         copy_rec(&s, *root, &mut buffer)?;
 
-        self.base.put_many_keyed(buffer)?;
+        self.base
+            .put_many_keyed(buffer)
+            .map_err(Error::Blockstore)?;
         *s = Default::default();
 
         Ok(())
     }
 }
 
+#[derive(thiserror::Error, Debug)]
+pub enum FlushError {
+    #[error("io: {0}")]
+    Io(#[from] std::io::Error),
+    #[error("cid: {0}")]
+    Cid(#[from] cid::Error),
+    #[error("cbor input was not canonical (lval 24 with value < 24)")]
+    HeaderLval24,
+    #[error("cbor input was not canonical (lval 25 with value <= MaxUint8)")]
+    HeaderLval25,
+    #[error("cbor input was not canonical (lval 26 with value <= MaxUint16)")]
+    HeaderLval26,
+    #[error("cbor input was not canonical (lval 27 with value <= MaxUint32)")]
+    HeaderLval27,
+    #[error("invalid header cbor_read_header_buf")]
+    HeaderInvalid,
+    #[error("expected cbor type byte string in input")]
+    UnexpectedByteString,
+    #[error("string in cbor input too long")]
+    StringTooLong,
+    #[error("Invalid link ({0}) in flushing buffered store")]
+    InvalidLink(Cid),
+    #[error("unhandled cbor type: {0}")]
+    UnhandledCborType(u8),
+}
+
 /// Given a CBOR encoded Buffer, returns a tuple of:
 /// the type of the CBOR object along with extra
 /// elements we expect to read. More info on this can be found in
 /// Appendix C. of RFC 7049 which defines the CBOR specification.
 /// This was implemented because the CBOR library we use does not expose low
 /// methods like this, requiring us to deserialize the whole CBOR payload, which
 /// is unnecessary and quite inefficient for our usecase here.
-fn cbor_read_header_buf<B: Read>(br: &mut B, scratch: &mut [u8]) -> anyhow::Result<(u8, usize)> {
+fn cbor_read_header_buf<B: Read>(
+    br: &mut B,
+    scratch: &mut [u8],
+) -> Result<(u8, usize), FlushError> {
     let first = br.read_u8()?;
     let maj = (first & 0xe0) >> 5;
     let low = first & 0x1f;
@@ -74,49 +112,41 @@ fn cbor_read_header_buf<B: Read>(br: &mut B, scratch: &mut [u8]) -> anyhow::Resu
     } else if low == 24 {
         let val = br.read_u8()?;
         if val < 24 {
-            return Err(anyhow!(
-                "cbor input was not canonical (lval 24 with value < 24)"
-            ));
+            return Err(FlushError::HeaderLval24);
         }
         Ok((maj, val as usize))
     } else if low == 25 {
         br.read_exact(&mut scratch[..2])?;
         let val = BigEndian::read_u16(&scratch[..2]);
         if val <= u8::MAX as u16 {
-            return Err(anyhow!(
-                "cbor input was not canonical (lval 25 with value <= MaxUint8)"
-            ));
+            return Err(FlushError::HeaderLval25);
         }
         Ok((maj, val as usize))
     } else if low == 26 {
         br.read_exact(&mut scratch[..4])?;
         let val = BigEndian::read_u32(&scratch[..4]);
         if val <= u16::MAX as u32 {
-            return Err(anyhow!(
-                "cbor input was not canonical (lval 26 with value <= MaxUint16)"
-            ));
+            return Err(FlushError::HeaderLval26);
         }
         Ok((maj, val as usize))
     } else if low == 27 {
         br.read_exact(&mut scratch[..8])?;
         let val = BigEndian::read_u64(&scratch[..8]);
         if val <= u32::MAX as u64 {
-            return Err(anyhow!(
-                "cbor input was not canonical (lval 27 with value <= MaxUint32)"
-            ));
+            return Err(FlushError::HeaderLval27);
         }
         Ok((maj, val as usize))
     } else {
-        Err(anyhow!("invalid header cbor_read_header_buf"))
+        Err(FlushError::HeaderInvalid)
     }
 }
 
 /// Given a CBOR serialized IPLD buffer, read through all of it and return all the Links.
 /// This function is useful because it is quite a bit more fast than doing this recursively on a
 /// deserialized IPLD object.
-fn scan_for_links<B: Read + Seek, F>(buf: &mut B, mut callback: F) -> Result<()>
+fn scan_for_links<B: Read + Seek, F>(buf: &mut B, mut callback: F) -> Result<(), FlushError>
 where
-    F: FnMut(Cid) -> anyhow::Result<()>,
+    F: FnMut(Cid) -> Result<(), FlushError>,
 {
     let mut scratch: [u8; 100] = [0; 100];
     let mut remaining = 1;
@@ -136,10 +166,10 @@ where
                     let (maj, extra) = cbor_read_header_buf(buf, &mut scratch)?;
                     // The actual CID is expected to be a byte string
                     if maj != 2 {
-                        return Err(anyhow!("expected cbor type byte string in input"));
+                        return Err(FlushError::UnexpectedByteString);
                     }
                     if extra > 100 {
-                        return Err(anyhow!("string in cbor input too long"));
+                        return Err(FlushError::StringTooLong);
                     }
                     buf.read_exact(&mut scratch[..extra])?;
                     let c = Cid::try_from(&scratch[1..extra])?;
@@ -157,7 +187,7 @@ where
                 remaining += extra * 2;
             }
             _ => {
-                return Err(anyhow!("unhandled cbor type: {}", maj));
+                return Err(FlushError::UnhandledCborType(maj));
             }
         }
         remaining -= 1;
@@ -170,16 +200,14 @@ fn copy_rec<'a>(
     cache: &'a HashMap<Cid, Vec<u8>>,
     root: Cid,
     buffer: &mut Vec<(Cid, &'a [u8])>,
-) -> Result<()> {
+) -> Result<(), FlushError> {
     // TODO: Make this non-recursive.
     // Skip identity and Filecoin commitment Cids
     if root.codec() != DAG_CBOR {
         return Ok(());
     }
 
-    let block = &*cache
-        .get(&root)
-        .ok_or_else(|| anyhow!("Invalid link ({}) in flushing buffered store", root))?;
+    let block = &*cache.get(&root).ok_or(FlushError::InvalidLink(root))?;
 
     scan_for_links(&mut Cursor::new(block), |link| {
         if link.codec() != DAG_CBOR {
@@ -205,28 +233,30 @@ impl<BS> Blockstore for BufferedBlockstore<BS>
 where
     BS: Blockstore,
 {
-    fn get(&self, cid: &Cid) -> Result<Option<Vec<u8>>> {
+    type Error = Error<BS::Error>;
+
+    fn get(&self, cid: &Cid) -> Result<Option<Vec<u8>>, Self::Error> {
         Ok(if let Some(data) = self.write.borrow().get(cid) {
             Some(data.clone())
         } else {
-            self.base.get(cid)?
+            self.base.get(cid).map_err(Error::Blockstore)?
         })
     }
 
-    fn put_keyed(&self, cid: &Cid, buf: &[u8]) -> Result<()> {
+    fn put_keyed(&self, cid: &Cid, buf: &[u8]) -> Result<(), Self::Error> {
         self.write.borrow_mut().insert(*cid, Vec::from(buf));
         Ok(())
     }
 
-    fn has(&self, k: &Cid) -> Result<bool> {
+    fn has(&self, k: &Cid) -> Result<bool, Self::Error> {
         if self.write.borrow().contains_key(k) {
             Ok(true)
         } else {
-            Ok(self.base.has(k)?)
+            Ok(self.base.has(k).map_err(Error::Blockstore)?)
         }
     }
 
-    fn put_many_keyed<D, I>(&self, blocks: I) -> Result<()>
+    fn put_many_keyed<D, I>(&self, blocks: I) -> Result<(), Self::Error>
     where
         Self: Sized,
         D: AsRef<[u8]>,

ipld/amt/benches/amt_benchmark.rs

@@ -117,7 +117,9 @@ fn for_each(c: &mut Criterion) {
     c.bench_function("AMT for_each function", |b| {
         b.iter(|| {
             let a = Amt::load(&cid, &db).unwrap();
-            black_box(a).for_each(|_, _v: &u64| Ok(())).unwrap();
+            black_box(a)
+                .for_each(|_, _v: &u64| Ok::<_, ()>(()))
+                .unwrap();
         })
     });
 }

ipld/amt/src/amt.rs

@@ -72,7 +72,7 @@ where
     }
 
     /// Constructs an AMT with a blockstore and a Cid of the root of the AMT
-    pub fn load(cid: &Cid, block_store: BS) -> Result<Self, Error<BS>> {
+    pub fn load(cid: &Cid, block_store: BS) -> Result<Self, Error<BS::Error>> {
         // Load root bytes from database
         let root: Root<V> = block_store
             .get_cbor(cid)?
@@ -100,7 +100,7 @@ where
     pub fn new_from_iter(
         block_store: BS,
         vals: impl IntoIterator<Item = V>,
-    ) -> Result<Cid, Error<BS>> {
+    ) -> Result<Cid, Error<BS::Error>> {
         let mut t = Self::new(block_store);
 
         t.batch_set(vals)?;
@@ -109,7 +109,7 @@ where
     }
 
     /// Get value at index of AMT
-    pub fn get(&self, i: u64) -> Result<Option<&V>, Error<BS>> {
+    pub fn get(&self, i: u64) -> Result<Option<&V>, Error<BS::Error>> {
         if i > MAX_INDEX {
             return Err(Error::OutOfRange(i));
         }
@@ -124,7 +124,7 @@ where
     }
 
     /// Set value at index
-    pub fn set(&mut self, i: u64, val: V) -> Result<(), Error<BS>> {
+    pub fn set(&mut self, i: u64, val: V) -> Result<(), Error<BS::Error>> {
         if i > MAX_INDEX {
             return Err(Error::OutOfRange(i));
         }
@@ -166,7 +166,7 @@ where
 
     /// Batch set (naive for now)
     // TODO Implement more efficient batch set to not have to traverse tree and keep cache for each
-    pub fn batch_set(&mut self, vals: impl IntoIterator<Item = V>) -> Result<(), Error<BS>> {
+    pub fn batch_set(&mut self, vals: impl IntoIterator<Item = V>) -> Result<(), Error<BS::Error>> {
         for (i, val) in (0u64..).zip(vals) {
             self.set(i, val)?;
         }
@@ -175,7 +175,7 @@ where
     }
 
     /// Delete item from AMT at index
-    pub fn delete(&mut self, i: u64) -> Result<Option<V>, Error<BS>> {
+    pub fn delete(&mut self, i: u64) -> Result<Option<V>, Error<BS::Error>> {
         if i > MAX_INDEX {
             return Err(Error::OutOfRange(i));
         }
@@ -246,7 +246,7 @@ where
         &mut self,
         iter: impl IntoIterator<Item = u64>,
         strict: bool,
-    ) -> Result<bool, Error<BS>> {
+    ) -> Result<bool, Error<BS::Error>> {
         // TODO: optimize this
         let mut modified = false;
 
@@ -262,7 +262,7 @@ where
     }
 
     /// flush root and return Cid used as key in block store
-    pub fn flush(&mut self) -> Result<Cid, Error<BS>> {
+    pub fn flush(&mut self) -> Result<Cid, Error<BS::Error>> {
         self.root.node.flush(&self.block_store)?;
         Ok(self.block_store.put_cbor(&self.root, Code::Blake2b256)?)
     }
@@ -291,7 +291,7 @@ where
     /// assert_eq!(&values, &[(1, "One".to_owned()), (4, "Four".to_owned())]);
     /// ```
     #[inline]
-    pub fn for_each<F, U>(&self, mut f: F) -> Result<(), EitherError<U, BS>>
+    pub fn for_each<F, U>(&self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
     where
         F: FnMut(u64, &V) -> Result<(), U>,
     {
@@ -303,7 +303,7 @@ where
 
     /// Iterates over each value in the Amt and runs a function on the values, for as long as that
     /// function keeps returning `true`.
-    pub fn for_each_while<F, U>(&self, mut f: F) -> Result<(), EitherError<U, BS>>
+    pub fn for_each_while<F, U>(&self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
     where
         F: FnMut(u64, &V) -> Result<bool, U>,
     {
@@ -321,7 +321,7 @@ where
 
     /// Iterates over each value in the Amt and runs a function on the values that allows modifying
     /// each value.
-    pub fn for_each_mut<F, U>(&mut self, mut f: F) -> Result<(), EitherError<U, BS>>
+    pub fn for_each_mut<F, U>(&mut self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
     where
         V: Clone,
         F: FnMut(u64, &mut ValueMut<'_, V>) -> Result<(), U>,
@@ -334,7 +334,7 @@ where
 
     /// Iterates over each value in the Amt and runs a function on the values that allows modifying
     /// each value, for as long as that function keeps returning `true`.
-    pub fn for_each_while_mut<F, U>(&mut self, mut f: F) -> Result<(), EitherError<U, BS>>
+    pub fn for_each_while_mut<F, U>(&mut self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
     where
         // TODO remove clone bound when go-interop doesn't require it.
         // (If needed without, this bound can be removed by duplicating function signatures)

ipld/amt/src/error.rs

@@ -2,13 +2,12 @@
 // SPDX-License-Identifier: Apache-2.0, MIT
 
 use cid::Error as CidError;
-use fvm_ipld_blockstore::Blockstore;
 use fvm_ipld_encoding::{CborStoreError, Error as EncodingError};
 use thiserror::Error;
 
 /// AMT Error
 #[derive(Debug, Error)]
-pub enum Error<BS: Blockstore> {
+pub enum Error<E> {
     /// Index referenced it above arbitrary max set
     #[error("index {0} out of range for the amt")]
     OutOfRange(u64),
@@ -29,13 +28,13 @@ pub enum Error<BS: Blockstore> {
     #[error("no such index {0} in Amt for batch delete")]
     BatchDelteNotFound(u64),
     #[error("blockstore {0}")]
-    Blockstore(BS::Error),
+    Blockstore(E),
     #[error("encoding error {0}")]
     Encoding(#[from] EncodingError),
 }
 
-impl<BS: Blockstore> From<CborStoreError<BS>> for Error<BS> {
-    fn from(err: CborStoreError<BS>) -> Self {
+impl<E> From<CborStoreError<E>> for Error<E> {
+    fn from(err: CborStoreError<E>) -> Self {
         match err {
             CborStoreError::Blockstore(err) => Error::Blockstore(err),
             CborStoreError::Encoding(err) => Error::Encoding(err),
@@ -44,15 +43,15 @@ impl<BS: Blockstore> From<CborStoreError<BS>> for Error<BS> {
 }
 
 #[derive(Debug, Error)]
-pub enum EitherError<U, BS: Blockstore> {
+pub enum EitherError<U, E> {
     #[error("user: {0}")]
     User(U),
     #[error("hamt: {0}")]
-    Hamt(#[from] Error<BS>),
+    Hamt(#[from] Error<E>),
 }
 
-impl<U, BS: Blockstore> From<CborStoreError<BS>> for EitherError<U, BS> {
-    fn from(err: CborStoreError<BS>) -> Self {
+impl<U, E> From<CborStoreError<E>> for EitherError<U, E> {
+    fn from(err: CborStoreError<E>) -> Self {
         EitherError::Hamt(err.into())
     }
 }