fix(l1): support multiple rpc requests in a single request #2006
+40
−24
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
now serde can deserialize both into an intermediate enum then match if the value is a single rpcRequest or a vec of requests
|
mpaulucci
reviewed
Feb 19, 2025
| } | ||
| .unwrap() | ||
| } | ||
|
|
There was a problem hiding this comment.
can you add a unit test?
There was a problem hiding this comment.
Done in add unit test for RpcRequestWrapper.
mpaulucci
approved these changes
Feb 19, 2025
mpaulucci
reviewed
Feb 21, 2025
crates/networking/rpc/rpc.rs
Outdated
| @@ -576,4 +594,64 @@ mod tests { | |||
| let expected_response = to_rpc_response_success_value(&expected_response_string); | |||
| assert_eq!(response.to_string(), expected_response.to_string()); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn map_http_requests_single_item_then_array_json_test() { | |||
There was a problem hiding this comment.
you're not calling handle_http_request , which is the function you want to test. Let's better remove this test and merge without a test. Seems we need to create some test utils to easily test endpoints
There was a problem hiding this comment.
Done in remove useless test
mpaulucci
approved these changes
Feb 21, 2025
mpaulucci
changed the title
rodrigo-o
approved these changes
Feb 25, 2025
JereSalo
pushed a commit
that referenced
this pull request
Feb 28, 2025
**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
fkrause98
pushed a commit
that referenced
this pull request
Mar 5, 2025
**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
fkrause98
pushed a commit
that referenced
this pull request
Mar 5, 2025
**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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The hive tests from
engine-withdrawalswere failing with a panic because we were asuming only a single RLPRequest could be received but the test sent an array of requetsDescription
Introduce a wrapper struct that contains a
Singletype for singleRPCRequestandMultiplefor 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_responsewas also changed to returnserde_json::Valuebecause it was returning an incorrect value when using it to create the array of responses.This PR fixes the following tests from
engine-withdrawals:Advances #1586