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

Recording fixes #169

Merged
merged 4 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
12 changes: 9 additions & 3 deletions arbitrum/recordingdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,21 @@ func (r *RecordingChainContext) GetMinBlockNumberAccessed() uint64 {
return r.minBlockNumberAccessed
}

func PrepareRecording(blockchain *core.BlockChain, lastBlockHeader *types.Header) (*state.StateDB, core.ChainContext, *RecordingKV, error) {
func PrepareRecording(blockchain *core.BlockChain, lastBlockHeader *types.Header, validateDeleted bool) (*state.StateDB, core.ChainContext, *RecordingKV, error) {
rawTrie := blockchain.StateCache().TrieDB()
recordingKeyValue := NewRecordingKV(rawTrie)
recordingStateDatabase := state.NewDatabase(rawdb.NewDatabase(recordingKeyValue))

trieDbConfig := trie.Config{
Cache: 0,
Preimages: true, // default
ValidateDeleted: validateDeleted,
}
recordingStateDatabase := state.NewDatabaseWithConfig(rawdb.NewDatabase(recordingKeyValue), &trieDbConfig)
var prevRoot common.Hash
if lastBlockHeader != nil {
prevRoot = lastBlockHeader.Root
}
recordingStateDb, err := state.New(prevRoot, recordingStateDatabase, nil)
recordingStateDb, err := state.NewDeterministic(prevRoot, recordingStateDatabase)
if err != nil {
return nil, nil, nil, fmt.Errorf("failed to create recordingStateDb: %w", err)
}
Expand Down
21 changes: 19 additions & 2 deletions core/state/state_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"io"
"math/big"
"sort"
"time"

"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -321,6 +322,11 @@ func (s *stateObject) updateTrie(db Database) Trie {
hasher := s.db.hasher

usedStorage := make([][]byte, 0, len(s.pendingStorage))
type hashAndBig struct {
hash common.Hash
big *big.Int
}
keysToDelete := make([]hashAndBig, 0, len(s.pendingStorage))
for key, value := range s.pendingStorage {
// Skip noop changes, persist actual changes
if value == s.originStorage[key] {
Expand All @@ -330,8 +336,12 @@ func (s *stateObject) updateTrie(db Database) Trie {

var v []byte
if (value == common.Hash{}) {
s.setError(tr.TryDelete(key[:]))
s.db.StorageDeleted += 1
if s.db.deterministic {
keysToDelete = append(keysToDelete, hashAndBig{key, key.Big()})
} else {
s.setError(tr.TryDelete(key[:]))
s.db.StorageDeleted += 1
}
} else {
// Encoding []byte cannot fail, ok to ignore the error.
v, _ = rlp.EncodeToBytes(common.TrimLeftZeroes(value[:]))
Expand All @@ -351,6 +361,13 @@ func (s *stateObject) updateTrie(db Database) Trie {
}
usedStorage = append(usedStorage, common.CopyBytes(key[:])) // Copy needed for closure
}
if len(keysToDelete) > 0 {
sort.Slice(keysToDelete, func(i, j int) bool { return keysToDelete[i].big.Cmp(keysToDelete[j].big) < 0 })
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can directly compare common.Hashs as they're a [32]byte right? It's worth a try to simplify this I think.

for _, key := range keysToDelete {
s.setError(tr.TryDelete(key.hash[:]))
s.db.StorageDeleted += 1
}
}
if s.db.prefetcher != nil {
s.db.prefetcher.used(s.addrHash, s.data.Root, usedStorage)
}
Expand Down
34 changes: 31 additions & 3 deletions core/state/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ type StateDB struct {
StorageUpdated int
AccountDeleted int
StorageDeleted int

deterministic bool
}

// New creates a new state from a given trie.
Expand Down Expand Up @@ -164,6 +166,15 @@ func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error)
return sdb, nil
}

func NewDeterministic(root common.Hash, db Database) (*StateDB, error) {
sdb, err := New(root, db, nil)
if err != nil {
return nil, err
}
sdb.deterministic = true
return sdb, nil
}

// StartPrefetcher initializes a new trie prefetcher to pull in nodes from the
// state trie concurrently while the state is mutated so that when we reach the
// commit phase, most of the needed data is already hot.
Expand Down Expand Up @@ -890,9 +901,26 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash {
// the account prefetcher. Instead, let's process all the storage updates
// first, giving the account prefeches just a few more milliseconds of time
// to pull useful data from disk.
for addr := range s.stateObjectsPending {
if obj := s.stateObjects[addr]; !obj.deleted {
obj.updateRoot(s.db)
if s.deterministic {
type addressAndBig struct {
address common.Address
big *big.Int
}
addressesToUpdate := make([]addressAndBig, 0, len(s.stateObjectsPending))
for addr := range s.stateObjectsPending {
addressesToUpdate = append(addressesToUpdate, addressAndBig{addr, addr.Hash().Big()})
}
sort.Slice(addressesToUpdate, func(i, j int) bool { return addressesToUpdate[i].big.Cmp(addressesToUpdate[j].big) < 0 })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing here, I think we can directly compare addresses

for _, addr := range addressesToUpdate {
if obj := s.stateObjects[addr.address]; !obj.deleted {
obj.updateRoot(s.db)
}
}
} else {
for addr := range s.stateObjectsPending {
if obj := s.stateObjects[addr]; !obj.deleted {
obj.updateRoot(s.db)
}
}
}
// Now we're about to start to write changes to the trie. The trie is so far
Expand Down
6 changes: 6 additions & 0 deletions trie/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ type Database struct {
childrenSize common.StorageSize // Storage size of the external children tracking
preimagesSize common.StorageSize // Storage size of the preimages cache

validateDeleted bool // Validate nodes when deleting them

lock sync.RWMutex
}

Expand Down Expand Up @@ -266,6 +268,8 @@ type Config struct {
Cache int // Memory allowance (MB) to use for caching trie nodes in memory
Journal string // Journal of clean cache to survive node restarts
Preimages bool // Flag whether the preimage of trie key is recorded

ValidateDeleted bool // Validate nodes when deleting them
}

// NewDatabase creates a new trie database to store ephemeral trie content before
Expand Down Expand Up @@ -293,6 +297,8 @@ func NewDatabaseWithConfig(diskdb ethdb.KeyValueStore, config *Config) *Database
dirties: map[common.Hash]*cachedNode{{}: {
children: make(map[common.Hash]uint16),
}},

validateDeleted: (config != nil) && config.ValidateDeleted,
}
if config == nil || config.Preimages { // TODO(karalabe): Flip to default off in the future
db.preimages = make(map[common.Hash][]byte)
Expand Down
18 changes: 17 additions & 1 deletion trie/trie.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,13 @@ func (t *Trie) delete(n node, prefix, key []byte) (bool, node, error) {
// need to be tracked at all since it's always embedded.
t.tracer.onDelete(prefix)

return true, nil, nil // remove n entirely for whole matches
var err error

if t.db.validateDeleted {
_, _, err = t.delete(n.Val, append(prefix, key...), []byte{})
}

return true, nil, err // remove n entirely for whole matches
}
// The key is longer than n.Key. Remove the remaining suffix
// from the subtrie. Child can never be nil here since the
Expand Down Expand Up @@ -504,6 +510,16 @@ func (t *Trie) delete(n node, prefix, key []byte) (bool, node, error) {
}
}
}
if t.db.validateDeleted {
for _, child := range n.Children {
if child != nil {
_, err = t.resolve(child, prefix)
if err != nil {
return false, nil, err
}
}
}
}
if pos >= 0 {
if pos != 16 {
// If the remaining entry is a short node, it replaces
Expand Down