From 97f3c8e2769301e4a35f988f935d6a8636e8b156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marin=20Ver=C5=A1i=C4=87?= Date: Wed, 27 Sep 2023 19:14:49 +0200 Subject: [PATCH] [refactor] #3895: move `commit_topology` into block payload (#3916) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marin Veršić --- core/benches/kura.rs | 2 +- core/benches/validation.rs | 2 +- core/src/block.rs | 51 ++++++++------------------- core/src/smartcontracts/isi/query.rs | 4 +-- core/src/sumeragi/main_loop.rs | 10 +++--- core/src/sumeragi/mod.rs | 3 +- core/src/sumeragi/network_topology.rs | 2 +- data_model/src/block.rs | 6 ++-- docs/source/references/schema.json | 8 ++--- 9 files changed, 32 insertions(+), 56 deletions(-) diff --git a/core/benches/kura.rs b/core/benches/kura.rs index 558e83c0cd5..c3874bd8a59 100644 --- a/core/benches/kura.rs +++ b/core/benches/kura.rs @@ -45,7 +45,7 @@ async fn measure_block_size_for_n_validators(n_validators: u32) { let mut wsv = WorldStateView::new(World::new(), kura); let topology = Topology::new(UniqueVec::new()); let mut block = BlockBuilder::new(vec![tx], topology, Vec::new()) - .chain_first(&mut wsv) + .chain(0, &mut wsv) .sign(KeyPair::generate().unwrap()) .unwrap(); diff --git a/core/benches/validation.rs b/core/benches/validation.rs index 36886ac87d4..02085aef02e 100644 --- a/core/benches/validation.rs +++ b/core/benches/validation.rs @@ -152,7 +152,7 @@ fn sign_blocks(criterion: &mut Criterion) { let mut success_count = 0; let mut failures_count = 0; - let block = BlockBuilder::new(vec![transaction], topology, Vec::new()).chain_first(&mut wsv); + let block = BlockBuilder::new(vec![transaction], topology, Vec::new()).chain(0, &mut wsv); let _ = criterion.bench_function("sign_block", |b| { b.iter(|| match block.clone().sign(key_pair.clone()) { diff --git a/core/src/block.rs b/core/src/block.rs index 609179c710b..fc7837934f2 100644 --- a/core/src/block.rs +++ b/core/src/block.rs @@ -110,13 +110,11 @@ mod pending { /// the previous round, which might then be processed by the trigger system. #[derive(Debug, Clone)] pub struct Pending { - /// Unix timestamp - timestamp_ms: u64, + /// The topology at the time of block commit. + commit_topology: Topology, /// Collection of transactions which have been accepted. /// Transaction will be validated when block is chained. transactions: Vec, - /// The topology at the time of block commit. - commit_topology: Topology, /// Event recommendations for use in triggers and off-chain work event_recommendations: Vec, } @@ -136,10 +134,6 @@ mod pending { assert!(!transactions.is_empty(), "Empty block created"); Self(Pending { - timestamp_ms: iroha_data_model::current_time() - .as_millis() - .try_into() - .expect("Time should fit into u64"), transactions, commit_topology, event_recommendations, @@ -147,15 +141,16 @@ mod pending { } fn make_header( - timestamp_ms: u64, previous_height: u64, previous_block_hash: Option>, view_change_index: u64, transactions: &[TransactionValue], - commit_topology: Topology, ) -> BlockHeader { BlockHeader { - timestamp_ms, + timestamp_ms: iroha_data_model::current_time() + .as_millis() + .try_into() + .expect("Time should fit into u64"), consensus_estimation_ms: DEFAULT_CONSENSUS_ESTIMATION_MS, height: previous_height + 1, view_change_index, @@ -165,7 +160,6 @@ mod pending { .map(TransactionValue::hash) .collect::>() .hash(), - commit_topology: commit_topology.ordered_peers, } } @@ -196,6 +190,8 @@ mod pending { } /// Chain the block with existing blockchain. + /// + /// Upon executing this method current timestamp is stored in the block header. pub fn chain( self, view_change_index: u64, @@ -205,32 +201,13 @@ mod pending { BlockBuilder(Chained(BlockPayload { header: Self::make_header( - self.0.timestamp_ms, wsv.height(), wsv.latest_block_hash(), view_change_index, &transactions, - self.0.commit_topology, - ), - transactions, - event_recommendations: self.0.event_recommendations, - })) - } - - /// Create a new blockchain with current block as the first block. - pub fn chain_first(self, wsv: &mut WorldStateView) -> BlockBuilder { - let transactions = Self::categorize_transactions(self.0.transactions, wsv); - - BlockBuilder(Chained(BlockPayload { - header: Self::make_header( - self.0.timestamp_ms, - 0, - None, - 0, - &transactions, - self.0.commit_topology, ), transactions, + commit_topology: self.0.commit_topology.ordered_peers, event_recommendations: self.0.event_recommendations, })) } @@ -297,7 +274,7 @@ mod valid { topology: &Topology, wsv: &mut WorldStateView, ) -> Result { - let actual_commit_topology = &block.payload().header.commit_topology; + let actual_commit_topology = &block.payload().commit_topology; let expected_commit_topology = &topology.ordered_peers; if actual_commit_topology != expected_commit_topology { @@ -491,9 +468,9 @@ mod valid { view_change_index: 0, previous_block_hash: None, transactions_hash: None, - commit_topology: UniqueVec::new(), }, transactions: Vec::new(), + commit_topology: UniqueVec::new(), event_recommendations: Vec::new(), })) .sign(KeyPair::generate().unwrap()) @@ -779,7 +756,7 @@ mod tests { let transactions = vec![tx.clone(), tx]; let topology = Topology::new(UniqueVec::new()); let valid_block = BlockBuilder::new(transactions, topology, Vec::new()) - .chain_first(&mut wsv) + .chain(0, &mut wsv) .sign(alice_keys) .expect("Valid"); @@ -846,7 +823,7 @@ mod tests { let transactions = vec![tx0, tx, tx2]; let topology = Topology::new(UniqueVec::new()); let valid_block = BlockBuilder::new(transactions, topology, Vec::new()) - .chain_first(&mut wsv) + .chain(0, &mut wsv) .sign(alice_keys) .expect("Valid"); @@ -899,7 +876,7 @@ mod tests { let transactions = vec![tx_fail, tx_accept]; let topology = Topology::new(UniqueVec::new()); let valid_block = BlockBuilder::new(transactions, topology, Vec::new()) - .chain_first(&mut wsv) + .chain(0, &mut wsv) .sign(alice_keys) .expect("Valid"); diff --git a/core/src/smartcontracts/isi/query.rs b/core/src/smartcontracts/isi/query.rs index f33d9cff7b0..8c4f3ed88f0 100644 --- a/core/src/smartcontracts/isi/query.rs +++ b/core/src/smartcontracts/isi/query.rs @@ -293,7 +293,7 @@ mod tests { let topology = Topology::new(UniqueVec::new()); let first_block = BlockBuilder::new(transactions.clone(), topology.clone(), Vec::new()) - .chain_first(&mut wsv) + .chain(0, &mut wsv) .sign(ALICE_KEYS.clone())? .commit(&topology) .expect("Block is valid"); @@ -429,7 +429,7 @@ mod tests { let topology = Topology::new(UniqueVec::new()); let vcb = BlockBuilder::new(vec![va_tx.clone()], topology.clone(), Vec::new()) - .chain_first(&mut wsv) + .chain(0, &mut wsv) .sign(ALICE_KEYS.clone())? .commit(&topology) .expect("Block is valid"); diff --git a/core/src/sumeragi/main_loop.rs b/core/src/sumeragi/main_loop.rs index 9691564db65..333d487cddf 100644 --- a/core/src/sumeragi/main_loop.rs +++ b/core/src/sumeragi/main_loop.rs @@ -249,7 +249,7 @@ impl Sumeragi { let mut new_wsv = self.wsv.clone(); let genesis = BlockBuilder::new(transactions, self.current_topology.clone(), vec![]) - .chain_first(&mut new_wsv) + .chain(0, &mut new_wsv) .sign(self.key_pair.clone()) .expect("Genesis signing failed"); @@ -324,7 +324,7 @@ impl Sumeragi { // Parameters are updated before updating public copy of sumeragi self.update_params(); - let block_topology = block.payload().header.commit_topology.clone(); + let block_topology = block.payload().commit_topology.clone(); let block_signees = block .signatures() .into_iter() @@ -1195,7 +1195,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_first(&mut wsv) + .chain(0, &mut wsv) .sign(leader_key_pair.clone()) .expect("Block is valid"); @@ -1247,7 +1247,7 @@ mod tests { let wsv = finalized_wsv.clone(); // Malform block to make it invalid - block.payload_mut().header.commit_topology.clear(); + block.payload_mut().commit_topology.clear(); let result = handle_block_sync(block, &wsv, &finalized_wsv); assert!(matches!(result, Err((_, BlockSyncError::BlockNotValid(_))))) @@ -1270,7 +1270,7 @@ mod tests { kura.store_block(committed_block); // Malform block to make it invalid - block.payload_mut().header.commit_topology.clear(); + block.payload_mut().commit_topology.clear(); let result = handle_block_sync(block, &wsv, &finalized_wsv); assert!(matches!( diff --git a/core/src/sumeragi/mod.rs b/core/src/sumeragi/mod.rs index 59a0e1ae729..3123025f805 100644 --- a/core/src/sumeragi/mod.rs +++ b/core/src/sumeragi/mod.rs @@ -266,8 +266,7 @@ impl SumeragiHandle { "Sumeragi could not load block that was reported as present. \ Please check that the block storage was not disconnected.", ); - let mut topology = - Topology::new(block_ref.payload().header.commit_topology.clone()); + let mut topology = Topology::new(block_ref.payload().commit_topology.clone()); topology.rotate_set_a(); topology } diff --git a/core/src/sumeragi/network_topology.rs b/core/src/sumeragi/network_topology.rs index 0ca6e1d3f9a..753694d34e1 100644 --- a/core/src/sumeragi/network_topology.rs +++ b/core/src/sumeragi/network_topology.rs @@ -192,7 +192,7 @@ impl Topology { view_change_index: u64, new_peers: UniqueVec, ) -> Self { - let mut topology = Topology::new(block.payload().header().commit_topology.clone()); + let mut topology = Topology::new(block.payload().commit_topology.clone()); let block_signees = block .signatures() .into_iter() diff --git a/data_model/src/block.rs b/data_model/src/block.rs index 170a8efbd42..fe471df2901 100644 --- a/data_model/src/block.rs +++ b/data_model/src/block.rs @@ -61,9 +61,6 @@ pub mod model { pub previous_block_hash: Option>, /// Hash of merkle tree root of transactions' hashes. pub transactions_hash: Option>>, - /// Topology of the network at the time of block commit. - #[getset(skip)] // FIXME: Because ffi related issues - pub commit_topology: UniqueVec, /// Value of view change index. Used to resolve soft forks. pub view_change_index: u64, /// Estimation of consensus duration (in milliseconds). @@ -92,6 +89,9 @@ pub mod model { pub struct BlockPayload { /// Block header pub header: BlockHeader, + /// Topology of the network at the time of block commit. + #[getset(skip)] // FIXME: Because ffi related issues + pub commit_topology: UniqueVec, /// array of transactions, which successfully passed validation and consensus step. #[getset(skip)] // FIXME: Because ffi related issues pub transactions: Vec, diff --git a/docs/source/references/schema.json b/docs/source/references/schema.json index d8763aeb9c6..ef55739736e 100644 --- a/docs/source/references/schema.json +++ b/docs/source/references/schema.json @@ -672,10 +672,6 @@ "name": "transactions_hash", "type": "Option>>" }, - { - "name": "commit_topology", - "type": "UniqueVec" - }, { "name": "view_change_index", "type": "u64" @@ -693,6 +689,10 @@ "name": "header", "type": "BlockHeader" }, + { + "name": "commit_topology", + "type": "UniqueVec" + }, { "name": "transactions", "type": "Vec"