Merge branch 'tomas/vp-tx-env-concrete-error' (#465)

* tomas/vp-tx-env-concrete-error:
  changelog: add #465
  [ci skip] wasm checksums update
  ledger: use storage_api::Error in VpEnv and TxEnv instead of generic

.changelog/unreleased/improvements/465-vp-tx-env-conrete-error.md

@@ -0,0 +1,2 @@
+- Re-use `storage_api::Error` type that supports wrapping custom error in `VpEnv` and `TxEnv` traits.
+  ([#465](https://github.com/anoma/namada/pull/465))

shared/src/ledger/ibc/handler.rs

@@ -70,6 +70,7 @@ use crate::ibc::events::IbcEvent;
 use crate::ibc::mock::client_state::{MockClientState, MockConsensusState};
 use crate::ibc::timestamp::Timestamp;
 use crate::ledger::ibc::storage;
+use crate::ledger::storage_api;
 use crate::tendermint::Time;
 use crate::tendermint_proto::{Error as ProtoError, Protobuf};
 use crate::types::address::{Address, InternalAddress};
@@ -117,6 +118,14 @@ pub enum Error {
     ReceivingToken(String),
 }
 
+// This is needed to use `ibc::Handler::Error` with `IbcActions` in
+// `tx_prelude/src/ibc.rs`
+impl From<Error> for storage_api::Error {
+    fn from(err: Error) -> Self {
+        storage_api::Error::new(err)
+    }
+}
+
 /// for handling IBC modules
 pub type Result<T> = std::result::Result<T, Error>;
 

shared/src/ledger/native_vp.rs

@@ -17,7 +17,9 @@ use crate::vm::prefix_iter::PrefixIterators;
 use crate::vm::WasmCacheAccess;
 
 /// Possible error in a native VP host function call
-pub type Error = vp_env::RuntimeError;
+/// The `storage_api::Error` may wrap the `vp_env::RuntimeError` and can
+/// be extended with other custom errors when using `trait VpEnv`.
+pub type Error = storage_api::Error;
 
 /// A native VP module should implement its validation logic using this trait.
 pub trait NativeVp {
@@ -201,23 +203,23 @@ where
         &self,
         prefix: &crate::types::storage::Key,
     ) -> Result<Self::PrefixIter, storage_api::Error> {
-        self.ctx.iter_prefix(prefix).into_storage_result()
+        self.ctx.iter_prefix(prefix)
     }
 
     fn get_chain_id(&self) -> Result<String, storage_api::Error> {
-        self.ctx.get_chain_id().into_storage_result()
+        self.ctx.get_chain_id()
     }
 
     fn get_block_height(&self) -> Result<BlockHeight, storage_api::Error> {
-        self.ctx.get_block_height().into_storage_result()
+        self.ctx.get_block_height()
     }
 
     fn get_block_hash(&self) -> Result<BlockHash, storage_api::Error> {
-        self.ctx.get_block_hash().into_storage_result()
+        self.ctx.get_block_hash()
     }
 
     fn get_block_epoch(&self) -> Result<Epoch, storage_api::Error> {
-        self.ctx.get_block_epoch().into_storage_result()
+        self.ctx.get_block_epoch()
     }
 }
 
@@ -275,23 +277,23 @@ where
         &self,
         prefix: &crate::types::storage::Key,
     ) -> Result<Self::PrefixIter, storage_api::Error> {
-        self.ctx.iter_prefix(prefix).into_storage_result()
+        self.ctx.iter_prefix(prefix)
     }
 
     fn get_chain_id(&self) -> Result<String, storage_api::Error> {
-        self.ctx.get_chain_id().into_storage_result()
+        self.ctx.get_chain_id()
     }
 
     fn get_block_height(&self) -> Result<BlockHeight, storage_api::Error> {
-        self.ctx.get_block_height().into_storage_result()
+        self.ctx.get_block_height()
     }
 
     fn get_block_hash(&self) -> Result<BlockHash, storage_api::Error> {
-        self.ctx.get_block_hash().into_storage_result()
+        self.ctx.get_block_hash()
     }
 
     fn get_block_epoch(&self) -> Result<Epoch, storage_api::Error> {
-        self.ctx.get_block_epoch().into_storage_result()
+        self.ctx.get_block_epoch()
     }
 }
 
@@ -301,7 +303,6 @@ where
     H: 'static + StorageHasher,
     CA: 'static + WasmCacheAccess,
 {
-    type Error = Error;
     type Post = CtxPostStorageRead<'view, 'a, DB, H, CA>;
     type Pre = CtxPreStorageRead<'view, 'a, DB, H, CA>;
     type PrefixIter = <DB as storage::DBIter<'a>>::PrefixIter;
@@ -317,61 +318,70 @@ where
     fn read_temp<T: borsh::BorshDeserialize>(
         &self,
         key: &Key,
-    ) -> Result<Option<T>, Self::Error> {
+    ) -> Result<Option<T>, storage_api::Error> {
         vp_env::read_temp(
             &mut *self.gas_meter.borrow_mut(),
             self.write_log,
             key,
         )
         .map(|data| data.and_then(|t| T::try_from_slice(&t[..]).ok()))
+        .into_storage_result()
     }
 
     fn read_bytes_temp(
         &self,
         key: &Key,
-    ) -> Result<Option<Vec<u8>>, Self::Error> {
+    ) -> Result<Option<Vec<u8>>, storage_api::Error> {
         vp_env::read_temp(
             &mut *self.gas_meter.borrow_mut(),
             self.write_log,
             key,
         )
+        .into_storage_result()
     }
 
-    fn get_chain_id(&'view self) -> Result<String, Self::Error> {
+    fn get_chain_id(&'view self) -> Result<String, storage_api::Error> {
         vp_env::get_chain_id(&mut *self.gas_meter.borrow_mut(), self.storage)
+            .into_storage_result()
     }
 
-    fn get_block_height(&'view self) -> Result<BlockHeight, Self::Error> {
+    fn get_block_height(
+        &'view self,
+    ) -> Result<BlockHeight, storage_api::Error> {
         vp_env::get_block_height(
             &mut *self.gas_meter.borrow_mut(),
             self.storage,
         )
+        .into_storage_result()
     }
 
-    fn get_block_hash(&'view self) -> Result<BlockHash, Self::Error> {
+    fn get_block_hash(&'view self) -> Result<BlockHash, storage_api::Error> {
         vp_env::get_block_hash(&mut *self.gas_meter.borrow_mut(), self.storage)
+            .into_storage_result()
     }
 
-    fn get_block_epoch(&'view self) -> Result<Epoch, Self::Error> {
+    fn get_block_epoch(&'view self) -> Result<Epoch, storage_api::Error> {
         vp_env::get_block_epoch(&mut *self.gas_meter.borrow_mut(), self.storage)
+            .into_storage_result()
     }
 
     fn iter_prefix(
         &'view self,
         prefix: &Key,
-    ) -> Result<Self::PrefixIter, Self::Error> {
+    ) -> Result<Self::PrefixIter, storage_api::Error> {
         vp_env::iter_prefix(
             &mut *self.gas_meter.borrow_mut(),
             self.storage,
             prefix,
         )
+        .into_storage_result()
     }
 
     fn eval(
         &self,
         vp_code: Vec<u8>,
         input_data: Vec<u8>,
-    ) -> Result<bool, Self::Error> {
+    ) -> Result<bool, storage_api::Error> {
         #[cfg(feature = "wasm-runtime")]
         {
             use std::marker::PhantomData;
@@ -429,11 +439,12 @@ where
         &self,
         pk: &crate::types::key::common::PublicKey,
         sig: &crate::types::key::common::Signature,
-    ) -> Result<bool, Self::Error> {
+    ) -> Result<bool, storage_api::Error> {
         Ok(self.tx.verify_sig(pk, sig).is_ok())
     }
 
-    fn get_tx_code_hash(&self) -> Result<Hash, Self::Error> {
+    fn get_tx_code_hash(&self) -> Result<Hash, storage_api::Error> {
         vp_env::get_tx_code_hash(&mut *self.gas_meter.borrow_mut(), self.tx)
+            .into_storage_result()
     }
 }

shared/src/ledger/pos/mod.rs

@@ -13,6 +13,7 @@ use namada_proof_of_stake::PosBase;
 pub use storage::*;
 pub use vp::PosVP;
 
+use super::storage_api;
 use crate::ledger::storage::{self as ledger_storage, Storage, StorageHasher};
 use crate::types::address::{self, Address, InternalAddress};
 use crate::types::storage::Epoch;
@@ -88,6 +89,40 @@ impl From<namada_proof_of_stake::types::Epoch> for Epoch {
     }
 }
 
+// The error conversions are needed to implement `PosActions` in
+// `tx_prelude/src/proof_of_stake.rs`
+impl From<namada_proof_of_stake::BecomeValidatorError<Address>>
+    for storage_api::Error
+{
+    fn from(err: namada_proof_of_stake::BecomeValidatorError<Address>) -> Self {
+        Self::new(err)
+    }
+}
+
+impl From<namada_proof_of_stake::BondError<Address>> for storage_api::Error {
+    fn from(err: namada_proof_of_stake::BondError<Address>) -> Self {
+        Self::new(err)
+    }
+}
+
+impl From<namada_proof_of_stake::UnbondError<Address, token::Amount>>
+    for storage_api::Error
+{
+    fn from(
+        err: namada_proof_of_stake::UnbondError<Address, token::Amount>,
+    ) -> Self {
+        Self::new(err)
+    }
+}
+
+impl From<namada_proof_of_stake::WithdrawError<Address>>
+    for storage_api::Error
+{
+    fn from(err: namada_proof_of_stake::WithdrawError<Address>) -> Self {
+        Self::new(err)
+    }
+}
+
 #[macro_use]
 mod macros {
     /// Implement `PosReadOnly` for a type that implements
@@ -115,7 +150,6 @@ mod macros {
         $( $any )*
         {
             type Address = $crate::types::address::Address;
-            // type Error = $crate::ledger::native_vp::Error;
             type $error = $err_ty;
             type PublicKey = $crate::types::key::common::PublicKey;
             type TokenAmount = $crate::types::token::Amount;

shared/src/ledger/pos/vp.rs

@@ -47,8 +47,6 @@ use crate::vm::WasmCacheAccess;
 pub enum Error {
     #[error("Native VP error: {0}")]
     NativeVpError(native_vp::Error),
-    #[error("Storage error: {0}")]
-    StorageApi(storage_api::Error),
 }
 
 /// PoS functions result
@@ -303,7 +301,7 @@ where
 }
 
 impl_pos_read_only! {
-    type Error = native_vp::Error;
+    type Error = storage_api::Error;
     impl<'f, 'a, DB, H, CA> PosReadOnly for CtxPreStorageRead<'f, 'a, DB, H, CA>
         where
             DB: ledger_storage::DB + for<'iter> ledger_storage::DBIter<'iter> +'static,
@@ -312,7 +310,7 @@ impl_pos_read_only! {
 }
 
 impl_pos_read_only! {
-    type Error = native_vp::Error;
+    type Error = storage_api::Error;
     impl<'f, 'a, DB, H, CA> PosReadOnly for CtxPostStorageRead<'f, 'a, DB, H, CA>
         where
             DB: ledger_storage::DB + for<'iter> ledger_storage::DBIter<'iter> +'static,
@@ -325,9 +323,3 @@ impl From<native_vp::Error> for Error {
         Self::NativeVpError(err)
     }
 }
-
-impl From<storage_api::Error> for Error {
-    fn from(err: storage_api::Error) -> Self {
-        Self::StorageApi(err)
-    }
-}

shared/src/ledger/storage_api/error.rs

@@ -6,6 +6,8 @@ use thiserror::Error;
 #[allow(missing_docs)]
 #[derive(Error, Debug)]
 pub enum Error {
+    #[error("{0}")]
+    SimpleMessage(&'static str),
     #[error("{0}")]
     Custom(CustomError),
     #[error("{0}: {1}")]
@@ -48,6 +50,12 @@ impl Error {
         Self::Custom(CustomError(error.into()))
     }
 
+    /// Create an [`enum@Error`] from a static message.
+    #[inline]
+    pub const fn new_const(msg: &'static str) -> Self {
+        Self::SimpleMessage(msg)
+    }
+
     /// Wrap another [`std::error::Error`] with a static message.
     pub fn wrap<E>(msg: &'static str, error: E) -> Self
     where
@@ -66,3 +74,18 @@ impl std::fmt::Display for CustomError {
         self.0.fmt(f)
     }
 }
+
+/// An extension to [`Option`] to allow turning `None` case to an Error from a
+/// static string (handy for WASM).
+pub trait OptionExt<T> {
+    /// Transforms the [`Option<T>`] into a [`Result<T>`], mapping
+    /// [`Some(v)`] to [`Ok(v)`] and [`None`] to the given static error
+    /// message.
+    fn ok_or_err_msg(self, msg: &'static str) -> Result<T>;
+}
+
+impl<T> OptionExt<T> for Option<T> {
+    fn ok_or_err_msg(self, msg: &'static str) -> Result<T> {
+        self.ok_or_else(|| Error::new_const(msg))
+    }
+}

shared/src/ledger/storage_api/mod.rs

@@ -4,7 +4,7 @@
 mod error;
 
 use borsh::{BorshDeserialize, BorshSerialize};
-pub use error::{CustomError, Error, Result, ResultExt};
+pub use error::{CustomError, Error, OptionExt, Result, ResultExt};
 
 use crate::types::storage::{self, BlockHash, BlockHeight, Epoch};