test: StateAccount.Extra via trie.StateTrie.{Update,Get}Account() (#45)

* refactor: abstract `testonly` package

* test: `StateAccount.Extra` via `trie.StateTrie.{Update,Get}Account()`

* chore: `types.TestOnlyClearRegisteredExtras()` at beginning of tests

This is a purely defensive approach in case future tests forget to clean up.

* chore: placate the linter

core/types/rlp_payload.libevm.go

@@ -21,6 +21,7 @@ import (
 	"io"
 
 	"github.com/ethereum/go-ethereum/libevm/pseudo"
+	"github.com/ethereum/go-ethereum/libevm/testonly"
 	"github.com/ethereum/go-ethereum/rlp"
 )
 
@@ -51,6 +52,18 @@ func RegisterExtras[SA any]() ExtraPayloads[SA] {
 	return extra
 }
 
+// TestOnlyClearRegisteredExtras clears the [Extras] previously passed to
+// [RegisterExtras]. It panics if called from a non-testing call stack.
+//
+// In tests it SHOULD be called before every call to [RegisterExtras] and then
+// defer-called afterwards, either directly or via testing.TB.Cleanup(). This is
+// a workaround for the single-call limitation on [RegisterExtras].
+func TestOnlyClearRegisteredExtras() {
+	testonly.OrPanic(func() {
+		registeredExtras = nil
+	})
+}
+
 var registeredExtras *extraConstructors
 
 type extraConstructors struct {
@@ -126,6 +139,27 @@ func (e *StateAccountExtra) payload() *pseudo.Type {
 	return e.t
 }
 
+// Equal reports whether `e` is semantically equivalent to `f` for the purpose
+// of tests.
+//
+// Equal MUST NOT be used in production. Instead, compare values returned by
+// [ExtraPayloads.FromStateAccount].
+func (e *StateAccountExtra) Equal(f *StateAccountExtra) bool {
+	if false {
+		// TODO(arr4n): calling this results in an error from cmp.Diff():
+		// "non-deterministic or non-symmetric function detected". Explore the
+		// issue and then enable the enforcement.
+		testonly.OrPanic(func() {})
+	}
+
+	eNil := e == nil || e.t == nil
+	fNil := f == nil || f.t == nil
+	if eNil && fNil || eNil && f.t.IsZero() || fNil && e.t.IsZero() {
+		return true
+	}
+	return e.t.Equal(f.t)
+}
+
 var _ interface {
 	rlp.Encoder
 	rlp.Decoder

core/types/state_account.libevm_test.go

@@ -30,15 +30,6 @@ import (
 	"github.com/ethereum/go-ethereum/rlp"
 )
 
-func (e *StateAccountExtra) Equal(f *StateAccountExtra) bool {
-	eNil := e == nil || e.t == nil
-	fNil := f == nil || f.t == nil
-	if eNil && fNil || eNil && f.t.IsZero() || fNil && e.t.IsZero() {
-		return true
-	}
-	return e.t.Equal(f.t)
-}
-
 func TestStateAccountRLP(t *testing.T) {
 	// RLP encodings that don't involve extra payloads were generated on raw
 	// geth StateAccounts *before* any libevm modifications, thus locking in
@@ -123,11 +114,9 @@ func TestStateAccountRLP(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			if tt.register != nil {
-				registeredExtras = nil
+				TestOnlyClearRegisteredExtras()
 				tt.register()
-				t.Cleanup(func() {
+				t.Cleanup(TestOnlyClearRegisteredExtras)
-					registeredExtras = nil
-				})
 			}
 			assertRLPEncodingAndReturn(t, tt.acc, tt.wantHex)
 

core/types/state_account_storage.libevm_test.go

@@ -0,0 +1,153 @@
+// Copyright 2024 the libevm authors.
+//
+// The libevm additions to go-ethereum are free software: you can redistribute
+// them and/or modify them under the terms of the GNU Lesser General Public License
+// as published by the Free Software Foundation, either version 3 of the License,
+// or (at your option) any later version.
+//
+// The libevm additions are distributed in the hope that they will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
+// General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see
+// <http://www.gnu.org/licenses/>.
+
+package types_test
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/holiman/uint256"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/rawdb"
+	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/libevm/ethtest"
+	"github.com/ethereum/go-ethereum/trie"
+	"github.com/ethereum/go-ethereum/triedb"
+)
+
+func TestStateAccountExtraViaTrieStorage(t *testing.T) {
+	rng := ethtest.NewPseudoRand(1984)
+	addr := rng.Address()
+
+	type arbitraryPayload struct {
+		Data string
+	}
+	const arbitraryData = "Hello, RLP world!"
+
+	var (
+		// The specific trie hashes after inserting the account are irrelevant;
+		// what's important is that: (a) they are all different; and (b) tests
+		// of implicit and explicit zero-value payloads have the same hash.
+		vanillaGeth = common.HexToHash("0x2108846aaec8a88cfa02887527ad8c1beffc11b5ec428b68f15d9ce4e71e4ce1")
+		trueBool    = common.HexToHash("0x665576885e52711e4cf90b72750fc1c17c80c5528bc54244e327414d486a10a4")
+		falseBool   = common.HexToHash("0xa53fcb27d01347e202fb092d0af2a809cb84390c6001cbc151052ee29edc2294")
+		arbitrary   = common.HexToHash("0x94eecff1444ab69437636630918c15596e001b30b973f03e06006ae20aa6e307")
+	)
+
+	tests := []struct {
+		name                string
+		registerAndSetExtra func(*types.StateAccount) *types.StateAccount
+		assertExtra         func(*testing.T, *types.StateAccount)
+		wantTrieHash        common.Hash
+	}{
+		{
+			name: "vanilla geth",
+			registerAndSetExtra: func(a *types.StateAccount) *types.StateAccount {
+				return a
+			},
+			assertExtra: func(t *testing.T, a *types.StateAccount) {
+				t.Helper()
+				assert.Truef(t, a.Extra.Equal(nil), "%T.%T.IsEmpty()", a, a.Extra)
+			},
+			wantTrieHash: vanillaGeth,
+		},
+		{
+			name: "true-boolean payload",
+			registerAndSetExtra: func(a *types.StateAccount) *types.StateAccount {
+				types.RegisterExtras[bool]().SetOnStateAccount(a, true)
+				return a
+			},
+			assertExtra: func(t *testing.T, sa *types.StateAccount) {
+				t.Helper()
+				assert.Truef(t, types.ExtraPayloads[bool]{}.FromStateAccount(sa), "")
+			},
+			wantTrieHash: trueBool,
+		},
+		{
+			name: "explicit false-boolean payload",
+			registerAndSetExtra: func(a *types.StateAccount) *types.StateAccount {
+				p := types.RegisterExtras[bool]()
+				p.SetOnStateAccount(a, false) // the explicit part
+				return a
+			},
+			assertExtra: func(t *testing.T, sa *types.StateAccount) {
+				t.Helper()
+				assert.Falsef(t, types.ExtraPayloads[bool]{}.FromStateAccount(sa), "")
+			},
+			wantTrieHash: falseBool,
+		},
+		{
+			name: "implicit false-boolean payload",
+			registerAndSetExtra: func(a *types.StateAccount) *types.StateAccount {
+				types.RegisterExtras[bool]()
+				// Note that `a` is reflected, unchanged (the implicit part).
+				return a
+			},
+			assertExtra: func(t *testing.T, sa *types.StateAccount) {
+				t.Helper()
+				assert.Falsef(t, types.ExtraPayloads[bool]{}.FromStateAccount(sa), "")
+			},
+			wantTrieHash: falseBool,
+		},
+		{
+			name: "arbitrary payload",
+			registerAndSetExtra: func(a *types.StateAccount) *types.StateAccount {
+				p := arbitraryPayload{arbitraryData}
+				types.RegisterExtras[arbitraryPayload]().SetOnStateAccount(a, p)
+				return a
+			},
+			assertExtra: func(t *testing.T, sa *types.StateAccount) {
+				t.Helper()
+				got := types.ExtraPayloads[arbitraryPayload]{}.FromStateAccount(sa)
+				assert.Equalf(t, arbitraryPayload{arbitraryData}, got, "")
+			},
+			wantTrieHash: arbitrary,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			types.TestOnlyClearRegisteredExtras()
+			t.Cleanup(types.TestOnlyClearRegisteredExtras)
+
+			acct := tt.registerAndSetExtra(&types.StateAccount{
+				Nonce:    42,
+				Balance:  uint256.NewInt(314159),
+				Root:     types.EmptyRootHash,
+				CodeHash: types.EmptyCodeHash[:],
+			})
+
+			db := triedb.NewDatabase(rawdb.NewMemoryDatabase(), nil)
+			id := trie.TrieID(types.EmptyRootHash)
+			state, err := trie.NewStateTrie(id, db)
+			require.NoError(t, err, "trie.NewStateTrie(types.EmptyRootHash, ...)")
+
+			require.NoErrorf(t, state.UpdateAccount(addr, acct), "%T.UpdateAccount(...)", state)
+			assert.Equalf(t, tt.wantTrieHash, state.Hash(), "%T.Hash() after UpdateAccount()", state)
+
+			got, err := state.GetAccount(addr)
+			require.NoError(t, err, "state.GetAccount({account updated earlier})")
+			if diff := cmp.Diff(acct, got); diff != "" {
+				t.Errorf("%T.GetAccount() not equal to value passed to %[1]T.UpdateAccount(); diff (-want +got):\n%s", state, diff)
+			}
+			tt.assertExtra(t, got)
+		})
+	}
+}

libevm/testonly/testonly.go

@@ -0,0 +1,40 @@
+// Copyright 2024 the libevm authors.
+//
+// The libevm additions to go-ethereum are free software: you can redistribute
+// them and/or modify them under the terms of the GNU Lesser General Public License
+// as published by the Free Software Foundation, either version 3 of the License,
+// or (at your option) any later version.
+//
+// The libevm additions are distributed in the hope that they will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
+// General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see
+// <http://www.gnu.org/licenses/>.
+
+// Package testonly enforces functionality that MUST be limited to tests.
+package testonly
+
+import (
+	"runtime"
+	"strings"
+)
+
+// OrPanic runs `fn` i.f.f. called from within a testing environment.
+func OrPanic(fn func()) {
+	pc := make([]uintptr, 64)
+	runtime.Callers(0, pc)
+	frames := runtime.CallersFrames(pc)
+	for {
+		f, more := frames.Next()
+		if strings.Contains(f.File, "/testing/") || strings.HasSuffix(f.File, "_test.go") {
+			fn()
+			return
+		}
+		if !more {
+			panic("no _test.go file in call stack")
+		}
+	}
+}

params/config.libevm.go

@@ -19,10 +19,9 @@ import (
 	"fmt"
 	"math/big"
 	"reflect"
-	"runtime"
-	"strings"
 
 	"github.com/ethereum/go-ethereum/libevm/pseudo"
+	"github.com/ethereum/go-ethereum/libevm/testonly"
 )
 
 // Extras are arbitrary payloads to be added as extra fields in [ChainConfig]
@@ -92,19 +91,9 @@ func RegisterExtras[C ChainConfigHooks, R RulesHooks](e Extras[C, R]) ExtraPaylo
 // defer-called afterwards, either directly or via testing.TB.Cleanup(). This is
 // a workaround for the single-call limitation on [RegisterExtras].
 func TestOnlyClearRegisteredExtras() {
-	pc := make([]uintptr, 10)
+	testonly.OrPanic(func() {
-	runtime.Callers(0, pc)
+		registeredExtras = nil
-	frames := runtime.CallersFrames(pc)
+	})
-	for {
-		f, more := frames.Next()
-		if strings.Contains(f.File, "/testing/") || strings.HasSuffix(f.File, "_test.go") {
-			registeredExtras = nil
-			return
-		}
-		if !more {
-			panic("no _test.go file in call stack")
-		}
-	}
 }
 
 // registeredExtras holds non-generic constructors for the [Extras] types