Skip to content

Commit

Permalink
refactor: remove public key from transaction and query
Browse files Browse the repository at this point in the history
Signed-off-by: Marin Veršić <[email protected]>
  • Loading branch information
mversic committed Jun 4, 2024
1 parent 4483d2b commit e9a2788
Show file tree
Hide file tree
Showing 37 changed files with 466 additions and 396 deletions.
2 changes: 1 addition & 1 deletion client/examples/million_accounts_genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{thread, time::Duration};

use iroha::{
crypto::KeyPair,
data_model::{::isi::InstructionBox, prelude::*},
data_model::{isi::InstructionBox, prelude::*},
};
use iroha_genesis::{GenesisNetwork, RawGenesisBlock, RawGenesisBlockBuilder};
use iroha_primitives::unique_vec;
Expand Down
18 changes: 6 additions & 12 deletions client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,15 +450,17 @@ impl Client {
tx_builder.set_nonce(nonce);
};

tx_builder.with_metadata(metadata).sign(&self.key_pair)
tx_builder
.with_metadata(metadata)
.sign(self.key_pair.private_key())
}

/// Signs transaction
///
/// # Errors
/// Fails if signature generation fails
pub fn sign_transaction(&self, transaction: TransactionBuilder) -> SignedTransaction {
transaction.sign(&self.key_pair)
transaction.sign(self.key_pair.private_key())
}

/// Signs query
Expand Down Expand Up @@ -1634,20 +1636,12 @@ mod tests {
use http::Response;

use super::*;
use crate::data_model::{asset::Asset, query::error::QueryExecutionFail, ValidationFail};
use crate::data_model::{asset::Asset, ValidationFail};

#[test]
fn certain_errors() -> Result<()> {
let mut sut = QueryResponseHandler::<Vec<Asset>>::new(QueryRequest::dummy());
let responses = vec![
(
StatusCode::UNAUTHORIZED,
ValidationFail::QueryFailed(QueryExecutionFail::Signature(
"whatever".to_owned(),
)),
),
(StatusCode::UNPROCESSABLE_ENTITY, ValidationFail::TooComplex),
];
let responses = vec![(StatusCode::UNPROCESSABLE_ENTITY, ValidationFail::TooComplex)];
for (status_code, err) in responses {
let resp = Response::builder().status(status_code).body(err.encode())?;

Expand Down
5 changes: 2 additions & 3 deletions client/src/query_builder.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use std::fmt::Debug;

use iroha_data_model::query::{IterableQuery, QueryOutputBox};

use crate::{
client::{Client, QueryOutput, QueryResult},
data_model::query::{
predicate::PredicateBox, sorting::Sorting, FetchSize, Pagination, Query, QueryOutputBox,
predicate::PredicateBox, sorting::Sorting, FetchSize, IterableQuery, Pagination, Query,
QueryOutputBox,
},
};

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 @@ -305,7 +305,7 @@ fn find_rate_and_make_exchange_isi_should_succeed() {
let transaction =
TransactionBuilder::new(ChainId::from("0"), asset_id.account_id().clone())
.with_instructions([instruction])
.sign(&owner_key_pair);
.sign(owner_key_pair.private_key());

test_client
.submit_transaction_blocking(&transaction)
Expand Down
8 changes: 4 additions & 4 deletions client/tests/integration/domain_owner_permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,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);
.sign(bob_keypair.private_key());
let err = test_client
.submit_transaction_blocking(&transaction)
.expect_err("Tx should fail due to permissions");
Expand All @@ -52,7 +52,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.private_key());
test_client.submit_transaction_blocking(&transaction)?;
test_client.submit_blocking(Revoke::permission(token, bob_id.clone()))?;

Expand Down Expand Up @@ -148,7 +148,7 @@ fn domain_owner_asset_definition_permissions() -> Result<()> {
let coin = AssetDefinition::numeric(coin_id.clone());
let transaction = TransactionBuilder::new(chain_id, bob_id.clone())
.with_instructions([Register::asset_definition(coin)])
.sign(&bob_keypair);
.sign(bob_keypair.private_key());
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 @@ -217,7 +217,7 @@ fn domain_owner_asset_permissions() -> Result<()> {
Register::asset_definition(coin),
Register::asset_definition(store),
])
.sign(&bob_keypair);
.sign(bob_keypair.private_key());
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 @@ -102,7 +102,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.private_key());
let err = iroha
.submit_transaction_blocking(&transfer_tx)
.expect_err("Transaction was not rejected.");
Expand Down Expand Up @@ -151,7 +151,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.private_key());

let err = iroha
.submit_transaction_blocking(&burn_tx)
Expand Down Expand Up @@ -239,7 +239,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);
.sign(mouse_keypair.private_key());
client
.submit_transaction_blocking(&grant_hats_access_tx)
.expect("Failed grant permission to modify Mouse's hats");
Expand Down Expand Up @@ -275,7 +275,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.private_key());

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.private_key());
iroha
.submit_transaction_blocking(&transaction)
.expect("Failed to grant permission to alice.");
Expand Down
8 changes: 4 additions & 4 deletions client/tests/integration/roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,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_keypair);
.sign(mouse_keypair.private_key());
test_client.submit_transaction_blocking(&grant_role_tx)?;

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

let set_key_value = SetKeyValue::account(
Expand All @@ -263,7 +263,7 @@ fn grant_revoke_role_permissions() -> Result<()> {
// Alice can modify Mouse's metadata after permission token is granted to role
let grant_role_permission_tx = TransactionBuilder::new(chain_id.clone(), mouse_id.clone())
.with_instructions([grant_role_permission])
.sign(&mouse_keypair);
.sign(mouse_keypair.private_key());
test_client.submit_transaction_blocking(&grant_role_permission_tx)?;
let found_permissions = test_client
.request(FindPermissionsByAccountId::new(alice_id.clone()))?
Expand All @@ -274,7 +274,7 @@ fn grant_revoke_role_permissions() -> Result<()> {
// Alice can't modify Mouse's metadata after permission token is removed from role
let revoke_role_permission_tx = TransactionBuilder::new(chain_id.clone(), mouse_id.clone())
.with_instructions([revoke_role_permission])
.sign(&mouse_keypair);
.sign(mouse_keypair.private_key());
test_client.submit_transaction_blocking(&revoke_role_permission_tx)?;
let found_permissions = test_client
.request(FindPermissionsByAccountId::new(alice_id.clone()))?
Expand Down
6 changes: 3 additions & 3 deletions client/tests/integration/tx_chain_id.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::str::FromStr;

use iroha_client::data_model::prelude::*;
use iroha::data_model::prelude::*;
use iroha_primitives::numeric::numeric;
use test_network::*;
use test_samples::gen_account_in;
Expand Down Expand Up @@ -44,10 +44,10 @@ fn send_tx_with_different_chain_id() {
);
let asset_transfer_tx_0 = TransactionBuilder::new(chain_id_0, sender_id.clone())
.with_instructions([transfer_instruction.clone()])
.sign(&sender_keypair);
.sign(sender_keypair.private_key());
let asset_transfer_tx_1 = TransactionBuilder::new(chain_id_1, sender_id.clone())
.with_instructions([transfer_instruction])
.sign(&sender_keypair);
.sign(sender_keypair.private_key());
test_client
.submit_transaction_blocking(&asset_transfer_tx_0)
.unwrap();
Expand Down
13 changes: 5 additions & 8 deletions client/tests/integration/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use eyre::Result;
use futures_util::TryStreamExt as _;
use iroha::{
client::{self, Client, QueryResult},
crypto::KeyPair,
data_model::prelude::*,
};
use iroha_logger::info;
Expand All @@ -23,11 +22,9 @@ fn executor_upgrade_should_work() -> Result<()> {
let admin_id: AccountId = format!("{ADMIN_PUBLIC_KEY_MULTIHASH}@admin")
.parse()
.unwrap();
let admin_keypair = KeyPair::new(
admin_id.signatory().clone(),
ADMIN_PRIVATE_KEY_MULTIHASH.parse().unwrap(),
)
.unwrap();
let admin_private_key = ADMIN_PRIVATE_KEY_MULTIHASH
.parse::<iroha::crypto::PrivateKey>()
.unwrap();

let (_rt, _peer, client) = <PeerBuilder>::new().with_port(10_795).start_with_runtime();
wait_for_genesis_committed(&vec![client.clone()], 0);
Expand All @@ -48,7 +45,7 @@ fn executor_upgrade_should_work() -> Result<()> {
let transfer_alice_rose = Transfer::asset_numeric(alice_rose, 1u32, admin_id.clone());
let transfer_rose_tx = TransactionBuilder::new(chain_id.clone(), admin_id.clone())
.with_instructions([transfer_alice_rose.clone()])
.sign(&admin_keypair);
.sign(&admin_private_key);
let _ = client
.submit_transaction_blocking(&transfer_rose_tx)
.expect_err("Should fail");
Expand All @@ -62,7 +59,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.clone())
.with_instructions([transfer_alice_rose])
.sign(&admin_keypair);
.sign(&admin_private_key);
client
.submit_transaction_blocking(&transfer_rose_tx)
.expect("Should succeed");
Expand Down
Binary file modified configs/swarm/executor.wasm
Binary file not shown.
2 changes: 1 addition & 1 deletion core/benches/blocks/apply_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl StateApplyBlocks {
&mut state_block,
instructions,
alice_id.clone(),
&alice_keypair,
alice_keypair.private_key(),
);
let _events = state_block.apply_without_execution(&block);
state_block.commit();
Expand Down
8 changes: 4 additions & 4 deletions core/benches/blocks/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,25 @@ pub fn create_block(
state: &mut StateBlock<'_>,
instructions: Vec<InstructionBox>,
account_id: AccountId,
key_pair: &KeyPair,
private_key: &PrivateKey,
) -> CommittedBlock {
let chain_id = ChainId::from("0");

let transaction = TransactionBuilder::new(chain_id.clone(), account_id)
.with_instructions(instructions)
.sign(key_pair);
.sign(private_key);
let limits = state.transaction_executor().transaction_limits;

let (peer_public_key, _) = KeyPair::random().into_parts();
let peer_id = PeerId::new("127.0.0.1:8080".parse().unwrap(), peer_public_key);
let topology = Topology::new(vec![peer_id]);
let block = BlockBuilder::new(
vec![AcceptedTransaction::accept(transaction, &chain_id, &limits).unwrap()],
vec![AcceptedTransaction::accept(transaction, &chain_id, limits).unwrap()],
topology.clone(),
Vec::new(),
)
.chain(0, state)
.sign(key_pair.private_key())
.sign(private_key)
.unpack(|_| {})
.commit(&topology)
.unpack(|_| {})
Expand Down
8 changes: 4 additions & 4 deletions core/benches/blocks/validate_blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use common::*;
pub struct StateValidateBlocks {
state: State,
instructions: Vec<Vec<InstructionBox>>,
key_pair: KeyPair,
private_key: PrivateKey,
account_id: AccountId,
}

Expand Down Expand Up @@ -41,7 +41,7 @@ impl StateValidateBlocks {
Self {
state,
instructions,
key_pair: alice_keypair,
private_key: alice_keypair.private_key().clone(),
account_id: alice_id,
}
}
Expand All @@ -58,7 +58,7 @@ impl StateValidateBlocks {
Self {
state,
instructions,
key_pair,
private_key,
account_id,
}: Self,
) {
Expand All @@ -68,7 +68,7 @@ impl StateValidateBlocks {
&mut state_block,
instructions,
account_id.clone(),
&key_pair,
&private_key,
);
let _events = state_block.apply_without_execution(&block);
assert_eq!(state_block.height(), i);
Expand Down
4 changes: 2 additions & 2 deletions core/benches/kura.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ async fn measure_block_size_for_n_executors(n_executors: u32) {
let transfer = Transfer::asset_numeric(alice_xor_id, 10u32, bob_id);
let tx = TransactionBuilder::new(chain_id.clone(), alice_id.clone())
.with_instructions([transfer])
.sign(&alice_keypair);
.sign(alice_keypair.private_key());
let transaction_limits = TransactionLimits {
max_instruction_number: 4096,
max_wasm_size_bytes: 0,
};
let tx = AcceptedTransaction::accept(tx, &chain_id, &transaction_limits)
let tx = AcceptedTransaction::accept(tx, &chain_id, transaction_limits)
.expect("Failed to accept Transaction.");
let dir = tempfile::tempdir().expect("Could not create tempfile.");
let cfg = Config {
Expand Down
16 changes: 8 additions & 8 deletions core/benches/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ fn build_test_and_transient_state() -> State {
fn accept_transaction(criterion: &mut Criterion) {
let chain_id = ChainId::from("0");

let transaction = build_test_transaction(chain_id.clone()).sign(&STARTER_KEYPAIR);
let transaction = build_test_transaction(chain_id.clone()).sign(STARTER_KEYPAIR.private_key());
let mut success_count = 0;
let mut failures_count = 0;
let _ = criterion.bench_function("accept", |b| {
b.iter(|| {
match AcceptedTransaction::accept(transaction.clone(), &chain_id, &TRANSACTION_LIMITS) {
match AcceptedTransaction::accept(transaction.clone(), &chain_id, TRANSACTION_LIMITS) {
Ok(_) => success_count += 1,
Err(_) => failures_count += 1,
}
Expand All @@ -98,13 +98,13 @@ fn sign_transaction(criterion: &mut Criterion) {
let chain_id = ChainId::from("0");

let transaction = build_test_transaction(chain_id);
let key_pair = KeyPair::random();
let (_, private_key) = KeyPair::random().into_parts();
let mut count = 0;
let _ = criterion.bench_function("sign", |b| {
b.iter_batched(
|| transaction.clone(),
|transaction| {
let _: SignedTransaction = transaction.sign(&key_pair);
let _: SignedTransaction = transaction.sign(&private_key);
count += 1;
},
BatchSize::SmallInput,
Expand All @@ -117,9 +117,9 @@ fn validate_transaction(criterion: &mut Criterion) {
let chain_id = ChainId::from("0");

let transaction = AcceptedTransaction::accept(
build_test_transaction(chain_id.clone()).sign(&STARTER_KEYPAIR),
build_test_transaction(chain_id.clone()).sign(STARTER_KEYPAIR.private_key()),
&chain_id,
&TRANSACTION_LIMITS,
TRANSACTION_LIMITS,
)
.expect("Failed to accept transaction.");
let mut success_count = 0;
Expand All @@ -142,9 +142,9 @@ fn sign_blocks(criterion: &mut Criterion) {
let chain_id = ChainId::from("0");

let transaction = AcceptedTransaction::accept(
build_test_transaction(chain_id.clone()).sign(&STARTER_KEYPAIR),
build_test_transaction(chain_id.clone()).sign(STARTER_KEYPAIR.private_key()),
&chain_id,
&TRANSACTION_LIMITS,
TRANSACTION_LIMITS,
)
.expect("Failed to accept transaction.");
let kura = iroha_core::kura::Kura::blank_kura_for_testing();
Expand Down
Loading

0 comments on commit e9a2788

Please sign in to comment.