erigontech / erigontech/erigon

execution: EIP-161 emptiness is open-coded in four places alongside Account.Empty()

Open
#22,794 1 comment 0 reactions 1 assignee Claimed by @yperbasis View on GitHub
tech debt reduction
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

## What

The EIP-161 emptiness test — zero balance, zero nonce, empty code hash — is open-coded at four sites, even though `accounts.Account.Empty()` (`execution/types/accounts/account.go:268`) states exactly that rule:

| site | spelling |
|---|---|
| `execution/state/rw_v3.go:359` | `acc.Nonce == 0 && acc.Balance.IsZero() && acc.IsEmptyCodeHash()` |
| `execution/stagedsync/calc_state.go:294` | `acc.Balance.IsZero() && acc.Nonce == 0 && acc.CodeHash == empty.CodeHash` |
| `execution/stagedsync/calc_state.go:318` | the same, as `isAllZero` |
| `execution/state/writeset_normalize.go:493` | `s.balance.IsZero() && s.nonce == 0 && s.codeHash.IsEmpty()` |

Three different spellings of one predicate, none of them the canonical one.

## Why it's worth tidying

Nothing is wrong today. The cost is that the rule has four homes: anyone changing what "empty" means — or auditing it, which happens whenever empty-account clearing is touched — has to find all of them, and the differing spellings make them hard to grep for as a set.

## Not a straight substitution

Only the first is a drop-in: it already holds an `accounts.Account`.

The two in `calc_state.go` operate on `calcAccountState`, and the one in `writeset_normalize.go` on a function-local `acctState`. Both are their own structs with their own `CodeHash` field types, so they want a small `Empty()` method on the type rather than a call to `Account.Empty()` — which still gets to one predicate per type instead of open-coded field comparisons.

Worth flagging for whoever picks this up: `Account.Empty()` returns true for a nil receiver, which none of the open-coded sites do. It should not be dropped in anywhere the account may be nil without first confirming that nil is meant to read as empty.

## Scope

Hygiene only, no behaviour change intended. The `EIP161EmptyRemoval` fork/AuRa gating that sits alongside these checks stays exactly where it is.

Came out of review of #22768, which added a fifth copy and then collapsed it onto `Account.Empty()`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.