diff --git a/client/src/client.rs b/client/src/client.rs index fadfcafef36..00ff6e0f893 100644 --- a/client/src/client.rs +++ b/client/src/client.rs @@ -32,7 +32,6 @@ use crate::{ predicate::PredicateBox, prelude::*, query::{Pagination, Query, Sorting}, - transaction::TransactionPayload, BatchedResponse, ChainId, ValidationFail, }, http::{Method as HttpMethod, RequestBuilder, Response, StatusCode}, @@ -491,7 +490,7 @@ impl Client { /// /// # Errors /// Fails if sending transaction to peer fails or if it response with error - pub fn submit(&self, instruction: impl Instruction) -> Result> { + pub fn submit(&self, instruction: impl Instruction) -> Result> { let isi = instruction.into(); self.submit_all([isi]) } @@ -504,7 +503,7 @@ impl Client { pub fn submit_all( &self, instructions: impl IntoIterator, - ) -> Result> { + ) -> Result> { self.submit_all_with_metadata(instructions, UnlimitedMetadata::new()) } @@ -518,7 +517,7 @@ impl Client { &self, instruction: impl Instruction, metadata: UnlimitedMetadata, - ) -> Result> { + ) -> Result> { self.submit_all_with_metadata([instruction], metadata) } @@ -532,7 +531,7 @@ impl Client { &self, instructions: impl IntoIterator, metadata: UnlimitedMetadata, - ) -> Result> { + ) -> Result> { self.submit_transaction(&self.build_transaction(instructions, metadata)) } @@ -544,7 +543,7 @@ impl Client { pub fn submit_transaction( &self, transaction: &SignedTransaction, - ) -> Result> { + ) -> Result> { iroha_logger::trace!(tx=?transaction, "Submitting"); let (req, hash) = self.prepare_transaction_request::(transaction); let response = req @@ -563,9 +562,9 @@ impl Client { pub fn submit_transaction_blocking( &self, transaction: &SignedTransaction, - ) -> Result> { + ) -> Result> { let (init_sender, init_receiver) = tokio::sync::oneshot::channel(); - let hash = transaction.hash_of_payload(); + let hash = transaction.hash(); thread::scope(|spawner| { let submitter_handle = spawner.spawn(move || -> Result<()> { @@ -592,8 +591,8 @@ impl Client { fn listen_for_tx_confirmation( &self, init_sender: tokio::sync::oneshot::Sender, - hash: HashOf, - ) -> Result> { + hash: HashOf, + ) -> Result> { let deadline = tokio::time::Instant::now() + self.transaction_status_timeout; let rt = tokio::runtime::Builder::new_current_thread() .enable_all() @@ -629,8 +628,8 @@ impl Client { async fn listen_for_tx_confirmation_loop( event_iterator: &mut AsyncEventStream, - hash: HashOf, - ) -> Result> { + hash: HashOf, + ) -> Result> { while let Some(event) = event_iterator.next().await { if let Event::Pipeline(this_event) = event? { match this_event.status() { @@ -657,7 +656,7 @@ impl Client { fn prepare_transaction_request( &self, transaction: &SignedTransaction, - ) -> (B, HashOf) { + ) -> (B, HashOf) { let transaction_bytes: Vec = transaction.encode_versioned(); ( @@ -669,7 +668,7 @@ impl Client { ) .headers(self.headers.clone()) .body(transaction_bytes), - transaction.hash_of_payload(), + transaction.hash(), ) } @@ -681,7 +680,7 @@ impl Client { pub fn submit_blocking( &self, instruction: impl Instruction, - ) -> Result> { + ) -> Result> { self.submit_all_blocking(vec![instruction.into()]) } @@ -693,7 +692,7 @@ impl Client { pub fn submit_all_blocking( &self, instructions: impl IntoIterator, - ) -> Result> { + ) -> Result> { self.submit_all_blocking_with_metadata(instructions, UnlimitedMetadata::new()) } @@ -707,7 +706,7 @@ impl Client { &self, instruction: impl Instruction, metadata: UnlimitedMetadata, - ) -> Result> { + ) -> Result> { self.submit_all_blocking_with_metadata(vec![instruction.into()], metadata) } @@ -721,7 +720,7 @@ impl Client { &self, instructions: impl IntoIterator, metadata: UnlimitedMetadata, - ) -> Result> { + ) -> Result> { let transaction = self.build_transaction(instructions, metadata); self.submit_transaction_blocking(&transaction) } @@ -973,50 +972,6 @@ impl Client { ) } - /// Find the original transaction in the local pending tx queue. - /// Should be used for an MST case. - /// - /// # Errors - /// - if sending request fails - pub fn get_original_matching_transactions( - &self, - transaction: &SignedTransaction, - retry_count: u32, - retry_in: Duration, - ) -> Result> { - let url = self - .torii_url - .join(torii_uri::MATCHING_PENDING_TRANSACTIONS) - .expect("Valid URI"); - let body = transaction.encode(); - - for _ in 0..retry_count { - let response = DefaultRequestBuilder::new(HttpMethod::POST, url.clone()) - .headers(self.headers.clone()) - .header(http::header::CONTENT_TYPE, APPLICATION_JSON) - .body(body.clone()) - .build()? - .send()?; - - if response.status() == StatusCode::OK { - let pending_transactions: Vec = - DecodeAll::decode_all(&mut response.body().as_slice())?; - - if !pending_transactions.is_empty() { - return Ok(pending_transactions); - } - thread::sleep(retry_in); - } else { - return Err(eyre!( - "Failed to make query request with HTTP status: {}, {}", - response.status(), - std::str::from_utf8(response.body()).unwrap_or(""), - )); - } - } - Ok(Vec::new()) - } - /// Get value of config on peer /// /// # Errors @@ -1622,7 +1577,7 @@ mod tests { || client.build_transaction(Vec::::new(), UnlimitedMetadata::new()); let tx1 = build_transaction(); let tx2 = build_transaction(); - assert_ne!(tx1.hash_of_payload(), tx2.hash_of_payload()); + assert_ne!(tx1.hash(), tx2.hash()); let tx2 = { let mut tx = @@ -1640,7 +1595,7 @@ mod tests { client.sign_transaction(tx) }; - assert_eq!(tx1.hash_of_payload(), tx2.hash_of_payload()); + assert_eq!(tx1.hash(), tx2.hash()); } #[test] diff --git a/client/tests/integration/burn_public_keys.rs b/client/tests/integration/burn_public_keys.rs index b28c9637eae..d86348da2d2 100644 --- a/client/tests/integration/burn_public_keys.rs +++ b/client/tests/integration/burn_public_keys.rs @@ -1,7 +1,7 @@ use iroha_client::{ client::{account, transaction, Client}, crypto::{HashOf, KeyPair, PublicKey}, - data_model::{isi::Instruction, prelude::*, transaction::TransactionPayload}, + data_model::{isi::Instruction, prelude::*}, }; use test_network::*; @@ -11,7 +11,7 @@ fn submit( submitter: Option<(AccountId, KeyPair)>, ) -> ( HashOf, - eyre::Result>, + eyre::Result>, ) { let chain_id = ChainId::from("0"); diff --git a/client/tests/integration/events/pipeline.rs b/client/tests/integration/events/pipeline.rs index aa546b63153..96f85500fac 100644 --- a/client/tests/integration/events/pipeline.rs +++ b/client/tests/integration/events/pipeline.rs @@ -52,7 +52,7 @@ fn test_with_instruction_and_status_and_port( // Given let submitter = client; let transaction = submitter.build_transaction(instruction, UnlimitedMetadata::new()); - let hash = transaction.hash_of_payload(); + let hash = transaction.hash(); let mut handles = Vec::new(); for listener in clients { let checker = Checker { listener, hash }; @@ -74,7 +74,7 @@ fn test_with_instruction_and_status_and_port( #[derive(Clone)] struct Checker { listener: iroha_client::client::Client, - hash: HashOf, + hash: HashOf, } impl Checker { diff --git a/client/tests/integration/multisignature_transaction.rs b/client/tests/integration/multisignature_transaction.rs index 66c218e32ff..66914cc7be0 100644 --- a/client/tests/integration/multisignature_transaction.rs +++ b/client/tests/integration/multisignature_transaction.rs @@ -1,8 +1,9 @@ -use std::{str::FromStr as _, thread, time::Duration}; +use std::{str::FromStr as _, thread}; use eyre::Result; use iroha_client::{ - client::{self, Client, QueryResult}, + client, + client::{Client, QueryResult}, config::Config as ClientConfig, crypto::KeyPair, data_model::{ @@ -15,7 +16,7 @@ use test_network::*; #[allow(clippy::too_many_lines)] #[test] -fn multisignature_transactions_should_wait_for_all_signatures() -> Result<()> { +fn multisignature_transactions_should_be_accepted_after_fully_signed() -> Result<()> { let (_rt, network, client) = Network::start_test_with_runtime(4, Some(10_945)); wait_for_genesis_committed(&network.clients(), 0); let pipeline_time = Config::pipeline_time(); @@ -54,7 +55,11 @@ fn multisignature_transactions_should_wait_for_all_signatures() -> Result<()> { let client = Client::new(client_config.clone()); let instructions = [mint_asset.clone()]; let transaction = client.build_transaction(instructions, UnlimitedMetadata::new()); - client.submit_transaction(&client.sign_transaction(transaction))?; + // The tx signed by the first account + let _ = client + .submit_transaction(&client.sign_transaction(transaction.clone())) + .expect_err("Transaction should not be added into the queue"); + thread::sleep(pipeline_time); //Then @@ -77,14 +82,11 @@ fn multisignature_transactions_should_wait_for_all_signatures() -> Result<()> { client_config.key_pair = key_pair_2; let client_2 = Client::new(client_config); - let instructions = [mint_asset]; - let transaction = client_2.build_transaction(instructions, UnlimitedMetadata::new()); - let transaction = client_2 - .get_original_matching_transactions(&transaction, 3, Duration::from_millis(100))? - .pop() - .expect("Found no pending transaction for this account."); + // The tx signed by the second account client_2.submit_transaction(&client_2.sign_transaction(transaction))?; + thread::sleep(pipeline_time); + let assets = client_1 .request(request)? .collect::>>()?; @@ -94,5 +96,6 @@ fn multisignature_transactions_should_wait_for_all_signatures() -> Result<()> { .find(|asset| *asset.id() == asset_id) .expect("Failed to find expected asset"); assert_eq!(AssetValue::Quantity(quantity), *camomile_asset.value()); + Ok(()) } diff --git a/client_cli/pytests/README.md b/client_cli/pytests/README.md index f565dc71e26..f702ab4f6ab 100644 --- a/client_cli/pytests/README.md +++ b/client_cli/pytests/README.md @@ -164,7 +164,7 @@ The variables: ```shell CLIENT_CLI_BINARY=/path/to/iroha_client_cli -CLIENT_CLI_CONFIG=/path/to/config.toml +CLIENT_CLI_CONFIG=/path/to/client.toml TORII_API_PORT_MIN=8080 TORII_API_PORT_MAX=8083 ``` diff --git a/client_cli/pytests/src/client_cli/client_cli.py b/client_cli/pytests/src/client_cli/client_cli.py index 6f939018035..a5b0c6af34a 100644 --- a/client_cli/pytests/src/client_cli/client_cli.py +++ b/client_cli/pytests/src/client_cli/client_cli.py @@ -22,10 +22,7 @@ class ClientCli: """ BASE_PATH = CLIENT_CLI_PATH - # --skip-mst-check flag is used because - # MST isn't used in the tests - # and don't using this flag results in tests being broken by interactive prompt - BASE_FLAGS = ["--config=" + PATH_CONFIG_CLIENT_CLI, "--skip-mst-check"] + BASE_FLAGS = ["--config=" + PATH_CONFIG_CLIENT_CLI] def __init__(self, config: Config): """ @@ -132,7 +129,7 @@ def domain(self, domain: str): :return: The current ClientCli object. :rtype: ClientCli """ - self.command.insert(3, "domain") + self.command.insert(2, "domain") self.command.append("--id=" + domain) self.execute() return self @@ -150,7 +147,7 @@ def account(self, account: str, domain: str, key: str): :return: The current ClientCli object. :rtype: ClientCli """ - self.command.insert(3, "account") + self.command.insert(2, "account") self.command.append("--id=" + account + "@" + domain) self.command.append("--key=ed0120" + key) self.execute() @@ -169,7 +166,7 @@ def asset(self, asset_definition=None, account=None, value_of_value_type=None): :return: The current ClientCli object. :rtype: ClientCli """ - self.command.insert(3, "asset") + self.command.insert(2, "asset") if asset_definition and account and value_of_value_type: self.command.append( "--asset-id=" diff --git a/client_cli/pytests/src/client_cli/configuration.py b/client_cli/pytests/src/client_cli/configuration.py index 86a06a66b4f..5e04cdfbecd 100644 --- a/client_cli/pytests/src/client_cli/configuration.py +++ b/client_cli/pytests/src/client_cli/configuration.py @@ -39,6 +39,10 @@ def load(self, path_config_client_cli): """ if not os.path.exists(path_config_client_cli): raise IOError(f"No config file found at {path_config_client_cli}") + + if not os.path.isfile(path_config_client_cli): + raise IOError(f"The path is not a file: {path_config_client_cli}") + with open(path_config_client_cli, "r", encoding="utf-8") as config_file: self._config = tomlkit.load(config_file) self.file = path_config_client_cli diff --git a/client_cli/pytests/src/client_cli/iroha.py b/client_cli/pytests/src/client_cli/iroha.py index 602bbe8a838..8c35810c21a 100644 --- a/client_cli/pytests/src/client_cli/iroha.py +++ b/client_cli/pytests/src/client_cli/iroha.py @@ -21,7 +21,7 @@ def _execute_command(self, command_name: str): :param command_name: The name of the command to execute. :type command_name: str """ - self.command.insert(3, command_name) + self.command.insert(2, command_name) self.execute() def should(self, *args, **kwargs): diff --git a/client_cli/src/main.rs b/client_cli/src/main.rs index 8e89a729d9a..7bb4fc8ea32 100644 --- a/client_cli/src/main.rs +++ b/client_cli/src/main.rs @@ -4,7 +4,6 @@ use std::{ io::{stdin, stdout}, path::PathBuf, str::FromStr, - time::Duration, }; use color_eyre::{ @@ -12,7 +11,6 @@ use color_eyre::{ Result, }; // FIXME: sync with `kagami` (it uses `inquiry`, migrate both to something single) -use dialoguer::Confirm; use erased_serde::Serialize; use iroha_client::{ client::{Client, QueryResult}, @@ -95,11 +93,6 @@ struct Args { /// More verbose output #[arg(short, long)] verbose: bool, - /// Skip MST check. By setting this flag searching similar transactions on the server can be omitted. - /// Thus if you don't use multisignature transactions you should use this flag as it will increase speed of submitting transactions. - /// Also setting this flag could be useful when `iroha_client_cli` is used to submit the same transaction multiple times (like mint for example) in short period of time. - #[arg(long)] - skip_mst_check: bool, /// Subcommands of client cli #[command(subcommand)] subcommand: Subcommand, @@ -139,9 +132,6 @@ trait RunContext { Client::new(self.configuration().clone()) } - /// Skip check for MST - fn skip_mst_check(&self) -> bool; - /// Serialize and print data /// /// # Errors @@ -153,7 +143,6 @@ trait RunContext { struct PrintJsonContext { write: W, config: Config, - skip_mst_check: bool, } impl RunContext for PrintJsonContext { @@ -165,10 +154,6 @@ impl RunContext for PrintJsonContext { writeln!(&mut self.write, "{}", serde_json::to_string_pretty(data)?)?; Ok(()) } - - fn skip_mst_check(&self) -> bool { - self.skip_mst_check - } } /// Runs subcommand @@ -195,10 +180,6 @@ impl RunArgs for Subcommand { } } -// TODO: move into config? -const RETRY_COUNT_MST: u32 = 1; -const RETRY_IN_MST: Duration = Duration::from_millis(100); - fn main() -> Result<()> { color_eyre::install()?; @@ -206,7 +187,6 @@ fn main() -> Result<()> { config: config_path, subcommand, verbose, - skip_mst_check, } = clap::Parser::parse(); let config = Config::load(config_path)?; @@ -222,7 +202,6 @@ fn main() -> Result<()> { let mut context = PrintJsonContext { write: stdout(), config, - skip_mst_check, }; subcommand.run(&mut context) @@ -241,39 +220,15 @@ fn submit( let iroha_client = context.client_from_config(); let instructions = instructions.into(); let tx = iroha_client.build_transaction(instructions, metadata); - let transactions = if context.skip_mst_check() { - vec![tx] - } else { - match iroha_client.get_original_matching_transactions( - &tx, - RETRY_COUNT_MST, - RETRY_IN_MST, - ) { - Ok(original_transactions) if !original_transactions.is_empty() && Confirm::new() - .with_prompt("There are similar transactions from your account waiting for more signatures. \ - This could be because they weren't signed with the right key, \ - or because they're a multi-signature transactions (MST). \ - Do you want to sign these transactions (yes) \ - instead of submitting a new transaction (no)?") - .interact() - .wrap_err("Failed to show interactive prompt.")? => { - original_transactions.into_iter().map(|transaction| { - iroha_client.sign_transaction(transaction) - }).collect() - } - _ => vec![tx], - } - }; + #[cfg(not(debug_assertions))] let err_msg = "Failed to submit transaction."; - for tx in transactions { - #[cfg(debug_assertions)] - let err_msg = format!("Failed to submit transaction {tx:?}"); - let hash = iroha_client - .submit_transaction_blocking(&tx) - .wrap_err(err_msg)?; - context.print_data(&hash)?; - } + #[cfg(debug_assertions)] + let err_msg = format!("Failed to submit transaction {tx:?}"); + let hash = iroha_client + .submit_transaction_blocking(&tx) + .wrap_err(err_msg)?; + context.print_data(&hash)?; Ok(()) } diff --git a/core/src/block.rs b/core/src/block.rs index 8fa62f2fdb3..89baf5b3fa8 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -637,7 +637,7 @@ mod commit { PipelineEvent { entity_kind: PipelineEntityKind::Transaction, status, - hash: tx.as_ref().hash_of_payload().into(), + hash: tx.as_ref().hash().into(), } }); let current_block = core::iter::once(PipelineEvent { diff --git a/core/src/gossiper.rs b/core/src/gossiper.rs index a67709fbb9a..fdb5bcc85f0 100644 --- a/core/src/gossiper.rs +++ b/core/src/gossiper.rs @@ -128,10 +128,10 @@ impl TransactionGossiper { tx, err: crate::queue::Error::InBlockchain, }) => { - iroha_logger::debug!(tx_payload_hash = %tx.as_ref().hash_of_payload(), "Transaction already in blockchain, ignoring...") + iroha_logger::debug!(tx_payload_hash = %tx.as_ref().hash(), "Transaction already in blockchain, ignoring...") } Err(crate::queue::Failure { tx, err }) => { - iroha_logger::error!(?err, tx_payload_hash = %tx.as_ref().hash_of_payload(), "Failed to enqueue transaction.") + iroha_logger::error!(?err, tx_payload_hash = %tx.as_ref().hash(), "Failed to enqueue transaction.") } }, Err(err) => iroha_logger::error!(%err, "Transaction rejected"), diff --git a/core/src/queue.rs b/core/src/queue.rs index 4256cca4584..856e3cc3b41 100644 --- a/core/src/queue.rs +++ b/core/src/queue.rs @@ -4,12 +4,12 @@ use std::num::NonZeroUsize; use crossbeam_queue::ArrayQueue; use dashmap::{mapref::entry::Entry, DashMap}; -use eyre::{Report, Result}; +use eyre::Result; use indexmap::IndexSet; use iroha_config::parameters::actual::Queue as Config; use iroha_crypto::HashOf; use iroha_data_model::{account::AccountId, transaction::prelude::*}; -use iroha_logger::{debug, trace, warn}; +use iroha_logger::{trace, warn}; use iroha_primitives::must_use::MustUse; use rand::seq::IteratorRandom; use thiserror::Error; @@ -18,7 +18,7 @@ use crate::prelude::*; impl AcceptedTransaction { // TODO: We should have another type of transaction like `CheckedTransaction` in the type system? - fn check_signature_condition(&self, wsv: &WorldStateView) -> Result> { + fn check_signature_condition(&self, wsv: &WorldStateView) -> MustUse { let authority = self.as_ref().authority(); let transaction_signatories = self @@ -30,8 +30,9 @@ impl AcceptedTransaction { .collect(); wsv.map_account(authority, |account| { - Ok(account.check_signature_check_condition(&transaction_signatories)) - })? + account.check_signature_check_condition(&transaction_signatories) + }) + .unwrap_or(MustUse(false)) } /// Check if [`self`] is committed or rejected. @@ -46,9 +47,9 @@ impl AcceptedTransaction { #[derive(Debug)] pub struct Queue { /// The queue for transactions - tx_hashes: ArrayQueue>, + tx_hashes: ArrayQueue>, /// [`AcceptedTransaction`]s addressed by `Hash` - accepted_txs: DashMap, AcceptedTransaction>, + accepted_txs: DashMap, AcceptedTransaction>, /// Amount of transactions per user in the queue txs_per_user: DashMap, /// The maximum number of transactions in the queue @@ -63,7 +64,7 @@ pub struct Queue { } /// Queue push error -#[derive(Error, Debug, displaydoc::Display)] +#[derive(Error, Copy, Clone, Debug, displaydoc::Display)] #[allow(variant_size_differences)] pub enum Error { /// Queue is full @@ -76,14 +77,10 @@ pub enum Error { InBlockchain, /// User reached maximum number of transactions in the queue MaximumTransactionsPerUser, - /// Failure during signature condition execution, tx payload hash: {tx_hash} - SignatureCondition { - /// Transaction hash - tx_hash: HashOf, - /// Failure reason - #[source] - reason: Report, - }, + /// The transaction is already in the queue + IsInQueue, + /// Failure during signature condition execution + SignatureCondition, } /// Failure that can pop up when pushing transaction into the queue @@ -157,23 +154,17 @@ impl Queue { ) } - fn check_tx( - &self, - tx: &AcceptedTransaction, - wsv: &WorldStateView, - ) -> Result, Error> { + fn check_tx(&self, tx: &AcceptedTransaction, wsv: &WorldStateView) -> Result<(), Error> { if self.is_in_future(tx) { Err(Error::InFuture) } else if self.is_expired(tx) { Err(Error::Expired) } else if tx.is_in_blockchain(wsv) { Err(Error::InBlockchain) + } else if !tx.check_signature_condition(wsv).into_inner() { + Err(Error::SignatureCondition) } else { - tx.check_signature_condition(wsv) - .map_err(|reason| Error::SignatureCondition { - tx_hash: tx.as_ref().hash_of_payload(), - reason, - }) + Ok(()) } } @@ -189,21 +180,17 @@ impl Queue { // Get `txs_len` before entry to avoid deadlock let txs_len = self.accepted_txs.len(); - let hash = tx.as_ref().hash_of_payload(); + let hash = tx.as_ref().hash(); let entry = match self.accepted_txs.entry(hash) { - Entry::Occupied(mut old_tx) => { - // MST case - let signatures_amount_before = old_tx.get().as_ref().signatures().len(); - assert!(old_tx.get_mut().merge_signatures(tx)); - let signatures_amount_after = old_tx.get().as_ref().signatures().len(); - let new_signatures_amount = signatures_amount_after - signatures_amount_before; - if new_signatures_amount > 0 { - debug!(%hash, new_signatures_amount, "Signatures added to existing multisignature transaction"); - } - return Ok(()); + Entry::Occupied(_) => { + return Err(Failure { + tx, + err: Error::IsInQueue, + }) } Entry::Vacant(entry) => entry, }; + if txs_len >= self.capacity.get() { warn!( max = self.capacity, @@ -237,10 +224,10 @@ impl Queue { Ok(()) } - /// Pop single transaction from the queue. Record all visited and not removed transactions in `seen`. + /// Pop single transaction from the queue. Removes all transactions that fail the `tx_check`. fn pop_from_queue( &self, - seen: &mut Vec>, + seen: &mut Vec>, wsv: &WorldStateView, expired_transactions: &mut Vec, ) -> Option { @@ -259,23 +246,19 @@ impl Queue { }; let tx = entry.get(); - if tx.is_in_blockchain(wsv) { - debug!("Transaction is already in blockchain"); - let (_, tx) = entry.remove_entry(); - self.decrease_per_user_tx_count(tx.as_ref().authority()); - continue; - } - if self.is_expired(tx) { - debug!("Transaction is expired"); - let (_, tx) = entry.remove_entry(); - self.decrease_per_user_tx_count(tx.as_ref().authority()); - expired_transactions.push(tx); - continue; - } - seen.push(hash); - if *tx.check_signature_condition(wsv).unwrap_or(MustUse(false)) { - // Transactions are not removed from the queue until expired or committed - return Some(entry.get().clone()); + match self.check_tx(tx, wsv) { + Err(e) => { + let (_, tx) = entry.remove_entry(); + self.decrease_per_user_tx_count(tx.as_ref().authority()); + if let Error::Expired = e { + expired_transactions.push(tx); + } + continue; + } + Ok(()) => { + seen.push(hash); + return Some(tx.clone()); + } } } } @@ -320,12 +303,10 @@ impl Queue { self.pop_from_queue(&mut seen_queue, wsv, &mut expired_transactions_queue) }); - let transactions_hashes: IndexSet> = transactions - .iter() - .map(|tx| tx.as_ref().hash_of_payload()) - .collect(); + let transactions_hashes: IndexSet> = + transactions.iter().map(|tx| tx.as_ref().hash()).collect(); let txs = txs_from_queue - .filter(|tx| !transactions_hashes.contains(&tx.as_ref().hash_of_payload())) + .filter(|tx| !transactions_hashes.contains(&tx.as_ref().hash())) .take(max_txs_in_block - transactions.len()); transactions.extend(txs); @@ -489,7 +470,6 @@ mod tests { async fn push_multisignature_tx() { let chain_id = ChainId::from("0"); - let max_txs_in_block = 2; let key_pairs = [KeyPair::generate(), KeyPair::generate()]; let kura = Kura::blank_kura_for_testing(); let wsv = { @@ -526,10 +506,10 @@ mod tests { AcceptedTransaction::accept(signed_tx, &chain_id, &tx_limits) .expect("Failed to accept Transaction.") }; - // Check that fully signed transaction pass signature check + // Check that fully signed transaction passes signature check assert!(matches!( fully_signed_tx.check_signature_condition(&wsv), - Ok(MustUse(true)) + MustUse(true) )); let get_tx = |key_pair| { @@ -538,27 +518,16 @@ mod tests { }; for key_pair in key_pairs { let partially_signed_tx: AcceptedTransaction = get_tx(key_pair); - // Check that non of partially signed pass signature check - assert!(matches!( + // Check that none of partially signed txs passes signature check + assert_eq!( partially_signed_tx.check_signature_condition(&wsv), - Ok(MustUse(false)) - )); - queue - .push(partially_signed_tx, &wsv) - .expect("Should be possible to put partially signed transaction into the queue"); + MustUse(false) + ); + assert!(matches!( + queue.push(partially_signed_tx, &wsv).unwrap_err().err, + Error::SignatureCondition + )) } - - // Check that transactions combined into one instead of duplicating - assert_eq!(queue.tx_len(), 1); - - let mut available = queue.collect_transactions_for_block(&wsv, max_txs_in_block); - assert_eq!(available.len(), 1); - let tx_from_queue = available.pop().expect("Checked that have one transactions"); - // Check that transaction from queue pass signature check - assert!(matches!( - tx_from_queue.check_signature_condition(&wsv), - Ok(MustUse(true)) - )); } #[test] @@ -646,7 +615,7 @@ mod tests { query_handle, )); let queue = Queue::from_config(Config { - transaction_time_to_live: Duration::from_millis(200), + transaction_time_to_live: Duration::from_millis(300), ..config_factory() }); for _ in 0..(max_txs_in_block - 1) { @@ -713,7 +682,7 @@ mod tests { #[test] async fn custom_expired_transaction_is_rejected() { - const TTL_MS: u64 = 100; + const TTL_MS: u64 = 200; let chain_id = ChainId::from("0"); diff --git a/core/src/sumeragi/main_loop.rs b/core/src/sumeragi/main_loop.rs index 2959afa1151..bc8e0956f57 100644 --- a/core/src/sumeragi/main_loop.rs +++ b/core/src/sumeragi/main_loop.rs @@ -1002,7 +1002,7 @@ fn expired_event(txn: &AcceptedTransaction) -> Event { status: PipelineStatus::Rejected(PipelineRejectionReason::Transaction( TransactionRejectionReason::Expired, )), - hash: txn.as_ref().hash_of_payload().into(), + hash: txn.as_ref().hash().into(), } .into() } diff --git a/core/src/tx.rs b/core/src/tx.rs index a9f2f8b6fa1..76397356d1c 100644 --- a/core/src/tx.rs +++ b/core/src/tx.rs @@ -120,11 +120,6 @@ impl AcceptedTransaction { Ok(Self(tx)) } - #[inline] - pub(crate) fn merge_signatures(&mut self, other: Self) -> bool { - self.0.merge_signatures(other.0) - } - #[inline] fn len_u64(instruction_count: usize) -> u64 { u64::try_from(instruction_count).expect("`usize` should always fit into `u64`") diff --git a/data_model/src/transaction.rs b/data_model/src/transaction.rs index bd13a66cf28..fbfded831ab 100644 --- a/data_model/src/transaction.rs +++ b/data_model/src/transaction.rs @@ -311,14 +311,6 @@ impl SignedTransaction { iroha_crypto::HashOf::new(self) } - /// Calculate transaction payload [`Hash`](`iroha_crypto::HashOf`). - #[inline] - #[cfg(feature = "std")] - pub fn hash_of_payload(&self) -> iroha_crypto::HashOf { - let SignedTransaction::V1(tx) = self; - iroha_crypto::HashOf::new(&tx.payload) - } - /// Sign transaction with provided key pair. #[must_use] pub fn sign(self, key_pair: &iroha_crypto::KeyPair) -> SignedTransaction { @@ -332,20 +324,6 @@ impl SignedTransaction { } .into() } - - /// Add additional signatures to this transaction - #[cfg(feature = "transparent_api")] - pub fn merge_signatures(&mut self, other: Self) -> bool { - if self.hash_of_payload() != other.hash_of_payload() { - return false; - } - - let SignedTransaction::V1(tx1) = self; - let SignedTransaction::V1(tx2) = other; - tx1.signatures.extend(tx2.signatures); - - true - } } #[cfg(feature = "transparent_api")] diff --git a/torii/const/src/lib.rs b/torii/const/src/lib.rs index 241522c09b6..080149b851b 100644 --- a/torii/const/src/lib.rs +++ b/torii/const/src/lib.rs @@ -20,8 +20,6 @@ pub mod uri { pub const SUBSCRIPTION: &str = "events"; /// The web socket uri used to subscribe to blocks stream. pub const BLOCKS_STREAM: &str = "block/stream"; - /// Get pending transactions. - pub const MATCHING_PENDING_TRANSACTIONS: &str = "matching_pending_transactions"; /// The URI for local config changing inspecting pub const CONFIGURATION: &str = "configuration"; /// URI to report status for administration diff --git a/torii/src/lib.rs b/torii/src/lib.rs index 700dc700315..0e770b2667a 100644 --- a/torii/src/lib.rs +++ b/torii/src/lib.rs @@ -163,12 +163,6 @@ impl Torii { )) .and(body::versioned()), ) - .or(endpoint3( - routing::handle_pending_transactions, - warp::path(uri::MATCHING_PENDING_TRANSACTIONS) - .and(add_state!(self.queue, self.sumeragi)) - .and(body::versioned()), - )) .or(endpoint3( routing::handle_queries, warp::path(uri::QUERY) @@ -338,7 +332,7 @@ impl Error { Config(_) | StatusSegmentNotFound(_) => StatusCode::NOT_FOUND, PushIntoQueue(err) => match **err { queue::Error::Full => StatusCode::INTERNAL_SERVER_ERROR, - queue::Error::SignatureCondition { .. } => StatusCode::UNAUTHORIZED, + queue::Error::SignatureCondition => StatusCode::UNAUTHORIZED, _ => StatusCode::BAD_REQUEST, }, #[cfg(feature = "telemetry")] diff --git a/torii/src/routing.rs b/torii/src/routing.rs index fe72ff0e27d..54599a6fb82 100644 --- a/torii/src/routing.rs +++ b/torii/src/routing.rs @@ -88,7 +88,7 @@ pub async fn handle_transaction( .push(transaction, &wsv) .map_err(|queue::Failure { tx, err }| { iroha_logger::warn!( - tx_hash=%tx.as_ref().hash_of_payload(), ?err, + tx_hash=%tx.as_ref().hash(), ?err, "Failed to push into queue" ); @@ -145,36 +145,6 @@ pub async fn handle_schema() -> Json { reply::json(&iroha_schema_gen::build_schemas()) } -/// Check if two transactions are the same. Compare their contents excluding the creation time. -fn transaction_payload_eq_excluding_creation_time( - first: &SignedTransaction, - second: &SignedTransaction, -) -> bool { - first.authority() == second.authority() - && first.instructions() == second.instructions() - && first.time_to_live() == second.time_to_live() - && first.metadata().eq(second.metadata()) -} - -#[iroha_futures::telemetry_future] -pub async fn handle_pending_transactions( - queue: Arc, - sumeragi: SumeragiHandle, - transaction: SignedTransaction, -) -> Result>> { - let query_response = sumeragi.apply_wsv(|wsv| { - queue - .all_transactions(wsv) - .map(Into::into) - .filter(|current_transaction: &SignedTransaction| { - transaction_payload_eq_excluding_creation_time(current_transaction, &transaction) - }) - .collect() - }); - - Ok(Scale(query_response)) -} - #[iroha_futures::telemetry_future] pub async fn handle_get_configuration(kiso: KisoHandle) -> Result { let dto = kiso.get_dto().await?;