Merge pull request #5793 from stacks-network/fix/4145

Fix: well-formed JSON error responses on /v2/fees/transaction

CHANGELOG.md

@@ -13,6 +13,8 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ### Fixed
 
+- Error responses to /v2/transactions/fees are once again expressed as JSON ([#4145](https://github.com/stacks-network/stacks-core/issues/4145)).
+
 ## [3.1.0.0.5]
 
 ### Added

stackslib/src/cost_estimates/tests/fee_rate_fuzzer.rs

@@ -15,7 +15,7 @@ use crate::cost_estimates::fee_rate_fuzzer::FeeRateFuzzer;
 use crate::cost_estimates::tests::common::make_block_receipt;
 use crate::cost_estimates::{EstimatorError, FeeEstimator, FeeRateEstimate};
 
-struct ConstantFeeEstimator {}
+pub struct ConstantFeeEstimator {}
 
 /// Returns a constant fee rate estimate.
 impl FeeEstimator for ConstantFeeEstimator {

stackslib/src/net/api/postfeerate.rs

@@ -18,6 +18,7 @@ use std::io::{Read, Write};
 
 use clarity::vm::costs::ExecutionCost;
 use regex::{Captures, Regex};
+use serde_json::json;
 use stacks_common::codec::{StacksMessageCodec, MAX_PAYLOAD_LEN};
 use stacks_common::types::chainstate::{
     BlockHeaderHash, ConsensusHash, StacksBlockId, StacksPublicKey,
@@ -118,13 +119,7 @@ impl RPCPostFeeRateRequestHandler {
         let scalar_cost =
             metric.from_cost_and_len(&estimated_cost, &stacks_epoch.block_limit, estimated_len);
         let fee_rates = fee_estimator.get_rate_estimates().map_err(|e| {
-            StacksHttpResponse::new_error(
-                preamble,
-                &HttpBadRequest::new(format!(
-                    "Estimator RPC endpoint failed to estimate fees for tx: {:?}",
-                    &e
-                )),
-            )
+            StacksHttpResponse::new_error(preamble, &HttpBadRequest::new_json(e.into_json()))
         })?;
 
         let mut estimations = RPCFeeEstimate::estimate_fees(scalar_cost, fee_rates).to_vec();
@@ -243,11 +238,7 @@ impl RPCRequestHandler for RPCPostFeeRateRequestHandler {
                         .map_err(|e| {
                             StacksHttpResponse::new_error(
                                 &preamble,
-                                &HttpBadRequest::new(format!(
-                                    "Estimator RPC endpoint failed to estimate tx {}: {:?}",
-                                    &tx.name(),
-                                    &e
-                                )),
+                                &HttpBadRequest::new_json(e.into_json()),
                             )
                         })?;
 
@@ -263,9 +254,9 @@ impl RPCRequestHandler for RPCPostFeeRateRequestHandler {
                     debug!("Fee and cost estimation not configured on this stacks node");
                     Err(StacksHttpResponse::new_error(
                         &preamble,
-                        &HttpBadRequest::new(
-                            "Fee estimation not supported on this node".to_string(),
-                        ),
+                        &HttpBadRequest::new_json(json!(
+                            "Fee estimation not supported on this node"
+                        )),
                     ))
                 }
             });

stackslib/src/net/api/tests/mod.rs

@@ -15,6 +15,7 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 use std::net::SocketAddr;
+use std::sync::{Arc, Mutex};
 
 use clarity::vm::costs::ExecutionCost;
 use clarity::vm::types::{QualifiedContractIdentifier, StacksAddressExtensions};
@@ -46,7 +47,7 @@ use crate::net::db::PeerDB;
 use crate::net::httpcore::{StacksHttpRequest, StacksHttpResponse};
 use crate::net::relay::Relayer;
 use crate::net::rpc::ConversationHttp;
-use crate::net::test::{TestEventObserver, TestPeer, TestPeerConfig};
+use crate::net::test::{RPCHandlerArgsType, TestEventObserver, TestPeer, TestPeerConfig};
 use crate::net::tests::inv::nakamoto::make_nakamoto_peers_from_invs_ext;
 use crate::net::{
     Attachment, AttachmentInstance, MemPoolEventDispatcher, RPCHandlerArgs, StackerDBConfig,
@@ -226,10 +227,28 @@ pub struct TestRPC<'a> {
 
 impl<'a> TestRPC<'a> {
     pub fn setup(test_name: &str) -> TestRPC<'a> {
-        Self::setup_ex(test_name, true)
+        Self::setup_ex(test_name, true, None, None)
     }
 
-    pub fn setup_ex(test_name: &str, process_microblock: bool) -> TestRPC<'a> {
+    pub fn setup_with_rpc_args(
+        test_name: &str,
+        rpc_handler_args_opt_1: Option<RPCHandlerArgsType>,
+        rpc_handler_args_opt_2: Option<RPCHandlerArgsType>,
+    ) -> TestRPC<'a> {
+        Self::setup_ex(
+            test_name,
+            true,
+            rpc_handler_args_opt_1,
+            rpc_handler_args_opt_2,
+        )
+    }
+
+    pub fn setup_ex(
+        test_name: &str,
+        process_microblock: bool,
+        rpc_handler_args_opt_1: Option<RPCHandlerArgsType>,
+        rpc_handler_args_opt_2: Option<RPCHandlerArgsType>,
+    ) -> TestRPC<'a> {
         // ST2DS4MSWSGJ3W9FBC6BVT0Y92S345HY8N3T6AV7R
         let privk1 = StacksPrivateKey::from_hex(
             "9f1f85a512a96a244e4c0d762788500687feb97481639572e3bffbd6860e6ab001",
@@ -317,6 +336,9 @@ impl<'a> TestRPC<'a> {
         let mut peer_1 = TestPeer::new(peer_1_config);
         let mut peer_2 = TestPeer::new(peer_2_config);
 
+        peer_1.rpc_handler_args = rpc_handler_args_opt_1;
+        peer_2.rpc_handler_args = rpc_handler_args_opt_2;
+
         // mine one block with a contract in it
         // first the coinbase
         // make a coinbase for this miner
@@ -976,7 +998,11 @@ impl<'a> TestRPC<'a> {
             }
 
             {
-                let mut rpc_args = RPCHandlerArgs::default();
+                let mut rpc_args = peer_1
+                    .rpc_handler_args
+                    .as_ref()
+                    .map(|args_type| args_type.instantiate())
+                    .unwrap_or(RPCHandlerArgsType::make_default());
                 rpc_args.event_observer = event_observer;
                 let mut node_state = StacksNodeState::new(
                     &mut peer_1.network,
@@ -1020,7 +1046,11 @@ impl<'a> TestRPC<'a> {
             }
 
             {
-                let mut rpc_args = RPCHandlerArgs::default();
+                let mut rpc_args = peer_2
+                    .rpc_handler_args
+                    .as_ref()
+                    .map(|args_type| args_type.instantiate())
+                    .unwrap_or(RPCHandlerArgsType::make_default());
                 rpc_args.event_observer = event_observer;
                 let mut node_state = StacksNodeState::new(
                     &mut peer_2.network,
@@ -1076,7 +1106,11 @@ impl<'a> TestRPC<'a> {
                     let mut peer_1_stacks_node = peer_1.stacks_node.take().unwrap();
                     let mut peer_1_mempool = peer_1.mempool.take().unwrap();
 
-                    let rpc_args = RPCHandlerArgs::default();
+                    let rpc_args = peer_1
+                        .rpc_handler_args
+                        .as_ref()
+                        .map(|args_type| args_type.instantiate())
+                        .unwrap_or(RPCHandlerArgsType::make_default());
                     let mut node_state = StacksNodeState::new(
                         &mut peer_1.network,
                         &peer_1_sortdb,

stackslib/src/net/api/tests/postfeerate.rs

@@ -15,23 +15,29 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 use std::net::{IpAddr, Ipv4Addr, SocketAddr};
+use std::sync::Arc;
 
 use clarity::vm::types::{PrincipalData, QualifiedContractIdentifier, StacksAddressExtensions};
 use clarity::vm::{ClarityName, ContractName, Value};
 use stacks_common::types::chainstate::StacksAddress;
 use stacks_common::types::net::PeerHost;
 use stacks_common::types::Address;
-use stacks_common::util::hash::to_hex;
+use stacks_common::util::hash::{to_hex, Sha256Sum};
 
 use super::test_rpc;
 use crate::chainstate::stacks::TransactionPayload;
 use crate::core::BLOCK_LIMIT_MAINNET_21;
+use crate::cost_estimates::metrics::UnitMetric;
+use crate::cost_estimates::tests::fee_rate_fuzzer::ConstantFeeEstimator;
+use crate::cost_estimates::UnitEstimator;
+use crate::net::api::tests::TestRPC;
 use crate::net::api::*;
 use crate::net::connection::ConnectionOptions;
 use crate::net::httpcore::{
     HttpRequestContentsExtensions, RPCRequestHandler, StacksHttp, StacksHttpRequest,
 };
-use crate::net::{ProtocolFamily, TipRequest};
+use crate::net::test::RPCHandlerArgsType;
+use crate::net::{ProtocolFamily, RPCHandlerArgs, TipRequest};
 
 #[test]
 fn test_try_parse_request() {
@@ -89,17 +95,50 @@ fn test_try_make_response() {
         TransactionPayload::new_contract_call(sender_addr, "hello-world", "add-unit", vec![])
             .unwrap();
 
+    // case 1: no fee estimates
     let mut requests = vec![];
     let request = StacksHttpRequest::new_post_fee_rate(
-        addr.into(),
+        addr.clone().into(),
+        postfeerate::FeeRateEstimateRequestBody {
+            estimated_len: Some(123),
+            transaction_payload: to_hex(&tx_payload.serialize_to_vec()),
+        },
+    );
+    requests.push(request);
+
+    let test_rpc = TestRPC::setup(function_name!());
+    let mut responses = test_rpc.run(requests);
+
+    let response = responses.remove(0);
+    debug!(
+        "Response:\n{}\n",
+        std::str::from_utf8(&response.try_serialize().unwrap()).unwrap()
+    );
+
+    let (preamble, body) = response.destruct();
+    let body_json: serde_json::Value = body.try_into().unwrap();
+
+    // get back a JSON string and a 400
+    assert_eq!(preamble.status_code, 400);
+    debug!("Response JSON no estimator: {}", &body_json);
+
+    // case 2: no estimate available
+    let mut requests = vec![];
+    let request = StacksHttpRequest::new_post_fee_rate(
+        addr.clone().into(),
         postfeerate::FeeRateEstimateRequestBody {
             estimated_len: Some(123),
             transaction_payload: to_hex(&tx_payload.serialize_to_vec()),
         },
     );
     requests.push(request);
 
-    let mut responses = test_rpc(function_name!(), requests);
+    let test_rpc = TestRPC::setup_with_rpc_args(
+        function_name!(),
+        Some(RPCHandlerArgsType::Null),
+        Some(RPCHandlerArgsType::Null),
+    );
+    let mut responses = test_rpc.run(requests);
 
     let response = responses.remove(0);
     debug!(
@@ -108,5 +147,46 @@ fn test_try_make_response() {
     );
 
     let (preamble, body) = response.destruct();
+    let body_json: serde_json::Value = body.try_into().unwrap();
+
+    // get back a JSON object and a 400
     assert_eq!(preamble.status_code, 400);
+    debug!("Response JSON no estimate fee: {}", &body_json);
+    assert_eq!(
+        body_json.get("reason").unwrap().as_str().unwrap(),
+        "NoEstimateAvailable"
+    );
+    assert!(body_json.get("error").is_some());
+    assert!(body_json.get("reason_data").is_some());
+
+    // case 3: get an estimate
+    let mut requests = vec![];
+    let request = StacksHttpRequest::new_post_fee_rate(
+        addr.clone().into(),
+        postfeerate::FeeRateEstimateRequestBody {
+            estimated_len: Some(123),
+            transaction_payload: to_hex(&tx_payload.serialize_to_vec()),
+        },
+    );
+    requests.push(request);
+
+    let test_rpc = TestRPC::setup_with_rpc_args(
+        function_name!(),
+        Some(RPCHandlerArgsType::Unit),
+        Some(RPCHandlerArgsType::Unit),
+    );
+    let mut responses = test_rpc.run(requests);
+
+    let response = responses.remove(0);
+    debug!(
+        "Response:\n{}\n",
+        std::str::from_utf8(&response.try_serialize().unwrap()).unwrap()
+    );
+
+    let (preamble, body) = response.destruct();
+    let body_json: serde_json::Value = body.try_into().unwrap();
+
+    // get back a JSON object and a 200
+    assert_eq!(preamble.status_code, 200);
+    debug!("Response JSON success: {}", &body_json);
 }

stackslib/src/net/api/tests/postmicroblock.rs

@@ -102,7 +102,7 @@ fn test_try_parse_request() {
 fn test_try_make_response() {
     let addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 33333);
 
-    let test_rpc = TestRPC::setup_ex(function_name!(), false);
+    let test_rpc = TestRPC::setup_ex(function_name!(), false, None, None);
     let mblock = test_rpc.next_microblock.clone().unwrap();
 
     let mut requests = vec![];