From 31e4a7bbf94cba66293b635cd773b6a21e0184a9 Mon Sep 17 00:00:00 2001 From: jaoleal Date: Thu, 5 Feb 2026 20:05:55 -0300 Subject: [PATCH 1/3] fix(rpcserver): Refactor error types, arg_parser, and remove unwrap() calls - Move JsonRpcError into a jsonrpc_interface submodule with structured RpcError/Response types, HTTP status code mapping, and named constants for floresta-specific RPC error codes (TX_NOT_FOUND, BLOCK_NOT_FOUND, PEER_NOT_FOUND, etc.) - Add From for JsonRpcError conversion - Rewrite arg_parser with generic get_at(), get_with_default(), and try_into_optional() functions supporting both positional (array) and named (object) parameters - Change RpcRequest.params from Vec to Option - Split method dispatch into no-param methods and param methods - Replace all .unwrap() calls - Apply #[deny(clippy::unwrap_used)] lint to the json_rpc module - Change ScriptPubKeyJson.address to Option for nonstandard scripts (matching Bitcoin Core behavior) - Add module-level documentation for JSON-RPC 1.0/2.0 version support --- .../floresta-node/src/json_rpc/blockchain.rs | 57 +- crates/floresta-node/src/json_rpc/control.rs | 2 +- crates/floresta-node/src/json_rpc/mod.rs | 9 + crates/floresta-node/src/json_rpc/network.rs | 2 +- crates/floresta-node/src/json_rpc/request.rs | 160 ++-- crates/floresta-node/src/json_rpc/res.rs | 711 ++++++++++++------ crates/floresta-node/src/json_rpc/server.rs | 551 ++++++-------- crates/floresta-node/src/lib.rs | 1 + 8 files changed, 827 insertions(+), 666 deletions(-) diff --git a/crates/floresta-node/src/json_rpc/blockchain.rs b/crates/floresta-node/src/json_rpc/blockchain.rs index efeb6c463..db16f009d 100644 --- a/crates/floresta-node/src/json_rpc/blockchain.rs +++ b/crates/floresta-node/src/json_rpc/blockchain.rs @@ -33,15 +33,20 @@ use tracing::debug; use super::res::GetBlockHeaderRes; use super::res::GetBlockchainInfoRes; use super::res::GetTxOutProof; -use super::res::JsonRpcError; +use super::res::jsonrpc_interface::JsonRpcError; use super::server::RpcChain; use super::server::RpcImpl; use crate::json_rpc::res::GetBlockRes; use crate::json_rpc::res::RescanConfidence; +use crate::json_rpc::server::SERIALIZATION_EXPECT_MSG; impl RpcImpl { async fn get_block_inner(&self, hash: BlockHash) -> Result { - let is_genesis = self.chain.get_block_hash(0).unwrap().eq(&hash); + let is_genesis = self + .chain + .get_block_hash(0) + .map_err(|_| JsonRpcError::Chain)? + .eq(&hash); if is_genesis { return Ok(genesis_block(self.network)); @@ -64,7 +69,11 @@ impl RpcImpl { .wallet .get_height(txid) .ok_or(JsonRpcError::TxNotFound)?; - let blockhash = self.chain.get_block_hash(height).unwrap(); + let blockhash = self + .chain + .get_block_hash(height) + .map_err(|_| JsonRpcError::BlockNotFound)?; + self.chain .get_block(&blockhash) .map_err(|_| JsonRpcError::BlockNotFound) @@ -73,17 +82,11 @@ impl RpcImpl { pub fn get_rescan_interval( &self, use_timestamp: bool, - start: Option, - stop: Option, - confidence: Option, + start: u32, + stop: u32, + confidence: RescanConfidence, ) -> Result<(u32, u32), JsonRpcError> { - let start = start.unwrap_or(0u32); - let stop = stop.unwrap_or(0u32); - if use_timestamp { - let confidence = confidence.unwrap_or(RescanConfidence::Medium); - // `get_block_height_by_timestamp` already does the time validity checks. - let start_height = self.get_block_height_by_timestamp(start, &confidence)?; let stop_height = self.get_block_height_by_timestamp(stop, &RescanConfidence::Exact)?; @@ -176,7 +179,11 @@ impl RpcImpl { // getbestblockhash pub(super) fn get_best_block_hash(&self) -> Result { - Ok(self.chain.get_best_block().unwrap().1) + Ok(self + .chain + .get_best_block() + .map_err(|_| JsonRpcError::Chain)? + .1) } // getblock @@ -239,10 +246,19 @@ impl RpcImpl { // getblockchaininfo pub(super) fn get_blockchain_info(&self) -> Result { - let (height, hash) = self.chain.get_best_block().unwrap(); - let validated = self.chain.get_validation_index().unwrap(); + let (height, hash) = self + .chain + .get_best_block() + .map_err(|_| JsonRpcError::Chain)?; + let validated = self + .chain + .get_validation_index() + .map_err(|_| JsonRpcError::Chain)?; let ibd = self.chain.is_in_ibd(); - let latest_header = self.chain.get_block_header(&hash).unwrap(); + let latest_header = self + .chain + .get_block_header(&hash) + .map_err(|_| JsonRpcError::Chain)?; let latest_work = latest_header .calculate_chain_work(&self.chain)? .to_string_hex(); @@ -257,7 +273,10 @@ impl RpcImpl { .map(|r| r.to_string()) .collect(); - let validated_blocks = self.chain.get_validation_index().unwrap(); + let validated_blocks = self + .chain + .get_validation_index() + .map_err(|_| JsonRpcError::Chain)?; let validated_percentage = if height != 0 { validated_blocks as f32 / height as f32 @@ -283,7 +302,7 @@ impl RpcImpl { // getblockcount pub(super) fn get_block_count(&self) -> Result { - Ok(self.chain.get_height().unwrap()) + self.chain.get_height().map_err(|_| JsonRpcError::Chain) } // getblockfilter @@ -708,7 +727,7 @@ impl RpcImpl { height: u32, ) -> Result { if let Some(txout) = self.wallet.get_utxo(&OutPoint { txid, vout }) { - return Ok(serde_json::to_value(txout).unwrap()); + return Ok(serde_json::to_value(txout).expect(SERIALIZATION_EXPECT_MSG)); } // if we are on IBD, we don't have any filters to find this txout. diff --git a/crates/floresta-node/src/json_rpc/control.rs b/crates/floresta-node/src/json_rpc/control.rs index 443da05a8..b6efa2429 100644 --- a/crates/floresta-node/src/json_rpc/control.rs +++ b/crates/floresta-node/src/json_rpc/control.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; use serde::Deserialize; use serde::Serialize; -use super::res::JsonRpcError; +use super::res::jsonrpc_interface::JsonRpcError; use super::server::RpcChain; use super::server::RpcImpl; diff --git a/crates/floresta-node/src/json_rpc/mod.rs b/crates/floresta-node/src/json_rpc/mod.rs index 4d69352b4..f04e366bf 100644 --- a/crates/floresta-node/src/json_rpc/mod.rs +++ b/crates/floresta-node/src/json_rpc/mod.rs @@ -1,5 +1,14 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 +//! Floresta's JSON-RPC server. +//! +//! The server accepts both JSON-RPC 1.0 and 2.0 requests. Clients may send +//! `"jsonrpc": "1.0"`, `"jsonrpc": "2.0"`, or omit the field entirely +//! (JSON-RPC 1.0 style). All responses follow the JSON-RPC 2.0 format. +//! +//! Version acceptance is validated in [`server`] and covered by integration +//! tests in `tests/florestad/rpcserver_request_parsing.py`. + pub mod request; pub mod res; pub mod server; diff --git a/crates/floresta-node/src/json_rpc/network.rs b/crates/floresta-node/src/json_rpc/network.rs index ff48d644c..547659ef7 100644 --- a/crates/floresta-node/src/json_rpc/network.rs +++ b/crates/floresta-node/src/json_rpc/network.rs @@ -20,7 +20,7 @@ use floresta_wire::node_interface::PeerInfo; use serde_json::Value; use serde_json::json; -use super::res::JsonRpcError; +use super::res::jsonrpc_interface::JsonRpcError; use super::server::RpcChain; use super::server::RpcImpl; diff --git a/crates/floresta-node/src/json_rpc/request.rs b/crates/floresta-node/src/json_rpc/request.rs index 2f72e9866..1e25058b5 100644 --- a/crates/floresta-node/src/json_rpc/request.rs +++ b/crates/floresta-node/src/json_rpc/request.rs @@ -3,10 +3,12 @@ //! This module defines the structure for JSON-RPC requests and provides utility functions to //! extract parameters from the request. +use serde::Deserialize; +use serde::Serialize; use serde_json::Value; -#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] -/// Represents a JSON-RPC 2.0 request. +#[derive(Clone, Debug, Serialize, Deserialize)] +/// Represents a JSON-RPC request (versions 1.0 and 2.0). pub struct RpcRequest { /// The JSON-RPC version, typically "2.0". /// @@ -18,8 +20,8 @@ pub struct RpcRequest { /// The method to be invoked, e.g., "getblock", "sendtransaction". pub method: String, - /// The parameters for the method, as an array of json values. - pub params: Vec, + /// The parameters for the method, json value that must be an array or an object. + pub params: Option, /// An optional identifier for the request, which can be used to match responses. pub id: Value, @@ -29,130 +31,66 @@ pub struct RpcRequest { /// methods already handle the case where the parameter is missing or has an /// unexpected type, returning an error if so. pub mod arg_parser { - use core::str::FromStr; + use serde::Deserialize; use serde_json::Value; - use crate::json_rpc::res::JsonRpcError; + use crate::json_rpc::res::jsonrpc_interface::JsonRpcError; - /// Extracts a u64 parameter from the request parameters at the specified index. - /// - /// This function checks if the parameter exists, is of type u64 and can be converted to `T`. - /// Returns an error otherwise. - pub fn get_numeric>( - params: &[Value], - index: usize, - opt_name: &str, - ) -> Result { - let v = params - .get(index) - .ok_or_else(|| JsonRpcError::MissingParameter(opt_name.to_string()))?; - - let n = v.as_u64().ok_or_else(|| { - JsonRpcError::InvalidParameterType(format!("{opt_name} must be a number")) - })?; - - T::try_from(n) - .map_err(|_| JsonRpcError::InvalidParameterType(format!("{opt_name} is out-of-range"))) - } - - /// Extracts a string parameter from the request parameters at the specified index. - /// - /// This function checks if the parameter exists and is of type string. Returns an error - /// otherwise. - pub fn get_string( - params: &[Value], - index: usize, - opt_name: &str, - ) -> Result { - let v = params - .get(index) - .ok_or_else(|| JsonRpcError::MissingParameter(opt_name.to_string()))?; - - let str = v.as_str().ok_or_else(|| { - JsonRpcError::InvalidParameterType(format!("{opt_name} must be a string")) - })?; - - Ok(str.to_string()) - } - - /// Extracts a boolean parameter from the request parameters at the specified index. - /// - /// This function checks if the parameter exists and is of type boolean. Returns an error - /// otherwise. - pub fn get_bool(params: &[Value], index: usize, opt_name: &str) -> Result { - let v = params - .get(index) - .ok_or_else(|| JsonRpcError::MissingParameter(opt_name.to_string()))?; - - v.as_bool().ok_or_else(|| { - JsonRpcError::InvalidParameterType(format!("{opt_name} must be a boolean")) - }) - } - - /// Extracts a hash parameter from the request parameters at the specified index. + /// Extracts a parameter from the request parameters at the specified index. /// /// This function can extract any type that implements `FromStr`, such as `BlockHash` or /// `Txid`. It checks if the parameter exists and is a valid string representation of the type. /// Returns an error otherwise. - pub fn get_hash( - params: &[Value], + pub fn get_at<'de, T: Deserialize<'de>>( + params: &'de Value, index: usize, - opt_name: &str, + field_name: &str, ) -> Result { - let v = params - .get(index) - .ok_or_else(|| JsonRpcError::MissingParameter(opt_name.to_string()))?; - - v.as_str().and_then(|s| s.parse().ok()).ok_or_else(|| { - JsonRpcError::InvalidParameterType(format!("{opt_name} must be a valid hash")) - }) - } + if params.is_null() { + return Err(JsonRpcError::MissingParameter(field_name.to_string())); + } - /// Extracts an array of hashes from the request parameters at the specified index. - /// - /// This function can extract an array of any type that implements `FromStr`, such as - /// `BlockHash` or `Txid`. It checks if the parameter exists and is an array of valid string - /// representations of the type. Returns an error otherwise. - pub fn get_hashes_array( - params: &[Value], - index: usize, - opt_name: &str, - ) -> Result, JsonRpcError> { - let v = params - .get(index) - .ok_or_else(|| JsonRpcError::MissingParameter(opt_name.to_string()))?; + let v = match (params.is_array(), params.is_object()) { + (true, false) => params.get(index), + (false, true) => params.get(field_name), + _ => { + return Err(JsonRpcError::InvalidParameterStructure( + (*params).to_string(), + )); + } + }; - let array = v.as_array().ok_or_else(|| { - JsonRpcError::InvalidParameterType(format!("{opt_name} must be an array of hashes")) - })?; + let value = v.ok_or(JsonRpcError::MissingParameter(field_name.to_string()))?; - array - .iter() - .map(|v| { - v.as_str().and_then(|s| s.parse().ok()).ok_or_else(|| { - JsonRpcError::InvalidParameterType(format!("{opt_name} must be a valid hash")) - }) - }) - .collect() + T::deserialize(value) + .map_err(|e| JsonRpcError::InvalidParameterType(format!("{field_name}: {e}"))) } - /// Extracts an optional field from the request parameters at the specified index. - /// - /// This function checks if the parameter exists and is of the expected type. If the parameter - /// doesn't exist, it returns `None`. If it exists but is of an unexpected type, it returns an - /// error. - pub fn get_optional_field( - params: &[Value], - index: usize, - opt_name: &str, - extractor_fn: impl Fn(&[Value], usize, &str) -> Result, + /// Wraps a parameter extraction result so that a missing parameter yields `Ok(None)` + /// instead of an error. Other errors are propagated unchanged. + pub fn try_into_optional( + result: Result, ) -> Result, JsonRpcError> { - if params.len() <= index { - return Ok(None); + match result { + Ok(t) => Ok(Some(t)), + Err(JsonRpcError::MissingParameter(_)) => Ok(None), + Err(e) => Err(e), } + } - let value = extractor_fn(params, index, opt_name)?; - Ok(Some(value)) + /// Like [`get_at`], but returns `default` when the parameter is missing instead of + /// an error. Type mismatches are still propagated as errors. + pub fn get_with_default<'de, T: Deserialize<'de>>( + v: &'de Value, + index: usize, + field_name: &str, + default: T, + ) -> Result { + match get_at(v, index, field_name) { + Ok(t) => Ok(t), + Err(JsonRpcError::MissingParameter(_)) => Ok(default), + Err(e) => Err(e), + } } } diff --git a/crates/floresta-node/src/json_rpc/res.rs b/crates/floresta-node/src/json_rpc/res.rs index c23557f07..d369f0ceb 100644 --- a/crates/floresta-node/src/json_rpc/res.rs +++ b/crates/floresta-node/src/json_rpc/res.rs @@ -1,22 +1,511 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 -use core::fmt; +//! Response types for floresta's JSON-RPC server. +//! +//! This module is split into two main sections: +//! +//! - [`jsonrpc_interface`] — Protocol-level types that implement the +//! [`JSON-RPC 2.0 specification`]: the [`Response`] / +//! [`RpcError`] envelope, standard error code constants, and the [`JsonRpcError`] enum that +//! maps every floresta-specific failure into the appropriate JSON-RPC error code and HTTP +//! status. The server accepts both JSON-RPC 1.0 and 2.0 requests, but always responds +//! using the 2.0 format. +//! +//! - **Serialization structs** (outside the inner module) — Rust representations of the JSON +//! objects returned by individual RPC methods (`getblockchaininfo`, `getrawtransaction`, +//! `getblock`, etc.). These structs are `Serialize`/`Deserialize` and mirror the Bitcoin Core +//! JSON schema where applicable. +//! +//! [`JSON-RPC 2.0 specification`]: https://www.jsonrpc.org/specification +//! [`Response`]: jsonrpc_interface::Response +//! [`RpcError`]: jsonrpc_interface::RpcError +//! [`JsonRpcError`]: jsonrpc_interface::JsonRpcError + use core::fmt::Debug; -use core::fmt::Display; -use core::fmt::Formatter; -use core::num::TryFromIntError; -use std::convert::Infallible; -use axum::response::IntoResponse; use corepc_types::v30::GetBlockHeaderVerbose; use corepc_types::v30::GetBlockVerboseOne; -use floresta_chain::extensions::HeaderExtError; -use floresta_common::impl_error_from; -use floresta_mempool::mempool::MempoolError; -use floresta_watch_only::descriptor::DescriptorError; use serde::Deserialize; use serde::Serialize; +/// Types and methods implementing the [JSON-RPC 2.0 spec](https://www.jsonrpc.org/specification), +/// tailored for floresta's RPC server. Requests using JSON-RPC 1.0 (or omitting the version +/// field) are also accepted, but responses always follow the 2.0 format. +pub mod jsonrpc_interface { + use core::fmt; + use core::num::TryFromIntError; + use std::convert::Infallible; + use std::fmt::Display; + use std::fmt::Formatter; + + use axum::http::StatusCode; + use floresta_chain::BlockchainError; + use floresta_chain::extensions::HeaderExtError; + use floresta_common::impl_error_from; + use floresta_mempool::mempool::MempoolError; + use floresta_watch_only::WatchOnlyError; + use serde::Deserialize; + use serde::Serialize; + use serde_json::Value; + + use crate::json_rpc::server::SERIALIZATION_EXPECT_MSG; + + pub type RpcResult = std::result::Result; + + #[derive(Debug, Serialize)] + /// A JSON-RPC response object. + /// + /// Exactly one of `result` or `error` will be `Some`. + pub struct Response { + #[serde(flatten)] + /// Holds either a error os a success. + pub body: ResponseBody, + + /// Matches the `id` from the request. `Null` for notifications. + pub id: Value, + } + + impl Response { + /// Creates a successful JSON-RPC response with the given result. + pub fn success(result: Value, id: Value) -> Self { + Self { + body: ResponseBody::Success { result }, + id, + } + } + + /// Creates an error JSON-RPC response with the given error. + pub fn error(error: RpcError, id: Value) -> Self { + Self { + body: ResponseBody::Error { error }, + id, + } + } + + /// Converts a [RpcResult] into a success or error response. + pub fn from_result(result: RpcResult, id: Value) -> Self { + match result { + Ok(value) => Self::success(value, id), + Err(e) => Self::error(e.rpc_error(), id), + } + } + } + + #[derive(Debug, Serialize, Deserialize)] + #[serde(untagged)] + pub enum ResponseBody { + Success { result: Value }, + Error { error: RpcError }, + } + + #[derive(Debug, Deserialize, Serialize)] + /// A JSON-RPC error object. + pub struct RpcError { + /// Numeric error code indicating the type of error. + pub code: i16, + + /// Short description of the error. + pub message: String, + + /// Optional additional data about the error. + pub data: Option, + } + + impl Display for RpcError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!( + f, + "{}", + serde_json::to_string(self).expect(SERIALIZATION_EXPECT_MSG) + ) + } + } + + /// An invalid JSON was received by the server. + pub const PARSE_ERROR: i16 = -32700; + + /// The JSON sent is not a valid Request object. + pub const INVALID_REQUEST: i16 = -32600; + + /// The method does not exist or is not available. + pub const METHOD_NOT_FOUND: i16 = -32601; + + /// Invalid method parameter(s). + pub const INVALID_METHOD_PARAMETERS: i16 = -32602; + + /// Internal JSON-RPC error (infrastructure-level, not method-level). + pub const INTERNAL_ERROR: i16 = -32603; + + /// Lower bound of the implementation-defined server error range (`-32099..=-32000`). + /// + /// Floresta maps method-level errors to codes within this range. + pub const SERVER_ERROR_MIN: i16 = -32099; + + /// Upper bound of the implementation-defined server error range (`-32099..=-32000`). + /// + /// Floresta maps method-level errors to codes within this range. + pub const SERVER_ERROR_MAX: i16 = -32000; + + // Floresta-specific server error codes within the -32099..=-32000 range. + pub const TX_NOT_FOUND: i16 = SERVER_ERROR_MIN; // -32099 + pub const BLOCK_NOT_FOUND: i16 = SERVER_ERROR_MIN + 1; // -32098 + pub const PEER_NOT_FOUND: i16 = SERVER_ERROR_MIN + 2; // -32097 + pub const NO_ADDRESSES_TO_RESCAN: i16 = SERVER_ERROR_MIN + 3; // -32096 + pub const WALLET_ERROR: i16 = SERVER_ERROR_MIN + 4; // -32095 + pub const MEMPOOL_ERROR: i16 = SERVER_ERROR_MIN + 5; // -32094 + pub const IN_INITIAL_BLOCK_DOWNLOAD: i16 = SERVER_ERROR_MIN + 6; // -32093 + pub const NO_BLOCK_FILTERS: i16 = SERVER_ERROR_MIN + 7; // -32092 + pub const NODE_ERROR: i16 = SERVER_ERROR_MIN + 8; // -32091 + pub const CHAIN_ERROR: i16 = SERVER_ERROR_MIN + 9; // -32090 + pub const FILTERS_ERROR: i16 = SERVER_ERROR_MAX; // -32000 + + #[derive(Debug)] + pub enum JsonRpcError { + /// Rescan requested but the watch-only wallet has no addresses. + NoAddressesToRescan, + + /// Rescan requested with invalid values. + InvalidRescanVal, + + /// A required parameter is missing from the request. + MissingParameter(String), + + /// A parameter have an unexpected type (e.g. number where string was expected). + InvalidParameterType(String), + + /// A parameter is malformated, the parameter MUST be an array or an object + InvalidParameterStructure(String), + + /// The request contains a invalid jsonrpc version + InvalidJsonRpcVersion, + + /// Verbosity level received does not fit on available values. + InvalidVerbosityLevel, + + /// Transaction not found. + TxNotFound, + + /// The provided script is invalid. + InvalidScript, + + /// The provided descriptor is invalid. + InvalidDescriptor(miniscript::Error), + + /// Block not found in the blockchain. + BlockNotFound, + + /// Chain-level error (e.g. chain not synced or invalid). + Chain, + + /// The JSON-RPC request itself is malformed. + InvalidRequest, + + /// The requested RPC method does not exist. + MethodNotFound, + + /// Failed to decode the request payload. + Decode(String), + + /// The provided network address is invalid. + InvalidAddress, + + /// Node-level error (e.g. not connected or unresponsive). + Node(String), + + /// Block filters are not enabled, but the requested RPC requires them. + NoBlockFilters, + + /// The provided hex string is invalid. + InvalidHex, + + /// The node is still performing initial block download. + InInitialBlockDownload, + + /// Invalid mode passed to `getmemoryinfo`. + InvalidMemInfoMode, + + /// Wallet error (e.g. wallet not loaded or unavailable). + Wallet(String), + + /// Block filter error (e.g. filter data unavailable or corrupt). + Filters(String), + + /// Overflow when calculating cumulative chain work. + ChainWorkOverflow, + + /// Invalid `addnode` command or parameters. + InvalidAddnodeCommand, + + /// Invalid `disconnectnode` command (both address and node ID were provided). + InvalidDisconnectNodeCommand, + + /// Peer not found in the peer list. + PeerNotFound, + + /// Timestamp argument to `rescanblockchain` is before the genesis block + /// (and not zero, which is the default). + InvalidTimestamp, + + /// Transaction was rejected by the mempool. + MempoolAccept(MempoolError), + + /// A numeric conversion overflows, e.g., u64 to u32 + ConversionOverflow(String), + } + + impl_error_from!(JsonRpcError, MempoolError, MempoolAccept); + + impl JsonRpcError { + pub fn http_code(&self) -> StatusCode { + match self { + // 400 Bad Request - client sent invalid data + JsonRpcError::InvalidHex + | JsonRpcError::InvalidAddress + | JsonRpcError::InvalidScript + | JsonRpcError::InvalidRequest + | JsonRpcError::InvalidDescriptor(_) + | JsonRpcError::InvalidJsonRpcVersion + | JsonRpcError::InvalidVerbosityLevel + | JsonRpcError::Decode(_) + | JsonRpcError::MempoolAccept(_) + | JsonRpcError::InvalidMemInfoMode + | JsonRpcError::InvalidAddnodeCommand + | JsonRpcError::InvalidDisconnectNodeCommand + | JsonRpcError::InvalidTimestamp + | JsonRpcError::InvalidRescanVal + | JsonRpcError::NoAddressesToRescan + | JsonRpcError::InvalidParameterType(_) + | JsonRpcError::InvalidParameterStructure(_) + | JsonRpcError::MissingParameter(_) + | JsonRpcError::Wallet(_) => StatusCode::BAD_REQUEST, + + // 404 Not Found - resource/method doesn't exist + JsonRpcError::MethodNotFound + | JsonRpcError::BlockNotFound + | JsonRpcError::TxNotFound + | JsonRpcError::PeerNotFound => StatusCode::NOT_FOUND, + + // 500 Internal Server Error - server messed up + JsonRpcError::ChainWorkOverflow | JsonRpcError::ConversionOverflow(_) => { + StatusCode::INTERNAL_SERVER_ERROR + } + + // 503 Service Unavailable - server can't handle right now + JsonRpcError::InInitialBlockDownload + | JsonRpcError::NoBlockFilters + | JsonRpcError::Node(_) + | JsonRpcError::Chain + | JsonRpcError::Filters(_) => StatusCode::SERVICE_UNAVAILABLE, + } + } + + pub fn rpc_error(&self) -> RpcError { + match self { + // Parse error - invalid JSON received + JsonRpcError::Decode(msg) => RpcError { + code: PARSE_ERROR, + message: "Parse error".into(), + data: Some(Value::String(msg.clone())), + }, + + // Invalid request - not a valid JSON-RPC request + JsonRpcError::InvalidRequest => RpcError { + code: INVALID_REQUEST, + message: "Invalid request".into(), + data: None, + }, + + // Method not found + JsonRpcError::MethodNotFound => RpcError { + code: METHOD_NOT_FOUND, + message: "Method not found".into(), + data: None, + }, + + // Invalid params - invalid method parameters + JsonRpcError::InvalidHex => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid hex encoding".into(), + data: None, + }, + JsonRpcError::InvalidAddress => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid address".into(), + data: None, + }, + JsonRpcError::InvalidScript => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid script".into(), + data: None, + }, + JsonRpcError::InvalidDescriptor(e) => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid descriptor".into(), + data: Some(Value::String(e.to_string())), + }, + JsonRpcError::InvalidVerbosityLevel => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid verbosity level".into(), + data: None, + }, + JsonRpcError::InvalidTimestamp => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid timestamp".into(), + data: None, + }, + JsonRpcError::InvalidMemInfoMode => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid meminfo mode".into(), + data: None, + }, + JsonRpcError::InvalidAddnodeCommand => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid addnode command".into(), + data: None, + }, + JsonRpcError::InvalidDisconnectNodeCommand => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid disconnectnode command".into(), + data: None, + }, + JsonRpcError::InvalidRescanVal => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid rescan values".into(), + data: None, + }, + JsonRpcError::InvalidParameterType(param) => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Invalid parameter type".into(), + data: Some(Value::String(param.clone())), + }, + JsonRpcError::InvalidParameterStructure(param) => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: + "A parameter is malformated, the parameter MUST be an array or an object" + .into(), + data: Some(Value::String(param.clone())), + }, + JsonRpcError::InvalidJsonRpcVersion => RpcError { + code: INVALID_REQUEST, + message: "The request contains a invalid jsonrpc version".into(), + data: None, + }, + JsonRpcError::MissingParameter(param) => RpcError { + code: INVALID_METHOD_PARAMETERS, + message: "Missing parameter".into(), + data: Some(Value::String(param.clone())), + }, + + // Internal error + JsonRpcError::ChainWorkOverflow => RpcError { + code: INTERNAL_ERROR, + message: "Chain work overflow".into(), + data: None, + }, + JsonRpcError::ConversionOverflow(msg) => RpcError { + code: INTERNAL_ERROR, + message: "Numeric conversion overflow".into(), + data: Some(Value::String(msg.clone())), + }, + + // Server errors (implementation-defined: -32099..=-32000) + JsonRpcError::TxNotFound => RpcError { + code: TX_NOT_FOUND, + message: "Transaction not found".into(), + data: None, + }, + JsonRpcError::BlockNotFound => RpcError { + code: BLOCK_NOT_FOUND, + message: "Block not found".into(), + data: None, + }, + JsonRpcError::PeerNotFound => RpcError { + code: PEER_NOT_FOUND, + message: "Peer not found".into(), + data: None, + }, + JsonRpcError::NoAddressesToRescan => RpcError { + code: NO_ADDRESSES_TO_RESCAN, + message: "No addresses to rescan".into(), + data: None, + }, + JsonRpcError::Wallet(msg) => RpcError { + code: WALLET_ERROR, + message: "Wallet error".into(), + data: Some(Value::String(msg.clone())), + }, + JsonRpcError::MempoolAccept(msg) => RpcError { + code: MEMPOOL_ERROR, + message: "Mempool error".into(), + data: Some(Value::String(format!("{msg}"))), + }, + JsonRpcError::InInitialBlockDownload => RpcError { + code: IN_INITIAL_BLOCK_DOWNLOAD, + message: "Node is in initial block download".into(), + data: None, + }, + JsonRpcError::NoBlockFilters => RpcError { + code: NO_BLOCK_FILTERS, + message: "Block filters not available".into(), + data: None, + }, + JsonRpcError::Node(msg) => RpcError { + code: NODE_ERROR, + message: "Node error".into(), + data: Some(Value::String(msg.clone())), + }, + JsonRpcError::Chain => RpcError { + code: CHAIN_ERROR, + message: "Chain error".into(), + data: None, + }, + JsonRpcError::Filters(msg) => RpcError { + code: FILTERS_ERROR, + message: "Filters error".into(), + data: Some(Value::String(msg.clone())), + }, + } + } + } + + impl From for JsonRpcError { + fn from(value: HeaderExtError) -> Self { + match value { + HeaderExtError::Chain(_) => JsonRpcError::Chain, + HeaderExtError::BlockNotFound => JsonRpcError::BlockNotFound, + HeaderExtError::ChainWorkOverflow => JsonRpcError::ChainWorkOverflow, + } + } + } + + impl From for JsonRpcError { + fn from(e: TryFromIntError) -> Self { + JsonRpcError::ConversionOverflow(e.to_string()) + } + } + + impl From for JsonRpcError { + fn from(e: Infallible) -> Self { + JsonRpcError::ConversionOverflow(e.to_string()) + } + } + + impl_error_from!(JsonRpcError, miniscript::Error, InvalidDescriptor); + impl From> for JsonRpcError { + fn from(e: WatchOnlyError) -> Self { + JsonRpcError::Wallet(e.to_string()) + } + } + + impl From for JsonRpcError { + fn from(e: BlockchainError) -> Self { + match e { + BlockchainError::BlockNotPresent => JsonRpcError::BlockNotFound, + _ => JsonRpcError::Chain, + } + } + } +} #[derive(Deserialize, Serialize)] pub struct GetBlockchainInfoRes { pub best_block: String, @@ -109,7 +598,8 @@ pub struct ScriptPubKeyJson { pub req_sigs: u32, #[serde(rename = "type")] pub type_: String, - pub address: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub address: Option, } #[derive(Deserialize, Serialize)] @@ -146,207 +636,8 @@ pub enum GetBlockHeaderRes { Verbose(Box), } -#[derive(Debug, Deserialize, Serialize)] -pub struct RpcError { - pub code: i32, - pub message: String, - pub data: Option, -} - /// Return type for the `gettxoutproof` rpc command, the internal is /// just the hex representation of the Merkle Block, which was defined /// by btc core. #[derive(Debug, Deserialize, Serialize)] pub struct GetTxOutProof(pub Vec); - -#[derive(Debug)] -pub enum JsonRpcError { - /// There was a rescan request but we do not have any addresses in the watch-only wallet. - NoAddressesToRescan, - - /// There was a rescan request with invalid values - InvalidRescanVal, - - /// Missing parameter, e.g., if a required parameter is not provided in the request - MissingParameter(String), - - /// The provided parameter is of the wrong type, e.g., if a string is expected but a number is - /// provided - InvalidParameterType(String), - - /// Verbosity level is not 0 or 1 - InvalidVerbosityLevel, - - /// The requested transaction is not found in the blockchain - TxNotFound, - - /// The provided script is invalid, e.g., if it is not a valid P2PKH or P2SH script - InvalidScript, - - /// The provided descriptor is invalid, e.g., if it does not match the expected format - InvalidDescriptor(DescriptorError), - - /// The requested block is not found in the blockchain - BlockNotFound, - - /// There is an error with the chain, e.g., if the chain is not synced or when the chain is not valid - Chain, - - /// The request is invalid, e.g., some parameters use an incorrect type - InvalidRequest, - - /// The requested method is not found, e.g., if the method is not implemented or when the method is not available - MethodNotFound, - - /// This error is returned when there is an error decoding the request, e.g., if the request is not valid JSON - Decode(String), - - /// The provided address is invalid, e.g., when it is not a valid IP address or hostname - InvalidAddress, - - /// This error is returned when there is an error with the node, e.g., if the node is not connected or when the node is not responding - Node(String), - - /// This error is returned when the node does not have block filters enabled, which is required for some RPC calls - NoBlockFilters, - - /// This error is returned when a hex value is invalid - InvalidHex, - - /// This error is returned when the node is in initial block download, which means it is still syncing the blockchain - InInitialBlockDownload, - - InvalidMemInfoMode, - - /// This error is returned when there is an error with the wallet, e.g., if the wallet is not loaded or when the wallet is not available - Wallet(String), - - /// This error is returned when there is an error with block filters, e.g., if the filters are not available or when there is an issue with the filter data - Filters(String), - - /// This error is returned when there is an error calculating the chain work - ChainWorkOverflow, - - /// This error is returned when the addnode command is invalid, e.g., if the command is not recognized or when the parameters are incorrect - InvalidAddnodeCommand, - - /// Invalid `disconnect` node command (both address and ID parameters are present). - InvalidDisconnectNodeCommand, - - /// Peer was not found in the peer list. - PeerNotFound, - - /// Raised if when the rescanblockchain command, with the timestamp flag activated, contains some timestamp thats less than the genesis one and not zero which is the default value for this arg. - InvalidTimestamp, - - /// Something went wrong when attempting to publish a transaction to mempool - MempoolAccept(MempoolError), - - /// A numeric conversion overflows, e.g., u64 to u32 - ConversionOverflow(String), -} - -impl_error_from!(JsonRpcError, MempoolError, MempoolAccept); - -impl Display for JsonRpcError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - match self { - JsonRpcError::InvalidTimestamp => write!( - f, - "Invalid timestamp, ensure that it is between the genesis and the tip." - ), - JsonRpcError::InvalidRescanVal => { - write!(f, "Your rescan request contains invalid values") - } - JsonRpcError::NoAddressesToRescan => { - write!(f, "You do not have any address to proceed with the rescan") - } - JsonRpcError::MissingParameter(opt) => write!(f, "Missing parameter: {opt}"), - JsonRpcError::InvalidParameterType(opt) => { - write!(f, "Invalid parameter type for: {opt}") - } - JsonRpcError::InvalidRequest => write!(f, "Invalid request"), - JsonRpcError::InvalidHex => write!(f, "Invalid hex"), - JsonRpcError::MethodNotFound => write!(f, "Method not found"), - JsonRpcError::Decode(e) => write!(f, "error decoding request: {e}"), - JsonRpcError::TxNotFound => write!(f, "Transaction not found"), - JsonRpcError::InvalidDescriptor(e) => write!(f, "Invalid descriptor: {e}"), - JsonRpcError::BlockNotFound => write!(f, "Block not found"), - JsonRpcError::Chain => write!(f, "Chain error"), - JsonRpcError::InvalidAddress => write!(f, "Invalid address"), - JsonRpcError::Node(e) => write!(f, "Node error: {e}"), - JsonRpcError::NoBlockFilters => write!( - f, - "You don't have block filters enabled, please start florestad without --no-cfilters to run this RPC" - ), - JsonRpcError::InInitialBlockDownload => write!( - f, - "Node is in initial block download, wait until it's finished" - ), - JsonRpcError::InvalidScript => write!(f, "Invalid script"), - JsonRpcError::InvalidVerbosityLevel => write!(f, "Invalid verbosity level"), - JsonRpcError::InvalidMemInfoMode => { - write!(f, "Invalid meminfo mode, should be stats or mallocinfo") - } - JsonRpcError::Wallet(e) => write!(f, "Wallet error: {e}"), - JsonRpcError::Filters(e) => write!(f, "Error with filters: {e}"), - JsonRpcError::ChainWorkOverflow => { - write!(f, "Overflow while calculating the chain work") - } - JsonRpcError::InvalidAddnodeCommand => write!(f, "Invalid addnode command"), - JsonRpcError::InvalidDisconnectNodeCommand => { - write!(f, "Invalid disconnectnode command") - } - JsonRpcError::PeerNotFound => write!(f, "Peer not found in the peer list"), - JsonRpcError::MempoolAccept(e) => { - write!(f, "Could not send transaction to mempool due to {e}") - } - JsonRpcError::ConversionOverflow(e) => write!(f, "Numeric conversion overflow: {e}"), - } - } -} - -impl IntoResponse for JsonRpcError { - fn into_response(self) -> axum::http::Response { - let body = serde_json::json!({ - "error": self.to_string(), - "result": serde_json::Value::Null, - "id": serde_json::Value::Null, - }); - axum::http::Response::builder() - .status(axum::http::StatusCode::BAD_REQUEST) - .header("Content-Type", "application/json") - .body(axum::body::Body::from(serde_json::to_vec(&body).unwrap())) - .unwrap() - } -} - -impl From for JsonRpcError { - fn from(value: HeaderExtError) -> Self { - match value { - HeaderExtError::Chain(_) => JsonRpcError::Chain, - HeaderExtError::BlockNotFound => JsonRpcError::BlockNotFound, - HeaderExtError::ChainWorkOverflow => JsonRpcError::ChainWorkOverflow, - } - } -} - -impl From for JsonRpcError { - fn from(e: TryFromIntError) -> Self { - JsonRpcError::ConversionOverflow(e.to_string()) - } -} - -impl From for JsonRpcError { - fn from(e: Infallible) -> Self { - JsonRpcError::ConversionOverflow(e.to_string()) - } -} - -impl_error_from!(JsonRpcError, DescriptorError, InvalidDescriptor); - -impl From> for JsonRpcError { - fn from(e: floresta_watch_only::WatchOnlyError) -> Self { - JsonRpcError::Wallet(e.to_string()) - } -} diff --git a/crates/floresta-node/src/json_rpc/server.rs b/crates/floresta-node/src/json_rpc/server.rs index 4b22fafb6..96b73b530 100644 --- a/crates/floresta-node/src/json_rpc/server.rs +++ b/crates/floresta-node/src/json_rpc/server.rs @@ -13,7 +13,7 @@ use axum::body::Body; use axum::body::Bytes; use axum::extract::State; use axum::http::Method; -use axum::http::Response; +use axum::http::Response as HttpResponse; use axum::http::StatusCode; use axum::routing::post; use bitcoin::Address; @@ -44,22 +44,26 @@ use tracing::debug; use tracing::error; use tracing::info; -use super::res::JsonRpcError; use super::res::RawTxJson; -use super::res::RpcError; use super::res::ScriptPubKeyJson; use super::res::ScriptSigJson; use super::res::TxInJson; use super::res::TxOutJson; +use super::res::jsonrpc_interface::JsonRpcError; use crate::json_rpc::request::RpcRequest; -use crate::json_rpc::request::arg_parser::get_bool; -use crate::json_rpc::request::arg_parser::get_hash; -use crate::json_rpc::request::arg_parser::get_hashes_array; -use crate::json_rpc::request::arg_parser::get_numeric; -use crate::json_rpc::request::arg_parser::get_optional_field; -use crate::json_rpc::request::arg_parser::get_string; +use crate::json_rpc::request::arg_parser::get_at; +use crate::json_rpc::request::arg_parser::get_with_default; +use crate::json_rpc::request::arg_parser::try_into_optional; use crate::json_rpc::res::RescanConfidence; +use crate::json_rpc::res::jsonrpc_interface::Response; +/// Expect message for `serde_json` serialization of types that implement `Serialize`. +pub(super) const SERIALIZATION_EXPECT_MSG: &str = "types used in RPC responses implement Serialize"; + +/// Expect message for HTTP response builder with hardcoded valid headers. +pub(super) const HTTP_RESPONSE_EXPECT: &str = "HTTP response built from valid hardcoded headers"; + +/// The server holds this to tell which rpc method is awaiting to be processed and when the request were made. pub(super) struct InflightRpc { pub method: String, pub when: Instant, @@ -90,18 +94,23 @@ pub struct RpcImpl { type Result = std::result::Result; impl RpcImpl { - fn get_transaction(&self, tx_id: Txid, verbosity: Option) -> Result { - if verbosity == Some(true) { + fn get_transaction(&self, tx_id: Txid, verbosity: bool) -> Result { + if verbosity { let tx = self .wallet .get_transaction(&tx_id) - .ok_or(JsonRpcError::TxNotFound); - return tx.map(|tx| serde_json::to_value(self.make_raw_transaction(tx)).unwrap()); + .ok_or(JsonRpcError::TxNotFound)?; + let raw = self.make_raw_transaction(tx)?; + return Ok(serde_json::to_value(raw).expect(SERIALIZATION_EXPECT_MSG)); } self.wallet .get_transaction(&tx_id) - .and_then(|tx| serde_json::to_value(self.make_raw_transaction(tx)).ok()) + .and_then(|tx| { + self.make_raw_transaction(tx) + .ok() + .and_then(|v| serde_json::to_value(v).ok()) + }) .ok_or(JsonRpcError::TxNotFound) } @@ -112,11 +121,11 @@ impl RpcImpl { let addresses = self.wallet.get_cached_addresses(); let wallet = self.wallet.clone(); - if self.block_filter_storage.is_none() { - return Err(JsonRpcError::InInitialBlockDownload); - }; - - let cfilters = self.block_filter_storage.as_ref().unwrap().clone(); + let cfilters = self + .block_filter_storage + .as_ref() + .ok_or(JsonRpcError::NoBlockFilters)? + .clone(); let node = self.node.clone(); let chain = self.chain.clone(); @@ -129,10 +138,10 @@ impl RpcImpl { fn rescan_blockchain( &self, - start: Option, - stop: Option, + start: u32, + stop: u32, use_timestamp: bool, - confidence: Option, + confidence: RescanConfidence, ) -> Result { let (start_height, stop_height) = self.get_rescan_interval(use_timestamp, start, stop, confidence)?; @@ -155,11 +164,11 @@ impl RpcImpl { let wallet = self.wallet.clone(); - if self.block_filter_storage.is_none() { - return Err(JsonRpcError::NoBlockFilters); - }; - - let cfilters = self.block_filter_storage.as_ref().unwrap().clone(); + let cfilters = self + .block_filter_storage + .as_ref() + .ok_or(JsonRpcError::NoBlockFilters)? + .clone(); let node = self.node.clone(); @@ -193,7 +202,7 @@ impl RpcImpl { async fn handle_json_rpc_request( req: RpcRequest, state: Arc>, -) -> Result { +) -> Result { let RpcRequest { jsonrpc, method, @@ -203,7 +212,7 @@ async fn handle_json_rpc_request( if let Some(version) = jsonrpc { if !["1.0", "2.0"].contains(&version.as_str()) { - return Err(JsonRpcError::InvalidRequest); + return Err(JsonRpcError::InvalidJsonRpcVersion); } } @@ -215,18 +224,117 @@ async fn handle_json_rpc_request( }, ); + // Methods that don't require params match method.as_str() { - // blockchain "getbestblockhash" => { - let hash = state.get_best_block_hash()?; - Ok(serde_json::to_value(hash).unwrap()) + return state + .get_best_block_hash() + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)); + } + "getblockchaininfo" => { + return state + .get_blockchain_info() + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)); + } + "getblockcount" => { + return state + .get_block_count() + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)); + } + "getconnectioncount" => { + return state + .get_connection_count() + .await + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)); + } + "getnetworkinfo" => { + return state + .get_network_info() + .await + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)); + } + "getpeerinfo" => { + return state + .get_peer_info() + .await + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)); + } + "getroots" => { + return state + .get_roots() + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)); + } + "getrpcinfo" => { + return state + .get_rpc_info() + .await + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)); + } + "listdescriptors" => { + return state + .list_descriptors() + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)); + } + "ping" => { + state.ping().await?; + return Ok(serde_json::json!(null)); + } + "stop" => { + return state + .stop() + .await + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)); + } + "uptime" => { + return Ok(serde_json::to_value(state.uptime()).expect(SERIALIZATION_EXPECT_MSG)); + } + _ => {} + } + + // Methods that do require parameters. + // + // Here we use `unwrap_or_default()` because there are methods with only optional + // parameters. + // Therefore, even if the request is parsed and the `params` field was omitted it's nice + // to turn it into `Some(Value)` so the job of gathering inputs for calling the inner + // rpc method goes to the getters under request.rs. + let params = params.unwrap_or_default(); + + match method.as_str() { + "addnode" => { + let node = get_at(¶ms, 0, "node")?; + let command = get_at(¶ms, 1, "command")?; + let v2transport = get_with_default(¶ms, 2, "V2transport", false)?; + + state + .add_node(node, command, v2transport) + .await + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)) + } + + "disconnectnode" => { + let node_address = get_at(¶ms, 0, "node_address")?; + let node_id = try_into_optional(get_at(¶ms, 1, "node_id"))?; + + state + .disconnect_node(node_address, node_id) + .await + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)) + } + + "findtxout" => { + let txid = get_at(¶ms, 0, "txid")?; + let vout = get_at(¶ms, 1, "vout")?; + let script: String = get_at(¶ms, 2, "script")?; + let script = ScriptBuf::from_hex(&script).map_err(|_| JsonRpcError::InvalidScript)?; + let height = get_at(¶ms, 3, "height")?; + + state.clone().find_tx_out(txid, vout, script, height).await } "getblock" => { - let hash = get_hash(¶ms, 0, "block_hash")?; - // Default value in case of missing parameter is 1 - let verbosity: u8 = - get_optional_field(¶ms, 1, "verbosity", get_numeric)?.unwrap_or(1); + let hash = get_at(¶ms, 0, "block_hash")?; + let verbosity = get_with_default(¶ms, 1, "verbosity", 1)?; state .get_block(hash, verbosity) @@ -234,16 +342,8 @@ async fn handle_json_rpc_request( .map(|v| serde_json::to_value(v).expect("GetBlockRes implements serde")) } - "getblockchaininfo" => state - .get_blockchain_info() - .map(|v| serde_json::to_value(v).unwrap()), - - "getblockcount" => state - .get_block_count() - .map(|v| serde_json::to_value(v).unwrap()), - "getblockfrompeer" => { - let hash = get_hash(¶ms, 0, "block_hash")?; + let hash = get_at(¶ms, 0, "block_hash")?; state.get_block(hash, 0).await?; @@ -251,346 +351,146 @@ async fn handle_json_rpc_request( } "getblockhash" => { - let height = get_numeric(¶ms, 0, "block_height")?; + let height = get_at(¶ms, 0, "block_height")?; state .get_block_hash(height) - .map(|h| serde_json::to_value(h).unwrap()) + .map(|h| serde_json::to_value(h).expect(SERIALIZATION_EXPECT_MSG)) } "getblockheader" => { - let hash = get_hash(¶ms, 0, "block_hash")?; - let verbosity = get_optional_field(¶ms, 1, "verbosity", get_bool)?.unwrap_or(true); + let hash = get_at(¶ms, 0, "block_hash")?; + let verbosity = get_with_default(¶ms, 1, "verbosity", true)?; state .get_block_header(hash, verbosity) .await - .map(|h| serde_json::to_value(h).unwrap()) - } - - "getdeploymentinfo" => { - let blockhash = get_optional_field(¶ms, 0, "blockhash", get_hash)?; - state - .get_deployment_info(blockhash) - .map(|info| serde_json::to_value(info).unwrap()) + .map(|h| serde_json::to_value(h).expect(SERIALIZATION_EXPECT_MSG)) } - "getdifficulty" => state - .get_difficulty() - .map(|v| serde_json::to_value(v).unwrap()), - - "gettxout" => { - let txid = get_hash(¶ms, 0, "txid")?; - let vout = get_numeric(¶ms, 1, "vout")?; - let include_mempool = - get_optional_field(¶ms, 2, "include_mempool", get_bool)?.unwrap_or(false); + "getmemoryinfo" => { + let mode: String = get_with_default(¶ms, 0, "mode", "stats".into())?; state - .get_tx_out(txid, vout, include_mempool) - .map(|v| serde_json::to_value(v).unwrap()) - } - - "gettxoutproof" => { - let txids = get_hashes_array(¶ms, 0, "txids")?; - let block_hash = get_optional_field(¶ms, 1, "block_hash", get_hash)?; - - Ok(serde_json::to_value( - state - .get_txout_proof(&txids, block_hash) - .await? - .0 - .to_lower_hex_string(), - ) - .expect("GetTxOutProof implements serde")) + .get_memory_info(&mode) + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)) } "getrawtransaction" => { - let txid = get_hash(¶ms, 0, "txid")?; - let verbosity = get_optional_field(¶ms, 1, "verbosity", get_bool)?; + let txid = get_at(¶ms, 0, "txid")?; + let verbosity = get_with_default(¶ms, 1, "verbosity", false)?; state .get_transaction(txid, verbosity) - .map(|v| serde_json::to_value(v).unwrap()) + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)) } - "getroots" => state.get_roots().map(|v| serde_json::to_value(v).unwrap()), - - "findtxout" => { - let txid = get_hash(¶ms, 0, "txid")?; - let vout = get_numeric(¶ms, 1, "vout")?; - let script = get_string(¶ms, 2, "script")?; - let script = ScriptBuf::from_hex(&script).map_err(|_| JsonRpcError::InvalidScript)?; - let height = get_numeric(¶ms, 3, "height")?; - - let state = state.clone(); - state.find_tx_out(txid, vout, script, height).await - } - - // control - "getmemoryinfo" => { - let mode = - get_optional_field(¶ms, 0, "mode", get_string)?.unwrap_or("stats".into()); + "getdeploymentinfo" => { + let blockhash = try_into_optional(get_at(¶ms, 0, "blockhash"))?; state - .get_memory_info(&mode) - .map(|v| serde_json::to_value(v).unwrap()) - } - - "getrpcinfo" => state - .get_rpc_info() - .await - .map(|v| serde_json::to_value(v).unwrap()), - - // help - // logging - "stop" => state.stop().await.map(|v| serde_json::to_value(v).unwrap()), - - "uptime" => { - let uptime = state.uptime(); - Ok(serde_json::to_value(uptime).unwrap()) + .get_deployment_info(blockhash) + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)) } - // network - "getpeerinfo" => state - .get_peer_info() - .await - .map(|v| serde_json::to_value(v).unwrap()), - - "getconnectioncount" => state - .get_connection_count() - .await - .map(|v| serde_json::to_value(v).unwrap()), - - "getnetworkinfo" => state - .get_network_info() - .await - .map(|v| serde_json::to_value(v).unwrap()), - + "getdifficulty" => state + .get_difficulty() + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)), "getaddrmaninfo" => state .get_addrman_info() .await - .map(|v| serde_json::to_value(v).unwrap()), + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)), - "addnode" => { - let node = get_string(¶ms, 0, "node")?; - let command = get_string(¶ms, 1, "command")?; - let v2transport = - get_optional_field(¶ms, 2, "V2transport", get_bool)?.unwrap_or(false); + "gettxout" => { + let txid = get_at(¶ms, 0, "txid")?; + let vout = get_at(¶ms, 1, "vout")?; + let include_mempool = get_with_default(¶ms, 2, "include_mempool", false)?; state - .add_node(node, command, v2transport) - .await - .map(|v| serde_json::to_value(v).unwrap()) + .get_tx_out(txid, vout, include_mempool) + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)) } - "disconnectnode" => { - let node_address = get_string(¶ms, 0, "node_address")?; - let node_id = get_optional_field(¶ms, 1, "node_id", get_numeric)?; + "gettxoutproof" => { + let txids: Vec = get_at(¶ms, 0, "txids")?; + let block_hash = try_into_optional(get_at(¶ms, 1, "block_hash"))?; state - .disconnect_node(node_address, node_id) + .get_txout_proof(&txids, block_hash) .await - .map(|v| serde_json::to_value(v).unwrap()) - } - - "ping" => { - state.ping().await?; - - Ok(serde_json::json!(null)) + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)) } - // wallet "loaddescriptor" => { - let descriptor = get_string(¶ms, 0, "descriptor")?; + let descriptor = get_at(¶ms, 0, "descriptor")?; state .load_descriptor(descriptor) - .map(|v| serde_json::to_value(v).unwrap()) + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)) } "rescanblockchain" => { - let start_height = get_optional_field(¶ms, 0, "start_height", get_numeric)?; - let stop_height = get_optional_field(¶ms, 1, "stop_height", get_numeric)?; - let use_timestamp = - get_optional_field(¶ms, 2, "use_timestamp", get_bool)?.unwrap_or(false); - let confidence_str = get_optional_field(¶ms, 3, "confidence", get_string)? - .unwrap_or("medium".into()); - - let confidence = match confidence_str.as_str() { - "low" => RescanConfidence::Low, - "medium" => RescanConfidence::Medium, - "high" => RescanConfidence::High, - "exact" => RescanConfidence::Exact, - _ => return Err(JsonRpcError::InvalidRescanVal), - }; + let start_height = get_with_default(¶ms, 0, "start_height", 0)?; + let stop_height = get_with_default(¶ms, 1, "stop_height", 0)?; + let use_timestamp = get_with_default(¶ms, 2, "use_timestamp", false)?; + let confidence = get_with_default(¶ms, 3, "confidence", RescanConfidence::Medium)?; state - .rescan_blockchain(start_height, stop_height, use_timestamp, Some(confidence)) - .map(|v| serde_json::to_value(v).unwrap()) + .rescan_blockchain(start_height, stop_height, use_timestamp, confidence) + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)) } "sendrawtransaction" => { - let tx = get_string(¶ms, 0, "hex")?; + let tx = get_at(¶ms, 0, "hex")?; state .send_raw_transaction(tx) .await - .map(|v| serde_json::to_value(v).unwrap()) + .map(|v| serde_json::to_value(v).expect(SERIALIZATION_EXPECT_MSG)) } - "listdescriptors" => state - .list_descriptors() - .map(|v| serde_json::to_value(v).unwrap()), - - _ => { - let error = JsonRpcError::MethodNotFound; - Err(error) - } - } -} - -fn get_http_error_code(err: &JsonRpcError) -> u16 { - match err { - // you messed up - JsonRpcError::InvalidHex - | JsonRpcError::InvalidAddress - | JsonRpcError::InvalidScript - | JsonRpcError::InvalidRequest - | JsonRpcError::InvalidDescriptor(_) - | JsonRpcError::InvalidVerbosityLevel - | JsonRpcError::Decode(_) - | JsonRpcError::NoBlockFilters - | JsonRpcError::InvalidMemInfoMode - | JsonRpcError::InvalidAddnodeCommand - | JsonRpcError::InvalidDisconnectNodeCommand - | JsonRpcError::PeerNotFound - | JsonRpcError::InvalidTimestamp - | JsonRpcError::InvalidRescanVal - | JsonRpcError::NoAddressesToRescan - | JsonRpcError::InvalidParameterType(_) - | JsonRpcError::MissingParameter(_) - | JsonRpcError::ChainWorkOverflow - | JsonRpcError::ConversionOverflow(_) - | JsonRpcError::MempoolAccept(_) - | JsonRpcError::Wallet(_) => 400, - - // idunnolol - JsonRpcError::MethodNotFound | JsonRpcError::BlockNotFound | JsonRpcError::TxNotFound => { - 404 - } - - // we messed up, sowwy - JsonRpcError::InInitialBlockDownload - | JsonRpcError::Node(_) - | JsonRpcError::Chain - | JsonRpcError::Filters(_) => 503, - } -} - -fn get_json_rpc_error_code(err: &JsonRpcError) -> i32 { - match err { - // Parse Error - JsonRpcError::Decode(_) | JsonRpcError::InvalidParameterType(_) => -32700, - - // Invalid Request - JsonRpcError::InvalidHex - | JsonRpcError::MissingParameter(_) - | JsonRpcError::InvalidAddress - | JsonRpcError::InvalidScript - | JsonRpcError::MethodNotFound - | JsonRpcError::InvalidRequest - | JsonRpcError::InvalidDescriptor(_) - | JsonRpcError::InvalidVerbosityLevel - | JsonRpcError::TxNotFound - | JsonRpcError::BlockNotFound - | JsonRpcError::InvalidTimestamp - | JsonRpcError::InvalidMemInfoMode - | JsonRpcError::InvalidAddnodeCommand - | JsonRpcError::InvalidDisconnectNodeCommand - | JsonRpcError::PeerNotFound - | JsonRpcError::InvalidRescanVal - | JsonRpcError::NoAddressesToRescan - | JsonRpcError::ChainWorkOverflow - | JsonRpcError::ConversionOverflow(_) - | JsonRpcError::Wallet(_) - | JsonRpcError::MempoolAccept(_) => -32600, - - // server error - JsonRpcError::InInitialBlockDownload - | JsonRpcError::Node(_) - | JsonRpcError::Chain - | JsonRpcError::NoBlockFilters - | JsonRpcError::Filters(_) => -32603, + _ => Err(JsonRpcError::MethodNotFound), } } async fn json_rpc_request( State(state): State>>, body: Bytes, -) -> Response { - let req: RpcRequest = match serde_json::from_slice(&body) { - Ok(req) => req, - Err(e) => { - let error = RpcError { - code: -32700, - message: format!("Parse error: {e}"), - data: None, - }; - let body = json!({ - "error": error, - "id": Value::Null, - }); - return Response::builder() - .status(StatusCode::BAD_REQUEST) - .header("Content-Type", "application/json") - .body(Body::from(serde_json::to_vec(&body).unwrap())) - .unwrap(); - } +) -> HttpResponse { + let Ok(req): std::result::Result = serde_json::from_slice(&body) else { + let error = JsonRpcError::InvalidRequest; + let body = Response::error(error.rpc_error(), Value::Null); + return HttpResponse::builder() + .status(error.http_code()) + .header("Content-Type", "application/json") + .body(Body::from( + serde_json::to_vec(&body).expect(SERIALIZATION_EXPECT_MSG), + )) + .expect(HTTP_RESPONSE_EXPECT); }; debug!("Received JSON-RPC request: {req:?}"); let id = req.id.clone(); - let res = handle_json_rpc_request(req, state.clone()).await; + let method_res = handle_json_rpc_request(req, state.clone()).await; state.inflight.write().await.remove(&id); - match res { - Ok(res) => { - let body = serde_json::json!({ - "result": res, - "id": id, - }); - - axum::http::Response::builder() - .status(axum::http::StatusCode::OK) - .header("Content-Type", "application/json") - .body(axum::body::Body::from(serde_json::to_vec(&body).unwrap())) - .unwrap() - } - - Err(e) => { - let http_error_code = get_http_error_code(&e); - let json_rpc_error_code = get_json_rpc_error_code(&e); - let error = RpcError { - code: json_rpc_error_code, - message: e.to_string(), - data: None, - }; - - let body = serde_json::json!({ - "error": error, - "id": id, - }); - - axum::http::Response::builder() - .status(axum::http::StatusCode::from_u16(http_error_code).unwrap()) - .header("Content-Type", "application/json") - .body(axum::body::Body::from(serde_json::to_vec(&body).unwrap())) - .unwrap() - } - } + let response = HttpResponse::builder() + .status(match &method_res { + Err(e) => e.http_code(), + Ok(_) => StatusCode::OK, + }) + .header("Content-Type", "application/json"); + + let body = Response::from_result(method_res, id); + + response + .body(Body::from( + serde_json::to_vec(&body).expect(SERIALIZATION_EXPECT_MSG), + )) + .expect(HTTP_RESPONSE_EXPECT) } -async fn cannot_get(_state: State>>) -> Json { +async fn cannot_get(_state: State>>) -> Json { Json(json!({ "error": "Cannot get on this route", })) @@ -613,7 +513,7 @@ impl RpcImpl { stop_height, chain.clone(), ) - .unwrap(); + .map_err(|e| JsonRpcError::Filters(e.to_string()))?; info!("rescan filter hits: {blocks:?}"); @@ -621,8 +521,8 @@ impl RpcImpl { if let Ok(Some(block)) = node.get_block(block).await { let height = chain .get_block_height(&block.block_hash()) - .unwrap() - .unwrap(); + .map_err(|_| JsonRpcError::Chain)? + .ok_or(JsonRpcError::BlockNotFound)?; wallet.block_process(&block, height); } @@ -676,9 +576,12 @@ impl RpcImpl { asm: output.script_pubkey.to_asm_string(), hex: output.script_pubkey.to_hex_string(), req_sigs: 0, // This field is deprecated + // `Address::from_script` can fail for nonstandard scripts. Bitcoin Core + // omits the `address` field entirely when `ExtractDestination` fails: + // https://github.com/bitcoin/bitcoin/blob/f50d53c84736f8ada8419346c4d1734d5a6686d4/src/core_io.cpp#L424 address: Address::from_script(&output.script_pubkey, self.network) - .map(|a| a.to_string()) - .unwrap(), + .ok() + .map(|a| a.to_string()), type_: Self::get_script_type(output.script_pubkey) .unwrap_or("nonstandard") .to_string(), @@ -686,7 +589,7 @@ impl RpcImpl { } } - fn make_raw_transaction(&self, tx: CachedTransaction) -> RawTxJson { + fn make_raw_transaction(&self, tx: CachedTransaction) -> Result { let raw_tx = tx.tx; let in_active_chain = tx.height != 0; let hex = serialize_hex(&raw_tx); @@ -695,14 +598,14 @@ impl RpcImpl { .chain .get_block_hash(tx.height) .unwrap_or(BlockHash::all_zeros()); - let tip = self.chain.get_height().unwrap(); + let tip = self.chain.get_height().map_err(|_| JsonRpcError::Chain)?; let confirmations = if in_active_chain { tip - tx.height + 1 } else { 0 }; - RawTxJson { + Ok(RawTxJson { in_active_chain, hex, txid, @@ -735,7 +638,7 @@ impl RpcImpl { .get_block_header(&block_hash) .map(|h| h.time) .unwrap_or(0), - } + }) } // TODO(@luisschwab): get rid of this once @@ -766,7 +669,7 @@ impl RpcImpl { let address = address.unwrap_or_else(|| { format!("127.0.0.1:{}", Self::get_port(&network)) .parse() - .unwrap() + .expect("hardcoded address is valid") }); let listener = match tokio::net::TcpListener::bind(address).await { diff --git a/crates/floresta-node/src/lib.rs b/crates/floresta-node/src/lib.rs index fee7ba52d..9f27ac3cf 100644 --- a/crates/floresta-node/src/lib.rs +++ b/crates/floresta-node/src/lib.rs @@ -11,6 +11,7 @@ mod config_file; mod error; mod florestad; #[cfg(feature = "json-rpc")] +#[deny(clippy::unwrap_used)] mod json_rpc; #[cfg(feature = "zmq-server")] mod zmq; From 05a32f2a561e5ccf3c206a9535e63b39dd779a2a Mon Sep 17 00:00:00 2001 From: jaoleal Date: Wed, 13 May 2026 16:16:53 -0300 Subject: [PATCH 2/3] test(rpcserver): Extract test framework helpers and shared fixtures - Add JSON-RPC error code (JSONRPC_ERRCODE_*) and message (JSONRPC_ERRMSG_*) constants to test_framework/constants.py - Add make_raw_request, make_raw_data_request, make_request, assert_rpc_success, and assert_rpc_error to test_framework/rpc/base.py - Extract _create_logger helper to deduplicate fixture logging setup - Add class-scoped shared_florestad_node, shared_bitcoind_node, and shared_utreexod_node fixtures to conftest.py - Switch xdist to loadscope distribution for class-scoped fixture safety --- pyproject.toml | 3 +- tests/conftest.py | 89 ++++++++++++++++--- tests/test_framework/constants.py | 43 +++++++++ tests/test_framework/rpc/base.py | 140 +++++++++++++++++++++++++----- 4 files changed, 239 insertions(+), 36 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index aa55740f7..dfdaf45ef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,6 +32,7 @@ python_functions = ["test_*"] addopts = [ "-v", "-n 4", + "--dist=loadscope", "--strict-markers", "--strict-config", "--tb=short", @@ -52,4 +53,4 @@ minversion = "9.0" filterwarnings = [ "error::DeprecationWarning", "error::pytest.PytestCollectionWarning" -] \ No newline at end of file +] diff --git a/tests/conftest.py b/tests/conftest.py index f2ec1ea85..44a8b449b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,21 +9,21 @@ # pylint: disable=redefined-outer-name -import os import logging +import os import time from typing import List -import pytest +import pytest from test_framework import FlorestaTestFramework -from test_framework.node import Node, NodeType -from test_framework.util import Utility from test_framework.constants import ( FLORESTA_TEMP_DIR, WALLET_ADDRESS, WALLET_DESCRIPTOR_EXTERNAL, WALLET_DESCRIPTOR_INTERNAL, ) +from test_framework.node import Node, NodeType +from test_framework.util import Utility @pytest.fixture(scope="session", autouse=True) @@ -64,30 +64,38 @@ def pytest_runtest_makereport(item, call): setattr(item, f"rep_{rep.when}", rep) -@pytest.fixture(scope="function") -def setup_logging(request): - """ - Configure logging for the test, including the file and line number where the log was called. +def _create_logger(test_name): + """Create a logger with a file handler for the given test name. + + Shared helper used by both function-scoped and class-scoped logging + fixtures to avoid duplicating the setup logic. """ - test_name = request.node.name logger = logging.getLogger(test_name) - # Log format to include the file and line formatter = logging.Formatter( "%(asctime)s - %(levelname)s - %(pathname)s:%(lineno)d - %(message)s" ) - # Configure log file git_describe = Utility.get_git_describe() log_file = os.path.join(FLORESTA_TEMP_DIR, "logs", git_describe, f"{test_name}.log") os.makedirs(os.path.dirname(log_file), exist_ok=True) file_handler = logging.FileHandler(log_file, mode="w") file_handler.setFormatter(formatter) - # Add handlers to the logger if not logger.handlers: logger.addHandler(file_handler) + return logger, log_file + + +@pytest.fixture(scope="function") +def setup_logging(request): + """ + Configure logging for the test, including the file and line number where the log was called. + """ + test_name = request.node.name + logger, log_file = _create_logger(test_name) + yield logger # Capture test result and log it @@ -231,6 +239,63 @@ def _create_node(variant: NodeType) -> Node: return _create_node +@pytest.fixture(scope="class") +def shared_setup_logging(request): + """Class-scoped logging fixture for tests that share a single node.""" + test_name = request.node.name + logger, _log_file = _create_logger(test_name) + + yield logger + + if hasattr(request.node, "rep_call") and request.node.rep_call.failed: + logger.error("=" * 80) + logger.error("TEST FAILED: %s", test_name) + logger.error("=" * 80) + + logger.handlers.clear() + + +@pytest.fixture(scope="class") +def shared_node_manager(shared_setup_logging, request): + """Class-scoped node manager that lives for the entire test class.""" + manager = FlorestaTestFramework( + logger=shared_setup_logging, test_name=request.node.name + ) + yield manager + manager.stop() + + +@pytest.fixture(scope="class") +def shared_florestad_node(shared_node_manager) -> Node: + """Single florestad node shared across all methods in a test class.""" + node = shared_node_manager.add_node_default_args(variant=NodeType.FLORESTAD) + shared_node_manager.run_node(node) + return node + + +@pytest.fixture(scope="class") +def shared_bitcoind_node(shared_node_manager) -> Node: + """Single bitcoind node shared across all methods in a test class.""" + node = shared_node_manager.add_node_default_args(variant=NodeType.BITCOIND) + shared_node_manager.run_node(node) + return node + + +@pytest.fixture(scope="class") +def shared_utreexod_node(shared_node_manager) -> Node: + """Single utreexod node shared across all methods in a test class.""" + node = shared_node_manager.add_node_extra_args( + variant=NodeType.UTREEXOD, + extra_args=[ + f"--miningaddr={WALLET_ADDRESS}", + "--utreexoproofindex", + "--prune=0", + ], + ) + shared_node_manager.run_node(node) + return node + + @pytest.fixture def add_node_with_extra_args(node_manager): """ diff --git a/tests/test_framework/constants.py b/tests/test_framework/constants.py index f5f4d5c6b..af90ca0d1 100644 --- a/tests/test_framework/constants.py +++ b/tests/test_framework/constants.py @@ -28,3 +28,46 @@ # pylint: disable = line-too-long WALLET_XPUB_BIP_84 = "vpub5ZrpbMUWLCJ6MbpU1RzocWBddAQnk2XYry9JSXrtzxSqoicei28CzqUhiN2HJ8z2VjY6rsUNf4qxjym43ydhAFQJ7BDDcC2bK6et6x9hc4D" WALLET_ADDRESS = "bcrt1q427ze5mrzqupzyfmqsx9gxh7xav538yk2j4cft" + +# JSON-RPC spec error code constants +JSONRPC_ERRCODE_PARSE = -32700 +JSONRPC_ERRCODE_INVALID_REQUEST = -32600 +JSONRPC_ERRCODE_METHOD_NOT_FOUND = -32601 +JSONRPC_ERRCODE_INVALID_PARAMS = -32602 +JSONRPC_ERRCODE_INTERNAL = -32603 + +# JSON-RPC error message constants +JSONRPC_ERRMSG_MISSING_PARAMS = "Missing parameter" +JSONRPC_ERRMSG_WRONG_PARAM_TYPE = "Invalid parameter type" +JSONRPC_ERRMSG_METHOD_NOT_FOUND = "Method not found" +JSONRPC_ERRMSG_INVALID_VERSION = "The request contains a invalid jsonrpc version" +JSONRPC_ERRMSG_MALFORMATED_PARAMS = ( + "A parameter is malformated, the parameter MUST be an array or an object" +) + +# RPC method lists for testing +NO_PARAM_METHODS = [ + "getbestblockhash", + "getblockchaininfo", + "getblockcount", + "getroots", + "getrpcinfo", + "uptime", + "getpeerinfo", + "listdescriptors", +] + +METHODS_REQUIRING_PARAMS = [ + "getblock", + "getblockhash", + "getblockheader", + "getblockfrompeer", + "getrawtransaction", + "gettxout", + "gettxoutproof", + "findtxout", + "addnode", + "disconnectnode", + "loaddescriptor", + "sendrawtransaction", +] diff --git a/tests/test_framework/rpc/base.py b/tests/test_framework/rpc/base.py index a14486cce..221fd284b 100644 --- a/tests/test_framework/rpc/base.py +++ b/tests/test_framework/rpc/base.py @@ -101,28 +101,76 @@ def build_log_message( return logmsg - def build_request(self, method: str, params: List[Any]) -> Dict[str, Any]: + def _build_request_kwargs(self, timeout: int = None) -> Dict[str, Any]: """ - Build the request dictionary for the RPC call. + Build the common HTTP-level request kwargs (url, headers, auth, timeout). + Does NOT include the JSON-RPC payload body. """ - request = { - "url": f"{self.address}", + kwargs = { + "url": self.address, "headers": {"content-type": "application/json"}, - "data": json.dumps( - { - "jsonrpc": self._jsonrpc_version, - "id": "0", - "method": method, - "params": params, - } - ), - "timeout": self.TIMEOUT, + "timeout": timeout if timeout is not None else self.TIMEOUT, } if self._config.user is not None and self._config.password is not None: - request["auth"] = HTTPBasicAuth(self._config.user, self._config.password) + kwargs["auth"] = HTTPBasicAuth(self._config.user, self._config.password) + return kwargs + def build_request( + self, method: str, params: List[Any] = None, request_id: str = "test" + ) -> Dict[str, Any]: + """ + Build the full request dictionary for a JSON-RPC call. + """ + request = self._build_request_kwargs() + payload = { + "jsonrpc": self._jsonrpc_version, + "id": request_id, + "method": method, + } + if params is not None: + payload["params"] = params + request["data"] = json.dumps(payload) return request + def _send_request(self, request_kwargs: Dict[str, Any]) -> Dict[str, Any]: + """ + Execute an HTTP POST and return a normalized response dict: + {"status_code": int, "body": }. + """ + response = post(**request_kwargs) + return {"status_code": response.status_code, "body": response.json()} + + def noraise_request( + self, method: str, params: List[Any] = None, request_id: str = "test" + ) -> Dict[str, Any]: + """Send a standard JSON-RPC request and return the parsed response (no raise).""" + request = self.build_request(method, params, request_id) + return self._send_request(request) + + def noraise_raw_request( + self, payload: Dict[str, Any], content_type: str = "application/json" + ) -> Dict[str, Any]: + """ + Send an arbitrary dict as a JSON-RPC request body and return the raw + parsed response WITHOUT raising. + """ + request_kwargs = self._build_request_kwargs() + request_kwargs["headers"]["content-type"] = content_type + request_kwargs["data"] = json.dumps(payload) + return self._send_request(request_kwargs) + + def noraise_raw_data_request( + self, data, content_type: str = "application/json" + ) -> Dict[str, Any]: + """ + Send raw data (string/bytes) to the RPC endpoint and return the raw + parsed response WITHOUT raising. + """ + request_kwargs = self._build_request_kwargs() + request_kwargs["headers"]["content-type"] = content_type + request_kwargs["data"] = data + return self._send_request(request_kwargs) + # pylint: disable=unused-argument,dangerous-default-value def perform_request( self, @@ -138,21 +186,17 @@ def perform_request( The method will return the result of the request or raise a JSONRPCError if the request failed. """ - request = self.build_request(method, params) - - # Now make the POST request to the RPC server logmsg = BaseRPC.build_log_message( - request["url"], method, params, self._config.user, self._config.password + self.address, method, params, self._config.user, self._config.password ) - self.log.debug(self.log_msg(logmsg)) - response = post(**request) - # If response isnt 200, raise an HTTPError - if response.status_code != 200: + resp = self.noraise_request(method, params) + + if resp["status_code"] != 200: raise HTTPError - result = response.json() + result = resp["body"] # Error could be None or a str # If in the future this change, # cast the resulted error to str @@ -371,3 +415,53 @@ def get_addrman_info(self) -> dict: Get address manager statistics broken down by network """ return self.perform_request("getaddrmaninfo") + + +def make_raw_request(node, payload, content_type="application/json"): + """ + Send a raw JSON-RPC request and return the parsed response without raising on non-200. + """ + return node.rpc.noraise_raw_request(payload, content_type) + + +def make_raw_data_request(node, data, content_type="application/json"): + """ + Send raw data to the JSON-RPC endpoint and return the parsed response. + + Unlike ``make_raw_request`` this accepts a raw string/bytes body instead of + a dict, allowing tests to send malformed or non-JSON payloads. + """ + return node.rpc.noraise_raw_data_request(data, content_type) + + +def make_request(node, method, params=None, request_id="test"): + """ + Send a JSON-RPC request and return the parsed response without raising on non-200. + """ + return node.rpc.noraise_request(method, params, request_id) + + +def assert_rpc_success(resp): + """Assert that a JSON-RPC response indicates success (HTTP 200, no error).""" + assert resp["status_code"] == 200 + assert resp["body"].get("error") is None + + +def assert_rpc_error( + resp, expected_status_code=None, expected_rpcerror_code=None, expected_message=None +): + """ + Assert that a JSON-RPC response indicates an error (non-200, error present). + """ + assert resp["body"].get("error") is not None + + if expected_status_code is None: + assert resp["status_code"] != 200 + else: + assert resp["status_code"] == expected_status_code + + if expected_rpcerror_code is not None: + assert resp["body"]["error"]["code"] == expected_rpcerror_code + + if expected_message is not None: + assert resp["body"]["error"]["message"] == expected_message From b80de6ac9c88209c830e4620d4b290f3efd2e49a Mon Sep 17 00:00:00 2001 From: jaoleal Date: Wed, 13 May 2026 16:17:04 -0300 Subject: [PATCH 3/3] test(rpcserver): Integration tests for JSON-RPC request parsing - Add comprehensive tests covering positional and named parameters, null/omitted params, optional defaults, error codes and messages, and response structure validation - Add JSON-RPC 1.0 and 2.0 version acceptance tests (explicit version, omitted field, and response format verification) - Add content-type handling tests (application/json, text/plain, and non-JSON body rejection) --- tests/florestad/rpcserver_request_parsing.py | 301 +++++++++++++++++++ 1 file changed, 301 insertions(+) create mode 100644 tests/florestad/rpcserver_request_parsing.py diff --git a/tests/florestad/rpcserver_request_parsing.py b/tests/florestad/rpcserver_request_parsing.py new file mode 100644 index 000000000..b737f48aa --- /dev/null +++ b/tests/florestad/rpcserver_request_parsing.py @@ -0,0 +1,301 @@ +# SPDX-License-Identifier: MIT OR Apache-2.0 + +""" +Tests for JSON-RPC request parsing in florestad. + +Validates that the RPC server correctly handles: +- Positional (array) parameters +- Named (object) parameters +- Null / omitted parameters +- Default values for optional parameters +- Proper JSON-RPC error codes per the spec (-32700, -32600, -32601, -32602, -32603) +- HTTP status codes (400, 404, 500, 503) +- Methods that require no params vs methods that require params +- JSON-RPC 1.0 and 2.0 version acceptance +- Content-type handling +""" + +from test_framework.constants import ( + JSONRPC_ERRCODE_INVALID_PARAMS, + JSONRPC_ERRCODE_INVALID_REQUEST, + JSONRPC_ERRCODE_METHOD_NOT_FOUND, + JSONRPC_ERRMSG_INVALID_VERSION, + JSONRPC_ERRMSG_METHOD_NOT_FOUND, + JSONRPC_ERRMSG_MISSING_PARAMS, + JSONRPC_ERRMSG_WRONG_PARAM_TYPE, + METHODS_REQUIRING_PARAMS, + NO_PARAM_METHODS, +) +from test_framework.rpc.base import ( + assert_rpc_error, + assert_rpc_success, + make_raw_data_request, + make_raw_request, + make_request, +) + + +class TestRpcServerRequestParsing: + """ + Test JSON-RPC request parsing, parameter extraction (positional and named), + error codes, and edge cases on the florestad RPC server. + """ + + def test_noparammethods_omittedparams_succeeds(self, shared_florestad_node): + """Verify all no-param methods succeed when the params field is omitted.""" + for method in NO_PARAM_METHODS: + resp = make_request(shared_florestad_node, method) + assert_rpc_success(resp) + + def test_noparammethods_nullparams_succeeds(self, shared_florestad_node): + """Verify all no-param methods succeed when params is explicitly null.""" + for method in NO_PARAM_METHODS: + resp = make_request(shared_florestad_node, method, params=None) + assert_rpc_success(resp) + + def test_noparammethods_emptyarray_succeeds(self, shared_florestad_node): + """Verify all no-param methods succeed when params is an empty array.""" + for method in NO_PARAM_METHODS: + resp = make_request(shared_florestad_node, method, params=[]) + assert_rpc_success(resp) + + def test_positionalparams_validargs_succeeds(self, shared_florestad_node): + """Verify methods accept valid positional (array) parameters.""" + resp = make_request(shared_florestad_node, "getblockhash", params=[0]) + assert_rpc_success(resp) + genesis_hash = resp["body"]["result"] + resp = make_request( + shared_florestad_node, "getblockheader", params=[genesis_hash] + ) + assert_rpc_success(resp) + resp = make_request(shared_florestad_node, "getblock", params=[genesis_hash, 1]) + assert_rpc_success(resp) + + def test_namedparams_validargs_succeeds(self, shared_florestad_node): + """Verify methods accept valid named (object) parameters.""" + resp = make_request( + shared_florestad_node, "getblockhash", params={"block_height": 0} + ) + assert_rpc_success(resp) + genesis_hash = resp["body"]["result"] + resp = make_request( + shared_florestad_node, + "getblockheader", + params={"block_hash": genesis_hash}, + ) + assert_rpc_success(resp) + resp = make_request( + shared_florestad_node, + "getblock", + params={"block_hash": genesis_hash, "verbosity": 0}, + ) + assert_rpc_success(resp) + + def test_optionalparams_omitted_usesdefaults(self, shared_florestad_node): + """Verify omitted optional parameters fall back to their defaults.""" + genesis_hash = shared_florestad_node.rpc.get_bestblockhash() + resp_default = make_request( + shared_florestad_node, "getblock", params=[genesis_hash] + ) + assert_rpc_success(resp_default) + result = resp_default["body"]["result"] + # Check that the default verbosity was enabled. + assert "hash" in result + assert "tx" in result + + resp_explicit = make_request( + shared_florestad_node, "getblock", params=[genesis_hash, 1] + ) + assert_rpc_success(resp_explicit) + assert resp_default["body"]["result"] == resp_explicit["body"]["result"] + + resp = make_request( + shared_florestad_node, "getblock", params={"block_hash": genesis_hash} + ) + assert_rpc_success(resp) + assert resp_default["body"]["result"] == resp_explicit["body"]["result"] + assert "hash" in resp["body"]["result"] + + def test_unknownmethod_anyparams_returnsmethodnotfound(self, shared_florestad_node): + """Verify unknown methods return METHOD_NOT_FOUND (-32601).""" + resp = make_request(shared_florestad_node, "nonexistent_method", params=[]) + assert_rpc_error( + resp, + expected_status_code=404, + expected_rpcerror_code=JSONRPC_ERRCODE_METHOD_NOT_FOUND, + expected_message=JSONRPC_ERRMSG_METHOD_NOT_FOUND, + ) + + def test_requiredparams_missing_returnsinvalidparams(self, shared_florestad_node): + """Verify missing required parameters return INVALID_PARAMS (-32602).""" + resp = make_request(shared_florestad_node, "getblockhash", params=[]) + assert_rpc_error( + resp, + expected_status_code=400, + expected_rpcerror_code=JSONRPC_ERRCODE_INVALID_PARAMS, + expected_message=JSONRPC_ERRMSG_MISSING_PARAMS, + ) + + # {} is an empty object, so it should be accepted as an object + # but raise that is missing the fields + resp = make_request(shared_florestad_node, "getblockhash", params={}) + assert_rpc_error( + resp, + expected_status_code=400, + expected_rpcerror_code=JSONRPC_ERRCODE_INVALID_PARAMS, + expected_message=JSONRPC_ERRMSG_MISSING_PARAMS, + ) + + def test_paramtypes_wrongtype_returnsinvalidparams(self, shared_florestad_node): + """Verify wrong parameter types return INVALID_PARAMS (-32602).""" + + # getblockhash expects a number, but "not_a_number" is a string - params must be array + resp = make_request( + shared_florestad_node, "getblockhash", params=["not_a_number"] + ) + assert_rpc_error( + resp, + expected_status_code=400, + expected_rpcerror_code=JSONRPC_ERRCODE_INVALID_PARAMS, + expected_message=JSONRPC_ERRMSG_WRONG_PARAM_TYPE, + ) + + # getblock hash expects a string, but 12345 is a number - params must be array + resp = make_request(shared_florestad_node, "getblock", params=[12345]) + assert_rpc_error( + resp, + expected_status_code=400, + expected_rpcerror_code=JSONRPC_ERRCODE_INVALID_PARAMS, + expected_message=JSONRPC_ERRMSG_WRONG_PARAM_TYPE, + ) + + genesis_hash = shared_florestad_node.rpc.get_bestblockhash() + resp = make_request( + shared_florestad_node, + "getblock", + params=[genesis_hash, "invalid_verbosity"], + ) + + assert_rpc_error( + resp, + expected_status_code=400, + expected_rpcerror_code=JSONRPC_ERRCODE_INVALID_PARAMS, + expected_message=JSONRPC_ERRMSG_WRONG_PARAM_TYPE, + ) + + def test_jsonrpcversion_invalid_returnsrejection(self, shared_florestad_node): + """Verify invalid jsonrpc versions are rejected and valid ones accepted.""" + resp = make_raw_request( + shared_florestad_node, + {"jsonrpc": "3.0", "id": "test", "method": "getblockcount", "params": []}, + ) + + assert_rpc_error( + resp, + expected_status_code=400, + expected_rpcerror_code=JSONRPC_ERRCODE_INVALID_REQUEST, + expected_message=JSONRPC_ERRMSG_INVALID_VERSION, + ) + + for version in ["1.0", "2.0"]: + resp = make_raw_request( + shared_florestad_node, + { + "jsonrpc": version, + "id": "test", + "method": "getblockcount", + "params": [], + }, + ) + assert_rpc_success(resp) + + resp = make_raw_request( + shared_florestad_node, {"id": "test", "method": "getblockcount"} + ) + assert_rpc_success(resp) + + def test_parammethods_omittedparams_returnserror(self, shared_florestad_node): + """Verify methods that require params fail when params are omitted.""" + for method in METHODS_REQUIRING_PARAMS: + resp = make_request(shared_florestad_node, method) + assert_rpc_error( + resp, + expected_status_code=400, + expected_rpcerror_code=JSONRPC_ERRCODE_INVALID_PARAMS, + expected_message=JSONRPC_ERRMSG_MISSING_PARAMS, + ) + + def test_responsestructure_success_matchesjsonrpcspec(self, shared_florestad_node): + """Verify successful responses match the JSON-RPC spec structure.""" + resp = make_raw_request( + shared_florestad_node, + {"jsonrpc": "2.0", "id": "struct_test", "method": "getblockcount"}, + ) + body = resp["body"] + assert "result" in body + assert "id" in body + assert body["id"] == "struct_test" + assert body.get("result") is not None + + def test_responsestructure_error_matchesjsonrpcspec(self, shared_florestad_node): + """Verify error responses match the JSON-RPC spec structure.""" + resp = make_raw_request( + shared_florestad_node, + { + "jsonrpc": "2.0", + "id": "struct_err", + "method": "nonexistent", + "params": [], + }, + ) + body = resp["body"] + assert "error" in body + assert "id" in body + assert body["id"] == "struct_err" + err = body["error"] + assert "code" in err + assert "message" in err + assert isinstance(err["code"], int) + + def test_jsonrpc_v1_explicit_version_succeeds(self, shared_florestad_node): + """Verify requests with explicit jsonrpc 1.0 version succeed.""" + resp = make_raw_request( + shared_florestad_node, + {"jsonrpc": "1.0", "id": "test", "method": "getblockcount", "params": []}, + ) + assert_rpc_success(resp) + + def test_jsonrpc_v1_omitted_version_succeeds(self, shared_florestad_node): + """Verify requests without jsonrpc field succeed (JSON-RPC 1.0 style).""" + resp = make_raw_request( + shared_florestad_node, + {"id": "test", "method": "getblockcount"}, + ) + assert_rpc_success(resp) + + def test_contenttype_applicationjson_succeeds(self, shared_florestad_node): + """Verify requests with application/json content-type succeed.""" + resp = make_raw_request( + shared_florestad_node, + {"jsonrpc": "2.0", "id": "test", "method": "getblockcount"}, + content_type="application/json", + ) + assert_rpc_success(resp) + + def test_contenttype_textplain_succeeds(self, shared_florestad_node): + """Verify requests with text/plain content-type succeed.""" + resp = make_raw_request( + shared_florestad_node, + {"jsonrpc": "2.0", "id": "test", "method": "getblockcount"}, + content_type="text/plain", + ) + assert_rpc_success(resp) + + def test_contenttype_nonjson_body_rejected(self, shared_florestad_node): + """Verify non-JSON body is rejected regardless of content-type.""" + resp = make_raw_data_request( + shared_florestad_node, + data="this is not json", + content_type="text/plain", + ) + assert_rpc_error(resp, expected_status_code=400)