tracers/prestate: always remove empty accounts from pre-state

[jsign]

I am creating this PR to continue my chat with @s1na about the prestate native tracer.

The current code only removes empty accounts from the prestate if they are created contracts. Any other added empty accounts are left for the prestate output.

A priori, this looked weird since I don't see a reason to keep any empty account in the returned pre-state. If the tracer always keeps empty accounts or always removes them, that sounds reasonable (although adding empty accounts doesn't add much value).

Adding empty accounts to pre still makes sense since pre is used for config.DiffMode==true, so the pre vs. post diff can be generated.

Even if removing all empty accounts after generating the (potentially) state diff sounds reasonable, this might be considered a breaking change, i.e., maybe some users assume non-created empty accounts appear in the output.

I am leaving this as a draft PR to continue the discussion here, as requested by @s1na.

[jsign]

This is just a potential variant that avoids to include all empty state accesses, since the client can assume if it isn't in the result then it is empty.

The variant that I would personally prefer is actually the opposite: always including empty accesses. Althought they are redundant as mentioned before, I like knowing those accesses actually happened. But this is more useful for some use cases that I want, and might be a bit unjustified for what the API tries to provide.

[s1na]

This is a good change thank you. Evidence is in the failing tests:

have: {"0x00000000000000000000000000000000deadbeef":{"balance":"0x0","code":"0x6001600052600160ff60016000f560ff6000a0"},"0x71562b71999873db5b286df957af199ec94617f7":{"balance":"0x1c6bf52634000"}}
want: {"0x0000000000000000000000000000000000000000":{"balance":"0x0"},"0x00000000000000000000000000000000deadbeef":{"balance":"0x0","code":"0x6001600052600160ff60016000f560ff6000a0"},"0x71562b71999873db5b286df957af199ec94617f7":{"balance":"0x1c6bf52634000"}}

We can see that previously we had "0x0000000000000000000000000000000000000000":{"balance":"0x0"} -> ugly don't want.

[s1na]

Ok noting that we are also emitting empty storage slots it seems. This is another inconsistency IMO.

The variant that I would personally prefer is actually the opposite: always including empty accesses. Althought they are redundant as mentioned before, I like knowing those accesses actually happened. But this is more useful for some use cases that I want, and might be a bit unjustified for what the API tries to provide.

This is what I'm leaning towards: clear all empty accounts and storage slots by default. Add option to include empties. Would that satisfy your use-case? Also: would you need only keys or also the values for empty accts?

[jsign]

This is what I'm leaning towards: clear all empty accounts and storage slots by default. Add option to include empties. Would that satisfy your use-case? Also: would you need only keys or also the values for empty accts?

Cool, that would be useful as an option (disabled by default). I think a single option including both empty storage slots and empty accounts access might be the best. Feels a bit weird somebody would like only one but not the other.

[jsign]

@s1na, would you like to work on top of this branch, or should I close the PR?

[s1na]

Happy to work on top of your branch.