eth/tracers/logger: conform structLog tracing to spec (#34093)

This is a breaking change in the opcode (structLog) tracer. Several fields
will have a slight formatting difference to conform to the newly established
spec at: https://github.com/ethereum/execution-apis/pull/762. The differences
include:

- `memory`: words will have the 0x prefix. Also last word of memory will be padded to 32-bytes.
- `storage`: keys and values will have the 0x prefix.

---------

Co-authored-by: Sina M <1591639+s1na@users.noreply.github.com>

eth/api_backend.go

@@ -484,12 +484,12 @@ func (b *EthAPIBackend) CurrentHeader() *types.Header {
 	return b.eth.blockchain.CurrentHeader()
 }
 
-func (b *EthAPIBackend) StateAtBlock(ctx context.Context, block *types.Block, reexec uint64, base *state.StateDB, readOnly bool, preferDisk bool) (*state.StateDB, tracers.StateReleaseFunc, error) {
+func (b *EthAPIBackend) StateAtBlock(ctx context.Context, block *types.Block, base *state.StateDB, readOnly bool, preferDisk bool) (*state.StateDB, tracers.StateReleaseFunc, error) {
-	return b.eth.stateAtBlock(ctx, block, reexec, base, readOnly, preferDisk)
+	return b.eth.stateAtBlock(ctx, block, base, readOnly, preferDisk)
 }
 
-func (b *EthAPIBackend) StateAtTransaction(ctx context.Context, block *types.Block, txIndex int, reexec uint64) (*types.Transaction, vm.BlockContext, *state.StateDB, tracers.StateReleaseFunc, error) {
+func (b *EthAPIBackend) StateAtTransaction(ctx context.Context, block *types.Block, txIndex int) (*types.Transaction, vm.BlockContext, *state.StateDB, tracers.StateReleaseFunc, error) {
-	return b.eth.stateAtTransaction(ctx, block, txIndex, reexec)
+	return b.eth.stateAtTransaction(ctx, block, txIndex)
 }
 
 func (b *EthAPIBackend) RPCTxSyncDefaultTimeout() time.Duration {

eth/api_debug.go

@@ -222,7 +222,7 @@ func (api *DebugAPI) StorageRangeAt(ctx context.Context, blockNrOrHash rpc.Block
 	if block == nil {
 		return StorageRangeResult{}, fmt.Errorf("block %v not found", blockNrOrHash)
 	}
-	_, _, statedb, release, err := api.eth.stateAtTransaction(ctx, block, txIndex, 0)
+	_, _, statedb, release, err := api.eth.stateAtTransaction(ctx, block, txIndex)
 	if err != nil {
 		return StorageRangeResult{}, err
 	}

eth/state_accessor.go

@@ -38,7 +38,11 @@ import (
 // for releasing state.
 var noopReleaser = tracers.StateReleaseFunc(func() {})
 
-func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec uint64, base *state.StateDB, readOnly bool, preferDisk bool) (statedb *state.StateDB, release tracers.StateReleaseFunc, err error) {
+// reexecLimit is the maximum number of ancestor blocks to walk back when
+// attempting to reconstruct missing historical state for hash-scheme nodes.
+const reexecLimit = uint64(128)
+
+func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, base *state.StateDB, readOnly bool, preferDisk bool) (statedb *state.StateDB, release tracers.StateReleaseFunc, err error) {
 	var (
 		current  *types.Block
 		database state.Database
@@ -99,7 +103,7 @@ func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec u
 			}
 		}
 		// Database does not have the state for the given block, try to regenerate
-		for i := uint64(0); i < reexec; i++ {
+		for i := uint64(0); i < reexecLimit; i++ {
 			if err := ctx.Err(); err != nil {
 				return nil, nil, err
 			}
@@ -120,7 +124,7 @@ func (eth *Ethereum) hashState(ctx context.Context, block *types.Block, reexec u
 		if err != nil {
 			switch err.(type) {
 			case *trie.MissingNodeError:
-				return nil, nil, fmt.Errorf("required historical state unavailable (reexec=%d)", reexec)
+				return nil, nil, fmt.Errorf("required historical state unavailable (reexec=%d)", reexecLimit)
 			default:
 				return nil, nil, err
 			}
@@ -190,10 +194,9 @@ func (eth *Ethereum) pathState(block *types.Block) (*state.StateDB, func(), erro
 }
 
 // stateAtBlock retrieves the state database associated with a certain block.
-// If no state is locally available for the given block, a number of blocks
+// If no state is locally available for the given block, up to reexecLimit ancestor
-// are attempted to be reexecuted to generate the desired state. The optional
+// blocks are reexecuted to generate the desired state. The optional base layer
-// base layer statedb can be provided which is regarded as the statedb of the
+// statedb can be provided which is regarded as the statedb of the parent block.
-// parent block.
 //
 // An additional release function will be returned if the requested state is
 // available. Release is expected to be invoked when the returned state is no
@@ -202,7 +205,6 @@ func (eth *Ethereum) pathState(block *types.Block) (*state.StateDB, func(), erro
 //
 // Parameters:
 //   - block:      The block for which we want the state(state = block.Root)
-//   - reexec:     The maximum number of blocks to reprocess trying to obtain the desired state
 //   - base:       If the caller is tracing multiple blocks, the caller can provide the parent
 //     state continuously from the callsite.
 //   - readOnly:   If true, then the live 'blockchain' state database is used. No mutation should
@@ -211,9 +213,9 @@ func (eth *Ethereum) pathState(block *types.Block) (*state.StateDB, func(), erro
 //   - preferDisk: This arg can be used by the caller to signal that even though the 'base' is
 //     provided, it would be preferable to start from a fresh state, if we have it
 //     on disk.
-func (eth *Ethereum) stateAtBlock(ctx context.Context, block *types.Block, reexec uint64, base *state.StateDB, readOnly bool, preferDisk bool) (statedb *state.StateDB, release tracers.StateReleaseFunc, err error) {
+func (eth *Ethereum) stateAtBlock(ctx context.Context, block *types.Block, base *state.StateDB, readOnly bool, preferDisk bool) (statedb *state.StateDB, release tracers.StateReleaseFunc, err error) {
 	if eth.blockchain.TrieDB().Scheme() == rawdb.HashScheme {
-		return eth.hashState(ctx, block, reexec, base, readOnly, preferDisk)
+		return eth.hashState(ctx, block, base, readOnly, preferDisk)
 	}
 	return eth.pathState(block)
 }
@@ -225,7 +227,7 @@ func (eth *Ethereum) stateAtBlock(ctx context.Context, block *types.Block, reexe
 // function will return the state of block after the pre-block operations have
 // been completed (e.g. updating system contracts), but before post-block
 // operations are completed (e.g. processing withdrawals).
-func (eth *Ethereum) stateAtTransaction(ctx context.Context, block *types.Block, txIndex int, reexec uint64) (*types.Transaction, vm.BlockContext, *state.StateDB, tracers.StateReleaseFunc, error) {
+func (eth *Ethereum) stateAtTransaction(ctx context.Context, block *types.Block, txIndex int) (*types.Transaction, vm.BlockContext, *state.StateDB, tracers.StateReleaseFunc, error) {
 	// Short circuit if it's genesis block.
 	if block.NumberU64() == 0 {
 		return nil, vm.BlockContext{}, nil, nil, errors.New("no transaction in genesis")
@@ -237,7 +239,7 @@ func (eth *Ethereum) stateAtTransaction(ctx context.Context, block *types.Block,
 	}
 	// Lookup the statedb of parent block from the live database,
 	// otherwise regenerate it on the flight.
-	statedb, release, err := eth.stateAtBlock(ctx, parent, reexec, nil, true, false)
+	statedb, release, err := eth.stateAtBlock(ctx, parent, nil, true, false)
 	if err != nil {
 		return nil, vm.BlockContext{}, nil, nil, err
 	}

eth/tracers/api.go

@@ -51,11 +51,6 @@ const (
 	// by default before being forcefully aborted.
 	defaultTraceTimeout = 5 * time.Second
 
-	// defaultTraceReexec is the number of blocks the tracer is willing to go back
-	// and reexecute to produce missing historical state necessary to run a specific
-	// trace.
-	defaultTraceReexec = uint64(128)
-
 	// defaultTracechainMemLimit is the size of the triedb, at which traceChain
 	// switches over and tries to use a disk-backed database instead of building
 	// on top of memory.
@@ -89,8 +84,8 @@ type Backend interface {
 	ChainConfig() *params.ChainConfig
 	Engine() consensus.Engine
 	ChainDb() ethdb.Database
-	StateAtBlock(ctx context.Context, block *types.Block, reexec uint64, base *state.StateDB, readOnly bool, preferDisk bool) (*state.StateDB, StateReleaseFunc, error)
+	StateAtBlock(ctx context.Context, block *types.Block, base *state.StateDB, readOnly bool, preferDisk bool) (*state.StateDB, StateReleaseFunc, error)
-	StateAtTransaction(ctx context.Context, block *types.Block, txIndex int, reexec uint64) (*types.Transaction, vm.BlockContext, *state.StateDB, StateReleaseFunc, error)
+	StateAtTransaction(ctx context.Context, block *types.Block, txIndex int) (*types.Transaction, vm.BlockContext, *state.StateDB, StateReleaseFunc, error)
 }
 
 // API is the collection of tracing APIs exposed over the private debugging endpoint.
@@ -156,7 +151,6 @@ type TraceConfig struct {
 	*logger.Config
 	Tracer  *string
 	Timeout *string
-	Reexec  *uint64
 	// Config specific to given tracer. Note struct logger
 	// config are historically embedded in main object.
 	TracerConfig json.RawMessage
@@ -174,7 +168,6 @@ type TraceCallConfig struct {
 // StdTraceConfig holds extra parameters to standard-json trace functions.
 type StdTraceConfig struct {
 	logger.Config
-	Reexec *uint64
 	TxHash common.Hash
 }
 
@@ -245,10 +238,6 @@ func (api *API) TraceChain(ctx context.Context, start, end rpc.BlockNumber, conf
 // transaction, dependent on the requested tracer.
 // The tracing procedure should be aborted in case the closed signal is received.
 func (api *API) traceChain(start, end *types.Block, config *TraceConfig, closed <-chan error) chan *blockTraceResult {
-	reexec := defaultTraceReexec
-	if config != nil && config.Reexec != nil {
-		reexec = *config.Reexec
-	}
 	blocks := int(end.NumberU64() - start.NumberU64())
 	threads := runtime.NumCPU()
 	if threads > blocks {
@@ -374,7 +363,7 @@ func (api *API) traceChain(start, end *types.Block, config *TraceConfig, closed
 				s1, s2, s3 := statedb.Database().TrieDB().Size()
 				preferDisk = s1+s2+s3 > defaultTracechainMemLimit
 			}
-			statedb, release, err = api.backend.StateAtBlock(ctx, block, reexec, statedb, false, preferDisk)
+			statedb, release, err = api.backend.StateAtBlock(ctx, block, statedb, false, preferDisk)
 			if err != nil {
 				failed = err
 				break
@@ -522,11 +511,7 @@ func (api *API) IntermediateRoots(ctx context.Context, hash common.Hash, config
 	if err != nil {
 		return nil, err
 	}
-	reexec := defaultTraceReexec
+	statedb, release, err := api.backend.StateAtBlock(ctx, parent, nil, true, false)
-	if config != nil && config.Reexec != nil {
-		reexec = *config.Reexec
-	}
-	statedb, release, err := api.backend.StateAtBlock(ctx, parent, reexec, nil, true, false)
 	if err != nil {
 		return nil, err
 	}
@@ -591,11 +576,7 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac
 	if err != nil {
 		return nil, err
 	}
-	reexec := defaultTraceReexec
+	statedb, release, err := api.backend.StateAtBlock(ctx, parent, nil, true, false)
-	if config != nil && config.Reexec != nil {
-		reexec = *config.Reexec
-	}
-	statedb, release, err := api.backend.StateAtBlock(ctx, parent, reexec, nil, true, false)
 	if err != nil {
 		return nil, err
 	}
@@ -743,11 +724,7 @@ func (api *API) standardTraceBlockToFile(ctx context.Context, block *types.Block
 	if err != nil {
 		return nil, err
 	}
-	reexec := defaultTraceReexec
+	statedb, release, err := api.backend.StateAtBlock(ctx, parent, nil, true, false)
-	if config != nil && config.Reexec != nil {
-		reexec = *config.Reexec
-	}
-	statedb, release, err := api.backend.StateAtBlock(ctx, parent, reexec, nil, true, false)
 	if err != nil {
 		return nil, err
 	}
@@ -877,15 +854,11 @@ func (api *API) TraceTransaction(ctx context.Context, hash common.Hash, config *
 	if blockNumber == 0 {
 		return nil, errors.New("genesis is not traceable")
 	}
-	reexec := defaultTraceReexec
-	if config != nil && config.Reexec != nil {
-		reexec = *config.Reexec
-	}
 	block, err := api.blockByNumberAndHash(ctx, rpc.BlockNumber(blockNumber), blockHash)
 	if err != nil {
 		return nil, err
 	}
-	tx, vmctx, statedb, release, err := api.backend.StateAtTransaction(ctx, block, int(index), reexec)
+	tx, vmctx, statedb, release, err := api.backend.StateAtTransaction(ctx, block, int(index))
 	if err != nil {
 		return nil, err
 	}
@@ -939,15 +912,10 @@ func (api *API) TraceCall(ctx context.Context, args ethapi.TransactionArgs, bloc
 		return nil, err
 	}
 	// try to recompute the state
-	reexec := defaultTraceReexec
-	if config != nil && config.Reexec != nil {
-		reexec = *config.Reexec
-	}
-
 	if config != nil && config.TxIndex != nil {
-		_, _, statedb, release, err = api.backend.StateAtTransaction(ctx, block, int(*config.TxIndex), reexec)
+		_, _, statedb, release, err = api.backend.StateAtTransaction(ctx, block, int(*config.TxIndex))
 	} else {
-		statedb, release, err = api.backend.StateAtBlock(ctx, block, reexec, nil, true, false)
+		statedb, release, err = api.backend.StateAtBlock(ctx, block, nil, true, false)
 	}
 	if err != nil {
 		return nil, err

eth/tracers/api_test.go

@@ -151,7 +151,7 @@ func (b *testBackend) teardown() {
 	b.chain.Stop()
 }
 
-func (b *testBackend) StateAtBlock(ctx context.Context, block *types.Block, reexec uint64, base *state.StateDB, readOnly bool, preferDisk bool) (*state.StateDB, StateReleaseFunc, error) {
+func (b *testBackend) StateAtBlock(ctx context.Context, block *types.Block, base *state.StateDB, readOnly bool, preferDisk bool) (*state.StateDB, StateReleaseFunc, error) {
 	statedb, err := b.chain.StateAt(block.Root())
 	if err != nil {
 		return nil, nil, errStateNotFound
@@ -167,12 +167,12 @@ func (b *testBackend) StateAtBlock(ctx context.Context, block *types.Block, reex
 	return statedb, release, nil
 }
 
-func (b *testBackend) StateAtTransaction(ctx context.Context, block *types.Block, txIndex int, reexec uint64) (*types.Transaction, vm.BlockContext, *state.StateDB, StateReleaseFunc, error) {
+func (b *testBackend) StateAtTransaction(ctx context.Context, block *types.Block, txIndex int) (*types.Transaction, vm.BlockContext, *state.StateDB, StateReleaseFunc, error) {
 	parent := b.chain.GetBlock(block.ParentHash(), block.NumberU64()-1)
 	if parent == nil {
 		return nil, vm.BlockContext{}, nil, nil, errBlockNotFound
 	}
-	statedb, release, err := b.StateAtBlock(ctx, parent, reexec, nil, true, false)
+	statedb, release, err := b.StateAtBlock(ctx, parent, nil, true, false)
 	if err != nil {
 		return nil, vm.BlockContext{}, nil, nil, errStateNotFound
 	}
@@ -202,6 +202,18 @@ type stateTracer struct {
 	Storage map[common.Address]map[common.Hash]common.Hash
 }
 
+type tracedOpcodeLog struct {
+	Op      string            `json:"op"`
+	Refund  *uint64           `json:"refund,omitempty"`
+	Storage map[string]string `json:"storage,omitempty"`
+}
+
+type tracedOpcodeResult struct {
+	Failed      bool              `json:"failed"`
+	ReturnValue string            `json:"returnValue"`
+	StructLogs  []tracedOpcodeLog `json:"structLogs"`
+}
+
 func newStateTracer(ctx *Context, cfg json.RawMessage, chainCfg *params.ChainConfig) (*Tracer, error) {
 	t := &stateTracer{
 		Balance: make(map[common.Address]*hexutil.Big),
@@ -1058,6 +1070,176 @@ func TestTracingWithOverrides(t *testing.T) {
 	}
 }
 
+func TestTraceTransactionRefundAndStorageSnapshots(t *testing.T) {
+	t.Parallel()
+
+	accounts := newAccounts(1)
+	contract := common.HexToAddress("0x00000000000000000000000000000000deadbeef")
+	slot0 := common.BigToHash(big.NewInt(0))
+	txSigner := types.HomesteadSigner{}
+	genesis := &core.Genesis{
+		Config: params.TestChainConfig,
+		Alloc: types.GenesisAlloc{
+			accounts[0].addr: {Balance: big.NewInt(params.Ether)},
+			contract: {
+				Nonce: 1,
+				Code: []byte{
+					byte(vm.PUSH1), 0x00,
+					byte(vm.SLOAD),
+					byte(vm.POP),
+					byte(vm.PUSH1), 0x00,
+					byte(vm.PUSH1), 0x00,
+					byte(vm.SSTORE),
+					byte(vm.STOP),
+				},
+				Storage: map[common.Hash]common.Hash{
+					slot0: common.BigToHash(big.NewInt(1)),
+				},
+			},
+		},
+	}
+	var target common.Hash
+	backend := newTestBackend(t, 1, genesis, func(i int, b *core.BlockGen) {
+		tx, _ := types.SignTx(types.NewTx(&types.LegacyTx{
+			Nonce:    0,
+			To:       &contract,
+			Value:    big.NewInt(0),
+			Gas:      100000,
+			GasPrice: b.BaseFee(),
+		}), txSigner, accounts[0].key)
+		b.AddTx(tx)
+		target = tx.Hash()
+	})
+	defer backend.teardown()
+
+	api := NewAPI(backend)
+	result, err := api.TraceTransaction(context.Background(), target, nil)
+	if err != nil {
+		t.Fatalf("failed to trace refunding transaction: %v", err)
+	}
+	var traced tracedOpcodeResult
+	if err := json.Unmarshal(result.(json.RawMessage), &traced); err != nil {
+		t.Fatalf("failed to unmarshal trace result: %v", err)
+	}
+	if traced.Failed {
+		t.Fatal("expected refunding transaction to succeed")
+	}
+	if traced.ReturnValue != "0x" {
+		t.Fatalf("unexpected return value: have %s want 0x", traced.ReturnValue)
+	}
+	slotHex := slot0.Hex()
+	oneHex := common.BigToHash(big.NewInt(1)).Hex()
+	zeroHex := common.Hash{}.Hex()
+	var (
+		foundSloadSnapshot  bool
+		foundSstoreSnapshot bool
+		foundRefund         bool
+	)
+	for _, log := range traced.StructLogs {
+		switch log.Op {
+		case "SLOAD":
+			if got := log.Storage[slotHex]; got == oneHex {
+				foundSloadSnapshot = true
+			}
+		case "SSTORE":
+			if got := log.Storage[slotHex]; got == zeroHex {
+				foundSstoreSnapshot = true
+			}
+		}
+		if log.Refund != nil && *log.Refund > 0 {
+			foundRefund = true
+		}
+	}
+	if !foundSloadSnapshot {
+		t.Fatal("expected SLOAD snapshot to include the pre-existing non-zero storage value")
+	}
+	if !foundSstoreSnapshot {
+		t.Fatal("expected SSTORE snapshot to include the post-write zeroed storage value")
+	}
+	if !foundRefund {
+		t.Fatal("expected at least one structLog entry with a non-zero refund field")
+	}
+}
+
+func TestTraceTransactionFailureReturnValues(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name            string
+		code            []byte
+		wantReturnValue string
+	}{
+		{
+			name: "revert preserves return data",
+			code: []byte{
+				byte(vm.PUSH1), 0x2a,
+				byte(vm.PUSH1), 0x00,
+				byte(vm.MSTORE),
+				byte(vm.PUSH1), 0x20,
+				byte(vm.PUSH1), 0x00,
+				byte(vm.REVERT),
+			},
+			wantReturnValue: "0x000000000000000000000000000000000000000000000000000000000000002a",
+		},
+		{
+			name: "hard failure clears return data",
+			code: []byte{
+				byte(vm.INVALID),
+			},
+			wantReturnValue: "0x",
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			accounts := newAccounts(1)
+			contract := common.HexToAddress("0x00000000000000000000000000000000deadbeef")
+			txSigner := types.HomesteadSigner{}
+			genesis := &core.Genesis{
+				Config: params.TestChainConfig,
+				Alloc: types.GenesisAlloc{
+					accounts[0].addr: {Balance: big.NewInt(params.Ether)},
+					contract: {
+						Nonce: 1,
+						Code:  tc.code,
+					},
+				},
+			}
+			var target common.Hash
+			backend := newTestBackend(t, 1, genesis, func(i int, b *core.BlockGen) {
+				tx, _ := types.SignTx(types.NewTx(&types.LegacyTx{
+					Nonce:    0,
+					To:       &contract,
+					Value:    big.NewInt(0),
+					Gas:      100000,
+					GasPrice: b.BaseFee(),
+				}), txSigner, accounts[0].key)
+				b.AddTx(tx)
+				target = tx.Hash()
+			})
+			defer backend.teardown()
+
+			api := NewAPI(backend)
+			result, err := api.TraceTransaction(context.Background(), target, nil)
+			if err != nil {
+				t.Fatalf("failed to trace transaction: %v", err)
+			}
+			var traced tracedOpcodeResult
+			if err := json.Unmarshal(result.(json.RawMessage), &traced); err != nil {
+				t.Fatalf("failed to unmarshal trace result: %v", err)
+			}
+			if !traced.Failed {
+				t.Fatal("expected traced transaction to fail")
+			}
+			if traced.ReturnValue != tc.wantReturnValue {
+				t.Fatalf("unexpected returnValue: have %s want %s", traced.ReturnValue, tc.wantReturnValue)
+			}
+			if len(traced.StructLogs) == 0 {
+				t.Fatal("expected failing trace to still include structLogs")
+			}
+		})
+	}
+}
+
 type Account struct {
 	key  *ecdsa.PrivateKey
 	addr common.Address

eth/tracers/logger/logger.go

@@ -148,7 +148,7 @@ type structLogLegacy struct {
 	Gas           uint64             `json:"gas"`
 	GasCost       uint64             `json:"gasCost"`
 	Depth         int                `json:"depth"`
-	Error         string             `json:"error,omitempty"`
+	Error         string             `json:"error,omitempty,omitzero"`
 	Stack         *[]string          `json:"stack,omitempty"`
 	ReturnData    string             `json:"returnData,omitempty"`
 	Memory        *[]string          `json:"memory,omitempty"`
@@ -156,6 +156,15 @@ type structLogLegacy struct {
 	RefundCounter uint64             `json:"refund,omitempty"`
 }
 
+func formatMemoryWord(chunk []byte) string {
+	if len(chunk) == 32 {
+		return hexutil.Encode(chunk)
+	}
+	var word [32]byte
+	copy(word[:], chunk)
+	return hexutil.Encode(word[:])
+}
+
 // toLegacyJSON converts the structLog to legacy json-encoded legacy form.
 func (s *StructLog) toLegacyJSON() json.RawMessage {
 	msg := structLogLegacy{
@@ -175,7 +184,7 @@ func (s *StructLog) toLegacyJSON() json.RawMessage {
 		msg.Stack = &stack
 	}
 	if len(s.ReturnData) > 0 {
-		msg.ReturnData = hexutil.Bytes(s.ReturnData).String()
+		msg.ReturnData = hexutil.Encode(s.ReturnData)
 	}
 	if len(s.Memory) > 0 {
 		memory := make([]string, 0, (len(s.Memory)+31)/32)
@@ -184,14 +193,14 @@ func (s *StructLog) toLegacyJSON() json.RawMessage {
 			if end > len(s.Memory) {
 				end = len(s.Memory)
 			}
-			memory = append(memory, fmt.Sprintf("%x", s.Memory[i:end]))
+			memory = append(memory, formatMemoryWord(s.Memory[i:end]))
 		}
 		msg.Memory = &memory
 	}
 	if len(s.Storage) > 0 {
 		storage := make(map[string]string)
 		for i, storageValue := range s.Storage {
-			storage[fmt.Sprintf("%x", i)] = fmt.Sprintf("%x", storageValue)
+			storage[i.Hex()] = storageValue.Hex()
 		}
 		msg.Storage = &storage
 	}

eth/tracers/logger/logger_test.go

@@ -96,3 +96,46 @@ func TestStructLogMarshalingOmitEmpty(t *testing.T) {
 		})
 	}
 }
+
+func TestStructLogLegacyJSONSpecFormatting(t *testing.T) {
+	tests := []struct {
+		name string
+		log  *StructLog
+		want string
+	}{
+		{
+			name: "omits empty error and pads memory/storage",
+			log: &StructLog{
+				Pc:         7,
+				Op:         vm.SSTORE,
+				Gas:        100,
+				GasCost:    20,
+				Memory:     []byte{0xaa, 0xbb},
+				Storage:    map[common.Hash]common.Hash{common.BigToHash(big.NewInt(1)): common.BigToHash(big.NewInt(2))},
+				Depth:      1,
+				ReturnData: []byte{0x12, 0x34},
+			},
+			want: `{"pc":7,"op":"SSTORE","gas":100,"gasCost":20,"depth":1,"returnData":"0x1234","memory":["0xaabb000000000000000000000000000000000000000000000000000000000000"],"storage":{"0x0000000000000000000000000000000000000000000000000000000000000001":"0x0000000000000000000000000000000000000000000000000000000000000002"}}`,
+		},
+		{
+			name: "includes error only when present",
+			log: &StructLog{
+				Pc:      1,
+				Op:      vm.STOP,
+				Gas:     2,
+				GasCost: 3,
+				Depth:   1,
+				Err:     errors.New("boom"),
+			},
+			want: `{"pc":1,"op":"STOP","gas":2,"gasCost":3,"depth":1,"error":"boom"}`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			have := string(tt.log.toLegacyJSON())
+			if have != tt.want {
+				t.Fatalf("mismatched results\n\thave: %v\n\twant: %v", have, tt.want)
+			}
+		})
+	}
+}