From 313a43d09da98180d565b005641a9a4eff27a67e Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 18 Mar 2024 10:36:12 +0100 Subject: [PATCH] cmd/evm: fix tracing This commit fixes several missing interface-checks, where the code did not properly verify a Hook before calling it. It also fixes an error where the tracer was not set in the vmconfig during t8n initialization. It also adds a testcase which verifies the tracing-output during t8n executio. The testcase currently only checks the jsonlogger, but can be extended. This change also removes a spurious 'null' output in the tracing, when tracing result is null. --- cmd/evm/internal/t8ntool/execution.go | 14 +-- cmd/evm/t8n_test.go | 89 +++++++++++++++++++ cmd/evm/testdata/31/README.md | 1 + cmd/evm/testdata/31/alloc.json | 16 ++++ cmd/evm/testdata/31/env.json | 20 +++++ ...7543268a5aaf2a6b32a69d2c6d978c45dcfb.jsonl | 6 ++ cmd/evm/testdata/31/txs.json | 14 +++ 7 files changed, 155 insertions(+), 5 deletions(-) create mode 100644 cmd/evm/testdata/31/README.md create mode 100644 cmd/evm/testdata/31/alloc.json create mode 100644 cmd/evm/testdata/31/env.json create mode 100644 cmd/evm/testdata/31/trace-0-0x88f5fbd1524731a81e49f637aa847543268a5aaf2a6b32a69d2c6d978c45dcfb.jsonl create mode 100644 cmd/evm/testdata/31/txs.json diff --git a/cmd/evm/internal/t8ntool/execution.go b/cmd/evm/internal/t8ntool/execution.go index 52e5bbe9de..88ac13d1f0 100644 --- a/cmd/evm/internal/t8ntool/execution.go +++ b/cmd/evm/internal/t8ntool/execution.go @@ -228,7 +228,7 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig, if err != nil { return nil, nil, nil, err } - if vmConfig.Tracer != nil { + if tracer != nil { vmConfig.Tracer = tracer.Hooks } statedb.SetTxContext(tx.Hash(), txIndex) @@ -240,7 +240,7 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig, ) evm := vm.NewEVM(vmContext, txContext, statedb, chainConfig, vmConfig) - if tracer != nil { + if tracer != nil && tracer.OnTxStart != nil { tracer.OnTxStart(evm.GetVMContext(), tx, msg.From) } // (ret []byte, usedGas uint64, failed bool, err error) @@ -251,7 +251,9 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig, rejectedTxs = append(rejectedTxs, &rejectedTx{i, err.Error()}) gaspool.SetGas(prevGas) if tracer != nil { - tracer.OnTxEnd(nil, err) + if tracer.OnTxEnd != nil { + tracer.OnTxEnd(nil, err) + } if err := writeTraceResult(tracer, traceOutput); err != nil { log.Warn("Error writing tracer output", "err", err) } @@ -298,7 +300,9 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig, receipt.TransactionIndex = uint(txIndex) receipts = append(receipts, receipt) if tracer != nil { - tracer.Hooks.OnTxEnd(receipt, nil) + if tracer.Hooks.OnTxEnd != nil { + tracer.Hooks.OnTxEnd(receipt, nil) + } writeTraceResult(tracer, traceOutput) } } @@ -418,7 +422,7 @@ func calcDifficulty(config *params.ChainConfig, number, currentTime, parentTime func writeTraceResult(tracer *directory.Tracer, f io.WriteCloser) error { defer f.Close() result, err := tracer.GetResult() - if err != nil { + if err != nil || result == nil { return err } err = json.NewEncoder(f).Encode(result) diff --git a/cmd/evm/t8n_test.go b/cmd/evm/t8n_test.go index 03503d11c3..bb36434f52 100644 --- a/cmd/evm/t8n_test.go +++ b/cmd/evm/t8n_test.go @@ -17,9 +17,12 @@ package main import ( + "bufio" "encoding/json" "fmt" + "io" "os" + "path/filepath" "reflect" "strings" "testing" @@ -320,6 +323,92 @@ func TestT8n(t *testing.T) { } } +func lineIterator(path string) func() (string, error) { + data, err := os.ReadFile(path) + if err != nil { + return func() (string, error) { return err.Error(), err } + } + scanner := bufio.NewScanner(strings.NewReader(string(data))) + return func() (string, error) { + if scanner.Scan() { + return scanner.Text(), nil + } + if err := scanner.Err(); err != nil { + return "", err + } + return "", io.EOF // scanner gobbles io.EOF, but we want it + } +} + +// TestT8nTracing is a test that checks the tracing-output from t8n. +func TestT8nTracing(t *testing.T) { + t.Parallel() + tt := new(testT8n) + tt.TestCmd = cmdtest.NewTestCmd(t, tt) + for i, tc := range []struct { + base string + input t8nInput + expExitCode int + expectedTraces []string + }{ + { + base: "./testdata/31", + input: t8nInput{ + "alloc.json", "txs.json", "env.json", "Cancun", "", + }, + expectedTraces: []string{"trace-0-0x88f5fbd1524731a81e49f637aa847543268a5aaf2a6b32a69d2c6d978c45dcfb.jsonl"}, + }, + } { + args := []string{"t8n"} + args = append(args, tc.input.get(tc.base)...) + // Add tracing-args + args = append(args, "--trace") + // Place the output somewhere we can find it + outdir := t.TempDir() + + args = append(args, "--output.basedir", outdir) + + var qArgs []string // quoted args for debugging purposes + for _, arg := range args { + if len(arg) == 0 { + qArgs = append(qArgs, `""`) + } else { + qArgs = append(qArgs, arg) + } + } + tt.Logf("args: %v\n", strings.Join(qArgs, " ")) + tt.Run("evm-test", args...) + tt.WaitExit() + // Compare the expected traces + for _, traceFile := range tc.expectedTraces { + haveFn := lineIterator(filepath.Join(outdir, traceFile)) + wantFn := lineIterator(filepath.Join(tc.base, traceFile)) + + for line := 0; ; line++ { + want, wErr := wantFn() + have, hErr := haveFn() + if want != have { + t.Fatalf("test %d, trace %v, line %d\nwant: %v\nhave: %v\n", + i, traceFile, line, want, have) + } + if wErr != nil && hErr != nil { + break + } + if wErr != nil { + t.Fatal(wErr) + } + if hErr != nil { + t.Fatal(hErr) + } + t.Logf("%v\n", want) + } + } + if have, want := tt.ExitStatus(), tc.expExitCode; have != want { + t.Fatalf("test %d: wrong exit code, have %d, want %d", i, have, want) + } + } +} + type t9nInput struct { inTxs string stFork string diff --git a/cmd/evm/testdata/31/README.md b/cmd/evm/testdata/31/README.md new file mode 100644 index 0000000000..305e4f52da --- /dev/null +++ b/cmd/evm/testdata/31/README.md @@ -0,0 +1 @@ +This test does some EVM execution, and can be used to test the tracers and trace-outputs. diff --git a/cmd/evm/testdata/31/alloc.json b/cmd/evm/testdata/31/alloc.json new file mode 100644 index 0000000000..bad5481c4a --- /dev/null +++ b/cmd/evm/testdata/31/alloc.json @@ -0,0 +1,16 @@ +{ + "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b" : { + "balance" : "0x016345785d8a0000", + "code" : "0x", + "nonce" : "0x00", + "storage" : { + } + }, + "0x1111111111111111111111111111111111111111" : { + "balance" : "0x1", + "code" : "0x604060406040604000", + "nonce" : "0x00", + "storage" : { + } + } +} \ No newline at end of file diff --git a/cmd/evm/testdata/31/env.json b/cmd/evm/testdata/31/env.json new file mode 100644 index 0000000000..09b5f12d88 --- /dev/null +++ b/cmd/evm/testdata/31/env.json @@ -0,0 +1,20 @@ +{ + "currentCoinbase" : "0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba", + "currentNumber" : "0x01", + "currentTimestamp" : "0x03e8", + "currentGasLimit" : "0x1000000000", + "previousHash" : "0xe4e2a30b340bec696242b67584264f878600dce98354ae0b6328740fd4ff18da", + "currentDataGasUsed" : "0x2000", + "parentTimestamp" : "0x00", + "parentDifficulty" : "0x00", + "parentUncleHash" : "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347", + "parentBeaconBlockRoot" : "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347", + "currentRandom" : "0x0000000000000000000000000000000000000000000000000000000000020000", + "withdrawals" : [ + ], + "parentBaseFee" : "0x08", + "parentGasUsed" : "0x00", + "parentGasLimit" : "0x1000000000", + "parentExcessBlobGas" : "0x1000", + "parentBlobGasUsed" : "0x2000" +} \ No newline at end of file diff --git a/cmd/evm/testdata/31/trace-0-0x88f5fbd1524731a81e49f637aa847543268a5aaf2a6b32a69d2c6d978c45dcfb.jsonl b/cmd/evm/testdata/31/trace-0-0x88f5fbd1524731a81e49f637aa847543268a5aaf2a6b32a69d2c6d978c45dcfb.jsonl new file mode 100644 index 0000000000..26e5c7ee4e --- /dev/null +++ b/cmd/evm/testdata/31/trace-0-0x88f5fbd1524731a81e49f637aa847543268a5aaf2a6b32a69d2c6d978c45dcfb.jsonl @@ -0,0 +1,6 @@ +{"pc":0,"op":96,"gas":"0x13498","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"} +{"pc":2,"op":96,"gas":"0x13495","gasCost":"0x3","memSize":0,"stack":["0x40"],"depth":1,"refund":0,"opName":"PUSH1"} +{"pc":4,"op":96,"gas":"0x13492","gasCost":"0x3","memSize":0,"stack":["0x40","0x40"],"depth":1,"refund":0,"opName":"PUSH1"} +{"pc":6,"op":96,"gas":"0x1348f","gasCost":"0x3","memSize":0,"stack":["0x40","0x40","0x40"],"depth":1,"refund":0,"opName":"PUSH1"} +{"pc":8,"op":0,"gas":"0x1348c","gasCost":"0x0","memSize":0,"stack":["0x40","0x40","0x40","0x40"],"depth":1,"refund":0,"opName":"STOP"} +{"output":"","gasUsed":"0xc"} diff --git a/cmd/evm/testdata/31/txs.json b/cmd/evm/testdata/31/txs.json new file mode 100644 index 0000000000..473c1526f4 --- /dev/null +++ b/cmd/evm/testdata/31/txs.json @@ -0,0 +1,14 @@ +[ + { + "gas": "0x186a0", + "gasPrice": "0x600", + "input": "0x", + "nonce": "0x0", + "to": "0x1111111111111111111111111111111111111111", + "value": "0x1", + "v" : "0x0", + "r" : "0x0", + "s" : "0x0", + "secretKey" : "0x45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8" + } +]