From 719ef6251e2e8c36dc9ca94e1b137d65b136cc74 Mon Sep 17 00:00:00 2001 From: PiRK Date: Fri, 13 Dec 2024 15:29:28 +0100 Subject: [PATCH] [chronik] add macros to extract params from a JSONRPC request Summary: The `get_param!` and `get_optional_param!` macros will be reused a lot as more methods are added. Depends on D17314 Ref T3598 Test Plan: `ninja check-functional` Reviewers: #bitcoin_abc, Fabien, tobias_ruck Reviewed By: #bitcoin_abc, Fabien, tobias_ruck Subscribers: tobias_ruck, Fabien Maniphest Tasks: T3598 Differential Revision: https://reviews.bitcoinabc.org/D17347 --- chronik/chronik-http/src/electrum.rs | 83 +++++++++++-------- .../functional/chronik_electrum_blockchain.py | 54 ++++++++++++ 2 files changed, 104 insertions(+), 33 deletions(-) diff --git a/chronik/chronik-http/src/electrum.rs b/chronik/chronik-http/src/electrum.rs index 8026aa5bec..c783a77d97 100644 --- a/chronik/chronik-http/src/electrum.rs +++ b/chronik/chronik-http/src/electrum.rs @@ -280,41 +280,58 @@ impl RPCService for ChronikElectrumRPCBlockchainEndpoint { } } +/// Get a mandatory JSONRPC param by index or by name. +macro_rules! get_param { + ($params:expr, $index:expr, $key:expr) => {{ + match $params { + Value::Array(ref arr) => Ok(arr + .get($index) + .ok_or(RPCError::InvalidParams(concat!( + "Missing mandatory '", + $key, + "' parameter" + )))? + .clone()), + Value::Object(ref obj) => match obj.get($key) { + Some(value) => Ok(value.clone()), + None => Err(RPCError::InvalidParams(concat!( + "Missing mandatory '", + $key, + "' parameter" + ))), + }, + _ => Err(RPCError::InvalidParams( + "'params' must be an array or an object", + )), + } + }}; +} + +/// Get an optional JSONRPC param by index or by name, return the +/// provided default if the param not specified. +macro_rules! get_optional_param { + ($params:expr, $index:expr, $key:expr, $default:expr) => {{ + match $params { + Value::Array(ref arr) => match arr.get($index) { + Some(val) => Ok(val.clone()), + None => Ok($default), + }, + Value::Object(ref obj) => match obj.get($key) { + Some(value) => Ok(value.clone()), + None => Ok($default), + }, + _ => Err(RPCError::InvalidParams( + "'params' must be an array or an object", + )), + } + }}; +} + impl ChronikElectrumRPCBlockchainEndpoint { async fn transaction_get(&self, params: Value) -> Result { - let txid_hex: Value; - let verbose: Value; - match params { - Value::Array(arr) => { - txid_hex = arr - .first() - .ok_or(RPCError::InvalidParams( - "Missing mandatory 'txid' parameter", - ))? - .clone(); - verbose = match arr.get(1) { - Some(val) => val.clone(), - None => Value::Bool(false), - }; - } - Value::Object(obj) => { - txid_hex = match obj.get("txid") { - Some(txid) => Ok(txid.clone()), - None => Err(RPCError::InvalidParams( - "Missing mandatory 'txid' parameter", - )), - }?; - verbose = match obj.get("verbose") { - Some(verbose) => verbose.clone(), - None => Value::Bool(false), - }; - } - _ => { - return Err(RPCError::InvalidParams( - "'params' must be an array or an object", - )) - } - }; + let txid_hex = get_param!(params, 0, "txid")?; + let verbose = + get_optional_param!(params, 1, "verbose", Value::Bool(false))?; let txid_hex = match txid_hex { Value::String(s) => Ok(s), _ => Err(RPCError::InvalidParams( diff --git a/test/functional/chronik_electrum_blockchain.py b/test/functional/chronik_electrum_blockchain.py index d7ada20ffb..cebae38944 100644 --- a/test/functional/chronik_electrum_blockchain.py +++ b/test/functional/chronik_electrum_blockchain.py @@ -34,7 +34,61 @@ def skip_test_if_missing_module(self): def run_test(self): self.client = self.nodes[0].get_chronik_electrum_client() + self.test_invalid_params() + self.test_transaction_get() + def test_invalid_params(self): + # Invalid params type + for response in ( + self.client.synchronous_request("blockchain.transaction.get", params=None), + self.client.synchronous_request("blockchain.transaction.get", params=42), + ): + assert_equal( + response.error, + {"code": -32602, "message": "'params' must be an array or an object"}, + ) + + # Missing mandatory argument in otherwise valid params + for response in ( + self.client.synchronous_request("blockchain.transaction.get", params=[]), + self.client.synchronous_request("blockchain.transaction.get", params={}), + self.client.synchronous_request( + "blockchain.transaction.get", + params={"nottxid": 32 * "ff", "verbose": False}, + ), + self.client.blockchain.transaction.get(verbose=True), + ): + assert_equal( + response.error, + {"code": -32602, "message": "Missing mandatory 'txid' parameter"}, + ) + + # Non-string json type for txid + assert_equal( + self.client.blockchain.transaction.get(txid=int(32 * "ff", 16)).error, + {"code": -32602, "message": "'txid' must be a hexadecimal string"}, + ) + + for response in ( + # non-hex characters + self.client.blockchain.transaction.get("les sanglots longs"), + # odd number of hex chars + self.client.blockchain.transaction.get(GENESIS_CB_TXID[:-1]), + # valid hex but invalid length for a txid + self.client.blockchain.transaction.get(GENESIS_CB_TXID[:-2]), + ): + assert_equal( + response.error, + {"code": -32602, "message": "Failed to parse txid"}, + ) + + # Valid txid, but no such transaction was found + assert_equal( + self.client.blockchain.transaction.get(txid=32 * "ff").error, + {"code": -32600, "message": "Unknown transaction id"}, + ) + + def test_transaction_get(self): for response in ( self.client.blockchain.transaction.get(GENESIS_CB_TXID), self.client.blockchain.transaction.get(GENESIS_CB_TXID, False),