[refactor]: move instruction len from data_model into core

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>

client/src/client.rs

@@ -21,7 +21,7 @@ use http_default::{AsyncWebSocketStream, WebSocketStream};
 use iroha_config::{client::Configuration, torii::uri, GetConfiguration, PostConfiguration};
 use iroha_crypto::{HashOf, KeyPair};
 use iroha_data_model::{
-    block::VersionedCommittedBlock, predicate::PredicateBox, prelude::*,
+    block::VersionedCommittedBlock, isi::Instruction, predicate::PredicateBox, prelude::*,
     transaction::TransactionPayload, ValidationFail,
 };
 use iroha_logger::prelude::*;

client/tests/integration/burn_public_keys.rs

@@ -2,7 +2,7 @@
 
 use iroha_client::client::{account, transaction, Client};
 use iroha_crypto::{KeyPair, PublicKey};
-use iroha_data_model::prelude::*;
+use iroha_data_model::{isi::Instruction, prelude::*};
 use test_network::*;
 
 fn submit_and_get(

client/tests/integration/events/pipeline.rs

@@ -116,7 +116,14 @@ fn committed_block_must_be_available_in_kura() {
         .expect("Failed to submit transaction");
 
     let event = event_iter.next().expect("Block must be committed");
-    let Ok(Event::Pipeline(PipelineEvent { entity_kind: PipelineEntityKind::Block, status: PipelineStatus::Committed, hash })) = event else { panic!("Received unexpected event") };
+    let Ok(Event::Pipeline(PipelineEvent {
+        entity_kind: PipelineEntityKind::Block,
+        status: PipelineStatus::Committed,
+        hash,
+    })) = event
+    else {
+        panic!("Received unexpected event")
+    };
     let hash = HashOf::from_untyped_unchecked(hash);
 
     peer.iroha

client/tests/integration/query_errors.rs

@@ -24,7 +24,7 @@ fn non_existent_account_is_specific_error() {
     match err {
         ClientQueryError::Validation(ValidationFail::QueryFailed(QueryExecutionFail::Find(
             err,
-        ))) => match *err {
+        ))) => match err {
             FindError::Domain(id) => assert_eq!(id.name.as_ref(), "regalia"),
             x => panic!("FindError::Domain expected, got {x:?}"),
         },

client/tests/integration/triggers/by_call_trigger.rs

@@ -200,8 +200,7 @@ fn trigger_should_not_be_executed_with_zero_repeats_count() -> Result<()> {
             .chain()
             .last()
             .expect("At least two error causes expected")
-            .downcast_ref::<Box<FindError>>()
-            .map(std::ops::Deref::deref),
+            .downcast_ref::<FindError>(),
         Some(FindError::Trigger(id)) if *id == trigger_id
     ));
 

core/src/queue.rs

@@ -394,8 +394,9 @@ impl Queue {
     }
 
     fn decrease_per_user_tx_count(&self, account_id: &AccountId) {
-        let Entry::Occupied(mut occupied) = self.txs_per_user
-            .entry(account_id.clone()) else { panic!("Call to decrease always should be paired with increase count. This is a bug.") };
+        let Entry::Occupied(mut occupied) = self.txs_per_user.entry(account_id.clone()) else {
+            panic!("Call to decrease always should be paired with increase count. This is a bug.")
+        };
 
         let count = occupied.get_mut();
         if *count > 1 {

core/src/smartcontracts/isi/account.rs

@@ -50,7 +50,7 @@ pub mod isi {
             match wsv.asset(&asset_id) {
                 Err(err) => match err {
                     QueryExecutionFail::Find(find_err)
-                        if matches!(*find_err, FindError::Asset(_)) =>
+                        if matches!(find_err, FindError::Asset(_)) =>
                     {
                         assert_can_register(&asset_id.definition_id, wsv, &self.object.value)?;
                         let asset = wsv

core/src/smartcontracts/isi/asset.rs

@@ -619,7 +619,7 @@ pub mod query {
                 .asset(&id)
                 .map_err(|asset_err| {
                     if let Err(definition_err) = wsv.asset_definition(&id.definition_id) {
-                        Error::Find(Box::new(definition_err))
+                        Error::Find(definition_err)
                     } else {
                         asset_err
                     }
@@ -657,7 +657,7 @@ pub mod query {
                 .map_err(|e| Error::Evaluate(e.to_string()))?;
             let asset = wsv.asset(&id).map_err(|asset_err| {
                 if let Err(definition_err) = wsv.asset_definition(&id.definition_id) {
-                    Error::Find(Box::new(definition_err))
+                    Error::Find(definition_err)
                 } else {
                     asset_err
                 }
@@ -670,7 +670,7 @@ pub mod query {
                 .map_err(|e| Error::Conversion(e.to_string()))?;
             Ok(store
                 .get(&key)
-                .ok_or_else(|| Error::Find(Box::new(FindError::MetadataKey(key))))?
+                .ok_or_else(|| Error::Find(FindError::MetadataKey(key)))?
                 .clone())
         }
     }

core/src/smartcontracts/isi/block.rs

@@ -47,7 +47,7 @@ impl ValidQuery for FindBlockHeaderByHash {
         let block = wsv
             .all_blocks()
             .find(|block| block.hash() == hash)
-            .ok_or_else(|| QueryExecutionFail::Find(Box::new(FindError::Block(hash))))?;
+            .ok_or_else(|| QueryExecutionFail::Find(FindError::Block(hash)))?;
 
         Ok(block.as_v1().header.clone())
     }

core/src/smartcontracts/isi/mod.rs

@@ -443,7 +443,7 @@ pub mod prelude {
 mod tests {
     #![allow(clippy::restriction)]
 
-    use core::str::FromStr;
+    use core::str::FromStr as _;
     use std::sync::Arc;
 
     use iroha_crypto::KeyPair;

core/src/smartcontracts/isi/query.rs

@@ -100,7 +100,9 @@ mod tests {
     use std::str::FromStr as _;
 
     use iroha_crypto::{Hash, HashOf, KeyPair};
-    use iroha_data_model::{block::VersionedCommittedBlock, transaction::TransactionLimits};
+    use iroha_data_model::{
+        block::VersionedCommittedBlock, query::error::FindError, transaction::TransactionLimits,
+    };
     use once_cell::sync::Lazy;
 
     use super::*;
@@ -307,7 +309,7 @@ mod tests {
     #[test]
     fn find_block_header_by_hash() -> Result<()> {
         let wsv = wsv_with_test_blocks_and_transactions(1, 1, 1)?;
-        let block = wsv.all_blocks().into_iter().last().expect("WSV is empty");
+        let block = wsv.all_blocks().last().expect("WSV is empty");
 
         assert_eq!(
             FindBlockHeaderByHash::new(block.hash()).execute(&wsv)?,
@@ -384,7 +386,10 @@ mod tests {
             .sign(ALICE_KEYS.clone())?;
         let wrong_hash = unapplied_tx.hash();
         let not_found = FindTransactionByHash::new(wrong_hash).execute(&wsv);
-        assert!(matches!(not_found, Err(_)));
+        assert!(matches!(
+            not_found,
+            Err(Error::Find(FindError::Transaction(_)))
+        ));
 
         let found_accepted = FindTransactionByHash::new(va_tx.hash()).execute(&wsv)?;
         if found_accepted.transaction.error.is_none() {

core/src/smartcontracts/isi/triggers/mod.rs

@@ -225,7 +225,7 @@ pub mod query {
             } = wsv
                 .triggers()
                 .inspect_by_id(&id, |action| action.clone_and_box())
-                .ok_or_else(|| Error::Find(Box::new(FindError::Trigger(id.clone()))))?;
+                .ok_or_else(|| Error::Find(FindError::Trigger(id.clone())))?;
 
             let action =
                 Action::new(loaded_executable, repeats, authority, filter).with_metadata(metadata);
@@ -253,7 +253,7 @@ pub mod query {
                         .map(Clone::clone)
                         .ok_or_else(|| FindError::MetadataKey(key.clone()).into())
                 })
-                .ok_or_else(|| Error::Find(Box::new(FindError::Trigger(id))))?
+                .ok_or_else(|| Error::Find(FindError::Trigger(id)))?
         }
     }
 

core/src/smartcontracts/isi/world.rs

@@ -448,7 +448,7 @@ pub mod query {
             iroha_logger::trace!(%role_id);
 
             wsv.world.roles.get(&role_id).map_or_else(
-                || Err(Error::Find(Box::new(FindError::Role(role_id)))),
+                || Err(Error::Find(FindError::Role(role_id))),
                 |role_ref| Ok(role_ref.clone()),
             )
         }

core/src/sumeragi/message.rs

@@ -132,7 +132,6 @@ impl From<VersionedCommittedBlock> for BlockCommitted {
             hash: block.partial_hash(),
             signatures: block
                 .signatures()
-                .into_iter()
                 .cloned()
                 .collect::<std::collections::BTreeSet<SignatureOf<VersionedCommittedBlock>>>()
                 .try_into()

core/src/tx.rs

@@ -50,6 +50,34 @@ pub enum AcceptTransactionFail {
     SignatureVerification(#[source] SignatureVerificationFail<TransactionPayload>),
 }
 
+fn instruction_size(isi: &InstructionBox) -> usize {
+    use InstructionBox::*;
+
+    match isi {
+        Register(isi) => isi.object.len() + 1,
+        Unregister(isi) => isi.object_id.len() + 1,
+        Mint(isi) => isi.destination_id.len() + isi.object.len() + 1,
+        Burn(isi) => isi.destination_id.len() + isi.object.len() + 1,
+        Transfer(isi) => isi.destination_id.len() + isi.object.len() + isi.source_id.len() + 1,
+        If(isi) => {
+            let otherwise = isi.otherwise.as_ref().map_or(0, instruction_size);
+            isi.condition.len() + instruction_size(&isi.then) + otherwise + 1
+        }
+        Pair(isi) => {
+            instruction_size(&isi.left_instruction) + instruction_size(&isi.right_instruction) + 1
+        }
+        Sequence(isi) => isi.instructions.iter().map(instruction_size).sum::<usize>() + 1,
+        SetKeyValue(isi) => isi.object_id.len() + isi.key.len() + isi.value.len() + 1,
+        RemoveKeyValue(isi) => isi.object_id.len() + isi.key.len() + 1,
+        Grant(isi) => isi.object.len() + isi.destination_id.len() + 1,
+        Revoke(isi) => isi.object.len() + isi.destination_id.len() + 1,
+        SetParameter(isi) => isi.parameter.len() + 1,
+        NewParameter(isi) => isi.parameter.len() + 1,
+        Upgrade(isi) => isi.object.len() + 1,
+        Fail(_) | ExecuteTrigger(_) => 1,
+    }
+}
+
 impl AcceptedTransaction {
     /// Accept genesis transaction. Transition from [`GenesisTransaction`] to [`AcceptedTransaction`].
     pub fn accept_genesis(tx: GenesisTransaction) -> Self {
@@ -69,7 +97,7 @@ impl AcceptedTransaction {
             Executable::Instructions(instructions) => {
                 let instruction_count: u64 = instructions
                     .iter()
-                    .map(InstructionBox::len)
+                    .map(instruction_size)
                     .sum::<usize>()
                     .try_into()
                     .expect("`usize` should always fit in `u64`");
@@ -299,3 +327,67 @@ impl TransactionValidator {
             })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use core::str::FromStr as _;
+
+    use super::*;
+
+    fn if_instruction(
+        c: impl Into<Expression>,
+        then: InstructionBox,
+        otherwise: Option<InstructionBox>,
+    ) -> InstructionBox {
+        let condition: Expression = c.into();
+        let condition = EvaluatesTo::new_unchecked(condition);
+        Conditional {
+            condition,
+            then,
+            otherwise,
+        }
+        .into()
+    }
+
+    fn fail() -> InstructionBox {
+        FailBox {
+            message: String::default(),
+        }
+        .into()
+    }
+
+    #[test]
+    fn len_empty_sequence() {
+        assert_eq!(instruction_size(&SequenceBox::new(vec![]).into()), 1);
+    }
+
+    #[test]
+    fn len_if_one_branch() {
+        let instructions = vec![if_instruction(
+            ContextValue {
+                value_name: Name::from_str("a").expect("Cannot fail."),
+            },
+            fail(),
+            None,
+        )];
+
+        assert_eq!(instruction_size(&SequenceBox::new(instructions).into()), 4);
+    }
+
+    #[test]
+    fn len_sequence_if() {
+        let instructions = vec![
+            fail(),
+            if_instruction(
+                ContextValue {
+                    value_name: Name::from_str("b").expect("Cannot fail."),
+                },
+                fail(),
+                Some(fail()),
+            ),
+            fail(),
+        ];
+
+        assert_eq!(instruction_size(&SequenceBox::new(instructions).into()), 7);
+    }
+}

core/src/wsv.rs

@@ -387,7 +387,7 @@ impl WorldStateView {
         macro_rules! update_params {
             ($ident:ident, $($param:expr => $config:expr),+ $(,)?) => {
                 $(if let Some(param) = self.query_param($param) {
-                    let mut $ident = &mut self.config;
+                    let $ident = &mut self.config;
                     $config = param;
                 })+
 
@@ -474,7 +474,7 @@ impl WorldStateView {
                 account
                     .assets
                     .get(id)
-                    .ok_or_else(|| QueryExecutionFail::from(Box::new(FindError::Asset(id.clone()))))
+                    .ok_or_else(|| QueryExecutionFail::from(FindError::Asset(id.clone())))
                     .map(Clone::clone)
             },
         )?

core/test_network/src/lib.rs

@@ -21,7 +21,7 @@ use iroha_config::{
     torii::Configuration as ToriiConfiguration,
 };
 use iroha_core::prelude::*;
-use iroha_data_model::{peer::Peer as DataModelPeer, prelude::*};
+use iroha_data_model::{isi::Instruction, peer::Peer as DataModelPeer, prelude::*};
 use iroha_genesis::{GenesisNetwork, RawGenesisBlock};
 use iroha_logger::{Configuration as LoggerConfiguration, InstrumentFutures};
 use iroha_primitives::addr::{socket_addr, SocketAddr};

crypto/src/merkle.rs

@@ -288,7 +288,7 @@ mod tests {
         assert_eq!(tree.len(), 12);
         assert!(matches!(tree.get(6), Some(None)));
         assert!(matches!(tree.get(11), Some(Some(_))));
-        assert!(matches!(tree.get(12), None));
+        assert!(tree.get(12).is_none());
     }
 
     #[test]

data_model/derive/src/model.rs

@@ -15,7 +15,10 @@ pub fn impl_model(input: &syn::ItemMod) -> TokenStream {
     } = input;
 
     let syn::Visibility::Public(vis_public) = vis else {
-        abort!(input, "The `model` attribute can only be used on public modules");
+        abort!(
+            input,
+            "The `model` attribute can only be used on public modules"
+        );
     };
     if ident != "model" {
         abort!(

data_model/src/expression.rs

@@ -496,7 +496,7 @@ impl<V> EvaluatesTo<V> {
     pub fn new_unchecked(expression: impl Into<Expression>) -> Self {
         Self {
             expression: Box::new(expression.into()),
-            _value_type: PhantomData::default(),
+            _value_type: PhantomData,
         }
     }