[refactor] #4199: Sign by reference

Signed-off-by: Daniil Polyakov <[email protected]>

client/src/client.rs

@@ -63,17 +63,17 @@ pub type QueryResult<T> = core::result::Result<T, ClientQueryError>;
 /// Trait for signing transactions
 pub trait Sign {
     /// Sign transaction with provided key pair.
-    fn sign(self, key_pair: crate::crypto::KeyPair) -> SignedTransaction;
+    fn sign(self, key_pair: &crate::crypto::KeyPair) -> SignedTransaction;
 }
 
 impl Sign for TransactionBuilder {
-    fn sign(self, key_pair: crate::crypto::KeyPair) -> SignedTransaction {
+    fn sign(self, key_pair: &crate::crypto::KeyPair) -> SignedTransaction {
         self.sign(key_pair)
     }
 }
 
 impl Sign for SignedTransaction {
-    fn sign(self, key_pair: crate::crypto::KeyPair) -> SignedTransaction {
+    fn sign(self, key_pair: &crate::crypto::KeyPair) -> SignedTransaction {
         self.sign(key_pair)
     }
 }
@@ -471,25 +471,23 @@ impl Client {
             tx_builder.set_nonce(nonce);
         };
 
-        tx_builder
-            .with_metadata(metadata)
-            .sign(self.key_pair.clone())
+        tx_builder.with_metadata(metadata).sign(&self.key_pair)
     }
 
     /// Signs transaction
     ///
     /// # Errors
     /// Fails if signature generation fails
     pub fn sign_transaction<Tx: Sign>(&self, transaction: Tx) -> SignedTransaction {
-        transaction.sign(self.key_pair.clone())
+        transaction.sign(&self.key_pair)
     }
 
     /// Signs query
     ///
     /// # Errors
     /// Fails if signature generation fails
     pub fn sign_query(&self, query: QueryBuilder) -> SignedQuery {
-        query.sign(self.key_pair.clone())
+        query.sign(&self.key_pair)
     }
 
     /// Instructions API entry point. Submits one Iroha Special Instruction to `Iroha` peers.

client/tests/integration/asset.rs

@@ -281,7 +281,7 @@ fn find_rate_and_make_exchange_isi_should_succeed() {
         let grant_asset_transfer_tx =
             TransactionBuilder::new(chain_id, asset_id.account_id().clone())
                 .with_instructions([allow_alice_to_transfer_asset])
-                .sign(owner_keypair);
+                .sign(&owner_keypair);
 
         test_client
             .submit_transaction_blocking(&grant_asset_transfer_tx)

client/tests/integration/burn_public_keys.rs

@@ -18,7 +18,7 @@ fn submit(
     let tx = if let Some((account_id, keypair)) = submitter {
         TransactionBuilder::new(chain_id, account_id)
             .with_instructions(instructions)
-            .sign(keypair)
+            .sign(&keypair)
     } else {
         let tx = client.build_transaction(instructions, UnlimitedMetadata::default());
         client.sign_transaction(tx)

client/tests/integration/domain_owner.rs

@@ -29,7 +29,7 @@ fn domain_owner_domain_permissions() -> Result<()> {
     // Asset definitions can't be registered by "bob@kingdom" by default
     let transaction = TransactionBuilder::new(chain_id.clone(), bob_id.clone())
         .with_instructions([Register::asset_definition(coin.clone())])
-        .sign(bob_keypair.clone());
+        .sign(&bob_keypair);
     let err = test_client
         .submit_transaction_blocking(&transaction)
         .expect_err("Tx should fail due to permissions");
@@ -57,7 +57,7 @@ fn domain_owner_domain_permissions() -> Result<()> {
     test_client.submit_blocking(Grant::permission(token.clone(), bob_id.clone()))?;
     let transaction = TransactionBuilder::new(chain_id, bob_id.clone())
         .with_instructions([Register::asset_definition(coin)])
-        .sign(bob_keypair);
+        .sign(&bob_keypair);
     test_client.submit_transaction_blocking(&transaction)?;
     test_client.submit_blocking(Revoke::permission(token, bob_id.clone()))?;
 
@@ -175,7 +175,7 @@ fn domain_owner_asset_definition_permissions() -> Result<()> {
     let coin = AssetDefinition::quantity(coin_id.clone());
     let transaction = TransactionBuilder::new(chain_id, bob_id.clone())
         .with_instructions([Register::asset_definition(coin)])
-        .sign(bob_keypair);
+        .sign(&bob_keypair);
     test_client.submit_transaction_blocking(&transaction)?;
 
     // check that "alice@wonderland" as owner of domain can transfer asset definitions in her domain
@@ -246,7 +246,7 @@ fn domain_owner_asset_permissions() -> Result<()> {
             Register::asset_definition(coin),
             Register::asset_definition(store),
         ])
-        .sign(bob_keypair);
+        .sign(&bob_keypair);
     test_client.submit_transaction_blocking(&transaction)?;
 
     // check that "alice@wonderland" as owner of domain can register and unregister assets in her domain

client/tests/integration/permissions.rs

@@ -99,7 +99,7 @@ fn permissions_disallow_asset_transfer() {
     );
     let transfer_tx = TransactionBuilder::new(chain_id, mouse_id)
         .with_instructions([transfer_asset])
-        .sign(mouse_keypair);
+        .sign(&mouse_keypair);
     let err = iroha_client
         .submit_transaction_blocking(&transfer_tx)
         .expect_err("Transaction was not rejected.");
@@ -150,7 +150,7 @@ fn permissions_disallow_asset_burn() {
     );
     let burn_tx = TransactionBuilder::new(chain_id, mouse_id)
         .with_instructions([burn_asset])
-        .sign(mouse_keypair);
+        .sign(&mouse_keypair);
 
     let err = iroha_client
         .submit_transaction_blocking(&burn_tx)
@@ -241,7 +241,7 @@ fn permissions_differ_not_only_by_names() {
 
     let grant_hats_access_tx = TransactionBuilder::new(chain_id.clone(), mouse_id.clone())
         .with_instructions([allow_alice_to_set_key_value_in_hats])
-        .sign(mouse_keypair.clone());
+        .sign(&mouse_keypair);
     client
         .submit_transaction_blocking(&grant_hats_access_tx)
         .expect("Failed grant permission to modify Mouse's hats");
@@ -277,7 +277,7 @@ fn permissions_differ_not_only_by_names() {
 
     let grant_shoes_access_tx = TransactionBuilder::new(chain_id, mouse_id)
         .with_instructions([allow_alice_to_set_key_value_in_shoes])
-        .sign(mouse_keypair);
+        .sign(&mouse_keypair);
 
     client
         .submit_transaction_blocking(&grant_shoes_access_tx)
@@ -328,7 +328,7 @@ fn stored_vs_granted_token_payload() -> Result<()> {
 
     let transaction = TransactionBuilder::new(chain_id, mouse_id)
         .with_instructions([allow_alice_to_set_key_value_in_mouse_asset])
-        .sign(mouse_keypair);
+        .sign(&mouse_keypair);
     iroha_client
         .submit_transaction_blocking(&transaction)
         .expect("Failed to grant permission to alice.");

client/tests/integration/roles.rs

@@ -80,7 +80,7 @@ fn register_and_grant_role_for_metadata_access() -> Result<()> {
     let grant_role = Grant::role(role_id.clone(), alice_id.clone());
     let grant_role_tx = TransactionBuilder::new(chain_id, mouse_id.clone())
         .with_instructions([grant_role])
-        .sign(mouse_key_pair);
+        .sign(&mouse_key_pair);
     test_client.submit_transaction_blocking(&grant_role_tx)?;
 
     // Alice modifies Mouse's metadata

client/tests/integration/upgrade.rs

@@ -34,7 +34,7 @@ fn executor_upgrade_should_work() -> Result<()> {
     let transfer_alice_rose = Transfer::asset_quantity(alice_rose, 1_u32, admin_rose);
     let transfer_rose_tx = TransactionBuilder::new(chain_id.clone(), admin_id.clone())
         .with_instructions([transfer_alice_rose.clone()])
-        .sign(admin_keypair.clone());
+        .sign(&admin_keypair);
     let _ = client
         .submit_transaction_blocking(&transfer_rose_tx)
         .expect_err("Should fail");
@@ -48,7 +48,7 @@ fn executor_upgrade_should_work() -> Result<()> {
     // Creating new transaction instead of cloning, because we need to update it's creation time
     let transfer_rose_tx = TransactionBuilder::new(chain_id, admin_id)
         .with_instructions([transfer_alice_rose])
-        .sign(admin_keypair);
+        .sign(&admin_keypair);
     client
         .submit_transaction_blocking(&transfer_rose_tx)
         .expect("Should succeed");

core/benches/blocks/apply_blocks.rs

@@ -40,8 +40,7 @@ impl WsvApplyBlocks {
             instructions
                 .into_iter()
                 .map(|instructions| {
-                    let block =
-                        create_block(&mut wsv, instructions, account_id.clone(), key_pair.clone());
+                    let block = create_block(&mut wsv, instructions, account_id.clone(), &key_pair);
                     wsv.apply_without_execution(&block).map(|()| block)
                 })
                 .collect::<Result<Vec<_>, _>>()?

core/benches/blocks/common.rs

@@ -25,13 +25,13 @@ pub fn create_block(
     wsv: &mut WorldStateView,
     instructions: Vec<InstructionBox>,
     account_id: AccountId,
-    key_pair: KeyPair,
+    key_pair: &KeyPair,
 ) -> CommittedBlock {
     let chain_id = ChainId::new("0");
 
     let transaction = TransactionBuilder::new(chain_id.clone(), account_id)
         .with_instructions(instructions)
-        .sign(key_pair.clone());
+        .sign(key_pair);
     let limits = wsv.transaction_executor().transaction_limits;
 
     let topology = Topology::new(UniqueVec::new());

core/benches/blocks/validate_blocks.rs

@@ -69,7 +69,7 @@ impl WsvValidateBlocks {
         assert_eq!(wsv.height(), 0);
         for (instructions, i) in instructions.into_iter().zip(1..) {
             finalized_wsv = wsv.clone();
-            let block = create_block(&mut wsv, instructions, account_id.clone(), key_pair.clone());
+            let block = create_block(&mut wsv, instructions, account_id.clone(), &key_pair);
             wsv.apply_without_execution(&block)?;
             assert_eq!(wsv.height(), i);
             assert_eq!(wsv.height(), finalized_wsv.height() + 1);

core/benches/kura.rs

@@ -32,7 +32,7 @@ async fn measure_block_size_for_n_executors(n_executors: u32) {
         AccountId::from_str("alice@wonderland").expect("checked"),
     )
     .with_instructions([transfer])
-    .sign(keypair.clone());
+    .sign(&keypair);
     let transaction_limits = TransactionLimits {
         max_instruction_number: 4096,
         max_wasm_size_bytes: 0,
@@ -53,10 +53,10 @@ async fn measure_block_size_for_n_executors(n_executors: u32) {
     let topology = Topology::new(UniqueVec::new());
     let mut block = BlockBuilder::new(vec![tx], topology, Vec::new())
         .chain(0, &mut wsv)
-        .sign(KeyPair::generate().unwrap());
+        .sign(&KeyPair::generate().unwrap());
 
     for _ in 1..n_executors {
-        block = block.sign(KeyPair::generate().unwrap());
+        block = block.sign(&KeyPair::generate().unwrap());
     }
     let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked);
     block_store.create_files_if_they_do_not_exist().unwrap();

core/benches/validation.rs

@@ -23,7 +23,7 @@ const TRANSACTION_LIMITS: TransactionLimits = TransactionLimits {
     max_wasm_size_bytes: 0,
 };
 
-fn build_test_transaction(keys: KeyPair, chain_id: ChainId) -> SignedTransaction {
+fn build_test_transaction(keys: &KeyPair, chain_id: ChainId) -> SignedTransaction {
     let domain_name = "domain";
     let domain_id = DomainId::from_str(domain_name).expect("does not panic");
     let create_domain: InstructionBox = Register::domain(Domain::new(domain_id)).into();
@@ -98,7 +98,7 @@ fn accept_transaction(criterion: &mut Criterion) {
     let chain_id = ChainId::new("0");
 
     let keys = KeyPair::generate().expect("Failed to generate keys");
-    let transaction = build_test_transaction(keys, chain_id.clone());
+    let transaction = build_test_transaction(&keys, chain_id.clone());
     let mut success_count = 0;
     let mut failures_count = 0;
     let _ = criterion.bench_function("accept", |b| {
@@ -116,14 +116,14 @@ fn sign_transaction(criterion: &mut Criterion) {
     let chain_id = ChainId::new("0");
 
     let keys = KeyPair::generate().expect("Failed to generate keys");
-    let transaction = build_test_transaction(keys, chain_id);
+    let transaction = build_test_transaction(&keys, chain_id);
     let key_pair = KeyPair::generate().expect("Failed to generate KeyPair.");
     let mut count = 0;
     let _ = criterion.bench_function("sign", |b| {
         b.iter_batched(
             || transaction.clone(),
             |transaction| {
-                let _: SignedTransaction = transaction.sign(key_pair.clone());
+                let _: SignedTransaction = transaction.sign(&key_pair);
                 count += 1;
             },
             BatchSize::SmallInput,
@@ -137,7 +137,7 @@ fn validate_transaction(criterion: &mut Criterion) {
 
     let keys = KeyPair::generate().expect("Failed to generate keys");
     let transaction = AcceptedTransaction::accept(
-        build_test_transaction(keys.clone(), chain_id.clone()),
+        build_test_transaction(&keys, chain_id.clone()),
         &chain_id,
         &TRANSACTION_LIMITS,
     )
@@ -163,7 +163,7 @@ fn sign_blocks(criterion: &mut Criterion) {
 
     let keys = KeyPair::generate().expect("Failed to generate keys");
     let transaction = AcceptedTransaction::accept(
-        build_test_transaction(keys, chain_id.clone()),
+        build_test_transaction(&keys, chain_id.clone()),
         &chain_id,
         &TRANSACTION_LIMITS,
     )
@@ -182,7 +182,7 @@ fn sign_blocks(criterion: &mut Criterion) {
         b.iter_batched(
             || block.clone(),
             |block| {
-                let _: ValidBlock = block.sign(key_pair.clone());
+                let _: ValidBlock = block.sign(&key_pair);
                 count += 1;
             },
             BatchSize::SmallInput,