From 7501abe2d66ef58c58596948791718cd47004249 Mon Sep 17 00:00:00 2001 From: Stukalov-A-M Date: Thu, 1 Feb 2024 20:00:38 +0100 Subject: [PATCH] [refactor] #3887 Hide an internal structure of the versioned structs from the public API. Signed-off-by: Stukalov-A-M --- client/benches/tps/utils.rs | 14 +- client/src/client.rs | 16 +-- client/tests/integration/events/pipeline.rs | 2 +- client/tests/integration/tx_history.rs | 6 +- core/benches/blocks/common.rs | 2 +- core/src/block.rs | 110 +++++++--------- core/src/gossiper.rs | 4 +- core/src/kura.rs | 3 +- core/src/queue.rs | 52 ++++---- core/src/smartcontracts/isi/block.rs | 6 +- core/src/smartcontracts/isi/query.rs | 6 +- core/src/smartcontracts/isi/tx.rs | 21 ++- core/src/sumeragi/main_loop.rs | 96 +++++++------- core/src/sumeragi/message.rs | 8 +- core/src/sumeragi/mod.rs | 8 +- core/src/sumeragi/network_topology.rs | 2 +- core/src/tx.rs | 37 +++--- core/src/wsv.rs | 52 ++++---- data_model/src/block.rs | 42 ++++-- data_model/src/predicate.rs | 4 +- data_model/src/query/mod.rs | 43 +++---- data_model/src/transaction.rs | 136 +++++++++++--------- data_model/src/visit.rs | 2 +- smart_contract/executor/src/default.rs | 2 +- torii/src/routing.rs | 12 +- 25 files changed, 345 insertions(+), 341 deletions(-) diff --git a/client/benches/tps/utils.rs b/client/benches/tps/utils.rs index a8e2c086c71..bbafb4187c5 100644 --- a/client/benches/tps/utils.rs +++ b/client/benches/tps/utils.rs @@ -127,18 +127,8 @@ impl Config { .next() .expect("The block is not yet in WSV. Need more sleep?"); ( - block - .payload() - .transactions - .iter() - .filter(|tx| tx.error.is_none()) - .count(), - block - .payload() - .transactions - .iter() - .filter(|tx| tx.error.is_some()) - .count(), + block.transactions().filter(|tx| tx.error.is_none()).count(), + block.transactions().filter(|tx| tx.error.is_some()).count(), ) }) .fold((0, 0), |acc, pair| (acc.0 + pair.0, acc.1 + pair.1)); diff --git a/client/src/client.rs b/client/src/client.rs index 638c19a5305..cdc7b4748e8 100644 --- a/client/src/client.rs +++ b/client/src/client.rs @@ -569,7 +569,7 @@ impl Client { transaction: &SignedTransaction, ) -> Result> { let (init_sender, init_receiver) = tokio::sync::oneshot::channel(); - let hash = transaction.payload().hash(); + let hash = transaction.hash_of_payload(); thread::scope(|spawner| { let submitter_handle = spawner.spawn(move || -> Result<()> { @@ -673,7 +673,7 @@ impl Client { ) .headers(self.headers.clone()) .body(transaction_bytes), - transaction.payload().hash(), + transaction.hash_of_payload(), ) } @@ -1627,16 +1627,16 @@ mod tests { || client.build_transaction(Vec::::new(), UnlimitedMetadata::new()); let tx1 = build_transaction(); let tx2 = build_transaction(); - assert_ne!(tx1.payload().hash(), tx2.payload().hash()); + assert_ne!(tx1.hash_of_payload(), tx2.hash_of_payload()); let tx2 = { let mut tx = TransactionBuilder::new(client.chain_id.clone(), client.account_id.clone()) - .with_executable(tx1.payload().instructions.clone()) - .with_metadata(tx1.payload().metadata.clone()); + .with_executable(tx1.instructions().clone()) + .with_metadata(tx1.metadata().clone()); - tx.set_creation_time(tx1.payload().creation_time_ms); - if let Some(nonce) = tx1.payload().nonce { + tx.set_creation_time(tx1.creation_time().as_millis().try_into().unwrap()); + if let Some(nonce) = tx1.nonce() { tx.set_nonce(nonce); } if let Some(transaction_ttl) = client.transaction_ttl { @@ -1645,7 +1645,7 @@ mod tests { client.sign_transaction(tx) }; - assert_eq!(tx1.payload().hash(), tx2.payload().hash()); + assert_eq!(tx1.hash_of_payload(), tx2.hash_of_payload()); } #[test] diff --git a/client/tests/integration/events/pipeline.rs b/client/tests/integration/events/pipeline.rs index b4ee1520439..89d6c91bf0b 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.payload().hash(); + let hash = transaction.hash_of_payload(); let mut handles = Vec::new(); for listener in clients { let checker = Checker { listener, hash }; diff --git a/client/tests/integration/tx_history.rs b/client/tests/integration/tx_history.rs index 85d81fbd3f8..1a5fad4192d 100644 --- a/client/tests/integration/tx_history.rs +++ b/client/tests/integration/tx_history.rs @@ -65,10 +65,10 @@ fn client_has_rejected_and_acepted_txs_should_return_tx_history() -> Result<()> let mut prev_creation_time = core::time::Duration::from_millis(0); for tx in &transactions { - assert_eq!(tx.payload().authority(), &account_id); + assert_eq!(tx.as_ref().authority(), &account_id); //check sorted - assert!(tx.payload().creation_time() >= prev_creation_time); - prev_creation_time = tx.payload().creation_time(); + assert!(tx.as_ref().creation_time() >= prev_creation_time); + prev_creation_time = tx.as_ref().creation_time(); } Ok(()) } diff --git a/core/benches/blocks/common.rs b/core/benches/blocks/common.rs index 0c2ec273287..1bd989de4a4 100644 --- a/core/benches/blocks/common.rs +++ b/core/benches/blocks/common.rs @@ -46,7 +46,7 @@ pub fn create_block( .unwrap(); // Verify that transactions are valid - for tx in &block.payload().transactions { + for tx in block.as_ref().transactions() { assert_eq!(tx.error, None); } diff --git a/core/src/block.rs b/core/src/block.rs index ea695adfb1e..98dbdb2c931 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -150,7 +150,7 @@ mod pending { previous_block_hash, transactions_hash: transactions .iter() - .map(TransactionValue::hash) + .map(|value| value.as_ref().hash()) .collect::>() .hash(), } @@ -239,16 +239,9 @@ mod valid { /// Block that was validated and accepted #[derive(Debug, Clone)] #[repr(transparent)] - pub struct ValidBlock(pub(super) SignedBlock); + pub struct ValidBlock(pub(crate) SignedBlock); impl ValidBlock { - pub(crate) fn payload(&self) -> &BlockPayload { - self.0.payload() - } - pub(crate) fn signatures(&self) -> &SignaturesOf { - self.0.signatures() - } - /// Validate a block against the current state of the world. /// /// # Errors @@ -266,8 +259,8 @@ mod valid { expected_chain_id: &ChainId, wsv: &mut WorldStateView, ) -> Result { - if !block.payload().header.is_genesis() { - let actual_commit_topology = &block.payload().commit_topology; + if !block.header().is_genesis() { + let actual_commit_topology = block.commit_topology(); let expected_commit_topology = &topology.ordered_peers; if actual_commit_topology != expected_commit_topology { @@ -291,7 +284,7 @@ mod valid { } let expected_block_height = wsv.height() + 1; - let actual_height = block.payload().header.height; + let actual_height = block.header().height; if expected_block_height != actual_height { return Err(( @@ -304,7 +297,7 @@ mod valid { } let expected_previous_block_hash = wsv.latest_block_hash(); - let actual_block_hash = block.payload().header.previous_block_hash; + let actual_block_hash = block.header().previous_block_hash; if expected_previous_block_hash != actual_block_hash { return Err(( @@ -317,10 +310,8 @@ mod valid { } if block - .payload() - .transactions - .iter() - .any(|tx| wsv.has_transaction(tx.hash())) + .transactions() + .any(|tx| wsv.has_transaction(tx.as_ref().hash())) { return Err((block, BlockValidationError::HasCommittedTransactions)); } @@ -344,11 +335,9 @@ mod valid { expected_chain_id: &ChainId, wsv: &mut WorldStateView, ) -> Result<(), TransactionValidationError> { - let is_genesis = block.payload().header.is_genesis(); + let is_genesis = block.header().is_genesis(); - block.payload() - .transactions - .iter() + block.transactions() // TODO: Unnecessary clone? .cloned() .try_for_each(|TransactionValue{value, error}| { @@ -395,7 +384,7 @@ mod valid { return Err((self, SignatureVerificationError::LeaderMissing.into())); } - if !self.signatures().is_subset(&signatures) { + if !self.as_ref().signatures().is_subset(&signatures) { return Err((self, SignatureVerificationError::SignatureMissing.into())); } @@ -416,13 +405,13 @@ mod valid { self, topology: &Topology, ) -> Result { - if !self.payload().header.is_genesis() { + if !self.0.header().is_genesis() { if let Err(err) = self.verify_signatures(topology) { return Err((self, err.into())); } } - Ok(CommittedBlock(self.0)) + Ok(CommittedBlock(self)) } /// Add additional signatures for [`Self`]. @@ -471,17 +460,17 @@ mod valid { /// - Missing proxy tail signature fn verify_signatures(&self, topology: &Topology) -> Result<(), SignatureVerificationError> { // TODO: Should the peer that serves genesis have a fixed role of ProxyTail in topology? - if !self.payload().header.is_genesis() + if !self.as_ref().header().is_genesis() && topology.is_consensus_required().is_some() && topology - .filter_signatures_by_roles(&[Role::ProxyTail], self.signatures()) + .filter_signatures_by_roles(&[Role::ProxyTail], self.as_ref().signatures()) .is_empty() { return Err(SignatureVerificationError::ProxyTailMissing); } #[allow(clippy::collapsible_else_if)] - if self.payload().header.is_genesis() { + if self.as_ref().header().is_genesis() { // At genesis round we blindly take on the network topology from the genesis block. } else { let roles = [ @@ -492,7 +481,7 @@ mod valid { ]; let votes_count = topology - .filter_signatures_by_roles(&roles, self.signatures()) + .filter_signatures_by_roles(&roles, self.as_ref().signatures()) .len(); if votes_count < topology.min_votes_for_commit() { return Err(SignatureVerificationError::NotEnoughSignatures { @@ -512,11 +501,22 @@ mod valid { } } + impl AsRef for ValidBlock { + fn as_ref(&self) -> &SignedBlock { + &self.0 + } + } + #[cfg(test)] mod tests { use super::*; use crate::sumeragi::network_topology::test_peers; + fn payload(block: &ValidBlock) -> &BlockPayload { + let SignedBlock::V1(signed) = &block.0; + &signed.payload + } + #[test] fn signature_verification_ok() { let key_pairs = core::iter::repeat_with(|| { @@ -529,7 +529,7 @@ mod valid { let topology = Topology::new(peers); let mut block = ValidBlock::new_dummy(); - let payload = block.payload().clone(); + let payload = payload(&block).clone(); key_pairs .iter() .map(|key_pair| SignatureOf::new(key_pair, &payload)) @@ -551,7 +551,7 @@ mod valid { let topology = Topology::new(peers); let mut block = ValidBlock::new_dummy(); - let payload = block.payload().clone(); + let payload = payload(&block).clone(); key_pairs .iter() .enumerate() @@ -575,7 +575,7 @@ mod valid { let topology = Topology::new(peers); let mut block = ValidBlock::new_dummy(); - let payload = block.payload().clone(); + let payload = payload(&block).clone(); let proxy_tail_signature = SignatureOf::new(&key_pairs[4], &payload); block .add_signature(proxy_tail_signature) @@ -603,7 +603,7 @@ mod valid { let topology = Topology::new(peers); let mut block = ValidBlock::new_dummy(); - let payload = block.payload().clone(); + let payload = payload(&block).clone(); key_pairs .iter() .enumerate() @@ -626,27 +626,11 @@ mod commit { /// Represents a block accepted by consensus. /// Every [`Self`] will have a different height. #[derive(Debug, Clone)] - // TODO: Make it pub(super) at most - pub struct CommittedBlock(pub(crate) SignedBlock); - - impl CommittedBlock { - /// Calculate block hash - pub fn hash(&self) -> HashOf { - self.0.hash() - } - /// Get block payload - pub fn payload(&self) -> &BlockPayload { - self.0.payload() - } - /// Get block signatures - pub fn signatures(&self) -> &SignaturesOf { - self.0.signatures() - } - } + pub struct CommittedBlock(pub(crate) ValidBlock); impl CommittedBlock { pub(crate) fn produce_events(&self) -> Vec { - let tx = self.payload().transactions.iter().map(|tx| { + let tx = self.as_ref().transactions().map(|tx| { let status = tx.error.as_ref().map_or_else( || PipelineStatus::Committed, |error| PipelineStatus::Rejected(error.clone().into()), @@ -655,13 +639,13 @@ mod commit { PipelineEvent { entity_kind: PipelineEntityKind::Transaction, status, - hash: tx.payload().hash().into(), + hash: tx.as_ref().hash_of_payload().into(), } }); let current_block = core::iter::once(PipelineEvent { entity_kind: PipelineEntityKind::Block, status: PipelineStatus::Committed, - hash: self.hash().into(), + hash: self.as_ref().hash().into(), }); tx.chain(current_block).collect() @@ -670,20 +654,20 @@ mod commit { impl From for ValidBlock { fn from(source: CommittedBlock) -> Self { - ValidBlock(source.0) + ValidBlock(source.0.into()) } } impl From for SignedBlock { fn from(source: CommittedBlock) -> Self { - source.0 + source.0 .0 } } // Invariants of [`CommittedBlock`] can't be violated through immutable reference impl AsRef for CommittedBlock { fn as_ref(&self) -> &SignedBlock { - &self.0 + &self.0 .0 } } } @@ -704,8 +688,8 @@ mod tests { let committed_block = valid_block.clone().commit(&topology).unwrap(); assert_eq!( - valid_block.payload().hash(), - committed_block.payload().hash() + valid_block.0.hash_of_payload(), + committed_block.as_ref().hash_of_payload() ) } @@ -746,10 +730,10 @@ mod tests { .sign(&alice_keys); // The first transaction should be confirmed - assert!(valid_block.payload().transactions[0].error.is_none()); + assert!(valid_block.0.transactions().nth(0).unwrap().error.is_none()); // The second transaction should be rejected - assert!(valid_block.payload().transactions[1].error.is_some()); + assert!(valid_block.0.transactions().nth(1).unwrap().error.is_some()); } #[tokio::test] @@ -812,10 +796,10 @@ mod tests { .sign(&alice_keys); // The first transaction should fail - assert!(valid_block.payload().transactions[0].error.is_some()); + assert!(valid_block.0.transactions().nth(0).unwrap().error.is_some()); // The third transaction should succeed - assert!(valid_block.payload().transactions[2].error.is_none()); + assert!(valid_block.0.transactions().nth(2).unwrap().error.is_none()); } #[tokio::test] @@ -869,13 +853,13 @@ mod tests { // The first transaction should be rejected assert!( - valid_block.payload().transactions[0].error.is_some(), + valid_block.0.transactions().nth(0).unwrap().error.is_some(), "The first transaction should be rejected, as it contains `Fail`." ); // The second transaction should be accepted assert!( - valid_block.payload().transactions[1].error.is_none(), + valid_block.0.transactions().nth(1).unwrap().error.is_none(), "The second transaction should be accepted." ); } diff --git a/core/src/gossiper.rs b/core/src/gossiper.rs index ccfe657f21b..e9dbe4604e4 100644 --- a/core/src/gossiper.rs +++ b/core/src/gossiper.rs @@ -126,10 +126,10 @@ impl TransactionGossiper { tx, err: crate::queue::Error::InBlockchain, }) => { - iroha_logger::debug!(tx_payload_hash = %tx.payload().hash(), "Transaction already in blockchain, ignoring...") + iroha_logger::debug!(tx_payload_hash = %tx.as_ref().hash_of_payload(), "Transaction already in blockchain, ignoring...") } Err(crate::queue::Failure { tx, err }) => { - iroha_logger::error!(?err, tx_payload_hash = %tx.payload().hash(), "Failed to enqueue transaction.") + iroha_logger::error!(?err, tx_payload_hash = %tx.as_ref().hash_of_payload(), "Failed to enqueue transaction.") } }, Err(err) => iroha_logger::error!(%err, "Transaction rejected"), diff --git a/core/src/kura.rs b/core/src/kura.rs index 2f7bed3cc50..f248248e247 100644 --- a/core/src/kura.rs +++ b/core/src/kura.rs @@ -163,8 +163,7 @@ impl Kura { match block_store.read_block_data(block.start, &mut block_data_buffer) { Ok(()) => match SignedBlock::decode_all_versioned(&block_data_buffer) { Ok(decoded_block) => { - if previous_block_hash != decoded_block.payload().header.previous_block_hash - { + if previous_block_hash != decoded_block.header().previous_block_hash { error!("Block has wrong previous block hash. Not reading any blocks beyond this height."); break; } diff --git a/core/src/queue.rs b/core/src/queue.rs index d67abd31972..20ec63648e1 100644 --- a/core/src/queue.rs +++ b/core/src/queue.rs @@ -18,9 +18,10 @@ 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> { - let authority = &self.payload().authority; + let authority = self.as_ref().authority(); let transaction_signatories = self + .as_ref() .signatures() .iter() .map(|signature| signature.public_key()) @@ -36,7 +37,7 @@ impl AcceptedTransaction { /// Check if [`self`] is committed or rejected. fn is_in_blockchain(&self, wsv: &WorldStateView) -> bool { - wsv.has_transaction(self.hash()) + wsv.has_transaction(self.as_ref().hash()) } } @@ -119,9 +120,9 @@ impl Queue { /// transaction will be expired as soon as the lesser of the /// specified TTLs was reached. pub fn is_expired(&self, tx: &AcceptedTransaction) -> bool { - let tx_creation_time = tx.payload().creation_time(); + let tx_creation_time = tx.as_ref().creation_time(); - let time_limit = tx.payload().time_to_live().map_or_else( + let time_limit = tx.as_ref().time_to_live().map_or_else( || self.tx_time_to_live, |tx_time_to_live| core::cmp::min(self.tx_time_to_live, tx_time_to_live), ); @@ -131,7 +132,7 @@ impl Queue { /// If `true`, this transaction is regarded to have been tampered to have a future timestamp. fn is_in_future(&self, tx: &AcceptedTransaction) -> bool { - let tx_timestamp = Duration::from_millis(tx.payload().creation_time_ms); + let tx_timestamp = tx.as_ref().creation_time(); tx_timestamp.saturating_sub(iroha_data_model::current_time()) > self.future_threshold } @@ -175,7 +176,7 @@ impl Queue { } else { tx.check_signature_condition(wsv) .map_err(|reason| Error::SignatureCondition { - tx_hash: tx.payload().hash(), + tx_hash: tx.as_ref().hash_of_payload(), reason, }) } @@ -193,13 +194,13 @@ impl Queue { // Get `txs_len` before entry to avoid deadlock let txs_len = self.accepted_txs.len(); - let hash = tx.payload().hash(); + let hash = tx.as_ref().hash_of_payload(); let entry = match self.accepted_txs.entry(hash) { Entry::Occupied(mut old_tx) => { // MST case - let signatures_amount_before = old_tx.get().signatures().len(); + 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().signatures().len(); + 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"); @@ -219,7 +220,7 @@ impl Queue { }); } - if let Err(err) = self.check_and_increase_per_user_tx_count(&tx.payload().authority) { + if let Err(err) = self.check_and_increase_per_user_tx_count(tx.as_ref().authority()) { return Err(Failure { tx, err }); } @@ -231,7 +232,7 @@ impl Queue { .accepted_txs .remove(&err_hash) .expect("Inserted just before match"); - self.decrease_per_user_tx_count(&err_tx.payload().authority); + self.decrease_per_user_tx_count(err_tx.as_ref().authority()); Failure { tx: err_tx, err: Error::Full, @@ -266,13 +267,13 @@ impl Queue { if tx.is_in_blockchain(wsv) { debug!("Transaction is already in blockchain"); let (_, tx) = entry.remove_entry(); - self.decrease_per_user_tx_count(&tx.payload().authority); + 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.payload().authority); + self.decrease_per_user_tx_count(tx.as_ref().authority()); expired_transactions.push(tx); continue; } @@ -324,10 +325,12 @@ impl Queue { self.pop_from_queue(&mut seen_queue, wsv, &mut expired_transactions_queue) }); - let transactions_hashes: IndexSet> = - transactions.iter().map(|tx| tx.payload().hash()).collect(); + let transactions_hashes: IndexSet> = transactions + .iter() + .map(|tx| tx.as_ref().hash_of_payload()) + .collect(); let txs = txs_from_queue - .filter(|tx| !transactions_hashes.contains(&tx.payload().hash())) + .filter(|tx| !transactions_hashes.contains(&tx.as_ref().hash_of_payload())) .take(max_txs_in_block - transactions.len()); transactions.extend(txs); @@ -607,7 +610,7 @@ mod tests { query_handle, ); let tx = accepted_tx("alice@wonderland", &alice_key); - wsv.transactions.insert(tx.hash(), 1); + wsv.transactions.insert(tx.as_ref().hash(), 1); let queue = Queue::from_configuration(&Configuration { transaction_time_to_live_ms: 100_000, max_transactions_in_queue: 100, @@ -645,7 +648,7 @@ mod tests { .expect("Default queue config should always build") }); queue.push(tx.clone(), &wsv).unwrap(); - wsv.transactions.insert(tx.hash(), 1); + wsv.transactions.insert(tx.as_ref().hash(), 1); assert_eq!( queue .collect_transactions_for_block(&wsv, max_txs_in_block) @@ -730,12 +733,12 @@ mod tests { let a = queue .collect_transactions_for_block(&wsv, max_txs_in_block) .into_iter() - .map(|tx| tx.hash()) + .map(|tx| tx.as_ref().hash()) .collect::>(); let b = queue .collect_transactions_for_block(&wsv, max_txs_in_block) .into_iter() - .map(|tx| tx.hash()) + .map(|tx| tx.as_ref().hash()) .collect::>(); assert_eq!(a.len(), 1); assert_eq!(a, b); @@ -844,7 +847,7 @@ mod tests { for tx in queue_arc_clone.collect_transactions_for_block(&wsv_clone, max_txs_in_block) { - wsv_clone.transactions.insert(tx.hash(), 1); + wsv_clone.transactions.insert(tx.as_ref().hash(), 1); } // Simulate random small delays thread::sleep(Duration::from_millis(rand::thread_rng().gen_range(0..25))); @@ -894,9 +897,10 @@ mod tests { chain_id.clone(), AccountId::from_str(alice_id).expect("Valid"), ) - .with_executable(tx.0.payload().instructions.clone()); + .with_executable(tx.0.instructions().clone()); - new_tx.set_creation_time(tx.0.payload().creation_time_ms + 2 * future_threshold_ms); + let creation_time: u64 = tx.0.creation_time().as_millis().try_into().unwrap(); + new_tx.set_creation_time(creation_time + 2 * future_threshold_ms); let new_tx = new_tx.sign(&alice_key); let limits = TransactionLimits { @@ -977,7 +981,7 @@ mod tests { assert_eq!(transactions.len(), 2); for transaction in transactions { // Put transaction hashes into wsv as if they were in the blockchain - wsv.transactions.insert(transaction.hash(), 1); + wsv.transactions.insert(transaction.as_ref().hash(), 1); } // Cleanup transactions let transactions = queue.collect_transactions_for_block(&wsv, 10); diff --git a/core/src/smartcontracts/isi/block.rs b/core/src/smartcontracts/isi/block.rs index 8fa0a808bb3..752624f09c5 100644 --- a/core/src/smartcontracts/isi/block.rs +++ b/core/src/smartcontracts/isi/block.rs @@ -30,9 +30,7 @@ impl ValidQuery for FindAllBlockHeaders { wsv: &'wsv WorldStateView, ) -> Result + 'wsv>, QueryExecutionFail> { Ok(Box::new( - wsv.all_blocks() - .rev() - .map(|block| block.payload().header.clone()), + wsv.all_blocks().rev().map(|block| block.header().clone()), )) } } @@ -47,6 +45,6 @@ impl ValidQuery for FindBlockHeaderByHash { .find(|block| block.hash() == hash) .ok_or_else(|| QueryExecutionFail::Find(FindError::Block(hash)))?; - Ok(block.payload().header.clone()) + Ok(block.header().clone()) } } diff --git a/core/src/smartcontracts/isi/query.rs b/core/src/smartcontracts/isi/query.rs index ca8d4a90f35..c4afe933047 100644 --- a/core/src/smartcontracts/isi/query.rs +++ b/core/src/smartcontracts/isi/query.rs @@ -373,7 +373,7 @@ mod tests { assert_eq!( FindBlockHeaderByHash::new(block.hash()).execute(&wsv)?, - block.payload().header + *block.header() ); assert!( @@ -445,9 +445,9 @@ mod tests { Err(Error::Find(FindError::Transaction(_))) )); - let found_accepted = FindTransactionByHash::new(va_tx.hash()).execute(&wsv)?; + let found_accepted = FindTransactionByHash::new(va_tx.as_ref().hash()).execute(&wsv)?; if found_accepted.transaction.error.is_none() { - assert_eq!(va_tx.hash(), found_accepted.transaction.hash()) + assert_eq!(va_tx.as_ref().hash(), found_accepted.as_ref().hash()) } Ok(()) } diff --git a/core/src/smartcontracts/isi/tx.rs b/core/src/smartcontracts/isi/tx.rs index f103853a9d5..009b61484be 100644 --- a/core/src/smartcontracts/isi/tx.rs +++ b/core/src/smartcontracts/isi/tx.rs @@ -30,7 +30,7 @@ impl Iterator for BlockTransactionIter { type Item = BlockTransactionRef; fn next(&mut self) -> Option { - if self.1 < self.0.payload().transactions.len() { + if self.1 < self.0.transactions().len() { let res = Some(BlockTransactionRef(Arc::clone(&self.0), self.1)); self.1 += 1; @@ -47,10 +47,19 @@ impl BlockTransactionRef { } fn authority(&self) -> &AccountId { - &self.0.payload().transactions[self.1].payload().authority + self.0 + .transactions() + .nth(self.1) + .expect("The transaction is not found") + .as_ref() + .authority() } fn value(&self) -> TransactionValue { - self.0.payload().transactions[self.1].clone() + self.0 + .transactions() + .nth(self.1) + .expect("The transaction is not found") + .clone() } } @@ -105,10 +114,8 @@ impl ValidQuery for FindTransactionByHash { let block_hash = block.hash(); - block - .payload() - .transactions - .iter() + let mut transactions = block.transactions(); + transactions .find(|transaction| transaction.value.hash() == tx_hash) .cloned() .map(Box::new) diff --git a/core/src/sumeragi/main_loop.rs b/core/src/sumeragi/main_loop.rs index 19aa97bfb6a..482d1427132 100644 --- a/core/src/sumeragi/main_loop.rs +++ b/core/src/sumeragi/main_loop.rs @@ -195,7 +195,7 @@ impl Sumeragi { if let Some(msg) = block_msg.as_ref() { let vc_index : Option = match msg { - BlockMessage::BlockCreated(bc) => Some(bc.block.payload().header.view_change_index), + BlockMessage::BlockCreated(bc) => Some(bc.block.header().view_change_index), // Signed and Committed contain no block. // Block sync updates are exempt from early pruning. BlockMessage::BlockSigned(_) | BlockMessage::BlockCommitted(_) | BlockMessage::BlockSyncUpdate(_) => None, @@ -266,7 +266,8 @@ impl Sumeragi { } }; - new_wsv.world_mut().trusted_peers_ids = block.payload().commit_topology.clone(); + new_wsv.world_mut().trusted_peers_ids = + block.as_ref().commit_topology().clone(); self.commit_block(block, new_wsv); return Err(EarlyReturn::GenesisBlockReceivedAndCommitted); } @@ -301,17 +302,13 @@ impl Sumeragi { .expect("Genesis invalid"); assert!( - !genesis - .payload() - .transactions - .iter() - .any(|tx| tx.error.is_some()), + !genesis.as_ref().transactions().any(|tx| tx.error.is_some()), "Genesis contains invalid transactions" ); info!( role = ?self.current_topology.role(&self.peer_id), - block_hash = %genesis.hash(), + block_hash = %genesis.as_ref().hash(), "Genesis block created", ); @@ -335,8 +332,8 @@ impl Sumeragi { info!( addr=%self.peer_id.address, role=%self.current_topology.role(&self.peer_id), - block_height=%block.payload().header.height, - block_hash=%block.hash(), + block_height=%block.as_ref().header().height(), + block_hash=%block.as_ref().hash(), "{}", Strategy::LOG_MESSAGE, ); @@ -399,8 +396,9 @@ impl Sumeragi { } fn cache_transaction(&mut self) { - self.transaction_cache - .retain(|tx| !self.wsv.has_transaction(tx.hash()) && !self.queue.is_expired(tx)); + self.transaction_cache.retain(|tx| { + !self.wsv.has_transaction(tx.as_ref().hash()) && !self.queue.is_expired(tx) + }); } fn vote_for_block( @@ -408,7 +406,7 @@ impl Sumeragi { topology: &Topology, BlockCreated { block }: BlockCreated, ) -> Option { - let block_hash = block.payload().hash(); + let block_hash = block.hash_of_payload(); let addr = &self.peer_id.address; let role = self.current_topology.role(&self.peer_id); trace!(%addr, %role, block_hash=%block_hash, "Block received, voting..."); @@ -466,8 +464,8 @@ impl Sumeragi { %addr, %role, peer_latest_block_hash=?self.wsv.latest_block_hash(), peer_latest_block_view_change_index=?self.wsv.latest_block_view_change_index(), - consensus_latest_block_hash=%block.hash(), - consensus_latest_block_view_change_index=%block.payload().header.view_change_index, + consensus_latest_block_hash=%block.as_ref().hash(), + consensus_latest_block_view_change_index=%block.as_ref().header().view_change_index, "Soft fork occurred: peer in inconsistent state. Rolling back and replacing top block." ); self.replace_top_block(block, new_wsv) @@ -515,7 +513,7 @@ impl Sumeragi { { error!(%addr, %role, "Received BlockCommitted message, but shouldn't"); } else if let Some(voted_block) = voting_block.take() { - let voting_block_hash = voted_block.block.payload().hash(); + let voting_block_hash = voted_block.block.as_ref().hash_of_payload(); if hash == voting_block_hash { match voted_block @@ -547,7 +545,7 @@ impl Sumeragi { .expect("Peer has `ValidatingPeer` role, which mean that current topology require consensus"); if let Some(v_block) = self.vote_for_block(¤t_topology, block_created) { - let block_hash = v_block.block.payload().hash(); + let block_hash = v_block.block.as_ref().hash_of_payload(); let msg = BlockSigned::from(v_block.block.clone()).into(); @@ -564,7 +562,7 @@ impl Sumeragi { if let Some(v_block) = self.vote_for_block(¤t_topology, block_created) { if current_view_change_index >= 1 { - let block_hash = v_block.block.payload().hash(); + let block_hash = v_block.block.as_ref().hash_of_payload(); self.broadcast_packet_to( BlockSigned::from(v_block.block.clone()).into(), @@ -597,7 +595,7 @@ impl Sumeragi { current_topology.filter_signatures_by_roles(roles, &signatures); if let Some(voted_block) = voting_block.as_mut() { - let voting_block_hash = voted_block.block.payload().hash(); + let voting_block_hash = voted_block.block.as_ref().hash_of_payload(); if hash == voting_block_hash { add_signatures::(voted_block, valid_signatures); @@ -653,12 +651,11 @@ impl Sumeragi { let created_in = create_block_start_time.elapsed(); if let Some(current_topology) = current_topology.is_consensus_required() { - info!(%addr, created_in_ms=%created_in.as_millis(), block_payload_hash=%new_block.payload().hash(), "Block created"); + info!(%addr, created_in_ms=%created_in.as_millis(), block_payload_hash=%new_block.as_ref().hash_of_payload(), "Block created"); if created_in > self.pipeline_time() / 2 { warn!("Creating block takes too much time. This might prevent consensus from operating. Consider increasing `commit_time` or decreasing `max_transactions_in_block`"); } - *voting_block = Some(VotingBlock::new(new_block.clone(), new_wsv)); let msg = BlockCreated::from(new_block).into(); @@ -688,7 +685,7 @@ impl Sumeragi { match voted_block.block.commit(current_topology) { Ok(committed_block) => { - info!(voting_block_hash = %committed_block.hash(), "Block reached required number of votes"); + info!(voting_block_hash = %committed_block.as_ref().hash(), "Block reached required number of votes"); let msg = BlockCommitted::from(committed_block.clone()).into(); @@ -925,7 +922,7 @@ pub(crate) fn run( if node_expects_block { if let Some(VotingBlock { block, .. }) = voting_block.as_ref() { // NOTE: Suspecting the tail node because it hasn't yet committed a block produced by leader - warn!(peer_public_key=%sumeragi.peer_id.public_key, %role, block=%block.payload().hash(), "Block not committed in due time, requesting view change..."); + warn!(peer_public_key=%sumeragi.peer_id.public_key, %role, block=%block.as_ref().hash_of_payload(), "Block not committed in due time, requesting view change..."); } else { // NOTE: Suspecting the leader node because it hasn't produced a block // If the current node has a transaction, the leader should have as well @@ -1005,7 +1002,7 @@ fn expired_event(txn: &AcceptedTransaction) -> Event { status: PipelineStatus::Rejected(PipelineRejectionReason::Transaction( TransactionRejectionReason::Expired, )), - hash: txn.payload().hash().into(), + hash: txn.as_ref().hash_of_payload().into(), } .into() } @@ -1111,7 +1108,7 @@ fn handle_block_sync( wsv: &WorldStateView, finalized_wsv: &WorldStateView, ) -> Result { - let block_height = block.payload().header.height; + let block_height = block.header().height; let wsv_height = wsv.height(); if wsv_height + 1 == block_height { // Normal branch for adding new block on top of current @@ -1121,7 +1118,7 @@ fn handle_block_sync( .latest_block_ref() .expect("Not in genesis round so must have at least genesis block"); let new_peers = new_wsv.peers().cloned().collect(); - let view_change_index = block.payload().header().view_change_index; + let view_change_index = block.header().view_change_index; Topology::recreate_topology(&last_committed_block, view_change_index, new_peers) }; ValidBlock::validate(block, &topology, chain_id, &mut new_wsv) @@ -1141,7 +1138,7 @@ fn handle_block_sync( .latest_block_ref() .expect("Not in genesis round so must have at least genesis block"); let new_peers = new_wsv.peers().cloned().collect(); - let view_change_index = block.payload().header().view_change_index; + let view_change_index = block.header().view_change_index; Topology::recreate_topology(&last_committed_block, view_change_index, new_peers) }; ValidBlock::validate(block, &topology, chain_id, &mut new_wsv) @@ -1153,7 +1150,7 @@ fn handle_block_sync( .map_err(|(block, error)| (block, BlockSyncError::SoftForkBlockNotValid(error))) .and_then(|block| { let peer_view_change_index = wsv.latest_block_view_change_index(); - let block_view_change_index = block.payload().header.view_change_index; + let block_view_change_index = block.as_ref().header().view_change_index; if peer_view_change_index < block_view_change_index { Ok(BlockSyncOk::ReplaceTopBlock(block, new_wsv)) } else { @@ -1186,10 +1183,16 @@ mod tests { use super::*; use crate::{query::store::LiveQueryStore, smartcontracts::Registrable}; + /// Used to inject faulty payload for testing + fn payload_mut<'a>(block: &'a mut SignedBlock) -> &'a mut BlockPayload { + let SignedBlock::V1(signed) = block; + &mut signed.payload + } + fn create_data_for_test( chain_id: &ChainId, topology: &Topology, - leader_key_pair: &KeyPair, + leader_key_pair: KeyPair, ) -> (WorldStateView, Arc, SignedBlock) { // Predefined world state let alice_id: AccountId = "alice@wonderland".parse().expect("Valid"); @@ -1222,7 +1225,7 @@ mod tests { // Creating a block of two identical transactions and validating it let block = BlockBuilder::new(vec![tx.clone(), tx], topology.clone(), Vec::new()) .chain(0, &mut wsv) - .sign(leader_key_pair); + .sign(&leader_key_pair); let genesis = block.commit(topology).expect("Block is valid"); wsv.apply(&genesis).expect("Failed to apply block"); @@ -1260,7 +1263,7 @@ mod tests { // Creating a block of two identical transactions and validating it let block = BlockBuilder::new(vec![tx1, tx2], topology.clone(), Vec::new()) .chain(0, &mut wsv.clone()) - .sign(leader_key_pair); + .sign(&leader_key_pair); (wsv, kura, block.into()) } @@ -1276,11 +1279,11 @@ mod tests { leader_key_pair.public_key().clone(), )]); let (finalized_wsv, _, mut block) = - create_data_for_test(&chain_id, &topology, &leader_key_pair); + create_data_for_test(&chain_id, &topology, leader_key_pair); let wsv = finalized_wsv.clone(); // Malform block to make it invalid - block.payload_mut().commit_topology.clear(); + payload_mut(&mut block).commit_topology.clear(); let result = handle_block_sync(&chain_id, block, &wsv, &finalized_wsv); assert!(matches!(result, Err((_, BlockSyncError::BlockNotValid(_))))) @@ -1296,7 +1299,7 @@ mod tests { leader_key_pair.public_key().clone(), )]); let (finalized_wsv, kura, mut block) = - create_data_for_test(&chain_id, &topology, &leader_key_pair); + create_data_for_test(&chain_id, &topology, leader_key_pair); let mut wsv = finalized_wsv.clone(); let validated_block = @@ -1307,7 +1310,7 @@ mod tests { kura.store_block(committed_block); // Malform block to make it invalid - block.payload_mut().commit_topology.clear(); + payload_mut(&mut block).commit_topology.clear(); let result = handle_block_sync(&chain_id, block, &wsv, &finalized_wsv); assert!(matches!( @@ -1324,11 +1327,11 @@ mod tests { let topology = Topology::new(UniqueVec::new()); let leader_key_pair = KeyPair::generate().unwrap(); let (finalized_wsv, _, mut block) = - create_data_for_test(&chain_id, &topology, &leader_key_pair); + create_data_for_test(&chain_id, &topology, leader_key_pair); let wsv = finalized_wsv.clone(); // Change block height - block.payload_mut().header.height = 42; + payload_mut(&mut block).header.height = 42; let result = handle_block_sync(&chain_id, block, &wsv, &finalized_wsv); assert!(matches!( @@ -1353,8 +1356,7 @@ mod tests { "127.0.0.1:8080".parse().unwrap(), leader_key_pair.public_key().clone(), )]); - let (finalized_wsv, _, block) = - create_data_for_test(&chain_id, &topology, &leader_key_pair); + let (finalized_wsv, _, block) = create_data_for_test(&chain_id, &topology, leader_key_pair); let wsv = finalized_wsv.clone(); let result = handle_block_sync(&chain_id, block, &wsv, &finalized_wsv); assert!(matches!(result, Ok(BlockSyncOk::CommitBlock(_, _)))) @@ -1370,7 +1372,7 @@ mod tests { leader_key_pair.public_key().clone(), )]); let (finalized_wsv, kura, mut block) = - create_data_for_test(&chain_id, &topology, &leader_key_pair); + create_data_for_test(&chain_id, &topology, leader_key_pair); let mut wsv = finalized_wsv.clone(); let validated_block = @@ -1382,7 +1384,7 @@ mod tests { assert_eq!(wsv.latest_block_view_change_index(), 0); // Increase block view change index - block.payload_mut().header.view_change_index = 42; + payload_mut(&mut block).header.view_change_index = 42; let result = handle_block_sync(&chain_id, block, &wsv, &finalized_wsv); assert!(matches!(result, Ok(BlockSyncOk::ReplaceTopBlock(_, _)))) @@ -1398,11 +1400,11 @@ mod tests { leader_key_pair.public_key().clone(), )]); let (finalized_wsv, kura, mut block) = - create_data_for_test(&chain_id, &topology, &leader_key_pair); + create_data_for_test(&chain_id, &topology, leader_key_pair); let mut wsv = finalized_wsv.clone(); // Increase block view change index - block.payload_mut().header.view_change_index = 42; + payload_mut(&mut block).header.view_change_index = 42; let validated_block = ValidBlock::validate(block.clone(), &topology, &chain_id, &mut wsv).unwrap(); @@ -1413,7 +1415,7 @@ mod tests { assert_eq!(wsv.latest_block_view_change_index(), 42); // Decrease block view change index back - block.payload_mut().header.view_change_index = 0; + payload_mut(&mut block).header.view_change_index = 0; let result = handle_block_sync(&chain_id, block, &wsv, &finalized_wsv); assert!(matches!( @@ -1436,13 +1438,13 @@ mod tests { let topology = Topology::new(UniqueVec::new()); let leader_key_pair = KeyPair::generate().unwrap(); let (finalized_wsv, _, mut block) = - create_data_for_test(&chain_id, &topology, &leader_key_pair); + create_data_for_test(&chain_id, &topology, leader_key_pair); let wsv = finalized_wsv.clone(); // Change block height and view change index // Soft-fork on genesis block is not possible - block.payload_mut().header.view_change_index = 42; - block.payload_mut().header.height = 1; + payload_mut(&mut block).header.view_change_index = 42; + payload_mut(&mut block).header.height = 1; let result = handle_block_sync(&chain_id, block, &wsv, &finalized_wsv); assert!(matches!( diff --git a/core/src/sumeragi/message.rs b/core/src/sumeragi/message.rs index 329f9ba135e..b0a80207072 100644 --- a/core/src/sumeragi/message.rs +++ b/core/src/sumeragi/message.rs @@ -64,7 +64,7 @@ pub struct BlockSigned { impl From for BlockSigned { fn from(block: ValidBlock) -> Self { - let block_hash = block.payload().hash(); + let block_hash = block.as_ref().hash_of_payload(); let SignedBlock::V1(block) = block.into(); Self { @@ -86,12 +86,12 @@ pub struct BlockCommitted { impl From for BlockCommitted { fn from(block: CommittedBlock) -> Self { - let block_hash = block.payload().hash(); - let SignedBlock::V1(block) = block.into(); + let block_hash = block.as_ref().hash_of_payload(); + let block_signatures = block.as_ref().signatures().clone(); Self { hash: block_hash, - signatures: block.signatures, + signatures: block_signatures, } } } diff --git a/core/src/sumeragi/mod.rs b/core/src/sumeragi/mod.rs index b5d0a64f360..165f273bc4f 100644 --- a/core/src/sumeragi/mod.rs +++ b/core/src/sumeragi/mod.rs @@ -128,7 +128,7 @@ impl SumeragiHandle { block_index += 1; let mut block_txs_accepted = 0; let mut block_txs_rejected = 0; - for tx in &block.payload().transactions { + for tx in block.transactions() { if tx.error.is_none() { block_txs_accepted += 1; } else { @@ -231,15 +231,15 @@ impl SumeragiHandle { mut current_topology: Topology, ) -> Topology { // NOTE: topology need to be updated up to block's view_change_index - current_topology.rotate_all_n(block.payload().header.view_change_index); + current_topology.rotate_all_n(block.header().view_change_index); let block = ValidBlock::validate(block.clone(), ¤t_topology, chain_id, wsv) .expect("Kura blocks should be valid") .commit(¤t_topology) .expect("Kura blocks should be valid"); - if block.payload().header.is_genesis() { - wsv.world_mut().trusted_peers_ids = block.payload().commit_topology.clone(); + if block.as_ref().header().is_genesis() { + wsv.world_mut().trusted_peers_ids = block.as_ref().commit_topology().clone(); } wsv.apply_without_execution(&block).expect( diff --git a/core/src/sumeragi/network_topology.rs b/core/src/sumeragi/network_topology.rs index 35328b6298f..cbc2325d1ed 100644 --- a/core/src/sumeragi/network_topology.rs +++ b/core/src/sumeragi/network_topology.rs @@ -181,7 +181,7 @@ impl Topology { view_change_index: u64, new_peers: UniqueVec, ) -> Self { - let mut topology = Topology::new(block.payload().commit_topology.clone()); + let mut topology = Topology::new(block.commit_topology().clone()); let block_signees = block .signatures() .into_iter() diff --git a/core/src/tx.rs b/core/src/tx.rs index 0cee36bc3d4..a9f2f8b6fa1 100644 --- a/core/src/tx.rs +++ b/core/src/tx.rs @@ -7,8 +7,9 @@ //! //! This is also where the actual execution of instructions, as well //! as various forms of validation are performed. + use eyre::Result; -use iroha_crypto::{HashOf, SignatureVerificationFail, SignaturesOf}; +use iroha_crypto::SignatureVerificationFail; pub use iroha_data_model::prelude::*; use iroha_data_model::{ isi::error::Mismatch, @@ -49,7 +50,7 @@ impl AcceptedTransaction { tx: GenesisTransaction, expected_chain_id: &ChainId, ) -> Result { - let actual_chain_id = &tx.0.payload().chain_id; + let actual_chain_id = tx.0.chain_id(); if expected_chain_id != actual_chain_id { return Err(AcceptTransactionFail::ChainIdMismatch(Mismatch { @@ -71,7 +72,7 @@ impl AcceptedTransaction { expected_chain_id: &ChainId, limits: &TransactionLimits, ) -> Result { - let actual_chain_id = &tx.payload().chain_id; + let actual_chain_id = tx.chain_id(); if expected_chain_id != actual_chain_id { return Err(AcceptTransactionFail::ChainIdMismatch(Mismatch { @@ -80,11 +81,11 @@ impl AcceptedTransaction { })); } - if *iroha_genesis::GENESIS_ACCOUNT_ID == tx.payload().authority { + if *iroha_genesis::GENESIS_ACCOUNT_ID == *tx.authority() { return Err(AcceptTransactionFail::UnexpectedGenesisAccountSignature); } - match &tx.payload().instructions { + match &tx.instructions() { Executable::Instructions(instructions) => { let instruction_count = instructions.len(); if Self::len_u64(instruction_count) > limits.max_instruction_number { @@ -119,24 +120,12 @@ impl AcceptedTransaction { Ok(Self(tx)) } - /// Transaction hash - pub fn hash(&self) -> HashOf { - self.0.hash() - } - - /// Payload of the transaction - pub fn payload(&self) -> &TransactionPayload { - self.0.payload() - } - - pub(crate) fn signatures(&self) -> &SignaturesOf { - self.0.signatures() - } - + #[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`") } @@ -154,6 +143,12 @@ impl From for (AccountId, Executable) { } } +impl AsRef for AcceptedTransaction { + fn as_ref(&self) -> &SignedTransaction { + &self.0 + } +} + /// Used to validate transaction and thus move transaction lifecycle forward /// /// Validation is skipped for genesis. @@ -193,7 +188,7 @@ impl TransactionExecutor { tx: AcceptedTransaction, wsv: &mut WorldStateView, ) -> Result<(), TransactionRejectionReason> { - let authority = &tx.payload().authority; + let authority = tx.as_ref().authority(); if !wsv .domain(&authority.domain_id) @@ -259,7 +254,7 @@ impl TransactionExecutor { wsv: &mut WorldStateView, ) -> Result<(), TransactionRejectionReason> { let tx: SignedTransaction = tx.into(); - let authority = tx.payload().authority.clone(); + let authority = tx.authority().clone(); wsv.executor() .clone() // Cloning executor is a cheap operation diff --git a/core/src/wsv.rs b/core/src/wsv.rs index eab695bbd88..4cabb24465a 100644 --- a/core/src/wsv.rs +++ b/core/src/wsv.rs @@ -642,23 +642,22 @@ impl WorldStateView { /// Apply transactions without actually executing them. /// It's assumed that block's transaction was already executed (as part of validation for example). - #[iroha_logger::log(skip_all, fields(block_height = block.payload().header.height))] + #[iroha_logger::log(skip_all, fields(block_height = block.as_ref().header().height()))] pub fn apply_without_execution(&mut self, block: &CommittedBlock) -> Result<()> { - let block_hash = block.hash(); + let block_hash = block.as_ref().hash(); trace!(%block_hash, "Applying block"); let time_event = self.create_time_event(block); self.events_buffer.push(Event::Time(time_event)); - let block_height = block.payload().header.height; + let block_height = block.as_ref().header().height(); block - .payload() - .transactions - .iter() + .as_ref() + .transactions() .map(|tx| &tx.value) .map(SignedTransaction::hash) .for_each(|tx_hash| { - self.transactions.insert(tx_hash, block_height); + self.transactions.insert(tx_hash, *block_height); }); self.world.triggers.handle_time_event(time_event); @@ -718,7 +717,7 @@ impl WorldStateView { /// Create time event using previous and current blocks fn create_time_event(&self, block: &CommittedBlock) -> TimeEvent { let prev_interval = self.latest_block_ref().map(|latest_block| { - let header = &latest_block.payload().header; + let header = latest_block.header(); TimeInterval { since: header.timestamp(), @@ -727,8 +726,8 @@ impl WorldStateView { }); let interval = TimeInterval { - since: block.payload().header.timestamp(), - length: block.payload().header.consensus_estimation(), + since: block.as_ref().header().timestamp(), + length: block.as_ref().header().consensus_estimation(), }; TimeEvent { @@ -744,11 +743,11 @@ impl WorldStateView { /// Fails if transaction instruction execution fails fn execute_transactions(&mut self, block: &CommittedBlock) -> Result<()> { // TODO: Should this block panic instead? - for tx in &block.payload().transactions { + for tx in block.as_ref().transactions() { if tx.error.is_none() { self.process_executable( - tx.payload().instructions(), - tx.payload().authority.clone(), + tx.as_ref().instructions(), + tx.as_ref().authority().clone(), )?; } } @@ -952,7 +951,7 @@ impl WorldStateView { let opt = self .kura .get_block_by_height(1) - .map(|genesis_block| genesis_block.payload().header.timestamp()); + .map(|genesis_block| genesis_block.header().timestamp()); if opt.is_none() { error!("Failed to get genesis block from Kura."); @@ -982,7 +981,7 @@ impl WorldStateView { pub fn latest_block_view_change_index(&self) -> u64 { self.kura .get_block_by_height(self.height()) - .map_or(0, |block| block.payload().header.view_change_index) + .map_or(0, |block| *block.header().view_change_index()) } /// Return the hash of the block one before the latest block @@ -1364,6 +1363,7 @@ mod range_bounds { #[cfg(test)] mod tests { + use iroha_data_model::block::BlockPayload; use iroha_primitives::unique_vec::UniqueVec; use super::*; @@ -1372,24 +1372,28 @@ mod tests { sumeragi::network_topology::Topology, }; + /// Used to inject faulty payload for testing + fn payload_mut(block: &mut CommittedBlock) -> &mut BlockPayload { + let SignedBlock::V1(signed) = &mut block.0 .0; + &mut signed.payload + } + #[tokio::test] async fn get_block_hashes_after_hash() { const BLOCK_CNT: usize = 10; let topology = Topology::new(UniqueVec::new()); - let block = ValidBlock::new_dummy().commit(&topology).unwrap(); + let mut block = ValidBlock::new_dummy().commit(&topology).unwrap(); let kura = Kura::blank_kura_for_testing(); let query_handle = LiveQueryStore::test().start(); let mut wsv = WorldStateView::new(World::default(), kura, query_handle); let mut block_hashes = vec![]; for i in 1..=BLOCK_CNT { - let mut block = block.clone(); + payload_mut(&mut block).header.height = i as u64; + payload_mut(&mut block).header.previous_block_hash = block_hashes.last().copied(); - block.0.payload_mut().header.height = i as u64; - block.0.payload_mut().header.previous_block_hash = block_hashes.last().copied(); - - block_hashes.push(block.hash()); + block_hashes.push(block.as_ref().hash()); wsv.apply(&block).unwrap(); } @@ -1411,9 +1415,7 @@ mod tests { for i in 1..=BLOCK_CNT { let mut block = block.clone(); - - let SignedBlock::V1(v1_block) = &mut block.0; - v1_block.payload.header.height = i as u64; + payload_mut(&mut block).header.height = i as u64; wsv.apply(&block).unwrap(); kura.store_block(block); @@ -1422,7 +1424,7 @@ mod tests { assert_eq!( &wsv.all_blocks() .skip(7) - .map(|block| block.payload().header.height) + .map(|block| *block.header().height()) .collect::>(), &[8, 9, 10] ); diff --git a/data_model/src/block.rs b/data_model/src/block.rs index 0526fe33e5b..93ce5bec045 100644 --- a/data_model/src/block.rs +++ b/data_model/src/block.rs @@ -63,6 +63,7 @@ pub mod model { pub transactions_hash: Option>>, /// Value of view change index. Used to resolve soft forks. pub view_change_index: u64, + #[getset(skip)] /// Estimation of consensus duration (in milliseconds). pub consensus_estimation_ms: u64, } @@ -143,6 +144,7 @@ impl BlockPayload { impl BlockHeader { /// Checks if it's a header of a genesis block. #[inline] + #[cfg(feature = "transparent_api")] pub const fn is_genesis(&self) -> bool { self.height == 1 } @@ -166,37 +168,56 @@ impl SignedBlockV1 { } impl SignedBlock { - /// Block payload - // FIXME: Leaking concrete type BlockPayload from Versioned container. Payload should be versioned - pub fn payload(&self) -> &BlockPayload { + /// Block transactions + #[inline] + pub fn transactions(&self) -> impl ExactSizeIterator { let SignedBlock::V1(block) = self; - block.payload() + block.payload.transactions.iter() } - /// Used to inject faulty payload for testing + /// Block header + #[inline] + pub fn header(&self) -> &BlockHeader { + let SignedBlock::V1(block) = self; + block.payload.header() + } + + /// Block commit topology + #[inline] #[cfg(feature = "transparent_api")] - pub fn payload_mut(&mut self) -> &mut BlockPayload { + pub fn commit_topology(&self) -> &UniqueVec { let SignedBlock::V1(block) = self; - &mut block.payload + &block.payload.commit_topology } /// Signatures of peers which approved this block. + #[inline] pub fn signatures(&self) -> &SignaturesOf { let SignedBlock::V1(block) = self; &block.signatures } /// Calculate block hash + #[inline] pub fn hash(&self) -> HashOf { iroha_crypto::HashOf::new(self) } + /// Calculate block payload [`Hash`](`iroha_crypto::HashOf`). + #[inline] + #[cfg(feature = "std")] + #[cfg(feature = "transparent_api")] + pub fn hash_of_payload(&self) -> iroha_crypto::HashOf { + let SignedBlock::V1(block) = self; + iroha_crypto::HashOf::new(&block.payload) + } + /// Add additional signatures to this block #[cfg(feature = "transparent_api")] #[must_use] pub fn sign(mut self, key_pair: &KeyPair) -> Self { - let signature = iroha_crypto::SignatureOf::new(key_pair, self.payload()); let SignedBlock::V1(block) = &mut self; + let signature = iroha_crypto::SignatureOf::new(key_pair, &block.payload); block.signatures.insert(signature); self } @@ -211,7 +232,8 @@ impl SignedBlock { &mut self, signature: iroha_crypto::SignatureOf, ) -> Result<(), iroha_crypto::error::Error> { - signature.verify(self.payload())?; + let SignedBlock::V1(block) = self; + signature.verify(&block.payload)?; let SignedBlock::V1(block) = self; block.signatures.insert(signature); @@ -276,7 +298,7 @@ mod candidate { .payload .transactions .iter() - .map(TransactionValue::hash) + .map(|value| value.as_ref().hash()) .collect::>() .hash(); diff --git a/data_model/src/predicate.rs b/data_model/src/predicate.rs index 87e641da98d..a18fc3e7dde 100644 --- a/data_model/src/predicate.rs +++ b/data_model/src/predicate.rs @@ -1053,9 +1053,7 @@ pub mod value { ValuePredicate::Numerical(pred) => pred.applies(input), ValuePredicate::Display(pred) => pred.applies(&input.to_string()), ValuePredicate::TimeStamp(pred) => match input { - Value::Block(block) => { - pred.applies(block.payload().header.timestamp().as_millis()) - } + Value::Block(block) => pred.applies(block.header().timestamp().as_millis()), _ => false, }, ValuePredicate::Ipv4Addr(pred) => match input { diff --git a/data_model/src/query/mod.rs b/data_model/src/query/mod.rs index af4cf8b96b1..0501691ab85 100644 --- a/data_model/src/query/mod.rs +++ b/data_model/src/query/mod.rs @@ -10,7 +10,7 @@ use alloc::{ vec, vec::Vec, }; -use core::{cmp::Ordering, num::NonZeroU32}; +use core::{cmp::Ordering, num::NonZeroU32, time::Duration}; pub use cursor::ForwardCursor; use derive_more::{Constructor, Display}; @@ -30,7 +30,7 @@ use self::{ trigger::*, }; use crate::{ - account::Account, + account::{Account, AccountId}, block::SignedBlock, seal, transaction::{SignedTransaction, TransactionPayload, TransactionValue}, @@ -177,7 +177,18 @@ pub mod model { /// Output of [`FindAllTransactions`] query #[derive( - Debug, Clone, PartialEq, Eq, Getters, Decode, Encode, Deserialize, Serialize, IntoSchema, + Debug, + Clone, + PartialOrd, + Ord, + PartialEq, + Eq, + Getters, + Decode, + Encode, + Deserialize, + Serialize, + IntoSchema, )] #[getset(get = "pub")] #[ffi_type] @@ -185,6 +196,7 @@ pub mod model { /// The hash of the block to which `tx` belongs to pub block_hash: HashOf, /// Transaction + #[getset(skip)] pub transaction: Box, } @@ -250,28 +262,9 @@ impl Query for QueryBox { type Output = Value; } -impl TransactionQueryOutput { - #[inline] - /// Return payload of the transaction - pub fn payload(&self) -> &TransactionPayload { - self.transaction.payload() - } -} - -impl PartialOrd for TransactionQueryOutput { - #[inline] - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for TransactionQueryOutput { - #[inline] - fn cmp(&self, other: &Self) -> Ordering { - let tx1 = self.transaction.payload(); - let tx2 = other.transaction.payload(); - - tx1.creation_time().cmp(&tx2.creation_time()) +impl AsRef for TransactionQueryOutput { + fn as_ref(&self) -> &SignedTransaction { + &self.transaction.value } } diff --git a/data_model/src/transaction.rs b/data_model/src/transaction.rs index bb80ad2aa79..63a8a682953 100644 --- a/data_model/src/transaction.rs +++ b/data_model/src/transaction.rs @@ -2,7 +2,6 @@ #[cfg(not(feature = "std"))] use alloc::{boxed::Box, format, string::String, vec::Vec}; use core::{ - cmp::Ordering, fmt::{Display, Formatter, Result as FmtResult}, iter::IntoIterator, num::{NonZeroU32, NonZeroU64}, @@ -24,8 +23,7 @@ use crate::{ account::AccountId, isi::{Instruction, InstructionBox}, metadata::UnlimitedMetadata, - name::Name, - ChainId, Value, + ChainId, }; #[model] @@ -102,14 +100,14 @@ pub mod model { #[ffi_type] pub struct TransactionPayload { /// Unique id of the blockchain. Used for simple replay attack protection. - #[getset(skip)] + #[getset(skip)] // FIXME: ffi error pub chain_id: ChainId, /// Creation timestamp (unix time in milliseconds). #[getset(skip)] pub creation_time_ms: u64, /// Account ID of transaction creator. pub authority: AccountId, - /// ISI or a `WebAssembly` smartcontract. + /// ISI or a `WebAssembly` smart contract. pub instructions: Executable, /// If transaction is not committed by this time it will be dropped. #[getset(skip)] @@ -119,7 +117,7 @@ pub mod model { #[getset(skip)] pub nonce: Option, /// Store for additional information. - #[getset(skip)] + #[getset(skip)] // FIXME: ffi error pub metadata: UnlimitedMetadata, } @@ -171,10 +169,25 @@ pub mod model { } /// Transaction Value used in Instructions and Queries - #[derive(Debug, Clone, PartialEq, Eq, Decode, Encode, Deserialize, Serialize, IntoSchema)] + #[derive( + Debug, + PartialOrd, + Ord, + Getters, + Clone, + PartialEq, + Eq, + Decode, + Encode, + Deserialize, + Serialize, + IntoSchema, + )] #[ffi_type] + #[getset(get = "pub")] pub struct TransactionValue { /// Committed transaction + #[getset(skip)] pub value: SignedTransaction, /// Reason of rejection pub error: Option, @@ -228,54 +241,84 @@ impl WasmSmartContract { } } -impl TransactionPayload { - /// Calculate transaction payload [`Hash`](`iroha_crypto::HashOf`). - pub fn hash(&self) -> iroha_crypto::HashOf { - iroha_crypto::HashOf::new(self) +#[cfg(any(feature = "ffi_export", feature = "ffi_import"))] +declare_versioned!(SignedTransaction 1..2, Debug, Display, Clone, PartialEq, Eq, PartialOrd, Ord, FromVariant, iroha_ffi::FfiType, IntoSchema); +#[cfg(all(not(feature = "ffi_export"), not(feature = "ffi_import")))] +declare_versioned!(SignedTransaction 1..2, Debug, Display, Clone, PartialEq, Eq, PartialOrd, Ord, FromVariant, IntoSchema); + +impl SignedTransaction { + /// Return transaction instructions + #[inline] + pub fn instructions(&self) -> &Executable { + let SignedTransaction::V1(tx) = self; + tx.payload.instructions() } - /// Metadata. - // TODO: Should implement `HasMetadata` instead - pub fn metadata(&self) -> impl ExactSizeIterator { - self.metadata.iter() + /// Return transaction authority + #[inline] + pub fn authority(&self) -> &AccountId { + let SignedTransaction::V1(tx) = self; + tx.payload.authority() } - /// Creation timestamp + /// Return transaction metadata. + #[inline] + pub fn metadata(&self) -> &UnlimitedMetadata { + let SignedTransaction::V1(tx) = self; + &tx.payload.metadata + } + + /// Creation timestamp as [`core::time::Duration`] + #[inline] pub fn creation_time(&self) -> Duration { - Duration::from_millis(self.creation_time_ms) + let SignedTransaction::V1(tx) = self; + Duration::from_millis(tx.payload.creation_time_ms) } /// If transaction is not committed by this time it will be dropped. + #[inline] pub fn time_to_live(&self) -> Option { - self.time_to_live_ms + let SignedTransaction::V1(tx) = self; + tx.payload + .time_to_live_ms .map(|ttl| Duration::from_millis(ttl.into())) } -} -#[cfg(any(feature = "ffi_export", feature = "ffi_import"))] -declare_versioned!(SignedTransaction 1..2, Debug, Display, Clone, PartialEq, Eq, PartialOrd, Ord, FromVariant, iroha_ffi::FfiType, IntoSchema); -#[cfg(all(not(feature = "ffi_export"), not(feature = "ffi_import")))] -declare_versioned!(SignedTransaction 1..2, Debug, Display, Clone, PartialEq, Eq, PartialOrd, Ord, FromVariant, IntoSchema); + /// Transaction nonce + #[inline] + pub fn nonce(&self) -> Option { + let SignedTransaction::V1(tx) = self; + tx.payload.nonce + } -impl SignedTransaction { - /// Return transaction payload - // FIXME: Leaking concrete type TransactionPayload from Versioned container. Payload should be versioned - pub fn payload(&self) -> &TransactionPayload { + /// Transaction chain id + #[inline] + pub fn chain_id(&self) -> &ChainId { let SignedTransaction::V1(tx) = self; - &tx.payload + &tx.payload.chain_id } /// Return transaction signatures + #[inline] pub fn signatures(&self) -> &SignaturesOf { let SignedTransaction::V1(tx) = self; &tx.signatures } /// Calculate transaction [`Hash`](`iroha_crypto::HashOf`). + #[inline] pub fn hash(&self) -> iroha_crypto::HashOf { 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 { @@ -293,7 +336,7 @@ impl SignedTransaction { /// Add additional signatures to this transaction #[cfg(feature = "transparent_api")] pub fn merge_signatures(&mut self, other: Self) -> bool { - if self.payload().hash() != other.payload().hash() { + if self.hash_of_payload() != other.hash_of_payload() { return false; } @@ -320,38 +363,9 @@ impl SignedTransactionV1 { } } -impl TransactionValue { - /// Calculate transaction [`Hash`](`iroha_crypto::HashOf`). - pub fn hash(&self) -> iroha_crypto::HashOf { - self.value.hash() - } - - /// [`Transaction`] payload. - #[inline] - pub fn payload(&self) -> &TransactionPayload { - self.value.payload() - } - - /// [`iroha_crypto::SignatureOf`]<[`TransactionPayload`]>. - #[inline] - pub fn signatures(&self) -> &SignaturesOf { - self.value.signatures() - } -} - -impl PartialOrd for TransactionValue { - #[inline] - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for TransactionValue { - #[inline] - fn cmp(&self, other: &Self) -> Ordering { - self.payload() - .creation_time_ms - .cmp(&other.payload().creation_time_ms) +impl AsRef for TransactionValue { + fn as_ref(&self) -> &SignedTransaction { + &self.value } } diff --git a/data_model/src/visit.rs b/data_model/src/visit.rs index f20931a59ea..74183d4d04a 100644 --- a/data_model/src/visit.rs +++ b/data_model/src/visit.rs @@ -154,7 +154,7 @@ pub fn visit_transaction( authority: &AccountId, transaction: &SignedTransaction, ) { - match transaction.payload().instructions() { + match transaction.instructions() { Executable::Wasm(wasm) => visitor.visit_wasm(authority, wasm), Executable::Instructions(instructions) => { for isi in instructions { diff --git a/smart_contract/executor/src/default.rs b/smart_contract/executor/src/default.rs index 6ac2bde0afb..cb6d1617354 100644 --- a/smart_contract/executor/src/default.rs +++ b/smart_contract/executor/src/default.rs @@ -73,7 +73,7 @@ pub fn visit_transaction( authority: &AccountId, transaction: &SignedTransaction, ) { - match transaction.payload().instructions() { + match transaction.instructions() { Executable::Wasm(wasm) => executor.visit_wasm(authority, wasm), Executable::Instructions(instructions) => { for isi in instructions { diff --git a/torii/src/routing.rs b/torii/src/routing.rs index b88d05e37f3..f615b82ed60 100644 --- a/torii/src/routing.rs +++ b/torii/src/routing.rs @@ -23,7 +23,6 @@ use iroha_data_model::{ cursor::ForwardCursor, http, sorting::Sorting, Pagination, QueryRequest, QueryWithParameters, }, - transaction::TransactionPayload, BatchedResponse, }; #[cfg(feature = "telemetry")] @@ -89,7 +88,7 @@ pub async fn handle_transaction( .push(transaction, &wsv) .map_err(|queue::Failure { tx, err }| { iroha_logger::warn!( - tx_hash=%tx.payload().hash(), ?err, + tx_hash=%tx.as_ref().hash_of_payload(), ?err, "Failed to push into queue" ); @@ -148,8 +147,8 @@ pub async fn handle_schema() -> Json { /// Check if two transactions are the same. Compare their contents excluding the creation time. fn transaction_payload_eq_excluding_creation_time( - first: &TransactionPayload, - second: &TransactionPayload, + first: &SignedTransaction, + second: &SignedTransaction, ) -> bool { first.authority() == second.authority() && first.instructions() == second.instructions() @@ -168,10 +167,7 @@ pub async fn handle_pending_transactions( .all_transactions(wsv) .map(Into::into) .filter(|current_transaction: &SignedTransaction| { - transaction_payload_eq_excluding_creation_time( - current_transaction.payload(), - transaction.payload(), - ) + transaction_payload_eq_excluding_creation_time(current_transaction, &transaction) }) .collect() });