diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 6087f68a948..c3e376f6136 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -613,24 +613,62 @@ where // This RPC is used in `lightwalletd`'s initial sync of 2 million blocks, // so it needs to load all its fields very efficiently. // - // Currently, we get the transaction IDs from an index, which is much more - // efficient than loading all the block data and hashing all the transactions. + // Currently, we get the block hash and transaction IDs from indexes, + // which is much more efficient than loading all the block data, + // then hashing the block header and all the transactions. - // TODO: look up the hash if we only have a height, - // and look up the height if we only have a hash - let hash = hash_or_height.hash().map(GetBlockHash); - let height = hash_or_height.height(); + // Get the block hash from the height -> hash index, if needed + // + // # Concurrency + // + // For consistency, this lookup must be performed first, then all the other + // lookups must be based on the hash. + // + // All possible responses are valid, even if the best chain changes. Clients + // must be able to handle chain forks, including a hash for a block that is + // later discovered to be on a side chain. + + let hash = match hash_or_height { + HashOrHeight::Hash(hash) => hash, + HashOrHeight::Height(height) => { + let request = zebra_state::ReadRequest::BestChainBlockHash(height); + let response = state + .ready() + .and_then(|service| service.call(request)) + .await + .map_err(|error| Error { + code: ErrorCode::ServerError(0), + message: error.to_string(), + data: None, + })?; + + match response { + zebra_state::ReadResponse::BlockHash(Some(hash)) => hash, + zebra_state::ReadResponse::BlockHash(None) => { + return Err(Error { + code: MISSING_BLOCK_ERROR_CODE, + message: "block height not in best chain".to_string(), + data: None, + }) + } + _ => unreachable!("unmatched response to a block hash request"), + } + } + }; - // TODO: do these state queries in parallel? + // TODO: do the txids and confirmations state queries in parallel? - // Get transaction IDs from the transaction index + // Get transaction IDs from the transaction index by block hash // // # Concurrency // + // We look up by block hash so the hash, transaction IDs, and confirmations + // are consistent. + // // A block's transaction IDs are never modified, so all possible responses are // valid. Clients that query block heights must be able to handle chain forks, // including getting transaction IDs from any chain fork. - let request = zebra_state::ReadRequest::TransactionIdsForBlock(hash_or_height); + let request = zebra_state::ReadRequest::TransactionIdsForBlock(hash.into()); let response = state .ready() .and_then(|service| service.call(request)) @@ -656,6 +694,9 @@ where // // # Concurrency // + // We look up by block hash so the hash, transaction IDs, and confirmations + // are consistent. + // // All possible responses are valid, even if a block is added to the chain, or // the best chain changes. Clients must be able to handle chain forks, including // different confirmation values before or after added blocks, and switching @@ -664,34 +705,31 @@ where // From const NOT_IN_BEST_CHAIN_CONFIRMATIONS: i64 = -1; - let confirmations = if let Some(hash) = hash_or_height.hash() { - let request = zebra_state::ReadRequest::Depth(hash); - let response = state - .ready() - .and_then(|service| service.call(request)) - .await - .map_err(|error| Error { - code: ErrorCode::ServerError(0), - message: error.to_string(), - data: None, - })?; - - match response { - // Confirmations are one more than the depth. - // Depth is limited by height, so it will never overflow an i64. - zebra_state::ReadResponse::Depth(Some(depth)) => Some(i64::from(depth) + 1), - zebra_state::ReadResponse::Depth(None) => { - Some(NOT_IN_BEST_CHAIN_CONFIRMATIONS) - } - _ => unreachable!("unmatched response to a depth request"), - } - } else { - // TODO: make Depth support heights as well - None + let request = zebra_state::ReadRequest::Depth(hash); + let response = state + .ready() + .and_then(|service| service.call(request)) + .await + .map_err(|error| Error { + code: ErrorCode::ServerError(0), + message: error.to_string(), + data: None, + })?; + + let confirmations = match response { + // Confirmations are one more than the depth. + // Depth is limited by height, so it will never overflow an i64. + zebra_state::ReadResponse::Depth(Some(depth)) => i64::from(depth) + 1, + zebra_state::ReadResponse::Depth(None) => NOT_IN_BEST_CHAIN_CONFIRMATIONS, + _ => unreachable!("unmatched response to a depth request"), }; + // TODO: look up the height if we only have a hash, + // this needs a new state request for the height -> hash index + let height = hash_or_height.height(); + Ok(GetBlock::Object { - hash, + hash: GetBlockHash(hash), confirmations, height, tx, @@ -1285,7 +1323,7 @@ pub struct SentTransactionHash(#[serde(with = "hex")] transaction::Hash); /// Response to a `getblock` RPC request. /// -/// See the notes for the [`Rpc::get_block` method]. +/// See the notes for the [`Rpc::get_block`] method. #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] #[serde(untagged)] pub enum GetBlock { @@ -1294,13 +1332,11 @@ pub enum GetBlock { /// The block object. Object { /// The hash of the requested block. - #[serde(skip_serializing_if = "Option::is_none")] - hash: Option, + hash: GetBlockHash, /// The number of confirmations of this block in the best chain, /// or -1 if it is not in the best chain. - #[serde(skip_serializing_if = "Option::is_none")] - confirmations: Option, + confirmations: i64, /// The height of the requested block. #[serde(skip_serializing_if = "Option::is_none")] diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@mainnet_10.snap index 241484dfd5f..ad487d39140 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@mainnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@mainnet_10.snap @@ -3,6 +3,8 @@ source: zebra-rpc/src/methods/tests/snapshot.rs expression: block --- { + "hash": "0007bc227e1c57a4a70e237cad00e7b7ce565155ab49166bc57397a26d339283", + "confirmations": 10, "height": 1, "tx": [ "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609" diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@testnet_10.snap index 4e586be83b1..02469914e6d 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@testnet_10.snap @@ -3,6 +3,8 @@ source: zebra-rpc/src/methods/tests/snapshot.rs expression: block --- { + "hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23", + "confirmations": 10, "height": 1, "tx": [ "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75" diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@mainnet_10.snap index 241484dfd5f..ad487d39140 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@mainnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@mainnet_10.snap @@ -3,6 +3,8 @@ source: zebra-rpc/src/methods/tests/snapshot.rs expression: block --- { + "hash": "0007bc227e1c57a4a70e237cad00e7b7ce565155ab49166bc57397a26d339283", + "confirmations": 10, "height": 1, "tx": [ "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609" diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@testnet_10.snap index 4e586be83b1..02469914e6d 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@testnet_10.snap @@ -3,6 +3,8 @@ source: zebra-rpc/src/methods/tests/snapshot.rs expression: block --- { + "hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23", + "confirmations": 10, "height": 1, "tx": [ "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75" diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 28822e11545..a524ef78b57 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/zebra-rpc/src/methods/tests/vectors.rs @@ -128,8 +128,8 @@ async fn rpc_getblock() { assert_eq!( get_block, GetBlock::Object { - hash: None, - confirmations: None, + hash: GetBlockHash(block.hash()), + confirmations: (blocks.len() - i).try_into().expect("valid i64"), height: Some(Height(i.try_into().expect("valid u32"))), tx: block .transactions @@ -150,8 +150,8 @@ async fn rpc_getblock() { assert_eq!( get_block, GetBlock::Object { - hash: Some(GetBlockHash(block.hash())), - confirmations: Some((blocks.len() - i).try_into().expect("valid i64")), + hash: GetBlockHash(block.hash()), + confirmations: (blocks.len() - i).try_into().expect("valid i64"), height: None, tx: block .transactions @@ -172,8 +172,8 @@ async fn rpc_getblock() { assert_eq!( get_block, GetBlock::Object { - hash: None, - confirmations: None, + hash: GetBlockHash(block.hash()), + confirmations: (blocks.len() - i).try_into().expect("valid i64"), height: Some(Height(i.try_into().expect("valid u32"))), tx: block .transactions @@ -194,8 +194,8 @@ async fn rpc_getblock() { assert_eq!( get_block, GetBlock::Object { - hash: Some(GetBlockHash(block.hash())), - confirmations: Some((blocks.len() - i).try_into().expect("valid i64")), + hash: GetBlockHash(block.hash()), + confirmations: (blocks.len() - i).try_into().expect("valid i64"), height: None, tx: block .transactions diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 239257e37ea..3d881844ad5 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -53,6 +53,23 @@ impl HashOrHeight { } } + /// Unwrap the inner hash or attempt to retrieve the hash for a given + /// height if one exists. + /// + /// # Consensus + /// + /// In the non-finalized state, a height can have multiple valid hashes. + /// We typically use the hash that is currently on the best chain. + pub fn hash_or_else(self, op: F) -> Option + where + F: FnOnce(block::Height) -> Option, + { + match self { + HashOrHeight::Hash(hash) => Some(hash), + HashOrHeight::Height(height) => op(height), + } + } + /// Returns the hash if this is a [`HashOrHeight::Hash`]. pub fn hash(&self) -> Option { if let HashOrHeight::Hash(hash) = self { @@ -801,7 +818,6 @@ pub enum ReadRequest { /// Returns [`ReadResponse::BestChainNextMedianTimePast`] when successful. BestChainNextMedianTimePast, - #[cfg(feature = "getblocktemplate-rpcs")] /// Looks up a block hash by height in the current best chain. /// /// Returns @@ -862,7 +878,6 @@ impl ReadRequest { "best_chain_tip_nullifiers_anchors" } ReadRequest::BestChainNextMedianTimePast => "best_chain_next_median_time_past", - #[cfg(feature = "getblocktemplate-rpcs")] ReadRequest::BestChainBlockHash(_) => "best_chain_block_hash", #[cfg(feature = "getblocktemplate-rpcs")] ReadRequest::ChainInfo => "chain_info", diff --git a/zebra-state/src/response.rs b/zebra-state/src/response.rs index c5a6160c48e..71842c4ba63 100644 --- a/zebra-state/src/response.rs +++ b/zebra-state/src/response.rs @@ -137,7 +137,6 @@ pub enum ReadResponse { /// Contains the median-time-past for the *next* block on the best chain. BestChainNextMedianTimePast(DateTime32), - #[cfg(feature = "getblocktemplate-rpcs")] /// Response to [`ReadRequest::BestChainBlockHash`](crate::ReadRequest::BestChainBlockHash) with the /// specified block hash. BlockHash(Option), @@ -231,13 +230,13 @@ impl TryFrom for Response { Err("there is no corresponding Response for this ReadResponse") } - #[cfg(feature = "getblocktemplate-rpcs")] - ReadResponse::ValidBlockProposal => Ok(Response::ValidBlockProposal), - - #[cfg(feature = "getblocktemplate-rpcs")] ReadResponse::BlockHash(_) => { Err("there is no corresponding Response for this ReadResponse") } + + #[cfg(feature = "getblocktemplate-rpcs")] + ReadResponse::ValidBlockProposal => Ok(Response::ValidBlockProposal), + #[cfg(feature = "getblocktemplate-rpcs")] ReadResponse::ChainInfo(_) | ReadResponse::SolutionRate(_) => { Err("there is no corresponding Response for this ReadResponse") diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 623fbca1ba5..ead8880860c 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -1621,8 +1621,7 @@ impl Service for ReadStateService { .boxed() } - // Used by get_block_hash RPC. - #[cfg(feature = "getblocktemplate-rpcs")] + // Used by the get_block and get_block_hash RPCs. ReadRequest::BestChainBlockHash(height) => { let timer = CodeTimer::start(); diff --git a/zebrad/tests/common/lightwalletd.rs b/zebrad/tests/common/lightwalletd.rs index b8f60326942..a9f2a196915 100644 --- a/zebrad/tests/common/lightwalletd.rs +++ b/zebrad/tests/common/lightwalletd.rs @@ -247,10 +247,15 @@ where fn with_lightwalletd_config(self, zebra_rpc_listener: SocketAddr) -> Result { use std::fs; + // zcash/lightwalletd requires rpcuser and rpcpassword, or a zcashd cookie file + // But when a lightwalletd with this config is used by Zebra, + // Zebra ignores any authentication and provides access regardless. let lightwalletd_config = format!( "\ rpcbind={}\n\ rpcport={}\n\ + rpcuser=xxxxx + rpcpassword=xxxxx ", zebra_rpc_listener.ip(), zebra_rpc_listener.port(),