diff --git a/fvm/src/blockstore/buffered.rs b/fvm/src/blockstore/buffered.rs index 29049ac0c..065f934ee 100644 --- a/fvm/src/blockstore/buffered.rs +++ b/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,6 +37,14 @@ where } } +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("flush: {0}")] + Flush(#[from] FlushError), + #[error("blockstore: {0}")] + Blockstore(E), +} + impl Buffered for BufferedBlockstore where BS: Blockstore, @@ -45,18 +52,40 @@ where /// 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> { 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 {0} with value < {1})")] + HeaderNotCanonical(usize, &'static str), + #[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 @@ -64,7 +93,10 @@ where /// 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(br: &mut B, scratch: &mut [u8]) -> anyhow::Result<(u8, usize)> { +fn cbor_read_header_buf( + 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 +106,41 @@ fn cbor_read_header_buf(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::HeaderNotCanonical(24, "24")); } 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::HeaderNotCanonical(25, "MaxUint8")); } 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::HeaderNotCanonical(26, "MaxUint16")); } 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::HeaderNotCanonical(27, "MaxUint32")); } 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(buf: &mut B, mut callback: F) -> Result<()> +fn scan_for_links(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 +160,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 +181,7 @@ where remaining += extra * 2; } _ => { - return Err(anyhow!("unhandled cbor type: {}", maj)); + return Err(FlushError::UnhandledCborType(maj)); } } remaining -= 1; @@ -170,16 +194,14 @@ fn copy_rec<'a>( cache: &'a HashMap>, 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 +227,30 @@ impl Blockstore for BufferedBlockstore where BS: Blockstore, { - fn get(&self, cid: &Cid) -> Result>> { + type Error = Error; + + fn get(&self, cid: &Cid) -> Result>, 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 { + fn has(&self, k: &Cid) -> Result { 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(&self, blocks: I) -> Result<()> + fn put_many_keyed(&self, blocks: I) -> Result<(), Self::Error> where Self: Sized, D: AsRef<[u8]>, diff --git a/fvm/src/state_tree.rs b/fvm/src/state_tree.rs index bb9784661..da65816ba 100644 --- a/fvm/src/state_tree.rs +++ b/fvm/src/state_tree.rs @@ -504,7 +504,7 @@ where where F: FnMut(Address, &ActorState) -> anyhow::Result<()>, { - self.hamt.for_each(|k, v| { + self.hamt.try_for_each(|k, v| { let addr = Address::from_bytes(&k.0)?; f(addr, v) })?; diff --git a/ipld/amt/Cargo.toml b/ipld/amt/Cargo.toml index 0e6a72355..20bf742cd 100644 --- a/ipld/amt/Cargo.toml +++ b/ipld/amt/Cargo.toml @@ -14,7 +14,6 @@ thiserror = "1.0" once_cell = "1.5" ahash = { version = "0.7", optional = true } itertools = "0.10" -anyhow = "1.0.51" fvm_ipld_blockstore = { version = "0.1", path = "../blockstore" } fvm_ipld_encoding = { version = "0.1", path = "../encoding" } diff --git a/ipld/amt/benches/amt_benchmark.rs b/ipld/amt/benches/amt_benchmark.rs index c3827b722..c0ce676fb 100644 --- a/ipld/amt/benches/amt_benchmark.rs +++ b/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) + .try_for_each(|_, _v: &u64| Ok::<_, ()>(())) + .unwrap(); }) }); } diff --git a/ipld/amt/src/amt.rs b/ipld/amt/src/amt.rs index 0a0b83c2a..b30188ee1 100644 --- a/ipld/amt/src/amt.rs +++ b/ipld/amt/src/amt.rs @@ -1,7 +1,6 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use anyhow::anyhow; use cid::multihash::Code; use cid::Cid; use fvm_ipld_blockstore::Blockstore; @@ -11,6 +10,7 @@ use fvm_ipld_encoding::CborStore; use itertools::sorted; use super::ValueMut; +use crate::error::EitherError; use crate::node::{CollapsedNode, Link}; use crate::{ init_sized_vec, nodes_for_height, Error, Node, Root, DEFAULT_BIT_WIDTH, MAX_HEIGHT, MAX_INDEX, @@ -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 { + pub fn load(cid: &Cid, block_store: BS) -> Result> { // Load root bytes from database let root: Root = block_store .get_cbor(cid)? @@ -97,7 +97,10 @@ where } /// Generates an AMT with block store and array of cbor marshallable objects and returns Cid - pub fn new_from_iter(block_store: BS, vals: impl IntoIterator) -> Result { + pub fn new_from_iter( + block_store: BS, + vals: impl IntoIterator, + ) -> Result> { let mut t = Self::new(block_store); t.batch_set(vals)?; @@ -106,7 +109,7 @@ where } /// Get value at index of AMT - pub fn get(&self, i: u64) -> Result, Error> { + pub fn get(&self, i: u64) -> Result, Error> { if i > MAX_INDEX { return Err(Error::OutOfRange(i)); } @@ -121,7 +124,7 @@ where } /// Set value at index - pub fn set(&mut self, i: u64, val: V) -> Result<(), Error> { + pub fn set(&mut self, i: u64, val: V) -> Result<(), Error> { if i > MAX_INDEX { return Err(Error::OutOfRange(i)); } @@ -163,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) -> Result<(), Error> { + pub fn batch_set(&mut self, vals: impl IntoIterator) -> Result<(), Error> { for (i, val) in (0u64..).zip(vals) { self.set(i, val)?; } @@ -172,7 +175,7 @@ where } /// Delete item from AMT at index - pub fn delete(&mut self, i: u64) -> Result, Error> { + pub fn delete(&mut self, i: u64) -> Result, Error> { if i > MAX_INDEX { return Err(Error::OutOfRange(i)); } @@ -243,7 +246,7 @@ where &mut self, iter: impl IntoIterator, strict: bool, - ) -> Result { + ) -> Result> { // TODO: optimize this let mut modified = false; @@ -251,7 +254,7 @@ where for i in sorted(iter) { let found = self.delete(i)?.is_none(); if strict && found { - return Err(anyhow!("no such index {} in Amt for batch delete", i).into()); + return Err(Error::BatchDeleteNotFound(i)); } modified |= found; } @@ -259,7 +262,7 @@ where } /// flush root and return Cid used as key in block store - pub fn flush(&mut self) -> Result { + pub fn flush(&mut self) -> Result> { self.root.node.flush(&self.block_store)?; Ok(self.block_store.put_cbor(&self.root, Code::Blake2b256)?) } @@ -281,28 +284,44 @@ where /// map.set(4, "Four".to_owned()).unwrap(); /// /// let mut values: Vec<(u64, String)> = Vec::new(); - /// map.for_each(|i, v| { + /// map.try_for_each(|i, v| { /// values.push((i, v.clone())); - /// Ok(()) + /// Ok::<_, ()>(()) /// }).unwrap(); /// assert_eq!(&values, &[(1, "One".to_owned()), (4, "Four".to_owned())]); /// ``` #[inline] - pub fn for_each(&self, mut f: F) -> Result<(), Error> + pub fn try_for_each(&self, mut f: F) -> Result<(), EitherError> where - F: FnMut(u64, &V) -> anyhow::Result<()>, + F: FnMut(u64, &V) -> Result<(), U>, { - self.for_each_while(|i, x| { + self.try_for_each_while(|i, x| { f(i, x)?; Ok(true) }) } + #[inline] + pub fn for_each(&self, mut f: F) -> Result<(), Error> + where + V: DeserializeOwned, + F: FnMut(u64, &V), + { + self.try_for_each(|k, v| { + f(k, v); + Ok(()) + }) + .map_err(|err| match err { + EitherError::User(()) => unreachable!(), + EitherError::Amt(e) => e, + }) + } + /// 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(&self, mut f: F) -> Result<(), Error> + pub fn try_for_each_while(&self, mut f: F) -> Result<(), EitherError> where - F: FnMut(u64, &V) -> anyhow::Result, + F: FnMut(u64, &V) -> Result, { self.root .node @@ -316,27 +335,56 @@ where .map(|_| ()) } + /// 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(&self, mut f: F) -> Result<(), Error> + where + F: FnMut(u64, &V) -> bool, + { + self.try_for_each_while::<_, ()>(|key, value| Ok(f(key, value))) + .map_err(|err| match err { + EitherError::User(()) => unreachable!(), + EitherError::Amt(e) => e, + }) + } + /// Iterates over each value in the Amt and runs a function on the values that allows modifying /// each value. - pub fn for_each_mut(&mut self, mut f: F) -> Result<(), Error> + pub fn try_for_each_mut(&mut self, mut f: F) -> Result<(), EitherError> where V: Clone, - F: FnMut(u64, &mut ValueMut<'_, V>) -> anyhow::Result<()>, + F: FnMut(u64, &mut ValueMut<'_, V>) -> Result<(), U>, { - self.for_each_while_mut(|i, x| { + self.try_for_each_while_mut(|i, x| { f(i, x)?; Ok(true) }) } + /// Iterates over each value in the Amt and runs a function on the values that allows modifying + /// each value. + pub fn for_each_mut(&mut self, mut f: F) -> Result<(), Error> + where + V: Clone, + F: FnMut(u64, &mut ValueMut<'_, V>), + { + self.for_each_while_mut(|i, x| { + f(i, x); + true + }) + } + /// 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(&mut self, mut f: F) -> Result<(), Error> + pub fn try_for_each_while_mut( + &mut self, + mut f: F, + ) -> Result<(), EitherError> where // TODO remove clone bound when go-interop doesn't require it. // (If needed without, this bound can be removed by duplicating function signatures) V: Clone, - F: FnMut(u64, &mut ValueMut<'_, V>) -> anyhow::Result, + F: FnMut(u64, &mut ValueMut<'_, V>) -> Result, { #[cfg(not(feature = "go-interop"))] { @@ -389,4 +437,20 @@ where Ok(()) } } + + /// 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(&mut self, mut f: F) -> Result<(), 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) + V: Clone, + F: FnMut(u64, &mut ValueMut<'_, V>) -> bool, + { + self.try_for_each_while_mut::<_, ()>(|key, value| Ok(f(key, value))) + .map_err(|err| match err { + EitherError::User(()) => unreachable!(), + EitherError::Amt(e) => e, + }) + } } diff --git a/ipld/amt/src/error.rs b/ipld/amt/src/error.rs index 80d5ebd51..c29cc1d86 100644 --- a/ipld/amt/src/error.rs +++ b/ipld/amt/src/error.rs @@ -1,16 +1,13 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use std::error::Error as StdError; - -use anyhow::anyhow; use cid::Error as CidError; -use fvm_ipld_encoding::Error as EncodingError; +use fvm_ipld_encoding::{CborStoreError, Error as EncodingError}; use thiserror::Error; /// AMT Error #[derive(Debug, Error)] -pub enum Error { +pub enum Error { /// Index referenced it above arbitrary max set #[error("index {0} out of range for the amt")] OutOfRange(u64), @@ -20,49 +17,67 @@ pub enum Error { /// Error generating a Cid for data #[error(transparent)] Cid(#[from] CidError), - /// Error when trying to serialize an AMT without a flushed cache - #[error("Tried to serialize without saving cache, run flush() on Amt before serializing")] - Cached, /// Serialized vector less than number of bits set #[error("Vector length does not match bitmap")] InvalidVecLength, - /// Invalid formatted serialized node. - #[error("Serialized node cannot contain both links and values")] - LinksAndValues, /// Cid not found in store error #[error("Cid ({0}) did not match any in database")] CidNotFound(String), - /// Dynamic error for when the error needs to be forwarded as is. #[error("{0}")] - Dynamic(anyhow::Error), + CollapsedNode(#[from] CollapsedNodeError), + #[error("no such index {0} in Amt for batch delete")] + BatchDeleteNotFound(u64), + #[error("blockstore {0}")] + Blockstore(E), + #[error("encoding error {0}")] + Encoding(#[from] EncodingError), } -impl From for Error { - fn from(e: String) -> Self { - Self::Dynamic(anyhow::anyhow!(e)) +impl From> for Error { + fn from(err: CborStoreError) -> Self { + match err { + CborStoreError::Blockstore(err) => Error::Blockstore(err), + CborStoreError::Encoding(err) => Error::Encoding(err), + } } } -impl From<&'static str> for Error { - fn from(e: &'static str) -> Self { - Self::Dynamic(anyhow::anyhow!(e)) - } +/// This error wraps around around two different errors, either the native `Error` from `amt`, or +/// a custom user error, returned from executing a user defined function. +#[derive(Debug, Error)] +pub enum EitherError { + #[error("user: {0}")] + User(U), + #[error("amt: {0}")] + Amt(#[from] Error), } -impl From for Error { - fn from(e: anyhow::Error) -> Self { - e.downcast::().unwrap_or_else(Self::Dynamic) +impl From> for EitherError { + fn from(err: CborStoreError) -> Self { + EitherError::Amt(err.into()) } } -impl From for Error { - fn from(e: EncodingError) -> Self { - Self::Dynamic(anyhow!(e)) - } +#[derive(Debug, Error)] +pub enum CollapsedNodeError { + #[error("expected bitfield of length {0}, found bitfield with length {1}")] + LengthMissmatch(usize, usize), + #[error("Bitmap contained more set bits than links provided")] + MoreBitsThanLinks, + #[error("Bitmap contained less set bits than links provided")] + LessBitsThanLinks, + #[error("Bitmap contained more set bits than values provided")] + MoreBitsThanValues, + #[error("Bitmap contained less set bits than values provided")] + LessBitsThanValues, + /// Invalid formatted serialized node. + #[error("Serialized node cannot contain both links and values")] + LinksAndValues, } -impl From> for Error { - fn from(e: Box) -> Self { - Self::Dynamic(anyhow!(e)) - } +#[derive(Debug, Error)] +pub enum SerdeError { + /// Error when trying to serialize an AMT without a flushed cache + #[error("Tried to serialize without saving cache, run flush() on Amt before serializing")] + Cached, } diff --git a/ipld/amt/src/lib.rs b/ipld/amt/src/lib.rs index 72c7ed1f9..e70f57fbf 100644 --- a/ipld/amt/src/lib.rs +++ b/ipld/amt/src/lib.rs @@ -13,7 +13,7 @@ mod root; mod value_mut; pub use self::amt::Amt; -pub use self::error::Error; +pub use self::error::{EitherError, Error}; pub(crate) use self::node::Node; pub(crate) use self::root::Root; pub use self::value_mut::ValueMut; diff --git a/ipld/amt/src/node.rs b/ipld/amt/src/node.rs index d57199de4..44d1f54ba 100644 --- a/ipld/amt/src/node.rs +++ b/ipld/amt/src/node.rs @@ -3,7 +3,6 @@ use std::convert::{TryFrom, TryInto}; -use anyhow::anyhow; use cid::multihash::Code; use cid::Cid; use fvm_ipld_blockstore::Blockstore; @@ -13,6 +12,7 @@ use serde::de::{self, DeserializeOwned}; use serde::{ser, Deserialize, Serialize}; use super::ValueMut; +use crate::error::{CollapsedNodeError, EitherError, SerdeError}; use crate::{bmap_bytes, init_sized_vec, nodes_for_height, Error}; /// This represents a link to another Node @@ -106,7 +106,7 @@ where collapsed.push(cid); bmap[i / 8] |= 1 << (i % 8); } else { - return Err(ser::Error::custom(Error::Cached)); + return Err(ser::Error::custom(SerdeError::Cached)); } } } @@ -120,19 +120,17 @@ where pub(crate) struct CollapsedNode(#[serde(with = "serde_bytes")] Vec, Vec, Vec); impl CollapsedNode { - pub(crate) fn expand(self, bit_width: u32) -> Result, Error> { + pub(crate) fn expand(self, bit_width: u32) -> Result, CollapsedNodeError> { let CollapsedNode(bmap, links, values) = self; if !links.is_empty() && !values.is_empty() { - return Err(Error::LinksAndValues); + return Err(CollapsedNodeError::LinksAndValues); } if bmap_bytes(bit_width) != bmap.len() { - return Err(anyhow!( - "expected bitfield of length {}, found bitfield with length {}", + return Err(CollapsedNodeError::LengthMissmatch( bmap_bytes(bit_width), - bmap.len() - ) - .into()); + bmap.len(), + )); } if !links.is_empty() { @@ -140,13 +138,15 @@ impl CollapsedNode { let mut links = init_sized_vec::>(bit_width); for (i, v) in links.iter_mut().enumerate() { if bmap[i / 8] & (1 << (i % 8)) != 0 { - *v = Some(Link::from(links_iter.next().ok_or_else(|| { - anyhow!("Bitmap contained more set bits than links provided",) - })?)) + *v = Some(Link::from( + links_iter + .next() + .ok_or(CollapsedNodeError::MoreBitsThanLinks)?, + )) } } if links_iter.next().is_some() { - return Err(anyhow!("Bitmap contained less set bits than links provided",).into()); + return Err(CollapsedNodeError::LessBitsThanLinks); } Ok(Node::Link { links }) } else { @@ -154,13 +154,15 @@ impl CollapsedNode { let mut vals = init_sized_vec::(bit_width); for (i, v) in vals.iter_mut().enumerate() { if bmap[i / 8] & (1 << (i % 8)) != 0 { - *v = Some(val_iter.next().ok_or_else(|| { - anyhow!("Bitmap contained more set bits than values provided") - })?) + *v = Some( + val_iter + .next() + .ok_or(CollapsedNodeError::MoreBitsThanValues)?, + ) } } if val_iter.next().is_some() { - return Err(anyhow!("Bitmap contained less set bits than values provided").into()); + return Err(CollapsedNodeError::LessBitsThanValues); } Ok(Node::Leaf { vals }) } @@ -180,7 +182,7 @@ where } /// Flushes cache for node, replacing any cached values with a Cid variant - pub(super) fn flush(&mut self, bs: &DB) -> Result<(), Error> { + pub(super) fn flush(&mut self, bs: &DB) -> Result<(), Error> { if let Node::Link { links } = self { for link in links.iter_mut().flatten() { // links should only be flushed if the bitmap is set. @@ -235,21 +237,24 @@ where height: u32, bit_width: u32, i: u64, - ) -> Result, Error> { + ) -> Result, Error> { match self { Node::Leaf { vals, .. } => Ok(vals.get(i as usize).and_then(|v| v.as_ref())), Node::Link { links, .. } => { let sub_i: usize = (i / nodes_for_height(bit_width, height)) .try_into() .unwrap(); - match links.get(sub_i).and_then(|v| v.as_ref()) { - Some(Link::Cid { cid, cache }) => { - let cached_node = cache.get_or_try_init(|| { - bs.get_cbor::>(cid)? - .ok_or_else(|| Error::CidNotFound(cid.to_string()))? - .expand(bit_width) - .map(Box::new) - })?; + + match links.get(sub_i) { + Some(Some(Link::Cid { cid, cache })) => { + let cached_node = + cache.get_or_try_init(|| -> Result<_, Error> { + let node = bs + .get_cbor::>(cid)? + .ok_or_else(|| Error::CidNotFound(cid.to_string()))? + .expand(bit_width)?; + Ok(Box::new(node)) + })?; cached_node.get( bs, @@ -258,13 +263,13 @@ where i % nodes_for_height(bit_width, height), ) } - Some(Link::Dirty(n)) => n.get( + Some(Some(Link::Dirty(n))) => n.get( bs, height - 1, bit_width, i % nodes_for_height(bit_width, height), ), - None => Ok(None), + Some(None) | None => Ok(None), } } } @@ -278,7 +283,7 @@ where bit_width: u32, i: u64, val: V, - ) -> Result, Error> { + ) -> Result, Error> { if height == 0 { return Ok(self.set_leaf(i, val)); } @@ -350,7 +355,7 @@ where height: u32, bit_width: u32, i: u64, - ) -> Result, Error> { + ) -> Result, Error> { match self { Self::Leaf { vals } => Ok(vals .get_mut(usize::try_from(i).unwrap()) @@ -381,11 +386,13 @@ where } Some(Link::Cid { cid, cache }) => { // Take cache, will be replaced if no nodes deleted - cache.get_or_try_init(|| { - bs.get_cbor::>(cid)? + cache.get_or_try_init(|| -> Result<_, Error> { + let node = bs + .get_cbor::>(cid)? .ok_or_else(|| Error::CidNotFound(cid.to_string()))? - .expand(bit_width) - .map(Box::new) + .expand(bit_width)?; + + Ok(Box::new(node)) })?; let sub_node = cache.get_mut().expect("filled line above"); let deleted = sub_node.delete( @@ -419,23 +426,23 @@ where } } - pub(super) fn for_each_while( + pub(super) fn for_each_while( &self, bs: &S, height: u32, bit_width: u32, offset: u64, f: &mut F, - ) -> Result + ) -> Result> where - F: FnMut(u64, &V) -> anyhow::Result, + F: FnMut(u64, &V) -> Result, S: Blockstore, { match self { Node::Leaf { vals } => { for (i, v) in (0..).zip(vals.iter()) { if let Some(v) = v { - let keep_going = f(offset + i, v)?; + let keep_going = f(offset + i, v).map_err(EitherError::User)?; if !keep_going { return Ok(false); @@ -452,12 +459,14 @@ where sub.for_each_while(bs, height - 1, bit_width, offs, f)? } Link::Cid { cid, cache } => { - let cached_node = cache.get_or_try_init(|| { - bs.get_cbor::>(cid)? - .ok_or_else(|| Error::CidNotFound(cid.to_string()))? - .expand(bit_width) - .map(Box::new) - })?; + let cached_node = + cache.get_or_try_init(|| -> Result<_, Error> { + let node = bs + .get_cbor::>(cid)? + .ok_or_else(|| Error::CidNotFound(cid.to_string()))? + .expand(bit_width)?; + Ok(Box::new(node)) + })?; cached_node.for_each_while(bs, height - 1, bit_width, offs, f)? } @@ -478,16 +487,16 @@ where /// a closure call returned `Ok(false)`, indicating that a `break` has happened. /// `did_mutate` will be `true` iff any of the values in the node was actually /// mutated inside the closure, requiring the node to be cached. - pub(super) fn for_each_while_mut( + pub(super) fn for_each_while_mut( &mut self, bs: &S, height: u32, bit_width: u32, offset: u64, f: &mut F, - ) -> Result<(bool, bool), Error> + ) -> Result<(bool, bool), EitherError> where - F: FnMut(u64, &mut ValueMut<'_, V>) -> anyhow::Result, + F: FnMut(u64, &mut ValueMut<'_, V>) -> Result, S: Blockstore, { let mut did_mutate = false; @@ -498,7 +507,8 @@ where if let Some(v) = v { let mut value_mut = ValueMut::new(v); - let keep_going = f(offset + i, &mut value_mut)?; + let keep_going = + f(offset + i, &mut value_mut).map_err(EitherError::User)?; did_mutate |= value_mut.value_changed(); if !keep_going { @@ -516,11 +526,12 @@ where sub.for_each_while_mut(bs, height - 1, bit_width, offs, f)? } Link::Cid { cid, cache } => { - cache.get_or_try_init(|| { - bs.get_cbor::>(cid)? + cache.get_or_try_init(|| -> Result<_, Error> { + let node = bs + .get_cbor::>(cid)? .ok_or_else(|| Error::CidNotFound(cid.to_string()))? - .expand(bit_width) - .map(Box::new) + .expand(bit_width)?; + Ok(Box::new(node)) })?; let node = cache.get_mut().expect("cache filled on line above"); diff --git a/ipld/amt/tests/amt_tests.rs b/ipld/amt/tests/amt_tests.rs index a5263c1f5..04b86b0f7 100644 --- a/ipld/amt/tests/amt_tests.rs +++ b/ipld/amt/tests/amt_tests.rs @@ -322,7 +322,6 @@ fn for_each() { let mut x = 0; a.for_each(|_, _: &BytesDe| { x += 1; - Ok(()) }) .unwrap(); @@ -343,13 +342,12 @@ fn for_each() { ); } x += 1; - Ok(()) }) .unwrap(); assert_eq!(x, indexes.len()); // Iteration again will be read diff with go-interop, since they do not cache - new_amt.for_each(|_, _: &BytesDe| Ok(())).unwrap(); + new_amt.for_each(|_, _: &BytesDe| {}).unwrap(); assert_eq!( c.to_string().as_str(), @@ -385,7 +383,6 @@ fn for_each_mutate() { // Value it's set to doesn't matter, just cloning for expedience **v = v.clone(); } - Ok(()) }) .unwrap(); diff --git a/ipld/blockstore/Cargo.toml b/ipld/blockstore/Cargo.toml index 00681469a..f125bd56f 100644 --- a/ipld/blockstore/Cargo.toml +++ b/ipld/blockstore/Cargo.toml @@ -9,7 +9,6 @@ repository = "https://github.com/filecoin-project/ref-fvm" [dependencies] cid = { version = "0.8.2", default-features = false, features = ["serde-codec", "std"] } -anyhow = "1.0.51" [dev-dependencies] diff --git a/ipld/blockstore/src/lib.rs b/ipld/blockstore/src/lib.rs index 15a720d89..8e1c1eee0 100644 --- a/ipld/blockstore/src/lib.rs +++ b/ipld/blockstore/src/lib.rs @@ -1,6 +1,5 @@ use std::rc::Rc; -use anyhow::Result; use cid::{multihash, Cid}; pub mod tracking; @@ -15,8 +14,10 @@ pub use block::*; /// /// The cgo blockstore adapter implements this trait. pub trait Blockstore { + type Error: std::error::Error + Send + Sync + 'static; + /// Gets the block from the blockstore. - fn get(&self, k: &Cid) -> Result>>; + fn get(&self, k: &Cid) -> Result>, Self::Error>; /// Put a block with a pre-computed cid. /// @@ -24,17 +25,17 @@ pub trait Blockstore { /// even if you provide it. /// /// If you _do_ already know the CID, use this method as some blockstores _won't_ recompute it. - fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<()>; + fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<(), Self::Error>; /// Checks if the blockstore has the specified block. - fn has(&self, k: &Cid) -> Result { + fn has(&self, k: &Cid) -> Result { Ok(self.get(k)?.is_some()) } /// Puts the block into the blockstore, computing the hash with the specified multicodec. /// /// By default, this defers to put. - fn put(&self, mh_code: multihash::Code, block: &Block) -> Result + fn put(&self, mh_code: multihash::Code, block: &Block) -> Result where Self: Sized, D: AsRef<[u8]>, @@ -55,7 +56,7 @@ pub trait Blockstore { /// let blocks = vec![Block::new(0x55, vec![0, 1, 2])]; /// bs.put_many(blocks.iter().map(|b| (Blake2b256, b.into()))).unwrap(); /// ``` - fn put_many(&self, blocks: I) -> Result<()> + fn put_many(&self, blocks: I) -> Result<(), Self::Error> where Self: Sized, D: AsRef<[u8]>, @@ -68,7 +69,7 @@ pub trait Blockstore { /// Bulk-put pre-keyed blocks into the blockstore. /// /// By default, this defers to put_keyed. - fn put_many_keyed(&self, blocks: I) -> Result<()> + fn put_many_keyed(&self, blocks: I) -> Result<(), Self::Error> where Self: Sized, D: AsRef<[u8]>, @@ -82,26 +83,28 @@ pub trait Blockstore { } pub trait Buffered: Blockstore { - fn flush(&self, root: &Cid) -> Result<()>; + fn flush(&self, root: &Cid) -> Result<(), Self::Error>; } impl Blockstore for &BS where BS: Blockstore, { - fn get(&self, k: &Cid) -> Result>> { + type Error = BS::Error; + + fn get(&self, k: &Cid) -> Result>, Self::Error> { (*self).get(k) } - fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<()> { + fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<(), Self::Error> { (*self).put_keyed(k, block) } - fn has(&self, k: &Cid) -> Result { + fn has(&self, k: &Cid) -> Result { (*self).has(k) } - fn put(&self, mh_code: multihash::Code, block: &Block) -> Result + fn put(&self, mh_code: multihash::Code, block: &Block) -> Result where Self: Sized, D: AsRef<[u8]>, @@ -109,7 +112,7 @@ where (*self).put(mh_code, block) } - fn put_many(&self, blocks: I) -> Result<()> + fn put_many(&self, blocks: I) -> Result<(), Self::Error> where Self: Sized, D: AsRef<[u8]>, @@ -118,7 +121,7 @@ where (*self).put_many(blocks) } - fn put_many_keyed(&self, blocks: I) -> Result<()> + fn put_many_keyed(&self, blocks: I) -> Result<(), Self::Error> where Self: Sized, D: AsRef<[u8]>, @@ -132,19 +135,21 @@ impl Blockstore for Rc where BS: Blockstore, { - fn get(&self, k: &Cid) -> Result>> { + type Error = BS::Error; + + fn get(&self, k: &Cid) -> Result>, Self::Error> { (**self).get(k) } - fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<()> { + fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<(), Self::Error> { (**self).put_keyed(k, block) } - fn has(&self, k: &Cid) -> Result { + fn has(&self, k: &Cid) -> Result { (**self).has(k) } - fn put(&self, mh_code: multihash::Code, block: &Block) -> Result + fn put(&self, mh_code: multihash::Code, block: &Block) -> Result where Self: Sized, D: AsRef<[u8]>, @@ -152,7 +157,7 @@ where (**self).put(mh_code, block) } - fn put_many(&self, blocks: I) -> Result<()> + fn put_many(&self, blocks: I) -> Result<(), Self::Error> where Self: Sized, D: AsRef<[u8]>, @@ -161,7 +166,7 @@ where (**self).put_many(blocks) } - fn put_many_keyed(&self, blocks: I) -> Result<()> + fn put_many_keyed(&self, blocks: I) -> Result<(), Self::Error> where Self: Sized, D: AsRef<[u8]>, diff --git a/ipld/blockstore/src/memory.rs b/ipld/blockstore/src/memory.rs index 54a3a306a..7bec1a8ed 100644 --- a/ipld/blockstore/src/memory.rs +++ b/ipld/blockstore/src/memory.rs @@ -1,7 +1,7 @@ use std::cell::RefCell; use std::collections::HashMap; +use std::convert::Infallible; -use anyhow::Result; use cid::Cid; use super::Blockstore; @@ -18,15 +18,17 @@ impl MemoryBlockstore { } impl Blockstore for MemoryBlockstore { - fn has(&self, k: &Cid) -> Result { + type Error = Infallible; + + fn has(&self, k: &Cid) -> Result { Ok(self.blocks.borrow().contains_key(k)) } - fn get(&self, k: &Cid) -> Result>> { + fn get(&self, k: &Cid) -> Result>, Self::Error> { Ok(self.blocks.borrow().get(k).cloned()) } - fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<()> { + fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<(), Self::Error> { self.blocks.borrow_mut().insert(*k, block.into()); Ok(()) } diff --git a/ipld/blockstore/src/tracking.rs b/ipld/blockstore/src/tracking.rs index 244028162..7ca01fdf5 100644 --- a/ipld/blockstore/src/tracking.rs +++ b/ipld/blockstore/src/tracking.rs @@ -2,7 +2,6 @@ use std::cell::RefCell; -use anyhow::Result; use cid::multihash::{self, Code}; use cid::Cid; @@ -46,7 +45,9 @@ impl Blockstore for TrackingBlockstore where BS: Blockstore, { - fn get(&self, cid: &Cid) -> Result>> { + type Error = BS::Error; + + fn get(&self, cid: &Cid) -> Result>, Self::Error> { let mut stats = self.stats.borrow_mut(); stats.r += 1; let bytes = self.base.get(cid)?; @@ -55,12 +56,13 @@ where } Ok(bytes) } - fn has(&self, cid: &Cid) -> Result { + + fn has(&self, cid: &Cid) -> Result { self.stats.borrow_mut().r += 1; self.base.has(cid) } - fn put(&self, code: Code, block: &Block) -> Result + fn put(&self, code: Code, block: &Block) -> Result where D: AsRef<[u8]>, { @@ -70,14 +72,14 @@ where self.base.put(code, block) } - fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<()> { + fn put_keyed(&self, k: &Cid, block: &[u8]) -> Result<(), Self::Error> { let mut stats = self.stats.borrow_mut(); stats.w += 1; stats.bw += block.len(); self.base.put_keyed(k, block) } - fn put_many(&self, blocks: I) -> Result<()> + fn put_many(&self, blocks: I) -> Result<(), Self::Error> where Self: Sized, D: AsRef<[u8]>, @@ -91,7 +93,7 @@ where Ok(()) } - fn put_many_keyed(&self, blocks: I) -> Result<()> + fn put_many_keyed(&self, blocks: I) -> Result<(), Self::Error> where Self: Sized, D: AsRef<[u8]>, diff --git a/ipld/encoding/Cargo.toml b/ipld/encoding/Cargo.toml index d98d1a1ee..bf875f76d 100644 --- a/ipld/encoding/Cargo.toml +++ b/ipld/encoding/Cargo.toml @@ -15,8 +15,7 @@ serde_ipld_dagcbor = "0.1.2" serde_tuple = "0.5" serde_repr = "0.1" cid = { version = "0.8.2", default-features = false, features = ["serde-codec", "std"] } -thiserror = "1.0" -anyhow = "1.0.56" +thiserror = "1.0.30" fvm_ipld_blockstore = { version = "0.1", path = "../blockstore" } [features] diff --git a/ipld/encoding/src/cbor_store.rs b/ipld/encoding/src/cbor_store.rs index bb2a1a82e..02941d020 100644 --- a/ipld/encoding/src/cbor_store.rs +++ b/ipld/encoding/src/cbor_store.rs @@ -4,14 +4,22 @@ use serde::{de, ser}; use crate::DAG_CBOR; +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("blockstore: {0}")] + Blockstore(E), + #[error("encoding: {0}")] + Encoding(#[from] crate::errors::Error), +} + /// Wrapper for database to handle inserting and retrieving ipld data with Cids pub trait CborStore: Blockstore + Sized { /// Get typed object from block store by Cid. - fn get_cbor(&self, cid: &Cid) -> anyhow::Result> + fn get_cbor(&self, cid: &Cid) -> Result, Error> where T: de::DeserializeOwned, { - match self.get(cid)? { + match self.get(cid).map_err(Error::Blockstore)? { Some(bz) => { let res = crate::from_slice(&bz)?; Ok(Some(res)) @@ -21,7 +29,7 @@ pub trait CborStore: Blockstore + Sized { } /// Put an object in the block store and return the Cid identifier. - fn put_cbor(&self, obj: &S, code: multihash::Code) -> anyhow::Result + fn put_cbor(&self, obj: &S, code: multihash::Code) -> Result> where S: ser::Serialize, { @@ -33,6 +41,7 @@ pub trait CborStore: Blockstore + Sized { data: &bytes, }, ) + .map_err(Error::Blockstore) } } diff --git a/ipld/encoding/src/lib.rs b/ipld/encoding/src/lib.rs index d3eb0cb31..876ab1581 100644 --- a/ipld/encoding/src/lib.rs +++ b/ipld/encoding/src/lib.rs @@ -13,7 +13,7 @@ pub use serde_bytes; pub use self::bytes::*; pub use self::cbor::*; -pub use self::cbor_store::CborStore; +pub use self::cbor_store::{CborStore, Error as CborStoreError}; pub use self::errors::*; pub use self::vec::*; diff --git a/ipld/hamt/Cargo.toml b/ipld/hamt/Cargo.toml index 672eec161..08a6f41e1 100644 --- a/ipld/hamt/Cargo.toml +++ b/ipld/hamt/Cargo.toml @@ -17,7 +17,6 @@ thiserror = "1.0" sha2 = "0.10" once_cell = "1.5" forest_hash_utils = "0.1" -anyhow = "1.0.51" libipld-core = { version = "0.13.1", features = ["serde-codec"] } fvm_ipld_encoding = { version = "0.1", path = "../encoding" } fvm_ipld_blockstore = { version = "0.1", path = "../blockstore" } diff --git a/ipld/hamt/benches/hamt_benchmark.rs b/ipld/hamt/benches/hamt_benchmark.rs index 5e071e4ad..cf4ffde56 100644 --- a/ipld/hamt/benches/hamt_benchmark.rs +++ b/ipld/hamt/benches/hamt_benchmark.rs @@ -94,7 +94,9 @@ fn for_each(c: &mut Criterion) { c.bench_function("HAMT for_each function", |b| { b.iter(|| { let a = Hamt::<_, _>::load(&cid, &db).unwrap(); - black_box(a).for_each(|_k, _v: &BenchData| Ok(())).unwrap(); + black_box(a) + .try_for_each(|_k, _v: &BenchData| Ok::<_, ()>(())) + .unwrap(); }) }); } diff --git a/ipld/hamt/src/error.rs b/ipld/hamt/src/error.rs index 0937e5824..2b7ad2193 100644 --- a/ipld/hamt/src/error.rs +++ b/ipld/hamt/src/error.rs @@ -1,59 +1,57 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use std::error::Error as StdError; - -use fvm_ipld_encoding::Error as EncodingError; +use fvm_ipld_encoding::{CborStoreError, Error as EncodingError}; use thiserror::Error; /// HAMT Error #[derive(Debug, Error)] -pub enum Error { - /// Maximum depth error - #[error("Maximum depth reached")] - MaxDepth, - /// Hash bits does not support greater than 8 bit width - #[error("HashBits does not support retrieving more than 8 bits")] - InvalidHashBitLen, +pub enum Error { + #[error("hashbits: {0}")] + HashBits(#[from] HashBitsError), /// This should be treated as a fatal error, must have at least one pointer in node #[error("Invalid HAMT format, node cannot have 0 pointers")] ZeroPointers, /// Cid not found in store error #[error("Cid ({0}) did not match any in database")] CidNotFound(String), - // TODO: This should be something like "internal" or "io". And we shouldn't have both this and - // "other"; they serve the same purpose. - /// Dynamic error for when the error needs to be forwarded as is. - #[error("{0}")] - Dynamic(anyhow::Error), -} - -impl From for Error { - fn from(e: String) -> Self { - Self::Dynamic(anyhow::anyhow!(e)) - } + #[error("blockstore {0}")] + Blockstore(E), + #[error("encoding error {0}")] + Encoding(#[from] EncodingError), } -impl From<&'static str> for Error { - fn from(e: &'static str) -> Self { - Self::Dynamic(anyhow::anyhow!(e)) +impl From> for Error { + fn from(err: CborStoreError) -> Self { + match err { + CborStoreError::Blockstore(err) => Error::Blockstore(err), + CborStoreError::Encoding(err) => Error::Encoding(err), + } } } -impl From for Error { - fn from(e: anyhow::Error) -> Self { - e.downcast::().unwrap_or_else(Self::Dynamic) - } +/// This error wraps around around two different errors, either the native `Error` from `hamt`, or +/// a custom user error, returned from executing a user defined function. +#[derive(Debug, Error)] +pub enum EitherError { + #[error("user: {0}")] + User(U), + #[error("hamt: {0}")] + Hamt(#[from] Error), } -impl From for Error { - fn from(e: EncodingError) -> Self { - Self::Dynamic(anyhow::anyhow!(e)) +impl From> for EitherError { + fn from(err: CborStoreError) -> Self { + EitherError::Hamt(err.into()) } } -impl From> for Error { - fn from(e: Box) -> Self { - Self::Dynamic(anyhow::anyhow!(e)) - } +#[derive(Error, Debug)] +pub enum HashBitsError { + /// Maximum depth error + #[error("Maximum depth reached")] + MaxDepth, + /// Hash bits does not support greater than 8 bit width + #[error("HashBits does not support retrieving more than 8 bits")] + InvalidLen, } diff --git a/ipld/hamt/src/hamt.rs b/ipld/hamt/src/hamt.rs index 48f91d653..c1129169f 100644 --- a/ipld/hamt/src/hamt.rs +++ b/ipld/hamt/src/hamt.rs @@ -12,6 +12,7 @@ use multihash::Code; use serde::de::DeserializeOwned; use serde::{Serialize, Serializer}; +use crate::error::EitherError; use crate::node::Node; use crate::{Error, Hash, HashAlgorithm, Sha256, DEFAULT_BIT_WIDTH}; @@ -82,12 +83,16 @@ where } /// Lazily instantiate a hamt from this root Cid. - pub fn load(cid: &Cid, store: BS) -> Result { + pub fn load(cid: &Cid, store: BS) -> Result> { Self::load_with_bit_width(cid, store, DEFAULT_BIT_WIDTH) } /// Lazily instantiate a hamt from this root Cid with a specified bit width. - pub fn load_with_bit_width(cid: &Cid, store: BS, bit_width: u32) -> Result { + pub fn load_with_bit_width( + cid: &Cid, + store: BS, + bit_width: u32, + ) -> Result> { match store.get_cbor(cid)? { Some(root) => Ok(Self { root, @@ -100,7 +105,7 @@ where } /// Sets the root based on the Cid of the root node using the Hamt store - pub fn set_root(&mut self, cid: &Cid) -> Result<(), Error> { + pub fn set_root(&mut self, cid: &Cid) -> Result<(), Error> { match self.store.get_cbor(cid)? { Some(root) => self.root = root, None => return Err(Error::CidNotFound(cid.to_string())), @@ -136,7 +141,7 @@ where /// map.set(37, "b".to_string()).unwrap(); /// map.set(37, "c".to_string()).unwrap(); /// ``` - pub fn set(&mut self, key: K, value: V) -> Result, Error> + pub fn set(&mut self, key: K, value: V) -> Result, Error> where V: PartialEq, { @@ -171,7 +176,7 @@ where /// let c = map.set_if_absent(30, "c".to_string()).unwrap(); /// assert_eq!(c, true); /// ``` - pub fn set_if_absent(&mut self, key: K, value: V) -> Result + pub fn set_if_absent(&mut self, key: K, value: V) -> Result> where V: PartialEq, { @@ -200,7 +205,7 @@ where /// assert_eq!(map.get(&2).unwrap(), None); /// ``` #[inline] - pub fn get(&self, k: &Q) -> Result, Error> + pub fn get(&self, k: &Q) -> Result, Error> where K: Borrow, Q: Hash + Eq, @@ -232,7 +237,7 @@ where /// assert_eq!(map.contains_key(&2).unwrap(), false); /// ``` #[inline] - pub fn contains_key(&self, k: &Q) -> Result + pub fn contains_key(&self, k: &Q) -> Result> where K: Borrow, Q: Hash + Eq, @@ -263,7 +268,7 @@ where /// assert_eq!(map.delete(&1).unwrap(), Some((1, "a".to_string()))); /// assert_eq!(map.delete(&1).unwrap(), None); /// ``` - pub fn delete(&mut self, k: &Q) -> Result, Error> + pub fn delete(&mut self, k: &Q) -> Result, Error> where K: Borrow, Q: Hash + Eq, @@ -273,7 +278,7 @@ where } /// Flush root and return Cid for hamt - pub fn flush(&mut self) -> Result { + pub fn flush(&mut self) -> Result> { self.root.flush(self.store.borrow())?; Ok(self.store.put_cbor(&self.root, Code::Blake2b256)?) } @@ -299,21 +304,37 @@ where /// map.set(4, 2).unwrap(); /// /// let mut total = 0; - /// map.for_each(|_, v: &u64| { + /// map.try_for_each(|_, v: &u64| { /// total += v; - /// Ok(()) + /// Ok::<(), ()>(()) /// }).unwrap(); /// assert_eq!(total, 3); /// ``` #[inline] - pub fn for_each(&self, mut f: F) -> Result<(), Error> + pub fn try_for_each(&self, mut f: F) -> Result<(), EitherError> where V: DeserializeOwned, - F: FnMut(&K, &V) -> anyhow::Result<()>, + F: FnMut(&K, &V) -> Result<(), U>, { self.root.for_each(self.store.borrow(), &mut f) } + #[inline] + pub fn for_each(&self, mut f: F) -> Result<(), Error> + where + V: DeserializeOwned, + F: FnMut(&K, &V), + { + self.try_for_each(|k, v| { + f(k, v); + Ok(()) + }) + .map_err(|err| match err { + EitherError::User(()) => unreachable!(), + EitherError::Hamt(e) => e, + }) + } + /// Consumes this HAMT and returns the Blockstore it owns. pub fn into_store(self) -> BS { self.store diff --git a/ipld/hamt/src/hash_bits.rs b/ipld/hamt/src/hash_bits.rs index c688270e2..3309d7001 100644 --- a/ipld/hamt/src/hash_bits.rs +++ b/ipld/hamt/src/hash_bits.rs @@ -3,7 +3,7 @@ use std::cmp::Ordering; -use crate::{Error, HashedKey}; +use crate::{HashBitsError, HashedKey}; /// Helper struct which indexes and allows returning bits from a hashed key #[derive(Debug, Clone, Copy)] @@ -32,12 +32,12 @@ impl<'a> HashBits<'a> { /// Returns next `i` bits of the hash and returns the value as an integer and returns /// Error when maximum depth is reached - pub fn next(&mut self, i: u32) -> Result { + pub fn next(&mut self, i: u32) -> Result { if i > 8 { - return Err(Error::InvalidHashBitLen); + return Err(HashBitsError::InvalidLen); } if (self.consumed + i) as usize > self.b.len() * 8 { - return Err(Error::MaxDepth); + return Err(HashBitsError::MaxDepth); } Ok(self.next_bits(i)) } @@ -94,11 +94,11 @@ mod tests { assert_eq!(hb.next(5).unwrap(), 0b01010); assert_eq!(hb.next(6).unwrap(), 0b111111); assert_eq!(hb.next(8).unwrap(), 0b11111111); - assert!(matches!(hb.next(9), Err(Error::InvalidHashBitLen))); + assert!(matches!(hb.next(9), Err(HashBitsError::InvalidLen))); for _ in 0..28 { // Iterate through rest of key to test depth hb.next(8).unwrap(); } - assert!(matches!(hb.next(1), Err(Error::MaxDepth))); + assert!(matches!(hb.next(1), Err(HashBitsError::MaxDepth))); } } diff --git a/ipld/hamt/src/lib.rs b/ipld/hamt/src/lib.rs index c8de66b44..9cfa7abf2 100644 --- a/ipld/hamt/src/lib.rs +++ b/ipld/hamt/src/lib.rs @@ -21,7 +21,7 @@ mod pointer; pub use forest_hash_utils::{BytesKey, Hash}; use serde::{Deserialize, Serialize}; -pub use self::error::Error; +pub use self::error::*; pub use self::hamt::Hamt; pub use self::hash::*; pub use self::hash_algorithm::*; diff --git a/ipld/hamt/src/node.rs b/ipld/hamt/src/node.rs index 03ee347ba..ba135f56d 100644 --- a/ipld/hamt/src/node.rs +++ b/ipld/hamt/src/node.rs @@ -16,6 +16,7 @@ use super::bitfield::Bitfield; use super::hash_bits::HashBits; use super::pointer::Pointer; use super::{Error, Hash, HashAlgorithm, KeyValuePair, MAX_ARRAY_WIDTH}; +use crate::error::EitherError; /// Node in Hamt tree which contains bitfield of set indexes and pointers to nodes #[derive(Debug)] @@ -85,7 +86,7 @@ where store: &S, bit_width: u32, overwrite: bool, - ) -> Result<(Option, bool), Error> + ) -> Result<(Option, bool), Error> where V: PartialEq, { @@ -107,7 +108,7 @@ where k: &Q, store: &S, bit_width: u32, - ) -> Result, Error> + ) -> Result, Error> where K: Borrow, Q: Eq + Hash, @@ -121,7 +122,7 @@ where k: &Q, store: &S, bit_width: u32, - ) -> Result, Error> + ) -> Result, Error> where K: Borrow, Q: Eq + Hash, @@ -135,9 +136,13 @@ where self.pointers.is_empty() } - pub(crate) fn for_each(&self, store: &S, f: &mut F) -> Result<(), Error> + pub(crate) fn for_each( + &self, + store: &S, + f: &mut F, + ) -> Result<(), EitherError> where - F: FnMut(&K, &V) -> anyhow::Result<()>, + F: FnMut(&K, &V) -> Result<(), U>, S: Blockstore, { for p in &self.pointers { @@ -150,7 +155,7 @@ where node } else { #[cfg(not(feature = "ignore-dead-links"))] - return Err(Error::CidNotFound(cid.to_string())); + return Err(Error::CidNotFound(cid.to_string()).into()); #[cfg(feature = "ignore-dead-links")] continue; @@ -164,7 +169,7 @@ where Pointer::Dirty(n) => n.for_each(store, f)?, Pointer::Values(kvs) => { for kv in kvs { - f(kv.0.borrow(), kv.1.borrow())?; + f(kv.0.borrow(), kv.1.borrow()).map_err(EitherError::User)?; } } } @@ -178,7 +183,7 @@ where q: &Q, store: &S, bit_width: u32, - ) -> Result>, Error> + ) -> Result>, Error> where K: Borrow, Q: Eq + Hash, @@ -194,7 +199,7 @@ where depth: u64, key: &Q, store: &S, - ) -> Result>, Error> + ) -> Result>, Error> where K: Borrow, Q: Eq + Hash, @@ -244,7 +249,7 @@ where value: V, store: &S, overwrite: bool, - ) -> Result<(Option, bool), Error> + ) -> Result<(Option, bool), Error> where V: PartialEq, { @@ -364,7 +369,7 @@ where depth: u64, key: &Q, store: &S, - ) -> Result, Error> + ) -> Result, Error> where K: Borrow, Q: Hash + Eq, @@ -392,7 +397,7 @@ where if deleted.is_some() { *child = Pointer::Dirty(std::mem::take(child_node)); // Clean to retrieve canonical form - child.clean()?; + child.clean::()?; } Ok(deleted) @@ -402,7 +407,7 @@ where let deleted = n.rm_value(hashed_key, bit_width, depth + 1, key, store)?; // Clean to ensure canonical form - child.clean()?; + child.clean::()?; Ok(deleted) } Pointer::Values(vals) => { @@ -427,7 +432,7 @@ where } } - pub fn flush(&mut self, store: &S) -> Result<(), Error> { + pub fn flush(&mut self, store: &S) -> Result<(), Error> { for pointer in &mut self.pointers { if let Pointer::Dirty(node) = pointer { // Flush cached sub node to clear it's cache diff --git a/ipld/hamt/src/pointer.rs b/ipld/hamt/src/pointer.rs index e024cefb1..ff001e4c4 100644 --- a/ipld/hamt/src/pointer.rs +++ b/ipld/hamt/src/pointer.rs @@ -5,6 +5,7 @@ use std::cmp::Ordering; use std::convert::{TryFrom, TryInto}; use cid::Cid; +use fvm_ipld_blockstore::Blockstore; use libipld_core::ipld::Ipld; use once_cell::unsync::OnceCell; use serde::de::{self, DeserializeOwned}; @@ -111,7 +112,7 @@ where /// Internal method to cleanup children, to ensure consistent tree representation /// after deletes. - pub(crate) fn clean(&mut self) -> Result<(), Error> { + pub(crate) fn clean(&mut self) -> Result<(), Error> { match self { Pointer::Dirty(n) => match n.pointers.len() { 0 => Err(Error::ZeroPointers), diff --git a/ipld/hamt/tests/hamt_tests.rs b/ipld/hamt/tests/hamt_tests.rs index 4961663a2..37581dc42 100644 --- a/ipld/hamt/tests/hamt_tests.rs +++ b/ipld/hamt/tests/hamt_tests.rs @@ -273,7 +273,6 @@ fn for_each() { hamt.for_each(|k, v| { assert_eq!(k, v); count += 1; - Ok(()) }) .unwrap(); assert_eq!(count, 200); @@ -291,7 +290,6 @@ fn for_each() { hamt.for_each(|k, v| { assert_eq!(k, v); count += 1; - Ok(()) }) .unwrap(); assert_eq!(count, 200); @@ -301,7 +299,6 @@ fn for_each() { hamt.for_each(|k, v| { assert_eq!(k, v); count += 1; - Ok(()) }) .unwrap(); assert_eq!(count, 200); diff --git a/shared/Cargo.toml b/shared/Cargo.toml index 3e900bb82..6ef5acfe0 100644 --- a/shared/Cargo.toml +++ b/shared/Cargo.toml @@ -23,7 +23,6 @@ log = "0.4.8" cid = { version = "0.8.2", default-features = false, features = ["serde-codec", "std"] } multihash = { version = "0.16.1", default-features = false, features = ["blake2b", "multihash-impl"] } unsigned-varint = "0.7.1" -anyhow = "1.0.51" bimap = { version = "0.6.2", features = ["serde"] } fvm_ipld_blockstore = { version = "0.1", path = "../ipld/blockstore" } fvm_ipld_encoding = { version = "0.1", path = "../ipld/encoding" } diff --git a/shared/src/actor/builtin.rs b/shared/src/actor/builtin.rs index bf4260281..e4b2f9a2f 100644 --- a/shared/src/actor/builtin.rs +++ b/shared/src/actor/builtin.rs @@ -1,10 +1,9 @@ use std::fmt::{Debug, Display, Formatter}; -use anyhow::anyhow; use bimap::BiBTreeMap; use cid::Cid; use fvm_ipld_blockstore::Blockstore; -use fvm_ipld_encoding::CborStore; +use fvm_ipld_encoding::{CborStore, CborStoreError}; use num_derive::FromPrimitive; use serde_repr::{Deserialize_repr, Serialize_repr}; @@ -115,26 +114,36 @@ impl Display for Type { /// A mapping of builtin actor CIDs to their respective types. pub type Manifest = BiBTreeMap; -pub fn load_manifest(bs: &B, root_cid: &Cid, ver: u32) -> anyhow::Result { +pub fn load_manifest( + bs: &B, + root_cid: &Cid, + ver: u32, +) -> Result> { match ver { 0 => load_manifest_v0(bs, root_cid), 1 => load_manifest_v1(bs, root_cid), - _ => Err(anyhow!("unknown manifest version {}", ver)), + _ => Err(ManifestError::UnknownVersion(ver)), } } -pub fn load_manifest_v0(bs: &B, root_cid: &Cid) -> anyhow::Result { +pub fn load_manifest_v0( + bs: &B, + root_cid: &Cid, +) -> Result> { match bs.get_cbor::(root_cid)? { Some(mf) => Ok(mf), - None => Err(anyhow!("cannot find manifest root cid {}", root_cid)), + None => Err(ManifestError::MissingRootCid(*root_cid)), } } -pub fn load_manifest_v1(bs: &B, root_cid: &Cid) -> anyhow::Result { +pub fn load_manifest_v1( + bs: &B, + root_cid: &Cid, +) -> Result> { let vec: Vec<(String, Cid)> = match bs.get_cbor(root_cid)? { Some(vec) => vec, None => { - return Err(anyhow!("cannot find manifest root cid {}", root_cid)); + return Err(ManifestError::MissingRootCid(*root_cid)); } }; let mut manifest = Manifest::new(); @@ -145,9 +154,21 @@ pub fn load_manifest_v1(bs: &B, root_cid: &Cid) -> anyhow::Result manifest.insert(code_cid, t); } Err(what) => { - return Err(anyhow!("bad builtin actor name: {}: {} ", name, what)); + return Err(ManifestError::BadBuiltinActor(name, what)); } } } Ok(manifest) } + +#[derive(thiserror::Error, Debug)] +pub enum ManifestError { + #[error("unknown manifest version {0}")] + UnknownVersion(u32), + #[error("cannot find manifest root cid {0}")] + MissingRootCid(Cid), + #[error("bad builtin actor name: {0}: {1}")] + BadBuiltinActor(String, String), + #[error("encoding {0}")] + Encoding(#[from] CborStoreError), +} diff --git a/shared/src/message.rs b/shared/src/message.rs index f07333334..3dd2cded7 100644 --- a/shared/src/message.rs +++ b/shared/src/message.rs @@ -1,7 +1,6 @@ // Copyright 2019-2022 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use anyhow::anyhow; use fvm_ipld_encoding::de::{Deserialize, Deserializer}; use fvm_ipld_encoding::ser::{Serialize, Serializer}; use fvm_ipld_encoding::{Cbor, RawBytes}; @@ -38,17 +37,25 @@ impl Message { } /// Does some basic checks on the Message to see if the fields are valid. - pub fn check(self: &Message) -> anyhow::Result<()> { + pub fn check(self: &Message) -> Result<(), MessageError> { if self.gas_limit == 0 { - return Err(anyhow!("Message has no gas limit set")); + return Err(MessageError::MissingGasLimit); } if self.gas_limit < 0 { - return Err(anyhow!("Message has negative gas limit")); + return Err(MessageError::NegativeGasLimit); } Ok(()) } } +#[derive(thiserror::Error, Debug)] +pub enum MessageError { + #[error("Message has no gas limit set")] + MissingGasLimit, + #[error("Message has negative gas limit")] + NegativeGasLimit, +} + impl Serialize for Message { fn serialize(&self, s: S) -> std::result::Result where