From d577a948ea4e584ffec8c70769b971fc283ca452 Mon Sep 17 00:00:00 2001 From: 0x009922 <43530070+0x009922@users.noreply.github.com> Date: Thu, 6 Jun 2024 11:00:14 +0900 Subject: [PATCH] refactor: fix application of the core chain-wide parameters; chores Signed-off-by: 0x009922 <43530070+0x009922@users.noreply.github.com> --- config/src/parameters/actual.rs | 5 ++ core/src/block.rs | 12 ++- core/src/query/store.rs | 2 +- core/src/smartcontracts/isi/world.rs | 21 ++--- core/src/state.rs | 128 ++++++++++++++++++++------- core/src/sumeragi/main_loop.rs | 17 +--- data_model/src/block.rs | 7 +- 7 files changed, 127 insertions(+), 65 deletions(-) diff --git a/config/src/parameters/actual.rs b/config/src/parameters/actual.rs index 893a77592ab..f293923bcad 100644 --- a/config/src/parameters/actual.rs +++ b/config/src/parameters/actual.rs @@ -207,6 +207,11 @@ impl ChainWide { pub fn pipeline_time(&self) -> Duration { self.block_time + self.commit_time } + + /// Estimates as `block_time + commit_time / 2` + pub fn consensus_estimation(&self) -> Duration { + self.block_time + (self.commit_time / 2) + } } impl Default for ChainWide { diff --git a/core/src/block.rs b/core/src/block.rs index 711ef6431d8..9470265c398 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -6,7 +6,6 @@ //! [`Block`]s are organised into a linear sequence over time (also known as the block chain). use std::error::Error as _; -use iroha_config::parameters::defaults::chain_wide::CONSENSUS_ESTIMATION as DEFAULT_CONSENSUS_ESTIMATION; use iroha_crypto::{HashOf, KeyPair, MerkleTree, SignatureOf, SignaturesOf}; use iroha_data_model::{ block::*, @@ -101,7 +100,7 @@ pub enum SignatureVerificationError { pub struct BlockBuilder(B); mod pending { - use std::time::SystemTime; + use std::time::{Duration, SystemTime}; use iroha_data_model::transaction::CommittedTransaction; @@ -149,6 +148,7 @@ mod pending { prev_block_hash: Option>, view_change_index: u64, transactions: &[CommittedTransaction], + consensus_estimation: Duration, ) -> BlockHeader { BlockHeader { height: previous_height + 1, @@ -165,7 +165,7 @@ mod pending { .try_into() .expect("Time should fit into u64"), view_change_index, - consensus_estimation_ms: DEFAULT_CONSENSUS_ESTIMATION + consensus_estimation_ms: consensus_estimation .as_millis() .try_into() .expect("Time should fit into u64"), @@ -216,6 +216,7 @@ mod pending { state.latest_block_hash(), view_change_index, &transactions, + state.config.consensus_estimation(), ), transactions, commit_topology: self.0.commit_topology.ordered_peers, @@ -490,10 +491,7 @@ mod valid { transactions_hash: None, timestamp_ms: 0, view_change_index: 0, - consensus_estimation_ms: DEFAULT_CONSENSUS_ESTIMATION - .as_millis() - .try_into() - .expect("Time should fit into u64"), + consensus_estimation_ms: 4_000, }, transactions: Vec::new(), commit_topology: UniqueVec::new(), diff --git a/core/src/query/store.rs b/core/src/query/store.rs index 3cadc0d9831..be9da50dbd3 100644 --- a/core/src/query/store.rs +++ b/core/src/query/store.rs @@ -130,7 +130,7 @@ impl LiveQueryStore { } fn remove(&mut self, query_id: &str) -> Option { - self.queries.remove(query_id).map(|(output, _)| output) + self.queries.swap_remove(query_id).map(|(output, _)| output) } } diff --git a/core/src/smartcontracts/isi/world.rs b/core/src/smartcontracts/isi/world.rs index 5c882cd54ed..7ae1b2c2d6f 100644 --- a/core/src/smartcontracts/isi/world.rs +++ b/core/src/smartcontracts/isi/world.rs @@ -302,14 +302,14 @@ pub mod isi { let parameter = self.parameter; let parameter_id = parameter.id.clone(); - let world = &mut state_transaction.world; - if !world.parameters.remove(¶meter) { + if !state_transaction.world.parameters.swap_remove(¶meter) { return Err(FindError::Parameter(parameter_id).into()); } - - world.parameters.insert(parameter); - - world.emit_events(Some(ConfigurationEvent::Changed(parameter_id))); + state_transaction.world.parameters.insert(parameter.clone()); + state_transaction + .world + .emit_events(Some(ConfigurationEvent::Changed(parameter_id))); + state_transaction.try_apply_core_parameter(parameter); Ok(()) } @@ -325,16 +325,17 @@ pub mod isi { let parameter = self.parameter; let parameter_id = parameter.id.clone(); - let world = &mut state_transaction.world; - if !world.parameters.insert(parameter) { + if !state_transaction.world.parameters.insert(parameter.clone()) { return Err(RepetitionError { instruction_type: InstructionType::NewParameter, id: IdBox::ParameterId(parameter_id), } .into()); } - - world.emit_events(Some(ConfigurationEvent::Created(parameter_id))); + state_transaction + .world + .emit_events(Some(ConfigurationEvent::Created(parameter_id))); + state_transaction.try_apply_core_parameter(parameter); Ok(()) } diff --git a/core/src/state.rs b/core/src/state.rs index 38a74462a71..bec470813b1 100644 --- a/core/src/state.rs +++ b/core/src/state.rs @@ -1,5 +1,8 @@ //! This module provides the [`State`] — an in-memory representation of the current blockchain state. -use std::{borrow::Borrow, collections::BTreeSet, marker::PhantomData, sync::Arc, time::Duration}; +use std::{ + borrow::Borrow, collections::BTreeSet, marker::PhantomData, num::NonZeroU32, sync::Arc, + time::Duration, +}; use eyre::Result; use iroha_config::parameters::actual::ChainWide as Config; @@ -1239,7 +1242,6 @@ impl<'state> StateBlock<'state> { self.block_hashes.push(block_hash); - self.apply_parameters(); self.world.events_buffer.push( BlockEvent { header: block.as_ref().header().clone(), @@ -1253,20 +1255,18 @@ impl<'state> StateBlock<'state> { /// Create time event using previous and current blocks fn create_time_event(&self, block: &CommittedBlock) -> TimeEvent { - use iroha_config::parameters::defaults::chain_wide::CONSENSUS_ESTIMATION as DEFAULT_CONSENSUS_ESTIMATION; - let prev_interval = self.latest_block_ref().map(|latest_block| { let header = &latest_block.as_ref().header(); TimeInterval { since: header.timestamp(), - length: DEFAULT_CONSENSUS_ESTIMATION, + length: header.consensus_estimation(), } }); let interval = TimeInterval { since: block.as_ref().header().timestamp(), - length: DEFAULT_CONSENSUS_ESTIMATION, + length: block.as_ref().header().consensus_estimation(), }; TimeEvent { @@ -1323,32 +1323,6 @@ impl<'state> StateBlock<'state> { errors.is_empty().then_some(()).ok_or(errors) } - - fn apply_parameters(&mut self) { - use iroha_data_model::parameter::default::*; - - macro_rules! update_params { - ($($param:expr => $config:expr),+ $(,)?) => { - $(if let Some(param) = self.world.query_param($param) { - $config = param; - })+ - }; - } - - update_params! { - WSV_DOMAIN_METADATA_LIMITS => self.config.domain_metadata_limits, - WSV_ASSET_DEFINITION_METADATA_LIMITS => self.config.asset_definition_metadata_limits, - WSV_ACCOUNT_METADATA_LIMITS => self.config.account_metadata_limits, - WSV_ASSET_METADATA_LIMITS => self.config.asset_metadata_limits, - WSV_TRIGGER_METADATA_LIMITS => self.config.trigger_metadata_limits, - WSV_IDENT_LENGTH_LIMITS => self.config.ident_length_limits, - EXECUTOR_FUEL_LIMIT => self.config.executor_runtime.fuel_limit, - EXECUTOR_MAX_MEMORY => self.config.executor_runtime.max_memory_bytes, - WASM_FUEL_LIMIT => self.config.wasm_runtime.fuel_limit, - WASM_MAX_MEMORY => self.config.wasm_runtime.max_memory_bytes, - TRANSACTION_LIMITS => self.config.transaction_limits, - } - } } impl StateTransaction<'_, '_> { @@ -1360,6 +1334,96 @@ impl StateTransaction<'_, '_> { self.world.apply(); } + /// If given [`Parameter`] represents some of the core chain-wide + /// parameters ([`Config`]), apply it + pub fn try_apply_core_parameter(&mut self, parameter: Parameter) { + use iroha_data_model::parameter::default::*; + + struct Reader(Option); + + impl Reader { + fn try_and_then>( + self, + id: &str, + fun: impl FnOnce(T), + ) -> Self { + if let Some(param) = self.0 { + if param.id().name().as_ref() == id { + if let Some(value) = param.val.try_into().ok() { + fun(value); + } + Self(None) + } else { + Self(Some(param)) + } + } else { + Self(None) + } + } + + fn try_and_write>( + self, + id: &str, + destination: &mut T, + ) -> Self { + self.try_and_then(id, |value| { + *destination = value; + }) + } + } + + Reader(Some(parameter)) + .try_and_then(MAX_TRANSACTIONS_IN_BLOCK, |value| { + if let Some(checked) = NonZeroU32::new(value) { + self.config.max_transactions_in_block = checked; + } + }) + .try_and_then(BLOCK_TIME, |value| { + self.config.block_time = Duration::from_millis(value); + }) + .try_and_then(COMMIT_TIME_LIMIT, |value| { + self.config.commit_time = Duration::from_millis(value); + }) + .try_and_write( + WSV_DOMAIN_METADATA_LIMITS, + &mut self.config.domain_metadata_limits, + ) + .try_and_write( + WSV_ASSET_DEFINITION_METADATA_LIMITS, + &mut self.config.asset_definition_metadata_limits, + ) + .try_and_write( + WSV_ACCOUNT_METADATA_LIMITS, + &mut self.config.account_metadata_limits, + ) + .try_and_write( + WSV_ASSET_METADATA_LIMITS, + &mut self.config.asset_metadata_limits, + ) + .try_and_write( + WSV_TRIGGER_METADATA_LIMITS, + &mut self.config.trigger_metadata_limits, + ) + .try_and_write( + WSV_IDENT_LENGTH_LIMITS, + &mut self.config.ident_length_limits, + ) + .try_and_write( + EXECUTOR_FUEL_LIMIT, + &mut self.config.executor_runtime.fuel_limit, + ) + .try_and_write( + EXECUTOR_MAX_MEMORY, + &mut self.config.executor_runtime.max_memory_bytes, + ) + .try_and_write(WASM_FUEL_LIMIT, &mut self.config.wasm_runtime.fuel_limit) + .try_and_write( + WASM_MAX_MEMORY, + &mut self.config.wasm_runtime.max_memory_bytes, + ) + .try_and_write(TRANSACTION_LIMITS, &mut self.config.transaction_limits); + } + fn process_executable(&mut self, executable: &Executable, authority: AccountId) -> Result<()> { match executable { Executable::Instructions(instructions) => { diff --git a/core/src/sumeragi/main_loop.rs b/core/src/sumeragi/main_loop.rs index 23be4d838c7..b9b2cb3b92e 100644 --- a/core/src/sumeragi/main_loop.rs +++ b/core/src/sumeragi/main_loop.rs @@ -356,20 +356,9 @@ impl Sumeragi { } fn update_params(&mut self, state_block: &StateBlock<'_>) { - use iroha_data_model::parameter::default::*; - - if let Some(block_time) = state_block.world.query_param(BLOCK_TIME) { - self.block_time = Duration::from_millis(block_time); - } - if let Some(commit_time) = state_block.world.query_param(COMMIT_TIME_LIMIT) { - self.commit_time = Duration::from_millis(commit_time); - } - if let Some(max_txs_in_block) = state_block - .world - .query_param::(MAX_TRANSACTIONS_IN_BLOCK) - { - self.max_txs_in_block = max_txs_in_block as usize; - } + self.block_time = state_block.config.block_time; + self.commit_time = state_block.config.commit_time; + self.max_txs_in_block = state_block.config.max_transactions_in_block.get() as usize; } fn cache_transaction(&mut self, state_block: &StateBlock<'_>) { diff --git a/data_model/src/block.rs b/data_model/src/block.rs index 2655beeaca2..2b5db8559d3 100644 --- a/data_model/src/block.rs +++ b/data_model/src/block.rs @@ -127,9 +127,14 @@ impl BlockHeader { } /// Creation timestamp - pub fn timestamp(&self) -> Duration { + pub const fn timestamp(&self) -> Duration { Duration::from_millis(self.timestamp_ms) } + + /// Consensus estimation + pub const fn consensus_estimation(&self) -> Duration { + Duration::from_millis(self.consensus_estimation_ms) + } } impl SignedBlockV1 {