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

feat(tracing): log failed expectEmit events as error traces #8506

Open
topocount opened this issue Jul 23, 2024 · 5 comments · May be fixed by #8686
Open

feat(tracing): log failed expectEmit events as error traces #8506

topocount opened this issue Jul 23, 2024 · 5 comments · May be fixed by #8686
Assignees
Labels
A-cheatcodes Area: cheatcodes A-tracing Area: tracing C-forge Command: forge P-low Priority: low T-feature Type: feature T-post-V1 Area: to tackle after V1

Comments

@topocount
Copy link

Component

Forge

Describe the feature you would like

Currently, the tracer and error logger aren't very helpful when a matching event is not caught. I'd like to attempt to highlight the mismatched event(s) in red instead of white (or neutral) as they they are currently displayed by the tracer.

I'd like to contribute this small feature myself.

Additional context

This seems like a superficial improvement, but digging through large stack traces of emitted logs, trying to manually pick out the one that failed can be quite time consuming during development and integration testing.

@topocount topocount added the T-feature Type: feature label Jul 23, 2024
@topocount topocount changed the title logging failed expectRevert events as error traces logging failed expectEmit events as error traces Jul 23, 2024
@zerosnacks
Copy link
Member

zerosnacks commented Jul 24, 2024

Hi @topocount thanks for your suggestion, any stylistic changes to traces are to be applied in https://github.com/paradigmxyz/revm-inspectors/blob/main/src/tracing/writer.rs and changes to indicating whether something is an error is to be done in Foundry (it may be the case that it requires that).

@zerosnacks zerosnacks added A-tracing Area: tracing C-forge Command: forge labels Jul 24, 2024
@zerosnacks zerosnacks changed the title logging failed expectEmit events as error traces feat(tracing): log failed expectEmit events as error traces Jul 24, 2024
@zerosnacks zerosnacks added the A-cheatcodes Area: cheatcodes label Jul 24, 2024
@zerosnacks
Copy link
Member

Assigned this to you @topocount, please let me know if you need any pointers or have any questions along the way

@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@topocount
Copy link
Author

topocount commented Jul 26, 2024

Hey @zerosnacks I think I have a bead on an implementation path. Thanks for your guidance

please let me know if you need any pointers or have any questions along the way

don't you mean "borrows?" 😎

@topocount
Copy link
Author

topocount commented Aug 2, 2024

@zerosnacks is it okay if I create a new error in the cheatcodes package that that holds the abi-encoded bytestring of the unmatched event? I'd also like to capture the index of the event in the expected_events Deque in case multiple events with same exact parameters are expected, so something like:

error UnmatchedEvent(uint256 positionExpected, bytes rawLog);

@zerosnacks
Copy link
Member

I'm not sure if I understand it completely but it makes sense to me to pass the reverted error in a structured manner.

Perhaps using an error with two structs holding an Expected case and a Result case makes this format more generalizable. Feel free to open a draft PR, that way others can chime in more easily as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cheatcodes Area: cheatcodes A-tracing Area: tracing C-forge Command: forge P-low Priority: low T-feature Type: feature T-post-V1 Area: to tackle after V1
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

4 participants