From e0ae00c6a059da4525dc46a63503b3879418fab3 Mon Sep 17 00:00:00 2001 From: Nadav Ivgi Date: Tue, 3 Dec 2024 04:32:25 +0200 Subject: [PATCH 1/4] Include full RPC error messages in HTTP replies Using the `From for HttpError` conversion, which uses the full Error `Display` instead of just its `description()`. Resolves #132. --- src/rest.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/rest.rs b/src/rest.rs index 43eab5c60..0d05a4334 100644 --- a/src/rest.rs +++ b/src/rest.rs @@ -987,9 +987,7 @@ fn handle_request( .ok_or_else(|| HttpError::from("Missing tx".to_string()))?, _ => return http_message(StatusCode::METHOD_NOT_ALLOWED, "Invalid method", 0), }; - let txid = query - .broadcast_raw(&txhex) - .map_err(|err| HttpError::from(err.description().to_string()))?; + let txid = query.broadcast_raw(&txhex)?; http_message(StatusCode::OK, txid.to_string(), 0) } From a66615f125a0574eef83a49378989f71598c9159 Mon Sep 17 00:00:00 2001 From: Nadav Ivgi Date: Tue, 3 Dec 2024 04:35:13 +0200 Subject: [PATCH 2/4] Improve RPC errors Display Example with new formatting: sendrawtransaction RPC error -22: TX decode failed. Make sure the tx has at least one input. Instead of: sendrawtransaction RPC error -22: {"code":-22,"message":"TX decode failed. Make sure the tx has at least one input."} --- src/daemon.rs | 5 ++++- src/errors.rs | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/daemon.rs b/src/daemon.rs index 680890792..3aa1f0f3c 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -88,10 +88,13 @@ fn parse_jsonrpc_reply(mut reply: Value, method: &str, expected_id: u64) -> Resu if let Some(err) = reply_obj.get_mut("error") { if !err.is_null() { if let Some(code) = parse_error_code(&err) { + let msg = err["message"] + .as_str() + .map_or_else(|| err.to_string(), |s| s.to_string()); match code { // RPC_IN_WARMUP -> retry by later reconnection -28 => bail!(ErrorKind::Connection(err.to_string())), - code => bail!(ErrorKind::RpcError(code, err.take(), method.to_string())), + code => bail!(ErrorKind::RpcError(code, msg, method.to_string())), } } } diff --git a/src/errors.rs b/src/errors.rs index c708d7dda..2c33f9fac 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -9,7 +9,7 @@ error_chain! { display("Connection error: {}", msg) } - RpcError(code: i64, error: serde_json::Value, method: String) { + RpcError(code: i64, error: String, method: String) { description("RPC error") display("{} RPC error {}: {}", method, code, error) } From ee588140c52ea866555cfe57048089aeb9e0b147 Mon Sep 17 00:00:00 2001 From: Nadav Ivgi Date: Thu, 12 Dec 2024 06:02:01 +0200 Subject: [PATCH 3/4] Add tests for POST /tx --- tests/rest.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/rest.rs b/tests/rest.rs index 7892fb6ee..96d6b67ff 100644 --- a/tests/rest.rs +++ b/tests/rest.rs @@ -168,6 +168,23 @@ fn test_rest() -> Result<()> { tester.mine()?; assert_eq!(get_json("/mempool")?["count"].as_u64(), Some(0)); + // Test POST /tx + let txid = tester.send(&addr1, "9.9 BTC".parse().unwrap())?; + let tx_hex = get_plain(&format!("/tx/{}/hex", txid))?; + // Re-send the tx created by send(). It'll be accepted again since its still in the mempool. + let broadcast1_resp = ureq::post(&format!("http://{}/tx", rest_addr)).send_string(&tx_hex)?; + assert_eq!(broadcast1_resp.status(), 200); + assert_eq!(broadcast1_resp.into_string()?, txid.to_string()); + // Mine the tx then submit it again. Should now fail. + tester.mine()?; + let broadcast2_res = ureq::post(&format!("http://{}/tx", rest_addr)).send_string(&tx_hex); + let broadcast2_resp = broadcast2_res.unwrap_err().into_response().unwrap(); + assert_eq!(broadcast2_resp.status(), 400); + assert_eq!( + broadcast2_resp.into_string()?, + "sendrawtransaction RPC error -27: Transaction already in block chain" + ); + // Elements-only tests #[cfg(feature = "liquid")] { From 4878e0a1f5a0028cf0a02a0e4a09584ff2a07040 Mon Sep 17 00:00:00 2001 From: Nadav Ivgi Date: Thu, 12 Dec 2024 06:47:44 +0200 Subject: [PATCH 4/4] Fix Elements dynafed tests They were broken by the test added in the last commit, which mined an extra block and caused the `GET /block/:hash` tests for Elements to run against a Compact params blocks instead of a Full one. This commit fixes it by running the tests against blocks 1 & 2 which are known to be Full & Compact respectively. --- tests/rest.rs | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/tests/rest.rs b/tests/rest.rs index 96d6b67ff..ce49feb78 100644 --- a/tests/rest.rs +++ b/tests/rest.rs @@ -313,22 +313,35 @@ fn test_rest() -> Result<()> { // Test GET /block/:hash { - let bestblockhash = get_plain("/blocks/tip/hash")?; - let block = get_json(&format!("/block/{}", bestblockhash))?; + let block1_hash = get_plain("/block-height/1")?; + let block1 = get_json(&format!("/block/{}", block1_hash))?; // No PoW-related stuff - assert!(block["bits"].is_null()); - assert!(block["nonce"].is_null()); - assert!(block["difficulty"].is_null()); + assert!(block1["bits"].is_null()); + assert!(block1["nonce"].is_null()); + assert!(block1["difficulty"].is_null()); // Dynamic Federations (dynafed) fields - assert!(block["ext"]["current"]["signblockscript"].is_string()); - assert!(block["ext"]["current"]["fedpegscript"].is_string()); - assert!(block["ext"]["current"]["fedpeg_program"].is_string()); - assert!(block["ext"]["current"]["signblock_witness_limit"].is_u64()); - assert!(block["ext"]["current"]["extension_space"].is_array()); - assert!(block["ext"]["proposed"].is_object()); - assert!(block["ext"]["signblock_witness"].is_array()); + // Block #1 should have the Full dynafed params + // See https://docs.rs/elements/latest/elements/dynafed/enum.Params.html + assert!(block1["ext"]["current"]["signblockscript"].is_string()); + assert!(block1["ext"]["current"]["fedpegscript"].is_string()); + assert!(block1["ext"]["current"]["fedpeg_program"].is_string()); + assert!(block1["ext"]["current"]["signblock_witness_limit"].is_u64()); + assert!(block1["ext"]["current"]["extension_space"].is_array()); + assert!(block1["ext"]["proposed"].is_object()); + assert!(block1["ext"]["signblock_witness"].is_array()); + + // Block #2 should have the Compact params + let block2_hash = get_plain("/block-height/2")?; + let block2 = get_json(&format!("/block/{}", block2_hash))?; + assert!(block2["ext"]["current"]["signblockscript"].is_string()); + assert!(block2["ext"]["current"]["signblock_witness_limit"].is_u64()); + // With the `elided_root` in place of `fedpegscript`/`fedpeg_program`/`extension_space`` + assert!(block2["ext"]["current"]["elided_root"].is_string()); + assert!(block2["ext"]["current"]["fedpegscript"].is_null()); + assert!(block2["ext"]["current"]["fedpeg_program"].is_null()); + assert!(block2["ext"]["current"]["extension_space"].is_null()); } }