Address review comments.

Put block rewards into a constant, remove duplicated switch block
detection from era supervisor, and rename `relative_height` to `height`.

Author: afck

node/src/components/consensus/consensus_protocol.rs

@@ -53,7 +53,8 @@ pub(crate) enum ConsensusProtocolResult<I, C: ConsensusValueT, VID> {
         new_equivocators: Vec<VID>,
         rewards: BTreeMap<VID, u64>,
         timestamp: Timestamp,
-        relative_height: u64,
+        height: u64,
+        switch_block: bool,
     },
     /// Request validation of the consensus value, contained in a message received from the given
     /// node.

node/src/components/consensus/era_supervisor.rs

@@ -39,6 +39,10 @@ use crate::{
     utils::WithDir,
 };
 
+// We use one trillion as a block reward unit because it's large enough to allow precise
+// fractions, and small enough for many block rewards to fit into a u64.
+const BLOCK_REWARD: u64 = 1_000_000_000_000;
+
 #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
 pub struct EraId(pub(crate) u64);
 
@@ -58,8 +62,6 @@ impl EraId {
 pub(crate) struct Era<I> {
     /// The consensus protocol instance.
     consensus: Box<dyn ConsensusProtocol<I, ProtoBlock, PublicKey>>,
-    /// The timestamp of the last block of the previous era.
-    start_time: Timestamp,
     /// The height of this era's first block.
     start_height: u64,
 }
@@ -163,16 +165,13 @@ where
         let ftt = validators.total_weight()
             * u64::from(self.highway_config.finality_threshold_percent)
             / 100;
-        // We use one trillion as a block reward unit because it's large enough to allow precise
-        // fractions, and small enough for many block rewards to fit into a u64.
-        let block_reward = 1_000_000_000_000;
         // The number of rounds after which a block reward is paid out.
         // TODO: Make this configurable?
         let reward_delay = 8;
         let params = Params::new(
             0, // TODO: get a proper seed.
-            block_reward,
-            block_reward / 5, // TODO: Make reduced block reward configurable?
+            BLOCK_REWARD,
+            BLOCK_REWARD / 5, // TODO: Make reduced block reward configurable?
             reward_delay,
             self.highway_config.minimum_round_exponent,
             self.highway_config.minimum_era_height,
@@ -190,22 +189,23 @@ where
 
         let era = Era {
             consensus: Box::new(highway),
-            start_time,
             start_height,
         };
         let _ = self.active_eras.insert(era_id, era);
 
         results
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn handle_finalized_block(
         &mut self,
         era_id: EraId,
         proto_block: ProtoBlock,
         new_equivocators: Vec<PublicKey>,
         rewards: BTreeMap<PublicKey, u64>,
         timestamp: Timestamp,
-        relative_height: u64,
+        height: u64,
+        switch_block: bool,
     ) -> (
         Vec<ConsensusProtocolResult<I, ProtoBlock, PublicKey>>,
         FinalizedBlock,
@@ -221,18 +221,14 @@ where
         if !rewards.is_empty() {
             system_transactions.push(SystemTransaction::Rewards(rewards));
         };
-        let start_height = self.active_eras[&era_id].start_height;
-        let start_time = self.active_eras[&era_id].start_time;
-        let switch_block = relative_height + 1 >= self.highway_config.minimum_era_height
-            && timestamp >= start_time + self.highway_config.era_duration;
         // Request execution of the finalized block.
         let fb = FinalizedBlock::new(
             proto_block,
             timestamp,
             system_transactions,
             switch_block,
             era_id,
-            start_height + relative_height,
+            self.active_eras[&era_id].start_height + height,
         );
         let results = if fb.switch_block() {
             self.current_era_mut().consensus.deactivate_validator();
@@ -457,7 +453,8 @@ where
                 new_equivocators,
                 rewards,
                 timestamp,
-                relative_height,
+                height,
+                switch_block,
             } => {
                 // Announce the finalized proto block.
                 let mut effects = self
@@ -470,7 +467,8 @@ where
                     new_equivocators,
                     rewards,
                     timestamp,
-                    relative_height,
+                    height,
+                    switch_block,
                 );
                 effects.extend(
                     results

node/src/components/consensus/highway_core/finality_detector.rs

@@ -31,6 +31,8 @@ pub(crate) enum FinalityOutcome<C: Context> {
         timestamp: Timestamp,
         /// The relative height in this instance of the protocol.
         height: u64,
+        /// Whether this is a terminal block, i.e. the last one to be finalized.
+        terminal: bool,
     },
     /// The fault tolerance threshold has been exceeded: The number of observed equivocations
     /// invalidates this finality detector's results.
@@ -82,6 +84,7 @@ impl<C: Context> FinalityDetector<C> {
             rewards: rewards_iter.map(|(vidx, r)| (to_id(vidx), *r)).collect(),
             timestamp: state.vote(bhash).timestamp,
             height: block.height,
+            terminal: state.is_terminal_block(bhash),
         }
     }
 

node/src/components/consensus/highway_core/highway_testing.rs

@@ -479,6 +479,7 @@ where
                 rewards,
                 timestamp,
                 height,
+                terminal,
             } => {
                 if !new_equivocators.is_empty() {
                     warn!("New equivocators detected: {:?}", new_equivocators);
@@ -488,7 +489,8 @@ where
                     warn!("Rewards are not verified yet: {:?}", rewards);
                 }
                 trace!(
-                    "Consensus value finalized: {:?}, height: {:?}",
+                    "{}consensus value finalized: {:?}, height: {:?}",
+                    if terminal { "last " } else { "" },
                     value,
                     height
                 );

node/src/components/consensus/protocols/highway.rs

@@ -131,13 +131,15 @@ impl<I: NodeIdT, C: Context> HighwayProtocol<I, C> {
                 rewards,
                 timestamp,
                 height,
+                terminal,
             } => {
                 results.push(ConsensusProtocolResult::FinalizedBlock {
                     value,
                     new_equivocators,
                     rewards,
                     timestamp,
-                    relative_height: height,
+                    height,
+                    switch_block: terminal,
                 });
             }
         }