chore: few small code improvements (#5864)

Description
---
Ensures that both header_sync and its rpc methods use the same
constants.
Improves the code readability 

Motivation and Context
---
These changes where discussed and brought up during the hazop code
meeting

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

@@ -19,7 +19,7 @@
 //  SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
 //  WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
 //  USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
+pub const HEADER_SYNC_INITIAL_MAX_HEADERS: usize = 1000;
 mod error;
 pub use error::BlockHeaderSyncError;
 

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

@@ -38,7 +38,14 @@ use tari_utilities::hex::Hex;
 
 use super::{validator::BlockHeaderSyncValidator, BlockHeaderSyncError};
 use crate::{
-    base_node::sync::{ban::PeerBanManager, hooks::Hooks, rpc, BlockchainSyncConfig, SyncPeer},
+    base_node::sync::{
+        ban::PeerBanManager,
+        header_sync::HEADER_SYNC_INITIAL_MAX_HEADERS,
+        hooks::Hooks,
+        rpc,
+        BlockchainSyncConfig,
+        SyncPeer,
+    },
     blocks::{BlockHeader, ChainBlock, ChainHeader},
     chain_storage::{async_db::AsyncBlockchainDb, BlockchainBackend, ChainStorageError},
     common::rolling_avg::RollingAverageTime,
@@ -52,8 +59,6 @@ use crate::{
 
 const LOG_TARGET: &str = "c::bn::header_sync";
 
-const NUM_INITIAL_HEADERS_TO_REQUEST: usize = 1000;
-
 const MAX_LATENCY_INCREASES: usize = 5;
 
 pub struct HeaderSynchronizer<'a, B> {
@@ -387,13 +392,13 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> {
                     return Err(err.into());
                 },
             };
-            if resp.headers.len() > NUM_INITIAL_HEADERS_TO_REQUEST {
+            if resp.headers.len() > HEADER_SYNC_INITIAL_MAX_HEADERS {
                 warn!(
                     target: LOG_TARGET,
                     "Peer `{}` sent too many headers {}, only requested {}. Peer will be banned.",
                     peer_node_id,
                     resp.headers.len(),
-                    NUM_INITIAL_HEADERS_TO_REQUEST,
+                    HEADER_SYNC_INITIAL_MAX_HEADERS,
                 );
                 return Err(BlockHeaderSyncError::PeerSentTooManyHeaders(resp.headers.len()));
             }
@@ -448,7 +453,7 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> {
     ) -> Result<(HeaderSyncStatus, FindChainSplitResult), BlockHeaderSyncError> {
         // This method will return ban-able errors for certain offenses.
         let chain_split_result = self
-            .find_chain_split(sync_peer.node_id(), client, NUM_INITIAL_HEADERS_TO_REQUEST as u64)
+            .find_chain_split(sync_peer.node_id(), client, HEADER_SYNC_INITIAL_MAX_HEADERS as u64)
             .await?;
         if chain_split_result.reorg_steps_back > 0 {
             debug!(
@@ -572,7 +577,7 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> {
             .expect("synchronize_headers: expected there to be a valid tip header but it was None");
 
         // If we already have a stronger chain at this point, switch over to it.
-        // just in case we happen to be exactly NUM_INITIAL_HEADERS_TO_REQUEST headers behind.
+        // just in case we happen to be exactly HEADER_SYNC_INITIAL_MAX_HEADERS headers behind.
         let has_better_pow = self.pending_chain_has_higher_pow(&split_info.best_block_header);
 
         if has_better_pow {
@@ -585,7 +590,7 @@ impl<'a, B: BlockchainBackend + 'static> HeaderSynchronizer<'a, B> {
             has_switched_to_new_chain = true;
         }
 
-        if pending_len < NUM_INITIAL_HEADERS_TO_REQUEST {
+        if pending_len < HEADER_SYNC_INITIAL_MAX_HEADERS {
             // Peer returned less than the max number of requested headers. This indicates that we have all the
             // available headers from the peer.
             if !has_better_pow {

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

@@ -26,7 +26,7 @@ use tari_common_types::types::HashOutput;
 use tari_utilities::{epoch_time::EpochTime, hex::Hex};
 
 use crate::{
-    base_node::sync::BlockHeaderSyncError,
+    base_node::sync::{header_sync::HEADER_SYNC_INITIAL_MAX_HEADERS, BlockHeaderSyncError},
     blocks::{BlockHeader, BlockHeaderAccumulatedData, BlockHeaderValidationError, ChainHeader},
     chain_storage::{async_db::AsyncBlockchainDb, BlockchainBackend, ChainStorageError, TargetDifficulties},
     common::rolling_vec::RollingVec,
@@ -99,7 +99,7 @@ impl<B: BlockchainBackend + 'static> BlockHeaderSyncValidator<B> {
             previous_accum,
             previous_header: start_header,
             // One large allocation is usually better even if it is not always used.
-            valid_headers: Vec::with_capacity(1000),
+            valid_headers: Vec::with_capacity(HEADER_SYNC_INITIAL_MAX_HEADERS),
         });
 
         Ok(())

base_layer/core/src/base_node/sync/rpc/service.rs

@@ -44,7 +44,10 @@ use crate::{
     base_node::{
         comms_interface::BlockEvent,
         metrics,
-        sync::rpc::{sync_utxos_task::SyncUtxosTask, BaseNodeSyncService},
+        sync::{
+            header_sync::HEADER_SYNC_INITIAL_MAX_HEADERS,
+            rpc::{sync_utxos_task::SyncUtxosTask, BaseNodeSyncService},
+        },
         LocalNodeCommsInterface,
     },
     chain_storage::{async_db::AsyncBlockchainDb, BlockAddResult, BlockchainBackend},
@@ -383,7 +386,6 @@ impl<B: BlockchainBackend + 'static> BaseNodeSyncService for BaseNodeSyncRpcServ
         request: Request<FindChainSplitRequest>,
     ) -> Result<Response<FindChainSplitResponse>, RpcStatus> {
         const MAX_ALLOWED_BLOCK_HASHES: usize = 1000;
-        const MAX_ALLOWED_HEADER_COUNT: u64 = 1000;
 
         let peer = request.context().peer_node_id().clone();
         let message = request.into_message();
@@ -398,10 +400,10 @@ impl<B: BlockchainBackend + 'static> BaseNodeSyncService for BaseNodeSyncRpcServ
                 MAX_ALLOWED_BLOCK_HASHES,
             )));
         }
-        if message.header_count > MAX_ALLOWED_HEADER_COUNT {
+        if message.header_count > (HEADER_SYNC_INITIAL_MAX_HEADERS as u64) {
             return Err(RpcStatus::bad_request(&format!(
                 "Cannot ask for more than {} headers",
-                MAX_ALLOWED_HEADER_COUNT,
+                HEADER_SYNC_INITIAL_MAX_HEADERS,
             )));
         }
 

base_layer/core/src/chain_storage/blockchain_database.rs

@@ -728,7 +728,7 @@ where B: BlockchainBackend
     ) -> Result<TargetDifficulties, ChainStorageError> {
         let db = self.db_read_access()?;
         let mut current_header = db.fetch_chain_header_in_all_chains(&current_block_hash)?;
-        let mut targets = TargetDifficulties::new(&self.consensus_manager, current_header.height() + 1)
+        let mut targets = TargetDifficulties::new(&self.consensus_manager, current_header.height().saturating_add(1))
             .map_err(ChainStorageError::UnexpectedResult)?;
         // Add start header since we have it on hand
         targets.add_front(

base_layer/core/src/proof_of_work/target_difficulty_window.rs

@@ -73,7 +73,8 @@ impl TargetDifficultyWindow {
 
     /// Calculates the target difficulty for the current set of target difficulties.
     pub fn calculate(&self, min: Difficulty, max: Difficulty) -> Difficulty {
-        cmp::max(min, cmp::min(max, self.lwma.get_difficulty().unwrap_or(min)))
+        let difficulty = self.lwma.get_difficulty().unwrap_or(min);
+        cmp::max(min, cmp::min(max, difficulty))
     }
 }