diff --git a/crates/iroha_core/src/block.rs b/crates/iroha_core/src/block.rs index 4634c4c9bd4..a4f1450c5d6 100644 --- a/crates/iroha_core/src/block.rs +++ b/crates/iroha_core/src/block.rs @@ -153,6 +153,31 @@ mod pending { Self(Pending { transactions }) } + /// Create new BlockPayload + pub fn new_unverified( + prev_block: Option<&SignedBlock>, + view_change_index: usize, + transactions_a: Vec, + consensus_estimation: Duration, + ) -> 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, + consensus_estimation, + ), + transactions, + } + } + fn make_header( prev_block: Option<&SignedBlock>, view_change_index: usize, @@ -264,7 +289,7 @@ mod chained { pub struct Chained(pub(super) BlockPayload); impl BlockBuilder { - /// Sign this block and get [`SignedBlock`]. + /// 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))) } @@ -415,7 +440,8 @@ mod valid { Ok(()) } - /// Validate a block against the current state of the world. + /// Validate a block against the current state of the world. Individual transaction + /// errors will be updated. /// /// # Errors /// @@ -430,7 +456,7 @@ mod valid { /// - Error during validation of individual transactions /// - Transaction in the genesis block is not signed by the genesis public key pub fn validate( - block: SignedBlock, + mut block: SignedBlock, topology: &Topology, expected_chain_id: &ChainId, genesis_account: &AccountId, @@ -442,9 +468,12 @@ mod valid { return WithEvents::new(Err((block, error))); } - if let Err(error) = - Self::validate_transactions(&block, expected_chain_id, genesis_account, state_block) - { + if let Err(error) = Self::validate_transactions( + &mut block, + expected_chain_id, + genesis_account, + state_block, + ) { return WithEvents::new(Err((block, error.into()))); } @@ -456,7 +485,7 @@ mod valid { /// * If block header is valid, `voting_block` will be released, /// and transactions will be validated with write state pub fn validate_keep_voting_block<'state>( - block: SignedBlock, + mut block: SignedBlock, topology: &Topology, expected_chain_id: &ChainId, genesis_account: &AccountId, @@ -480,7 +509,7 @@ mod valid { }; if let Err(error) = Self::validate_transactions( - &block, + &mut block, expected_chain_id, genesis_account, &mut state_block, @@ -577,7 +606,7 @@ mod valid { } fn validate_transactions( - block: &SignedBlock, + block: &mut SignedBlock, expected_chain_id: &ChainId, genesis_account: &AccountId, state_block: &mut StateBlock<'_>, @@ -589,40 +618,29 @@ mod valid { (params.sumeragi().max_clock_drift(), params.transaction) }; - block - .transactions() - // TODO: Unnecessary clone? - .cloned() - .try_for_each(|CommittedTransaction { value, error }| { - let tx = if is_genesis { - AcceptedTransaction::accept_genesis( - value, - expected_chain_id, - max_clock_drift, - genesis_account, - ) - } else { - AcceptedTransaction::accept( - value, - expected_chain_id, - max_clock_drift, - tx_limits, - ) - }?; - - if error.is_some() { - match state_block.validate(tx) { - Err(rejected_transaction) => Ok(rejected_transaction), - Ok(_) => Err(TransactionValidationError::RejectedIsValid), - }?; - } else { - state_block - .validate(tx) - .map_err(|(_tx, error)| TransactionValidationError::NotValid(error))?; - } + 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, + ) + }?; - Ok(()) - }) + *error = match state_block.validate(tx) { + Ok(_) => None, + Err((_tx, error)) => Some(Box::new(error)), + }; + } + Ok(()) } /// Add additional signature for [`Self`] diff --git a/crates/iroha_core/src/sumeragi/main_loop.rs b/crates/iroha_core/src/sumeragi/main_loop.rs index 5183728ef81..36ee077e493 100644 --- a/crates/iroha_core/src/sumeragi/main_loop.rs +++ b/crates/iroha_core/src/sumeragi/main_loop.rs @@ -595,7 +595,6 @@ impl Sumeragi { block=%block_created.block.hash(), "Block received" ); - if let Some(mut valid_block) = self.validate_block( state, &self.topology, @@ -605,7 +604,6 @@ impl Sumeragi { ) { // 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) { if let Err(error) = valid_block.block.add_signature(signature, &self.topology) @@ -827,6 +825,7 @@ 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); @@ -839,7 +838,11 @@ impl Sumeragi { .max_transactions .try_into() .expect("INTERNAL BUG: transactions in block exceed usize::MAX"); - let block_time = state.world.view().parameters.sumeragi.block_time(); + 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 deadline_reached = self.round_start_time.elapsed() > block_time; let tx_cache_non_empty = !self.transaction_cache.is_empty(); @@ -851,47 +854,57 @@ impl Sumeragi { .map(|tx| tx.deref().clone()) .collect::>(); - let mut state_block = state.block(); - let create_block_start_time = Instant::now(); - let new_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)); - - let created_in = create_block_start_time.elapsed(); - let pipeline_time = state.world.view().parameters().sumeragi.pipeline_time(); - if created_in > pipeline_time / 2 { - warn!( - role=%self.role(), - peer_id=%self.peer_id, - "Creating block takes too much time. \ - This might prevent consensus from operating. \ - Consider increasing `commit_time` or decreasing `max_transactions_in_block`" - ); - } + let pre_signed_block = BlockBuilder::new_unverified( + state.view().latest_block().as_deref(), + self.topology.view_change_index(), + transactions, + state + .view() + .world + .parameters() + .sumeragi + .consensus_estimation(), + ) + .sign(self.key_pair.private_key()); + let block_created_msg = BlockCreated { + block: pre_signed_block, + }; if self.topology.is_consensus_required().is_some() { - info!( - peer_id=%self.peer_id, - block=%new_block.as_ref().hash(), - view_change_index=%self.topology.view_change_index(), - txns=%new_block.as_ref().transactions().len(), - created_in_ms=%created_in.as_millis(), - "Block created" - ); + self.broadcast_packet(block_created_msg.clone()); + } - let msg = BlockCreated::from(&new_block); - *voting_block = Some(VotingBlock::new(new_block, state_block)); - self.broadcast_packet(msg); + info!( + peer_id=%self.peer_id, + 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); } else { - let committed_block = new_block + let committed_block = new_voting_block + .block .commit(&self.topology) .unpack(|e| self.send_event(e)) .expect("INTERNAL BUG: Leader failed to commit created block"); let msg = BlockCommitted::from(&committed_block); self.broadcast_packet(msg); - self.commit_block(committed_block, state_block); + self.commit_block(committed_block, new_voting_block.state_block); + *voting_block = None; } } } @@ -1009,7 +1022,6 @@ pub(crate) fn run( let _enter_for_sumeragi_cycle = span_for_sumeragi_cycle.enter(); let state_view = state.view(); - sumeragi .transaction_cache // Checking if transactions are in the blockchain is costly @@ -1167,7 +1179,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, &mut voting_block); + sumeragi.try_create_block(&state, &genesis_account, &mut voting_block); } } } diff --git a/crates/iroha_data_model/src/block.rs b/crates/iroha_data_model/src/block.rs index 4306fba2188..13b006888c8 100644 --- a/crates/iroha_data_model/src/block.rs +++ b/crates/iroha_data_model/src/block.rs @@ -185,6 +185,13 @@ 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(