From bc7329e2c8470c2dacd2ba2593fe6b677f288fb4 Mon Sep 17 00:00:00 2001 From: teor Date: Wed, 18 Jan 2023 13:15:06 +1000 Subject: [PATCH 1/2] Return detailed errors to the RPC client when a block proposal fails --- .../get_block_template.rs | 22 +++++-- .../types/get_block_template.rs | 2 +- .../types/get_block_template/proposal.rs | 58 +++++++++---------- ..._template_invalid-proposal@mainnet_10.snap | 7 +-- ..._template_invalid-proposal@testnet_10.snap | 7 +-- 5 files changed, 48 insertions(+), 48 deletions(-) diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs index 16cb2a9e212..584e485249a 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs/get_block_template.rs @@ -10,7 +10,7 @@ use zebra_chain::{ block::{ self, merkle::{self, AuthDataRoot}, - ChainHistoryBlockTxAuthCommitmentHash, Height, + Block, ChainHistoryBlockTxAuthCommitmentHash, Height, }, chain_sync_status::ChainSyncStatus, chain_tip::ChainTip, @@ -108,8 +108,18 @@ where + Sync + 'static, { - let Ok(block) = block_proposal_bytes.zcash_deserialize_into::() else { - return Ok(ProposalRejectReason::Rejected.into()) + let block: Block = match block_proposal_bytes.zcash_deserialize_into() { + Ok(block) => block, + Err(parse_error) => { + tracing::info!( + ?parse_error, + "error response from block parser in CheckProposal request" + ); + + return Ok( + ProposalResponse::rejected("invalid proposal format", parse_error.into()).into(), + ); + } }; let chain_verifier_response = chain_verifier @@ -127,11 +137,11 @@ where .map(|_hash| ProposalResponse::Valid) .unwrap_or_else(|verify_chain_error| { tracing::info!( - verify_chain_error, - "Got error response from chain_verifier CheckProposal request" + ?verify_chain_error, + "error response from chain_verifier in CheckProposal request" ); - ProposalRejectReason::Rejected.into() + ProposalResponse::rejected("invalid proposal", verify_chain_error) }) .into()) } diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template.rs b/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template.rs index 8672ac1a13c..4135fb1232b 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template.rs @@ -31,7 +31,7 @@ pub mod parameters; pub mod proposal; pub use parameters::{GetBlockTemplateCapability, GetBlockTemplateRequestMode, JsonParameters}; -pub use proposal::{proposal_block_from_template, ProposalRejectReason, ProposalResponse}; +pub use proposal::{proposal_block_from_template, ProposalResponse}; /// A serialized `getblocktemplate` RPC response in template mode. #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template/proposal.rs b/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template/proposal.rs index 8a44f179ca0..9f4e1a1cca9 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template/proposal.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template/proposal.rs @@ -2,13 +2,14 @@ //! //! `ProposalResponse` is the output of the `getblocktemplate` RPC method in 'proposal' mode. -use std::{num::ParseIntError, str::FromStr, sync::Arc}; +use std::{error::Error, num::ParseIntError, str::FromStr, sync::Arc}; use zebra_chain::{ block::{self, Block, Height}, serialization::{DateTime32, SerializationError, ZcashDeserializeInto}, work::equihash::Solution, }; +use zebra_node_services::BoxError; use crate::methods::{ get_block_template_rpcs::types::{ @@ -18,47 +19,46 @@ use crate::methods::{ GetBlockHash, }; -/// Error response to a `getblocktemplate` RPC request in proposal mode. -/// -/// See -#[derive(Copy, Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] -#[serde(rename_all = "kebab-case")] -pub enum ProposalRejectReason { - /// Block proposal rejected as invalid. - Rejected, -} - /// Response to a `getblocktemplate` RPC request in proposal mode. /// -/// See +/// +/// +/// Note: +/// The error response specification at +/// seems to have a copy-paste issue, or it is under-specified. We follow the `zcashd` +/// implementation instead, which returns a single raw string. #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(untagged, rename_all = "kebab-case")] pub enum ProposalResponse { - /// Block proposal was rejected as invalid, returns `reject-reason` and server `capabilities`. - ErrorResponse { - /// Reason the proposal was invalid as-is. - reject_reason: ProposalRejectReason, - - /// The getblocktemplate RPC capabilities supported by Zebra. - capabilities: Vec, - }, + /// Block proposal was rejected as invalid. + /// Contains the reason that the proposal was invalid. + /// + /// TODO: turn this into a typed error enum? + Rejected(String), /// Block proposal was successfully validated, returns null. Valid, } -impl From for ProposalResponse { - fn from(reject_reason: ProposalRejectReason) -> Self { - Self::ErrorResponse { - reject_reason, - capabilities: GetBlockTemplate::capabilities(), - } +impl ProposalResponse { + /// Return a rejected response containing an error kind and detailed error info. + pub fn rejected(kind: S, error: BoxError) -> Self { + let kind = kind.to_string(); + + // Pretty-print the detailed error for now + ProposalResponse::Rejected(format!("{kind}: {error:#?}")) + } + + /// Return a rejected response containing just the detailed error information. + pub fn error(error: BoxError) -> Self { + // Pretty-print the detailed error for now + ProposalResponse::Rejected(format!("{error:#?}")) } } -impl From for Response { - fn from(error_response: ProposalRejectReason) -> Self { - Self::ProposalMode(ProposalResponse::from(error_response)) +impl From for ProposalResponse { + fn from(error: E) -> Self { + Self::error(error.into()) } } diff --git a/zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_template_invalid-proposal@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_template_invalid-proposal@mainnet_10.snap index 3a63ad3879a..6d80c4277fa 100644 --- a/zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_template_invalid-proposal@mainnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_template_invalid-proposal@mainnet_10.snap @@ -2,9 +2,4 @@ source: zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs expression: block_template --- -{ - "reject_reason": "rejected", - "capabilities": [ - "proposal" - ] -} +"invalid proposal format: Io(\n Error {\n kind: UnexpectedEof,\n message: \"failed to fill whole buffer\",\n },\n)" diff --git a/zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_template_invalid-proposal@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_template_invalid-proposal@testnet_10.snap index 3a63ad3879a..6d80c4277fa 100644 --- a/zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_template_invalid-proposal@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshot/snapshots/get_block_template_invalid-proposal@testnet_10.snap @@ -2,9 +2,4 @@ source: zebra-rpc/src/methods/tests/snapshot/get_block_template_rpcs.rs expression: block_template --- -{ - "reject_reason": "rejected", - "capabilities": [ - "proposal" - ] -} +"invalid proposal format: Io(\n Error {\n kind: UnexpectedEof,\n message: \"failed to fill whole buffer\",\n },\n)" From fde6d52de4af2cfd824e3309b8fb7a320cd27aea Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 19 Jan 2023 12:52:20 +1000 Subject: [PATCH 2/2] Change proposal reject type in acceptance test --- .../tests/common/get_block_template_rpcs/get_block_template.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebrad/tests/common/get_block_template_rpcs/get_block_template.rs b/zebrad/tests/common/get_block_template_rpcs/get_block_template.rs index 74069655993..1c0133b4ff6 100644 --- a/zebrad/tests/common/get_block_template_rpcs/get_block_template.rs +++ b/zebrad/tests/common/get_block_template_rpcs/get_block_template.rs @@ -167,7 +167,7 @@ async fn try_validate_block_template(client: &RPCRequestClient) -> Result<()> { "got getblocktemplate proposal response" ); - if let ProposalResponse::ErrorResponse { reject_reason, .. } = json_result { + if let ProposalResponse::Rejected(reject_reason) = json_result { Err(eyre!( "unsuccessful block proposal validation, reason: {reject_reason:?}" ))?;