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] all *_many methods don't work on Reth #1074

Closed
0xlosh opened this issue Jul 18, 2024 · 5 comments · Fixed by #1278
Closed

[Bug] all *_many methods don't work on Reth #1074

0xlosh opened this issue Jul 18, 2024 · 5 comments · Fixed by #1278
Assignees
Labels
bug Something isn't working

Comments

@0xlosh
Copy link

0xlosh commented Jul 18, 2024

Component

rpc

What version of Alloy are you on?

0.1/0.2

Operating System

Linux

Describe the bug

Following methods do not work with a reth node

    async fn call_many: not implemented
    
    async fn debug_trace_call_many(
        &self,
        txs: Vec<TransactionRequest>,
        block: BlockNumberOrTag,
        trace_options: GethDebugTracingCallOptions,
    )
    
    async fn trace_call_many<'a>(
        &self,
        request: TraceCallList<'a, N>,
    )

reason being that reth accepts different types

    async fn call_many(
        &self,
        bundle: Bundle,
        state_context: Option<StateContext>,
        state_override: Option<StateOverride>,
    )
    
     pub async fn debug_trace_call_many(
        &self,
        bundles: Vec<Bundle>,
        state_context: Option<StateContext>,
        opts: Option<GethDebugTracingCallOptions>,
    ) 
    
     pub async fn trace_call_many(
        &self,
        calls: Vec<(TransactionRequest, HashSet<TraceType>)>,
        block_id: Option<BlockId>,
    )

instead of Vec<TransactionRequest>, reth expects a Vec<Bundle> (debug_trace_call_many)
instead of BlockNumberOrTag, reth expects a StateContext (debug_trace_call_many)
instead of Vec<TraceType>, reth expects a HashSet<TraceType> (trace_call_many)

@0xlosh 0xlosh added the bug Something isn't working label Jul 18, 2024
@mattsse
Copy link
Member

mattsse commented Jul 19, 2024

@zerosnacks mind looking into this, we likely have wrong arg order here

@zerosnacks zerosnacks self-assigned this Jul 19, 2024
@zerosnacks
Copy link
Member

Spotted a discrepancy, specifically in debug_trace_call_many where the bundles parameter doesn't skip over block_override, incompatible with Vec<TransactionRequest>.

/// Block overrides to apply
pub block_override: Option<BlockOverrides>,

txs: Vec<TransactionRequest>,

eth_callMany does not seem to be implemented in Alloy so that will have to be added in the eth_ namespace

trace_call_many implementation seems to be correct though it could possibly use BlockId instead of BlockNumberOrTag in the Alloy implementation depending on if BlockHash is supported by all clients

Testing effectively blocked until #1062 is implemented so I'm picking that up as a prerequisite

@zerosnacks zerosnacks changed the title [Bug] all _Many methods dont work on reth [Bug] all *_many methods don't work on Reth Jul 19, 2024
@0xlosh
Copy link
Author

0xlosh commented Jul 20, 2024

Spotted a discrepancy, specifically in debug_trace_call_many where the bundles parameter doesn't skip over block_override, incompatible with Vec<TransactionRequest>.

/// Block overrides to apply
pub block_override: Option<BlockOverrides>,

txs: Vec<TransactionRequest>,

eth_callMany does not seem to be implemented in Alloy so that will have to be added in the eth_ namespace

trace_call_many implementation seems to be correct though it could possibly use BlockId instead of BlockNumberOrTag in the Alloy implementation depending on if BlockHash is supported by all clients

Testing effectively blocked until #1062 is implemented so I'm picking that up as a prerequisite

trace_call_many is wrong too because alloy wants a Vec<TraceType> but reth wants a HashSet<TraceType> since pub type TraceCallList<'a, N> = &'a [(<N as Network>::TransactionRequest, Vec<TraceType>)]; i think.
I also edited my first post, should be clearer now hopefully :)

@zerosnacks
Copy link
Member

Now unblocked with #1062 merged

@zerosnacks
Copy link
Member

Fixed debug_traceCallMany and trace_callMany in #1278

#1274 will add eth_callMany as it requires some more consideration

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
3 participants