Skip to content

Commit

Permalink
[refactor] #4199: Sign by reference
Browse files Browse the repository at this point in the history
Signed-off-by: Daniil Polyakov <arjentix@gmail.com>
  • Loading branch information
Arjentix committed Jan 31, 2024
1 parent b9aba63 commit fca3e04
Show file tree
Hide file tree
Showing 25 changed files with 128 additions and 144 deletions.
14 changes: 6 additions & 8 deletions client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/burn_public_keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions client/tests/integration/domain_owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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()))?;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions client/tests/integration/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.");
Expand Down
2 changes: 1 addition & 1 deletion client/tests/integration/roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions client/tests/integration/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand Down
Binary file modified configs/peer/executor.wasm
Binary file not shown.
3 changes: 1 addition & 2 deletions core/benches/blocks/apply_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<_>, _>>()?
Expand Down
4 changes: 2 additions & 2 deletions core/benches/blocks/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion core/benches/blocks/validate_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions core/benches/kura.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
Expand Down
14 changes: 7 additions & 7 deletions core/benches/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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| {
Expand All @@ -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,
Expand All @@ -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,
)
Expand All @@ -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,
)
Expand All @@ -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,
Expand Down
Loading

0 comments on commit fca3e04

Please sign in to comment.