[fix] #2232: Make Iroha print meaningful message when genesis has too many isi

cli/src/lib.rs

@@ -96,11 +96,15 @@ where
     /// To make `Iroha` peer work all actors should be started first.
     /// After that moment it you can start it with listening to torii events.
     ///
+    /// # Side effect
+    /// - Prints welcome message in the log
+    ///
     /// # Errors
     /// - Reading genesis from disk
     /// - Reading telemetry configs
     /// - telemetry setup
-    /// - Initialisation of [`Sumeragi`]
+    /// - Initialization of [`Sumeragi`]
+    #[allow(clippy::non_ascii_literal)]
     pub async fn new(
         args: &Arguments,
         instruction_validator: IsInstructionAllowedBoxed<K::World>,
@@ -113,6 +117,10 @@ where
         };
         config.load_environment()?;
 
+        let telemetry = iroha_logger::init(&config.logger)?;
+        iroha_logger::info!("Hyperledgerいろは2にようこそ！");
+        iroha_logger::info!("(translation) Welcome to Hyperledger Iroha 2!");
+
         let genesis = G::from_configuration(
             args.submit_genesis,
             RawGenesisBlock::from_path(&args.genesis_path)?,
@@ -127,6 +135,7 @@ where
             instruction_validator,
             query_validator,
             broker,
+            telemetry,
         )
         .await
     }
@@ -137,22 +146,20 @@ where
     /// - Reading telemetry configs
     /// - telemetry setup
     /// - Initialization of [`Sumeragi`]
-    #[allow(clippy::non_ascii_literal)]
     pub async fn with_genesis(
         genesis: Option<G>,
         config: Configuration,
         instruction_validator: IsInstructionAllowedBoxed<K::World>,
         query_validator: IsQueryAllowedBoxed<K::World>,
         broker: Broker,
+        telemetry: Option<iroha_logger::Telemetries>,
     ) -> Result<Self> {
         if !config.disable_panic_terminal_colors {
             if let Err(e) = color_eyre::install() {
-                iroha_logger::error!("Tried to install eyre_hook twice: {:?}", e);
+                let error_message = format!("{e:#}");
+                iroha_logger::error!(error = %error_message, "Tried to install eyre_hook twice",);
             }
         }
-        let telemetry = iroha_logger::init(&config.logger)?;
-        iroha_logger::info!("Hyperledgerいろは2にようこそ！");
-        iroha_logger::info!("(translation) Welcome to Hyperledger Iroha 2!");
         let listen_addr = config.torii.p2p_addr.clone();
         iroha_logger::info!(%listen_addr, "Starting peer");
         let network = IrohaNetwork::new(

cli/src/torii/tests.rs

@@ -195,7 +195,7 @@ impl From<warp::http::Response<warp::hyper::body::Bytes>> for QueryResponseBody
         if StatusCode::OK == src.status() {
             let body = VersionedQueryResult::decode_versioned(src.body())
                 .expect("The response body failed to be decoded to VersionedQueryResult even though the status is Ok 200");
-            Self::Ok(body)
+            Self::Ok(Box::new(body))
         } else {
             let body = query::Error::decode(&mut src.body().as_ref())
                 .expect("The response body failed to be decoded to query::Error even though the status is not Ok 200");
@@ -213,7 +213,7 @@ struct QueryResponseTest {
 
 #[allow(variant_size_differences)]
 enum QueryResponseBody {
-    Ok(VersionedQueryResult),
+    Ok(Box<VersionedQueryResult>),
     Err(query::Error),
 }
 

client/tests/integration/restart_peer.rs

@@ -1,10 +1,10 @@
 #![allow(clippy::restriction)]
 
-use std::{str::FromStr, thread, time::Duration};
+use std::{str::FromStr, sync::Arc, thread, time::Duration};
 
 use eyre::Result;
 use iroha_client::client::{self, Client};
-use iroha_core::prelude::*;
+use iroha_core::{genesis::GenesisNetwork, prelude::*};
 use iroha_data_model::prelude::*;
 use tempfile::TempDir;
 use test_network::{Peer as TestPeer, *};
@@ -14,7 +14,7 @@ use super::Configuration;
 
 #[test]
 fn restarted_peer_should_have_the_same_asset_amount() -> Result<()> {
-    let temp_dir = TempDir::new()?;
+    let temp_dir = Arc::new(TempDir::new()?);
 
     let mut configuration = Configuration::test();
     let mut peer = <TestPeer>::new()?;
@@ -24,7 +24,13 @@ fn restarted_peer_should_have_the_same_asset_amount() -> Result<()> {
 
     // Given
     let rt = Runtime::test();
-    rt.block_on(peer.start_with_config_permissions_dir(configuration.clone(), AllowAll, &temp_dir));
+    rt.block_on(peer.start_with_config_permissions_dir(
+        configuration.clone(),
+        GenesisNetwork::test(true),
+        AllowAll,
+        AllowAll,
+        Arc::clone(&temp_dir),
+    ));
     let mut iroha_client = Client::test(&peer.api_address, &peer.telemetry_address);
     wait_for_genesis_committed(&vec![iroha_client.clone()], 0);
 
@@ -34,7 +40,7 @@ fn restarted_peer_should_have_the_same_asset_amount() -> Result<()> {
         RegisterBox::new(AssetDefinition::quantity(asset_definition_id.clone()).build());
     iroha_client.submit(create_asset)?;
     thread::sleep(pipeline_time * 2);
-    //When
+    // When
     let quantity: u32 = 200;
     let mint_asset = MintBox::new(
         Value::U32(quantity),
@@ -46,7 +52,7 @@ fn restarted_peer_should_have_the_same_asset_amount() -> Result<()> {
     iroha_client.submit(mint_asset)?;
     thread::sleep(pipeline_time * 2);
 
-    //Then
+    // Then
     let asset = iroha_client
         .request(client::asset::by_account_id(account_id.clone()))?
         .into_iter()
@@ -60,7 +66,13 @@ fn restarted_peer_should_have_the_same_asset_amount() -> Result<()> {
     thread::sleep(Duration::from_millis(2000));
 
     let rt = Runtime::test();
-    rt.block_on(peer.start_with_config_permissions_dir(configuration, AllowAll, &temp_dir));
+    rt.block_on(peer.start_with_config_permissions_dir(
+        configuration,
+        GenesisNetwork::test(true),
+        AllowAll,
+        AllowAll,
+        temp_dir,
+    ));
 
     let account_asset = iroha_client
         .poll_request(client::asset::by_account_id(account_id), |assets| {

core/src/genesis.rs

@@ -196,7 +196,14 @@ impl GenesisNetworkTrait for GenesisNetwork {
 
                     raw_transaction.sign_and_accept(genesis_key_pair, tx_limits)
                 })
-                .filter_map(Result::ok)
+                .enumerate()
+                .filter_map(|(i, res)| {
+                    res.map_err(|error| {
+                        let error_msg = format!("{error:#}");
+                        iroha_logger::error!(error = %error_msg, "Genesis transaction #{i} failed")
+                    })
+                    .ok()
+                })
                 .collect(),
             wait_for_peers_retry_count: genesis_config.wait_for_peers_retry_count,
             wait_for_peers_retry_period_ms: genesis_config.wait_for_peers_retry_period_ms,
@@ -276,7 +283,7 @@ impl GenesisTransaction {
     /// Convert `GenesisTransaction` into `AcceptedTransaction` with signature
     ///
     /// # Errors
-    /// Fails if signing fails
+    /// Fails if signing or accepting fails
     pub fn sign_and_accept(
         &self,
         genesis_key_pair: KeyPair,

core/src/sumeragi/fault.rs

@@ -466,7 +466,9 @@ impl<G: GenesisNetworkTrait, K: KuraTrait, W: WorldTrait, F: FaultInjection>
         ctx: &mut Context<Self>,
     ) -> Result<()> {
         if transactions.is_empty() {
-            Err(eyre!("Genesis transactions set is empty."))
+            Err(eyre!(
+                "Genesis transactions set contains no valid transactions"
+            ))
         } else if genesis_topology.leader() != &self.peer_id {
             Err(eyre!(
                 "Incorrect network topology this peer should be {:?} but is {:?}",

core/src/tx.rs

@@ -439,7 +439,11 @@ mod tests {
         );
         assert_eq!(
             chain.next().unwrap().to_string(),
-            "Too many instructions in payload"
+            format!(
+                "Too many instructions in payload, max number is {}, but got {}",
+                tx_limits.max_instruction_number,
+                DEFAULT_MAX_INSTRUCTION_NUMBER + 1
+            )
         );
     }
 }

core/test_network/src/lib.rs

@@ -3,7 +3,7 @@
 #![allow(clippy::restriction, clippy::future_not_send)]
 
 use core::{fmt::Debug, str::FromStr as _, time::Duration};
-use std::{collections::HashMap, thread};
+use std::{collections::HashMap, sync::Arc, thread};
 
 use eyre::{Error, Result};
 use futures::{prelude::*, stream::FuturesUnordered};
@@ -435,11 +435,16 @@ where
     /// - Block store path not readable
     /// - [`Iroha::start_as_task`] failed or produced empty job handle.
     /// - `receiver` fails to produce a message.
+    ///
+    /// # TODO
+    /// Use *Builder* pattern (#2238)
     pub async fn start_with_config_permissions_dir(
         &mut self,
         configuration: Configuration,
-        permissions: impl Into<IsInstructionAllowedBoxed<W>> + Send + 'static,
-        temp_dir: &TempDir,
+        genesis: Option<G>,
+        instruction_validator: impl Into<IsInstructionAllowedBoxed<W>> + Send + 'static,
+        query_validator: impl Into<IsQueryAllowedBoxed<W>> + Send + 'static,
+        temp_dir: Arc<TempDir>,
     ) {
         let mut configuration = self.get_config(configuration);
         configuration
@@ -453,15 +458,20 @@ where
             telemetry_addr = %self.telemetry_address
         );
         let broker = self.broker.clone();
+        let telemetry =
+            iroha_logger::init(&configuration.logger).expect("Failed to initialize telemetry");
         let (sender, receiver) = std::sync::mpsc::sync_channel(1);
         let handle = task::spawn(
             async move {
+                // Prevent temporary directory deleting
+                let _temp_dir = Arc::clone(&temp_dir);
                 let mut iroha = <Iroha<W, G, K, S, B>>::with_genesis(
-                    G::test(true),
+                    genesis,
                     configuration,
-                    permissions.into(),
-                    AllowAll.into(),
+                    instruction_validator.into(),
+                    query_validator.into(),
                     broker,
+                    telemetry,
                 )
                 .await
                 .expect("Failed to start iroha");
@@ -473,13 +483,15 @@ where
         );
 
         self.iroha = Some(receiver.recv().unwrap());
+        time::sleep(Duration::from_millis(300)).await;
         self.shutdown = Some(handle);
     }
 
     /// Starts peer with config and permissions
     ///
     /// # Panics
     /// - [`TempDir::new`] failed.
+    /// - Initializing [telemetry](iroha_logger::init()) failed
     /// - [`Iroha::with_genesis`] failed.
     /// - Failed to send [`Iroha`] via sender.
     /// - [`Iroha::start_as_task`] failed or produced empty job handle.
@@ -490,42 +502,15 @@ where
         instruction_validator: impl Into<IsInstructionAllowedBoxed<W>> + Send + 'static,
         query_validator: impl Into<IsQueryAllowedBoxed<W>> + Send + 'static,
     ) {
-        let temp_dir = TempDir::new().expect("Failed to create temp dir.");
-        let mut configuration = self.get_config(configuration);
-        configuration
-            .kura
-            .block_store_path(temp_dir.path())
-            .expect("Guaranteed to exist");
-        let info_span = iroha_logger::info_span!(
-            "test-peer",
-            p2p_addr = %self.p2p_address,
-            api_addr = %self.api_address,
-            telemetry_addr = %self.telemetry_address
-        );
-        let broker = self.broker.clone();
-        let (sender, receiver) = std::sync::mpsc::sync_channel(1);
-        let join_handle = tokio::spawn(
-            async move {
-                let _temp_dir = temp_dir;
-                let mut iroha = <Iroha<W, G, K, S, B>>::with_genesis(
-                    genesis,
-                    configuration,
-                    instruction_validator.into(),
-                    query_validator.into(),
-                    broker,
-                )
-                .await
-                .expect("Failed to start Iroha");
-                let job_handle = iroha.start_as_task().unwrap();
-                sender.send(iroha).unwrap();
-                job_handle.await.unwrap().unwrap();
-            }
-            .instrument(info_span),
-        );
-
-        self.iroha = Some(receiver.recv().unwrap());
-        time::sleep(Duration::from_millis(300)).await;
-        self.shutdown = Some(join_handle);
+        let temp_dir = Arc::new(TempDir::new().expect("Failed to create temp dir."));
+        self.start_with_config_permissions_dir(
+            configuration,
+            genesis,
+            instruction_validator,
+            query_validator,
+            temp_dir,
+        )
+        .await;
     }
 
     /// Start peer with config

data_model/src/transaction.rs

@@ -61,20 +61,35 @@ pub trait Txn {
     ///
     /// Fails if number of instructions or wasm size exceeds maximum
     #[inline]
+    #[allow(clippy::expect_used)]
     fn check_limits(&self, limits: &TransactionLimits) -> Result<(), TransactionLimitError> {
         match &self.payload().instructions {
             Executable::Instructions(instructions) => {
-                let instruction_count: usize = instructions.iter().map(Instruction::len).sum();
-
-                if instruction_count as u64 > limits.max_instruction_number {
-                    return Err(TransactionLimitError(String::from(
-                        "Too many instructions in payload",
+                let instruction_count: u64 = instructions
+                    .iter()
+                    .map(Instruction::len)
+                    .sum::<usize>()
+                    .try_into()
+                    .expect("`usize` should always fit in `u64`");
+
+                if instruction_count > limits.max_instruction_number {
+                    return Err(TransactionLimitError(format!(
+                        "Too many instructions in payload, max number is {}, but got {}",
+                        limits.max_instruction_number, instruction_count
                     )));
                 }
             }
             Executable::Wasm(WasmSmartContract { raw_data }) => {
-                if raw_data.len() as u64 > limits.max_wasm_size_bytes {
-                    return Err(TransactionLimitError(String::from("wasm binary too large")));
+                let len: u64 = raw_data
+                    .len()
+                    .try_into()
+                    .expect("`usize` should always fit in `u64`");
+
+                if len > limits.max_wasm_size_bytes {
+                    return Err(TransactionLimitError(format!(
+                        "Wasm binary too large, max size is {}, but got {}",
+                        limits.max_wasm_size_bytes, len
+                    )));
                 }
             }
         }