From f11c187954f55b405eaad310e06a01c38fb14636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marin=20Ver=C5=A1i=C4=87?= Date: Wed, 9 Oct 2024 17:56:06 +0200 Subject: [PATCH] refactor: increase type safety of block lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marin Veršić --- .../iroha/examples/register_1000_triggers.rs | 10 +- crates/iroha/tests/integration/multisig.rs | 1 - crates/iroha_core/benches/blocks/common.rs | 2 + crates/iroha_core/benches/kura.rs | 2 + crates/iroha_core/benches/validation.rs | 2 +- crates/iroha_core/src/block.rs | 436 +++++++++--------- crates/iroha_core/src/block_sync.rs | 16 +- crates/iroha_core/src/kura.rs | 6 + .../src/smartcontracts/isi/query.rs | 10 +- crates/iroha_core/src/snapshot.rs | 14 +- crates/iroha_core/src/state.rs | 15 +- crates/iroha_core/src/sumeragi/main_loop.rs | 313 ++++++------- crates/iroha_core/src/sumeragi/message.rs | 15 +- crates/iroha_core/src/tx.rs | 4 +- crates/iroha_data_model/src/block.rs | 148 +++--- .../iroha_data_model/src/events/pipeline.rs | 2 + crates/iroha_data_model/src/lib.rs | 1 - crates/iroha_data_model/src/transaction.rs | 4 +- crates/iroha_ffi/src/ir.rs | 7 +- .../tests/numbers_compact_and_fixed.rs | 1 + crates/iroha_schema_derive/src/lib.rs | 2 +- crates/iroha_wasm_builder/src/lib.rs | 2 +- docs/source/references/schema.json | 12 +- 23 files changed, 542 insertions(+), 483 deletions(-) diff --git a/crates/iroha/examples/register_1000_triggers.rs b/crates/iroha/examples/register_1000_triggers.rs index 87bda9aa534..5d17b7c414d 100644 --- a/crates/iroha/examples/register_1000_triggers.rs +++ b/crates/iroha/examples/register_1000_triggers.rs @@ -27,7 +27,7 @@ fn generate_genesis( chain_id: ChainId, genesis_key_pair: &KeyPair, topology: Vec, -) -> Result> { +) -> GenesisBlock { let builder = GenesisBuilder::default() .append_instruction(SetParameter::new(Parameter::Executor( SmartContractParameter::Fuel(NonZeroU64::MAX), @@ -61,10 +61,10 @@ fn generate_genesis( .fold(builder, GenesisBuilder::append_instruction); let executor = Executor::new(load_sample_wasm("default_executor")); - Ok(builder.build_and_sign(chain_id, executor, topology, genesis_key_pair)) + builder.build_and_sign(chain_id, executor, topology, genesis_key_pair) } -fn main() -> Result<(), Box> { +fn main() { let mut peer: TestPeer = ::new().expect("Failed to create peer"); let chain_id = get_chain_id(); @@ -77,7 +77,7 @@ fn main() -> Result<(), Box> { genesis_key_pair.public_key(), ); - let genesis = generate_genesis(1_000_u32, chain_id, &genesis_key_pair, topology)?; + let genesis = generate_genesis(1_000_u32, chain_id, &genesis_key_pair, topology); let builder = PeerBuilder::new() .with_genesis(genesis) @@ -88,6 +88,4 @@ fn main() -> Result<(), Box> { rt.block_on(builder.start_with_peer(&mut peer)); wait_for_genesis_committed_with_max_retries(&vec![test_client.clone()], 0, 600); - - Ok(()) } diff --git a/crates/iroha/tests/integration/multisig.rs b/crates/iroha/tests/integration/multisig.rs index c0c74eed539..ddd57eec4b9 100644 --- a/crates/iroha/tests/integration/multisig.rs +++ b/crates/iroha/tests/integration/multisig.rs @@ -19,7 +19,6 @@ use iroha_test_samples::{gen_account_in, load_sample_wasm, ALICE_ID}; use nonzero_ext::nonzero; #[test] -#[expect(clippy::too_many_lines)] fn mutlisig() -> Result<()> { let (_rt, _peer, test_client) = ::new().with_port(11_400).start_with_runtime(); wait_for_genesis_committed(&vec![test_client.clone()], 0); diff --git a/crates/iroha_core/benches/blocks/common.rs b/crates/iroha_core/benches/blocks/common.rs index d7d3959e7e1..fa9aafa6fca 100644 --- a/crates/iroha_core/benches/blocks/common.rs +++ b/crates/iroha_core/benches/blocks/common.rs @@ -51,6 +51,8 @@ pub fn create_block( .chain(0, state) .sign(peer_private_key) .unpack(|_| {}) + .categorize(state) + .unpack(|_| {}) .commit(topology) .unpack(|_| {}) .unwrap(); diff --git a/crates/iroha_core/benches/kura.rs b/crates/iroha_core/benches/kura.rs index 6fcf81c40e0..b4acb7c2546 100644 --- a/crates/iroha_core/benches/kura.rs +++ b/crates/iroha_core/benches/kura.rs @@ -58,6 +58,8 @@ async fn measure_block_size_for_n_executors(n_executors: u32) { .chain(0, &mut state_block) .sign(&peer_private_key) .unpack(|_| {}) + .categorize(&mut state_block) + .unpack(|_| {}) }; let key_pair = KeyPair::random(); diff --git a/crates/iroha_core/benches/validation.rs b/crates/iroha_core/benches/validation.rs index 56999a2c20f..6c25ba3ef7e 100644 --- a/crates/iroha_core/benches/validation.rs +++ b/crates/iroha_core/benches/validation.rs @@ -173,7 +173,7 @@ fn sign_blocks(criterion: &mut Criterion) { b.iter_batched( || block.clone(), |block| { - let _: ValidBlock = block.sign(&peer_private_key).unpack(|_| {}); + let _: NewBlock = block.sign(&peer_private_key).unpack(|_| {}); count += 1; }, BatchSize::SmallInput, diff --git a/crates/iroha_core/src/block.rs b/crates/iroha_core/src/block.rs index 70fe9e1f99f..e2f473b8d51 100644 --- a/crates/iroha_core/src/block.rs +++ b/crates/iroha_core/src/block.rs @@ -4,7 +4,7 @@ //! 2. If a block is received, i.e. deserialized: //! `SignedBlock` -> `ValidBlock` -> `CommittedBlock` //! [`Block`]s are organised into a linear sequence over time (also known as the block chain). -use std::{error::Error as _, time::Duration}; +use std::time::Duration; use iroha_crypto::{HashOf, KeyPair, MerkleTree}; use iroha_data_model::{ @@ -16,7 +16,7 @@ use iroha_data_model::{ use thiserror::Error; pub(crate) use self::event::WithEvents; -pub use self::{chained::Chained, commit::CommittedBlock, valid::ValidBlock}; +pub use self::{chained::Chained, commit::CommittedBlock, new::NewBlock, valid::ValidBlock}; use crate::{ prelude::*, state::State, @@ -87,11 +87,6 @@ pub enum SignatureVerificationError { /// Minimal required number of signatures min_votes_for_commit: usize, }, - /// Block was signed by the same node multiple times - DuplicateSignatures { - /// Index of the faulty node in the topology - signatory: usize, - }, /// Block signatory doesn't correspond to any in topology UnknownSignatory, /// Block signature doesn't correspond to block payload @@ -100,6 +95,8 @@ pub enum SignatureVerificationError { ProxyTailMissing, /// The block doesn't have leader signature LeaderMissing, + /// Miscellaneous + Other, } /// Errors occurred on genesis block validation @@ -118,7 +115,6 @@ pub struct BlockBuilder(B); mod pending { use std::time::SystemTime; - use iroha_data_model::transaction::CommittedTransaction; use nonzero_ext::nonzero; use super::*; @@ -131,7 +127,6 @@ mod pending { #[derive(Debug, Clone)] pub struct Pending { /// Collection of transactions which have been accepted. - /// Transaction will be validated when block is chained. transactions: Vec, } @@ -153,36 +148,18 @@ mod pending { Self(Pending { transactions }) } - /// Create new BlockPayload - pub fn new_unverified( - prev_block: Option<&SignedBlock>, - view_change_index: usize, - transactions_a: Vec, - ) -> BlockPayload { - let transactions = transactions_a - .into_iter() - .map(|tx| CommittedTransaction { - value: tx.clone().into(), - error: None, - }) - .collect::>(); - BlockPayload { - header: Self::make_header(prev_block, view_change_index, &transactions), - transactions, - } - } - fn make_header( prev_block: Option<&SignedBlock>, view_change_index: usize, - transactions: &[CommittedTransaction], + transactions: &[AcceptedTransaction], ) -> BlockHeader { let prev_block_time = prev_block.map_or(Duration::ZERO, |block| block.header().creation_time()); let latest_txn_time = transactions .iter() - .map(|tx| tx.as_ref().creation_time()) + .map(AsRef::as_ref) + .map(SignedTransaction::creation_time) .max() .expect("INTERNAL BUG: Block empty"); @@ -213,7 +190,8 @@ mod pending { prev_block_hash: prev_block.map(SignedBlock::hash), transactions_hash: transactions .iter() - .map(|value| value.as_ref().hash()) + .map(AsRef::as_ref) + .map(SignedTransaction::hash) .collect::>() .hash() .expect("INTERNAL BUG: Empty block created"), @@ -227,32 +205,6 @@ mod pending { } } - fn categorize_transactions( - transactions: Vec, - state_block: &mut StateBlock<'_>, - ) -> Vec { - transactions - .into_iter() - .map(|tx| match state_block.validate(tx) { - Ok(tx) => CommittedTransaction { - value: tx, - error: None, - }, - Err((tx, error)) => { - iroha_logger::warn!( - reason = %error, - caused_by = ?error.source(), - "Transaction validation failed", - ); - CommittedTransaction { - value: tx, - error: Some(Box::new(error)), - } - } - }) - .collect() - } - /// Chain the block with existing blockchain. /// /// Upon executing this method current timestamp is stored in the block header. @@ -261,31 +213,98 @@ mod pending { view_change_index: usize, state: &mut StateBlock<'_>, ) -> BlockBuilder { - let transactions = Self::categorize_transactions(self.0.transactions, state); - - BlockBuilder(Chained(BlockPayload { + BlockBuilder(Chained { header: Self::make_header( state.latest_block().as_deref(), view_change_index, - &transactions, + &self.0.transactions, ), - transactions, - })) + transactions: self.0.transactions, + }) } } } mod chained { + use iroha_crypto::SignatureOf; + use new::NewBlock; + use super::*; /// When a [`Pending`] block is chained with the blockchain it becomes [`Chained`] block. #[derive(Debug, Clone)] - pub struct Chained(pub(super) BlockPayload); + pub struct Chained { + pub(super) header: BlockHeader, + pub(super) transactions: Vec, + } impl BlockBuilder { - /// Sign this block as Leader and get [`SignedBlock`]. - pub fn sign(self, private_key: &PrivateKey) -> WithEvents { - WithEvents::new(ValidBlock(self.0 .0.sign(private_key))) + /// Sign this block and get [`NewBlock`]. + pub fn sign(self, private_key: &PrivateKey) -> WithEvents { + let signature = BlockSignature(0, SignatureOf::new(private_key, &self.0.header)); + + WithEvents::new(NewBlock { + signature, + header: self.0.header, + transactions: self.0.transactions, + }) + } + } +} + +mod new { + use std::collections::BTreeMap; + + use super::*; + use crate::state::StateBlock; + + /// First stage in the life-cycle of a [`Block`]. + /// + /// Transactions in this block are not categorized. + #[derive(Debug, Clone)] + pub struct NewBlock { + pub(crate) signature: BlockSignature, + pub(crate) header: BlockHeader, + pub(crate) transactions: Vec, + } + + impl NewBlock { + /// Categorize transactions of this block to produce a [`ValidBlock`] + pub fn categorize(self, state_block: &mut StateBlock<'_>) -> WithEvents { + let errors = self + .transactions + .iter() + // FIXME: Redundant clone + .cloned() + .enumerate() + .fold(BTreeMap::new(), |mut acc, (idx, tx)| { + if let Err((rejected_tx, error)) = state_block.validate(tx) { + iroha_logger::debug!( + block=%self.header.hash(), + tx=%rejected_tx.hash(), + reason=?error, + "Transaction rejected" + ); + + acc.insert(idx, error); + } + + acc + }); + + let mut block: SignedBlock = self.into(); + block.set_transaction_errors(errors); + WithEvents::new(ValidBlock(block)) + } + } + + impl From for SignedBlock { + fn from(block: NewBlock) -> Self { + SignedBlock::presigned( + block.signature, + block.header, + block.transactions.into_iter().map(Into::into), + ) } } } @@ -294,7 +313,6 @@ mod valid { use std::time::SystemTime; use commit::CommittedBlock; - use indexmap::IndexMap; use iroha_data_model::{account::AccountId, events::pipeline::PipelineEventBox, ChainId}; use mv::storage::StorageReadOnly; @@ -311,34 +329,19 @@ mod valid { block: &SignedBlock, topology: &Topology, ) -> Result<(), SignatureVerificationError> { - let leader_index = topology.leader_index(); - let mut block_signatures = block.signatures(); - - let leader_signature = match block_signatures.next() { - Some(BlockSignature(signatory, signature)) - if usize::try_from(*signatory) - .map_err(|_err| SignatureVerificationError::LeaderMissing)? - == leader_index => - { - let mut additional_leader_signatures = - topology.filter_signatures_by_roles(&[Role::Leader], block_signatures); - - if additional_leader_signatures.next().is_some() { - return Err(SignatureVerificationError::DuplicateSignatures { - signatory: leader_index, - }); - } + use SignatureVerificationError::LeaderMissing; + let leader_idx = topology.leader_index(); - signature - } - _ => { - return Err(SignatureVerificationError::LeaderMissing); - } - }; + let signature = block.signatures().next().ok_or(LeaderMissing)?; + if leader_idx != usize::try_from(signature.0).map_err(|_err| LeaderMissing)? { + return Err(LeaderMissing); + } - leader_signature + signature + .1 .verify(topology.leader().public_key(), &block.payload().header) - .map_err(|_err| SignatureVerificationError::LeaderMissing)?; + .map_err(|_err| LeaderMissing)?; + Ok(()) } @@ -354,28 +357,18 @@ mod valid { topology .filter_signatures_by_roles(valid_roles, block.signatures()) - .try_fold(IndexMap::::default(), |mut acc, signature| { - let signatory_idx = usize::try_from(signature.0) - .map_err(|_err| SignatureVerificationError::UnknownSignatory)?; - - if acc.insert(signatory_idx, signature.1.clone()).is_some() { - return Err(SignatureVerificationError::DuplicateSignatures { - signatory: signatory_idx, - }); - } + .try_for_each(|signature| { + use SignatureVerificationError::{UnknownSignatory, UnknownSignature}; - Ok(acc) - })? - .into_iter() - .try_for_each(|(signatory_idx, signature)| { - let signatory: &PeerId = topology - .as_ref() - .get(signatory_idx) - .ok_or(SignatureVerificationError::UnknownSignatory)?; + let signatory = + usize::try_from(signature.0).map_err(|_err| UnknownSignatory)?; + let signatory: &PeerId = + topology.as_ref().get(signatory).ok_or(UnknownSignatory)?; signature + .1 .verify(signatory.public_key(), &block.payload().header) - .map_err(|_err| SignatureVerificationError::UnknownSignature)?; + .map_err(|_err| UnknownSignature)?; Ok(()) })?; @@ -402,47 +395,30 @@ mod valid { block: &SignedBlock, topology: &Topology, ) -> Result<(), SignatureVerificationError> { - let proxy_tail_index = topology.proxy_tail_index(); - let mut signatures = block.signatures().rev(); - - let proxy_tail_signature = match signatures.next() { - Some(BlockSignature(signatory, signature)) - if usize::try_from(*signatory) - .map_err(|_err| SignatureVerificationError::ProxyTailMissing)? - == proxy_tail_index => - { - let mut additional_proxy_tail_signatures = - topology.filter_signatures_by_roles(&[Role::ProxyTail], signatures); - - if additional_proxy_tail_signatures.next().is_some() { - return Err(SignatureVerificationError::DuplicateSignatures { - signatory: proxy_tail_index, - }); - } + use SignatureVerificationError::ProxyTailMissing; + let proxy_tail_idx = topology.proxy_tail_index(); - signature - } - _ => { - return Err(SignatureVerificationError::ProxyTailMissing); - } - }; + let signature = block.signatures().next_back().ok_or(ProxyTailMissing)?; + if proxy_tail_idx != usize::try_from(signature.0).map_err(|_err| ProxyTailMissing)? { + return Err(ProxyTailMissing); + } - proxy_tail_signature + signature + .1 .verify(topology.proxy_tail().public_key(), &block.payload().header) - .map_err(|_err| SignatureVerificationError::ProxyTailMissing)?; + .map_err(|_err| ProxyTailMissing)?; Ok(()) } - /// Validate a block against the current state of the world. Individual transaction - /// errors will be updated. + /// Validate a block against the current state of the world. + /// Individual transaction errors will be updated. /// /// # Errors /// /// - There is a mismatch between candidate block height and actual blockchain height /// - There is a mismatch between candidate block previous block hash and actual previous block hash /// - Block is not signed by the leader - /// - Block has duplicate signatures /// - Block has unknown signatories /// - Block has incorrect signatures /// - Topology field is incorrect @@ -462,12 +438,9 @@ mod valid { return WithEvents::new(Err((block, error))); } - if let Err(error) = Self::validate_transactions( - &mut block, - expected_chain_id, - genesis_account, - state_block, - ) { + if let Err(error) = + Self::categorize(&mut block, expected_chain_id, genesis_account, state_block) + { return WithEvents::new(Err((block, error.into()))); } @@ -502,7 +475,7 @@ mod valid { state.block() }; - if let Err(error) = Self::validate_transactions( + if let Err(error) = Self::categorize( &mut block, expected_chain_id, genesis_account, @@ -599,41 +572,56 @@ mod valid { Ok(()) } - fn validate_transactions( + fn categorize( block: &mut SignedBlock, expected_chain_id: &ChainId, genesis_account: &AccountId, state_block: &mut StateBlock<'_>, ) -> Result<(), TransactionValidationError> { - let is_genesis = block.header().is_genesis(); - let (max_clock_drift, tx_limits) = { let params = state_block.world().parameters(); (params.sumeragi().max_clock_drift(), params.transaction) }; - for CommittedTransaction { value, error } in block.transactions_mut() { - let tx = if is_genesis { - AcceptedTransaction::accept_genesis( - value.clone(), - expected_chain_id, - max_clock_drift, - genesis_account, - ) - } else { - AcceptedTransaction::accept( - value.clone(), - expected_chain_id, - max_clock_drift, - tx_limits, - ) - }?; + let errors = block + .transactions() + .map(AsRef::as_ref) + // FIXME: Redundant clone + .cloned() + .enumerate() + .try_fold(Vec::new(), |mut acc, (idx, tx)| { + let accepted_tx = if block.header().is_genesis() { + AcceptedTransaction::accept_genesis( + tx, + expected_chain_id, + max_clock_drift, + genesis_account, + ) + } else { + AcceptedTransaction::accept( + tx, + expected_chain_id, + max_clock_drift, + tx_limits, + ) + }?; + + if let Err((rejected_tx, error)) = state_block.validate(accepted_tx) { + iroha_logger::debug!( + tx=%rejected_tx.hash(), + block=%block.hash(), + reason=?error, + "Transaction rejected" + ); + + acc.push((idx, error)); + } + + Ok::<_, TransactionValidationError>(acc) + })?; + + block.set_transaction_errors(errors); - *error = match state_block.validate(tx) { - Ok(_) => None, - Err((_tx, error)) => Some(Box::new(error)), - }; - } Ok(()) } @@ -647,20 +635,25 @@ mod valid { signature: BlockSignature, topology: &Topology, ) -> Result<(), SignatureVerificationError> { - let signatory_idx = usize::try_from(signature.0) - .expect("INTERNAL BUG: Number of peers exceeds usize::MAX"); - let signatory = &topology.as_ref()[signatory_idx]; + use SignatureVerificationError::{Other, UnknownSignatory, UnknownSignature}; + + let signatory = usize::try_from(signature.0).map_err(|_err| UnknownSignatory)?; + let signatory = topology.as_ref().get(signatory).ok_or(UnknownSignatory)?; assert_ne!(Role::Leader, topology.role(signatory)); + assert_ne!(Role::ProxyTail, topology.role(signatory)); + assert_ne!(Role::Undefined, topology.role(signatory)); + if topology.view_change_index() == 0 { assert_ne!(Role::ObservingPeer, topology.role(signatory),); } - assert_ne!(Role::Undefined, topology.role(signatory)); - assert_ne!(Role::ProxyTail, topology.role(signatory)); - self.0 - .add_signature(signature, signatory.public_key()) - .map_err(|_err| SignatureVerificationError::UnknownSignature) + signature + .1 + .verify(signatory.public_key(), &self.as_ref().payload().header) + .map_err(|_err| UnknownSignature)?; + + self.0.add_signature(signature).map_err(|_err| Other) } /// Replace block's signatures. Returns previous block signatures @@ -668,32 +661,37 @@ mod valid { /// # Errors /// /// - Replacement signatures don't contain the leader signature - /// - Replacement signatures contain duplicate signatures /// - Replacement signatures contain unknown signatories /// - Replacement signatures contain incorrect signatures + /// - Replacement signatures contain duplicate signatures pub fn replace_signatures( &mut self, signatures: Vec, topology: &Topology, ) -> WithEvents, SignatureVerificationError>> { - let prev_signatures = self.0.replace_signatures_unchecked(signatures); + let Ok(prev_signatures) = self.0.replace_signatures(signatures) else { + return WithEvents::new(Err(SignatureVerificationError::Other)); + }; - if let Err(err) = Self::verify_leader_signature(self.as_ref(), topology) + let result = if let Err(err) = Self::verify_leader_signature(self.as_ref(), topology) .and_then(|()| Self::verify_validator_signatures(self.as_ref(), topology)) .and_then(|()| Self::verify_no_undefined_signatures(self.as_ref(), topology)) { - self.0.replace_signatures_unchecked(prev_signatures); - WithEvents::new(Err(err)) + self.0 + .replace_signatures(prev_signatures) + .expect("INTERNAL BUG: invalid signatures in block"); + Err(err) } else { - WithEvents::new(Ok(prev_signatures)) - } + Ok(prev_signatures) + }; + + WithEvents::new(result) } /// commit block to the store. /// /// # Errors /// - /// - Block has duplicate proxy tail signatures /// - Block is not signed by the proxy tail /// - Block doesn't have enough signatures pub fn commit( @@ -749,7 +747,6 @@ mod valid { /// /// # Errors /// - /// - Block has duplicate proxy tail signatures /// - Block is not signed by the proxy tail /// - Block doesn't have enough signatures fn is_commit(block: &SignedBlock, topology: &Topology) -> Result<(), BlockValidationError> { @@ -780,32 +777,36 @@ mod valid { #[cfg(test)] pub(crate) fn new_dummy(leader_private_key: &PrivateKey) -> Self { - Self::new_dummy_and_modify_payload(leader_private_key, |_| {}) + Self::new_dummy_and_modify_header(leader_private_key, |_| {}) } #[cfg(test)] - pub(crate) fn new_dummy_and_modify_payload( + pub(crate) fn new_dummy_and_modify_header( leader_private_key: &PrivateKey, - f: impl FnOnce(&mut BlockPayload), + f: impl FnOnce(&mut BlockHeader), ) -> Self { - use nonzero_ext::nonzero; - - let mut payload = BlockPayload { - header: BlockHeader { - height: nonzero!(2_u64), - prev_block_hash: None, - transactions_hash: HashOf::from_untyped_unchecked(Hash::prehashed( - [1; Hash::LENGTH], - )), - creation_time_ms: 0, - view_change_index: 0, - }, - transactions: Vec::new(), + let mut header = BlockHeader { + height: nonzero_ext::nonzero!(2_u64), + prev_block_hash: None, + transactions_hash: HashOf::from_untyped_unchecked(Hash::prehashed( + [1; Hash::LENGTH], + )), + creation_time_ms: 0, + view_change_index: 0, }; - f(&mut payload); - BlockBuilder(Chained(payload)) - .sign(leader_private_key) - .unpack(|_| {}) + f(&mut header); + let unverified_block = BlockBuilder(Chained { + header, + transactions: Vec::new(), + }) + .sign(leader_private_key) + .unpack(|_| {}); + + Self(SignedBlock::presigned( + unverified_block.signature, + unverified_block.header, + unverified_block.transactions.into_iter().map(Into::into), + )) } } @@ -993,6 +994,8 @@ mod commit { } mod event { + use new::NewBlock; + use super::*; use crate::state::StateBlock; @@ -1056,6 +1059,17 @@ mod event { } } + impl EventProducer for NewBlock { + fn produce_events(&self) -> impl Iterator { + let block_event = BlockEvent { + header: self.header.clone(), + status: BlockStatus::Created, + }; + + core::iter::once(block_event.into()) + } + } + impl EventProducer for ValidBlock { fn produce_events(&self) -> impl Iterator { let block_height = self.as_ref().header().height; @@ -1177,6 +1191,8 @@ mod tests { let valid_block = BlockBuilder::new(transactions) .chain(0, &mut state_block) .sign(alice_keypair.private_key()) + .unpack(|_| {}) + .categorize(&mut state_block) .unpack(|_| {}); // The first transaction should be confirmed @@ -1257,6 +1273,8 @@ mod tests { let valid_block = BlockBuilder::new(transactions) .chain(0, &mut state_block) .sign(alice_keypair.private_key()) + .unpack(|_| {}) + .categorize(&mut state_block) .unpack(|_| {}); // The first transaction should fail @@ -1321,6 +1339,8 @@ mod tests { let valid_block = BlockBuilder::new(transactions) .chain(0, &mut state_block) .sign(alice_keypair.private_key()) + .unpack(|_| {}) + .categorize(&mut state_block) .unpack(|_| {}); // The first transaction should be rejected @@ -1395,6 +1415,8 @@ mod tests { let valid_block = BlockBuilder::new(transactions) .chain(0, &mut state_block) .sign(genesis_correct_key.private_key()) + .unpack(|_| {}) + .categorize(&mut state_block) .unpack(|_| {}); // Validate genesis block diff --git a/crates/iroha_core/src/block_sync.rs b/crates/iroha_core/src/block_sync.rs index c7ea19eb4d0..128e0fc2323 100644 --- a/crates/iroha_core/src/block_sync.rs +++ b/crates/iroha_core/src/block_sync.rs @@ -429,8 +429,8 @@ pub mod message { PeerId::new("127.0.0.1:1234".parse().unwrap(), leader_public_key); let block0: SignedBlock = ValidBlock::new_dummy(&leader_private_key).into(); let block1 = - ValidBlock::new_dummy_and_modify_payload(&leader_private_key, |payload| { - payload.header.height = block0.header().height.checked_add(2).unwrap(); + ValidBlock::new_dummy_and_modify_header(&leader_private_key, |header| { + header.height = block0.header().height.checked_add(2).unwrap(); }) .into(); let candidate = ShareBlocksCandidate { @@ -450,9 +450,9 @@ pub mod message { PeerId::new("127.0.0.1:1234".parse().unwrap(), leader_public_key); let block0: SignedBlock = ValidBlock::new_dummy(&leader_private_key).into(); let block1 = - ValidBlock::new_dummy_and_modify_payload(&leader_private_key, |payload| { - payload.header.height = block0.header().height.checked_add(1).unwrap(); - payload.header.prev_block_hash = + ValidBlock::new_dummy_and_modify_header(&leader_private_key, |header| { + header.height = block0.header().height.checked_add(1).unwrap(); + header.prev_block_hash = Some(HashOf::from_untyped_unchecked(Hash::prehashed([0; 32]))); }) .into(); @@ -473,9 +473,9 @@ pub mod message { PeerId::new("127.0.0.1:1234".parse().unwrap(), leader_public_key); let block0: SignedBlock = ValidBlock::new_dummy(&leader_private_key).into(); let block1 = - ValidBlock::new_dummy_and_modify_payload(&leader_private_key, |payload| { - payload.header.height = block0.header().height.checked_add(1).unwrap(); - payload.header.prev_block_hash = Some(block0.hash()); + ValidBlock::new_dummy_and_modify_header(&leader_private_key, |header| { + header.height = block0.header().height.checked_add(1).unwrap(); + header.prev_block_hash = Some(block0.hash()); }) .into(); let candidate = ShareBlocksCandidate { diff --git a/crates/iroha_core/src/kura.rs b/crates/iroha_core/src/kura.rs index 1ecb99d8729..46df90f60a5 100644 --- a/crates/iroha_core/src/kura.rs +++ b/crates/iroha_core/src/kura.rs @@ -1163,6 +1163,8 @@ mod tests { .chain(0, &mut state_block) .sign(&leader_private_key) .unpack(|_| {}) + .categorize(&mut state_block) + .unpack(|_| {}) .commit(&topology) .unpack(|_| {}) .unwrap(); @@ -1179,6 +1181,8 @@ mod tests { .chain(1, &mut state_block) .sign(&leader_private_key) .unpack(|_| {}) + .categorize(&mut state_block) + .unpack(|_| {}) .commit(&topology) .unpack(|_| {}) .unwrap(); @@ -1196,6 +1200,8 @@ mod tests { .chain(0, &mut state_block) .sign(&leader_private_key) .unpack(|_| {}) + .categorize(&mut state_block) + .unpack(|_| {}) .commit(&topology) .unpack(|_| {}) .unwrap(); diff --git a/crates/iroha_core/src/smartcontracts/isi/query.rs b/crates/iroha_core/src/smartcontracts/isi/query.rs index 87404e97ff0..2c6d60a4dd6 100644 --- a/crates/iroha_core/src/smartcontracts/isi/query.rs +++ b/crates/iroha_core/src/smartcontracts/isi/query.rs @@ -437,9 +437,11 @@ mod tests { .chain(0, &mut state_block) .sign(&peer_private_key) .unpack(|_| {}) + .categorize(&mut state_block) + .unpack(|_| {}) .commit(&topology) .unpack(|_| {}) - .expect("Block is valid"); + .unwrap(); let _events = state_block.apply(&first_block, topology.as_ref().to_owned())?; kura.store_block(first_block); @@ -449,6 +451,8 @@ mod tests { .chain(0, &mut state_block) .sign(&peer_private_key) .unpack(|_| {}) + .categorize(&mut state_block) + .unpack(|_| {}) .commit(&topology) .unpack(|_| {}) .expect("Block is valid"); @@ -609,9 +613,11 @@ mod tests { .chain(0, &mut state_block) .sign(ALICE_KEYPAIR.private_key()) .unpack(|_| {}) + .categorize(&mut state_block) + .unpack(|_| {}) .commit(&topology) .unpack(|_| {}) - .expect("Block is valid"); + .unwrap(); let _events = state_block.apply(&vcb, topology.as_ref().to_owned())?; kura.store_block(vcb); diff --git a/crates/iroha_core/src/snapshot.rs b/crates/iroha_core/src/snapshot.rs index 11d979cdb08..bc0b37312f1 100644 --- a/crates/iroha_core/src/snapshot.rs +++ b/crates/iroha_core/src/snapshot.rs @@ -362,8 +362,8 @@ mod tests { kura.store_block(committed_block); let valid_block = - ValidBlock::new_dummy_and_modify_payload(peer_key_pair.private_key(), |block| { - block.header.height = block.header.height.checked_add(1).unwrap(); + ValidBlock::new_dummy_and_modify_header(peer_key_pair.private_key(), |header| { + header.height = header.height.checked_add(1).unwrap(); }); let committed_block = valid_block .clone() @@ -421,8 +421,8 @@ mod tests { kura.store_block(committed_block); let valid_block = - ValidBlock::new_dummy_and_modify_payload(peer_key_pair.private_key(), |block| { - block.header.height = block.header.height.checked_add(1).unwrap(); + ValidBlock::new_dummy_and_modify_header(peer_key_pair.private_key(), |header| { + header.height = header.height.checked_add(1).unwrap(); }); let committed_block = valid_block .clone() @@ -440,9 +440,9 @@ mod tests { // Store inside kura different block at the same height with different view change index so that block // This case imitate situation when snapshot was created for block which later is discarded as soft-fork let valid_block = - ValidBlock::new_dummy_and_modify_payload(peer_key_pair.private_key(), |block| { - block.header.height = block.header.height.checked_add(1).unwrap(); - block.header.view_change_index += 1; + ValidBlock::new_dummy_and_modify_header(peer_key_pair.private_key(), |header| { + header.height = header.height.checked_add(1).unwrap(); + header.view_change_index += 1; }); let committed_block = valid_block .clone() diff --git a/crates/iroha_core/src/state.rs b/crates/iroha_core/src/state.rs index 95b3439580d..6eb0c7e1fa1 100644 --- a/crates/iroha_core/src/state.rs +++ b/crates/iroha_core/src/state.rs @@ -2129,7 +2129,6 @@ pub(crate) mod deserialize { mod tests { use core::num::NonZeroU64; - use iroha_data_model::block::BlockPayload; use iroha_test_samples::gen_account_in; use super::*; @@ -2139,12 +2138,12 @@ mod tests { }; /// Used to inject faulty payload for testing - fn new_dummy_block_with_payload(f: impl FnOnce(&mut BlockPayload)) -> CommittedBlock { + fn new_dummy_block_with_payload(f: impl FnOnce(&mut BlockHeader)) -> CommittedBlock { let (leader_public_key, leader_private_key) = iroha_crypto::KeyPair::random().into_parts(); let peer_id = PeerId::new("127.0.0.1:8080".parse().unwrap(), leader_public_key); let topology = Topology::new(vec![peer_id]); - ValidBlock::new_dummy_and_modify_payload(&leader_private_key, f) + ValidBlock::new_dummy_and_modify_header(&leader_private_key, f) .commit(&topology) .unpack(|_| {}) .unwrap() @@ -2161,9 +2160,9 @@ mod tests { let mut block_hashes = vec![]; for i in 1..=BLOCK_CNT { - let block = new_dummy_block_with_payload(|payload| { - payload.header.height = NonZeroU64::new(i as u64).unwrap(); - payload.header.prev_block_hash = block_hashes.last().copied(); + let block = new_dummy_block_with_payload(|header| { + header.height = NonZeroU64::new(i as u64).unwrap(); + header.prev_block_hash = block_hashes.last().copied(); }); block_hashes.push(block.as_ref().hash()); @@ -2186,8 +2185,8 @@ mod tests { let mut state_block = state.block(); for i in 1..=BLOCK_CNT { - let block = new_dummy_block_with_payload(|payload| { - payload.header.height = NonZeroU64::new(i as u64).unwrap(); + let block = new_dummy_block_with_payload(|header| { + header.height = NonZeroU64::new(i as u64).unwrap(); }); let _events = state_block.apply(&block, Vec::new()).unwrap(); diff --git a/crates/iroha_core/src/sumeragi/main_loop.rs b/crates/iroha_core/src/sumeragi/main_loop.rs index d77bd70eeb8..2cdf071d362 100644 --- a/crates/iroha_core/src/sumeragi/main_loop.rs +++ b/crates/iroha_core/src/sumeragi/main_loop.rs @@ -282,7 +282,7 @@ impl Sumeragi { fn init_commit_genesis( &mut self, - genesis: GenesisBlock, + GenesisBlock(genesis): GenesisBlock, genesis_account: &AccountId, state: &State, ) { @@ -295,9 +295,13 @@ impl Sumeragi { } let mut state_block = state.block(); - state_block.world.genesis_creation_time_ms = Some(genesis.0.header().creation_time_ms); + state_block.world.genesis_creation_time_ms = Some(genesis.header().creation_time_ms); + + let msg = BlockCreated::from(&genesis); + self.broadcast_packet(msg); + let genesis = ValidBlock::validate( - genesis.0, + genesis, &self.topology, &self.chain_id, genesis_account, @@ -314,13 +318,10 @@ impl Sumeragi { // NOTE: By this time genesis block is executed and list of trusted peers is updated self.topology = Topology::new(state_block.world.trusted_peers_ids.clone()); - let msg = BlockCreated::from(&genesis); let genesis = genesis .commit(&self.topology) .unpack(|e| self.send_event(e)) .expect("Genesis invalid"); - - self.broadcast_packet(msg); self.commit_block(genesis, state_block); } @@ -386,20 +387,13 @@ impl Sumeragi { fn validate_block<'state>( &self, + block: SignedBlock, state: &'state State, topology: &Topology, genesis_account: &AccountId, - BlockCreated { block }: BlockCreated, existing_voting_block: &mut Option, ) -> Option> { - if state.view().height() == 1 && block.header().height.get() == 1 { - // Consider our peer has genesis, - // and some other peer has genesis and broadcast it to our peer, - // then we can ignore such genesis block because we already has genesis. - // Note: `ValidBlock::validate` also checks it, - // but we don't want warning to be printed since this is correct behaviour. - return None; - } + assert!(!block.header().is_genesis()); ValidBlock::validate_keep_voting_block( block, @@ -539,11 +533,11 @@ impl Sumeragi { } } } - (BlockMessage::BlockCreated(block_created), Role::ValidatingPeer) => { + (BlockMessage::BlockCreated(BlockCreated { block }), Role::ValidatingPeer) => { info!( peer_id=%self.peer_id, role=%self.role(), - block=%block_created.block.hash(), + block=%block.hash(), "Block received" ); @@ -552,13 +546,9 @@ impl Sumeragi { .is_consensus_required() .expect("INTERNAL BUG: Consensus required for validating peer"); - if let Some(mut v_block) = self.validate_block( - state, - topology, - genesis_account, - block_created, - voting_block, - ) { + if let Some(mut v_block) = + self.validate_block(block, state, topology, genesis_account, voting_block) + { v_block.block.sign(&self.key_pair, topology); let msg = BlockSigned::from(&v_block.block); @@ -573,11 +563,11 @@ impl Sumeragi { *voting_block = Some(v_block); } } - (BlockMessage::BlockCreated(block_created), Role::ObservingPeer) => { + (BlockMessage::BlockCreated(BlockCreated { block }), Role::ObservingPeer) => { info!( peer_id=%self.peer_id, role=%self.role(), - block=%block_created.block.hash(), + block=%block.hash(), "Block received" ); @@ -586,13 +576,9 @@ impl Sumeragi { .is_consensus_required() .expect("INTERNAL BUG: Consensus required for observing peer"); - if let Some(mut v_block) = self.validate_block( - state, - topology, - genesis_account, - block_created, - voting_block, - ) { + if let Some(mut v_block) = + self.validate_block(block, state, topology, genesis_account, voting_block) + { if view_change_index >= 1 { v_block.block.sign(&self.key_pair, topology); @@ -610,20 +596,16 @@ impl Sumeragi { *voting_block = Some(v_block); } } - (BlockMessage::BlockCreated(block_created), Role::ProxyTail) => { + (BlockMessage::BlockCreated(BlockCreated { block }), Role::ProxyTail) => { info!( peer_id=%self.peer_id, role=%self.role(), - block=%block_created.block.hash(), + block=%block.hash(), "Block received" ); - if let Some(mut valid_block) = self.validate_block( - state, - &self.topology, - genesis_account, - block_created, - voting_block, - ) { + if let Some(mut valid_block) = + self.validate_block(block, state, &self.topology, genesis_account, voting_block) + { // NOTE: Up until this point it was unknown which block is expected to be received, // therefore all the signatures (of any hash) were collected and will now be pruned for signature in core::mem::take(voting_signatures) { @@ -859,7 +841,6 @@ impl Sumeragi { fn try_create_block<'state>( &mut self, state: &'state State, - genesis_account: &AccountId, voting_block: &mut Option>, ) { assert_eq!(self.role(), Role::Leader); @@ -872,67 +853,56 @@ impl Sumeragi { .max_transactions .try_into() .expect("INTERNAL BUG: transactions in block exceed usize::MAX"); - let block_time = if self.topology.view_change_index() > 0 { - Duration::from_secs(0) - } else { - state.world.view().parameters.sumeragi.block_time() - }; + let tx_cache_full = self.transaction_cache.len() >= max_transactions.get(); + let view_change_in_progress = self.topology.view_change_index() > 0; + let block_time = state.world.view().parameters.sumeragi.block_time(); let deadline_reached = self.round_start_time.elapsed() > block_time; let tx_cache_non_empty = !self.transaction_cache.is_empty(); - if tx_cache_full || (deadline_reached && tx_cache_non_empty) { + if tx_cache_full || tx_cache_non_empty && (view_change_in_progress || deadline_reached) { let transactions = self .transaction_cache .iter() .map(|tx| tx.deref().clone()) .collect::>(); - let pre_signed_block = BlockBuilder::new_unverified( - state.view().latest_block().as_deref(), - self.topology.view_change_index(), - transactions, - ) - .sign(self.key_pair.private_key()); - - let block_created_msg = BlockCreated { - block: pre_signed_block, - }; - if self.topology.is_consensus_required().is_some() { - self.broadcast_packet(block_created_msg.clone()); - } + let mut state_block = state.block(); + let unverified_block = BlockBuilder::new(transactions) + .chain(self.topology.view_change_index(), &mut state_block) + .sign(self.key_pair.private_key()) + .unpack(|e| self.send_event(e)); info!( peer_id=%self.peer_id, + block_hash=%unverified_block.header.hash(), + txns=%unverified_block.transactions.len(), view_change_index=%self.topology.view_change_index(), - block_hash=%block_created_msg.block.hash(), - txns=%block_created_msg.block.transactions().len(), "Block created" ); - let new_voting_block = self - .validate_block( - state, - &self.topology, - genesis_account, - block_created_msg, - voting_block, - ) - .expect("We just created this block ourselves, it has to be valid."); - if self.topology.is_consensus_required().is_some() { - *voting_block = Some(new_voting_block); + let msg = BlockCreated::from(&unverified_block); + self.broadcast_packet(msg); + } + + let block = unverified_block + .categorize(&mut state_block) + .unpack(|e| self.send_event(e)); + + *voting_block = if self.topology.is_consensus_required().is_some() { + Some(VotingBlock::new(block, state_block)) } else { - let committed_block = new_voting_block - .block + let committed_block = block .commit(&self.topology) .unpack(|e| self.send_event(e)) - .expect("INTERNAL BUG: Leader failed to commit created block"); + .expect("INTERNAL BUG: Leader failed to commit block"); let msg = BlockCommitted::from(&committed_block); self.broadcast_packet(msg); - self.commit_block(committed_block, new_voting_block.state_block); - *voting_block = None; + self.commit_block(committed_block, state_block); + + None } } } @@ -1136,7 +1106,8 @@ pub(crate) fn run( // We broadcast our view change suggestion after having processed the latest from others inside `receive_network_packet` let block_expected = !sumeragi.transaction_cache.is_empty(); - if (block_expected || view_change_index > 0) + let view_change_in_progress = view_change_index > 0; + if (block_expected || view_change_in_progress) && last_view_change_time.elapsed() > view_change_time { if block_expected { @@ -1225,7 +1196,7 @@ pub(crate) fn run( .set(sumeragi.topology.view_change_index() as u64); if sumeragi.role() == Role::Leader && voting_block.is_none() { - sumeragi.try_create_block(&state, &genesis_account, &mut voting_block); + sumeragi.try_create_block(&state, &mut voting_block); } } } @@ -1416,6 +1387,7 @@ fn categorize_block_sync( #[cfg(test)] mod tests { + use iroha_crypto::SignatureOf; use iroha_data_model::{isi::InstructionBox, transaction::TransactionBuilder}; use iroha_genesis::GENESIS_DOMAIN_ID; use iroha_test_samples::gen_account_in; @@ -1426,22 +1398,26 @@ mod tests { use crate::{query::store::LiveQueryStore, smartcontracts::Registrable}; /// Used to inject faulty payload for testing - fn clone_and_modify_payload( - block: &SignedBlock, + fn clone_and_modify_header( + block: &NewBlock, private_key: &PrivateKey, - f: impl FnOnce(&mut BlockPayload), - ) -> SignedBlock { - let mut payload = block.payload().clone(); - f(&mut payload); - - payload.sign(private_key) + f: impl FnOnce(&mut BlockHeader), + ) -> NewBlock { + let mut header = block.header.clone(); + f(&mut header); + + NewBlock { + signature: BlockSignature(0, SignatureOf::new(private_key, &header)), + header, + transactions: block.transactions.clone(), + } } fn create_data_for_test( chain_id: &ChainId, topology: &Topology, leader_private_key: &PrivateKey, - ) -> (State, Arc, SignedBlock, AccountId) { + ) -> (State, Arc, NewBlock, AccountId) { // Predefined world state let (alice_id, alice_keypair) = gen_account_in("wonderland"); let genesis_account = AccountId::new( @@ -1489,15 +1465,16 @@ mod tests { .expect("Valid"); // Creating a block of two identical transactions and validating it - let block = BlockBuilder::new(vec![peers, tx.clone(), tx]) + let genesis = BlockBuilder::new(vec![peers, tx.clone(), tx]) .chain(0, &mut state_block) .sign(leader_private_key) - .unpack(|_| {}); - - let genesis = block + .unpack(|_| {}) + .categorize(&mut state_block) + .unpack(|_| {}) .commit(topology) .unpack(|_| {}) .expect("Block is valid"); + let _events = state_block.apply_without_execution(&genesis, topology.as_ref().to_owned()); state_block.commit(); kura.store_block(genesis); @@ -1532,11 +1509,10 @@ mod tests { .unpack(|_| {}) }; - (state, kura, block.into(), genesis_account) + (state, kura, block, genesis_account) } #[test] - #[allow(clippy::redundant_clone)] async fn block_sync_invalid_block() { let chain_id = ChainId::from("00000000-0000-0000-0000-000000000000"); @@ -1547,10 +1523,10 @@ mod tests { create_data_for_test(&chain_id, &topology, &leader_private_key); // Malform block to make it invalid - let block = clone_and_modify_payload(&block, &leader_private_key, |payload| { - payload.header.prev_block_hash = - Some(HashOf::from_untyped_unchecked(Hash::new([1; 32]))); - }); + let block = clone_and_modify_header(&block, &leader_private_key, |header| { + header.prev_block_hash = Some(HashOf::from_untyped_unchecked(Hash::new([1; 32]))); + }) + .into(); let result = handle_block_sync(&chain_id, block, &state, &genesis_public_key, &|_| {}); assert!(matches!(result, Err((_, BlockSyncError::BlockNotValid(_))))) @@ -1563,33 +1539,28 @@ mod tests { let (leader_public_key, leader_private_key) = KeyPair::random().into_parts(); let peer_id = PeerId::new("127.0.0.1:8080".parse().unwrap(), leader_public_key); let topology = Topology::new(vec![peer_id]); - let (state, kura, block, genesis_public_key) = + let (state, kura, unverified_block, genesis_public_key) = create_data_for_test(&chain_id, &topology, &leader_private_key); let mut state_block = state.block(); - let committed_block = ValidBlock::validate( - block.clone(), - &topology, - &chain_id, - &genesis_public_key, - &mut state_block, - ) - .unpack(|_| {}) - .unwrap() - .commit(&topology) - .unpack(|_| {}) - .expect("Block is valid"); + let committed_block = unverified_block + .clone() + .categorize(&mut state_block) + .unpack(|_| {}) + .commit(&topology) + .unpack(|_| {}) + .expect("Block is valid"); let _events = state_block.apply_without_execution(&committed_block, topology.as_ref().to_owned()); state_block.commit(); kura.store_block(committed_block); // Malform block to make it invalid - let block = clone_and_modify_payload(&block, &leader_private_key, |payload| { - payload.header.prev_block_hash = - Some(HashOf::from_untyped_unchecked(Hash::new([1; 32]))); - payload.header.view_change_index = 1; - }); + let block = clone_and_modify_header(&unverified_block, &leader_private_key, |header| { + header.prev_block_hash = Some(HashOf::from_untyped_unchecked(Hash::new([1; 32]))); + header.view_change_index = 1; + }) + .into(); let result = handle_block_sync(&chain_id, block, &state, &genesis_public_key, &|_| {}); assert!(matches!( @@ -1599,7 +1570,6 @@ mod tests { } #[test] - #[allow(clippy::redundant_clone)] async fn block_sync_not_proper_height() { let chain_id = ChainId::from("00000000-0000-0000-0000-000000000000"); @@ -1610,9 +1580,10 @@ mod tests { create_data_for_test(&chain_id, &topology, &leader_private_key); // Change block height - let block = clone_and_modify_payload(&block, &leader_private_key, |payload| { - payload.header.height = nonzero!(42_u64); - }); + let block = clone_and_modify_header(&block, &leader_private_key, |header| { + header.height = nonzero!(42_u64); + }) + .into(); let result = handle_block_sync(&chain_id, block, &state, &genesis_public_key, &|_| {}); @@ -1634,7 +1605,6 @@ mod tests { } #[test] - #[allow(clippy::redundant_clone)] async fn block_sync_commit_block() { let chain_id = ChainId::from("00000000-0000-0000-0000-000000000000"); @@ -1643,7 +1613,13 @@ mod tests { let topology = Topology::new(vec![peer_id]); let (state, _, block, genesis_public_key) = create_data_for_test(&chain_id, &topology, &leader_private_key); - let result = handle_block_sync(&chain_id, block, &state, &genesis_public_key, &|_| {}); + let result = handle_block_sync( + &chain_id, + block.into(), + &state, + &genesis_public_key, + &|_| {}, + ); assert!(matches!(result, Ok(BlockSyncOk::CommitBlock(_, _, _)))) } @@ -1654,38 +1630,31 @@ mod tests { let (leader_public_key, leader_private_key) = KeyPair::random().into_parts(); let peer_id = PeerId::new("127.0.0.1:8080".parse().unwrap(), leader_public_key); let topology = Topology::new(vec![peer_id]); - let (state, kura, block, genesis_public_key) = + let (state, kura, unverified_block, genesis_public_key) = create_data_for_test(&chain_id, &topology, &leader_private_key); let mut state_block = state.block(); - let committed_block = ValidBlock::validate( - block.clone(), - &topology, - &chain_id, - &genesis_public_key, - &mut state_block, - ) - .unpack(|_| {}) - .unwrap() - .commit(&topology) - .unpack(|_| {}) - .expect("Block is valid"); + let committed_block = unverified_block + .clone() + .categorize(&mut state_block) + .unpack(|_| {}) + .commit(&topology) + .unpack(|_| {}) + .unwrap(); let _events = state_block.apply_without_execution(&committed_block, topology.as_ref().to_owned()); state_block.commit(); kura.store_block(committed_block); - let latest_block = state - .view() - .latest_block() - .expect("INTERNAL BUG: No latest block"); + let latest_block = state.view().latest_block().unwrap(); let latest_block_view_change_index = latest_block.header().view_change_index; assert_eq!(latest_block_view_change_index, 0); // Increase block view change index - let block = clone_and_modify_payload(&block, &leader_private_key, |payload| { - payload.header.view_change_index = 42; - }); + let block = clone_and_modify_header(&unverified_block, &leader_private_key, |header| { + header.view_change_index = 42; + }) + .into(); let result = handle_block_sync(&chain_id, block, &state, &genesis_public_key, &|_| {}); assert!(matches!(result, Ok(BlockSyncOk::ReplaceTopBlock(_, _, _)))) @@ -1702,23 +1671,18 @@ mod tests { create_data_for_test(&chain_id, &topology, &leader_private_key); // Increase block view change index - let block = clone_and_modify_payload(&block, &leader_private_key, |payload| { - payload.header.view_change_index = 42; + let unverified_block = clone_and_modify_header(&block, &leader_private_key, |header| { + header.view_change_index = 42; }); let mut state_block = state.block(); - let committed_block = ValidBlock::validate( - block.clone(), - &topology, - &chain_id, - &genesis_public_key, - &mut state_block, - ) - .unpack(|_| {}) - .unwrap() - .commit(&topology) - .unpack(|_| {}) - .expect("Block is valid"); + let committed_block = unverified_block + .clone() + .categorize(&mut state_block) + .unpack(|_| {}) + .commit(&topology) + .unpack(|_| {}) + .expect("Block is valid"); let _events = state_block.apply_without_execution(&committed_block, topology.as_ref().to_owned()); state_block.commit(); @@ -1731,9 +1695,10 @@ mod tests { assert_eq!(latest_block_view_change_index, 42); // Decrease block view change index back - let block = clone_and_modify_payload(&block, &leader_private_key, |payload| { - payload.header.view_change_index = 0; - }); + let block = clone_and_modify_header(&unverified_block, &leader_private_key, |header| { + header.view_change_index = 0; + }) + .into(); let result = handle_block_sync(&chain_id, block, &state, &genesis_public_key, &|_| {}); assert!(matches!( @@ -1749,7 +1714,6 @@ mod tests { } #[test] - #[allow(clippy::redundant_clone)] async fn block_sync_genesis_block_do_not_replace() { let chain_id = ChainId::from("00000000-0000-0000-0000-000000000000"); @@ -1761,10 +1725,11 @@ mod tests { // Change block height and view change index // Soft-fork on genesis block is not possible - let block = clone_and_modify_payload(&block, &leader_private_key, |payload| { - payload.header.view_change_index = 42; - payload.header.height = nonzero!(1_u64); - }); + let block = clone_and_modify_header(&block, &leader_private_key, |header| { + header.view_change_index = 42; + header.height = nonzero!(1_u64); + }) + .into(); let result = handle_block_sync(&chain_id, block, &state, &genesis_public_key, &|_| {}); @@ -1786,19 +1751,25 @@ mod tests { } #[test] - #[allow(clippy::redundant_clone)] async fn block_sync_commit_err_keep_voting_block() { let chain_id = ChainId::from("00000000-0000-0000-0000-000000000000"); let (leader_public_key, leader_private_key) = KeyPair::random().into_parts(); let peer_id = PeerId::new("127.0.0.1:8080".parse().unwrap(), leader_public_key); let topology = Topology::new(vec![peer_id]); - let (state, _, mut block, genesis_public_key) = + let (state, _, unverified_block, genesis_public_key) = create_data_for_test(&chain_id, &topology, &leader_private_key); + let valid_block = unverified_block + .categorize(&mut state.block()) + .unpack(|_| {}); // Malform block signatures so that block going to be rejected - block.replace_signatures_unchecked(Vec::new()); - + let dummy_signature = BlockSignature( + 42, + valid_block.as_ref().signatures().next().unwrap().1.clone(), + ); + let mut block: SignedBlock = valid_block.into(); + let _prev_signatures = block.replace_signatures(vec![dummy_signature]).unwrap(); let mut voting_block = Some(VotingBlock::new( ValidBlock::new_dummy(&leader_private_key), state.block(), diff --git a/crates/iroha_core/src/sumeragi/message.rs b/crates/iroha_core/src/sumeragi/message.rs index 6e6cdf8195b..94ee7503f91 100644 --- a/crates/iroha_core/src/sumeragi/message.rs +++ b/crates/iroha_core/src/sumeragi/message.rs @@ -5,7 +5,7 @@ use iroha_macro::*; use parity_scale_codec::{Decode, Encode}; use super::view_change; -use crate::block::{CommittedBlock, ValidBlock}; +use crate::block::{CommittedBlock, NewBlock, ValidBlock}; #[allow(clippy::enum_variant_names)] /// Message's variants that are used by peers to communicate in the process of consensus. @@ -43,8 +43,8 @@ pub struct BlockCreated { pub block: SignedBlock, } -impl From<&ValidBlock> for BlockCreated { - fn from(block: &ValidBlock) -> Self { +impl From<&NewBlock> for BlockCreated { + fn from(block: &NewBlock) -> Self { Self { // TODO: Redundant clone block: block.clone().into(), @@ -52,6 +52,15 @@ impl From<&ValidBlock> for BlockCreated { } } +impl From<&SignedBlock> for BlockCreated { + fn from(block: &SignedBlock) -> Self { + Self { + // TODO: Redundant clone + block: block.clone(), + } + } +} + /// `BlockSigned` message structure. #[derive(Debug, Clone, Decode, Encode)] pub struct BlockSigned { diff --git a/crates/iroha_core/src/tx.rs b/crates/iroha_core/src/tx.rs index fe51896b572..2d6e672f417 100644 --- a/crates/iroha_core/src/tx.rs +++ b/crates/iroha_core/src/tx.rs @@ -29,8 +29,8 @@ use crate::{ /// `AcceptedTransaction` — a transaction accepted by Iroha peer. #[derive(Debug, Clone, PartialEq, Eq)] -// FIX: Inner field should be private to maintain invariants -pub struct AcceptedTransaction(pub(crate) SignedTransaction); +#[repr(transparent)] +pub struct AcceptedTransaction(pub(super) SignedTransaction); /// Verification failed of some signature due to following reason #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/iroha_data_model/src/block.rs b/crates/iroha_data_model/src/block.rs index bf227a2f1df..4f060e0be67 100644 --- a/crates/iroha_data_model/src/block.rs +++ b/crates/iroha_data_model/src/block.rs @@ -42,11 +42,7 @@ mod model { Serialize, IntoSchema, )] - #[cfg_attr( - feature = "std", - display(fmt = "Block №{height} (hash: {});", "HashOf::new(&self)") - )] - #[cfg_attr(not(feature = "std"), display(fmt = "Block №{height}"))] + #[display(fmt = "{} (№{height})", "self.hash()")] #[allow(missing_docs)] #[ffi_type] pub struct BlockHeader { @@ -105,7 +101,7 @@ mod model { #[derive( Debug, Display, Clone, PartialEq, Eq, PartialOrd, Ord, Encode, Serialize, IntoSchema, )] - #[display(fmt = "{}", "self.hash()")] + #[display(fmt = "{}", "self.header()")] #[ffi_type] pub struct SignedBlockV1 { /// Signatures of peers which approved this block. @@ -140,30 +136,61 @@ impl BlockHeader { } } -impl BlockPayload { - /// Create new signed block, using `key_pair` to sign `payload` +impl SignedBlockV1 { + fn hash(&self) -> HashOf { + self.payload.header.hash() + } + + fn header(&self) -> &BlockHeader { + &self.payload.header + } +} + +impl SignedBlock { + /// Create new block with a given signature + /// + /// # Warning + /// + /// All transactions are categorized as valid #[cfg(feature = "transparent_api")] - pub fn sign(self, private_key: &iroha_crypto::PrivateKey) -> SignedBlock { - let signatures = vec![BlockSignature( - 0, - SignatureOf::new(private_key, &self.header), - )]; + pub fn presigned( + signature: BlockSignature, + header: BlockHeader, + transactions: impl IntoIterator, + ) -> SignedBlock { + let transactions = transactions + .into_iter() + .map(|tx| CommittedTransaction { + value: tx, + error: None, + }) + .collect(); SignedBlockV1 { - signatures, - payload: self, + signatures: vec![signature], + payload: BlockPayload { + header, + transactions, + }, } .into() } -} -impl SignedBlockV1 { - fn hash(&self) -> iroha_crypto::HashOf { - self.payload.header.hash() + /// Setter for transaction errors + #[cfg(feature = "transparent_api")] + pub fn set_transaction_errors( + &mut self, + errors: impl IntoIterator, + ) -> &mut Self { + let SignedBlock::V1(block) = self; + + for (tx, error) in errors { + block.payload.transactions[tx].error = Some(Box::new(error)); + } + + self } -} -impl SignedBlock { /// Block payload. Used for tests #[cfg(feature = "transparent_api")] pub fn payload(&self) -> &BlockPayload { @@ -185,13 +212,6 @@ impl SignedBlock { block.payload.transactions.iter() } - /// Block transactions with mutable access - #[inline] - pub fn transactions_mut(&mut self) -> &mut [CommittedTransaction] { - let SignedBlock::V1(block) = self; - block.payload.transactions.as_mut_slice() - } - /// Signatures of peers which approved this block. #[inline] pub fn signatures( @@ -208,25 +228,30 @@ impl SignedBlock { block.hash() } + /// Add additional signature to this block + #[cfg(feature = "transparent_api")] + pub fn sign(&mut self, private_key: &iroha_crypto::PrivateKey, signatory: usize) { + let SignedBlock::V1(block) = self; + + block.signatures.push(BlockSignature( + signatory as u64, + SignatureOf::new(private_key, &block.payload.header), + )); + } + /// Add signature to the block /// /// # Errors /// /// if signature is invalid #[cfg(feature = "transparent_api")] - pub fn add_signature( - &mut self, - signature: BlockSignature, - public_key: &iroha_crypto::PublicKey, - ) -> Result<(), iroha_crypto::Error> { + pub fn add_signature(&mut self, signature: BlockSignature) -> Result<(), iroha_crypto::Error> { if self.signatures().any(|s| signature.0 == s.0) { return Err(iroha_crypto::Error::Signing( "Duplicate signature".to_owned(), )); } - signature.1.verify(public_key, &self.payload().header)?; - let SignedBlock::V1(block) = self; block.signatures.push(signature); @@ -234,41 +259,56 @@ impl SignedBlock { } /// Replace signatures without verification + /// + /// # Errors + /// + /// if there is a duplicate signature #[cfg(feature = "transparent_api")] - pub fn replace_signatures_unchecked( + pub fn replace_signatures( &mut self, signatures: Vec, - ) -> Vec { - let SignedBlock::V1(block) = self; - std::mem::replace(&mut block.signatures, signatures) - } + ) -> Result, iroha_crypto::Error> { + #[cfg(not(feature = "std"))] + use alloc::collections::BTreeSet; + #[cfg(feature = "std")] + use std::collections::BTreeSet; + + if signatures.is_empty() { + return Err(iroha_crypto::Error::Signing("Signatures empty".to_owned())); + } - /// Add additional signatures to this block - #[cfg(all(feature = "std", feature = "transparent_api"))] - pub fn sign(&mut self, private_key: &iroha_crypto::PrivateKey, signatory: usize) { - let SignedBlock::V1(block) = self; + signatures.iter().map(|signature| signature.0).try_fold( + BTreeSet::new(), + |mut acc, elem| { + if !acc.insert(elem) { + return Err(iroha_crypto::Error::Signing(format!( + "{elem}: Duplicate signature" + ))); + } - block.signatures.push(BlockSignature( - signatory as u64, - SignatureOf::new(private_key, &block.payload.header), - )); + Ok(acc) + }, + )?; + + let SignedBlock::V1(block) = self; + Ok(core::mem::replace(&mut block.signatures, signatures)) } /// Creates genesis block signed with genesis private key (and not signed by any peer) #[cfg(feature = "std")] pub fn genesis( - genesis_transactions: Vec, - genesis_private_key: &iroha_crypto::PrivateKey, + transactions: Vec, + private_key: &iroha_crypto::PrivateKey, ) -> SignedBlock { use nonzero_ext::nonzero; - let transactions_hash = genesis_transactions + let transactions_hash = transactions .iter() .map(SignedTransaction::hash) .collect::>() .hash() .expect("Tree is not empty"); - let creation_time_ms = Self::get_genesis_block_creation_time(&genesis_transactions); + let creation_time_ms = Self::get_genesis_block_creation_time(&transactions); let header = BlockHeader { height: nonzero!(1_u64), prev_block_hash: None, @@ -276,7 +316,7 @@ impl SignedBlock { creation_time_ms, view_change_index: 0, }; - let transactions = genesis_transactions + let transactions = transactions .into_iter() .map(|transaction| CommittedTransaction { value: transaction, @@ -284,12 +324,12 @@ impl SignedBlock { }) .collect(); + let signature = BlockSignature(0, SignatureOf::new(private_key, &header)); let payload = BlockPayload { header, transactions, }; - let signature = BlockSignature(0, SignatureOf::new(genesis_private_key, &payload.header)); SignedBlockV1 { signatures: vec![signature], payload, diff --git a/crates/iroha_data_model/src/events/pipeline.rs b/crates/iroha_data_model/src/events/pipeline.rs index 8d593e3ce35..90ffa165461 100644 --- a/crates/iroha_data_model/src/events/pipeline.rs +++ b/crates/iroha_data_model/src/events/pipeline.rs @@ -103,6 +103,8 @@ mod model { )] #[ffi_type(opaque)] pub enum BlockStatus { + /// Block created (only emitted by the leader node) + Created, /// Block was approved to participate in consensus Approved, /// Block was rejected by consensus diff --git a/crates/iroha_data_model/src/lib.rs b/crates/iroha_data_model/src/lib.rs index b0f296f2845..b35e9e6ddbf 100644 --- a/crates/iroha_data_model/src/lib.rs +++ b/crates/iroha_data_model/src/lib.rs @@ -11,7 +11,6 @@ //! This gives about 50% performance boost, see #4995. // Clippy bug -#![allow(clippy::items_after_test_module)] #![cfg_attr(not(feature = "std"), no_std)] #[cfg(not(feature = "std"))] diff --git a/crates/iroha_data_model/src/transaction.rs b/crates/iroha_data_model/src/transaction.rs index 6bd5efb4cc5..c050310de39 100644 --- a/crates/iroha_data_model/src/transaction.rs +++ b/crates/iroha_data_model/src/transaction.rs @@ -144,8 +144,7 @@ mod model { #[derive( Debug, Display, Clone, PartialEq, Eq, PartialOrd, Ord, Encode, Serialize, IntoSchema, )] - #[cfg_attr(not(feature = "std"), display(fmt = "Signed transaction"))] - #[cfg_attr(feature = "std", display(fmt = "{}", "self.hash()"))] + #[display(fmt = "{}", "self.hash()")] #[ffi_type] pub struct SignedTransactionV1 { /// Signature of [`Self::payload`]. @@ -305,7 +304,6 @@ impl From for (AccountId, Executable) { } impl SignedTransactionV1 { - #[cfg(feature = "std")] fn hash(&self) -> iroha_crypto::HashOf { iroha_crypto::HashOf::from_untyped_unchecked(iroha_crypto::HashOf::new(self).into()) } diff --git a/crates/iroha_ffi/src/ir.rs b/crates/iroha_ffi/src/ir.rs index 7b6ef5cbd16..3d84f84a7ac 100644 --- a/crates/iroha_ffi/src/ir.rs +++ b/crates/iroha_ffi/src/ir.rs @@ -51,9 +51,10 @@ pub unsafe trait Transmute { unsafe fn is_valid(target: &Self::Target) -> bool; } -/// Marker trait for a type whose [`Transmute::is_valid`] always returns true. The main -/// use of this trait is to guard against the use of `&mut T` in FFI where the caller -/// can set the underlying `T` to a trap representation and cause UB. +/// Marker trait for a type whose [`Transmute::is_valid`] always returns true. +/// +/// Main use of this trait is to guard against the use of `&mut T` in FFI where +/// the caller can set the underlying `T` to a trap representation and cause UB. /// /// # Safety /// diff --git a/crates/iroha_schema/tests/numbers_compact_and_fixed.rs b/crates/iroha_schema/tests/numbers_compact_and_fixed.rs index 107e85321b0..2cb79d462fc 100644 --- a/crates/iroha_schema/tests/numbers_compact_and_fixed.rs +++ b/crates/iroha_schema/tests/numbers_compact_and_fixed.rs @@ -25,6 +25,7 @@ struct Foo { } #[test] +#[expect(clippy::too_many_lines)] fn compact() { use alloc::collections::BTreeMap; diff --git a/crates/iroha_schema_derive/src/lib.rs b/crates/iroha_schema_derive/src/lib.rs index b1c8b40753f..fe91c788674 100644 --- a/crates/iroha_schema_derive/src/lib.rs +++ b/crates/iroha_schema_derive/src/lib.rs @@ -17,7 +17,7 @@ fn override_where_clause( ) -> Option { bounds .and_then(|bounds| emitter.handle(syn::parse_str(&format!("where {bounds}")))) - .unwrap_or(where_clause.cloned()) + .unwrap_or_else(|| where_clause.cloned()) } /// Derive [`iroha_schema::TypeId`] diff --git a/crates/iroha_wasm_builder/src/lib.rs b/crates/iroha_wasm_builder/src/lib.rs index 0311a05b95a..ae088d1c916 100644 --- a/crates/iroha_wasm_builder/src/lib.rs +++ b/crates/iroha_wasm_builder/src/lib.rs @@ -364,7 +364,7 @@ impl Output { fn cargo_command() -> Command { const INSTRUMENT_COVERAGE_FLAG: &str = "instrument-coverage"; for var in ["RUSTFLAGS", "CARGO_ENCODED_RUSTFLAGS"] { - if let Some(value) = env::var(var).ok() { + if let Ok(value) = env::var(var) { if value.contains(INSTRUMENT_COVERAGE_FLAG) { eprintln!("WARNING: found `{INSTRUMENT_COVERAGE_FLAG}` rustc flag in `{var}` environment variable\n \ This directly interferes with `-Z build-std` flag set by `iroha_wasm_builder`\n \ diff --git a/docs/source/references/schema.json b/docs/source/references/schema.json index 137fc358ffb..40709192242 100644 --- a/docs/source/references/schema.json +++ b/docs/source/references/schema.json @@ -741,21 +741,25 @@ "BlockStatus": { "Enum": [ { - "tag": "Approved", + "tag": "Created", "discriminant": 0 }, + { + "tag": "Approved", + "discriminant": 1 + }, { "tag": "Rejected", - "discriminant": 1, + "discriminant": 2, "type": "BlockRejectionReason" }, { "tag": "Committed", - "discriminant": 2 + "discriminant": 3 }, { "tag": "Applied", - "discriminant": 3 + "discriminant": 4 } ] },