Skip to content

Commit

Permalink
cmd/evm, core/state: fix post-exec dump of state (statetests, blockch…
Browse files Browse the repository at this point in the history 
…aintests) (ethereum#28504)

There were several problems related to dumping state. 

- If a preimage was missing, even if we had set the `OnlyWithAddresses` to `false`, to export them anyway, the way the mapping was constructed (using `common.Address` as key) made the entries get lost anyway. Concerns both state- and blockchain tests. 
- Blockchain test execution was not configured to store preimages.

This changes makes it so that the block test executor takes a callback, just like the state test executor already does. This callback can be used to examine the post-execution state, e.g. to aid debugging of test failures.
  • Loading branch information
@holiman @Dergarcon
holiman authored and Dergarcon committed Jan 31, 2024
1 parent 2db82a7 commit 61275cb
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 99 deletions.
9 changes: 8 additions & 1 deletion cmd/evm/blockrunner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"regexp"
"sort"

"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/rawdb"
"github.com/ethereum/go-ethereum/core/vm"
"github.com/ethereum/go-ethereum/eth/tracers/logger"
Expand Down Expand Up @@ -85,7 +86,13 @@ func blockTestCmd(ctx *cli.Context) error {
continue
}
test := tests[name]
if err := test.Run(false, rawdb.HashScheme, tracer); err != nil {
if err := test.Run(false, rawdb.HashScheme, tracer, func(res error, chain *core.BlockChain) {
if ctx.Bool(DumpFlag.Name) {
if state, _ := chain.State(); state != nil {
fmt.Println(string(state.Dump(nil)))
}
}
}); err != nil {
return fmt.Errorf("test %v: %w", name, err)
}
}
Expand Down
5 changes: 0 additions & 5 deletions cmd/geth/chaincmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,11 +473,6 @@ func dump(ctx *cli.Context) error {
if ctx.Bool(utils.IterativeOutputFlag.Name) {
state.IterativeDump(conf, json.NewEncoder(os.Stdout))
} else {
if conf.OnlyWithAddresses {
fmt.Fprintf(os.Stderr, "If you want to include accounts with missing preimages, you need iterative output, since"+
" otherwise the accounts will overwrite each other in the resulting mapping.")
return errors.New("incompatible options")
}
fmt.Println(string(state.Dump(conf)))
}
return nil
Expand Down
10 changes: 5 additions & 5 deletions cmd/geth/snapshot.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,11 +580,11 @@ func dumpState(ctx *cli.Context) error {
return err
}
da := &state.DumpAccount{
Balance: account.Balance.String(),
Nonce: account.Nonce,
Root: account.Root.Bytes(),
CodeHash: account.CodeHash,
SecureKey: accIt.Hash().Bytes(),
Balance: account.Balance.String(),
Nonce: account.Nonce,
Root: account.Root.Bytes(),
CodeHash: account.CodeHash,
AddressHash: accIt.Hash().Bytes(),
}
if !conf.SkipCode && !bytes.Equal(account.CodeHash, types.EmptyCodeHash.Bytes()) {
da.Code = rawdb.ReadCode(db, common.BytesToHash(account.CodeHash))
Expand Down
100 changes: 40 additions & 60 deletions core/state/dump.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,24 @@ type DumpCollector interface {

// DumpAccount represents an account in the state.
type DumpAccount struct {
Balance string `json:"balance"`
Nonce uint64 `json:"nonce"`
Root hexutil.Bytes `json:"root"`
CodeHash hexutil.Bytes `json:"codeHash"`
Code hexutil.Bytes `json:"code,omitempty"`
Storage map[common.Hash]string `json:"storage,omitempty"`
Address *common.Address `json:"address,omitempty"` // Address only present in iterative (line-by-line) mode
SecureKey hexutil.Bytes `json:"key,omitempty"` // If we don't have address, we can output the key
Balance string `json:"balance"`
Nonce uint64 `json:"nonce"`
Root hexutil.Bytes `json:"root"`
CodeHash hexutil.Bytes `json:"codeHash"`
Code hexutil.Bytes `json:"code,omitempty"`
Storage map[common.Hash]string `json:"storage,omitempty"`
Address *common.Address `json:"address,omitempty"` // Address only present in iterative (line-by-line) mode
AddressHash hexutil.Bytes `json:"key,omitempty"` // If we don't have address, we can output the key

}

// Dump represents the full dump in a collected format, as one large map.
type Dump struct {
Root string `json:"root"`
Accounts map[common.Address]DumpAccount `json:"accounts"`
Root string `json:"root"`
Accounts map[string]DumpAccount `json:"accounts"`
// Next can be set to represent that this dump is only partial, and Next
// is where an iterator should be positioned in order to continue the dump.
Next []byte `json:"next,omitempty"` // nil if no more accounts
}

// OnRoot implements DumpCollector interface
Expand All @@ -73,27 +76,11 @@ func (d *Dump) OnRoot(root common.Hash) {

// OnAccount implements DumpCollector interface
func (d *Dump) OnAccount(addr *common.Address, account DumpAccount) {
if addr != nil {
d.Accounts[*addr] = account
if addr == nil {
d.Accounts[fmt.Sprintf("pre(%s)", account.AddressHash)] = account
}
}

// IteratorDump is an implementation for iterating over data.
type IteratorDump struct {
Root string `json:"root"`
Accounts map[common.Address]DumpAccount `json:"accounts"`
Next []byte `json:"next,omitempty"` // nil if no more accounts
}

// OnRoot implements DumpCollector interface
func (d *IteratorDump) OnRoot(root common.Hash) {
d.Root = fmt.Sprintf("%x", root)
}

// OnAccount implements DumpCollector interface
func (d *IteratorDump) OnAccount(addr *common.Address, account DumpAccount) {
if addr != nil {
d.Accounts[*addr] = account
d.Accounts[(*addr).String()] = account
}
}

Expand All @@ -105,14 +92,14 @@ type iterativeDump struct {
// OnAccount implements DumpCollector interface
func (d iterativeDump) OnAccount(addr *common.Address, account DumpAccount) {
dumpAccount := &DumpAccount{
Balance: account.Balance,
Nonce: account.Nonce,
Root: account.Root,
CodeHash: account.CodeHash,
Code: account.Code,
Storage: account.Storage,
SecureKey: account.SecureKey,
Address: addr,
Balance: account.Balance,
Nonce: account.Nonce,
Root: account.Root,
CodeHash: account.CodeHash,
Code: account.Code,
Storage: account.Storage,
AddressHash: account.AddressHash,
Address: addr,
}
d.Encode(dumpAccount)
}
Expand Down Expand Up @@ -150,26 +137,27 @@ func (s *StateDB) DumpToCollector(c DumpCollector, conf *DumpConfig) (nextKey []
if err := rlp.DecodeBytes(it.Value, &data); err != nil {
panic(err)
}
account := DumpAccount{
Balance: data.Balance.String(),
Nonce: data.Nonce,
Root: data.Root[:],
CodeHash: data.CodeHash,
SecureKey: it.Key,
}
var (
addrBytes = s.trie.GetKey(it.Key)
addr = common.BytesToAddress(addrBytes)
account = DumpAccount{
Balance: data.Balance.String(),
Nonce: data.Nonce,
Root: data.Root[:],
CodeHash: data.CodeHash,
AddressHash: it.Key,
}
address *common.Address
addr common.Address
addrBytes = s.trie.GetKey(it.Key)
)
if addrBytes == nil {
// Preimage missing
missingPreimages++
if conf.OnlyWithAddresses {
continue
}
} else {
addr = common.BytesToAddress(addrBytes)
address = &addr
account.Address = address
}
obj := newObject(s, addr, &data)
if !conf.SkipCode {
Expand Down Expand Up @@ -220,12 +208,13 @@ func (s *StateDB) DumpToCollector(c DumpCollector, conf *DumpConfig) (nextKey []
return nextKey
}

// RawDump returns the entire state an a single large object
// RawDump returns the state. If the processing is aborted e.g. due to options
// reaching Max, the `Next` key is set on the returned Dump.
func (s *StateDB) RawDump(opts *DumpConfig) Dump {
dump := &Dump{
Accounts: make(map[common.Address]DumpAccount),
Accounts: make(map[string]DumpAccount),
}
s.DumpToCollector(dump, opts)
dump.Next = s.DumpToCollector(dump, opts)
return *dump
}

Expand All @@ -234,7 +223,7 @@ func (s *StateDB) Dump(opts *DumpConfig) []byte {
dump := s.RawDump(opts)
json, err := json.MarshalIndent(dump, "", " ")
if err != nil {
fmt.Println("Dump err", err)
log.Error("Error dumping state", "err", err)
}
return json
}
Expand All @@ -243,12 +232,3 @@ func (s *StateDB) Dump(opts *DumpConfig) []byte {
func (s *StateDB) IterativeDump(opts *DumpConfig, output *json.Encoder) {
s.DumpToCollector(iterativeDump{output}, opts)
}

// IteratorDump dumps out a batch of accounts starts with the given start key
func (s *StateDB) IteratorDump(opts *DumpConfig) IteratorDump {
iterator := &IteratorDump{
Accounts: make(map[common.Address]DumpAccount),
}
iterator.Next = s.DumpToCollector(iterator, opts)
return *iterator
}
3 changes: 3 additions & 0 deletions core/state/state_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,15 @@ func TestDump(t *testing.T) {
"nonce": 0,
"root": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"codeHash": "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
"address": "0x0000000000000000000000000000000000000001",
"key": "0x1468288056310c82aa4c01a7e12a10f8111a0560e72b700555479031b86c357d"
},
"0x0000000000000000000000000000000000000002": {
"balance": "44",
"nonce": 0,
"root": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"codeHash": "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
"address": "0x0000000000000000000000000000000000000002",
"key": "0xd52688a8f926c816ca1e079067caba944f158e764817b83fc43594370ca9cf62"
},
"0x0000000000000000000000000000000000000102": {
Expand All @@ -86,6 +88,7 @@ func TestDump(t *testing.T) {
"root": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"codeHash": "0x87874902497a5bb968da31a2998d8f22e949d1ef6214bcdedd8bae24cca4b9e3",
"code": "0x03030303030303",
"address": "0x0000000000000000000000000000000000000102",
"key": "0xa17eacbc25cda025e81db9c5c62868822c73ce097cee2a63e33a2e41268358a1"
}
}
Expand Down
18 changes: 9 additions & 9 deletions eth/api_debug.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (api *DebugAPI) GetBadBlocks(ctx context.Context) ([]*BadBlockArgs, error)
const AccountRangeMaxResults = 256

// AccountRange enumerates all accounts in the given block and start point in paging request
func (api *DebugAPI) AccountRange(blockNrOrHash rpc.BlockNumberOrHash, start hexutil.Bytes, maxResults int, nocode, nostorage, incompletes bool) (state.IteratorDump, error) {
func (api *DebugAPI) AccountRange(blockNrOrHash rpc.BlockNumberOrHash, start hexutil.Bytes, maxResults int, nocode, nostorage, incompletes bool) (state.Dump, error) {
var stateDb *state.StateDB
var err error

Expand All @@ -144,7 +144,7 @@ func (api *DebugAPI) AccountRange(blockNrOrHash rpc.BlockNumberOrHash, start hex
// the miner and operate on those
_, stateDb = api.eth.miner.Pending()
if stateDb == nil {
return state.IteratorDump{}, errors.New("pending state is not available")
return state.Dump{}, errors.New("pending state is not available")
}
} else {
var header *types.Header
Expand All @@ -158,29 +158,29 @@ func (api *DebugAPI) AccountRange(blockNrOrHash rpc.BlockNumberOrHash, start hex
default:
block := api.eth.blockchain.GetBlockByNumber(uint64(number))
if block == nil {
return state.IteratorDump{}, fmt.Errorf("block #%d not found", number)
return state.Dump{}, fmt.Errorf("block #%d not found", number)
}
header = block.Header()
}
if header == nil {
return state.IteratorDump{}, fmt.Errorf("block #%d not found", number)
return state.Dump{}, fmt.Errorf("block #%d not found", number)
}
stateDb, err = api.eth.BlockChain().StateAt(header.Root)
if err != nil {
return state.IteratorDump{}, err
return state.Dump{}, err
}
}
} else if hash, ok := blockNrOrHash.Hash(); ok {
block := api.eth.blockchain.GetBlockByHash(hash)
if block == nil {
return state.IteratorDump{}, fmt.Errorf("block %s not found", hash.Hex())
return state.Dump{}, fmt.Errorf("block %s not found", hash.Hex())
}
stateDb, err = api.eth.BlockChain().StateAt(block.Root())
if err != nil {
return state.IteratorDump{}, err
return state.Dump{}, err
}
} else {
return state.IteratorDump{}, errors.New("either block number or block hash must be specified")
return state.Dump{}, errors.New("either block number or block hash must be specified")
}

opts := &state.DumpConfig{
Expand All @@ -193,7 +193,7 @@ func (api *DebugAPI) AccountRange(blockNrOrHash rpc.BlockNumberOrHash, start hex
if maxResults > AccountRangeMaxResults || maxResults <= 0 {
opts.Max = AccountRangeMaxResults
}
return stateDb.IteratorDump(opts), nil
return stateDb.RawDump(opts), nil
}

// StorageRangeResult is the result of a debug_storageRangeAt API call.
Expand Down
25 changes: 13 additions & 12 deletions eth/api_debug_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"math/big"
"reflect"
"strings"
"testing"

"github.com/davecgh/go-spew/spew"
Expand All @@ -35,8 +36,8 @@ import (

var dumper = spew.ConfigState{Indent: " "}

func accountRangeTest(t *testing.T, trie *state.Trie, statedb *state.StateDB, start common.Hash, requestedNum int, expectedNum int) state.IteratorDump {
result := statedb.IteratorDump(&state.DumpConfig{
func accountRangeTest(t *testing.T, trie *state.Trie, statedb *state.StateDB, start common.Hash, requestedNum int, expectedNum int) state.Dump {
result := statedb.RawDump(&state.DumpConfig{
SkipCode: true,
SkipStorage: true,
OnlyWithAddresses: false,
Expand All @@ -47,12 +48,12 @@ func accountRangeTest(t *testing.T, trie *state.Trie, statedb *state.StateDB, st
if len(result.Accounts) != expectedNum {
t.Fatalf("expected %d results, got %d", expectedNum, len(result.Accounts))
}
for address := range result.Accounts {
if address == (common.Address{}) {
t.Fatalf("empty address returned")
for addr, acc := range result.Accounts {
if strings.HasSuffix(addr, "pre") || acc.Address == nil {
t.Fatalf("account without prestate (address) returned: %v", addr)
}
if !statedb.Exist(address) {
t.Fatalf("account not found in state %s", address.Hex())
if !statedb.Exist(*acc.Address) {
t.Fatalf("account not found in state %s", acc.Address.Hex())
}
}
return result
Expand Down Expand Up @@ -92,16 +93,16 @@ func TestAccountRange(t *testing.T) {
secondResult := accountRangeTest(t, &trie, sdb, common.BytesToHash(firstResult.Next), AccountRangeMaxResults, AccountRangeMaxResults)

hList := make([]common.Hash, 0)
for addr1 := range firstResult.Accounts {
// If address is empty, then it makes no sense to compare
for addr1, acc := range firstResult.Accounts {
// If address is non-available, then it makes no sense to compare
// them as they might be two different accounts.
if addr1 == (common.Address{}) {
if acc.Address == nil {
continue
}
if _, duplicate := secondResult.Accounts[addr1]; duplicate {
t.Fatalf("pagination test failed: results should not overlap")
}
hList = append(hList, crypto.Keccak256Hash(addr1.Bytes()))
hList = append(hList, crypto.Keccak256Hash(acc.Address.Bytes()))
}
// Test to see if it's possible to recover from the middle of the previous
// set and get an even split between the first and second sets.
Expand Down Expand Up @@ -140,7 +141,7 @@ func TestEmptyAccountRange(t *testing.T) {
st.Commit(0, true)
st, _ = state.New(types.EmptyRootHash, statedb, nil)

results := st.IteratorDump(&state.DumpConfig{
results := st.RawDump(&state.DumpConfig{
SkipCode: true,
SkipStorage: true,
OnlyWithAddresses: true,
Expand Down
8 changes: 4 additions & 4 deletions tests/block_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,19 @@ func TestExecutionSpec(t *testing.T) {
}

func execBlockTest(t *testing.T, bt *testMatcher, test *BlockTest) {
if err := bt.checkFailure(t, test.Run(false, rawdb.HashScheme, nil)); err != nil {
if err := bt.checkFailure(t, test.Run(false, rawdb.HashScheme, nil, nil)); err != nil {
t.Errorf("test in hash mode without snapshotter failed: %v", err)
return
}
if err := bt.checkFailure(t, test.Run(true, rawdb.HashScheme, nil)); err != nil {
if err := bt.checkFailure(t, test.Run(true, rawdb.HashScheme, nil, nil)); err != nil {
t.Errorf("test in hash mode with snapshotter failed: %v", err)
return
}
if err := bt.checkFailure(t, test.Run(false, rawdb.PathScheme, nil)); err != nil {
if err := bt.checkFailure(t, test.Run(false, rawdb.PathScheme, nil, nil)); err != nil {
t.Errorf("test in path mode without snapshotter failed: %v", err)
return
}
if err := bt.checkFailure(t, test.Run(true, rawdb.PathScheme, nil)); err != nil {
if err := bt.checkFailure(t, test.Run(true, rawdb.PathScheme, nil, nil)); err != nil {
t.Errorf("test in path mode with snapshotter failed: %v", err)
return
}
Expand Down
Loading

0 comments on commit 61275cb

Please sign in to comment.