Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge queue: embarking main (4241e32) and #8384 together #8397

Closed
wants to merge 5 commits into from
Closed

merge queue: embarking main (4241e32) and #8384 together #8397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4aa00fa
Returns block times from the getblock RPC when used with verbosity = …
arya2 Apr 4, 2024
8513d31
cleanup/refactor, adds MapServerError and OkOrServerError traits
arya2 Apr 9, 2024
5da0ca6
moves rpc error conversion traits to their own module
arya2 Apr 9, 2024
c4fe473
Only returns block time for verbosity = 2, updates snapshots
arya2 Apr 10, 2024
487c2b0
Merge of #8384
mergify[bot] Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
200 changes: 89 additions & 111 deletions zebra-rpc/src/methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ use crate::{
queue::Queue,
};

mod errors;

use errors::{MapServerError, OkOrServerError};

// We don't use a types/ module here, because it is redundant.
pub mod trees;

Expand Down Expand Up @@ -675,7 +679,6 @@ where
}

// TODO:
// - use a generic error constructor (#5548)
// - use `height_from_signed_int()` to handle negative heights
// (this might be better in the state request, because it needs the state height)
// - create a function that handles block hashes or heights, and use it in `z_get_treestate()`
Expand Down Expand Up @@ -727,7 +730,7 @@ where
}),
_ => unreachable!("unmatched response to a block request"),
}
} else if verbosity == 1 {
} else if verbosity == 1 || verbosity == 2 {
// # Performance
//
// This RPC is used in `lightwalletd`'s initial sync of 2 million blocks,
Expand All @@ -748,6 +751,8 @@ where
// must be able to handle chain forks, including a hash for a block that is
// later discovered to be on a side chain.

let should_read_block_header = verbosity == 2;

let hash = match hash_or_height {
HashOrHeight::Hash(hash) => hash,
HashOrHeight::Height(height) => {
Expand Down Expand Up @@ -776,135 +781,102 @@ where
}
};

// 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.into());
let tx_ids_response_fut = state.clone().oneshot(request);

// Get block confirmations from the block height index
//
// # 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
// between -1 and multiple different confirmation values.

// From <https://zcash.github.io/rpc/getblock.html>
const NOT_IN_BEST_CHAIN_CONFIRMATIONS: i64 = -1;

let request = zebra_state::ReadRequest::Depth(hash);
let depth_response_fut = state.clone().oneshot(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();

// Sapling trees
//
// # Concurrency
//
// We look up by block hash so the hash, transaction IDs, and confirmations
// are consistent.
let request = zebra_state::ReadRequest::SaplingTree(hash.into());
let sapling_tree_response_fut = state.clone().oneshot(request);

// Orchard trees
//
// # Concurrency
//
// We look up by block hash so the hash, transaction IDs, and confirmations
// are consistent.
let request = zebra_state::ReadRequest::OrchardTree(hash.into());
let orchard_tree_response_fut = state.clone().oneshot(request);
let mut requests = vec![
// Get transaction IDs from the transaction index by block hash
//
// # Concurrency
//
// 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.
zebra_state::ReadRequest::TransactionIdsForBlock(hash.into()),
// Sapling trees
zebra_state::ReadRequest::SaplingTree(hash.into()),
// Orchard trees
zebra_state::ReadRequest::OrchardTree(hash.into()),
// Get block confirmations from the block height index
//
// # Concurrency
//
// 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
// between -1 and multiple different confirmation values.
zebra_state::ReadRequest::Depth(hash),
];

if should_read_block_header {
// Block header
requests.push(zebra_state::ReadRequest::BlockHeader(hash.into()))
}

let mut futs = FuturesOrdered::new();
futs.push_back(tx_ids_response_fut);
futs.push_back(sapling_tree_response_fut);
futs.push_back(orchard_tree_response_fut);
futs.push_back(depth_response_fut);

let tx_ids_response = futs
.next()
.await
.expect("should have 4 items in futs")
.map_err(|error| Error {
code: ErrorCode::ServerError(0),
message: error.to_string(),
data: None,
})?;

let sapling_tree_response = futs
.next()
.await
.expect("should have 3 items in futs")
.map_err(|error| Error {
code: ErrorCode::ServerError(0),
message: error.to_string(),
data: None,
})?;

let orchard_tree_response = futs
.next()
.await
.expect("should have 2 items in futs")
.map_err(|error| Error {
code: ErrorCode::ServerError(0),
message: error.to_string(),
data: None,
})?;

let depth_response = futs
.next()
.await
.expect("should have an item in futs")
.map_err(|error| Error {
code: ErrorCode::ServerError(0),
message: error.to_string(),
data: None,
})?;
for request in requests {
futs.push_back(state.clone().oneshot(request));
}

let tx = match tx_ids_response {
zebra_state::ReadResponse::TransactionIdsForBlock(Some(tx_ids)) => {
tx_ids.iter().map(|tx_id| tx_id.encode_hex()).collect()
}
zebra_state::ReadResponse::TransactionIdsForBlock(None) => Err(Error {
code: MISSING_BLOCK_ERROR_CODE,
message: "Block not found".to_string(),
data: None,
})?,
let tx_ids_response = futs.next().await.expect("`futs` should not be empty");
let tx = match tx_ids_response.map_server_error()? {
zebra_state::ReadResponse::TransactionIdsForBlock(tx_ids) => tx_ids
.ok_or_server_error("Block not found")?
.iter()
.map(|tx_id| tx_id.encode_hex())
.collect(),
_ => unreachable!("unmatched response to a transaction_ids_for_block request"),
};

let sapling_note_commitment_tree_count = match sapling_tree_response {
zebra_state::ReadResponse::SaplingTree(Some(nct)) => nct.count(),
zebra_state::ReadResponse::SaplingTree(None) => 0,
_ => unreachable!("unmatched response to a SaplingTree request"),
};
let sapling_tree_response = futs.next().await.expect("`futs` should not be empty");
let sapling_note_commitment_tree_count =
match sapling_tree_response.map_server_error()? {
zebra_state::ReadResponse::SaplingTree(Some(nct)) => nct.count(),
zebra_state::ReadResponse::SaplingTree(None) => 0,
_ => unreachable!("unmatched response to a SaplingTree request"),
};

let orchard_tree_response = futs.next().await.expect("`futs` should not be empty");
let orchard_note_commitment_tree_count =
match orchard_tree_response.map_server_error()? {
zebra_state::ReadResponse::OrchardTree(Some(nct)) => nct.count(),
zebra_state::ReadResponse::OrchardTree(None) => 0,
_ => unreachable!("unmatched response to a OrchardTree request"),
};

// From <https://zcash.github.io/rpc/getblock.html>
const NOT_IN_BEST_CHAIN_CONFIRMATIONS: i64 = -1;

let confirmations = match depth_response {
let depth_response = futs.next().await.expect("`futs` should not be empty");
let confirmations = match depth_response.map_server_error()? {
// 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();

let orchard_note_commitment_tree_count = match orchard_tree_response {
zebra_state::ReadResponse::OrchardTree(Some(nct)) => nct.count(),
zebra_state::ReadResponse::OrchardTree(None) => 0,
_ => unreachable!("unmatched response to a OrchardTree request"),
let time = if should_read_block_header {
let block_header_response =
futs.next().await.expect("`futs` should not be empty");

match block_header_response.map_server_error()? {
zebra_state::ReadResponse::BlockHeader(header) => Some(
header
.ok_or_server_error("Block not found")?
.time
.timestamp(),
),
_ => unreachable!("unmatched response to a BlockHeader request"),
}
} else {
None
};

let sapling = SaplingTrees {
Expand All @@ -921,6 +893,7 @@ where
hash: GetBlockHash(hash),
confirmations,
height,
time,
tx,
trees,
})
Expand Down Expand Up @@ -1639,6 +1612,10 @@ pub enum GetBlock {
#[serde(skip_serializing_if = "Option::is_none")]
height: Option<Height>,

/// The height of the requested block.
#[serde(skip_serializing_if = "Option::is_none")]
time: Option<i64>,

/// List of transaction IDs in block order, hex-encoded.
//
// TODO: use a typed Vec<transaction::Hash> here
Expand All @@ -1655,6 +1632,7 @@ impl Default for GetBlock {
hash: GetBlockHash::default(),
confirmations: 0,
height: None,
time: None,
tx: Vec::new(),
trees: GetBlockTrees::default(),
}
Expand Down
37 changes: 37 additions & 0 deletions zebra-rpc/src/methods/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//! Error conversions for Zebra's RPC methods.

use jsonrpc_core::ErrorCode;

pub(crate) trait MapServerError<T, E> {
fn map_server_error(self) -> std::result::Result<T, jsonrpc_core::Error>;
}

pub(crate) trait OkOrServerError<T> {
fn ok_or_server_error<S: ToString>(
self,
message: S,
) -> std::result::Result<T, jsonrpc_core::Error>;
}

impl<T, E> MapServerError<T, E> for Result<T, E>
where
E: ToString,
{
fn map_server_error(self) -> Result<T, jsonrpc_core::Error> {
self.map_err(|error| jsonrpc_core::Error {
code: ErrorCode::ServerError(0),
message: error.to_string(),
data: None,
})
}
}

impl<T> OkOrServerError<T> for Option<T> {
fn ok_or_server_error<S: ToString>(self, message: S) -> Result<T, jsonrpc_core::Error> {
self.ok_or(jsonrpc_core::Error {
code: ErrorCode::ServerError(0),
message: message.to_string(),
data: None,
})
}
}
21 changes: 20 additions & 1 deletion zebra-rpc/src/methods/tests/snapshot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//!
//! To update these snapshots, run:
//! ```sh
//! cargo insta test --review
//! cargo insta test --review -p zebra-rpc --lib -- test_rpc_response_data
//! ```

use std::{collections::BTreeMap, sync::Arc};
Expand Down Expand Up @@ -167,6 +167,25 @@ async fn test_rpc_response_data_for_network(network: &Network) {
.expect("We should have a GetBlock struct");
snapshot_rpc_getblock_verbose("hash_verbosity_1", get_block, &settings);

// `getblock`, verbosity=2, height
let get_block = rpc
.get_block(BLOCK_HEIGHT.to_string(), Some(2u8))
.await
.expect("We should have a GetBlock struct");
snapshot_rpc_getblock_verbose("height_verbosity_2", get_block, &settings);

let get_block = rpc
.get_block(EXCESSIVE_BLOCK_HEIGHT.to_string(), Some(2u8))
.await;
snapshot_rpc_getblock_invalid("excessive_height_verbosity_2", get_block, &settings);

// `getblock`, verbosity=2, hash
let get_block = rpc
.get_block(block_hash.to_string(), Some(2u8))
.await
.expect("We should have a GetBlock struct");
snapshot_rpc_getblock_verbose("hash_verbosity_2", get_block, &settings);

// `getblock`, no verbosity - defaults to 1, height
let get_block = rpc
.get_block(BLOCK_HEIGHT.to_string(), None)
Expand Down
10 changes: 10 additions & 0 deletions ...rc/methods/tests/snapshots/get_block_invalid_excessive_height_verbosity_2@mainnet_10.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: zebra-rpc/src/methods/tests/snapshot.rs
expression: response
---
{
"Err": {
"code": -8,
"message": "block height not in best chain"
}
}
10 changes: 10 additions & 0 deletions ...rc/methods/tests/snapshots/get_block_invalid_excessive_height_verbosity_2@testnet_10.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: zebra-rpc/src/methods/tests/snapshot.rs
expression: response
---
{
"Err": {
"code": -8,
"message": "block height not in best chain"
}
}
13 changes: 13 additions & 0 deletions zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_2@mainnet_10.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
source: zebra-rpc/src/methods/tests/snapshot.rs
expression: block
---
{
"hash": "0007bc227e1c57a4a70e237cad00e7b7ce565155ab49166bc57397a26d339283",
"confirmations": 10,
"time": 1477671596,
"tx": [
"851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609"
],
"trees": {}
}
13 changes: 13 additions & 0 deletions zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_2@testnet_10.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
source: zebra-rpc/src/methods/tests/snapshot.rs
expression: block
---
{
"hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23",
"confirmations": 10,
"time": 1477674473,
"tx": [
"f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75"
],
"trees": {}
}
Loading
Loading