[refactor] #3895: move commit_topology into block payload (#3916)

Signed-off-by: Marin Veršić <[email protected]>

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();
 

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()) {

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<AcceptedTransaction>,
-        /// The topology at the time of block commit.
-        commit_topology: Topology,
         /// Event recommendations for use in triggers and off-chain work
         event_recommendations: Vec<Event>,
     }
@@ -136,26 +134,23 @@ 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,
             })
         }
 
         fn make_header(
-            timestamp_ms: u64,
             previous_height: u64,
             previous_block_hash: Option<HashOf<VersionedSignedBlock>>,
             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::<MerkleTree<_>>()
                     .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<Chained> {
-            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<ValidBlock, (VersionedSignedBlock, BlockValidationError)> {
-            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");
 

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");

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!(

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
             }

core/src/sumeragi/network_topology.rs

@@ -192,7 +192,7 @@ impl Topology {
         view_change_index: u64,
         new_peers: UniqueVec<PeerId>,
     ) -> 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()

data_model/src/block.rs

@@ -61,9 +61,6 @@ pub mod model {
         pub previous_block_hash: Option<HashOf<VersionedSignedBlock>>,
         /// Hash of merkle tree root of transactions' hashes.
         pub transactions_hash: Option<HashOf<MerkleTree<VersionedSignedTransaction>>>,
-        /// Topology of the network at the time of block commit.
-        #[getset(skip)] // FIXME: Because ffi related issues
-        pub commit_topology: UniqueVec<peer::PeerId>,
         /// 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<peer::PeerId>,
         /// array of transactions, which successfully passed validation and consensus step.
         #[getset(skip)] // FIXME: Because ffi related issues
         pub transactions: Vec<TransactionValue>,

docs/source/references/schema.json

@@ -672,10 +672,6 @@
         "name": "transactions_hash",
         "type": "Option<HashOf<MerkleTree<VersionedSignedTransaction>>>"
       },
-      {
-        "name": "commit_topology",
-        "type": "UniqueVec<PeerId>"
-      },
       {
         "name": "view_change_index",
         "type": "u64"
@@ -693,6 +689,10 @@
         "name": "header",
         "type": "BlockHeader"
       },
+      {
+        "name": "commit_topology",
+        "type": "UniqueVec<PeerId>"
+      },
       {
         "name": "transactions",
         "type": "Vec<TransactionValue>"