Skip to content

Commit

Permalink
fix(l1): support multiple rpc requests in a single request (#2006)
Browse files Browse the repository at this point in the history
**Motivation**

The hive tests from `engine-withdrawals` were failing with a panic
because we were asuming only a single RLPRequest could be received but
the test sent an array of requets

**Description**

Introduce a wrapper struct that contains a `Single` type for single
`RPCRequest` and `Multiple` for an array of requests, so serde can
deserialize into the correct one.
If we have an array of request we execute them sequentially. Then return
a json encoded array of responses.
The function `rpc_response` was also changed to return
`serde_json::Value` because it was returning an incorrect value when
using it to create the array of responses.

This PR fixes the following tests from `engine-withdrawals`:

- "Withdrawals Fork on Block 1 - 1 Block Re-Org"
- "Withdrawals Fork on Block 1 - 8 Block Re-Org NewPayload"
- "Withdrawals Fork on Block 8 - 10 Block Re-Org NewPayload"
- "Withdrawals Fork on Canonical Block 8 / Side Block 7 - 10 Block
Re-Org"
- "Withdrawals Fork on Canonical Block 8 / Side Block 9 - 10 Block
Re-Org"

Advances #1586
  • Loading branch information
LeanSerra authored Feb 25, 2025
1 parent 2e9e67c commit 86b6bda
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 24 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci_l1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ jobs:
ethrex_flags: ""
- name: "Engine withdrawal tests"
simulation: ethereum/engine
test_pattern: "engine-withdrawals/engine-withdrawals test loader|GetPayloadV2 Block Value|Sync after 2 blocks - Withdrawals on Genesis|Max Initcode Size|Pre-Merge Fork Number > 0|Empty Withdrawals|Corrupted Block Hash Payload|Withdrawals Fork on Block 2|Withdrawals Fork on Block 3|GetPayloadBodies"
test_pattern: "engine-withdrawals/engine-withdrawals test loader|GetPayloadV2 Block Value|Sync after 2 blocks - Withdrawals on Genesis|Max Initcode Size|Pre-Merge Fork Number > 0|Empty Withdrawals|Corrupted Block Hash Payload|Withdrawals Fork on Block 2|Withdrawals Fork on Block 3|GetPayloadBodies|Withdrawals Fork on Block 1 - 1 Block Re-Org|Withdrawals Fork on Block 1 - 8 Block Re-Org NewPayload|Withdrawals Fork on Block 8 - 10 Block Re-Org NewPayload|Withdrawals Fork on Canonical Block 8 / Side Block 7 - 10 Block Re-Org [^S]|Withdrawals Fork on Canonical Block 8 / Side Block 9 - 10 Block Re-Org [^S]"
ethrex_flags: ""
- name: "Sync full"
simulation: ethereum/sync
Expand Down
62 changes: 39 additions & 23 deletions crates/networking/rpc/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use eth::{
},
};
use ethrex_p2p::{sync::SyncManager, types::NodeRecord};
use serde::Deserialize;
use serde_json::Value;
use std::{
collections::HashMap,
Expand Down Expand Up @@ -68,6 +69,13 @@ use axum::extract::State;
use ethrex_p2p::types::Node;
use ethrex_storage::{error::StoreError, Store};

#[derive(Deserialize)]
#[serde(untagged)]
enum RpcRequestWrapper {
Single(RpcRequest),
Multiple(Vec<RpcRequest>),
}

#[derive(Debug, Clone)]
pub struct RpcApiContext {
storage: Store,
Expand Down Expand Up @@ -214,9 +222,22 @@ pub async fn handle_http_request(
State(service_context): State<RpcApiContext>,
body: String,
) -> Json<Value> {
let req: RpcRequest = serde_json::from_str(&body).unwrap();
let res = map_http_requests(&req, service_context).await;
rpc_response(req.id, res)
let req = serde_json::from_str::<RpcRequestWrapper>(&body).unwrap();
let res = match req {
RpcRequestWrapper::Single(request) => {
let res = map_http_requests(&request, service_context).await;
rpc_response(request.id, res)
}
RpcRequestWrapper::Multiple(requests) => {
let mut responses = Vec::new();
for req in requests {
let res = map_http_requests(&req, service_context.clone()).await;
responses.push(rpc_response(req.id, res));
}
serde_json::to_value(responses).unwrap()
}
};
Json(res)
}

pub async fn handle_authrpc_request(
Expand All @@ -226,11 +247,11 @@ pub async fn handle_authrpc_request(
) -> Json<Value> {
let req: RpcRequest = serde_json::from_str(&body).unwrap();
match authenticate(&service_context.jwt_secret, auth_header) {
Err(error) => rpc_response(req.id, Err(error)),
Err(error) => Json(rpc_response(req.id, Err(error))),
Ok(()) => {
// Proceed with the request
let res = map_authrpc_requests(&req, service_context).await;
rpc_response(req.id, res)
Json(rpc_response(req.id, res))
}
}
}
Expand Down Expand Up @@ -398,28 +419,23 @@ pub fn map_net_requests(req: &RpcRequest, contex: RpcApiContext) -> Result<Value
}
}

fn rpc_response<E>(id: RpcRequestId, res: Result<Value, E>) -> Json<Value>
fn rpc_response<E>(id: RpcRequestId, res: Result<Value, E>) -> Value
where
E: Into<RpcErrorMetadata>,
{
match res {
Ok(result) => Json(
serde_json::to_value(RpcSuccessResponse {
id,
jsonrpc: "2.0".to_string(),
result,
})
.unwrap(),
),
Err(error) => Json(
serde_json::to_value(RpcErrorResponse {
id,
jsonrpc: "2.0".to_string(),
error: error.into(),
})
.unwrap(),
),
Ok(result) => serde_json::to_value(RpcSuccessResponse {
id,
jsonrpc: "2.0".to_string(),
result,
}),
Err(error) => serde_json::to_value(RpcErrorResponse {
id,
jsonrpc: "2.0".to_string(),
error: error.into(),
}),
}
.unwrap()
}

#[cfg(test)]
Expand Down Expand Up @@ -593,7 +609,7 @@ mod tests {
};
let result = map_http_requests(&request, context).await;
let response =
serde_json::from_value::<RpcSuccessResponse>(rpc_response(request.id, result).0)
serde_json::from_value::<RpcSuccessResponse>(rpc_response(request.id, result))
.expect("Request failed");
let expected_response_string = r#"{"jsonrpc":"2.0","id":1,"result":{"accessList":[{"address":"0x7dcd17433742f4c0ca53122ab541d0ba67fc27df","storageKeys":["0x0000000000000000000000000000000000000000000000000000000000000000","0x13a08e3cd39a1bc7bf9103f63f83273cced2beada9f723945176d6b983c65bd2"]}],"gasUsed":"0xca3c"}}"#;
let expected_response =
Expand Down

0 comments on commit 86b6bda

Please sign in to comment.