Inline logs

[aathan]

Motivation

It can be very useful to see both event emits and console.log() calls be logged/printed inline vs batched at the end of contract execution, particularly when working on forge or anvil itself. The current implementation relies on a call to print_logs() after the transaction has run and all logs have been gathered by LogCollector.

Solution

I am new to Rust, so this patch may suffer from that. I saw two ways to achieve the necessary features. (1) store an additional assigned member in the LogCollector inspector and check this member every time log() and call() are called. or (2) make LogCollector a generic on a type with a trait that has an fn on_log() which gets called every time log() and call() are called on the LogCollector.

I decided to experiment and see where the latter approach took me, because it also offered the possibility of easily extending the LogCollector and/or Executor/Inspector/InspectorStack behavior beyond just ONLOG. The change had a larger footprint than I anticipated, but the real benefit, beyond the --inline-logs command line parameter, is that the core foundry evm Inspector<> is now generic and the overall codebase is now ready to receive either more generic behavior traits or to have the OnLog trait be generalized into a trait with additional notification calls. Whether this is more useful than either creating a special Inspector, or InspectorStack, is something I'm not qualified to decide on.

I left the call to print_logs() intact, and currently the printing of emit events is just via their Debug rendering. Probably, if you like this change and plan to accept it, we should remove the print_logs() call and/or option it as a --batched-logs option and/or add the emit events to its output so it mirrors what --inline-logs does but batched at the end of the transaction.

EDIT: Incorporates #5498

EDIT: We may also want to option whether emit events are logged vs remaining silent. I think hardhat does not normally print those to the console.

[aathan]

Not sure what to do about these clippy warnings.

warning: unknown lint: `clippy::arc_with_non_send_sync`
   --> common/src/shell.rs:219:17
    |
219 |         #[allow(clippy::arc_with_non_send_sync)]
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unknown_lints)]` on by default

[Evalir]

hey @aathan would love to review this and get this in—could you fix conflicts? thank you for the contrib, slowly getting to them! 🙌

[aathan]

I think what I just submitted took care of the conflicts, but please review.
It's now back to being called logger vs log_collector at least in part because the LogCollector itself is renamed to EvmEventLogger<> in this PR ... yes, it collects logs, but its real function is more to "do bespoke things with events"

I am not going to argue over names of course. I just went with what had the smallest footprint given the already existing changes for EvmEventLogger. Please feel free to do a simple global replace to change the names to whatever you'd like.

[aathan]

@Evalir separately I want to get a conversation started to support polygon/bor in foundry and reth. Not sure how to engage in conversations with the paradigm and/or foundry teams on this and other points. Should I open a feature issue in both projects to discuss? I guess that would touch revm as well since bor changes some signature things.

[aathan]

I'm not sure what I'm doing wrong here but I've now spent 30+ minutes trying to resolve these conflicts by fetching from foundry's origin, merging master, and resolving all conflicts, then being told I'm behind my fork's inline_logs, merging that, ... several times over. I'm currently on "rebase" merge mode. I keep having to resolve all the same conflicts over and over.

I'm going to leave things as is because it's just not clear to me what's going on with all these rebases.

Should I just diff my branch vs master and apply that diff to a completely new PR based on the current origin/master to cleanup what might now be a mess? Every now and then git trips me up...

[Evalir]

Yep @aathan i think by now the rebase is just too big—let's reopen in a new branch so we can quickly review and merge?