alloy-rs / alloy

Transports, Middleware, and Networks for the Alloy project
https://alloy.rs
Apache License 2.0
652 stars 234 forks source link

[Bug] recursion limit exceeded when deserialization #1156

Open ZzPoLariszZ opened 3 months ago

ZzPoLariszZ commented 3 months ago

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 commented 3 months ago

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

DaniPopes commented 3 months ago

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.

paberr commented 3 months ago

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 commented 3 months ago

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

paberr commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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