Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions core/types/rlp_payload.libevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was moved from the test file, unchanged.

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
Expand Down
15 changes: 2 additions & 13 deletions core/types/state_account.libevm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() {
registeredExtras = nil
})
t.Cleanup(TestOnlyClearRegisteredExtras)
}
assertRLPEncodingAndReturn(t, tt.acc, tt.wantHex)

Expand Down
153 changes: 153 additions & 0 deletions core/types/state_account_storage.libevm_test.go
Original file line number Diff line number Diff line change
@@ -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)
Comment on lines +142 to +143
Copy link
Collaborator

@darioush darioush Oct 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can can we also add testing for Copy & Snapshot/Revert being handled properly?
For example statedb.Copy makes a shallow copy of the accounts modified, so we should ensure that setting the extras on the origin does not impact the copied statedb & the other way around.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Snapshot/Revert covered in #48

I'm about to test Copy too. If it fails then I'll see how much needs to change and might start a new PR if the fix spreads too far.


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)
})
}
}
40 changes: 40 additions & 0 deletions libevm/testonly/testonly.go
Original file line number Diff line number Diff line change
@@ -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()) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was abstracted from params/config.libevm.go, unchanged except for the call to fn() and the size of the pc slice.

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")
}
}
}
19 changes: 4 additions & 15 deletions params/config.libevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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)
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") {
registeredExtras = nil
return
}
if !more {
panic("no _test.go file in call stack")
}
}
testonly.OrPanic(func() {
registeredExtras = nil
})
}

// registeredExtras holds non-generic constructors for the [Extras] types
Expand Down