diff --git a/applications/tari_app_grpc/proto/base_node.proto b/applications/tari_app_grpc/proto/base_node.proto index 09c3fa6064..e55ed9db02 100644 --- a/applications/tari_app_grpc/proto/base_node.proto +++ b/applications/tari_app_grpc/proto/base_node.proto @@ -33,10 +33,6 @@ service BaseNode { rpc GetHeaderByHash(GetHeaderByHashRequest) returns (BlockHeaderResponse); // 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) { - option deprecated = true; - }; // Returns the block timing for the chain heights rpc GetBlockTiming(HeightRequest) returns (BlockTimingResponse); // Returns the network Constants @@ -234,14 +230,6 @@ message HeightRequest { uint64 end_height = 3; } -// 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; diff --git a/applications/tari_base_node/src/commands/command/mod.rs b/applications/tari_base_node/src/commands/command/mod.rs index 80fcee6a95..e32b908ef0 100644 --- a/applications/tari_base_node/src/commands/command/mod.rs +++ b/applications/tari_base_node/src/commands/command/mod.rs @@ -118,7 +118,6 @@ pub enum Command { PeriodStats(period_stats::Args), HeaderStats(header_stats::Args), BlockTiming(block_timing::Args), - CalcTiming(block_timing::Args), ListReorgs(list_reorgs::Args), DiscoverPeer(discover_peer::Args), GetBlock(get_block::Args), @@ -234,7 +233,7 @@ impl HandleCommand for CommandContext { Command::CheckDb(args) => self.handle_command(args).await, Command::PeriodStats(args) => self.handle_command(args).await, Command::HeaderStats(args) => self.handle_command(args).await, - Command::BlockTiming(args) | Command::CalcTiming(args) => self.handle_command(args).await, + Command::BlockTiming(args) => self.handle_command(args).await, Command::ListReorgs(args) => self.handle_command(args).await, Command::DiscoverPeer(args) => self.handle_command(args).await, Command::GetBlock(args) => self.handle_command(args).await, diff --git a/applications/tari_base_node/src/grpc/base_node_grpc_server.rs b/applications/tari_base_node/src/grpc/base_node_grpc_server.rs index 2ef8258366..6f328b4404 100644 --- a/applications/tari_base_node/src/grpc/base_node_grpc_server.rs +++ b/applications/tari_base_node/src/grpc/base_node_grpc_server.rs @@ -80,6 +80,8 @@ const LIST_HEADERS_PAGE_SIZE: usize = 10; // The `num_headers` value if none is provided. const LIST_HEADERS_DEFAULT_NUM_HEADERS: u64 = 10; +const BLOCK_TIMING_MAX_BLOCKS: u64 = 10_000; + pub struct BaseNodeGrpcServer { node_service: LocalNodeCommsInterface, mempool_service: LocalMempoolService, @@ -1153,22 +1155,6 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer { Ok(Response::new(rx)) } - // deprecated - async fn get_calc_timing( - &self, - request: Request, - ) -> Result, 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, @@ -1185,8 +1171,19 @@ impl tari_rpc::base_node_server::BaseNode for BaseNodeGrpcServer { let mut handler = self.node_service.clone(); let (start, end) = get_heights(&request, handler.clone()).await?; + let num_requested = end.saturating_sub(start); + if num_requested > BLOCK_TIMING_MAX_BLOCKS { + warn!( + target: LOG_TARGET, + "GetBlockTiming request for too many blocks. Requested: {}. Max: {}.", + num_requested, + BLOCK_TIMING_MAX_BLOCKS + ); + return Err(Status::invalid_argument("Max request size exceeded.")); + } + let headers = match handler.get_headers(start..=end).await { - Ok(headers) => headers.into_iter().map(|h| h.into_header()).collect::>(), + Ok(headers) => headers.into_iter().map(|h| h.into_header()).rev().collect::>(), Err(err) => { warn!(target: LOG_TARGET, "Error getting headers for GRPC client: {}", err); Vec::new() diff --git a/base_layer/core/src/blocks/block_header.rs b/base_layer/core/src/blocks/block_header.rs index 25fae3882c..99e962064c 100644 --- a/base_layer/core/src/blocks/block_header.rs +++ b/base_layer/core/src/blocks/block_header.rs @@ -38,6 +38,7 @@ //! This hash is called the UTXO merkle root, and is used as the output_mr use std::{ + cmp::Ordering, fmt, fmt::{Display, Error, Formatter}, }; @@ -173,11 +174,25 @@ impl BlockHeader { BlockBuilder::new(self.version).with_header(self) } - /// Given a slice of headers (in reverse order), calculate the maximum, minimum and average periods between them + /// Given a slice of headers, calculate the maximum, minimum and average periods between them. + /// Expects the slice of headers to be ordered from youngest to oldest, but will reverse them if not. + /// This function always allocates a vec of the slice length. This is in case it needs to reverse the list. pub fn timing_stats(headers: &[BlockHeader]) -> (u64, u64, f64) { if headers.len() < 2 { (0, 0, 0.0) } else { + let mut headers = headers.to_vec(); + + // ensure the slice is in reverse order + let ordering = headers[0].timestamp.cmp(&headers[headers.len() - 1].timestamp); + if ordering == Ordering::Less { + headers.reverse(); + } + + // unwraps: length already checked + let last_ts = headers.first().unwrap().timestamp; + let first_ts = headers.last().unwrap().timestamp; + 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(), @@ -186,7 +201,10 @@ impl BlockHeader { (max.max(dt), min.min(dt)) }); - let dt = headers.first().unwrap().timestamp - headers.last().unwrap().timestamp; + let dt = match last_ts.checked_sub(first_ts) { + Some(t) => t, + None => 0.into(), + }; let n = headers.len() - 1; let avg = dt.as_u64() as f64 / n as f64; @@ -384,6 +402,8 @@ pub(crate) mod hash_serializer { #[cfg(test)] mod test { + use std::cmp::Ordering; + use tari_crypto::tari_utilities::Hashable; use crate::blocks::BlockHeader; @@ -461,10 +481,46 @@ mod test { ..BlockHeader::default() }) .collect::>(); - let (max, min, avg) = dbg!(BlockHeader::timing_stats(&headers)); + let (max, min, avg) = 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); + } + + #[test] + fn timing_wrong_order() { + let headers = vec![90, 150] + .into_iter() + .map(|t| BlockHeader { + timestamp: t.into(), + ..BlockHeader::default() + }) + .collect::>(); + let (max, min, avg) = 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); } + + #[test] + fn compare_timestamps() { + let headers = vec![90, 90, 150] + .into_iter() + .map(|t| BlockHeader { + timestamp: t.into(), + ..BlockHeader::default() + }) + .collect::>(); + + let ordering = headers[0].timestamp.cmp(&headers[1].timestamp); + assert_eq!(ordering, Ordering::Equal); + + let ordering = headers[1].timestamp.cmp(&headers[2].timestamp); + assert_eq!(ordering, Ordering::Less); + + let ordering = headers[2].timestamp.cmp(&headers[0].timestamp); + assert_eq!(ordering, Ordering::Greater); + } } diff --git a/base_layer/core/src/transactions/test_helpers.rs b/base_layer/core/src/transactions/test_helpers.rs index 92818c9d0f..ee0e00425c 100644 --- a/base_layer/core/src/transactions/test_helpers.rs +++ b/base_layer/core/src/transactions/test_helpers.rs @@ -542,7 +542,7 @@ pub fn create_unblinded_txos( (inputs, outputs) } -/// Create an unconfirmed transaction for testing with a valid fee, unique access_sig, random inputs and outputs, the +/// Create an unconfirmed transaction for testing with a valid fee, unique excess_sig, random inputs and outputs, the /// transaction is only partially constructed pub fn create_transaction_with( lock_height: u64, diff --git a/base_layer/core/src/validation/test.rs b/base_layer/core/src/validation/test.rs index 9be64de586..d94aeae1ec 100644 --- a/base_layer/core/src/validation/test.rs +++ b/base_layer/core/src/validation/test.rs @@ -125,11 +125,9 @@ mod header_validators { } #[test] -// TODO: Fix this test with the new DB structure -#[ignore = "to be fixed with new db structure"] fn chain_balance_validation() { let factories = CryptoFactories::default(); - let consensus_manager = ConsensusManagerBuilder::new(Network::Weatherwax).build(); + let consensus_manager = ConsensusManagerBuilder::new(Network::Dibbler).build(); let genesis = consensus_manager.get_genesis_block(); let faucet_value = 5000 * uT; let (faucet_utxo, faucet_key, _) = create_utxo(