Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

Add Geth js tracer option #2088

Closed
wants to merge 2 commits into from
Closed

Conversation

zsluedem
Copy link
Contributor

@zsluedem zsluedem commented Jan 29, 2023

Motivation

In aa-bundler, we need debug_traceCall javascript tracer option. So we add this js tracer config in making the request.

And the CallTracer return data is not correct which needs to be fixed, too.

Solution

Add additional options.

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog
  • Breaking changes

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to add a CallTracer, similar to #2064
and modifies the debug_trace_call so it is generic and can be deserialized into a DeserializeOwned

I'd suggest we go with #2064 for the model changes and tackle the provider function in this PR?

Comment on lines +598 to +601
async fn debug_trace_call<
T: Into<TypedTransaction> + Send + Sync,
R: Serialize + DeserializeOwned + Debug + Send,
>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this because this would allow deserializing into any type.

what types are possible here?

perhaps we can add some safety trait restrictions here. For example
introduce a trait that is

trait CallTrace: DeserializeOwned + Debug + Send {}

impl CallTrace for /*All types we can return in `debug_trace_call`*/ {}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this because this would allow deserializing into any type.

what types are possible here?

Here are some tricky things with JS tracer. For JS tracer, it could return any type depending on the JS. Look at the example here

"{data: [], fault: function(log) {},step: function(log) { if(log.op.toString() == \"CALL\") this.data.push(log.stack.peek(0));},result: function() { return {\"result\":\"HelloWorld\"};}}".to_string()
.

Focus on the result: function() { return {\"result\":\"HelloWorld\"}; here. For this JS tracer, it would return something like

struct Custom{
  result: String
}

And you could define some other results like result: function() { return {\"a\":\"HelloWorld\", \"b\": 123}; then the return type would be

struct Custom{
  a: String,
  b: u64
}

perhaps we can add some safety trait restrictions here.

That's a good idea.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so adding another trait bound is likely unnecessary.

@zsluedem
Copy link
Contributor Author

zsluedem commented Jan 29, 2023

I'd suggest we go with #2064 for the model changes and tackle the provider function in this PR?

@mattsse Agree. Then I would just wait for #2064 to merge first.

@gakonst
Copy link
Owner

gakonst commented Jan 31, 2023

2064 is merged!

@zsluedem
Copy link
Contributor Author

zsluedem commented Feb 1, 2023

Closing this since #2064 is adopted

@zsluedem zsluedem closed this Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants