From 964a7854dd8f7aa964bc40b18cf35c46333ac227 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 28 Sep 2023 16:11:28 +1000 Subject: [PATCH 01/17] Handle negative and zero getnetworksolsps arguments correctly --- zebra-chain/src/block/height.rs | 8 ++++ .../src/methods/get_block_template_rpcs.rs | 44 ++++++++++++------- .../get_block_template_rpcs/constants.rs | 2 +- zebra-rpc/src/methods/tests/vectors.rs | 25 ++++++----- zebra-state/src/request.rs | 5 ++- zebra-state/src/service/read/difficulty.rs | 3 ++ 6 files changed, 59 insertions(+), 28 deletions(-) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index c7363a8cff0..b06a9623291 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -169,6 +169,14 @@ impl TryIntoHeight for String { } } +impl TryIntoHeight for i32 { + type Error = BoxError; + + fn try_into_height(&self) -> Result { + u32::try_from(*self)?.try_into().map_err(Into::into) + } +} + // We don't implement Add or Sub, because they cause type inference issues for integer constants. impl Sub for Height { diff --git a/zebra-rpc/src/methods/get_block_template_rpcs.rs b/zebra-rpc/src/methods/get_block_template_rpcs.rs index 4e1ae2c61bd..a970f8aaac3 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs.rs @@ -11,10 +11,10 @@ use zcash_address::{self, unified::Encoding, TryFromAddress}; use zebra_chain::{ amount::Amount, - block::{self, Block, Height}, + block::{self, Block, Height, TryIntoHeight}, chain_sync_status::ChainSyncStatus, chain_tip::ChainTip, - parameters::Network, + parameters::{Network, POW_AVERAGING_WINDOW}, primitives, serialization::ZcashDeserializeInto, transparent::{ @@ -142,27 +142,32 @@ pub trait GetBlockTemplateRpc { #[rpc(name = "getmininginfo")] fn get_mining_info(&self) -> BoxFuture>; - /// Returns the estimated network solutions per second based on the last `num_blocks` before `height`. - /// If `num_blocks` is not supplied, uses 120 blocks. - /// If `height` is not supplied or is 0, uses the tip height. + /// Returns the estimated network solutions per second based on the last `num_blocks` before + /// `height`. + /// + /// If `num_blocks` is not supplied, uses 120 blocks. If it is 0 or -1, uses the difficulty + /// averaging window. + /// If `height` is not supplied or is -1, uses the tip height. /// /// zcashd reference: [`getnetworksolps`](https://zcash.github.io/rpc/getnetworksolps.html) #[rpc(name = "getnetworksolps")] fn get_network_sol_ps( &self, - num_blocks: Option, + num_blocks: Option, height: Option, ) -> BoxFuture>; - /// Returns the estimated network solutions per second based on the last `num_blocks` before `height`. - /// If `num_blocks` is not supplied, uses 120 blocks. - /// If `height` is not supplied or is 0, uses the tip height. + /// Returns the estimated network solutions per second based on the last `num_blocks` before + /// `height`. + /// + /// This method name is deprecated, use [`getnetworksolps`](Self::get_network_sol_ps) instead. + /// See that method for details. /// /// zcashd reference: [`getnetworkhashps`](https://zcash.github.io/rpc/getnetworkhashps.html) #[rpc(name = "getnetworkhashps")] fn get_network_hash_ps( &self, - num_blocks: Option, + num_blocks: Option, height: Option, ) -> BoxFuture> { self.get_network_sol_ps(num_blocks, height) @@ -838,13 +843,22 @@ where fn get_network_sol_ps( &self, - num_blocks: Option, + num_blocks: Option, height: Option, ) -> BoxFuture> { - let num_blocks = num_blocks - .map(|num_blocks| num_blocks.max(1)) - .unwrap_or(DEFAULT_SOLUTION_RATE_WINDOW_SIZE); - let height = height.and_then(|height| (height > 1).then_some(Height(height as u32))); + // Default number of blocks is 120 if not supplied. + let mut num_blocks = num_blocks.unwrap_or(DEFAULT_SOLUTION_RATE_WINDOW_SIZE); + // But if it is 0 or negative, it uses the proof of work averaging window. + if num_blocks < 1 { + num_blocks = i32::try_from(POW_AVERAGING_WINDOW).expect("fits in i32"); + } + let num_blocks = + usize::try_from(num_blocks).expect("just checked for negatives, i32 fits in usize"); + + // Default height is the tip height if not supplied. Negative values also mean the tip + // height. Since negative values aren't valid heights, we can just use the conversion. + let height = height.and_then(|height| height.try_into_height().ok()); + let mut state = self.state.clone(); async move { diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/constants.rs b/zebra-rpc/src/methods/get_block_template_rpcs/constants.rs index 8b6b9174a58..75b26055c8e 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs/constants.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs/constants.rs @@ -54,7 +54,7 @@ pub const NOT_SYNCED_ERROR_CODE: ErrorCode = ErrorCode::ServerError(-10); /// The default window size specifying how many blocks to check when estimating the chain's solution rate. /// /// Based on default value in zcashd. -pub const DEFAULT_SOLUTION_RATE_WINDOW_SIZE: usize = 120; +pub const DEFAULT_SOLUTION_RATE_WINDOW_SIZE: i32 = 120; /// The funding stream order in `zcashd` RPC responses. /// diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 9d89a459c72..d6f926ff051 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/zebra-rpc/src/methods/tests/vectors.rs @@ -1149,21 +1149,26 @@ async fn rpc_getnetworksolps() { let get_network_sol_ps_inputs = [ (None, None), + (Some(-4), None), + (Some(-3), Some(0)), + (Some(-2), Some(-4)), + (Some(-1), Some(10)), + (Some(-1), Some(i32::MAX)), (Some(0), None), (Some(0), Some(0)), - (Some(0), Some(-1)), + (Some(0), Some(-3)), (Some(0), Some(10)), (Some(0), Some(i32::MAX)), (Some(1), None), (Some(1), Some(0)), - (Some(1), Some(-1)), + (Some(1), Some(-2)), (Some(1), Some(10)), (Some(1), Some(i32::MAX)), - (Some(usize::MAX), None), - (Some(usize::MAX), Some(0)), - (Some(usize::MAX), Some(-1)), - (Some(usize::MAX), Some(10)), - (Some(usize::MAX), Some(i32::MAX)), + (Some(i32::MAX), None), + (Some(i32::MAX), Some(0)), + (Some(i32::MAX), Some(-1)), + (Some(i32::MAX), Some(10)), + (Some(i32::MAX), Some(i32::MAX)), ]; for (num_blocks_input, height_input) in get_network_sol_ps_inputs { @@ -1171,9 +1176,9 @@ async fn rpc_getnetworksolps() { .get_network_sol_ps(num_blocks_input, height_input) .await; assert!( - get_network_sol_ps_result - .is_ok(), - "get_network_sol_ps({num_blocks_input:?}, {height_input:?}) call with should be ok, got: {get_network_sol_ps_result:?}" + get_network_sol_ps_result.is_ok(), + "get_network_sol_ps({num_blocks_input:?}, {height_input:?}) call should be ok, \ + got: {get_network_sol_ps_result:?}" ); } } diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 700814ae1c9..f2513119e2d 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -938,9 +938,10 @@ pub enum ReadRequest { /// /// Returns [`ReadResponse::SolutionRate`] SolutionRate { - /// Specifies over difficulty averaging window. + /// The number of blocks to calculate the average difficulty for. num_blocks: usize, - /// Optionally estimate the network speed at the time when a certain block was found + /// Optionally estimate the network solution rate at the time when this height was mined. + /// Otherwise, estimate at the current tip height. height: Option, }, diff --git a/zebra-state/src/service/read/difficulty.rs b/zebra-state/src/service/read/difficulty.rs index b990daa5638..c3e544f0e69 100644 --- a/zebra-state/src/service/read/difficulty.rs +++ b/zebra-state/src/service/read/difficulty.rs @@ -89,6 +89,9 @@ pub fn solution_rate( ) -> Option { // Take 1 extra block for calculating the number of seconds between when mining on the first block likely started. // The work for the last block in this iterator is not added to `total_work`. + // + // Since we can't take more blocks than are actually in the chain, this automatically limits + // `num_blocks` to the chain length, like `zcashd` does. let mut block_iter = any_ancestor_blocks(non_finalized_state, db, start_hash) .take(num_blocks.checked_add(1).unwrap_or(num_blocks)) .peekable(); From 1986e3967661502acc3209d1555a3d8ba1a89d8e Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 28 Sep 2023 16:20:18 +1000 Subject: [PATCH 02/17] Simplify the solsps loop structure --- zebra-state/src/service/read/difficulty.rs | 29 ++++++++++------------ 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/zebra-state/src/service/read/difficulty.rs b/zebra-state/src/service/read/difficulty.rs index c3e544f0e69..5c5e1555572 100644 --- a/zebra-state/src/service/read/difficulty.rs +++ b/zebra-state/src/service/read/difficulty.rs @@ -109,23 +109,20 @@ pub fn solution_rate( let mut total_work: PartialCumulativeWork = get_work(block).into(); - loop { - // Return `None` if the iterator doesn't yield a second item. - let block = block_iter.next()?; - - if block_iter.peek().is_some() { - // Add the block's work to `total_work` if it's not the last item in the iterator. - // The last item in the iterator is only used to estimate when mining on the first block - // in the window of `num_blocks` likely started. - total_work += get_work(block); - } else { - let first_block_time = block.header.time; - let duration_between_first_and_last_block = last_block_time - first_block_time; - return Some( - total_work.as_u128() / duration_between_first_and_last_block.num_seconds() as u128, - ); - } + // Return `None` if the iterator doesn't yield a second item. + let block = block_iter.next()?; + + while block_iter.peek().is_some() { + // Add the block's work to `total_work` if it's not the last item in the iterator. + // The last item in the iterator is only used to estimate when mining on the first block + // in the window of `num_blocks` likely started. + total_work += get_work(block); } + + let first_block_time = block.header.time; + let duration_between_first_and_last_block = last_block_time - first_block_time; + + Some(total_work.as_u128() / duration_between_first_and_last_block.num_seconds() as u128) } /// Do a consistency check by checking the finalized tip before and after all other database From 2f25bcaae3ec03c1750a0c08a9b0a24f4f8c279e Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 28 Sep 2023 16:38:14 +1000 Subject: [PATCH 03/17] Simplify loop by avoiding manual iteration and peeking --- zebra-state/src/service/read/difficulty.rs | 28 ++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/zebra-state/src/service/read/difficulty.rs b/zebra-state/src/service/read/difficulty.rs index 5c5e1555572..c78d6de6e2f 100644 --- a/zebra-state/src/service/read/difficulty.rs +++ b/zebra-state/src/service/read/difficulty.rs @@ -93,8 +93,7 @@ pub fn solution_rate( // Since we can't take more blocks than are actually in the chain, this automatically limits // `num_blocks` to the chain length, like `zcashd` does. let mut block_iter = any_ancestor_blocks(non_finalized_state, db, start_hash) - .take(num_blocks.checked_add(1).unwrap_or(num_blocks)) - .peekable(); + .take(num_blocks.checked_add(1).unwrap_or(num_blocks)); let get_work = |block: Arc| { block @@ -104,22 +103,27 @@ pub fn solution_rate( .expect("work has already been validated") }; - let block = block_iter.next()?; - let last_block_time = block.header.time; + let last_block = block_iter.next()?; + let last_block_time = last_block.header.time; - let mut total_work: PartialCumulativeWork = get_work(block).into(); + let mut total_work: PartialCumulativeWork = get_work(last_block).into(); - // Return `None` if the iterator doesn't yield a second item. - let block = block_iter.next()?; + // We need at least 2 blocks to calculate average work over time. + if block_iter.len() == 0 { + return None; + } - while block_iter.peek().is_some() { - // Add the block's work to `total_work` if it's not the last item in the iterator. - // The last item in the iterator is only used to estimate when mining on the first block - // in the window of `num_blocks` likely started. + // We added an extra block so we could estimate when mining on the first block + // in the window of `num_blocks` likely started. But we don't want to add the work + // for that block. Since we've already taken the first block, we need to subtract 1 here. + for block in (&mut block_iter).take(num_blocks.checked_sub(1)?) { total_work += get_work(block); } - let first_block_time = block.header.time; + let first_block = block_iter + .next() + .expect("already checked for at least 1 block"); + let first_block_time = first_block.header.time; let duration_between_first_and_last_block = last_block_time - first_block_time; Some(total_work.as_u128() / duration_between_first_and_last_block.num_seconds() as u128) From 7e1bb304a1414a61c0e8a3ec8dda239a76a78531 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 28 Sep 2023 16:38:33 +1000 Subject: [PATCH 04/17] Avoid division by zero --- zebra-state/src/service/read/difficulty.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/zebra-state/src/service/read/difficulty.rs b/zebra-state/src/service/read/difficulty.rs index c78d6de6e2f..9ad077756a9 100644 --- a/zebra-state/src/service/read/difficulty.rs +++ b/zebra-state/src/service/read/difficulty.rs @@ -126,6 +126,11 @@ pub fn solution_rate( let first_block_time = first_block.header.time; let duration_between_first_and_last_block = last_block_time - first_block_time; + // Avoid division by zero errors. + if duration_between_first_and_last_block.num_seconds() <= 0 { + return None; + } + Some(total_work.as_u128() / duration_between_first_and_last_block.num_seconds() as u128) } From 343ecbea0c860fba5b9d904d4f2276cd220925d7 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 28 Sep 2023 16:53:49 +1000 Subject: [PATCH 05/17] Use min and max times rather than first and last times --- zebra-chain/src/work/difficulty.rs | 10 +++++ zebra-state/src/service/read/difficulty.rs | 46 ++++++++++++---------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/zebra-chain/src/work/difficulty.rs b/zebra-chain/src/work/difficulty.rs index 8964ae23469..a15c1f13ce8 100644 --- a/zebra-chain/src/work/difficulty.rs +++ b/zebra-chain/src/work/difficulty.rs @@ -129,6 +129,11 @@ pub struct ExpandedDifficulty(U256); pub struct Work(u128); impl Work { + /// Returns a value representing no work. + pub fn zero() -> Self { + Self(0) + } + /// Return the inner `u128` value. pub fn as_u128(self) -> u128 { self.0 @@ -660,6 +665,11 @@ impl std::ops::Add for Work { pub struct PartialCumulativeWork(u128); impl PartialCumulativeWork { + /// Returns a value representing no work. + pub fn zero() -> Self { + Self(0) + } + /// Return the inner `u128` value. pub fn as_u128(self) -> u128 { self.0 diff --git a/zebra-state/src/service/read/difficulty.rs b/zebra-state/src/service/read/difficulty.rs index 9ad077756a9..6fa3dff1b4f 100644 --- a/zebra-state/src/service/read/difficulty.rs +++ b/zebra-state/src/service/read/difficulty.rs @@ -9,7 +9,7 @@ use zebra_chain::{ history_tree::HistoryTree, parameters::{Network, NetworkUpgrade, POST_BLOSSOM_POW_TARGET_SPACING}, serialization::{DateTime32, Duration32}, - work::difficulty::{CompactDifficulty, PartialCumulativeWork}, + work::difficulty::{CompactDifficulty, PartialCumulativeWork, Work}, }; use crate::{ @@ -93,9 +93,10 @@ pub fn solution_rate( // Since we can't take more blocks than are actually in the chain, this automatically limits // `num_blocks` to the chain length, like `zcashd` does. let mut block_iter = any_ancestor_blocks(non_finalized_state, db, start_hash) - .take(num_blocks.checked_add(1).unwrap_or(num_blocks)); + .take(num_blocks.checked_add(1).unwrap_or(num_blocks)) + .peekable(); - let get_work = |block: Arc| { + let get_work = |block: &Block| { block .header .difficulty_threshold @@ -103,35 +104,38 @@ pub fn solution_rate( .expect("work has already been validated") }; - let last_block = block_iter.next()?; - let last_block_time = last_block.header.time; + // If there are no blocks in the range, we can't return a useful result. + let last_block = block_iter.peek()?; - let mut total_work: PartialCumulativeWork = get_work(last_block).into(); + // Initialize the cumulative variables. + let mut min_time = last_block.header.time; + let mut max_time = last_block.header.time; - // We need at least 2 blocks to calculate average work over time. - if block_iter.len() == 0 { - return None; + let mut last_work = Work::zero(); + let mut total_work = PartialCumulativeWork::zero(); + + for block in block_iter { + min_time = min_time.min(block.header.time); + max_time = max_time.max(block.header.time); + + last_work = get_work(&block); + total_work += last_work; } // We added an extra block so we could estimate when mining on the first block // in the window of `num_blocks` likely started. But we don't want to add the work - // for that block. Since we've already taken the first block, we need to subtract 1 here. - for block in (&mut block_iter).take(num_blocks.checked_sub(1)?) { - total_work += get_work(block); - } + // for that block. + total_work -= last_work; - let first_block = block_iter - .next() - .expect("already checked for at least 1 block"); - let first_block_time = first_block.header.time; - let duration_between_first_and_last_block = last_block_time - first_block_time; + let work_duration = (max_time - min_time).num_seconds(); - // Avoid division by zero errors. - if duration_between_first_and_last_block.num_seconds() <= 0 { + // Avoid division by zero errors and negative average work. + // This also handles the case where there's only one block in the range. + if work_duration <= 0 { return None; } - Some(total_work.as_u128() / duration_between_first_and_last_block.num_seconds() as u128) + Some(total_work.as_u128() / work_duration as u128) } /// Do a consistency check by checking the finalized tip before and after all other database From 9be53d2327b292382cad17db98236b22c6a73751 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 28 Sep 2023 17:23:23 +1000 Subject: [PATCH 06/17] Refactor block iterators so they are more efficient --- zebra-state/src/service/block_iter.rs | 54 +++++++++---------- .../src/service/non_finalized_state.rs | 8 +++ 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/zebra-state/src/service/block_iter.rs b/zebra-state/src/service/block_iter.rs index 9fc9bc8ff3c..c1e60ea22d2 100644 --- a/zebra-state/src/service/block_iter.rs +++ b/zebra-state/src/service/block_iter.rs @@ -4,13 +4,14 @@ use std::sync::Arc; use zebra_chain::block::{self, Block}; -use crate::{service::non_finalized_state::NonFinalizedState, HashOrHeight}; +use crate::service::non_finalized_state::NonFinalizedState; use super::finalized_state::ZebraDb; -/// Iterator for state blocks. +/// State chain iterator by block height or hash. +/// Can be used for blocks, block headers, or any type indexed by [`HashOrHeight`]. /// -/// Starts at any block in any non-finalized or finalized chain, +/// Starts at any hash or height in any non-finalized or finalized chain, /// and iterates in reverse height order. (Towards the genesis block.) pub(crate) struct Iter<'a> { pub(super) non_finalized_state: &'a NonFinalizedState, @@ -37,49 +38,44 @@ impl Iter<'_> { IterState::Finalized(_) | IterState::Finished => unreachable!(), }; - if let Some(block) = non_finalized_state.any_block_by_hash(hash) { - let hash = block.header.previous_block_hash; - self.state = IterState::NonFinalized(hash); - Some(block) + // This is efficient because the number of chains is limited, and the blocks are in memory. + if let Some(prev_hash) = non_finalized_state.any_prev_block_hash_for_hash(hash) { + self.state = IterState::NonFinalized(prev_hash); } else { - None + self.state = IterState::Finished; } + + non_finalized_state.any_block_by_hash(hash) } #[allow(clippy::unwrap_in_result)] fn next_finalized_block(&mut self) -> Option> { - let Iter { - non_finalized_state: _, - db, - state, - } = self; - - let hash_or_height: HashOrHeight = match *state { - IterState::Finalized(height) => height.into(), - IterState::NonFinalized(hash) => hash.into(), + let height = match self.state { + IterState::Finalized(height) => Some(height), + // This is efficient because we only read the database once, when transitioning between + // non-finalized hashes and finalized heights. + IterState::NonFinalized(hash) => self.any_height_by_hash(hash), IterState::Finished => unreachable!(), }; - if let Some(block) = db.block(hash_or_height) { - let height = block - .coinbase_height() - .expect("valid blocks have a coinbase height"); + // We're finished unless we have a valid height and a valid previous height. + self.state = IterState::Finished; - if let Some(next_height) = height - 1 { - self.state = IterState::Finalized(next_height); - } else { - self.state = IterState::Finished; + if let Some(height) = height { + if let Ok(prev_height) = height.previous() { + self.state = IterState::Finalized(prev_height); } - Some(block) - } else { - self.state = IterState::Finished; - None + return self.db.block(height.into()); } + + None } /// Return the height for the block at `hash` in any chain. fn any_height_by_hash(&self, hash: block::Hash) -> Option { + // This is efficient because we check the blocks in memory first, then read a small column + // family if needed. self.non_finalized_state .any_height_by_hash(hash) .or_else(|| self.db.height(hash)) diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index bc342e5be9a..4bd1e62256e 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -461,6 +461,7 @@ impl NonFinalizedState { /// Returns the `block` with the given hash in any chain. pub fn any_block_by_hash(&self, hash: block::Hash) -> Option> { + // This performs efficiently because the number of chains is limited to 10. for chain in self.chain_set.iter().rev() { if let Some(prepared) = chain .height_by_hash @@ -474,6 +475,13 @@ impl NonFinalizedState { None } + /// Returns the previous block hash for the given block hash in any chain. + pub fn any_prev_block_hash_for_hash(&self, hash: block::Hash) -> Option { + // This performs efficiently because the blocks are in memory. + self.any_block_by_hash(hash) + .map(|block| block.header.previous_block_hash) + } + /// Returns the hash for a given `block::Height` if it is present in the best chain. #[allow(dead_code)] pub fn best_hash(&self, height: block::Height) -> Option { From 53757d846f895396d7c3c9b8f9baa51fbd7c1c3d Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 28 Sep 2023 17:44:16 +1000 Subject: [PATCH 07/17] Make finding chains easier --- .../src/service/non_finalized_state.rs | 15 ++++++++------- .../src/service/non_finalized_state/chain.rs | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 4bd1e62256e..5b74c12077c 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -436,16 +436,20 @@ impl NonFinalizedState { .any(|chain| chain.height_by_hash.contains_key(hash)) } - /// Removes and returns the first chain satisfying the given predicate. + /// Returns the first chain satisfying the given predicate. /// /// If multiple chains satisfy the predicate, returns the chain with the highest difficulty. /// (Using the tip block hash tie-breaker.) - fn find_chain

(&mut self, mut predicate: P) -> Option<&Arc> + pub fn find_chain

(&self, mut predicate: P) -> Option> where P: FnMut(&Chain) -> bool, { // Reverse the iteration order, to find highest difficulty chains first. - self.chain_set.iter().rev().find(|chain| predicate(chain)) + self.chain_set + .iter() + .rev() + .find(|chain| predicate(chain)) + .cloned() } /// Returns the [`transparent::Utxo`] pointed to by the given @@ -576,10 +580,7 @@ impl NonFinalizedState { /// The chain can be an existing chain in the non-finalized state, or a freshly /// created fork. #[allow(clippy::unwrap_in_result)] - fn parent_chain( - &mut self, - parent_hash: block::Hash, - ) -> Result, ValidateContextError> { + fn parent_chain(&self, parent_hash: block::Hash) -> Result, ValidateContextError> { match self.find_chain(|chain| chain.non_finalized_tip_hash() == parent_hash) { // Clone the existing Arc in the non-finalized state Some(chain) => Ok(chain.clone()), diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 2b66b17cdf1..4fec969fa1c 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -454,6 +454,25 @@ impl Chain { self.height_by_hash.contains_key(hash) } + /// Returns true is the chain contains the given block height. + /// Returns false otherwise. + pub fn contains_block_height(&self, height: Height) -> bool { + self.blocks.contains_key(&height) + } + + /// Returns true is the chain contains the given block hash or height. + /// Returns false otherwise. + pub fn contains_hash_or_height(&self, hash_or_height: impl Into) -> bool { + use HashOrHeight::*; + + let hash_or_height = hash_or_height.into(); + + match hash_or_height { + Hash(hash) => self.contains_block_hash(&hash), + Height(height) => self.contains_block_height(height), + } + } + /// Returns the non-finalized tip block height and hash. pub fn non_finalized_tip(&self) -> (Height, block::Hash) { ( From 72afe14b8ffdc2cb84d32af66d89edb26f813f79 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Sep 2023 10:29:31 +1000 Subject: [PATCH 08/17] Simplify block iteration code --- zebra-chain/src/block/height.rs | 5 + zebra-state/src/service/block_iter.rs | 194 ++++++++++++------ .../service/finalized_state/zebra_db/block.rs | 16 ++ 3 files changed, 152 insertions(+), 63 deletions(-) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index b06a9623291..20925c41cf5 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -99,6 +99,11 @@ impl Height { pub fn is_min(self) -> bool { self == Self::MIN } + + /// Returns the value as a `usize`. + pub fn as_usize(self) -> usize { + self.0.try_into().expect("fits in usize") + } } /// A difference between two [`Height`]s, possibly negative. diff --git a/zebra-state/src/service/block_iter.rs b/zebra-state/src/service/block_iter.rs index c1e60ea22d2..47e74217670 100644 --- a/zebra-state/src/service/block_iter.rs +++ b/zebra-state/src/service/block_iter.rs @@ -1,79 +1,101 @@ //! Iterators for blocks in the non-finalized and finalized state. -use std::sync::Arc; +use std::{marker::PhantomData, sync::Arc}; -use zebra_chain::block::{self, Block}; +use zebra_chain::block::{self, Block, Height}; -use crate::service::non_finalized_state::NonFinalizedState; +use crate::{ + service::{finalized_state::ZebraDb, non_finalized_state::NonFinalizedState, read}, + HashOrHeight, +}; -use super::finalized_state::ZebraDb; - -/// State chain iterator by block height or hash. +/// Generic state chain iterator, which iterates by block height or hash. /// Can be used for blocks, block headers, or any type indexed by [`HashOrHeight`]. /// /// Starts at any hash or height in any non-finalized or finalized chain, /// and iterates in reverse height order. (Towards the genesis block.) -pub(crate) struct Iter<'a> { +#[derive(Clone, Debug)] +pub(crate) struct Iter<'a, Iterable: ChainIterable> { + // TODO: replace these lifetimes with Arc and a cloned ZebraDb. + /// The non-finalized state we're iterating. + // + // TODO: consider using an `Arc` here instead, and iterating by height. pub(super) non_finalized_state: &'a NonFinalizedState, + + /// The finalized database we're iterating. pub(super) db: &'a ZebraDb, + + /// The internal state of the iterator: which block will be yielded next. pub(super) state: IterState, + + /// An internal marker type that lets the Rust type system track what we're iterating. + iterable: PhantomData, } +/// Internal state of the chain iterator. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] pub(super) enum IterState { - NonFinalized(block::Hash), - Finalized(block::Height), + /// Iterating non-finalized or finalized hashes, using previous block hashes. + /// + /// This state is used for: + /// - iterating through the non-finalized state, + /// - the initial iteration of a hash in the finalized state, and + /// - the transition between the root of a non-finalized chain and the finalized state. + NonFinalizedOrFinalized(block::Hash), + + /// Iterating finalized heights using previous heights, which are unique in the finalized state. + Finalized(Height), + + /// Finished iterating, or an invalid iterator. Finished, } -impl Iter<'_> { - fn next_non_finalized_block(&mut self) -> Option> { - let Iter { - non_finalized_state, - db: _, - state, - } = self; - - let hash = match state { - IterState::NonFinalized(hash) => *hash, - IterState::Finalized(_) | IterState::Finished => unreachable!(), - }; - - // This is efficient because the number of chains is limited, and the blocks are in memory. - if let Some(prev_hash) = non_finalized_state.any_prev_block_hash_for_hash(hash) { - self.state = IterState::NonFinalized(prev_hash); +impl Iter<'_, Iterable> +where + Iterable: ChainIterable, +{ + /// Returns an item by hash, and updates the iterator's internal state to point to the previous + /// hash or height. + fn yield_by_hash(&mut self, hash: block::Hash) -> Option { + // This is efficient because we check the blocks in memory first, then read a small column + // family if needed. + if let Some(prev_hash) = self.non_finalized_state.any_prev_block_hash_for_hash(hash) { + // Iterating through a non-finalized chain. + // + // TODO: + // Always check that the root of the chain connects to the finalized state. Cloned + // chains can become disconnected if they are concurrently pruned by a finalized block + // from another chain fork. + // If that happens, the iterator is invalid and should return nothing. + self.state = IterState::NonFinalizedOrFinalized(prev_hash); + } else if let Some(prev_height) = self.db.prev_block_height_for_hash(hash) { + // First iteration of a hash in the finalized state, whether from the chain root + // or the initial iteration of a new iterator. + self.state = IterState::Finalized(prev_height); } else { + // The iterator is invalid if the block with `hash` is not in the state. + // The iterator is finished if the current block is the genesis block. self.state = IterState::Finished; } - non_finalized_state.any_block_by_hash(hash) + Iterable::read(self.non_finalized_state, self.db, hash) } - #[allow(clippy::unwrap_in_result)] - fn next_finalized_block(&mut self) -> Option> { - let height = match self.state { - IterState::Finalized(height) => Some(height), - // This is efficient because we only read the database once, when transitioning between - // non-finalized hashes and finalized heights. - IterState::NonFinalized(hash) => self.any_height_by_hash(hash), - IterState::Finished => unreachable!(), - }; - - // We're finished unless we have a valid height and a valid previous height. - self.state = IterState::Finished; - - if let Some(height) = height { - if let Ok(prev_height) = height.previous() { - self.state = IterState::Finalized(prev_height); - } - - return self.db.block(height.into()); + /// Returns an item by height, and updates the iterator's internal state to point to the + /// previous height. + fn yield_by_height(&mut self, height: Height) -> Option { + if let Ok(prev_height) = height.previous() { + self.state = IterState::Finalized(prev_height); + } else { + // The iterator is finished if the current block is the genesis block. + self.state = IterState::Finished; } - None + Iterable::read(self.non_finalized_state, self.db, height) } - /// Return the height for the block at `hash` in any chain. - fn any_height_by_hash(&self, hash: block::Hash) -> Option { + /// Return the height for the block with `hash` in any chain. + fn any_height_by_hash(&self, hash: block::Hash) -> Option { // This is efficient because we check the blocks in memory first, then read a small column // family if needed. self.non_finalized_state @@ -82,15 +104,16 @@ impl Iter<'_> { } } -impl Iterator for Iter<'_> { - type Item = Arc; +impl Iterator for Iter<'_, Iterable> +where + Iterable: ChainIterable, +{ + type Item = Iterable::Type; fn next(&mut self) -> Option { match self.state { - IterState::NonFinalized(_) => self - .next_non_finalized_block() - .or_else(|| self.next_finalized_block()), - IterState::Finalized(_) => self.next_finalized_block(), + IterState::NonFinalizedOrFinalized(hash) => self.yield_by_hash(hash), + IterState::Finalized(height) => self.yield_by_height(height), IterState::Finished => None, } } @@ -101,21 +124,65 @@ impl Iterator for Iter<'_> { } } -impl std::iter::FusedIterator for Iter<'_> {} - -impl ExactSizeIterator for Iter<'_> { +impl<'a, Iterable> ExactSizeIterator for Iter<'a, Iterable> +where + Iterable: ChainIterable, + // If the Iterable can return None in the middle of iteration, we can't calculate its length + // using the block height. + Iter<'a, Block>: std::iter::FusedIterator, +{ fn len(&self) -> usize { match self.state { - IterState::NonFinalized(hash) => self + // TODO: + // Always check that the root of the chain connects to the finalized state. Cloned + // chains can become disconnected if they are concurrently pruned by a finalized block + // from another chain fork. + // If that happens, the iterator is invalid and the length should be zero. + IterState::NonFinalizedOrFinalized(hash) => self .any_height_by_hash(hash) - .map(|height| (height.0 + 1) as _) - .unwrap_or(0), - IterState::Finalized(height) => (height.0 + 1) as _, + // Add a block for the genesis block. + .and_then(|height| height.next().ok()) + // If the iterator hash is invalid then there are no blocks in it. + .map_or(0, Height::as_usize), + // Add a block for the genesis block. + IterState::Finalized(height) => height.as_usize() + 1, IterState::Finished => 0, } } } +pub(crate) trait ChainIterable { + type Type; + + /// Read the `Type` at `hash_or_height` from the non-finalized state or finalized `db`. + fn read( + non_finalized_state: &NonFinalizedState, + db: &ZebraDb, + hash_or_height: impl Into, + ) -> Option; +} + +impl ChainIterable for Block { + type Type = Arc; + + fn read( + non_finalized_state: &NonFinalizedState, + db: &ZebraDb, + hash_or_height: impl Into, + ) -> Option { + let hash_or_height = hash_or_height.into(); + + // Since the block read method takes a chain, we need to look up the relevant chain first. + let chain = + non_finalized_state.find_chain(|chain| chain.contains_hash_or_height(hash_or_height)); + + read::block(chain, db, hash_or_height) + } +} + +/// Blocks are continuous with no gaps, so they are exhausted the first time they return `None`. +impl std::iter::FusedIterator for Iter<'_, Block> {} + /// Return an iterator over the relevant chain of the block identified by /// `hash`, in order from the largest height to the genesis block. /// @@ -125,10 +192,11 @@ pub(crate) fn any_ancestor_blocks<'a>( non_finalized_state: &'a NonFinalizedState, db: &'a ZebraDb, hash: block::Hash, -) -> Iter<'a> { +) -> Iter<'a, Block> { Iter { non_finalized_state, db, - state: IterState::NonFinalized(hash), + state: IterState::NonFinalizedOrFinalized(hash), + iterable: PhantomData, } } diff --git a/zebra-state/src/service/finalized_state/zebra_db/block.rs b/zebra-state/src/service/finalized_state/zebra_db/block.rs index 25745e44afb..4505d063029 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/block.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/block.rs @@ -97,6 +97,22 @@ impl ZebraDb { self.db.zs_get(&height_by_hash, &hash) } + /// Returns the previous block hash for the given block hash in the finalized state. + #[allow(dead_code)] + pub fn prev_block_hash_for_hash(&self, hash: block::Hash) -> Option { + let height = self.height(hash)?; + let prev_height = height.previous().ok()?; + + self.hash(prev_height) + } + + /// Returns the previous block height for the given block hash in the finalized state. + pub fn prev_block_height_for_hash(&self, hash: block::Hash) -> Option { + let height = self.height(hash)?; + + height.previous().ok() + } + /// Returns the [`block::Header`] with [`block::Hash`] or /// [`Height`], if it exists in the finalized chain. // From e269cc99bd97f8ad3887d0b1b612e0ea9870ae87 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Sep 2023 10:29:44 +1000 Subject: [PATCH 09/17] Remove implemented TODO comments --- zebra-chain/src/block/height.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/zebra-chain/src/block/height.rs b/zebra-chain/src/block/height.rs index 20925c41cf5..c5fb22efde1 100644 --- a/zebra-chain/src/block/height.rs +++ b/zebra-chain/src/block/height.rs @@ -80,7 +80,6 @@ impl Height { /// # Panics /// /// - If the current height is at its maximum. - // TODO Return an error instead of panicking #7263. pub fn next(self) -> Result { (self + 1).ok_or(HeightError::Overflow) } @@ -90,7 +89,6 @@ impl Height { /// # Panics /// /// - If the current height is at its minimum. - // TODO Return an error instead of panicking #7263. pub fn previous(self) -> Result { (self - 1).ok_or(HeightError::Underflow) } From 42379d1a603c70a99ef522c717e42527c6fc275f Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Sep 2023 11:12:02 +1000 Subject: [PATCH 10/17] Simplify internal iterator state --- zebra-state/src/service/block_iter.rs | 233 ++++++++---------- .../src/service/non_finalized_state/chain.rs | 6 +- zebra-state/src/service/read/find.rs | 2 +- 3 files changed, 107 insertions(+), 134 deletions(-) diff --git a/zebra-state/src/service/block_iter.rs b/zebra-state/src/service/block_iter.rs index 47e74217670..2def675fd0c 100644 --- a/zebra-state/src/service/block_iter.rs +++ b/zebra-state/src/service/block_iter.rs @@ -5,7 +5,11 @@ use std::{marker::PhantomData, sync::Arc}; use zebra_chain::block::{self, Block, Height}; use crate::{ - service::{finalized_state::ZebraDb, non_finalized_state::NonFinalizedState, read}, + service::{ + finalized_state::ZebraDb, + non_finalized_state::{Chain, NonFinalizedState}, + read, + }, HashOrHeight, }; @@ -15,107 +19,68 @@ use crate::{ /// Starts at any hash or height in any non-finalized or finalized chain, /// and iterates in reverse height order. (Towards the genesis block.) #[derive(Clone, Debug)] -pub(crate) struct Iter<'a, Iterable: ChainIterable> { - // TODO: replace these lifetimes with Arc and a cloned ZebraDb. - /// The non-finalized state we're iterating. - // - // TODO: consider using an `Arc` here instead, and iterating by height. - pub(super) non_finalized_state: &'a NonFinalizedState, +pub(crate) struct Iter { + /// The non-finalized chain fork we're iterating, if the iterator is in the non-finalized state. + /// + /// This is a cloned copy of a potentially out-of-date chain fork. + pub(super) chain: Option>, /// The finalized database we're iterating. - pub(super) db: &'a ZebraDb, - - /// The internal state of the iterator: which block will be yielded next. - pub(super) state: IterState, - - /// An internal marker type that lets the Rust type system track what we're iterating. - iterable: PhantomData, -} - -/// Internal state of the chain iterator. -#[derive(Copy, Clone, Debug, Eq, PartialEq)] -pub(super) enum IterState { - /// Iterating non-finalized or finalized hashes, using previous block hashes. /// - /// This state is used for: - /// - iterating through the non-finalized state, - /// - the initial iteration of a hash in the finalized state, and - /// - the transition between the root of a non-finalized chain and the finalized state. - NonFinalizedOrFinalized(block::Hash), + /// This is the shared live database instance, which can concurrently write blocks. + pub(super) db: ZebraDb, - /// Iterating finalized heights using previous heights, which are unique in the finalized state. - Finalized(Height), + /// The height of the item which will be yielded by `Iterator::next()`. + pub(super) height: Option, - /// Finished iterating, or an invalid iterator. - Finished, + /// An internal marker type that tells the Rust type system what we're iterating. + iterable: PhantomData, } -impl Iter<'_, Iterable> +impl Iter where Iterable: ChainIterable, { - /// Returns an item by hash, and updates the iterator's internal state to point to the previous - /// hash or height. - fn yield_by_hash(&mut self, hash: block::Hash) -> Option { - // This is efficient because we check the blocks in memory first, then read a small column - // family if needed. - if let Some(prev_hash) = self.non_finalized_state.any_prev_block_hash_for_hash(hash) { - // Iterating through a non-finalized chain. - // - // TODO: - // Always check that the root of the chain connects to the finalized state. Cloned - // chains can become disconnected if they are concurrently pruned by a finalized block - // from another chain fork. - // If that happens, the iterator is invalid and should return nothing. - self.state = IterState::NonFinalizedOrFinalized(prev_hash); - } else if let Some(prev_height) = self.db.prev_block_height_for_hash(hash) { - // First iteration of a hash in the finalized state, whether from the chain root - // or the initial iteration of a new iterator. - self.state = IterState::Finalized(prev_height); - } else { - // The iterator is invalid if the block with `hash` is not in the state. - // The iterator is finished if the current block is the genesis block. - self.state = IterState::Finished; - } - - Iterable::read(self.non_finalized_state, self.db, hash) - } - /// Returns an item by height, and updates the iterator's internal state to point to the /// previous height. - fn yield_by_height(&mut self, height: Height) -> Option { - if let Ok(prev_height) = height.previous() { - self.state = IterState::Finalized(prev_height); - } else { - // The iterator is finished if the current block is the genesis block. - self.state = IterState::Finished; + fn yield_by_height(&mut self) -> Option { + let current_height = self.height?; + + // TODO: + // Check if the root of the chain connects to the finalized state. Cloned chains can become + // disconnected if they are concurrently pruned by a finalized block from another chain + // fork. If that happens, the iterator is invalid and should stop returning items. + // + // Currently, we skip from the disconnected chain root to the previous height in the + // finalized state, which is usually ok, but could cause consensus or light wallet bugs. + let item = Iterable::read(self.chain.as_ref(), &self.db, current_height); + + // The iterator is finished if the current height is genesis. + self.height = current_height.previous().ok(); + + // Drop the chain if we've finished using it. + if let Some(chain) = self.chain.as_ref() { + if let Some(height) = self.height { + if !chain.contains_block_height(height) { + std::mem::drop(self.chain.take()); + } + } else { + std::mem::drop(self.chain.take()); + } } - Iterable::read(self.non_finalized_state, self.db, height) - } - - /// Return the height for the block with `hash` in any chain. - fn any_height_by_hash(&self, hash: block::Hash) -> Option { - // This is efficient because we check the blocks in memory first, then read a small column - // family if needed. - self.non_finalized_state - .any_height_by_hash(hash) - .or_else(|| self.db.height(hash)) + item } } -impl Iterator for Iter<'_, Iterable> +impl Iterator for Iter where Iterable: ChainIterable, { type Item = Iterable::Type; fn next(&mut self) -> Option { - match self.state { - IterState::NonFinalizedOrFinalized(hash) => self.yield_by_hash(hash), - IterState::Finalized(height) => self.yield_by_height(height), - IterState::Finished => None, - } + self.yield_by_height() } fn size_hint(&self) -> (usize, Option) { @@ -124,79 +89,87 @@ where } } -impl<'a, Iterable> ExactSizeIterator for Iter<'a, Iterable> +impl ExactSizeIterator for Iter where Iterable: ChainIterable, - // If the Iterable can return None in the middle of iteration, we can't calculate its length - // using the block height. - Iter<'a, Block>: std::iter::FusedIterator, { fn len(&self) -> usize { - match self.state { - // TODO: - // Always check that the root of the chain connects to the finalized state. Cloned - // chains can become disconnected if they are concurrently pruned by a finalized block - // from another chain fork. - // If that happens, the iterator is invalid and the length should be zero. - IterState::NonFinalizedOrFinalized(hash) => self - .any_height_by_hash(hash) - // Add a block for the genesis block. - .and_then(|height| height.next().ok()) - // If the iterator hash is invalid then there are no blocks in it. - .map_or(0, Height::as_usize), - // Add a block for the genesis block. - IterState::Finalized(height) => height.as_usize() + 1, - IterState::Finished => 0, - } + // Add one to the height for the genesis block. + // + // TODO: + // If the Iterable can skip heights, or return multiple items per block, we can't calculate + // its length using the block height. For example, subtree end height iterators, or + // transaction iterators. + // + // TODO: + // Check if the root of the chain connects to the finalized state. If that happens, the + // iterator is invalid and the length should be zero. See the comment in yield_by_height() + // for details. + self.height.map_or(0, |height| height.as_usize() + 1) } } +// TODO: +// If the Iterable can return None before it gets to genesis, it is not fused. For example, subtree +// end height iterators. +impl std::iter::FusedIterator for Iter where Iterable: ChainIterable {} + pub(crate) trait ChainIterable { type Type; - /// Read the `Type` at `hash_or_height` from the non-finalized state or finalized `db`. - fn read( - non_finalized_state: &NonFinalizedState, - db: &ZebraDb, - hash_or_height: impl Into, - ) -> Option; + /// Read the `Type` at `height` from the non-finalized `chain` or finalized `db`. + fn read(chain: Option<&Arc>, db: &ZebraDb, height: Height) -> Option; } impl ChainIterable for Block { type Type = Arc; - fn read( - non_finalized_state: &NonFinalizedState, - db: &ZebraDb, - hash_or_height: impl Into, - ) -> Option { - let hash_or_height = hash_or_height.into(); + fn read(chain: Option<&Arc>, db: &ZebraDb, height: Height) -> Option { + read::block(chain, db, height.into()) + } +} - // Since the block read method takes a chain, we need to look up the relevant chain first. - let chain = - non_finalized_state.find_chain(|chain| chain.contains_hash_or_height(hash_or_height)); +/// Returns an iterator over the relevant chain of the block with `hash`, +/// in order from the largest height to the genesis block. +/// +/// The block with `hash` is included in the chain of blocks yielded by the iterator. +/// `hash` can come from any chain or `db`. +pub(crate) fn any_ancestor_blocks( + non_finalized_state: &NonFinalizedState, + db: &ZebraDb, + hash: block::Hash, +) -> Iter { + // We need to look up the relevant chain, and the height for the hash. + let chain = non_finalized_state.find_chain(|chain| chain.contains_block_hash(hash)); + let height = read::height_by_hash(chain.as_ref(), db, hash); - read::block(chain, db, hash_or_height) + Iter { + chain, + db: db.clone(), + height, + iterable: PhantomData, } } -/// Blocks are continuous with no gaps, so they are exhausted the first time they return `None`. -impl std::iter::FusedIterator for Iter<'_, Block> {} - -/// Return an iterator over the relevant chain of the block identified by -/// `hash`, in order from the largest height to the genesis block. +/// Returns an iterator over a `chain` containing the block with `hash_or_height`, +/// in order from the largest height to the genesis block. /// -/// The block identified by `hash` is included in the chain of blocks yielded -/// by the iterator. `hash` can come from any chain. -pub(crate) fn any_ancestor_blocks<'a>( - non_finalized_state: &'a NonFinalizedState, - db: &'a ZebraDb, - hash: block::Hash, -) -> Iter<'a, Block> { +/// The block with `hash_or_height` is included in the chain of blocks yielded by the iterator. +/// `hash` must be in `chain` or `db`. +#[allow(dead_code)] +pub(crate) fn known_chain_ancestor_blocks( + chain: Option>, + db: &ZebraDb, + hash_or_height: HashOrHeight, +) -> Iter { + // We need to look up the height for the hash. + let height = + hash_or_height.height_or_else(|hash| read::height_by_hash(chain.as_ref(), db, hash)); + Iter { - non_finalized_state, - db, - state: IterState::NonFinalizedOrFinalized(hash), + chain, + db: db.clone(), + height, iterable: PhantomData, } } diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index 4fec969fa1c..cb131600fce 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -450,8 +450,8 @@ impl Chain { /// Returns true is the chain contains the given block hash. /// Returns false otherwise. - pub fn contains_block_hash(&self, hash: &block::Hash) -> bool { - self.height_by_hash.contains_key(hash) + pub fn contains_block_hash(&self, hash: block::Hash) -> bool { + self.height_by_hash.contains_key(&hash) } /// Returns true is the chain contains the given block height. @@ -468,7 +468,7 @@ impl Chain { let hash_or_height = hash_or_height.into(); match hash_or_height { - Hash(hash) => self.contains_block_hash(&hash), + Hash(hash) => self.contains_block_hash(hash), Height(height) => self.contains_block_height(height), } } diff --git a/zebra-state/src/service/read/find.rs b/zebra-state/src/service/read/find.rs index 3e1c4996d8a..44491876ec9 100644 --- a/zebra-state/src/service/read/find.rs +++ b/zebra-state/src/service/read/find.rs @@ -108,7 +108,7 @@ pub fn non_finalized_state_contains_block_hash( hash: block::Hash, ) -> Option { let mut chains_iter = non_finalized_state.chain_iter(); - let is_hash_in_chain = |chain: &Arc| chain.contains_block_hash(&hash); + let is_hash_in_chain = |chain: &Arc| chain.contains_block_hash(hash); // Equivalent to `chain_set.iter().next_back()` in `NonFinalizedState.best_chain()` method. let best_chain = chains_iter.next(); From 1cf5e4addd9d5f7497a7d47169a849e018b23589 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Sep 2023 11:23:54 +1000 Subject: [PATCH 11/17] Implement iteration by any chain item --- zebra-state/src/service/block_iter.rs | 84 +++++++++++++++++++-------- 1 file changed, 59 insertions(+), 25 deletions(-) diff --git a/zebra-state/src/service/block_iter.rs b/zebra-state/src/service/block_iter.rs index 2def675fd0c..6cb0df903ba 100644 --- a/zebra-state/src/service/block_iter.rs +++ b/zebra-state/src/service/block_iter.rs @@ -19,7 +19,7 @@ use crate::{ /// Starts at any hash or height in any non-finalized or finalized chain, /// and iterates in reverse height order. (Towards the genesis block.) #[derive(Clone, Debug)] -pub(crate) struct Iter { +pub(crate) struct Iter { /// The non-finalized chain fork we're iterating, if the iterator is in the non-finalized state. /// /// This is a cloned copy of a potentially out-of-date chain fork. @@ -34,16 +34,16 @@ pub(crate) struct Iter { pub(super) height: Option, /// An internal marker type that tells the Rust type system what we're iterating. - iterable: PhantomData, + iterable: PhantomData, } -impl Iter +impl Iter where - Iterable: ChainIterable, + Item: ChainItem, { /// Returns an item by height, and updates the iterator's internal state to point to the /// previous height. - fn yield_by_height(&mut self) -> Option { + fn yield_by_height(&mut self) -> Option { let current_height = self.height?; // TODO: @@ -53,7 +53,7 @@ where // // Currently, we skip from the disconnected chain root to the previous height in the // finalized state, which is usually ok, but could cause consensus or light wallet bugs. - let item = Iterable::read(self.chain.as_ref(), &self.db, current_height); + let item = Item::read(self.chain.as_ref(), &self.db, current_height); // The iterator is finished if the current height is genesis. self.height = current_height.previous().ok(); @@ -73,11 +73,11 @@ where } } -impl Iterator for Iter +impl Iterator for Iter where - Iterable: ChainIterable, + Item: ChainItem, { - type Item = Iterable::Type; + type Item = Item::Type; fn next(&mut self) -> Option { self.yield_by_height() @@ -89,15 +89,15 @@ where } } -impl ExactSizeIterator for Iter +impl ExactSizeIterator for Iter where - Iterable: ChainIterable, + Item: ChainItem, { fn len(&self) -> usize { // Add one to the height for the genesis block. // // TODO: - // If the Iterable can skip heights, or return multiple items per block, we can't calculate + // If the Item can skip heights, or return multiple items per block, we can't calculate // its length using the block height. For example, subtree end height iterators, or // transaction iterators. // @@ -110,18 +110,21 @@ where } // TODO: -// If the Iterable can return None before it gets to genesis, it is not fused. For example, subtree +// If the Item can return None before it gets to genesis, it is not fused. For example, subtree // end height iterators. -impl std::iter::FusedIterator for Iter where Iterable: ChainIterable {} +impl std::iter::FusedIterator for Iter where Item: ChainItem {} -pub(crate) trait ChainIterable { +/// A trait that implements iteration for a specific chain type. +pub(crate) trait ChainItem { type Type; /// Read the `Type` at `height` from the non-finalized `chain` or finalized `db`. fn read(chain: Option<&Arc>, db: &ZebraDb, height: Height) -> Option; } -impl ChainIterable for Block { +// Block iteration + +impl ChainItem for Block { type Type = Arc; fn read(chain: Option<&Arc>, db: &ZebraDb, height: Height) -> Option { @@ -129,16 +132,44 @@ impl ChainIterable for Block { } } -/// Returns an iterator over the relevant chain of the block with `hash`, -/// in order from the largest height to the genesis block. +// Block header iteration + +impl ChainItem for block::Header { + type Type = Arc; + + fn read(chain: Option<&Arc>, db: &ZebraDb, height: Height) -> Option { + read::block_header(chain, db, height.into()) + } +} + +/// Returns a block iterator over the relevant chain containing `hash`, +/// in order from the largest height to genesis. /// -/// The block with `hash` is included in the chain of blocks yielded by the iterator. +/// The block with `hash` is included in the iterator. /// `hash` can come from any chain or `db`. +/// +/// Use [`any_chain_ancestor_iter()`] in new code. pub(crate) fn any_ancestor_blocks( non_finalized_state: &NonFinalizedState, db: &ZebraDb, hash: block::Hash, ) -> Iter { + any_chain_ancestor_iter(non_finalized_state, db, hash) +} + +/// Returns a generic chain item iterator over the relevant chain containing `hash`, +/// in order from the largest height to genesis. +/// +/// The item with `hash` is included in the iterator. +/// `hash` can come from any chain or `db`. +pub(crate) fn any_chain_ancestor_iter( + non_finalized_state: &NonFinalizedState, + db: &ZebraDb, + hash: block::Hash, +) -> Iter +where + Item: ChainItem, +{ // We need to look up the relevant chain, and the height for the hash. let chain = non_finalized_state.find_chain(|chain| chain.contains_block_hash(hash)); let height = read::height_by_hash(chain.as_ref(), db, hash); @@ -151,17 +182,20 @@ pub(crate) fn any_ancestor_blocks( } } -/// Returns an iterator over a `chain` containing the block with `hash_or_height`, -/// in order from the largest height to the genesis block. +/// Returns a generic chain item iterator over a `chain` containing `hash_or_height`, +/// in order from the largest height to genesis. /// -/// The block with `hash_or_height` is included in the chain of blocks yielded by the iterator. -/// `hash` must be in `chain` or `db`. +/// The item with `hash_or_height` is included in the iterator. +/// `hash_or_height` must be in `chain` or `db`. #[allow(dead_code)] -pub(crate) fn known_chain_ancestor_blocks( +pub(crate) fn known_chain_ancestor_iter( chain: Option>, db: &ZebraDb, hash_or_height: HashOrHeight, -) -> Iter { +) -> Iter +where + Item: ChainItem, +{ // We need to look up the height for the hash. let height = hash_or_height.height_or_else(|hash| read::height_by_hash(chain.as_ref(), db, hash)); From 930e2b4b1accda4437d6ac5eef13fc5e2378cdef Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Sep 2023 11:29:06 +1000 Subject: [PATCH 12/17] Iterate block headers rather than full blocks --- zebra-state/src/service/read/difficulty.rs | 37 +++++++++++----------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/zebra-state/src/service/read/difficulty.rs b/zebra-state/src/service/read/difficulty.rs index 6fa3dff1b4f..cb9734892b6 100644 --- a/zebra-state/src/service/read/difficulty.rs +++ b/zebra-state/src/service/read/difficulty.rs @@ -15,6 +15,7 @@ use zebra_chain::{ use crate::{ service::{ any_ancestor_blocks, + block_iter::any_chain_ancestor_iter, check::{ difficulty::{ BLOCK_MAX_TIME_SINCE_MEDIAN, POW_ADJUSTMENT_BLOCK_SPAN, POW_MEDIAN_BLOCK_SPAN, @@ -87,44 +88,44 @@ pub fn solution_rate( num_blocks: usize, start_hash: Hash, ) -> Option { - // Take 1 extra block for calculating the number of seconds between when mining on the first block likely started. - // The work for the last block in this iterator is not added to `total_work`. + // Take 1 extra header for calculating the number of seconds between when mining on the first + // block likely started. The work for the extra header is not added to `total_work`. // - // Since we can't take more blocks than are actually in the chain, this automatically limits + // Since we can't take more headers than are actually in the chain, this automatically limits // `num_blocks` to the chain length, like `zcashd` does. - let mut block_iter = any_ancestor_blocks(non_finalized_state, db, start_hash) - .take(num_blocks.checked_add(1).unwrap_or(num_blocks)) - .peekable(); + let mut header_iter = + any_chain_ancestor_iter::(non_finalized_state, db, start_hash) + .take(num_blocks.checked_add(1).unwrap_or(num_blocks)) + .peekable(); - let get_work = |block: &Block| { - block - .header + let get_work = |header: &block::Header| { + header .difficulty_threshold .to_work() .expect("work has already been validated") }; // If there are no blocks in the range, we can't return a useful result. - let last_block = block_iter.peek()?; + let last_header = header_iter.peek()?; // Initialize the cumulative variables. - let mut min_time = last_block.header.time; - let mut max_time = last_block.header.time; + let mut min_time = last_header.time; + let mut max_time = last_header.time; let mut last_work = Work::zero(); let mut total_work = PartialCumulativeWork::zero(); - for block in block_iter { - min_time = min_time.min(block.header.time); - max_time = max_time.max(block.header.time); + for header in header_iter { + min_time = min_time.min(header.time); + max_time = max_time.max(header.time); - last_work = get_work(&block); + last_work = get_work(&header); total_work += last_work; } - // We added an extra block so we could estimate when mining on the first block + // We added an extra header so we could estimate when mining on the first block // in the window of `num_blocks` likely started. But we don't want to add the work - // for that block. + // for that header. total_work -= last_work; let work_duration = (max_time - min_time).num_seconds(); From 8ad580b0a569076eb8209bce5b190ceafda78e9a Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 29 Sep 2023 11:50:22 +1000 Subject: [PATCH 13/17] Ignore code that is (sometimes) dead --- zebra-state/src/service/non_finalized_state.rs | 3 +++ zebra-state/src/service/non_finalized_state/chain.rs | 1 + 2 files changed, 4 insertions(+) diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index 5b74c12077c..c01c16767d9 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -464,6 +464,7 @@ impl NonFinalizedState { } /// Returns the `block` with the given hash in any chain. + #[allow(dead_code)] pub fn any_block_by_hash(&self, hash: block::Hash) -> Option> { // This performs efficiently because the number of chains is limited to 10. for chain in self.chain_set.iter().rev() { @@ -480,6 +481,7 @@ impl NonFinalizedState { } /// Returns the previous block hash for the given block hash in any chain. + #[allow(dead_code)] pub fn any_prev_block_hash_for_hash(&self, hash: block::Hash) -> Option { // This performs efficiently because the blocks are in memory. self.any_block_by_hash(hash) @@ -522,6 +524,7 @@ impl NonFinalizedState { } /// Returns the height of `hash` in any chain. + #[allow(dead_code)] pub fn any_height_by_hash(&self, hash: block::Hash) -> Option { for chain in self.chain_set.iter().rev() { if let Some(height) = chain.height_by_hash.get(&hash) { diff --git a/zebra-state/src/service/non_finalized_state/chain.rs b/zebra-state/src/service/non_finalized_state/chain.rs index cb131600fce..ef1e54215e8 100644 --- a/zebra-state/src/service/non_finalized_state/chain.rs +++ b/zebra-state/src/service/non_finalized_state/chain.rs @@ -462,6 +462,7 @@ impl Chain { /// Returns true is the chain contains the given block hash or height. /// Returns false otherwise. + #[allow(dead_code)] pub fn contains_hash_or_height(&self, hash_or_height: impl Into) -> bool { use HashOrHeight::*; From a7ba5071da91816064c14b4da8b7c6eb54939b2d Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 9 Oct 2023 10:57:52 +1000 Subject: [PATCH 14/17] Fix a dead code warning --- zebra-state/src/service/finalized_state/zebra_db/block.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zebra-state/src/service/finalized_state/zebra_db/block.rs b/zebra-state/src/service/finalized_state/zebra_db/block.rs index 4505d063029..85959849985 100644 --- a/zebra-state/src/service/finalized_state/zebra_db/block.rs +++ b/zebra-state/src/service/finalized_state/zebra_db/block.rs @@ -107,6 +107,7 @@ impl ZebraDb { } /// Returns the previous block height for the given block hash in the finalized state. + #[allow(dead_code)] pub fn prev_block_height_for_hash(&self, hash: block::Hash) -> Option { let height = self.height(hash)?; From 8e8830adf0abca178a5c3193898881e84c98b6b6 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 9 Oct 2023 10:58:17 +1000 Subject: [PATCH 15/17] Add a no blocks in state error constant --- zebra-rpc/src/constants.rs | 21 ++++++- .../src/methods/get_block_template_rpcs.rs | 57 ++++++++++--------- 2 files changed, 49 insertions(+), 29 deletions(-) diff --git a/zebra-rpc/src/constants.rs b/zebra-rpc/src/constants.rs index 5665f7ceb8e..e8be508595f 100644 --- a/zebra-rpc/src/constants.rs +++ b/zebra-rpc/src/constants.rs @@ -1,6 +1,6 @@ //! Constants for RPC methods and server responses. -use jsonrpc_core::ErrorCode; +use jsonrpc_core::{Error, ErrorCode}; /// The RPC error code used by `zcashd` for incorrect RPC parameters. /// @@ -17,5 +17,24 @@ pub const INVALID_PARAMETERS_ERROR_CODE: ErrorCode = ErrorCode::ServerError(-1); /// pub const MISSING_BLOCK_ERROR_CODE: ErrorCode = ErrorCode::ServerError(-8); +/// The RPC error code used by `zcashd` when there are no blocks in the state. +/// +/// `lightwalletd` expects error code `0` when there are no blocks in the state. +// +// TODO: find the source code that expects or generates this error +pub const NO_BLOCKS_IN_STATE_ERROR_CODE: ErrorCode = ErrorCode::ServerError(0); + +/// The RPC error used by `zcashd` when there are no blocks in the state. +// +// TODO: find the source code that expects or generates this error text, if there is any +// replace literal Error { ... } with this error +pub fn no_blocks_in_state_error() -> Error { + Error { + code: NO_BLOCKS_IN_STATE_ERROR_CODE, + message: "No blocks in state".to_string(), + data: None, + } +} + /// When logging parameter data, only log this much data. pub const MAX_PARAMS_LOG_LENGTH: usize = 100; diff --git a/zebra-rpc/src/methods/get_block_template_rpcs.rs b/zebra-rpc/src/methods/get_block_template_rpcs.rs index a970f8aaac3..a0304ed52aa 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs.rs @@ -30,31 +30,34 @@ use zebra_network::AddressBookPeers; use zebra_node_services::mempool; use zebra_state::{ReadRequest, ReadResponse}; -use crate::methods::{ - best_chain_tip_height, - get_block_template_rpcs::{ - constants::{ - DEFAULT_SOLUTION_RATE_WINDOW_SIZE, GET_BLOCK_TEMPLATE_MEMPOOL_LONG_POLL_INTERVAL, - ZCASHD_FUNDING_STREAM_ORDER, - }, - get_block_template::{ - check_miner_address, check_synced_to_tip, fetch_mempool_transactions, - fetch_state_tip_and_local_time, validate_block_proposal, - }, - // TODO: move the types/* modules directly under get_block_template_rpcs, - // and combine any modules with the same names. - types::{ - get_block_template::GetBlockTemplate, - get_mining_info, - hex_data::HexData, - long_poll::LongPollInput, - peer_info::PeerInfo, - submit_block, - subsidy::{BlockSubsidy, FundingStream}, - unified_address, validate_address, z_validate_address, +use crate::{ + constants::no_blocks_in_state_error, + methods::{ + best_chain_tip_height, + get_block_template_rpcs::{ + constants::{ + DEFAULT_SOLUTION_RATE_WINDOW_SIZE, GET_BLOCK_TEMPLATE_MEMPOOL_LONG_POLL_INTERVAL, + ZCASHD_FUNDING_STREAM_ORDER, + }, + get_block_template::{ + check_miner_address, check_synced_to_tip, fetch_mempool_transactions, + fetch_state_tip_and_local_time, validate_block_proposal, + }, + // TODO: move the types/* modules directly under get_block_template_rpcs, + // and combine any modules with the same names. + types::{ + get_block_template::GetBlockTemplate, + get_mining_info, + hex_data::HexData, + long_poll::LongPollInput, + peer_info::PeerInfo, + submit_block, + subsidy::{BlockSubsidy, FundingStream}, + unified_address, validate_address, z_validate_address, + }, }, + height_from_signed_int, GetBlockHash, MISSING_BLOCK_ERROR_CODE, }, - height_from_signed_int, GetBlockHash, MISSING_BLOCK_ERROR_CODE, }; pub mod constants; @@ -875,11 +878,9 @@ where })?; let solution_rate = match response { - ReadResponse::SolutionRate(solution_rate) => solution_rate.ok_or(Error { - code: ErrorCode::ServerError(0), - message: "No blocks in state".to_string(), - data: None, - })?, + ReadResponse::SolutionRate(solution_rate) => { + solution_rate.ok_or_else(|| no_blocks_in_state_error())? + } _ => unreachable!("unmatched response to a solution rate request"), }; From bcce69bc6e5ac1cc21f94ed3d802f859ab4c41a4 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 9 Oct 2023 10:58:48 +1000 Subject: [PATCH 16/17] Check result values in the RPC test --- zebra-rpc/src/methods/tests/vectors.rs | 54 ++++++++++++++------------ 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index d6f926ff051..d5d2dea5391 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/zebra-rpc/src/methods/tests/vectors.rs @@ -1123,6 +1123,8 @@ async fn rpc_getnetworksolps() { use zebra_chain::chain_sync_status::MockSyncStatus; use zebra_network::address_book_peers::MockAddressBookPeers; + use crate::constants::no_blocks_in_state_error; + let _init_guard = zebra_test::init(); // Create a continuous chain of mainnet blocks from genesis @@ -1148,36 +1150,38 @@ async fn rpc_getnetworksolps() { ); let get_network_sol_ps_inputs = [ - (None, None), - (Some(-4), None), - (Some(-3), Some(0)), - (Some(-2), Some(-4)), - (Some(-1), Some(10)), - (Some(-1), Some(i32::MAX)), - (Some(0), None), - (Some(0), Some(0)), - (Some(0), Some(-3)), - (Some(0), Some(10)), - (Some(0), Some(i32::MAX)), - (Some(1), None), - (Some(1), Some(0)), - (Some(1), Some(-2)), - (Some(1), Some(10)), - (Some(1), Some(i32::MAX)), - (Some(i32::MAX), None), - (Some(i32::MAX), Some(0)), - (Some(i32::MAX), Some(-1)), - (Some(i32::MAX), Some(10)), - (Some(i32::MAX), Some(i32::MAX)), + // num_blocks, height, return value + (None, None, Ok(2)), + (Some(-4), None, Ok(2)), + (Some(-3), Some(0), Err(no_blocks_in_state_error())), + (Some(-2), Some(-4), Ok(2)), + (Some(-1), Some(10), Ok(2)), + (Some(-1), Some(i32::MAX), Ok(2)), + (Some(0), None, Ok(2)), + (Some(0), Some(0), Err(no_blocks_in_state_error())), + (Some(0), Some(-3), Ok(2)), + (Some(0), Some(10), Ok(2)), + (Some(0), Some(i32::MAX), Ok(2)), + (Some(1), None, Ok(4096)), + (Some(1), Some(0), Err(no_blocks_in_state_error())), + (Some(1), Some(-2), Ok(4096)), + (Some(1), Some(10), Ok(4096)), + (Some(1), Some(i32::MAX), Ok(4096)), + (Some(i32::MAX), None, Ok(2)), + (Some(i32::MAX), Some(0), Err(no_blocks_in_state_error())), + (Some(i32::MAX), Some(-1), Ok(2)), + (Some(i32::MAX), Some(10), Ok(2)), + (Some(i32::MAX), Some(i32::MAX), Ok(2)), ]; - for (num_blocks_input, height_input) in get_network_sol_ps_inputs { + for (num_blocks_input, height_input, return_value) in get_network_sol_ps_inputs { let get_network_sol_ps_result = get_block_template_rpc .get_network_sol_ps(num_blocks_input, height_input) .await; - assert!( - get_network_sol_ps_result.is_ok(), - "get_network_sol_ps({num_blocks_input:?}, {height_input:?}) call should be ok, \ + assert_eq!( + get_network_sol_ps_result, return_value, + "get_network_sol_ps({num_blocks_input:?}, {height_input:?}) result\n\ + should be {return_value:?},\n\ got: {get_network_sol_ps_result:?}" ); } From f9442da25212c8a608f2143d1b21416fabc52471 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 10 Oct 2023 14:38:13 +1000 Subject: [PATCH 17/17] Fix invalid calculation handling --- .../src/methods/get_block_template_rpcs.rs | 55 +++++++++---------- zebra-rpc/src/methods/tests/vectors.rs | 10 ++-- 2 files changed, 30 insertions(+), 35 deletions(-) diff --git a/zebra-rpc/src/methods/get_block_template_rpcs.rs b/zebra-rpc/src/methods/get_block_template_rpcs.rs index a0304ed52aa..e99258ad78f 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs.rs @@ -30,34 +30,31 @@ use zebra_network::AddressBookPeers; use zebra_node_services::mempool; use zebra_state::{ReadRequest, ReadResponse}; -use crate::{ - constants::no_blocks_in_state_error, - methods::{ - best_chain_tip_height, - get_block_template_rpcs::{ - constants::{ - DEFAULT_SOLUTION_RATE_WINDOW_SIZE, GET_BLOCK_TEMPLATE_MEMPOOL_LONG_POLL_INTERVAL, - ZCASHD_FUNDING_STREAM_ORDER, - }, - get_block_template::{ - check_miner_address, check_synced_to_tip, fetch_mempool_transactions, - fetch_state_tip_and_local_time, validate_block_proposal, - }, - // TODO: move the types/* modules directly under get_block_template_rpcs, - // and combine any modules with the same names. - types::{ - get_block_template::GetBlockTemplate, - get_mining_info, - hex_data::HexData, - long_poll::LongPollInput, - peer_info::PeerInfo, - submit_block, - subsidy::{BlockSubsidy, FundingStream}, - unified_address, validate_address, z_validate_address, - }, +use crate::methods::{ + best_chain_tip_height, + get_block_template_rpcs::{ + constants::{ + DEFAULT_SOLUTION_RATE_WINDOW_SIZE, GET_BLOCK_TEMPLATE_MEMPOOL_LONG_POLL_INTERVAL, + ZCASHD_FUNDING_STREAM_ORDER, + }, + get_block_template::{ + check_miner_address, check_synced_to_tip, fetch_mempool_transactions, + fetch_state_tip_and_local_time, validate_block_proposal, + }, + // TODO: move the types/* modules directly under get_block_template_rpcs, + // and combine any modules with the same names. + types::{ + get_block_template::GetBlockTemplate, + get_mining_info, + hex_data::HexData, + long_poll::LongPollInput, + peer_info::PeerInfo, + submit_block, + subsidy::{BlockSubsidy, FundingStream}, + unified_address, validate_address, z_validate_address, }, - height_from_signed_int, GetBlockHash, MISSING_BLOCK_ERROR_CODE, }, + height_from_signed_int, GetBlockHash, MISSING_BLOCK_ERROR_CODE, }; pub mod constants; @@ -878,9 +875,9 @@ where })?; let solution_rate = match response { - ReadResponse::SolutionRate(solution_rate) => { - solution_rate.ok_or_else(|| no_blocks_in_state_error())? - } + // zcashd returns a 0 rate when the calculation is invalid + ReadResponse::SolutionRate(solution_rate) => solution_rate.unwrap_or(0), + _ => unreachable!("unmatched response to a solution rate request"), }; diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index d5d2dea5391..769d561897b 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/zebra-rpc/src/methods/tests/vectors.rs @@ -1123,8 +1123,6 @@ async fn rpc_getnetworksolps() { use zebra_chain::chain_sync_status::MockSyncStatus; use zebra_network::address_book_peers::MockAddressBookPeers; - use crate::constants::no_blocks_in_state_error; - let _init_guard = zebra_test::init(); // Create a continuous chain of mainnet blocks from genesis @@ -1153,22 +1151,22 @@ async fn rpc_getnetworksolps() { // num_blocks, height, return value (None, None, Ok(2)), (Some(-4), None, Ok(2)), - (Some(-3), Some(0), Err(no_blocks_in_state_error())), + (Some(-3), Some(0), Ok(0)), (Some(-2), Some(-4), Ok(2)), (Some(-1), Some(10), Ok(2)), (Some(-1), Some(i32::MAX), Ok(2)), (Some(0), None, Ok(2)), - (Some(0), Some(0), Err(no_blocks_in_state_error())), + (Some(0), Some(0), Ok(0)), (Some(0), Some(-3), Ok(2)), (Some(0), Some(10), Ok(2)), (Some(0), Some(i32::MAX), Ok(2)), (Some(1), None, Ok(4096)), - (Some(1), Some(0), Err(no_blocks_in_state_error())), + (Some(1), Some(0), Ok(0)), (Some(1), Some(-2), Ok(4096)), (Some(1), Some(10), Ok(4096)), (Some(1), Some(i32::MAX), Ok(4096)), (Some(i32::MAX), None, Ok(2)), - (Some(i32::MAX), Some(0), Err(no_blocks_in_state_error())), + (Some(i32::MAX), Some(0), Ok(0)), (Some(i32::MAX), Some(-1), Ok(2)), (Some(i32::MAX), Some(10), Ok(2)), (Some(i32::MAX), Some(i32::MAX), Ok(2)),