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.

Sign up for GitHub

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

cmd/evm: don't reuse state between iterations, show errors #30780

Merged
merged 12 commits into from
Nov 26, 2024
+14 −9
Merged

cmd/evm: don't reuse state between iterations, show errors #30780

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
095b466
cmd/evm: don't reuse state between evm benchmark iterations. ensure …
jwasinger Nov 21, 2024
dc78fce
fix
jwasinger Nov 21, 2024
39a39ce
Update cmd/evm/runner.go
jwasinger Nov 21, 2024
5ef597e
prefer to panic instead of returning an error
jwasinger Nov 21, 2024
0cf16d8
fix
jwasinger Nov 21, 2024
1ba590b
remove todo comment
jwasinger Nov 22, 2024
0ac7ff0
ensure logs are printed from mutated state. not prestate.
jwasinger Nov 22, 2024
83d6b30
Update cmd/evm/runner.go
jwasinger Nov 22, 2024
d8a18e1
Update cmd/evm/runner.go
jwasinger Nov 22, 2024
c07b5fd
Update cmd/evm/runner.go
jwasinger Nov 22, 2024
6342d59
Update cmd/evm/runner.go
jwasinger Nov 22, 2024
99d915d
cmd/evm: init testing package
holiman Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions cmd/evm/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,20 @@ type execStats struct {

func timedExec(bench bool, execFunc func() ([]byte, uint64, error)) ([]byte, execStats, error) {
if bench {
testing.Init()
// Do one warm-up run
output, gasUsed, err := execFunc()
result := testing.Benchmark(func(b *testing.B) {
for i := 0; i < b.N; i++ {
haveOutput, haveGasUsed, haveErr := execFunc()
if !bytes.Equal(haveOutput, output) {
b.Fatalf("output differs, have\n%x\nwant%x\n", haveOutput, output)
panic(fmt.Sprintf("output differs\nhave %x\nwant %x\n", haveOutput, output))
}
if haveGasUsed != gasUsed {
b.Fatalf("gas differs, have %v want%v", haveGasUsed, gasUsed)
panic(fmt.Sprintf("gas differs, have %v want %v", haveGasUsed, gasUsed))
}
if haveErr != err {
b.Fatalf("err differs, have %v want%v", haveErr, err)
panic(fmt.Sprintf("err differs, have %v want %v", haveErr, err))
}
}
})
Expand Down Expand Up @@ -137,7 +138,7 @@ func runCmd(ctx *cli.Context) error {
var (
tracer *tracing.Hooks
debugLogger *logger.StructLogger
statedb *state.StateDB
prestate *state.StateDB
chainConfig *params.ChainConfig
sender = common.BytesToAddress([]byte("sender"))
receiver = common.BytesToAddress([]byte("receiver"))
Expand Down Expand Up @@ -174,7 +175,7 @@ func runCmd(ctx *cli.Context) error {
defer triedb.Close()
genesis := genesisConfig.MustCommit(db, triedb)
sdb := state.NewDatabase(triedb, nil)
statedb, _ = state.New(genesis.Root(), sdb)
prestate, _ = state.New(genesis.Root(), sdb)
chainConfig = genesisConfig.Config

if ctx.String(SenderFlag.Name) != "" {
Expand Down Expand Up @@ -231,7 +232,7 @@ func runCmd(ctx *cli.Context) error {
}
runtimeConfig := runtime.Config{
Origin: sender,
State: statedb,
State: prestate,
GasLimit: initialGas,
GasPrice: flags.GlobalBig(ctx, PriceFlag.Name),
Value: flags.GlobalBig(ctx, ValueFlag.Name),
Expand Down Expand Up @@ -274,14 +275,18 @@ func runCmd(ctx *cli.Context) error {
if ctx.Bool(CreateFlag.Name) {
input = append(code, input...)
execFunc = func() ([]byte, uint64, error) {
// don't mutate the state!
runtimeConfig.State = prestate.Copy()
Comment on lines +278 to +279
Copy link

@xermicus xermicus Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@jwasinger @holiman

Why was this done here?

This makes it impossible to re-use the state dump. The account still gets inserted however instead of an address I now see this in the state dump:

"pre(0xdfeeda06f79105a5aa975e5ef88e1b24d0062f31b5f42850e35d9cd4596df77e)": {

Which is wrong (should be the address) and means I can no longer take the state dump and execute another call based on the state dump.

Copy link
Member

Choose a reason for hiding this comment

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

Could you please open an issue with this, so we wont forget about it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Execution will modify the runtimeConfig.State. We wanted to ensure that the same prestate is used for executing every benchmark iteration, so we copy it here.

Copy link

Choose a reason for hiding this comment

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

@jwasinger thanks for pointing this out, got it. #31151 fixes this by only copying it during benchmarks.

Copy link

Choose a reason for hiding this comment

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

@MariusVanDerWijden since the fix looks obvious to me I decided to open a small PR instead (#31151).

output, _, gasLeft, err := runtime.Create(input, &runtimeConfig)
return output, gasLeft, err
}
} else {
if len(code) > 0 {
statedb.SetCode(receiver, code)
prestate.SetCode(receiver, code)
}
execFunc = func() ([]byte, uint64, error) {
// don't mutate the state!
runtimeConfig.State = prestate.Copy()
output, gasLeft, err := runtime.Call(receiver, input, &runtimeConfig)
return output, initialGas - gasLeft, err
}
Expand All @@ -291,7 +296,7 @@ func runCmd(ctx *cli.Context) error {
output, stats, err := timedExec(bench, execFunc)

if ctx.Bool(DumpFlag.Name) {
root, err := statedb.Commit(genesisConfig.Number, true)
root, err := runtimeConfig.State.Commit(genesisConfig.Number, true)
if err != nil {
fmt.Printf("Failed to commit changes %v\n", err)
return err
Expand All @@ -310,7 +315,7 @@ func runCmd(ctx *cli.Context) error {
logger.WriteTrace(os.Stderr, debugLogger.StructLogs())
}
fmt.Fprintln(os.Stderr, "#### LOGS ####")
logger.WriteLogs(os.Stderr, statedb.Logs())
logger.WriteLogs(os.Stderr, runtimeConfig.State.Logs())
}

if bench || ctx.Bool(StatDumpFlag.Name) {
Expand Down