From 25128ddda6d4cc47ee7bca6c3169ae2259bebf13 Mon Sep 17 00:00:00 2001 From: Aleksandr Date: Mon, 20 Dec 2021 13:58:07 +0400 Subject: [PATCH] [fix] #1742: Concrete errors returned in `core` instructions. Signed-off-by: Aleksandr --- core/src/smartcontracts/isi/account.rs | 49 +-- core/src/smartcontracts/isi/asset.rs | 131 ++++---- core/src/smartcontracts/isi/domain.rs | 55 ++-- core/src/smartcontracts/isi/expression.rs | 227 ++++++++++---- core/src/smartcontracts/isi/mod.rs | 361 +++++++++++++++------- core/src/smartcontracts/isi/query.rs | 32 +- core/src/smartcontracts/isi/tx.rs | 14 +- core/src/smartcontracts/isi/world.rs | 19 +- core/src/smartcontracts/mod.rs | 16 +- core/src/torii/mod.rs | 2 +- core/src/tx.rs | 49 ++- core/src/wsv.rs | 53 ++-- data_model/src/fixed.rs | 22 +- data_model/src/lib.rs | 29 +- data_model/src/metadata.rs | 73 ++++- data_model/src/transaction.rs | 4 +- 16 files changed, 760 insertions(+), 376 deletions(-) diff --git a/core/src/smartcontracts/isi/account.rs b/core/src/smartcontracts/isi/account.rs index cc8ebaedb36..24ce08bb234 100644 --- a/core/src/smartcontracts/isi/account.rs +++ b/core/src/smartcontracts/isi/account.rs @@ -164,18 +164,22 @@ pub mod isi { /// Account-related [`Query`] instructions. pub mod query { - use eyre::{eyre, Result, WrapErr}; + use eyre::{Result, WrapErr}; use iroha_logger::prelude::*; use super::{super::Evaluate, *}; - use crate::smartcontracts::isi::prelude::WorldTrait; + use crate::smartcontracts::{isi::prelude::WorldTrait, query::Error, FindError}; #[cfg(feature = "roles")] impl ValidQuery for FindRolesByAccountId { #[log] #[metrics(+"find_roles_by_account_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { - let account_id = self.id.evaluate(wsv, &Context::new())?; + fn execute(&self, wsv: &WorldStateView) -> Result { + let account_id = self + .id + .evaluate(wsv, &Context::new()) + .wrap_err("Failed to evaluate account id") + .map_err(Error::Evaluate)?; let roles = wsv.map_account(&account_id, |account| { account.roles.iter().cloned().collect::>() })?; @@ -186,8 +190,12 @@ pub mod query { impl ValidQuery for FindPermissionTokensByAccountId { #[log] #[metrics(+"find_permission_tokens_by_account_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { - let account_id = self.id.evaluate(wsv, &Context::new())?; + fn execute(&self, wsv: &WorldStateView) -> Result { + let account_id = self + .id + .evaluate(wsv, &Context::new()) + .wrap_err("Failed to evaluate account id") + .map_err(Error::Evaluate)?; let tokens = wsv.map_account(&account_id, |account| { wsv.account_permission_tokens(account) .iter() @@ -201,7 +209,7 @@ pub mod query { impl ValidQuery for FindAllAccounts { #[log] #[metrics(+"find_all_accounts")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let mut vec = Vec::new(); for domain in wsv.domains().iter() { for account in domain.accounts.values() { @@ -215,23 +223,25 @@ pub mod query { impl ValidQuery for FindAccountById { #[log] #[metrics(+"find_account_by_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get id")?; - wsv.map_account(&id, Clone::clone) + .wrap_err("Failed to get id") + .map_err(Error::Evaluate)?; + wsv.map_account(&id, Clone::clone).map_err(Into::into) } } impl ValidQuery for FindAccountsByName { #[log] #[metrics(+"find_account_by_name")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let name = self .name .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get account name")?; + .wrap_err("Failed to get account name") + .map_err(Error::Evaluate)?; let mut vec = Vec::new(); for domain in wsv.domains().iter() { for (id, account) in &domain.accounts { @@ -247,11 +257,12 @@ pub mod query { impl ValidQuery for FindAccountsByDomainId { #[log] #[metrics(+"find_accounts_by_domain_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .domain_id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get domain id")?; + .wrap_err("Failed to get domain id") + .map_err(Error::Evaluate)?; Ok(wsv .domain(&id)? .accounts @@ -264,17 +275,19 @@ pub mod query { impl ValidQuery for FindAccountKeyValueByIdAndKey { #[log] #[metrics(+"find_account_key_value_by_id_and_key")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get account id")?; + .wrap_err("Failed to get account id") + .map_err(Error::Evaluate)?; let key = self .key .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get key")?; + .wrap_err("Failed to get key") + .map_err(Error::Evaluate)?; wsv.map_account(&id, |account| account.metadata.get(&key).map(Clone::clone))? - .ok_or_else(|| eyre!("No metadata entry with this key.")) + .ok_or_else(|| query::Error::Find(Box::new(FindError::MetadataKey(key)))) } } } diff --git a/core/src/smartcontracts/isi/asset.rs b/core/src/smartcontracts/isi/asset.rs index 98f7ac98784..c8546961db7 100644 --- a/core/src/smartcontracts/isi/asset.rs +++ b/core/src/smartcontracts/isi/asset.rs @@ -12,7 +12,6 @@ use crate::prelude::*; /// - update metadata /// - transfer, etc. pub mod isi { - use eyre::eyre; use iroha_logger::prelude::*; use super::*; @@ -27,11 +26,10 @@ pub mod isi { if definition.value_type == expected_value_type { Ok(definition) } else { - Err(TypeError::from(AssetTypeError { + Err(Error::Type(TypeError::Asset(AssetTypeError { expected: expected_value_type, got: definition.value_type, - }) - .into()) + }))) } } @@ -43,7 +41,7 @@ pub mod isi { ) -> Result<(), Error> { let definition = assert_asset_type(definition_id, wsv, expected_value_type)?; if !definition.mintable { - return Err(MintabilityError::MintUnmintableError.into()); + return Err(Error::Mintability(MintabilityError::MintUnmintableError)); } Ok(()) } @@ -64,10 +62,10 @@ pub mod isi { )?; wsv.asset_or_insert(&self.destination_id, 0_u32)?; wsv.modify_asset(&self.destination_id, |asset| { - let quantity: &mut u32 = asset.try_as_mut()?; + let quantity: &mut u32 = asset.try_as_mut().map_err(Error::Conversion)?; *quantity = quantity .checked_add(self.object) - .ok_or(MathError::OverflowError)?; + .ok_or(Error::Math(MathError::Overflow))?; wsv.metrics.tx_amounts.observe(f64::from(*quantity)); Ok(()) }) @@ -79,9 +77,10 @@ pub mod isi { type Error = Error; #[metrics(+"mint_big_qty")] + #[log] fn execute( self, - _authority: ::Id, + authority: ::Id, wsv: &WorldStateView, ) -> Result<(), Error> { assert_can_mint( @@ -91,10 +90,10 @@ pub mod isi { )?; wsv.asset_or_insert(&self.destination_id, 0_u128)?; wsv.modify_asset(&self.destination_id, |asset| { - let quantity: &mut u128 = asset.try_as_mut()?; + let quantity: &mut u128 = asset.try_as_mut().map_err(Error::Conversion)?; *quantity = quantity .checked_add(self.object) - .ok_or(MathError::OverflowError)?; + .ok_or(Error::Math(MathError::Overflow))?; #[allow(clippy::cast_precision_loss)] wsv.metrics.tx_amounts.observe(*quantity as f64); Ok(()) @@ -119,7 +118,7 @@ pub mod isi { )?; wsv.asset_or_insert(&self.destination_id, Fixed::ZERO)?; wsv.modify_asset(&self.destination_id, |asset| { - let quantity: &mut Fixed = asset.try_as_mut()?; + let quantity: &mut Fixed = asset.try_as_mut().map_err(Error::Conversion)?; *quantity = quantity.checked_add(self.object)?; wsv.metrics.tx_amounts.observe((*quantity).into()); Ok(()) @@ -141,7 +140,7 @@ pub mod isi { let asset_metadata_limits = wsv.config.asset_metadata_limits; wsv.asset_or_insert(&self.object_id, Metadata::new())?; wsv.modify_asset(&self.object_id, |asset| { - let store: &mut Metadata = asset.try_as_mut()?; + let store: &mut Metadata = asset.try_as_mut().map_err(Error::Conversion)?; store.insert_with_limits( self.key.clone(), self.value.clone(), @@ -168,7 +167,7 @@ pub mod isi { AssetValueType::Quantity, )?; wsv.modify_asset(&self.destination_id, |asset| { - let quantity: &mut u32 = asset.try_as_mut()?; + let quantity: &mut u32 = asset.try_as_mut().map_err(Error::Conversion)?; *quantity = quantity .checked_sub(self.object) .ok_or(MathError::NotEnoughQuantity)?; @@ -194,7 +193,7 @@ pub mod isi { AssetValueType::BigQuantity, )?; wsv.modify_asset(&self.destination_id, |asset| { - let quantity: &mut u128 = asset.try_as_mut()?; + let quantity: &mut u128 = asset.try_as_mut().map_err(Error::Conversion)?; *quantity = quantity .checked_sub(self.object) .ok_or(MathError::NotEnoughQuantity)?; @@ -221,7 +220,7 @@ pub mod isi { AssetValueType::Fixed, )?; wsv.modify_asset(&self.destination_id, |asset| { - let quantity: &mut Fixed = asset.try_as_mut()?; + let quantity: &mut Fixed = asset.try_as_mut().map_err(Error::Conversion)?; *quantity = quantity.checked_sub(self.object)?; // Careful if `Fixed` stops being `Copy`. wsv.metrics.tx_amounts.observe((*quantity).into()); @@ -242,7 +241,7 @@ pub mod isi { ) -> Result<(), Error> { assert_asset_type(&self.object_id.definition_id, wsv, AssetValueType::Store)?; wsv.modify_asset(&self.object_id, |asset| { - let store: &mut Metadata = asset.try_as_mut()?; + let store: &mut Metadata = asset.try_as_mut().map_err(Error::Conversion)?; store .remove(&self.key) .ok_or_else(|| FindError::MetadataKey(self.key.clone()))?; @@ -263,7 +262,18 @@ pub mod isi { wsv: &WorldStateView, ) -> Result<(), Error> { if self.destination_id.definition_id != self.source_id.definition_id { - return Err(eyre!("Can not transfer asset between different asset types.").into()); + let expected = wsv + .asset_definition_entry(&self.destination_id.definition_id)? + .definition + .value_type; + let got = wsv + .asset_definition_entry(&self.source_id.definition_id)? + .definition + .value_type; + return Err(Error::Type(TypeError::Asset(AssetTypeError { + expected, + got, + }))); } assert_asset_type(&self.source_id.definition_id, wsv, AssetValueType::Quantity)?; assert_asset_type( @@ -272,18 +282,18 @@ pub mod isi { AssetValueType::Quantity, )?; wsv.modify_asset(&self.source_id, |asset| { - let quantity: &mut u32 = asset.try_as_mut()?; + let quantity: &mut u32 = asset.try_as_mut().map_err(Error::Conversion)?; *quantity = quantity .checked_sub(self.object) - .ok_or_else(|| eyre!("Insufficient assets at source account."))?; + .ok_or(Error::Math(MathError::NotEnoughQuantity))?; Ok(()) })?; wsv.asset_or_insert(&self.destination_id, 0_u32)?; wsv.modify_asset(&self.destination_id, |asset| { - let quantity: &mut u32 = asset.try_as_mut()?; + let quantity: &mut u32 = asset.try_as_mut().map_err(Error::Conversion)?; *quantity = quantity .checked_add(self.object) - .ok_or(MathError::OverflowError)?; + .ok_or(MathError::Overflow)?; wsv.metrics.tx_amounts.observe(f64::from(*quantity)); Ok(()) }) @@ -294,15 +304,17 @@ pub mod isi { /// Asset-related query implementations. pub mod query { - use eyre::{eyre, Result, WrapErr}; + use eyre::{Result, WrapErr}; + // use futures::TryFutureExt; use iroha_logger::prelude::*; use super::*; + use crate::smartcontracts::query::Error; impl ValidQuery for FindAllAssets { #[log] #[metrics(+"find_all_assets")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let mut vec = Vec::new(); for domain in wsv.domains().iter() { for account in domain.accounts.values() { @@ -318,7 +330,7 @@ pub mod query { impl ValidQuery for FindAllAssetsDefinitions { #[log] #[metrics(+"find_all_asset_definitions")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let mut vec = Vec::new(); for domain in wsv.domains().iter() { for asset_definition_entry in domain.asset_definitions.values() { @@ -332,28 +344,32 @@ pub mod query { impl ValidQuery for FindAssetById { #[log] #[metrics(+"find_asset_by_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get asset id")?; - wsv.asset(&id).map_err(|asset_err| { - match wsv.asset_definition_entry(&id.definition_id) { - Ok(_) => asset_err, - Err(definition_err) => definition_err, - } - }) + .wrap_err("Failed to get asset id") + .map_err(Error::Evaluate)?; + wsv.asset(&id) + .map_err( + |asset_err| match wsv.asset_definition_entry(&id.definition_id) { + Ok(_) => asset_err, + Err(definition_err) => definition_err, + }, + ) + .map_err(Into::into) } } impl ValidQuery for FindAssetsByName { #[log] #[metrics(+"find_assets_by_name")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let name = self .name .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get asset name")?; + .wrap_err("Failed to get asset name") + .map_err(Error::Evaluate)?; let mut vec = Vec::new(); for domain in wsv.domains().iter() { for account in domain.accounts.values() { @@ -371,23 +387,25 @@ pub mod query { impl ValidQuery for FindAssetsByAccountId { #[log] #[metrics(+"find_assets_by_account_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .account_id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get account id")?; - wsv.account_assets(&id) + .wrap_err("Failed to get account id") + .map_err(Error::Evaluate)?; + wsv.account_assets(&id).map_err(Into::into) } } impl ValidQuery for FindAssetsByAssetDefinitionId { #[log] #[metrics(+"find_assets_by_asset_definition_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .asset_definition_id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get asset definition id")?; + .wrap_err("Failed to get asset definition id") + .map_err(Error::Evaluate)?; let mut vec = Vec::new(); for domain in wsv.domains().iter() { for account in domain.accounts.values() { @@ -405,11 +423,12 @@ pub mod query { impl ValidQuery for FindAssetsByDomainId { #[log] #[metrics(+"find_assets_by_domain_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .domain_id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get domain id")?; + .wrap_err("Failed to get domain id") + .map_err(Error::Evaluate)?; let mut vec = Vec::new(); for account in wsv.domain(&id)?.accounts.values() { for asset in account.assets.values() { @@ -423,15 +442,17 @@ pub mod query { impl ValidQuery for FindAssetsByDomainIdAndAssetDefinitionId { #[log] #[metrics(+"find_assets_by_domain_id_and_asset_definition_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let domain_id = self .domain_id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get domain id")?; + .wrap_err("Failed to get domain id") + .map_err(Error::Evaluate)?; let asset_definition_id = self .asset_definition_id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get asset definition id")?; + .wrap_err("Failed to get asset definition id") + .map_err(Error::Evaluate)?; let domain = wsv.domain(&domain_id)?; let _definition = domain .asset_definitions @@ -454,20 +475,22 @@ pub mod query { impl ValidQuery for FindAssetQuantityById { #[log] #[metrics(+"find_asset_quantity_by_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get asset id")?; + .wrap_err("Failed to get asset id") + .map_err(Error::Evaluate)?; wsv.asset(&id) .map_err( |asset_err| match wsv.asset_definition_entry(&id.definition_id) { - Ok(_) => asset_err, - Err(definition_err) => definition_err, + Ok(_) => Error::Find(Box::new(asset_err)), + Err(definition_err) => Error::Find(Box::new(definition_err)), }, )? .value .try_as_ref() + .map_err(Error::Conversion) .map(Clone::clone) } } @@ -475,25 +498,27 @@ pub mod query { impl ValidQuery for FindAssetKeyValueByIdAndKey { #[log] #[metrics(+"find_asset_key_value_by_id_and_key")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get asset id")?; + .wrap_err("Failed to get asset id") + .map_err(Error::Evaluate)?; let key = self .key .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get key")?; + .wrap_err("Failed to get key") + .map_err(Error::Evaluate)?; let asset = wsv.asset(&id).map_err(|asset_err| { match wsv.asset_definition_entry(&id.definition_id) { Ok(_) => asset_err, Err(definition_err) => definition_err, } })?; - let store: &Metadata = asset.value.try_as_ref()?; + let store: &Metadata = asset.value.try_as_ref().map_err(Error::Conversion)?; Ok(store .get(&key) - .ok_or_else(|| eyre!("Key {} not found in asset {}", key, id))? + .ok_or_else(|| Error::Find(Box::new(FindError::MetadataKey(key))))? .clone()) } } diff --git a/core/src/smartcontracts/isi/domain.rs b/core/src/smartcontracts/isi/domain.rs index 6e83261a9ca..26d45f8d0f9 100644 --- a/core/src/smartcontracts/isi/domain.rs +++ b/core/src/smartcontracts/isi/domain.rs @@ -1,7 +1,7 @@ //! This module contains [`Domain`] structure and related implementations and trait implementations. use std::collections::btree_map::Entry; -use eyre::{eyre, Result}; +use eyre::Result; use iroha_data_model::prelude::*; use iroha_telemetry::metrics; @@ -14,6 +14,7 @@ use crate::prelude::*; /// - update metadata /// - transfer, etc. pub mod isi { + use super::*; impl Execute for Register { @@ -29,7 +30,8 @@ pub mod isi { account .id .name - .validate_len(wsv.config.ident_length_limits)?; + .validate_len(wsv.config.ident_length_limits) + .map_err(Error::Validate)?; let domain_id = account.id.domain_id.clone(); match wsv .domain_mut(&domain_id)? @@ -37,11 +39,10 @@ pub mod isi { .entry(account.id.clone()) { Entry::Occupied(_) => { - return Err(eyre!( - "Domain already contains an account with this Id: {:?}", - &account.id - ) - .into()) + return Err(Error::Repetition( + InstructionType::Register, + IdBox::AccountId(account.id), + )) } Entry::Vacant(entry) => { let _ = entry.insert(account.into()); @@ -81,7 +82,8 @@ pub mod isi { asset_definition .id .name - .validate_len(wsv.config.ident_length_limits)?; + .validate_len(wsv.config.ident_length_limits) + .map_err(Error::Validate)?; let domain_id = asset_definition.id.domain_id.clone(); let mut domain = wsv.domain_mut(&domain_id)?; match domain.asset_definitions.entry(asset_definition.id.clone()) { @@ -92,11 +94,10 @@ pub mod isi { }); } Entry::Occupied(entry) => { - return Err(eyre!( - "Asset definition already exists and was registered by {}", - entry.get().registered_by - ) - .into()) + return Err(Error::Repetition( + InstructionType::Register, + IdBox::AccountId(entry.get().registered_by.clone()), + )) } } Ok(()) @@ -226,11 +227,12 @@ pub mod query { use iroha_logger::prelude::*; use super::*; + use crate::smartcontracts::query::Error; impl ValidQuery for FindAllDomains { #[log] #[metrics(+"find_all_domains")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { Ok(wsv .domains() .iter() @@ -241,11 +243,12 @@ pub mod query { impl ValidQuery for FindDomainById { #[metrics(+"find_domain_by_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get domain id")?; + .wrap_err("Failed to get domain id") + .map_err(Error::Evaluate)?; Ok(wsv.domain(&id)?.clone()) } } @@ -253,37 +256,41 @@ pub mod query { impl ValidQuery for FindDomainKeyValueByIdAndKey { #[log] #[metrics(+"find_domain_key_value_by_id_and_key")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get domain id")?; + .wrap_err("Failed to get domain id") + .map_err(Error::Evaluate)?; let key = self .key .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get key")?; + .wrap_err("Failed to get key") + .map_err(Error::Evaluate)?; wsv.map_domain(&id, |domain| domain.metadata.get(&key).map(Clone::clone))? - .ok_or_else(|| eyre!("No metadata entry with this key.")) + .ok_or_else(|| FindError::MetadataKey(key).into()) } } impl ValidQuery for FindAssetDefinitionKeyValueByIdAndKey { #[metrics(+"find_asset_definition_key_value_by_id_and_key")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get asset definition id")?; + .wrap_err("Failed to get asset definition id") + .map_err(Error::Evaluate)?; let key = self .key .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get key")?; + .wrap_err("Failed to get key") + .map_err(Error::Evaluate)?; Ok(wsv .asset_definition_entry(&id)? .definition .metadata .get(&key) - .ok_or_else(|| eyre!("Key {} not found in asset {}", key, id))? + .ok_or(FindError::MetadataKey(key))? .clone()) } } diff --git a/core/src/smartcontracts/isi/expression.rs b/core/src/smartcontracts/isi/expression.rs index 5daae9abdf5..32e82e7cbfb 100644 --- a/core/src/smartcontracts/isi/expression.rs +++ b/core/src/smartcontracts/isi/expression.rs @@ -1,70 +1,91 @@ //! Implementations for Expression evaluation for different expressions. -use eyre::{eyre, Error, Result}; +use eyre::Result; use iroha_data_model::{ expression::{prelude::*, Expression}, prelude::*, }; -use super::Evaluate; +use super::{Error, Evaluate, FindError, MathError}; use crate::{prelude::*, wsv::WorldTrait}; -impl, V: TryFrom, W: WorldTrait> Evaluate for EvaluatesTo { +impl, V: TryFrom, W: WorldTrait> Evaluate + for EvaluatesTo +{ type Value = V; - - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { - match V::try_from(self.expression.evaluate(wsv, context)?) { - Ok(value) => Ok(value), - Err(err) => Err(err.into()), - } + type Error = Error; + + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { + let expr = self.expression.evaluate(wsv, context)?; + V::try_from(expr).map_err(|e| Error::Conversion(e.into())) } } impl Evaluate for Expression { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { use Expression::*; - match self { - Add(add) => add.evaluate(wsv, context), - Subtract(subtract) => subtract.evaluate(wsv, context), - Greater(greater) => greater.evaluate(wsv, context), - Less(less) => less.evaluate(wsv, context), - Equal(equal) => equal.evaluate(wsv, context), - Not(not) => not.evaluate(wsv, context), - And(and) => and.evaluate(wsv, context), - Or(or) => or.evaluate(wsv, context), - If(if_expression) => if_expression.evaluate(wsv, context), - Raw(value) => Ok(*value.clone()), - Query(query) => query.execute(wsv), - Contains(contains) => contains.evaluate(wsv, context), - ContainsAll(contains_all) => contains_all.evaluate(wsv, context), - ContainsAny(contains_any) => contains_any.evaluate(wsv, context), - Where(where_expression) => where_expression.evaluate(wsv, context), - ContextValue(context_value) => context_value.evaluate(wsv, context), - Multiply(multiply) => multiply.evaluate(wsv, context), - Divide(divide) => divide.evaluate(wsv, context), - Mod(modulus) => modulus.evaluate(wsv, context), - RaiseTo(raise_to) => raise_to.evaluate(wsv, context), - } + let eval_res = match self { + Add(add) => add.evaluate(wsv, context)?, + Subtract(subtract) => subtract.evaluate(wsv, context)?, + Greater(greater) => greater.evaluate(wsv, context)?, + Less(less) => less.evaluate(wsv, context)?, + Equal(equal) => equal.evaluate(wsv, context)?, + Not(not) => not.evaluate(wsv, context)?, + And(and) => and.evaluate(wsv, context)?, + Or(or) => or.evaluate(wsv, context)?, + If(if_expression) => if_expression.evaluate(wsv, context)?, + Raw(value) => *value.clone(), + Query(query) => query.execute(wsv)?, + Contains(contains) => contains.evaluate(wsv, context)?, + ContainsAll(contains_all) => contains_all.evaluate(wsv, context)?, + ContainsAny(contains_any) => contains_any.evaluate(wsv, context)?, + Where(where_expression) => where_expression.evaluate(wsv, context)?, + ContextValue(context_value) => context_value.evaluate(wsv, context)?, + Multiply(multiply) => multiply.evaluate(wsv, context)?, + Divide(divide) => divide.evaluate(wsv, context)?, + Mod(modulus) => modulus.evaluate(wsv, context)?, + RaiseTo(raise_to) => raise_to.evaluate(wsv, context)?, + }; + Ok(eval_res) } } impl Evaluate for ContextValue { type Value = Value; + type Error = Error; - fn evaluate(&self, _wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + _wsv: &WorldStateView, + context: &Context, + ) -> Result { context .get(&self.value_name) - .ok_or_else(|| eyre!("Value with name {} not found in context", self.value_name)) + .ok_or_else(|| Error::Find(Box::new(FindError::Context(self.value_name.clone())))) .map(ToOwned::to_owned) } } impl Evaluate for Add { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let left = self.left.evaluate(wsv, context)?; let right = self.right.evaluate(wsv, context)?; Ok((left + right).into()) @@ -73,8 +94,13 @@ impl Evaluate for Add { impl Evaluate for Subtract { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let left = self.left.evaluate(wsv, context)?; let right = self.right.evaluate(wsv, context)?; Ok((left - right).into()) @@ -83,8 +109,13 @@ impl Evaluate for Subtract { impl Evaluate for Greater { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let left = self.left.evaluate(wsv, context)?; let right = self.right.evaluate(wsv, context)?; Ok((left > right).into()) @@ -93,8 +124,13 @@ impl Evaluate for Greater { impl Evaluate for Less { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let left = self.left.evaluate(wsv, context)?; let right = self.right.evaluate(wsv, context)?; Ok((left < right).into()) @@ -103,8 +139,13 @@ impl Evaluate for Less { impl Evaluate for Not { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let expression = self.expression.evaluate(wsv, context)?; Ok((!expression).into()) } @@ -112,8 +153,13 @@ impl Evaluate for Not { impl Evaluate for And { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let left = self.left.evaluate(wsv, context)?; let right = self.right.evaluate(wsv, context)?; Ok((left && right).into()) @@ -122,8 +168,13 @@ impl Evaluate for And { impl Evaluate for Or { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let left = self.left.evaluate(wsv, context)?; let right = self.right.evaluate(wsv, context)?; Ok((left || right).into()) @@ -132,8 +183,13 @@ impl Evaluate for Or { impl Evaluate for IfExpression { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let condition = self.condition.evaluate(wsv, context)?; if condition { self.then_expression.evaluate(wsv, context) @@ -145,8 +201,13 @@ impl Evaluate for IfExpression { impl Evaluate for Contains { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let collection = self.collection.evaluate(wsv, context)?; let element = self.element.evaluate(wsv, context)?; Ok(collection.contains(&element).into()) @@ -154,9 +215,14 @@ impl Evaluate for Contains { } impl Evaluate for ContainsAll { + type Error = Error; type Value = Value; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let collection = self.collection.evaluate(wsv, context)?; let elements = self.elements.evaluate(wsv, context)?; Ok(elements @@ -167,9 +233,14 @@ impl Evaluate for ContainsAll { } impl Evaluate for ContainsAny { + type Error = Error; type Value = Value; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let collection = self.collection.evaluate(wsv, context)?; let elements = self.elements.evaluate(wsv, context)?; Ok(elements @@ -180,9 +251,14 @@ impl Evaluate for ContainsAny { } impl Evaluate for Equal { + type Error = Error; type Value = Value; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let left = self.left.evaluate(wsv, context)?; let right = self.right.evaluate(wsv, context)?; Ok((left == right).into()) @@ -190,10 +266,15 @@ impl Evaluate for Equal { } impl Evaluate for Where { + type Error = Error; type Value = Value; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { - let additional_context: Result = self + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { + let additional_context: Result = self .values .clone() .into_iter() @@ -216,8 +297,13 @@ impl Evaluate for Where { impl Evaluate for Multiply { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let left = self.left.evaluate(wsv, context)?; let right = self.right.evaluate(wsv, context)?; Ok((left * right).into()) @@ -226,8 +312,13 @@ impl Evaluate for Multiply { impl Evaluate for RaiseTo { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let left = self.left.evaluate(wsv, context)?; let right = self.right.evaluate(wsv, context)?; Ok(left.pow(right).into()) @@ -236,20 +327,30 @@ impl Evaluate for RaiseTo { impl Evaluate for Divide { type Value = Value; - - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { - let left = self.left.evaluate(wsv, context)?; - let right = self.right.evaluate(wsv, context)?; + type Error = Error; + + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { + let left: u32 = self.left.evaluate(wsv, context)?; + let right: u32 = self.right.evaluate(wsv, context)?; left.checked_div(right) - .ok_or_else(|| eyre!("Failed to divide by zero")) - .map(Into::into) + .map(Value::U32) + .ok_or(Error::Math(MathError::DivideByZero)) } } impl Evaluate for Mod { type Value = Value; + type Error = Error; - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> Result { + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result { let left = self.left.evaluate(wsv, context)?; let right = self.right.evaluate(wsv, context)?; Ok((left % right).into()) @@ -405,7 +506,8 @@ mod tests { } #[test] - fn wrong_operand_types_are_caught() { + #[allow(clippy::unnecessary_wraps)] + fn incorrect_types_are_caught() -> Result<()> { fn assert_eval(inst: &I, err_msg: &str) where I: Evaluate + Debug, @@ -413,10 +515,8 @@ mod tests { E: StdError + Eq + Default + Send + Sync + 'static, { let wsv = WorldStateView::new(World::default()); - let result: Result<_> = inst.evaluate(&wsv, &Context::new()); - let err = result.expect_err(err_msg); - let err = err.downcast_ref::().unwrap(); - assert_eq!(err, &E::default()); + let result: Result<_, _> = inst.evaluate(&wsv, &Context::new()); + let _err = result.expect_err(err_msg); } assert_eval::<_, ErrorTryFromEnum>( @@ -447,6 +547,7 @@ mod tests { &IfExpression::new(1_u32, 2_u32, 3_u32), "If condition should be bool", ); + Ok(()) } #[test] @@ -512,7 +613,7 @@ mod tests { let serialized_expression = serde_json::to_string(&expression).expect("Failed to serialize."); let deserialized_expression: ExpressionBox = - serde_json::from_str(&serialized_expression).expect("Failed to deserialize."); + serde_json::from_str(&serialized_expression).expect("Failed to de-serialise."); let wsv = WorldStateView::::new(World::new()); assert_eq!( expression diff --git a/core/src/smartcontracts/isi/mod.rs b/core/src/smartcontracts/isi/mod.rs index 078f694bb92..5fca8047e26 100644 --- a/core/src/smartcontracts/isi/mod.rs +++ b/core/src/smartcontracts/isi/mod.rs @@ -9,132 +9,261 @@ pub mod query; pub mod tx; pub mod world; -use std::{ - error::Error as StdError, - fmt::{self, Display, Formatter}, -}; - -use eyre::{eyre, Result}; +pub use error::*; +use eyre::Result; use iroha_crypto::HashOf; use iroha_data_model::{expression::prelude::*, isi::*, prelude::*}; use iroha_logger::prelude::*; -use iroha_macro::FromVariant; -use thiserror::Error; use super::{Evaluate, Execute}; use crate::{prelude::*, wsv::WorldTrait}; -/// Instruction execution error type -#[derive(Debug, FromVariant, Error)] -pub enum Error { - /// Failed to find some entity - #[error("Failed to find")] - Find(#[source] Box), - /// Failed to assert type - #[error("Failed to assert type")] - Type(#[source] TypeError), - /// Failed to assert mintability - #[error("Failed to assert mintability")] - Mintability(#[source] MintabilityError), - /// Failed due to math exception - #[error("Math error occurred")] - Math(#[source] MathError), - /// Some other error happened - #[error("Some other error happened")] - Other(#[skip_try_from] eyre::Error), -} +pub mod error { + //! Errors used in Iroha special instructions and + //! queries. Instruction execution should fail with a specific + //! error variant, as opposed to a generic (printable-only) + //! [`eyre::Report`]. If it is impossible to predict what kind of + //! error shall be raised, there are types that wrap + //! [`eyre::Report`]. + use std::{ + error::Error as StdError, + fmt::{Display, Formatter}, + }; + + use iroha_crypto::HashOf; + use iroha_data_model::{fixed::FixedPointOperationError, metadata, prelude::*}; + use thiserror::Error; + + use super::{query, VersionedCommittedBlock}; + + /// Instruction execution error type + #[derive(Debug, Error)] + pub enum Error { + /// Failed to find some entity + #[error("Failed to find")] + Find(#[source] Box), + /// Failed to assert type + #[error("Failed to assert type")] + Type(#[source] TypeError), + /// Failed to assert mintability + #[error("Mintability violation. {0}")] + Mintability(#[source] MintabilityError), + /// Failed due to math exception + #[error("Math error. {0}")] + Math(#[source] MathError), + /// Query Error + #[error("Query failed. {0}")] + Query(#[source] query::Error), + /// Metadata Error. + #[error("Metadata error: {0}")] + Metadata(#[source] metadata::Error), + /// Unsupported instruction. + #[error("Unsupported {0} instruction")] + Unsupported(InstructionType), + /// [`FailBox`] error + #[error("Execution failed {0}")] + FailBox(std::string::String), + /// Conversion Error + #[error("Conversion Error: {0}")] + Conversion(#[source] eyre::Error), + /// Repeated instruction + #[error("Repetition")] + Repetition(InstructionType, IdBox), + /// Failed to validate. + #[error("Failed to validate.")] + Validate(#[source] eyre::Report), + } -/// Type assertion error -#[derive(Debug, Clone, Error)] -pub enum FindError { - /// Failed to find asset - #[error("Failed to find asset: `{0}`")] - Asset(AssetId), - /// Failed to find asset definition - #[error("Failed to find asset definition: `{0}`")] - AssetDefinition(AssetDefinitionId), - /// Failed to find account - #[error("Failed to find account: `{0}`")] - Account(AccountId), - /// Failed to find domain - #[error("Failed to find domain: `{0}`")] - Domain(DomainId), - /// Failed to find metadata key - #[error("Failed to find metadata key")] - MetadataKey(Name), - /// Failed to find Role by id. - #[cfg(feature = "roles")] - #[error("Failed to find role by id: `{0}`")] - Role(RoleId), - /// Block with supplied parent hash not found. More description in a string. - #[error("Block not found")] - Block(#[source] ParentHashNotFound), -} + // The main reason these are needed is because `FromVariant` can + // create conflicting implementations if two nodes of the tree of + // error types have the same type. For example, if query::Error + // and Error::Validate both have `eyre::Report` the + // implementations for both will clash. + impl From for Error { + fn from(err: metadata::Error) -> Self { + Self::Metadata(err) + } + } -/// Mintability logic error -#[derive(Debug, Clone, FromVariant, Error, Copy)] -pub enum MintabilityError { - /// Tried to mint an Un-mintable asset. - #[error("Minting of this asset is forbidden")] - MintUnmintableError, -} + impl From for Error { + fn from(err: FindError) -> Self { + Self::Find(Box::new(err)) + } + } + + impl From for Error { + fn from(err: query::Error) -> Self { + Self::Query(err) + } + } + + /// Enumeration of instructions which can have unsupported variants. + #[derive(Debug, Clone, Copy)] + pub enum InstructionType { + /// Mint + Mint, + /// Register. + Register, + /// Set key-value pair. + SetKV, + /// Remove key-value pair. + RemKV, + /// Grant + Grant, + /// Transfer + Transfer, + /// Burn + Burn, + /// Un-register. + Unregister, + } + + impl std::fmt::Display for InstructionType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(self, f) + } + } + + /// Type assertion error + #[derive(Debug, Clone, Error)] + pub enum FindError { + /// Failed to find asset + #[error("Failed to find asset: `{0}`")] + Asset(AssetId), + /// Failed to find asset definition + #[error("Failed to find asset definition: `{0}`")] + AssetDefinition(AssetDefinitionId), + /// Failed to find account + #[error("Failed to find account: `{0}`")] + Account(AccountId), + /// Failed to find domain + #[error("Failed to find domain: `{0}`")] + Domain(DomainId), + /// Failed to find metadata key + #[error("Failed to find metadata key")] + MetadataKey(Name), + /// Failed to find Role by id. + #[cfg(feature = "roles")] + #[error("Failed to find role by id: `{0}`")] + Role(RoleId), + /// Block with supplied parent hash not found. More description in a string. + #[error("Block not found")] + Block(#[source] ParentHashNotFound), + /// Transaction with given hash not found. + #[error("Transaction not found")] + Transaction(HashOf), + /// Value not found in context. + #[error("Value named {0} not found in context. ")] + Context(String), + /// Peer not found. + #[error("Peer {0} not found")] + Peer(PeerId), + } + + /// Mintability logic error + #[derive(Debug, Clone, Error, Copy, PartialEq, Eq)] + pub enum MintabilityError { + /// Tried to mint an Un-mintable asset. + #[error("Minting of this asset is forbidden")] + MintUnmintableError, + } + + /// Type assertion error + #[derive(Debug, Clone, Error, Copy, PartialEq, Eq)] + pub enum TypeError { + /// Asset type assertion error + #[error("Failed to assert type of asset")] + Asset(#[source] AssetTypeError), + } + + impl From for TypeError { + fn from(err: AssetTypeError) -> Self { + Self::Asset(err) + } + } -/// Type assertion error -#[derive(Debug, Clone, FromVariant, Error, Copy)] -pub enum TypeError { /// Asset type assertion error - #[error("Failed to assert type of asset")] - Asset(#[source] AssetTypeError), -} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + pub struct AssetTypeError { + /// Expected type + pub expected: AssetValueType, + /// Type which was discovered + pub got: AssetValueType, + } -/// Asset type assertion error -#[derive(Debug, Clone, Copy)] -pub struct AssetTypeError { - /// Expected type - pub expected: AssetValueType, - /// Type which was discovered - pub got: AssetValueType, -} + impl Display for AssetTypeError { + #[allow(clippy::use_debug)] + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Asset type error: expected asset of type {:?}, but got {:?}", + self.expected, self.got + ) + } + } -impl Display for AssetTypeError { - #[allow(clippy::use_debug)] - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!( - f, - "Asset type error: expected asset of type {:?}, but got {:?}", - self.expected, self.got - ) + impl StdError for AssetTypeError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + None + } } -} -impl StdError for AssetTypeError { - fn source(&self) -> Option<&(dyn StdError + 'static)> { - None + /// Math error type inside instruction + #[derive(Debug, Clone, Error, Copy, PartialEq, Eq)] + pub enum MathError { + /// Overflow error inside instruction + #[error("Overflow occurred.")] + Overflow, + /// Not enough quantity + #[error("Not enough quantity to transfer/burn.")] + NotEnoughQuantity, + /// Divide by zero + #[error("Divide by zero")] + DivideByZero, + /// Negative Value encountered + #[error("Negative value encountered")] + NegativeValue, + /// Domain violation + #[error("Domain violation")] + DomainViolation, + /// Unknown error. No actual function should ever return this if possible. + #[error("Unknown error")] + Unknown, } -} -/// Math error type inside instruction -#[derive(Debug, Clone, FromVariant, Error, Copy)] -pub enum MathError { - /// Overflow error inside instruction - #[error("Overflow occurred.")] - OverflowError, - /// Not enough quantity - #[error("Not enough quantity to burn.")] - NotEnoughQuantity, -} + impl From for Error { + fn from(err: FixedPointOperationError) -> Self { + match err { + FixedPointOperationError::NegativeValue(_) => Self::Math(MathError::NegativeValue), + FixedPointOperationError::Conversion(e) => { + Self::Conversion(eyre::eyre!("Mathematical conversion failed. {}", e)) + } + FixedPointOperationError::Overflow => Self::Math(MathError::Overflow), + FixedPointOperationError::DivideByZero => Self::Math(MathError::DivideByZero), + FixedPointOperationError::DomainViolation => Self::Math(MathError::DomainViolation), + FixedPointOperationError::Arithmetic => Self::Math(MathError::Unknown), + } + } + } -/// Block with parent hash not found struct -#[derive(Debug, Clone, Copy)] -pub struct ParentHashNotFound(pub HashOf); + impl From for Error { + fn from(err: MathError) -> Self { + Self::Math(err) + } + } + + /// Block with parent hash not found struct + #[derive(Debug, Clone, Copy)] + pub struct ParentHashNotFound(pub HashOf); -impl Display for ParentHashNotFound { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "Block with parent hash {} not found", self.0) + impl Display for ParentHashNotFound { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "Block with parent hash {} not found", self.0) + } } -} -impl StdError for ParentHashNotFound {} + impl StdError for ParentHashNotFound {} +} impl Execute for Instruction { type Error = Error; @@ -183,7 +312,7 @@ impl Execute for RegisterBox { Register::::new(*domain).execute(authority, wsv) } IdentifiableBox::Peer(peer) => Register::::new(*peer).execute(authority, wsv), - _ => Err(eyre!("Unsupported register instruction.").into()), + _ => Err(Error::Unsupported(InstructionType::Unregister)), } } } @@ -209,7 +338,7 @@ impl Execute for UnregisterBox { Unregister::::new(domain_id).execute(authority, wsv) } IdBox::PeerId(peer_id) => Unregister::::new(peer_id).execute(authority, wsv), - _ => Err(eyre!("Unsupported unregister instruction.").into()), + _ => Err(Error::Unsupported(InstructionType::Unregister)), } } } @@ -244,7 +373,7 @@ impl Execute for MintBox { Mint::::new(condition, account_id) .execute(authority, wsv) } - _ => Err(eyre!("Unsupported mint instruction.").into()), + _ => Err(Error::Unsupported(InstructionType::Mint)), } } } @@ -272,7 +401,7 @@ impl Execute for BurnBox { (IdBox::AccountId(account_id), Value::PublicKey(public_key)) => { Burn::::new(public_key, account_id).execute(authority, wsv) } - _ => Err(eyre!("Unsupported burn instruction.").into()), + _ => Err(Error::Unsupported(InstructionType::Burn)), } } } @@ -289,12 +418,12 @@ impl Execute for TransferBox { let context = Context::new(); let source_asset_id = match self.source_id.evaluate(wsv, &context)? { IdBox::AssetId(source_asset_id) => source_asset_id, - _ => return Err(eyre!("Unsupported transfer instruction.").into()), + _ => return Err(Error::Unsupported(InstructionType::Transfer)), }; let quantity = match self.object.evaluate(wsv, &context)? { Value::U32(quantity) => quantity, - _ => return Err(eyre!("Unsupported transfer instruction.").into()), + _ => return Err(Error::Unsupported(InstructionType::Transfer)), }; match self.destination_id.evaluate(wsv, &context)? { @@ -302,7 +431,7 @@ impl Execute for TransferBox { Transfer::::new(source_asset_id, quantity, destination_asset_id) .execute(authority, wsv) } - _ => Err(eyre!("Unsupported transfer instruction.").into()), + _ => Err(Error::Unsupported(InstructionType::Transfer)), } } } @@ -334,7 +463,7 @@ impl Execute for SetKeyValueBox { IdBox::DomainId(id) => { SetKeyValue::::new(id, key, value).execute(authority, wsv) } - _ => Err(eyre!("Unsupported set key-value instruction.").into()), + _ => Err(Error::Unsupported(InstructionType::SetKV)), } } } @@ -361,7 +490,7 @@ impl Execute for RemoveKeyValueBox { IdBox::AccountId(account_id) => { RemoveKeyValue::::new(account_id, key).execute(authority, wsv) } - _ => Err(eyre!("Unsupported remove key-value instruction.").into()), + _ => Err(Error::Unsupported(InstructionType::RemKV)), } } } @@ -425,7 +554,7 @@ impl Execute for FailBox { _authority: ::Id, _wsv: &WorldStateView, ) -> Result<(), Error> { - Err(eyre!("Execution failed: {}.", self.message).into()) + Err(Error::FailBox(self.message)) } } @@ -451,7 +580,7 @@ impl Execute for GrantBox { (IdBox::AccountId(account_id), Value::Id(IdBox::RoleId(role_id))) => { Grant::::new(role_id, account_id).execute(authority, wsv) } - _ => Err(eyre!("Unsupported grant instruction.").into()), + _ => Err(Error::Unsupported(InstructionType::Grant)), } } } diff --git a/core/src/smartcontracts/isi/query.rs b/core/src/smartcontracts/isi/query.rs index cb7a11c2987..ee280bf96cb 100644 --- a/core/src/smartcontracts/isi/query.rs +++ b/core/src/smartcontracts/isi/query.rs @@ -15,7 +15,7 @@ use warp::{ Reply, }; -use super::permissions::IsQueryAllowedBoxed; +use super::{permissions::IsQueryAllowedBoxed, FindError}; use crate::{prelude::*, WorldTrait}; /// Query Request verified on the Iroha node side. @@ -40,11 +40,9 @@ impl VerifiedQueryRequest { wsv: &WorldStateView, query_validator: &IsQueryAllowedBoxed, ) -> Result { - let account_has_public_key = wsv - .map_account(&self.payload.account_id, |account| { - account.signatories.contains(&self.signature.public_key) - }) - .map_err(Error::Find)?; + let account_has_public_key = wsv.map_account(&self.payload.account_id, |account| { + account.signatories.contains(&self.signature.public_key) + })?; if !account_has_public_key { return Err(Error::Signature(eyre!( "Signature public key doesn't correspond to the account." @@ -86,7 +84,7 @@ impl ValidQueryRequest { /// # Errors /// Forwards `self.query.execute` error. #[inline] - pub fn execute(&self, wsv: &WorldStateView) -> Result { + pub fn execute(&self, wsv: &WorldStateView) -> Result { self.query.execute(wsv) } } @@ -135,7 +133,19 @@ pub enum Error { Permission(String), /// Query found nothing. #[error("Query found nothing: {0}")] - Find(eyre::Error), + Find(#[source] Box), + /// Evaluate + #[error("Evaluation failed. {0}")] + Evaluate(#[source] eyre::Report), + /// Conversion failures + #[error("Conversion failed")] + Conversion(#[source] eyre::Report), +} + +impl From for Error { + fn from(err: FindError) -> Self { + Error::Find(Box::new(err)) + } } impl Error { @@ -143,9 +153,9 @@ impl Error { pub const fn status_code(&self) -> StatusCode { use Error::*; match *self { - Decode(_) | Version(_) => StatusCode::BAD_REQUEST, + Conversion(_) | Decode(_) | Version(_) => StatusCode::BAD_REQUEST, Signature(_) => StatusCode::UNAUTHORIZED, - Permission(_) | Find(_) => StatusCode::NOT_FOUND, + Evaluate(_) | Permission(_) | Find(_) => StatusCode::NOT_FOUND, } } } @@ -169,7 +179,7 @@ impl TryFrom<&Bytes> for VerifiedQueryRequest { } impl ValidQuery for QueryBox { - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { use QueryBox::*; match self { diff --git a/core/src/smartcontracts/isi/tx.rs b/core/src/smartcontracts/isi/tx.rs index f65a6d9a373..0e413a04d99 100644 --- a/core/src/smartcontracts/isi/tx.rs +++ b/core/src/smartcontracts/isi/tx.rs @@ -10,11 +10,12 @@ use super::*; impl ValidQuery for FindTransactionsByAccountId { #[log] #[metrics(+"find_transactions_by_account_id")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let id = self .account_id .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get id")?; + .wrap_err("Failed to get account id") + .map_err(query::Error::Evaluate)?; Ok(wsv.transactions_values_by_account_id(&id)) } } @@ -22,16 +23,17 @@ impl ValidQuery for FindTransactionsByAccountId { impl ValidQuery for FindTransactionByHash { #[log] #[metrics(+"find_transaction_by_hash")] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { let hash = self .hash .evaluate(wsv, &Context::default()) - .wrap_err("Failed to get hash")?; + .wrap_err("Failed to get hash") + .map_err(query::Error::Evaluate)?; let hash = HashOf::from_hash(hash); if !wsv.has_transaction(&hash) { - return Err(eyre!("Transaction not found")); + return Err(FindError::Transaction(hash).into()); }; wsv.transaction_value_by_hash(&hash) - .ok_or_else(|| eyre!("Failed to fetch transaction")) + .ok_or_else(|| FindError::Transaction(hash).into()) } } diff --git a/core/src/smartcontracts/isi/world.rs b/core/src/smartcontracts/isi/world.rs index 9e7f161b9ed..ba2794a9c27 100644 --- a/core/src/smartcontracts/isi/world.rs +++ b/core/src/smartcontracts/isi/world.rs @@ -5,7 +5,7 @@ use crate::prelude::*; /// Iroha Special Instructions that have `World` as their target. pub mod isi { - use eyre::{eyre, Result}; + use eyre::Result; use iroha_data_model::prelude::*; use iroha_telemetry::metrics; @@ -20,10 +20,13 @@ pub mod isi { _authority: ::Id, wsv: &WorldStateView, ) -> Result<(), Error> { - if wsv.trusted_peers_ids().insert(self.object.id) { + if wsv.trusted_peers_ids().insert(self.object.id.clone()) { Ok(()) } else { - Err(eyre!("Peer already trusted.",).into()) + Err(Error::Repetition( + InstructionType::Register, + IdBox::PeerId(self.object.id), + )) } } } @@ -40,7 +43,7 @@ pub mod isi { if wsv.trusted_peers_ids().remove(&self.object_id).is_some() { Ok(()) } else { - Err(eyre!("Peer wasn't trusted.").into()) + Err(FindError::Peer(self.object_id).into()) } } } @@ -58,7 +61,8 @@ pub mod isi { domain .id .name - .validate_len(wsv.config.ident_length_limits)?; + .validate_len(wsv.config.ident_length_limits) + .map_err(Error::Validate)?; wsv.domains().insert(domain.id.clone(), domain); wsv.metrics.domains.inc(); Ok(()) @@ -125,11 +129,12 @@ pub mod query { use iroha_logger::log; use super::*; + use crate::smartcontracts::query::Error; #[cfg(feature = "roles")] impl ValidQuery for FindAllRoles { #[log] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { Ok(wsv .world .roles @@ -141,7 +146,7 @@ pub mod query { impl ValidQuery for FindAllPeers { #[log] - fn execute(&self, wsv: &WorldStateView) -> Result { + fn execute(&self, wsv: &WorldStateView) -> Result { Ok(wsv.peers()) } } diff --git a/core/src/smartcontracts/mod.rs b/core/src/smartcontracts/mod.rs index 676e2f22321..33a353aa1e5 100644 --- a/core/src/smartcontracts/mod.rs +++ b/core/src/smartcontracts/mod.rs @@ -4,8 +4,6 @@ pub mod isi; -use std::error::Error; - use iroha_data_model::prelude::*; pub use isi::*; @@ -16,7 +14,7 @@ use crate::wsv::WorldTrait; #[allow(clippy::missing_errors_doc)] pub trait Execute { /// Error type returned by execute function - type Error: Error; + type Error: std::error::Error; /// Apply actions to `wsv` on behalf of `authority`. fn execute( @@ -31,9 +29,15 @@ pub trait Execute { pub trait Evaluate { /// The resulting type of the expression. type Value; + /// Error type returned if the evaluation fails. Typically just [`isi::error::Error`]. + type Error: std::error::Error; /// Calculates result. - fn evaluate(&self, wsv: &WorldStateView, context: &Context) -> eyre::Result; + fn evaluate( + &self, + wsv: &WorldStateView, + context: &Context, + ) -> Result; } /// This trait should be implemented for all Iroha Queries. @@ -43,10 +47,10 @@ pub trait ValidQuery: Query { /// Should not mutate [`WorldStateView`]! /// /// Returns Ok(QueryResult) if succeeded and Err(String) if failed. - fn execute(&self, wsv: &WorldStateView) -> eyre::Result; + fn execute(&self, wsv: &WorldStateView) -> eyre::Result; /// Executes query and maps it into value - fn execute_into_value(&self, wsv: &WorldStateView) -> eyre::Result { + fn execute_into_value(&self, wsv: &WorldStateView) -> eyre::Result { self.execute(wsv).map(Into::into) } } diff --git a/core/src/torii/mod.rs b/core/src/torii/mod.rs index 15cef05970c..659a0291bb9 100644 --- a/core/src/torii/mod.rs +++ b/core/src/torii/mod.rs @@ -387,7 +387,7 @@ async fn handle_queries( request: VerifiedQueryRequest, ) -> Result, query::Error> { let valid_request = request.validate(&wsv, &query_validator)?; - let result = valid_request.execute(&wsv).map_err(query::Error::Find)?; + let result = valid_request.execute(&wsv)?; let result = QueryResult(if let Value::Vec(value) = result { Value::Vec(value.into_iter().paginate(pagination).collect()) } else { diff --git a/core/src/tx.rs b/core/src/tx.rs index 24f556c4668..71c1b5fe96f 100644 --- a/core/src/tx.rs +++ b/core/src/tx.rs @@ -14,7 +14,7 @@ use crate::{ prelude::*, smartcontracts::{ permissions::{self, IsInstructionAllowedBoxed, IsQueryAllowedBoxed}, - Evaluate, Execute, + Evaluate, Execute, FindError, }, wsv::WorldTrait, }; @@ -53,21 +53,28 @@ impl VersionedAcceptedTransaction { AcceptedTransaction::from_transaction(transaction, max_instruction_number).map(Into::into) } - /// Checks if this transaction is waiting longer than specified in `transaction_time_to_live` from `QueueConfiguration` or `time_to_live_ms` of this transaction. - /// Meaning that the transaction will be expired as soon as the lesser of the specified TTLs was reached. + /// Checks if this transaction is waiting longer than specified in + /// `transaction_time_to_live` from `QueueConfiguration` or + /// `time_to_live_ms` of this transaction. Meaning that the + /// transaction will be expired as soon as the lesser of the + /// specified TTLs was reached. pub fn is_expired(&self, transaction_time_to_live: Duration) -> bool { self.as_v1().is_expired(transaction_time_to_live) } - /// If `true`, this transaction is regarded to have been tampered to have a future timestamp. + /// If `true`, this transaction is regarded as one that has been + /// tampered with. This is common practice in e.g. `https`, where + /// setting your system clock back in time will prevent you from + /// accessing any of the https web-sites. pub fn is_in_future(&self, threshold: Duration) -> bool { self.as_v1().is_in_future(threshold) } - /// Move transaction lifecycle forward by checking an ability to apply instructions to the - /// `WorldStateView`. + /// Move transaction lifecycle forward by checking an ability to + /// apply instructions to the `WorldStateView`. + /// /// # Errors - /// Fails if validation of instruction fails due to permissions or other kinds of errors. + /// Fails if validation of instruction fails (e.g. permissions mismatch). pub fn validate( self, wsv: &WorldStateView, @@ -81,9 +88,12 @@ impl VersionedAcceptedTransaction { .map_err(Into::into) } - /// Checks that the signatures of this transaction satisfy the signature condition specified in the account. + /// Checks that the signatures of this transaction satisfy the + /// signature condition specified in the account. + /// /// # Errors - /// Can fail if signature conditionon account fails or if account is not found + /// - if signature conditionon fails + /// - if account is not found pub fn check_signature_condition( &self, wsv: &WorldStateView, @@ -133,8 +143,11 @@ impl AcceptedTransaction { }) } - /// Checks if this transaction is waiting longer than specified in `transaction_time_to_live` from `QueueConfiguration` or `time_to_live_ms` of this transaction. - /// Meaning that the transaction will be expired as soon as the lesser of the specified TTLs was reached. + /// Checks if this transaction is waiting longer than specified in + /// `transaction_time_to_live` from `QueueConfiguration` or + /// `time_to_live_ms` of this transaction. Meaning that the + /// transaction will be expired as soon as the lesser of the + /// specified TTLs was reached. pub fn is_expired(&self, transaction_time_to_live: Duration) -> bool { let tx_timestamp = Duration::from_millis(self.payload.creation_time); current_time().saturating_sub(tx_timestamp) @@ -144,7 +157,8 @@ impl AcceptedTransaction { ) } - /// If `true`, this transaction is regarded to have been tampered to have a future timestamp. + /// If `true`, this transaction is regarded to have been tampered + /// to have a future timestamp. pub fn is_in_future(&self, threshold: Duration) -> bool { let tx_timestamp = Duration::from_millis(self.payload.creation_time); tx_timestamp.saturating_sub(current_time()) > threshold @@ -195,11 +209,10 @@ impl AcceptedTransaction { if !is_genesis { #[cfg(feature = "roles")] { - let instructions = permissions::unpack_if_role_grant( - instruction.clone(), - wsv, - ) - .expect("Unreachable as evalutions should have been checked previously by instruction executions."); + let instructions = permissions::unpack_if_role_grant(instruction.clone(), wsv) + .expect( + "Infallible. Evaluations have been checked by instruction execution.", + ); for isi in &instructions { is_instruction_allowed .check(&account_id, isi, wsv) @@ -265,7 +278,9 @@ impl AcceptedTransaction { account .check_signature_condition(&self.signatures) .evaluate(wsv, &Context::new()) + .map_err(|_err| FindError::Account(account_id.clone())) })? + .wrap_err("Failed to find the account") } /// Rejects transaction with the `rejection_reason`. diff --git a/core/src/wsv.rs b/core/src/wsv.rs index ee136aadc17..703cd3d8d48 100644 --- a/core/src/wsv.rs +++ b/core/src/wsv.rs @@ -22,7 +22,7 @@ use tokio::task; use crate::{ block::Chain, prelude::*, - smartcontracts::{Execute, FindError}, + smartcontracts::{isi::Error, Execute, FindError}, }; /// Sender type of the new block notification channel @@ -322,7 +322,7 @@ impl WorldStateView { pub fn domain( &self, id: &::Id, - ) -> Result> { + ) -> Result, FindError> { let domain = self .world .domains @@ -338,7 +338,7 @@ impl WorldStateView { pub fn domain_mut( &self, id: &::Id, - ) -> Result> { + ) -> Result, FindError> { let domain = self .world .domains @@ -355,7 +355,7 @@ impl WorldStateView { &self, id: &::Id, f: impl FnOnce(&Domain) -> T, - ) -> Result { + ) -> Result { let domain = self.domain(id)?; Ok(f(domain.value())) } @@ -367,8 +367,8 @@ impl WorldStateView { pub fn modify_domain( &self, id: &::Id, - f: impl FnOnce(&mut Domain) -> Result<()>, - ) -> Result<()> { + f: impl FnOnce(&mut Domain) -> Result<(), Error>, + ) -> Result<(), Error> { let mut domain = self.domain_mut(id)?; f(domain.value_mut()) } @@ -381,7 +381,7 @@ impl WorldStateView { &self, id: &::Id, f: impl FnOnce(&Account) -> T, - ) -> Result { + ) -> Result { let domain = self.domain(&id.domain_id)?; let account = domain .accounts @@ -397,8 +397,8 @@ impl WorldStateView { pub fn modify_account( &self, id: &::Id, - f: impl FnOnce(&mut Account) -> Result<()>, - ) -> Result<()> { + f: impl FnOnce(&mut Account) -> Result<(), Error>, + ) -> Result<(), Error> { let mut domain = self.domain_mut(&id.domain_id)?; let account = domain .accounts @@ -411,7 +411,10 @@ impl WorldStateView { /// /// # Errors /// Fails if account finding fails - pub fn account_assets(&self, id: &::Id) -> Result> { + pub fn account_assets( + &self, + id: &::Id, + ) -> Result, FindError> { self.map_account(id, |account| account.assets.values().cloned().collect()) } @@ -431,12 +434,12 @@ impl WorldStateView { /// /// # Errors /// Fails if there are no such asset or account - pub fn asset(&self, id: &::Id) -> Result { - self.map_account(&id.account_id, |account| -> Result { + pub fn asset(&self, id: &::Id) -> Result { + self.map_account(&id.account_id, |account| -> Result { account .assets .get(id) - .ok_or_else(|| FindError::Asset(id.clone()).into()) + .ok_or_else(|| FindError::Asset(id.clone())) .map(Clone::clone) })? } @@ -448,8 +451,8 @@ impl WorldStateView { pub fn modify_asset( &self, id: &::Id, - f: impl FnOnce(&mut Asset) -> Result<()>, - ) -> Result<()> { + f: impl FnOnce(&mut Asset) -> Result<(), Error>, + ) -> Result<(), Error> { self.modify_account(&id.account_id, |account| { let asset = account .assets @@ -471,21 +474,27 @@ impl WorldStateView { &self, id: &::Id, default_asset_value: impl Into, - ) -> Result { + ) -> Result { + // This function is strictly infallible. self.modify_account(&id.account_id, |account| { let _ = account .assets .entry(id.clone()) .or_insert_with(|| Asset::new(id.clone(), default_asset_value.into())); Ok(()) + }) + .map_err(|err| { + iroha_logger::warn!(?err); + err })?; - self.asset(id) + self.asset(id).map_err(Into::into) } /// Add new `Asset` entity. + /// /// # Errors /// Fails if there is no account for asset - pub fn add_asset(&self, asset: Asset) -> Result<()> { + pub fn add_asset(&self, asset: Asset) -> Result<(), Error> { let id = asset.id.account_id.clone(); self.modify_account(&id, move |account| { account.assets.insert(asset.id.clone(), asset); @@ -500,11 +509,11 @@ impl WorldStateView { pub fn asset_definition_entry( &self, id: &::Id, - ) -> Result { + ) -> Result { self.domain(&id.domain_id)? .asset_definitions .get(id) - .ok_or_else(|| FindError::AssetDefinition(id.clone()).into()) + .ok_or_else(|| FindError::AssetDefinition(id.clone())) .map(Clone::clone) } @@ -515,8 +524,8 @@ impl WorldStateView { pub fn modify_asset_definition_entry( &self, id: &::Id, - f: impl FnOnce(&mut AssetDefinitionEntry) -> Result<()>, - ) -> Result<()> { + f: impl FnOnce(&mut AssetDefinitionEntry) -> Result<(), Error>, + ) -> Result<(), Error> { let mut domain = self.domain_mut(&id.domain_id)?; let asset_definition_entry = domain .asset_definitions diff --git a/data_model/src/fixed.rs b/data_model/src/fixed.rs index e77251b2821..c092048c676 100644 --- a/data_model/src/fixed.rs +++ b/data_model/src/fixed.rs @@ -98,15 +98,29 @@ pub enum FixedPointOperationError { /// Conversion failed. #[error("Failed to produce fixed point number")] Conversion(#[source] ConvertError), - /// The arithmetic operation failed. - #[error("Arithmetic error")] - Arithmetic(#[source] ArithmeticError), + /// Overflow + #[error("Overflow")] + Overflow, + /// Division by zero + #[error("Division by zero")] + DivideByZero, + /// Domain violation. E.g. computing `sqrt(-1)` + #[error("Domain violation")] + DomainViolation, + /// Arithmetic + #[error("Unknown Arithmetic error")] + Arithmetic, } impl From for FixedPointOperationError { #[inline] fn from(err: ArithmeticError) -> Self { - Self::Arithmetic(err) + match err { + ArithmeticError::Overflow => Self::Overflow, + ArithmeticError::DivisionByZero => Self::DivideByZero, + ArithmeticError::DomainViolation => Self::DomainViolation, + _ => Self::Arithmetic, + } } } diff --git a/data_model/src/lib.rs b/data_model/src/lib.rs index c6431dbe130..d22677d3bc7 100644 --- a/data_model/src/lib.rs +++ b/data_model/src/lib.rs @@ -31,8 +31,9 @@ pub mod metadata; pub mod query; pub mod transaction; -/// `Name` struct represents type for Iroha Entities names, like [`Domain`](`domain::Domain`)'s name or [`Account`](`account::Account`)'s -/// name. +/// `Name` struct represents type for Iroha Entities names, like +/// [`Domain`](`domain::Domain`)'s name or +/// [`Account`](`account::Account`)'s name. #[derive( Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Decode, Encode, Deserialize, Serialize, IntoSchema, )] @@ -111,7 +112,7 @@ pub trait TryAsMut { /// The type returned in the event of a conversion error. type Error; - /// Performs the conversion. + /// Perform the conversion. fn try_as_mut(&mut self) -> Result<&mut T, Self::Error>; } @@ -121,7 +122,7 @@ pub trait TryAsRef { /// The type returned in the event of a conversion error. type Error; - /// Performs the conversion. + /// Perform the conversion. fn try_as_ref(&self) -> Result<&T, Self::Error>; } @@ -413,7 +414,7 @@ where .collect::, _>>() .wrap_err("Failed to convert to vector") } else { - Err(eyre!("Expected vector, but found something else")) + Err(eyre::eyre!("Expected vector, but found something else")) } } } @@ -1767,7 +1768,7 @@ pub mod domain { pub mod peer { //! This module contains [`Peer`] structure and related implementations and traits implementations. - use std::hash::Hash; + use std::{fmt, hash::Hash}; use dashmap::DashSet; use iroha_schema::IntoSchema; @@ -1820,6 +1821,12 @@ pub mod peer { pub public_key: PublicKey, } + impl fmt::Display for Id { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(self, f) + } + } + impl Peer { /// Construct `Peer` given `id`. #[inline] @@ -1847,7 +1854,7 @@ pub mod peer { fn from_iter>(iter: T) -> Self { iter.into_iter() .map(Into::into) - .collect::>() + .collect::>() .into() } } @@ -2111,7 +2118,11 @@ pub mod prelude { Bytes, IdBox, Identifiable, IdentifiableBox, Name, Parameter, TryAsMut, TryAsRef, Value, }; pub use crate::{ - events::prelude::*, expression::prelude::*, isi::prelude::*, metadata::prelude::*, - permissions::prelude::*, query::prelude::*, + events::prelude::*, + expression::prelude::*, + isi::prelude::*, + metadata::{self, prelude::*}, + permissions::prelude::*, + query::prelude::*, }; } diff --git a/data_model/src/metadata.rs b/data_model/src/metadata.rs index f35907cba3c..2fcfc2dbe1d 100644 --- a/data_model/src/metadata.rs +++ b/data_model/src/metadata.rs @@ -1,9 +1,9 @@ //! Metadata: key-value pairs that can be attached to accounts, //! transactions and assets. -use std::{borrow::Borrow, collections::BTreeMap}; +use std::{borrow::Borrow, collections::BTreeMap, fmt}; -use eyre::{eyre, Result}; +use eyre::Result; use iroha_schema::IntoSchema; use parity_scale_codec::{Decode, Encode}; use serde::{Deserialize, Serialize}; @@ -22,6 +22,42 @@ pub struct Limits { pub max_entry_byte_size: u32, } +impl fmt::Display for Limits { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(self, f) + } +} + +/// Metadata related errors. +#[derive(Debug, thiserror::Error, Clone)] +pub enum Error { + /// Metadata entry too big. + #[error("Metadata entry too big. {limits} - {actual}")] + EntrySize { + /// The limits that were set for this entry. + limits: Limits, + /// The actual *entry* size in bytes. + actual: usize, + }, + /// Metadata exceeds overall length limit. + #[error("Metadata exceeds overall length limit. {limits} - {actual}")] + OverallSize { + /// The limits that were set for this entry. + limits: Limits, + /// The actual *overall* size of metadata. + actual: usize, + }, + /// Empty path. + #[error("Path specification empty")] + EmptyPath(), + /// Middle path segment is missing. I.e. nothing was found at that key. + #[error("Path segment {0} not found")] + MissingSegment(Name), + /// Middle path segment is not nested metadata. I.e. something was found, but isn't an instance of [`Metadata`]. + #[error("Path segment {0} is not an instance of metadata.")] + InvalidSegment(Name), +} + impl Limits { /// Constructor. pub const fn new(max_len: u32, max_entry_byte_size: u32) -> Limits { @@ -115,23 +151,23 @@ impl Metadata { path: &Path, value: Value, limits: Limits, - ) -> Result> { + ) -> Result, Error> { if self.map.len() >= limits.max_len as usize { - return Err(eyre!( - "Metadata length limit is reached: {}", - limits.max_len - )); + return Err(Error::OverallSize { + limits, + actual: self.map.len(), + }); } - let key = path.last().ok_or_else(|| eyre!("Empty path"))?; + let key = path.last().ok_or_else(Error::EmptyPath)?; let mut layer = self; for k in path.iter().take(path.len() - 1) { layer = match layer .map .get_mut(k) - .ok_or_else(|| eyre!("No metadata for key {} in path. Path is malformed.", k))? + .ok_or_else(|| Error::MissingSegment(k.clone()))? { Value::LimitedMetadata(data) => data, - _ => return Err(eyre!("Path contains non-metadata segments at key {}.", k)), + _ => return Err(Error::InvalidSegment(k.clone())), }; } check_size_limits(key, value.clone(), limits)?; @@ -148,12 +184,12 @@ impl Metadata { key: Name, value: Value, limits: Limits, - ) -> Result> { + ) -> Result, Error> { if self.map.len() >= limits.max_len as usize && !self.map.contains_key(&key) { - return Err(eyre!( - "Metadata length limit is reached: {}", - limits.max_len - )); + return Err(Error::OverallSize { + limits, + actual: self.map.len(), + }); } check_size_limits(&key, value.clone(), limits)?; Ok(self.map.insert(key, value)) @@ -181,11 +217,14 @@ impl Metadata { } } -fn check_size_limits(key: &Name, value: Value, limits: Limits) -> Result<()> { +fn check_size_limits(key: &Name, value: Value, limits: Limits) -> Result<(), Error> { let entry_bytes: Vec = (key, value).encode(); let byte_size = entry_bytes.len(); if byte_size > limits.max_entry_byte_size as usize { - return Err(eyre!("Metadata entry exceeds maximum size. Expected less than or equal to {} bytes. Actual: {} bytes", limits.max_entry_byte_size, byte_size)); + return Err(Error::EntrySize { + limits, + actual: byte_size, + }); } Ok(()) } diff --git a/data_model/src/transaction.rs b/data_model/src/transaction.rs index fa674ab6c2d..dafcd7830cb 100644 --- a/data_model/src/transaction.rs +++ b/data_model/src/transaction.rs @@ -513,7 +513,7 @@ impl Display for InstructionExecutionFail { Register(_) => "register", Sequence(_) => "sequence", Transfer(_) => "transfer", - Unregister(_) => "unregister", + Unregister(_) => "un-register", SetKeyValue(_) => "set key-value pair", RemoveKeyValue(_) => "remove key-value pair", Grant(_) => "grant", @@ -530,7 +530,7 @@ impl StdError for InstructionExecutionFail {} /// Transaction was reject because of low authority #[derive(Debug, Clone, PartialEq, Eq, Decode, Encode, Deserialize, Serialize, IntoSchema)] pub struct NotPermittedFail { - /// Reason of failure + /// The cause of failure. pub reason: String, }