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

evm t8n log EOF header parse errors #26808

Closed
winsvega opened this issue Mar 3, 2023 · 15 comments
Closed

evm t8n log EOF header parse errors #26808

winsvega opened this issue Mar 3, 2023 · 15 comments

Comments

@winsvega
Copy link
Contributor

winsvega commented Mar 3, 2023

would be nice to see the reason EOF code was rejected in t8n log.
with this we can put expectations in the test to see that particular eof code was rejected for the reason we were testing.
maybe with --verbosity level flag

@ameya-deshmukh
Copy link

@winsvega would love to pick this up! Can you guide me through it maybe?

@winsvega
Copy link
Contributor Author

winsvega commented Mar 6, 2023

here we have the tests
ethereum/tests#1163

if you can install https://github.com/ethereum/retesteth develop branch
-t EOFTests
will trigger those tests with geth evm t8n installed in your PATH
you can debug where the EOF is processed by geth at geth EOF2 branch. the tests might fail as the specs can be outdated (geth or tests) but it does not matter as long as geth print the log.

@ameya-deshmukh
Copy link

got it, thanks! will get back to you after following this and replicating on my machine

@holiman
Copy link
Contributor

holiman commented Mar 8, 2023

I disagree with this. The idea is that evm t8n can be replaced by a different implementation. Therefore, we have pretty well defined inputs and outputs.
Adding the log outputs as part of the "specified outputs" seems hacky. The logs are not intended for machine-consumption, and they are messy to add tests for (internally, to verify behaviour).

After re-reading the description... If you want the reason there just to help the human operator check something from time to time, then sure, I agree, we could add that.

I interpret it as the latter. However, the eof code isn't merged to master, this needs can only be fixed in @lightclient 's fork/PR.

@holiman
Copy link
Contributor

holiman commented Mar 8, 2023

So, here's an example of how it works right now:

[user@work evm]$ go run . t8n --input.env=./testdata/26/env.json --input.alloc=./testdata/26/alloc.json --input.txs=./testdata/26/txs.json --output.result=stdout --state.fork=Shanghai
ERROR(3): code at 0xa94f5374fCe5edbc8E2A8697C15331677e6Ebf0C considered invalid: invalid section argument: arg 2, last 2, pos 2
exit status 3

The code already spits out the reason: invalid section argument: arg 2, last 2, pos 2. Is this not sufficient?

@lightclient
Copy link
Member

lightclient commented Mar 8, 2023

There is also the eofparser subcommand if you want the "standardized" error output. I think it is out of sync though, I recall Danno wanting to simplify the error numbers.

@winsvega
Copy link
Contributor Author

Let me check.
This erros we will check only when generating the test. Running the test rely on contract to be deployed in post state. If its not deployed then eof code gone wrong.

@winsvega
Copy link
Contributor Author

So, here's an example of how it works right now:

[user@work evm]$ go run . t8n --input.env=./testdata/26/env.json --input.alloc=./testdata/26/alloc.json --input.txs=./testdata/26/txs.json --output.result=stdout --state.fork=Shanghai
ERROR(3): code at 0xa94f5374fCe5edbc8E2A8697C15331677e6Ebf0C considered invalid: invalid section argument: arg 2, last 2, pos 2
exit status 3

The code already spits out the reason: invalid section argument: arg 2, last 2, pos 2. Is this not sufficient?

On which branch is that?

@holiman
Copy link
Contributor

holiman commented Mar 10, 2023

This one:

However, the eof code isn't merged to master, this needs can only be fixed in @lightclient 's fork/PR.

Meaning #26133

@winsvega
Copy link
Contributor Author

I see I was trying on Eof2

@holiman
Copy link
Contributor

holiman commented Mar 15, 2023

As far as I can tell, this is not a bug just some misunderstanding

@holiman holiman closed this as completed Mar 15, 2023
@winsvega
Copy link
Contributor Author

winsvega commented Apr 6, 2023

This one:

However, the eof code isn't merged to master, this needs can only be fixed in @lightclient 's fork/PR.

Meaning #26133

ah, I see this is only valid for code in state but not for transaction init code.
I don't get any errors from trying to execute transaction init code in EOF form

@winsvega
Copy link
Contributor Author

winsvega commented Apr 8, 2023

@winsvega
Copy link
Contributor Author

winsvega commented Apr 8, 2023

this is the log I get on my eof code
commit hash df23888 (lightclient/eof)

INFO [04-08|21:44:32.607] Trie dumping started                     root=994280..7be488
INFO [04-08|21:44:32.607] Trie dumping complete                    accounts=1 elapsed="148.301µs"
INFO [04-08|21:44:32.607] Wrote file                               file=/dev/shm/f64b03c9-ecd6-47d6-9368-e2e063991007/outAlloc.json
INFO [04-08|21:44:32.607] Wrote file                               file=/dev/shm/f64b03c9-ecd6-47d6-9368-e2e063991007/out.json

no vmtrace.
valid eof code works

trying to process tx
"0xf883f881800a83061a80808203e8b3ef0006010004020001000a030016000000000338600060003938601df3ef0001010004020001000303001d00000000013850001ca0554a436406c0ac2936b7611b200a24ad4085beaa63b9f06831731d7c6e830d9fa03a5e5e1cdf54570d8fc32640fc475ad76c9b8289792ceee88cdda7918e9a3715"

alloc

{
    "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b" : {
        "balance" : "0x0de0b6b3a7640000",
        "code" : "0x",
        "nonce" : "0x00",
        "storage" : {
        }
    }

@winsvega
Copy link
Contributor Author

Switched to evm eof command instead

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

No branches or pull requests

4 participants