[fix] address comments

add distinct error type for `try_from` for `SignedTransaction`
change `GenesisNetwork` to have single transaction instead of a Vec
some renamings

Signed-off-by: VAmuzing <amuzik95@gmail.com>

client/benches/torii.rs

@@ -12,7 +12,10 @@ use iroha_client::{
 use iroha_genesis::{GenesisNetwork, RawGenesisBlockBuilder};
 use iroha_primitives::unique_vec;
 use iroha_version::Encode;
-use test_network::{get_chain_id, get_genesis_encoded_signature, get_key_pair, Peer as TestPeer, PeerBuilder, TestRuntime};
+use test_network::{
+    get_chain_id, get_genesis_encoded_signature, get_key_pair, Peer as TestPeer, PeerBuilder,
+    TestRuntime,
+};
 use tokio::runtime::Runtime;
 
 const MINIMUM_SUCCESS_REQUEST_RATIO: f32 = 0.9;

client/examples/million_accounts_genesis.rs

@@ -8,7 +8,8 @@ use iroha_data_model::isi::InstructionBox;
 use iroha_genesis::{GenesisNetwork, RawGenesisBlock, RawGenesisBlockBuilder};
 use iroha_primitives::unique_vec;
 use test_network::{
-    get_chain_id, get_genesis_encoded_signature, get_key_pair, wait_for_genesis_committed, Peer as TestPeer, PeerBuilder, TestRuntime
+    get_chain_id, get_genesis_encoded_signature, get_key_pair, wait_for_genesis_committed,
+    Peer as TestPeer, PeerBuilder, TestRuntime,
 };
 use tokio::runtime::Runtime;
 

config/src/parameters/user/boilerplate.rs

@@ -232,13 +232,9 @@ impl FromEnv for GenesisPartial {
         let file =
             ParseEnvResult::parse_simple(&mut emitter, env, "GENESIS_FILE", "genesis.file").into();
 
-        let signature = ParseEnvResult::parse_simple(
-            &mut emitter,
-            env,
-            "SIGNATURE",
-            "genesis.signature",
-        )
-        .into();
+        let signature =
+            ParseEnvResult::parse_simple(&mut emitter, env, "SIGNATURE", "genesis.signature")
+                .into();
 
         emitter.finish()?;
 

configs/peer.template.toml

@@ -16,6 +16,7 @@
 [genesis]
 # file =
 # public_key =
+# signature = 
 
 [network]
 # address =

core/src/sumeragi/main_loop.rs

@@ -262,12 +262,10 @@ impl Sumeragi {
             assert_eq!(state_view.latest_block_hash(), None);
         }
 
-        let transactions: Vec<_> = genesis_network
-            .into_transactions()
-            .into_iter()
-            .map(|tx| AcceptedTransaction::accept_genesis(tx, &self.chain_id))
-            .collect::<Result<_, _>>()
-            .expect("Genesis invalid");
+        let genesis_accepted_tx =
+            AcceptedTransaction::accept_genesis(genesis_network.into_transaction(), &self.chain_id)
+                .expect("Genesis invalid");
+        let transactions = Vec::from([genesis_accepted_tx]);
 
         let mut state_block = state.block();
         let genesis = BlockBuilder::new(transactions, self.current_topology.clone(), vec![])

data_model/src/transaction.rs

@@ -320,22 +320,31 @@ impl SignedTransaction {
     }
 }
 
+/// Different errors as a result of payload and signatures verification for `SignedTransaction`
+#[derive(Copy, Clone, Debug, displaydoc::Display)]
+pub enum SignedTransactionParsingError {
+    ///  There are no instructions in the transaction
+    EmptyTransaction,
+    ///  The signature does not match with the given payload
+    InvalidSignature,
+}
+
 #[cfg(feature = "transparent_api")]
 impl TryFrom<(SignaturesOf<TransactionPayload>, TransactionPayload)> for SignedTransaction {
-    type Error = &'static str;
+    type Error = SignedTransactionParsingError;
 
     fn try_from(
         value: (SignaturesOf<TransactionPayload>, TransactionPayload),
     ) -> Result<Self, Self::Error> {
         let (signatures, payload) = value;
         if let Executable::Instructions(isi) = &payload.instructions {
             if isi.is_empty() {
-                return Err("Transaction is empty");
+                return Err(SignedTransactionParsingError::EmptyTransaction);
             }
         }
         signatures
             .verify(&payload)
-            .map_err(|_| "Transaction contains invalid signatures")?;
+            .map_err(|_| SignedTransactionParsingError::InvalidSignature)?;
         Ok(SignedTransaction::V1(SignedTransactionV1 {
             signatures,
             payload,

genesis/src/lib.rs

@@ -4,7 +4,8 @@ use std::{
     fmt::Debug,
     fs::{self, File},
     io::BufReader,
-    path::{Path, PathBuf}, time::Duration,
+    path::{Path, PathBuf},
+    time::Duration,
 };
 
 use eyre::{eyre, Report, Result, WrapErr};
@@ -28,7 +29,7 @@ pub static GENESIS_ACCOUNT_ID: Lazy<AccountId> =
     Lazy::new(|| AccountId::new(GENESIS_DOMAIN_ID.clone(), "genesis".parse().expect("Valid")));
 
 /// Genesis transaction
-#[derive(Debug, Clone, Decode, Encode, Deserialize, Serialize)]
+#[derive(Debug, Clone, Decode, Encode)]
 #[repr(transparent)]
 pub struct GenesisTransaction(pub SignedTransaction);
 
@@ -81,11 +82,10 @@ impl GenesisSignature {
 }
 
 /// [`GenesisNetwork`] contains initial transactions and genesis setup related parameters.
-#[derive(Debug, Clone, Decode, Encode, Deserialize, Serialize)]
+#[derive(Debug, Clone, Decode, Encode)]
 pub struct GenesisNetwork {
-    /// Transactions from [`RawGenesisBlock`]. This vector is guaranteed to be non-empty,
-    /// unless [`GenesisNetwork::transactions_mut()`] is used.
-    transactions: Vec<GenesisTransaction>,
+    /// Transactions from [`RawGenesisBlock`] packed into a single one.
+    transaction: GenesisTransaction,
 }
 
 impl GenesisNetwork {
@@ -95,19 +95,12 @@ impl GenesisNetwork {
         chain_id: &ChainId,
         genesis_key_pair: &KeyPair,
     ) -> GenesisNetwork {
-        // The first instruction should be Executor upgrade.
-        // This makes it possible to grant permissions to users in genesis.
-        let transactions_iter = std::iter::once(GenesisTransactionBuilder {
-            isi: vec![Upgrade::new(raw_block.executor).into()],
-        })
-        .chain(raw_block.transactions);
+        let unified_transaction =
+            GenesisTransaction::new_unified(raw_block, chain_id, genesis_key_pair);
 
-        let transactions = transactions_iter
-            .map(|raw_transaction| raw_transaction.sign(chain_id.clone(), genesis_key_pair))
-            .map(GenesisTransaction)
-            .collect();
-
-        GenesisNetwork { transactions }
+        GenesisNetwork {
+            transaction: unified_transaction,
+        }
     }
 
     /// Construct `GenesisSignature` from config
@@ -129,7 +122,10 @@ impl GenesisNetwork {
     /// Checks that [`SignedGenesisConfig`] corresponds to [`RawGenesisBlock`] and produces [`GenesisNetwork`] if it does.
     /// # Errors
     /// Fails if [`RawGenesisBlock`] does not correspond to [`SignedGenesisConfig`] and it was unable to verify it's integrity
-    pub fn try_parse(genesis_block: RawGenesisBlock, signature: GenesisSignature) -> Result<GenesisNetwork> {
+    pub fn try_parse(
+        genesis_block: RawGenesisBlock,
+        signature: GenesisSignature,
+    ) -> Result<GenesisNetwork> {
         let payload = TransactionPayload {
             chain_id: signature.chain_id,
             creation_time_ms: signature.creation_time_ms.as_millis().try_into()?,
@@ -140,16 +136,17 @@ impl GenesisNetwork {
             metadata: UnlimitedMetadata::new(),
         };
 
-        let tmp = SignedTransaction::try_from((signature.signatures, payload))
-            .map_err(|_| eyre!("Failed to"))?;
+        let genesis_signed_transaction =
+            SignedTransaction::try_from((signature.signatures, payload))
+                .map_err(|e| eyre!("{}", e))?;
         Ok(GenesisNetwork {
-            transactions: Vec::from([GenesisTransaction(tmp)]),
+            transaction: GenesisTransaction(genesis_signed_transaction),
         })
     }
 
     /// Transform into genesis transactions
-    pub fn into_transactions(self) -> Vec<GenesisTransaction> {
-        self.transactions
+    pub fn into_transaction(self) -> GenesisTransaction {
+        self.transaction
     }
 }
 
@@ -174,6 +171,7 @@ impl RawGenesisBlock {
     }
 
     fn unify(self) -> GenesisTransactionBuilder {
+        // Putting executor upgrade as a first instruction
         let mut unified_tx = GenesisTransactionBuilder {
             isi: vec![Upgrade::new(self.executor).into()],
         };

tools/kagami/Cargo.toml

@@ -26,3 +26,6 @@ serde_json = { workspace = true }
 derive_more = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 parity-scale-codec = { workspace = true }
+
+[dev-dependencies]
+tempfile = { workspace = true }

tools/kagami/src/crypto.rs

@@ -11,26 +11,26 @@ use super::*;
 pub struct Args {
     /// An algorithm to use for the key-pair generation
     #[clap(default_value_t, long, short)]
-    algorithm: AlgorithmArg,
+    pub algorithm: AlgorithmArg,
     /// A private key to generate the key-pair from
     ///
     /// `--private-key` specifies the payload of the private key, while `--algorithm`
     /// specifies its algorithm.
     #[clap(long, short, group = "generate_from")]
-    private_key: Option<String>,
+    pub private_key: Option<String>,
     /// The Unicode `seed` string to generate the key-pair from
     #[clap(long, short, group = "generate_from")]
-    seed: Option<String>,
+    pub seed: Option<String>,
     /// Output the key-pair in JSON format
     #[clap(long, short, group = "format")]
-    json: bool,
+    pub json: bool,
     /// Output the key-pair without additional text
     #[clap(long, short, group = "format")]
-    compact: bool,
+    pub compact: bool,
 }
 
 #[derive(Clone, Debug, Default, derive_more::Display)]
-pub(crate) struct AlgorithmArg(pub(crate) Algorithm);
+pub struct AlgorithmArg(pub Algorithm);
 
 impl ValueEnum for AlgorithmArg {
     fn value_variants<'a>() -> &'a [Self] {

tools/kagami/src/genesis.rs

@@ -22,16 +22,13 @@ use iroha_data_model::{
     ChainId,
 };
 use iroha_genesis::{
-    executor_state, GenesisNetwork, RawGenesisBlock,
-    RawGenesisBlockBuilder, RawGenesisBlockFile,
+    executor_state, GenesisNetwork, RawGenesisBlock, RawGenesisBlockBuilder, RawGenesisBlockFile,
 };
 use parity_scale_codec::Encode;
 use serde_json::json;
 
 use super::*;
 
-const ED25519_PREFIX: &str = "ed0120";
-
 #[derive(Subcommand, Debug, Clone)]
 pub enum Args {
     Generate(GenerateArgs),
@@ -367,23 +364,13 @@ impl SignArgs {
                     )
                 })
             }
-            KeyStorage::FromCLI(hex) => {
-                if hex.starts_with(ED25519_PREFIX) {
-                    PublicKey::from_str(hex).wrap_err_with(|| {
-                        eyre!(
-                            "Failed to deserialize public key from hex for algorithm `{}`",
-                            self.algorithm
-                        )
-                    })
-                } else {
-                    PublicKey::from_hex(self.algorithm.0, hex).wrap_err_with(|| {
-                        eyre!(
-                            "Failed to parse public key from hex for algorithm `{}`",
-                            self.algorithm
-                        )
-                    })
-                }
-            }
+            KeyStorage::FromCLI(multihash_string) => PublicKey::from_str(multihash_string)
+                .wrap_err_with(|| {
+                    eyre!(
+                        "Failed to deserialize public key from multihash string for algorithm `{}`",
+                        self.algorithm
+                    )
+                }),
         }
     }
 }
@@ -441,11 +428,15 @@ impl<T: Write> RunArgs<T> for SignArgs {
 
 #[cfg(test)]
 mod tests {
+    use iroha_genesis::{GenesisSignature, GenesisTransaction};
+
+    use super::*;
+    use crate::crypto::AlgorithmArg;
+
     const GENESIS_JSON_PATH: &str = "../../configs/swarm/genesis.json";
-    const GEN_KEYPAIR_JSON_PATH: &str = "test_keypair_path_for_crypt0_genesis_kagami.json";
 
     fn genesis_signing_works() -> Result<bool> {
-        let keypair_config = Args {
+        let keypair_config = crypto::Args {
             algorithm: AlgorithmArg::default(),
             private_key: None,
             seed: None,
@@ -457,16 +448,17 @@ mod tests {
         keypair_config.run(&mut keypair_json)?;
         let keypair: KeyPair = serde_json::from_slice(keypair_json.buffer())?;
 
-        fs::write(GEN_KEYPAIR_JSON_PATH, keypair_json.buffer())?;
+        let tmp_keypair_json_file = tempfile::NamedTempFile::new()?;
+        fs::write(tmp_keypair_json_file.path(), keypair_json.buffer())?;
 
         let chain_id = ChainId::from("0123456");
-        let crypto_genesis_config = SignGenesisArgs {
+        let crypto_genesis_config = SignArgs {
             algorithm: AlgorithmArg::default(),
             private_key_string: None,
             private_key_file: None,
             public_key_string: None,
             public_key_file: None,
-            keypair_file: Some(PathBuf::from_str(GEN_KEYPAIR_JSON_PATH)?),
+            keypair_file: Some(PathBuf::from(tmp_keypair_json_file.path())),
             chain_id: chain_id.clone(),
             genesis_file: PathBuf::from_str(GENESIS_JSON_PATH)?,
             out_file: None,
@@ -489,7 +481,7 @@ mod tests {
             GenesisTransaction::new_unified(raw_genesis.clone(), &chain_id, &keypair);
         let signed_genesis_from_config =
             &GenesisNetwork::try_parse(raw_genesis, decoded_signed_genesis_config)?
-                .into_transactions()[0];
+                .into_transaction();
 
         let cmp = |a: &SignedTransaction, b: &SignedTransaction| {
             a.metadata() == b.metadata()
@@ -509,7 +501,6 @@ mod tests {
     #[test]
     fn test_genesis_signing_works() {
         let result = genesis_signing_works();
-        let _ = fs::remove_file(GEN_KEYPAIR_JSON_PATH);
         assert!(result.is_ok_and(|result| result));
     }
 }