Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] recursion limit exceeded when deserialization #1156

Open
ZzPoLariszZ opened this issue Aug 19, 2024 · 7 comments
Open

[Bug] recursion limit exceeded when deserialization #1156

ZzPoLariszZ opened this issue Aug 19, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@ZzPoLariszZ
Copy link

ZzPoLariszZ commented Aug 19, 2024

Component

network, json-rpc

What version of Alloy are you on?

alloy v0.2.1

Operating System

Linux

Describe the bug

The function try_deserialize_ok will return a deserialization error when recursion limit exceeded.

Reproduce Example

Using debug_trace_block_by_number with CallTracer option to trace Block 15413811

Code

let block_number = 15413811;
let geth_trace_options = GethDebugTracingOptions::default().with_tracer(
    GethDebugTracerType::BuiltInTracer(GethDebugBuiltInTracerType::CallTracer),
);
let geth_trace_results = provider
    .debug_trace_block_by_number(block_number.into(), geth_trace_options)
    .await?;
println!("{}", geth_trace_results.len());

Result

Error: deserialization error: recursion limit exceeded at line 1 column 200590

Caused by:
    recursion limit exceeded at line 1 column 200590

Reason

The recursion limit is not large enough to handle call traces of these two transactions tx1 and tx2 inside.

let hash = b256!("aceacb7bbe075e5701638860e33de3dd60b480e4d0cfd7ce672ea0ab77412ba3");
let geth_trace_options = GethDebugTracingOptions::default().with_tracer(
    GethDebugTracerType::BuiltInTracer(GethDebugBuiltInTracerType::CallTracer),
);
let geth_trace_results = provider
    .debug_trace_transaction(hash, geth_trace_options)
    .await;
println!("{}", geth_trace_results.is_err());  // true
@ZzPoLariszZ ZzPoLariszZ added the bug Something isn't working label Aug 19, 2024
@ZzPoLariszZ
Copy link
Author

ZzPoLariszZ commented Aug 19, 2024

These might be related and helpful: ethereum/go-ethereum#22857 and serde-rs/json#509

@DaniPopes
Copy link
Member

If you want to deserialize that kind of heavily nested structure, enable the serde_json "disable_recursion_limit" feature yourself. This is not suitable to enable in Alloy itself.

@DaniPopes DaniPopes closed this as not planned Won't fix, can't repro, duplicate, stale Aug 19, 2024
@paberr
Copy link

paberr commented Aug 20, 2024

I ran into the same issue not too long ago. The problem with using the disable_recursion_limit feature of serde is that it needs to be enabled on the Deserializer itself, which is buried deep inside alloy (the try_deserialize_ok function that @ZzPoLariszZ has identified).

It would be good if alloy could somehow at least expose the option to the outside without the need of forking the project. After all this effectively currently disables the ability to work with certain transactions on the blockchain.

@DaniPopes
Copy link
Member

DaniPopes commented Aug 20, 2024

There is nothing that we need to expose, you have to add serde_json as a dependency and enable the feature.

@paberr
Copy link

paberr commented Aug 20, 2024

The unbounded_depth feature on serde_json only exposes the disable_recursion_limit function that needs to be called on the deserializer itself.

    let mut deserializer = serde_json::Deserializer::from_str(&json);
    deserializer.disable_recursion_limit();

Since the deserializer is instantiated inside the try_deserialize_ok, it is not possible to enable this right now.

@ZzPoLariszZ
Copy link
Author

The possible modification of try_deserialize_ok

/// Attempt to deserialize the `Ok(_)` variant of an [`RpcResult`].
pub fn try_deserialize_ok<'a, J, T, E, ErrResp>(
    result: RpcResult<J, E, ErrResp>,
) -> RpcResult<T, E, ErrResp>
where
    J: Borrow<RawValue> + 'a,
    T: RpcReturn,
    ErrResp: RpcReturn,
{
    let json = result?;
    let json = json.borrow().get();
    trace!(ty=%std::any::type_name::<T>(), json, "deserializing response");
    let mut deserializer = serde_json::Deserializer::from_str(&json);
    deserializer.disable_recursion_limit();
    let deserializer = serde_stacker::Deserializer::new(&mut deserializer);
    T::deserialize(deserializer)
        .inspect(|response| trace!(?response, "deserialized response"))
        .inspect_err(|err| trace!(?err, "failed to deserialize response"))
        .map_err(|err| RpcError::deser_err(err, json))
}

@DaniPopes
Copy link
Member

Ah I see, I thought it would be removed by enabling the feature alone

@DaniPopes DaniPopes reopened this Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants