From 4947df5259282efbd2048fd50dce6717d53d8118 Mon Sep 17 00:00:00 2001 From: SW van Heerden Date: Wed, 25 Oct 2023 09:45:44 +0200 Subject: [PATCH] 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 --- .../src/base_node/sync/header_sync/mod.rs | 2 +- .../sync/header_sync/synchronizer.rs | 21 ++++++++++++------- .../base_node/sync/header_sync/validator.rs | 4 ++-- .../core/src/base_node/sync/rpc/service.rs | 10 +++++---- .../src/chain_storage/blockchain_database.rs | 2 +- .../proof_of_work/target_difficulty_window.rs | 3 ++- 6 files changed, 25 insertions(+), 17 deletions(-) diff --git a/base_layer/core/src/base_node/sync/header_sync/mod.rs b/base_layer/core/src/base_node/sync/header_sync/mod.rs index da4f567664..3093cf8641 100644 --- a/base_layer/core/src/base_node/sync/header_sync/mod.rs +++ b/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; diff --git a/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs b/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs index e250539e1a..f8b16ceca9 100644 --- a/base_layer/core/src/base_node/sync/header_sync/synchronizer.rs +++ b/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 { diff --git a/base_layer/core/src/base_node/sync/header_sync/validator.rs b/base_layer/core/src/base_node/sync/header_sync/validator.rs index d086bcd0a0..d1140bb092 100644 --- a/base_layer/core/src/base_node/sync/header_sync/validator.rs +++ b/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 BlockHeaderSyncValidator { 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(()) diff --git a/base_layer/core/src/base_node/sync/rpc/service.rs b/base_layer/core/src/base_node/sync/rpc/service.rs index bee93f810e..e3ea5fa9c8 100644 --- a/base_layer/core/src/base_node/sync/rpc/service.rs +++ b/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 BaseNodeSyncService for BaseNodeSyncRpcServ request: Request, ) -> Result, 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 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, ))); } diff --git a/base_layer/core/src/chain_storage/blockchain_database.rs b/base_layer/core/src/chain_storage/blockchain_database.rs index 4e8547d4e9..1af0bbf972 100644 --- a/base_layer/core/src/chain_storage/blockchain_database.rs +++ b/base_layer/core/src/chain_storage/blockchain_database.rs @@ -728,7 +728,7 @@ where B: BlockchainBackend ) -> Result { let db = self.db_read_access()?; let mut current_header = db.fetch_chain_header_in_all_chains(¤t_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( diff --git a/base_layer/core/src/proof_of_work/target_difficulty_window.rs b/base_layer/core/src/proof_of_work/target_difficulty_window.rs index 5275a8a19c..06923b6a8e 100644 --- a/base_layer/core/src/proof_of_work/target_difficulty_window.rs +++ b/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)) } }