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

Pretty printed logs and events in cast run #8207

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ioterw
Copy link

@ioterw ioterw commented Jun 19, 2024

Motivation

Recently I got quite big events in cast, so made them a bit more pretty printed, also fixed padding of topics in logs

Solution

Before:
image
After:
image

P.S. I don't remember any log bigger than 4 topics, so removed number 13 for string formating.

Copy link
Member

@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.

I don't have strong feelings about this, but I find this easier to parse now.
I only worry about output bloat.

wdyt @klkvr @mds1

@DaniPopes
Copy link
Member

DaniPopes commented Jun 19, 2024

If we accept this, we should also do this for calls too.

Please also update revm-inspector's implementation as for now they are separate, but we want to merge them in the future.

@mattsse
Copy link
Member

mattsse commented Jun 19, 2024

looking at the failing tests, I think we want to this for topics > 1

@ioterw
Copy link
Author

ioterw commented Jun 20, 2024

Added calls + fixed calls with no params
image

@ioterw ioterw closed this Jun 20, 2024
@ioterw ioterw reopened this Jun 20, 2024
Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

Please apply these changes in revm-inspectors and I will review there. I don't want to modify the code here

@zerosnacks zerosnacks added T-blocked Type: blocked and removed T-blocked Type: blocked labels Jun 20, 2024
@mds1
Copy link
Collaborator

mds1 commented Jun 20, 2024

Maybe it's just because I'm not used to this format, but I find it verbose and hard to skim traces for the call I am looking for now. Perhaps the broken lines are throwing me off
image

With long traces especially, terminal scrollback buffers will now cut off more of the trace

Either way this will just be different user's personal preference so ideally we move it behind a trace configuration flag, ref #3390

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants