fix: check minimum number of headers for calc-timing (#3009)

applications/tari_app_grpc/proto/base_node.proto

@@ -35,7 +35,11 @@ service BaseNode {
     // Returns blocks in the current best chain. Currently only supports querying by height
     rpc GetBlocks(GetBlocksRequest) returns (stream HistoricalBlock);
     // Returns the calc timing for the chain heights
-    rpc GetCalcTiming(HeightRequest) returns (CalcTimingResponse);
+    rpc GetCalcTiming(HeightRequest) returns (CalcTimingResponse) {
+        option deprecated = true;
+    };
+    // Returns the block timing for the chain heights
+    rpc GetBlockTiming(HeightRequest) returns (BlockTimingResponse);
     // Returns the network Constants
     rpc GetConstants(Empty) returns (ConsensusConstants);
     // Returns Block Sizes
@@ -159,6 +163,14 @@ message HeightRequest {
 
 // The return type of the rpc GetCalcTiming
 message CalcTimingResponse {
+    option deprecated = true;
+    uint64 max = 1;
+    uint64 min = 2;
+    double avg = 3;
+}
+
+// The return type of the rpc GetBlockTiming
+message BlockTimingResponse {
     uint64 max = 1;
     uint64 min = 2;
     double avg = 3;

applications/tari_base_node/src/command_handler.rs

@@ -726,7 +726,7 @@ impl CommandHandler {
         }
     }
 
-    pub fn calc_timing(&self, start: u64, end: Option<u64>) {
+    pub fn block_timing(&self, start: u64, end: Option<u64>) {
         let blockchain_db = self.blockchain_db.clone();
         self.executor.spawn(async move {
             let headers = match Self::get_chain_headers(&blockchain_db, start, end).await {

applications/tari_base_node/src/grpc/base_node_grpc_server.rs

@@ -828,14 +828,30 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
         Ok(Response::new(rx))
     }
 
+    // deprecated
     async fn get_calc_timing(
         &self,
         request: Request<tari_rpc::HeightRequest>,
     ) -> Result<Response<tari_rpc::CalcTimingResponse>, Status> {
+        debug!(
+            target: LOG_TARGET,
+            "Incoming GRPC request for deprecated GetCalcTiming. Forwarding to GetBlockTiming.",
+        );
+
+        let tari_rpc::BlockTimingResponse { max, min, avg } = self.get_block_timing(request).await?.into_inner();
+        let response = tari_rpc::CalcTimingResponse { max, min, avg };
+
+        Ok(Response::new(response))
+    }
+
+    async fn get_block_timing(
+        &self,
+        request: Request<tari_rpc::HeightRequest>,
+    ) -> Result<Response<tari_rpc::BlockTimingResponse>, Status> {
         let request = request.into_inner();
         debug!(
             target: LOG_TARGET,
-            "Incoming GRPC request for GetCalcTiming: from_tip: {:?} start_height: {:?} end_height: {:?}",
+            "Incoming GRPC request for GetBlockTiming: from_tip: {:?} start_height: {:?} end_height: {:?}",
             request.from_tip,
             request.start_height,
             request.end_height
@@ -853,8 +869,8 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer {
         };
         let (max, min, avg) = BlockHeader::timing_stats(&headers);
 
-        let response: tari_rpc::CalcTimingResponse = tari_rpc::CalcTimingResponse { max, min, avg };
-        debug!(target: LOG_TARGET, "Sending GetCalcTiming response to client");
+        let response = tari_rpc::BlockTimingResponse { max, min, avg };
+        debug!(target: LOG_TARGET, "Sending GetBlockTiming response to client");
         Ok(Response::new(response))
     }
 

applications/tari_base_node/src/parser.rs

@@ -70,6 +70,7 @@ pub enum BaseNodeCommand {
     CheckDb,
     PeriodStats,
     HeaderStats,
+    BlockTiming,
     CalcTiming,
     DiscoverPeer,
     GetBlock,
@@ -228,8 +229,8 @@ impl Parser {
             ListHeaders => {
                 self.process_list_headers(args);
             },
-            CalcTiming => {
-                self.process_calc_timing(args);
+            BlockTiming | CalcTiming => {
+                self.process_block_timing(args);
             },
             GetBlock => {
                 self.process_get_block(args);
@@ -261,9 +262,9 @@ impl Parser {
     }
 
     /// Displays the commands or context specific help for a given command
-    fn print_help(&self, help_for: BaseNodeCommand) {
+    fn print_help(&self, command: BaseNodeCommand) {
         use BaseNodeCommand::*;
-        match help_for {
+        match command {
             Help => {
                 println!("Available commands are: ");
                 let joined = self.commands.join(", ");
@@ -298,7 +299,7 @@ impl Parser {
             },
             RewindBlockchain => {
                 println!("Rewinds the blockchain to the given height.");
-                println!("Usage: {} [new_height]", help_for);
+                println!("Usage: {} [new_height]", command);
                 println!("new_height must be less than the current height.");
             },
             BanPeer => {
@@ -344,8 +345,10 @@ impl Parser {
                 println!("list-headers [first header height] [last header height]");
                 println!("list-headers [number of headers starting from the chain tip back]");
             },
-            CalcTiming => {
-                println!("Calculates the time average time taken to mine a given range of blocks.");
+            BlockTiming | CalcTiming => {
+                println!("Calculates the maximum, minimum, and average time taken to mine a given range of blocks.");
+                println!("block-timing [start height] [end height]");
+                println!("block-timing [number of blocks from chain tip]");
             },
             GetBlock => {
                 println!("Display a block by height or hash:");
@@ -577,17 +580,21 @@ impl Parser {
     }
 
     /// Function to process the calc-timing command
-    fn process_calc_timing<'a, I: Iterator<Item = &'a str>>(&self, mut args: I) {
+    fn process_block_timing<'a, I: Iterator<Item = &'a str>>(&self, mut args: I) {
         let start = args.next().map(u64::from_str).map(Result::ok).flatten();
         let end = args.next().map(u64::from_str).map(Result::ok).flatten();
-        if start.is_none() {
-            println!("Command entered incorrectly, please use the following formats: ");
-            println!("calc-timing [first header height] [last header height]");
-            println!("calc-timing [number of headers from chain tip]");
-            return;
+
+        let command = BaseNodeCommand::BlockTiming;
+        if let Some(start) = start {
+            if end.is_none() && start < 2 {
+                println!("Number of headers must be at least 2.");
+                self.print_help(command);
+            } else {
+                self.command_handler.block_timing(start, end)
+            }
+        } else {
+            self.print_help(command);
         }
-        let start = start.unwrap();
-        self.command_handler.calc_timing(start, end)
     }
 
     fn process_period_stats<'a, I: Iterator<Item = &'a str>>(&self, args: I) {

base_layer/core/src/blocks/block_header.rs

@@ -173,23 +173,23 @@ impl BlockHeader {
 
     /// Given a slice of headers (in reverse order), calculate the maximum, minimum and average periods between them
     pub fn timing_stats(headers: &[BlockHeader]) -> (u64, u64, f64) {
-        let (max, min) = headers.windows(2).fold((0u64, std::u64::MAX), |(max, min), next| {
-            let delta_t = match next[0].timestamp.checked_sub(next[1].timestamp) {
-                Some(delta) => delta.as_u64(),
-                None => 0u64,
-            };
-            let min = min.min(delta_t);
-            let max = max.max(delta_t);
-            (max, min)
-        });
-        let avg = if headers.len() >= 2 {
+        if headers.len() < 2 {
+            (0, 0, 0.0)
+        } else {
+            let (max, min) = headers.windows(2).fold((0u64, std::u64::MAX), |(max, min), next| {
+                let dt = match next[0].timestamp.checked_sub(next[1].timestamp) {
+                    Some(delta) => delta.as_u64(),
+                    None => 0u64,
+                };
+                (max.max(dt), min.min(dt))
+            });
+
             let dt = headers.first().unwrap().timestamp - headers.last().unwrap().timestamp;
             let n = headers.len() - 1;
-            dt.as_u64() as f64 / n as f64
-        } else {
-            0.0
-        };
-        (max, min, avg)
+            let avg = dt.as_u64() as f64 / n as f64;
+
+            (max, min, avg)
+        }
     }
 
     /// Provides a hash of the header, used for the merge mining.
@@ -385,8 +385,36 @@ mod test {
     fn timing_empty_list() {
         let (max, min, avg) = BlockHeader::timing_stats(&[]);
         assert_eq!(max, 0);
-        assert_eq!(min, std::u64::MAX);
+        assert_eq!(min, 0);
         let error_margin = f64::EPSILON; // Use machine epsilon for comparison of floats
         assert!((avg - 0f64).abs() < error_margin);
     }
+
+    #[test]
+    fn timing_one_block() {
+        let header = BlockHeader {
+            timestamp: EpochTime::from(0),
+            ..BlockHeader::default()
+        };
+
+        let (max, min, avg) = BlockHeader::timing_stats(&[header]);
+        assert_eq!((max, min), (0, 0));
+        assert!((avg - 0f64).abs() < f64::EPSILON);
+    }
+
+    #[test]
+    fn timing_two_blocks() {
+        let headers = vec![150, 90]
+            .into_iter()
+            .map(|t| BlockHeader {
+                timestamp: EpochTime::from(t),
+                ..BlockHeader::default()
+            })
+            .collect::<Vec<BlockHeader>>();
+        let (max, min, avg) = dbg!(BlockHeader::timing_stats(&headers));
+        assert_eq!(max, 60);
+        assert_eq!(min, 60);
+        let error_margin = f64::EPSILON; // Use machine epsilon for comparison of floats
+        assert!((avg - 60f64).abs() < error_margin);
+    }
 }