fix(core): always pass the correct timestamp window to header validat…

…ior (tari-project#5624)

Description
---
fix(core): sanity check for timestamp window length checks for the
correct length of the timestamp window
fix(core): always include the correct timestamp window to header
validation
tests(core): add a reorg test of greater size than median timestamp
window size
chore: remove allocations when logging hashes in blockchain database  

Motivation and Context
---
`sanity_check_timestamp_count` (previously `check_timestamp_count`)
allowed timestamp windows of `min(median_window_size, header.height -
1)` or greater. However, the window size must always be the correct size
for a given block. If it is not, the median timestamp calculation will
be incorrect. This check also subtracted 1 from the header height BEFORE
checking the height is correct. This would cause a panic/underflow if a
header of height 0 is passed in.

In the reorg logic, we passed in the same timestamp window for the
candidate block when validating orphan chains, this could cause correct
reorgs to fail. This PR also ensures that timestamps are sorted as this
is required for a correct median calculation.

How Has This Been Tested?
---
Adds a new unit test
`it_does_a_sanity_check_on_the_number_of_timestamps_provided`
Adds block chain unit test
`it_links_many_orphan_branches_to_main_chain_with_greater_reorg_than_median_timestamp_window`
Existing test `it_links_many_orphan_branches_to_main_chain` failed after
the fix to `check_timestamp_count`. Fixed in this PR
Manually caused a about 90 block reorg on localnet nodes

What process can a PR reviewer use to test or verify this change?
---
Create a reorg using localnet
<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->

applications/minotari_app_grpc/src/conversions/historical_block.rs

@@ -31,7 +31,7 @@ impl TryFrom<HistoricalBlock> for grpc::HistoricalBlock {
 
     fn try_from(hb: HistoricalBlock) -> Result<Self, Self::Error> {
         Ok(Self {
-            confirmations: hb.confirmations,
+            confirmations: hb.confirmations(),
             block: Some(
                 hb.try_into_block()?
                     .try_into()

applications/minotari_node/src/commands/command/period_stats.rs

@@ -113,7 +113,7 @@ impl CommandContext {
             } else {
                 block.header().timestamp.as_u64() - prev_block.header().timestamp.as_u64()
             };
-            let diff = block.accumulated_data.target_difficulty.as_u64();
+            let diff = block.accumulated_data().target_difficulty.as_u64();
             period_difficulty += diff;
             period_solvetime += st;
             period_hash += diff as f64 / st as f64 / 1_000_000.0;

base_layer/core/src/base_node/sync/header_sync/validator.rs

@@ -138,7 +138,7 @@ impl<B: BlockchainBackend + 'static> BlockHeaderSyncValidator<B> {
             // We dont want to mark a block as bad for internal failures
             Err(
                 e @ ValidationError::FatalStorageError(_) |
-                e @ ValidationError::NotEnoughTimestamps { .. } |
+                e @ ValidationError::IncorrectNumberOfTimestampsProvided { .. } |
                 e @ ValidationError::AsyncTaskFailed(_),
             ) => return Err(e.into()),
             // We dont have to mark the block twice
@@ -159,12 +159,14 @@ impl<B: BlockchainBackend + 'static> BlockHeaderSyncValidator<B> {
         state.previous_header = header.clone();
 
         // Ensure that timestamps are inserted in sorted order
-        let maybe_index = state.timestamps.iter().position(|ts| ts >= &header.timestamp());
+        let maybe_index = state.timestamps.iter().position(|ts| *ts >= header.timestamp());
         match maybe_index {
             Some(pos) => {
                 state.timestamps.insert(pos, header.timestamp());
             },
-            None => state.timestamps.push(header.timestamp()),
+            None => {
+                state.timestamps.push(header.timestamp());
+            },
         }
 
         state.current_height = header.height;

base_layer/core/src/blocks/historical_block.rs

@@ -34,11 +34,11 @@ use crate::blocks::{error::BlockError, Block, BlockHeader, BlockHeaderAccumulate
 pub struct HistoricalBlock {
     /// The number of blocks that have been mined since this block, including this one. The current tip will have one
     /// confirmation.
-    pub confirmations: u64,
+    confirmations: u64,
     /// The underlying block
     block: Block,
     /// Accumulated data in the block header
-    pub accumulated_data: BlockHeaderAccumulatedData,
+    accumulated_data: BlockHeaderAccumulatedData,
     pruned_outputs: Vec<HashOutput>,
     pruned_input_count: u64,
 }
@@ -73,6 +73,14 @@ impl HistoricalBlock {
         &self.block
     }
 
+    pub fn into_block(self) -> Block {
+        self.block
+    }
+
+    pub fn accumulated_data(&self) -> &BlockHeaderAccumulatedData {
+        &self.accumulated_data
+    }
+
     pub fn hash(&self) -> &HashOutput {
         &self.accumulated_data.hash
     }