execution/tracers: emit empty code in diff mode for EIP-7702 deauthorization#20601
Conversation
7c27cbf to
8c20fbe
Compare
8c20fbe to
fad1dd6
Compare
yperbasis
left a comment
There was a problem hiding this comment.
Issues
- Dead empty field (prestate.go:58, set at prestate.go:403). The new unexported empty bool field is assigned in lookupAccount but never read. Either wire it into the
OnTxEnd/CaptureEnd filter (replacing !s.exists()) so it reflects emptiness at lookup time, or drop it entirely — right now it's noise. - Comment regression between commits. The first commit added a useful explanation above prevCodeHash := empty.CodeHash:
▎ "Empty code hashes are excluded from the prestate, so default to EmptyCodeHash to match what GetCodeHash returns for codeless accounts."
The second commit removed it. That rationale is non-obvious and worth keeping. Please restore it. - lookupAccount comment is unclear (prestate.go:400): // code fetched before emptiness check, then stripped if disabled. The intent (preserve emptiness semantics regardless of
DisableCode) could be stated more directly, e.g. "Fetch and use code for the emptiness check before honoring DisableCode — account emptiness shouldn't depend on the config flag."
79a0fad to
831668e
Compare
Thanks, I’ve addressed all three points.
|
There was a problem hiding this comment.
Pull request overview
This PR adjusts the native prestate tracer’s diff-mode semantics to correctly represent EIP-7702 deauthorization as a code update to empty code (emitting "code": "0x" in post), rather than omitting the code field as if it were deleted.
Changes:
- Make
account.Codeoptional via pointer semantics so diff-mode can omit unchanged code while still emitting empty code on code-clear transitions. - Use
empty.CodeHashas the default/empty code hash in diff-mode comparisons to match actual EVM semantics. - Add a diff-mode trace fixture for EIP-7702 deauthorization expecting
"code": "0x"inpost.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
execution/tracing/tracers/native/prestate.go |
Updates account code representation and diff-mode comparison/serialization behavior for cleared code + empty code hash handling. |
execution/tracing/tracers/native/gen_account_json.go |
Updates generated JSON marshal/unmarshal to match the new pointer-based Code field. |
execution/tracing/tracers/internal/tracetest/testdata/prestate_tracer_with_diff_mode/eip7702_deauth.json |
Adds regression test vector covering EIP-7702 deauthorization diff output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yperbasis
left a comment
There was a problem hiding this comment.
- Misleading comment in lookupAccount (execution/tracing/tracers/native/prestate.go:401)
// Fetch and use code for the emptiness check before honoring DisableCode.
// Account emptiness should not depend on the config flag.
if t.config.DisableCode {
acc.Code = nil
}
The comment claims emptiness shouldn't depend on DisableCode, but the implementation still nullifies acc.Code before any emptiness check runs (via OnTxEnd → exists()). So a
code-bearing account with Nonce=0, Balance=0, no storage, would be considered empty when DisableCode=true, but not when false — exactly the dependency the comment says to avoid.
Either delete the misleading comment, or actually fix the dependency by using CodeHash (which is always populated) in exists():
func (a *account) exists() bool {
return a.Nonce > 0 || a.CodeHash != nil || len(a.Storage) > 0 || (a.Balance != nil && a.Balance.Sign() != 0)
}
In practice this edge case is rare (most code-bearing accounts have nonce ≥ 1), so it's not blocking — but the comment as-is is confusing.
- Dead normalization in processDiffState (prestate.go:320-322)
if newCode == nil {
newCode = []byte{}
}
postAccount.Code = &newCode
Unnecessary. hexutil.Bytes.MarshalText (common/hexutil/bytes.go:34) computes make([]byte, len(b)*2+2) and writes the 0x prefix — on a nil slice it yields exactly "0x", same as
[]byte{}. The branch can be dropped; postAccount.Code = &newCode alone gives the right JSON in both cases.
I changed exists() to check CodeHash rather than Code, so whether or not DisableCode is set, the empty result remains the same. I changed the previous remark to a shorter one that just describes why we fetch code before respecting the flag because it was confusing. Since hexutil, I also removed the nil-to-empty normalization in processDiffState.The branch was useless because Bytes marshals both empty and nil slices to "0x" nonetheless. |
831668e to
ff222f1
Compare
This PR fixes prestate tracer diff-mode output for EIP-7702 deauthorization in Erigon.
Previously, when a transaction deauthorized an EIP-7702 delegated account (authorization to address(0)), the tracer omitted the
codefield from thepostobject after the delegation code was cleared. That treated the change as if the field had been deleted entirely.In reality, the account still exists and its code is simply updated from delegated code (
0xef0100 || delegate) to empty code (0x). Since this is a normal state transition - not an account deletion - thepostobject should include the updated empty code value.This makes code handling consistent with how other account fields like balance are represented when they transition to zero, and keeps diff-mode semantics clearer: omitted fields should represent actual deletions, while modified fields should appear in
posteven if their new value is empty.After this change, EIP-7702 deauthorization traces correctly return
"code": "0x"in thepoststate.Refrence: ethereum/go-ethereum#34648