chore(sidecar): JsonRpcResponse should be an enum

thedevbirb · thedevbirb · commit d94bb814ce7d · 2025-01-13T17:06:40.000+01:00
diff --git a/bolt-sidecar/src/api/commitments/firewall/processor.rs b/bolt-sidecar/src/api/commitments/firewall/processor.rs
@@ -2,6 +2,7 @@ use futures::{
     stream::{FuturesUnordered, SplitSink, SplitStream},
     FutureExt, SinkExt, StreamExt,
 };
+use serde::Serialize;
 use serde_json::{json, Value};
 use std::{collections::VecDeque, future::Future, pin::Pin, task::Poll};
 use tokio::{
@@ -30,7 +31,9 @@ use crate::{
     config::limits::LimitsOpts,
     primitives::{
         commitment::SignedCommitment,
-        jsonrpc::{JsonResponse, JsonRpcRequestUuid},
+        jsonrpc::{
+            JsonRpcErrorResponse, JsonRpcRequestUuid, JsonRpcResponse, JsonRpcSuccessResponse,
+        },
         misc::{Identified, IntoIdentified},
         CommitmentRequest, InclusionRequest,
     },
@@ -257,15 +260,14 @@ impl CommitmentRequestProcessor {
             return;
         };
 
-        let mut response =
-            JsonResponse { id: Some(Value::String(id.to_string())), ..Default::default() };
-
-        match result_commitment {
-            Ok(commitment) => response.result = json!(commitment),
+        let response: JsonRpcResponse = match result_commitment {
+            Ok(commitment) => JsonRpcSuccessResponse::new(json!(commitment))
+                .with_id(Value::String(id.to_string()))
+                .into(),
             Err(e) => {
-                response.error = Some(e.into());
+                JsonRpcErrorResponse::new(e.into()).with_id(Value::String(id.to_string())).into()
             }
-        }
+        };
 
         let message =
             Message::Text(serde_json::to_string(&response).expect("to stringify response"));
@@ -289,26 +291,27 @@ impl CommitmentRequestProcessor {
         };
 
         let id = request.id;
-        let mut response = JsonResponse {
-            id: Some(Value::String(id.to_string())),
-            jsonrpc: "2.0".to_string(),
-            ..Default::default()
-        };
 
         match request.method.as_str() {
             GET_VERSION_METHOD => {
-                response.result = Value::String(BOLT_SIDECAR_VERSION.clone());
+                let response =
+                    JsonRpcSuccessResponse::new(Value::String(BOLT_SIDECAR_VERSION.clone()))
+                        .with_uuid(id)
+                        .into();
                 self.send_response(response);
             }
             GET_METADATA_METHOD => {
-                response.result = serde_json::to_value(self.state.limits).expect("infallible");
+                let response =
+                    JsonRpcSuccessResponse::new(json!(self.state.limits)).with_uuid(id).into();
                 self.send_response(response);
             }
             REQUEST_INCLUSION_METHOD => {
                 let Some(param) = request.params.first().cloned() else {
-                    response.error = Some(
+                    let response: JsonRpcResponse = JsonRpcErrorResponse::new(
                         CommitmentError::InvalidParams("missing inclusion request".into()).into(),
-                    );
+                    )
+                    .with_uuid(id)
+                    .into();
                     self.send_response(response);
                     return;
                 };
@@ -318,7 +321,10 @@ impl CommitmentRequestProcessor {
                     Err(e) => {
                         let msg = format!("failed to parse inclusion request: {}", e);
                         error!(?e, "failed to parse inclusion request");
-                        response.error = Some(CommitmentError::InvalidParams(msg).into());
+                        let response: JsonRpcResponse =
+                            JsonRpcErrorResponse::new(CommitmentError::InvalidParams(msg).into())
+                                .with_uuid(id)
+                                .into();
                         self.send_response(response);
                         return;
                     }
@@ -330,7 +336,10 @@ impl CommitmentRequestProcessor {
 
                 if let Err(e) = self.api_events_tx.try_send(commitment_event) {
                     error!(?e, "failed to send commitment event through channel");
-                    response.error = Some(CommitmentError::Internal.into());
+                    let response: JsonRpcResponse =
+                        JsonRpcErrorResponse::new(CommitmentError::Internal.into())
+                            .with_uuid(id)
+                            .into();
                     self.send_response(response);
                     return;
                 }
@@ -344,7 +353,7 @@ impl CommitmentRequestProcessor {
         };
     }
 
-    fn send_response(&mut self, response: JsonResponse) {
+    fn send_response<T: Serialize>(&mut self, response: JsonRpcResponse<T>) {
         let message =
             Message::text(serde_json::to_string(&response).expect("to stringify response"));
         self.outgoing_messages.push_back(message);
diff --git a/bolt-sidecar/src/api/commitments/server/handlers.rs b/bolt-sidecar/src/api/commitments/server/handlers.rs
@@ -9,7 +9,7 @@ use axum::{
 };
 use axum_extra::extract::WithRejection;
 use serde::{Deserialize, Serialize};
-use serde_json::Value;
+use serde_json::json;
 use tracing::{debug, error, info, instrument};
 
 use crate::{
@@ -23,7 +23,7 @@ use crate::{
     common::BOLT_SIDECAR_VERSION,
     config::limits::LimitsOpts,
     primitives::{
-        jsonrpc::{JsonResponse, JsonRpcRequest},
+        jsonrpc::{JsonRpcRequest, JsonRpcResponse, JsonRpcSuccessResponse},
         signature::SignatureError,
         InclusionRequest,
     },
@@ -48,28 +48,31 @@ pub async fn rpc_entrypoint(
     headers: HeaderMap,
     State(api): State<Arc<CommitmentsApiInner>>,
     WithRejection(Json(payload), _): WithRejection<Json<JsonRpcRequest>, CommitmentError>,
-) -> Result<Json<JsonResponse>, CommitmentError> {
+) -> Result<Json<JsonRpcResponse>, CommitmentError> {
     debug!("Received new request");
 
     match payload.method.as_str() {
-        GET_VERSION_METHOD => Ok(Json(JsonResponse {
-            id: payload.id,
-            result: Value::String(BOLT_SIDECAR_VERSION.clone()),
-            ..Default::default()
-        })),
+        GET_VERSION_METHOD => Ok(Json(
+            JsonRpcSuccessResponse {
+                id: payload.id,
+                result: json!(BOLT_SIDECAR_VERSION.to_string()),
+                ..Default::default()
+            }
+            .into(),
+        )),
 
         GET_METADATA_METHOD => {
             let metadata = MetadataResponse {
                 limits: api.limits(),
                 version: BOLT_SIDECAR_VERSION.to_string(),
             };
 
-            let response = JsonResponse {
+            let response = JsonRpcSuccessResponse {
                 id: payload.id,
-                result: serde_json::to_value(metadata)
-                    .expect("infallible - metadata only contains primitive types"),
+                result: json!(metadata),
                 ..Default::default()
-            };
+            }
+            .into();
             Ok(Json(response))
         }
 
@@ -113,11 +116,12 @@ pub async fn rpc_entrypoint(
             let inclusion_commitment = api.request_inclusion(inclusion_request).await?;
 
             // Create the JSON-RPC response
-            let response = JsonResponse {
+            let response = JsonRpcSuccessResponse {
                 id: payload.id,
-                result: serde_json::to_value(inclusion_commitment).expect("infallible"),
+                result: json!(inclusion_commitment),
                 ..Default::default()
-            };
+            }
+            .into();
 
             Ok(Json(response))
         }
diff --git a/bolt-sidecar/src/api/commitments/server/mod.rs b/bolt-sidecar/src/api/commitments/server/mod.rs
@@ -179,14 +179,14 @@ fn make_router(state: Arc<CommitmentsApiInner>) -> Router {
 mod test {
     use crate::{
         api::commitments::spec::SIGNATURE_HEADER, common::BOLT_SIDECAR_VERSION,
-        primitives::jsonrpc::JsonError,
+        primitives::jsonrpc::JsonRpcError,
     };
     use alloy::signers::{k256::SecretKey, local::PrivateKeySigner};
     use handlers::MetadataResponse;
     use serde_json::json;
 
     use crate::{
-        primitives::{jsonrpc::JsonResponse, signature::ECDSASignatureExt},
+        primitives::{jsonrpc::JsonRpcResponse, signature::ECDSASignatureExt},
         test_util::{create_signed_inclusion_request, default_test_transaction},
     };
 
@@ -226,13 +226,13 @@ mod test {
             .send()
             .await
             .unwrap()
-            .json::<JsonResponse>()
+            .json::<JsonRpcResponse>()
             .await
             .unwrap();
 
         // Assert unauthorized because of missing signature
-        let expected_error: JsonError = CommitmentError::NoSignature.into();
-        assert_eq!(response.error.unwrap().code, expected_error.code);
+        let expected_error: JsonRpcError = CommitmentError::NoSignature.into();
+        assert_eq!(response.into_error().unwrap().code(), expected_error.code);
     }
 
     #[tokio::test]
@@ -276,10 +276,10 @@ mod test {
                 .await
                 .unwrap();
 
-            let json = response.json::<JsonResponse>().await.unwrap();
+            let json = response.json::<JsonRpcResponse>().await.unwrap();
 
             // Assert unauthorized because of missing signature
-            assert!(json.error.is_none());
+            assert!(json.into_success().is_some());
 
             let _ = tx.send(());
         });
@@ -320,11 +320,12 @@ mod test {
             .send()
             .await
             .unwrap()
-            .json::<JsonResponse>()
+            .json::<JsonRpcResponse>()
             .await
             .unwrap();
 
-        let metadata: MetadataResponse = serde_json::from_value(response.result).unwrap();
+        let metadata: MetadataResponse =
+            serde_json::from_value(response.into_success().unwrap().result).unwrap();
 
         assert_eq!(
             metadata.limits.max_committed_gas_per_slot,
diff --git a/bolt-sidecar/src/api/commitments/spec.rs b/bolt-sidecar/src/api/commitments/spec.rs
@@ -11,7 +11,7 @@ use thiserror::Error;
 use crate::{
     primitives::{
         commitment::InclusionCommitment,
-        jsonrpc::{JsonError, JsonResponse},
+        jsonrpc::{JsonRpcError, JsonRpcErrorResponse},
         signature::SignatureError,
         InclusionRequest,
     },
@@ -72,7 +72,7 @@ pub enum CommitmentError {
     RejectedJson(#[from] JsonRejection),
 }
 
-impl From<CommitmentError> for JsonError {
+impl From<CommitmentError> for JsonRpcError {
     fn from(err: CommitmentError) -> Self {
         // Reference: https://www.jsonrpc.org/specification#error_object
         // TODO: the custom defined ones should be clearly documented.
@@ -117,7 +117,8 @@ impl From<&CommitmentError> for StatusCode {
 impl IntoResponse for CommitmentError {
     fn into_response(self) -> Response<Body> {
         let status_code = StatusCode::from(&self);
-        let json = Json(JsonResponse::from_error(self.into()));
+        let err = JsonRpcError::from(self);
+        let json = Json(JsonRpcErrorResponse::new(err));
 
         (status_code, json).into_response()
     }
diff --git a/bolt-sidecar/src/primitives/jsonrpc.rs b/bolt-sidecar/src/primitives/jsonrpc.rs
