Adding erc7562 tracer #31006
Adding erc7562 tracer #31006
Conversation
eth/tracers/native/erc7562.go
Outdated
| @@ -0,0 +1,550 @@ | |||
| // Copyright 2021 The go-ethereum Authors | |||
There was a problem hiding this comment.
| // Copyright 2021 The go-ethereum Authors | |
| // Copyright 2025 The go-ethereum Authors |
eth/tracers/native/erc7562.go
Outdated
| } | ||
|
|
||
| type callFrameWithOpcodes struct { | ||
| Type vm.OpCode `json:"-"` |
There was a problem hiding this comment.
WDYT about reusing some code from the call tracer?
diff --git a/eth/tracers/native/erc7562.go b/eth/tracers/native/erc7562.go
index cd1a48975f..7f80c7d168 100644
--- a/eth/tracers/native/erc7562.go
+++ b/eth/tracers/native/erc7562.go
@@ -48,20 +48,7 @@ type contractSizeWithOpcode struct {
}
type callFrameWithOpcodes struct {
- Type vm.OpCode `json:"-"`
- From common.Address `json:"from"`
- Gas uint64 `json:"gas"`
- GasUsed uint64 `json:"gasUsed"`
- To *common.Address `json:"to,omitempty" rlp:"optional"`
- Input []byte `json:"input" rlp:"optional"`
- Output []byte `json:"output,omitempty" rlp:"optional"`
- Error string `json:"error,omitempty" rlp:"optional"`
- RevertReason string `json:"revertReason,omitempty"`
- Logs []callLog `json:"logs,omitempty" rlp:"optional"`
- // Placed at end on purpose. The RLP will be decoded to 0 instead of
- // nil if there are non-empty elements after in the struct.
- Value *big.Int `json:"value,omitempty" rlp:"optional"`
- revertedSnapshot bool
+ callFrame
AccessedSlots accessedSlots `json:"accessedSlots"`
ExtCodeAccessInfo []common.Address `json:"extCodeAccessInfo"`
@@ -228,12 +215,14 @@ func (t *erc7562Tracer) OnEnter(depth int, typ byte, from common.Address, to com
toCopy := to
call := callFrameWithOpcodes{
- Type: vm.OpCode(typ),
- From: from,
- To: &toCopy,
- Input: common.CopyBytes(input),
- Gas: gas,
- Value: value,
+ callFrame: callFrame{
+ Type: vm.OpCode(typ),
+ From: from,
+ To: &toCopy,
+ Input: common.CopyBytes(input),
+ Gas: gas,
+ Value: value,
+ },
AccessedSlots: accessedSlots{
Reads: map[string][]string{},
Writes: map[string]uint64{},
```| "github.com/holiman/uint256" | ||
| ) | ||
|
|
||
| //go:generate go run github.com/fjl/gencodec -type callFrameWithOpcodes -field-override callFrameWithOpcodesMarshaling -out gen_callframewithopcodes_json.go |
There was a problem hiding this comment.
| //go:generate go run github.com/fjl/gencodec -type callFrameWithOpcodes -field-override callFrameWithOpcodesMarshaling -out gen_callframewithopcodes_json.go | |
| //go:generate go run github.com/fjl/gencodec -type callFrameWithOpcodes -field-override callFrameMarshaling -out gen_callframewithopcodes_json.go |
There was a problem hiding this comment.
go generate doesn't work with that, and it seems that embedded structs aren't supported in gencodec. So my solution was to flatten out the struct instead. Wdyt?
eth/tracers/native/erc7562.go
Outdated
| } | ||
|
|
||
| // catchPanic handles panic recovery and logs the panic and stack trace. | ||
| func catchPanic() { |
There was a problem hiding this comment.
I really don't like this pattern. Your code should be correct, you should not rely on recovering. Rather fail fast and fix quickly
There was a problem hiding this comment.
Right, I'd like to second this. Did you need this for debug purposes?
Panics here should not crash the node. API internal has recovery mechanism from panics. If you managed to crash the node then that's an issue we should look at.
There was a problem hiding this comment.
I needed that for debugging purposes, since the errors on the hooks were silently ignored by geth it seems. I'm removing it.
eth/tracers/native/erc7562.go
Outdated
| return config | ||
| } | ||
|
|
||
| func newErc7562TracerObject(ctx *tracers.Context, cfg json.RawMessage) (*erc7562Tracer, error) { |
There was a problem hiding this comment.
| func newErc7562TracerObject(ctx *tracers.Context, cfg json.RawMessage) (*erc7562Tracer, error) { | |
| func newErc7562TracerObject(cfg json.RawMessage) (*erc7562Tracer, error) { |
ctx is unused here, no need to pass it then
There was a problem hiding this comment.
tracers.DefaultDirectory.Register("erc7562Tracer", newErc7562Tracer, false) expects a certain signature for the tracer that accepts that argument. That's also the case in the original callTracer
eth/tracers/native/erc7562.go
Outdated
| t.callstackWithOpcodes = append(t.callstackWithOpcodes, call) | ||
| } | ||
|
|
||
| func (t *erc7562Tracer) captureEnd(output []byte, gasUsed uint64, err error, reverted bool) { |
There was a problem hiding this comment.
| func (t *erc7562Tracer) captureEnd(output []byte, gasUsed uint64, err error, reverted bool) { | |
| func (t *erc7562Tracer) captureEnd(output []byte, err error, reverted bool) { |
gas used is unused
eth/tracers/native/erc7562.go
Outdated
| } | ||
|
|
||
| func incrementCount[K comparable](m map[K]uint64, k K) { | ||
| m[k] = m[k] + 1 |
| for i := 0; i < t.config.StackTopItemsSize && i < stackSize; i++ { | ||
| stackTopItems = append(stackTopItems, *peepStack(scope.StackData(), i)) | ||
| } | ||
| opcodeWithStack = &opcodeWithPartialStack{ |
There was a problem hiding this comment.
I would switch on the opcode here instead of calling into multiple methods that might or might not do anything
|
It would be nice to have some spec-compliance tests. |
| ignoredOpcodes map[vm.OpCode]struct{} | ||
| callstackWithOpcodes []callFrameWithOpcodes | ||
| lastOpWithStack *opcodeWithPartialStack | ||
| Keccak map[string]struct{} `json:"keccak"` |
There was a problem hiding this comment.
I think this field can be un-exported and the json tag is not needed. This struct is not directly serialized.
- Removing unused code - Adding test cases
|
@shahafn I see that you have added some test fixtures. I think you forgot to commit the test runner. Please push that too. |
| AccessedSlots accessedSlots `json:"accessedSlots"` | ||
| ExtCodeAccessInfo []common.Address `json:"extCodeAccessInfo"` | ||
| UsedOpcodes map[vm.OpCode]uint64 `json:"usedOpcodes"` | ||
| ContractSize map[common.Address]*contractSizeWithOpcode `json:"contractSize"` |
There was a problem hiding this comment.
Can you please add a short comment on these "new" fields what they should collect?
eth/tracers/native/erc7562.go
Outdated
| Output hexutil.Bytes | ||
| AccessedSlots accessedSlots `json:"accessedSlots"` | ||
| ExtCodeAccessInfo []common.Address `json:"extCodeAccessInfo"` | ||
| UsedOpcodes map[vm.OpCode]uint64 `json:"usedOpcodes"` |
There was a problem hiding this comment.
Probably also use hexutil.Uint64 here for consistency?
eth/tracers/native/erc7562.go
Outdated
| UsedOpcodes map[vm.OpCode]uint64 `json:"usedOpcodes"` | ||
| ContractSize map[common.Address]*contractSizeWithOpcode `json:"contractSize"` | ||
| OutOfGas bool `json:"outOfGas"` | ||
| Calls []callFrameWithOpcodes `json:"calls,omitempty"` |
There was a problem hiding this comment.
In the Marshaling struct you only need to specify the fields which you wish to override the type for. So in this case Calls is not necessary here, Same with OutOfGas, ContractSize, ExtCodeAccessInfo and AccessedSlots.
The reason for using this double-struct approach is so that in the tracer logic we can simply use normal Go types (like uint64) and when calling json.Marshal it will "automatically" understand how to encode them because we provide hints here that encoding should use these other specialized types (e.g. hexutil.Uint64).
| type accessedSlots struct { | ||
| Reads map[string][]string `json:"reads"` | ||
| Writes map[string]uint64 `json:"writes"` | ||
| TransientReads map[string]uint64 `json:"transientReads"` |
There was a problem hiding this comment.
We have to agree on the output format. I think it's not great that some fields are decimal numbers in JSON and some are hex-encoded.
Fixing marshaling
Comparing unmarshaled objects and sorting keccak array to prevent cosmetic diffs
Resolves #30546