Skip to content
This repository was archived by the owner on Nov 27, 2024. It is now read-only.

Conversation

@ZigaMr
Copy link

@ZigaMr ZigaMr commented May 10, 2022

What does this PR do?

Continuation of Supragya Raj's work. This PR adds support for geth nodes by introducing a translation layer between geth and parity traces. Makes use of debug_traceBlockByHash RPC call so make sure to enable debug mode when starting Geth "--http.api debug".

Related issue

#112

Testing

Tested with local node on host machine, used minikube instead of kind. Tested for use with .listener (./mev listener start geth).

Checklist before merging

  • Read the contributing guide
  • Installed and ran pre-commit hooks
  • All tests pass with ./mev test

Copy link
Contributor

@gheise gheise left a comment

Choose a reason for hiding this comment

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

Awesome work! Looking forward to your response, some suggestions above.



def get_base_provider(rpc: str, request_timeout: int = 500) -> Web3.AsyncHTTPProvider:
def get_base_provider(
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, rather than checking for the RPC type that's been passed as a variable, it would be beneficial to write a function somewhere that infers the RPC type, maybe from the result of a create_block attempt?

Copy link
Author

Choose a reason for hiding this comment

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

I agree, will commit new changes

Copy link

Choose a reason for hiding this comment

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

I believe there is a cleaner way to do this - namely the web3_clientVersion RPC Call

mev Outdated
;;
listener)
kubectl exec -ti deploy/mev-inspect -- ./listener $2
kubectl exec -ti deploy/mev-inspect -- ./listener $2 $3
Copy link
Contributor

Choose a reason for hiding this comment

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

Having to pass the RPC type to the user will become burdensome on every command

Copy link
Author

Choose a reason for hiding this comment

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

I agree, will commit new changes

async def inspect_block(
inspect_db_session: orm.Session,
w3: Web3,
type: RPCType,
Copy link
Contributor

Choose a reason for hiding this comment

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

rather than passing the rpc type as an argument, is it possible to retrieve a list of added middlewares from the w3 object itself, and determine the type from the presence/absence of the geth_poa_middleware?

Copy link
Author

Choose a reason for hiding this comment

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

I agree, will commit new changes

base_provider = AsyncHTTPProvider(rpc, request_kwargs={"timeout": request_timeout})
base_provider.middlewares += (http_retry_with_backoff_request_middleware,)
if type is RPCType.geth:
base_provider.middlewares += (
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the advantage of adding middlewares at the provider, rather than using Web3.middleware_onion.add or Web3(middlewares=[geth_middleware])?

Copy link
Author

Choose a reason for hiding this comment

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

What do you propose, to set middlewares at MevInspector constructor?
I'm not sure why it's set up like that, but for some reason when trying to set it up:

base_provider = AsyncHTTPProvider(rpc, request_kwargs={"timeout": request_timeout})
Web3(base_provider, modules={"eth": (AsyncEth,)}, middlewares=[geth_poa_middleware, http_retry_with_backoff_request_middleware,])

The function call
w3.provider.middlewares
returns empty set, so I'm assuming it has to be done at the provider.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting, not to picky about it. Were you able to get this working with a geth node and inspect any blocks?

Copy link
Author

Choose a reason for hiding this comment

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

Hey, yeah it works when enabling debug "--http.api debug" on geth. With a full node I was able to start the listener at few days old blocks (had to update latest_block_update before running listener).
Listener now infers rpc type from "trace_block" rpc call, but inspect and inspect-many still require the rpc_type parameter.

@arsenius-clbs
Copy link

So what happened to this PR?

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.

5 participants