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.

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 (3552eaf) and #3977 together #4010

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 10 additions & 2 deletions zebra-rpc/src/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ use zebra_node_services::{mempool, BoxError};
#[cfg(test)]
mod tests;

/// The RPC error code used by `zcashd` for missing blocks.
///
/// `lightwalletd` expects error code `-8` when a block is not found:
/// <https://github.com/adityapk00/lightwalletd/blob/c1bab818a683e4de69cd952317000f9bb2932274/common/common.go#L251-L254>
pub const MISSING_BLOCK_ERROR_CODE: ErrorCode = ErrorCode::ServerError(-8);

#[rpc(server)]
/// RPC method signatures.
pub trait Rpc {
Expand Down Expand Up @@ -78,6 +84,8 @@ pub trait Rpc {
) -> BoxFuture<Result<SentTransactionHash>>;

/// Returns the requested block by height, as a [`GetBlock`] JSON string.
/// If the block is not in Zebra's state, returns
/// [error code `-8`.](https://github.com/zcash/zcash/issues/5758)
///
/// zcashd reference: [`getblock`](https://zcash.github.io/rpc/getblock.html)
///
Expand All @@ -91,7 +99,7 @@ pub trait Rpc {
/// mode for all getblock calls: <https://github.com/zcash/lightwalletd/blob/v0.4.9/common/common.go#L232>
///
/// `lightwalletd` only requests blocks by height, so we don't support
/// getting blocks by hash but we do need to send the height number as a string.
/// getting blocks by hash. (But we parse the height as a JSON string, not an integer).
///
/// The `verbosity` parameter is ignored but required in the call.
#[rpc(name = "getblock")]
Expand Down Expand Up @@ -386,7 +394,7 @@ where
match response {
zebra_state::ReadResponse::Block(Some(block)) => Ok(GetBlock(block.into())),
zebra_state::ReadResponse::Block(None) => Err(Error {
code: ErrorCode::ServerError(0),
code: MISSING_BLOCK_ERROR_CODE,
message: "Block not found".to_string(),
data: None,
}),
Expand Down
51 changes: 50 additions & 1 deletion zebra-rpc/src/methods/tests/vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use std::sync::Arc;

use jsonrpc_core::ErrorCode;
use tower::buffer::Buffer;

use zebra_chain::{
Expand Down Expand Up @@ -85,7 +86,7 @@ async fn rpc_getblock() {
}

#[tokio::test]
async fn rpc_getblock_error() {
async fn rpc_getblock_parse_error() {
zebra_test::init();

let mut mempool: MockService<_, _, _, BoxError> = MockService::build().for_unit_tests();
Expand All @@ -110,6 +111,54 @@ async fn rpc_getblock_error() {
state.expect_no_requests().await;
}

#[tokio::test]
async fn rpc_getblock_missing_error() {
zebra_test::init();

let mut mempool: MockService<_, _, _, BoxError> = MockService::build().for_unit_tests();
let mut state: MockService<_, _, _, BoxError> = MockService::build().for_unit_tests();

// Init RPC
let rpc = RpcImpl::new(
"RPC test",
Buffer::new(mempool.clone(), 1),
Buffer::new(state.clone(), 1),
NoChainTip,
Mainnet,
);

// Make sure Zebra returns the correct error code `-8` for missing blocks
// https://github.com/adityapk00/lightwalletd/blob/c1bab818a683e4de69cd952317000f9bb2932274/common/common.go#L251-L254
let block_future = tokio::spawn(rpc.get_block("0".to_string(), 0u8));

// Make the mock service respond with no block
let response_handler = state
.expect_request(zebra_state::ReadRequest::Block(Height(0).into()))
.await;
response_handler.respond(zebra_state::ReadResponse::Block(None));

let block_response = block_future.await;
let block_response = block_response
.expect("unexpected panic in spawned request future")
.expect_err("unexpected success from missing block state response");
assert_eq!(block_response.code, ErrorCode::ServerError(-8),);

// Now check the error string the way `lightwalletd` checks it
assert_eq!(
serde_json::to_string(&block_response)
.expect("unexpected error serializing JSON error")
.split(':')
.nth(1)
.expect("unexpectedly low number of error fields")
.split(',')
.next(),
Some("-8")
);

mempool.expect_no_requests().await;
state.expect_no_requests().await;
}

#[tokio::test]
async fn rpc_getbestblockhash() {
zebra_test::init();
Expand Down
2 changes: 1 addition & 1 deletion zebra-test/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ impl<T> TestChild<T> {
stream_name
)
.context_from(self)
.with_section(|| format!("{:?}", success_regexes).header("Match Regex:"));
.with_section(|| format!("{:#?}", success_regexes.patterns()).header("Match Regex:"));

Err(report)
}
Expand Down
26 changes: 15 additions & 11 deletions zebrad/tests/acceptance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1072,18 +1072,22 @@ const LIGHTWALLETD_FAILURE_MESSAGES: &[&str] = &[
//
// get_block_chain_info
//
// TODO: enable these checks after PR #3891 merges
//
// invalid sapling height
//"Got sapling height 0",
"Got sapling height 0",
// missing BIP70 chain name, should be "main" or "test"
//" chain ",
" chain ",
// missing branchID, should be 8 hex digits
//" branchID \"",
" branchID \"",
// get_block
//
// a block error other than "-8: Block not found"
"error requesting block",
// a missing block with an incorrect error code
"Block not found",
//
// TODO: complete this list for each RPC with fields?
// get_info
// get_raw_transaction
// TODO: complete this list for each RPC with fields, if that RPC generates logs
// get_info - doesn't generate logs
// get_raw_transaction - might not generate logs
// z_get_tree_state
// get_address_txids
// get_address_balance
Expand Down Expand Up @@ -1206,9 +1210,9 @@ fn lightwalletd_integration() -> Result<()> {
//
// TODO: expect Ingestor log when we're using cached state (#3511)
// "Ingestor adding block to cache"
let result = lightwalletd.expect_stdout_line_matches(
r#"error requesting block: 0: Block not found","height":419200"#,
);
let result = lightwalletd.expect_stdout_line_matches(regex::escape(
"Waiting for zcashd height to reach Sapling activation height (419200)",
));
let (_, zebrad) = zebrad.kill_on_error(result)?;

// (next RPC)
Expand Down