From 2eb647cd8397b95b36c1f946d728fc86b5ede034 Mon Sep 17 00:00:00 2001 From: D-Stacks <78099568+D-Stacks@users.noreply.github.com> Date: Thu, 19 Sep 2024 21:31:13 +0200 Subject: [PATCH] Optionalize limts. --- components/consensusmanager/src/session.rs | 8 ++++++-- consensus/core/src/api/mod.rs | 10 +++++++--- consensus/src/consensus/mod.rs | 14 ++++++++++---- .../src/pipeline/virtual_processor/processor.rs | 2 +- consensus/src/processes/traversal_manager.rs | 6 +++--- rpc/service/src/converter/consensus.rs | 2 +- rpc/service/src/service.rs | 4 ++-- 7 files changed, 30 insertions(+), 16 deletions(-) diff --git a/components/consensusmanager/src/session.rs b/components/consensusmanager/src/session.rs index 830c1c672..2643739ee 100644 --- a/components/consensusmanager/src/session.rs +++ b/components/consensusmanager/src/session.rs @@ -267,7 +267,11 @@ impl ConsensusSessionOwned { self.clone().spawn_blocking(|c| c.is_nearly_synced()).await } - pub async fn async_get_virtual_chain_from_block(&self, low: Hash, chain_path_added_limit: usize) -> ConsensusResult { + pub async fn async_get_virtual_chain_from_block( + &self, + low: Hash, + chain_path_added_limit: Option, + ) -> ConsensusResult { self.clone().spawn_blocking(move |c| c.get_virtual_chain_from_block(low, chain_path_added_limit)).await } @@ -383,7 +387,7 @@ impl ConsensusSessionOwned { pub async fn async_get_blocks_acceptance_data( &self, hashes: Vec, - merged_blocks_limit: usize, + merged_blocks_limit: Option, ) -> ConsensusResult>> { self.clone().spawn_blocking(move |c| c.get_blocks_acceptance_data(&hashes, merged_blocks_limit)).await } diff --git a/consensus/core/src/api/mod.rs b/consensus/core/src/api/mod.rs index 4daa7fe19..91165b73d 100644 --- a/consensus/core/src/api/mod.rs +++ b/consensus/core/src/api/mod.rs @@ -161,8 +161,8 @@ pub trait ConsensusApi: Send + Sync { /// /// Note: /// 1) `chain_path_added_limit` will populate removed fully, and then the added chain path, up to `chain_path_added_limit` amount of hashes. - /// 1.1) use `usize::MAX` to impose no limit with optimized backward chain iteration, for better performance in cases where batching is not required. - fn get_virtual_chain_from_block(&self, low: Hash, chain_path_added_limit: usize) -> ConsensusResult { + /// 1.1) use `None to impose no limit with optimized backward chain iteration, for better performance in cases where batching is not required. + fn get_virtual_chain_from_block(&self, low: Hash, chain_path_added_limit: Option) -> ConsensusResult { unimplemented!() } @@ -302,7 +302,11 @@ pub trait ConsensusApi: Send + Sync { /// Returns acceptance data for a set of blocks belonging to the selected parent chain. /// /// See `self::get_virtual_chain` - fn get_blocks_acceptance_data(&self, hashes: &[Hash], merged_blocks_limit: usize) -> ConsensusResult>> { + fn get_blocks_acceptance_data( + &self, + hashes: &[Hash], + merged_blocks_limit: Option, + ) -> ConsensusResult>> { unimplemented!() } diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index 72e34571c..1731729a3 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -607,13 +607,13 @@ impl ConsensusApi for Consensus { self.config.is_nearly_synced(compact.timestamp, compact.daa_score) } - fn get_virtual_chain_from_block(&self, low: Hash, chain_path_added_limit: usize) -> ConsensusResult { + fn get_virtual_chain_from_block(&self, low: Hash, chain_path_added_limit: Option) -> ConsensusResult { // Calculate chain changes between the given `low` and the current sink hash (up to `limit` amount of block hashes). // Note: // 1) that we explicitly don't // do the calculation against the virtual itself so that we // won't later need to remove it from the result. - // 2) supplying `usize::MAX` as `chain_path_added_limit` will result in the full chain path, with optimized performance. + // 2) supplying `None` as `chain_path_added_limit` will result in the full chain path, with optimized performance. let _guard = self.pruning_lock.blocking_read(); // Verify that the block exists @@ -926,17 +926,23 @@ impl ConsensusApi for Consensus { self.acceptance_data_store.get(hash).unwrap_option().ok_or(ConsensusError::MissingData(hash)) } - fn get_blocks_acceptance_data(&self, hashes: &[Hash], merged_blocks_limit: usize) -> ConsensusResult>> { + fn get_blocks_acceptance_data( + &self, + hashes: &[Hash], + merged_blocks_limit: Option, + ) -> ConsensusResult>> { // Note: merged_blocks_limit will limit after the sum of merged blocks is breached along the supplied hash's acceptance data // and not limit the acceptance data within a queried hash. i.e. It has mergeset_size_limit granularity, this is to guarantee full acceptance data coverage. - if merged_blocks_limit == usize::MAX { + if merged_blocks_limit.is_none() { return hashes .iter() .copied() .map(|hash| self.acceptance_data_store.get(hash).unwrap_option().ok_or(ConsensusError::MissingData(hash))) .collect::>>(); } + let merged_blocks_limit = merged_blocks_limit.unwrap(); // we handle `is_none`, so may unwrap. let mut num_of_merged_blocks = 0usize; + hashes .iter() .copied() diff --git a/consensus/src/pipeline/virtual_processor/processor.rs b/consensus/src/pipeline/virtual_processor/processor.rs index d8cec5ff6..88fee97bf 100644 --- a/consensus/src/pipeline/virtual_processor/processor.rs +++ b/consensus/src/pipeline/virtual_processor/processor.rs @@ -290,7 +290,7 @@ impl VirtualStateProcessor { assert_eq!(virtual_ghostdag_data.selected_parent, new_sink); let sink_multiset = self.utxo_multisets_store.get(new_sink).unwrap(); - let chain_path = self.dag_traversal_manager.calculate_chain_path(prev_sink, new_sink, usize::MAX); + let chain_path = self.dag_traversal_manager.calculate_chain_path(prev_sink, new_sink, None); let new_virtual_state = self .calculate_and_commit_virtual_state( virtual_read, diff --git a/consensus/src/processes/traversal_manager.rs b/consensus/src/processes/traversal_manager.rs index ff958242c..23dc5c69f 100644 --- a/consensus/src/processes/traversal_manager.rs +++ b/consensus/src/processes/traversal_manager.rs @@ -31,7 +31,7 @@ impl ChainPath { + pub fn calculate_chain_path(&self, from: Hash, to: Hash, chain_path_added_limit: Option) -> ChainPath { let mut removed = Vec::new(); let mut common_ancestor = from; for current in self.reachability_service.default_backward_chain_iterator(from) { @@ -42,7 +42,7 @@ impl, ) -> RpcResult> { let acceptance_data = consensus.async_get_blocks_acceptance_data(chain_path.added.clone(), merged_blocks_limit).await.unwrap(); Ok(chain_path diff --git a/rpc/service/src/service.rs b/rpc/service/src/service.rs index 109e1496a..bdc2a9541 100644 --- a/rpc/service/src/service.rs +++ b/rpc/service/src/service.rs @@ -616,11 +616,11 @@ NOTE: This error usually indicates an RPC conversion error between the node and // else it returns the batch_size amount on pure chain blocks. // Note: batch_size does not bound removed chain blocks, only added chain blocks. let batch_size = (self.config.mergeset_size_limit * 10) as usize; - let mut virtual_chain_batch = session.async_get_virtual_chain_from_block(request.start_hash, batch_size).await?; + let mut virtual_chain_batch = session.async_get_virtual_chain_from_block(request.start_hash, Some(batch_size)).await?; let accepted_transaction_ids = if request.include_accepted_transaction_ids { let accepted_transaction_ids = self .consensus_converter - .get_virtual_chain_accepted_transaction_ids(&session, &virtual_chain_batch, batch_size) + .get_virtual_chain_accepted_transaction_ids(&session, &virtual_chain_batch, Some(batch_size)) .await?; // bound added to the length of the accepted transaction ids, which is bounded by merged blocks virtual_chain_batch.added = virtual_chain_batch.added[..accepted_transaction_ids.len()].to_vec();