Apply review comments

Signed-off-by: s8sato <[email protected]>

core/src/block.rs

@@ -719,7 +719,7 @@ impl VersionedCommittedBlock {
 }
 
 /// When Kura receives `ValidBlock`, the block is stored and
-/// then sent to later stage of the pipeline as `CommitedBlock`.
+/// then sent to later stage of the pipeline as `CommittedBlock`.
 #[version_with_scale(n = 1, versioned = "VersionedCommittedBlock")]
 #[derive(Debug, Clone, Decode, Encode, IntoSchema)]
 pub struct CommittedBlock {

core/src/sumeragi/network_topology.rs

@@ -139,9 +139,9 @@ impl GenesisBuilder {
 pub struct Builder {
     /// Current order of peers. The roles of peers are defined based on this order.
     peers: Option<HashSet<PeerId>>,
-
+    /// Hash of the last committed block.
     at_block: Option<HashOf<VersionedCommittedBlock>>,
-
+    /// [`ViewChangeProofs`] accumulated during this round.
     view_change_proofs: ViewChangeProofs,
 }
 
@@ -177,18 +177,19 @@ impl Builder {
     pub fn build(self) -> Result<Topology> {
         let peers = field_is_some_or_err!(self.peers)?;
         if peers.is_empty() {
-            return Err(eyre::eyre!(
-                "There must be at least one peer in the network."
-            ));
+            return Err(eyre!("There must be at least one peer in the network."));
         }
         let at_block = field_is_some_or_err!(self.at_block)?;
-
         let peers: Vec<_> = peers.into_iter().collect();
-        let sorted_peers = if self.view_change_proofs.len() > peers.len() {
-            sort_peers_by_hash_and_counter(peers, &at_block, self.view_change_proofs.len() as u64)
+        let n_view_changes = self.view_change_proofs.len();
+        let since_last_shuffle = n_view_changes % peers.len();
+        let is_full_circle = since_last_shuffle == 0;
+        let sorted_peers = if is_full_circle {
+            sort_peers_by_hash_and_counter(peers, &at_block, n_view_changes as u64)
         } else {
-            let peers = sort_peers_by_hash(peers, &at_block);
-            shift_peers_by_n(peers, self.view_change_proofs.len() as u64)
+            let last_shuffled_at = n_view_changes - since_last_shuffle;
+            let peers = sort_peers_by_hash_and_counter(peers, &at_block, last_shuffled_at as u64);
+            shift_peers_by_n(peers, since_last_shuffle as u64)
         };
         Ok(Topology {
             sorted_peers,
@@ -203,9 +204,9 @@ impl Builder {
 pub struct Topology {
     /// Current order of peers. The roles of peers are defined based on this order.
     sorted_peers: Vec<PeerId>,
-
+    /// Hash of the last committed block.
     at_block: HashOf<VersionedCommittedBlock>,
-
+    /// [`ViewChangeProofs`] accumulated during this round.
     view_change_proofs: ViewChangeProofs,
 }
 
@@ -363,11 +364,6 @@ impl Topology {
         &self.sorted_peers[..]
     }
 
-    /// Config param telling topology when to reshuffle at view change.
-    pub fn reshuffle_after(&self) -> u64 {
-        self.sorted_peers.len() as u64
-    }
-
     /// Block hash on which this topology is based.
     pub const fn at_block(&self) -> &HashOf<VersionedCommittedBlock> {
         &self.at_block
@@ -543,4 +539,48 @@ mod tests {
         let peers_2 = sort_peers_by_hash_and_counter(peers, &hash, 2);
         assert_ne!(peers_1, peers_2);
     }
+
+    #[test]
+    fn topology_shifts_or_shuffles() -> Result<()> {
+        let peers = topology_test_peers();
+        let n_peers = peers.len();
+        let dummy_hash = Hash::prehashed([0_u8; Hash::LENGTH]).typed();
+        let dummy_proof = crate::sumeragi::Proof::commit_timeout(
+            dummy_hash,
+            dummy_hash.transmute(),
+            dummy_hash.transmute(),
+            KeyPair::generate()?,
+        )?;
+        let mut last_topology = Builder::new()
+            .with_peers(peers)
+            .at_block(dummy_hash.transmute())
+            .build()?;
+        for _a_view_change in 0..2 * n_peers {
+            let mut topology = last_topology.clone();
+            // When
+            last_topology.sorted_peers.rotate_right(1);
+            topology.apply_view_change(dummy_proof.clone());
+            // Then
+            let is_shifted_by_one = last_topology.sorted_peers == topology.sorted_peers;
+            let nth_view_change = topology.view_change_proofs.len();
+            let is_full_circle = nth_view_change % n_peers == 0;
+            if is_full_circle {
+                // `topology` should have shuffled
+                if is_shifted_by_one {
+                    return Err(eyre!(
+                        "At {nth_view_change}: shifted by one despite full circle"
+                    ));
+                }
+            } else {
+                // `topology` should have shifted by one
+                if !is_shifted_by_one {
+                    return Err(eyre!(
+                        "At {nth_view_change}: not shifted by one despite incomplete circle"
+                    ));
+                }
+            }
+            last_topology = topology;
+        }
+        Ok(())
+    }
 }

docs/source/references/api_spec.md

@@ -256,7 +256,50 @@ Also returns current status of peer in json string:
 **Expects**: -
 
 **Responses**:
-In a typical use case, Prometheus handles the response
+- 200 OK - reports 8 of 10 metrics:
+
+```bash
+# HELP accounts User accounts registered at this time
+# TYPE accounts gauge
+accounts{domain="genesis"} 1
+accounts{domain="wonderland"} 1
+# HELP block_height Current block height
+# TYPE block_height counter
+block_height 1
+# HELP connected_peers Total number of currently connected peers
+# TYPE connected_peers gauge
+connected_peers 0
+# HELP domains Total number of domains
+# TYPE domains gauge
+domains 2
+# HELP tx_amount average amount involved in a transaction on this peer
+# TYPE tx_amount histogram
+tx_amount_bucket{le="0.005"} 0
+tx_amount_bucket{le="0.01"} 0
+tx_amount_bucket{le="0.025"} 0
+tx_amount_bucket{le="0.05"} 0
+tx_amount_bucket{le="0.1"} 0
+tx_amount_bucket{le="0.25"} 0
+tx_amount_bucket{le="0.5"} 0
+tx_amount_bucket{le="1"} 0
+tx_amount_bucket{le="2.5"} 0
+tx_amount_bucket{le="5"} 0
+tx_amount_bucket{le="10"} 0
+tx_amount_bucket{le="+Inf"} 2
+tx_amount_sum 26
+tx_amount_count 2
+# HELP txs Transactions committed
+# TYPE txs counter
+txs{type="accepted"} 1
+txs{type="rejected"} 0
+txs{type="total"} 1
+# HELP uptime_since_genesis_ms Network up-time, from creation of the genesis block
+# TYPE uptime_since_genesis_ms gauge
+uptime_since_genesis_ms 54572974
+# HELP view_changes Number of view_changes in the current round
+# TYPE view_changes gauge
+view_changes 0
+```
 
 ## Parity Scale Codec