Skip to content

Commit

Permalink
core/state: make journalling set-based
Browse files Browse the repository at this point in the history
core/state: add handling for DiscardSnapshot
core/state: use new journal
core/state, genesis: fix flaw re discard/commit.

In case the state is committed, the journal is reset, thus it is not correct to Discard/Revert snapshots at that point.

core/state: fix nil defer in merge
core/state: fix bugs in setjournal
core/state: journal api changes
core/state: bugfixes in sparse journal
core/state: journal tests
core/state: improve post-state check in journal-fuzzing test
core/state: post-rebase fixups
miner: remove discard-snapshot call, it's not needed since journal will be reset in Finalize
core/state: fix tests
  • Loading branch information
holiman committed Sep 24, 2024
1 parent 3140738 commit bf1f7a8
Show file tree
Hide file tree
Showing 13 changed files with 739 additions and 59 deletions.
1 change: 1 addition & 0 deletions cmd/evm/internal/t8ntool/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig,
}
continue
}
statedb.DiscardSnapshot(snapshot)
includedTxs = append(includedTxs, tx)
if hashError != nil {
return nil, nil, nil, NewError(ErrorMissingBlockhash, hashError)
Expand Down
3 changes: 2 additions & 1 deletion cmd/evm/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ func runCmd(ctx *cli.Context) error {
sdb := state.NewDatabase(triedb, nil)
statedb, _ = state.New(genesis.Root(), sdb)
chainConfig = genesisConfig.Config

id := statedb.Snapshot()
defer statedb.DiscardSnapshot(id)
if ctx.String(SenderFlag.Name) != "" {
sender = common.HexToAddress(ctx.String(SenderFlag.Name))
}
Expand Down
1 change: 1 addition & 0 deletions core/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func flushAlloc(ga *types.GenesisAlloc, triedb *triedb.Database) (common.Hash, e
if err != nil {
return common.Hash{}, err
}

for addr, account := range *ga {
if account.Balance != nil {
// This is not actually logged via tracer because OnGenesisBlock
Expand Down
24 changes: 15 additions & 9 deletions core/state/journal.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ func (j *linearJournal) snapshot() int {
return id
}

// revertToSnapshot reverts all state changes made since the given revision.
func (j *linearJournal) revertToSnapshot(revid int, s *StateDB) {
// Find the snapshot in the stack of valid snapshots.
idx := sort.Search(len(j.validRevisions), func(i int) bool {
Expand All @@ -109,6 +108,13 @@ func (j *linearJournal) revertToSnapshot(revid int, s *StateDB) {
j.validRevisions = j.validRevisions[:idx]
}

// DiscardSnapshot removes the snapshot with the given id; after calling this
// method, it is no longer possible to revert to that particular snapshot, the
// changes are considered part of the parent scope.
func (j *linearJournal) DiscardSnapshot(id int) {
//
}

// append inserts a new modification entry to the end of the change linearJournal.
func (j *linearJournal) append(entry journalEntry) {
j.entries = append(j.entries, entry)
Expand Down Expand Up @@ -168,11 +174,11 @@ func (j *linearJournal) createObject(addr common.Address) {
j.append(createObjectChange{account: addr})
}

func (j *linearJournal) createContract(addr common.Address) {
func (j *linearJournal) createContract(addr common.Address, account *types.StateAccount) {
j.append(createContractChange{account: addr})
}

func (j *linearJournal) destruct(addr common.Address) {
func (j *linearJournal) destruct(addr common.Address, account *types.StateAccount) {
j.append(selfDestructChange{account: addr})
}

Expand All @@ -197,25 +203,25 @@ func (j *linearJournal) refundChange(previous uint64) {
j.append(refundChange{prev: previous})
}

func (j *linearJournal) balanceChange(addr common.Address, previous *uint256.Int) {
func (j *linearJournal) balanceChange(addr common.Address, account *types.StateAccount, destructed, newContract bool) {
j.append(balanceChange{
account: addr,
prev: previous.Clone(),
prev: account.Balance.Clone(),
})
}

func (j *linearJournal) setCode(address common.Address) {
func (j *linearJournal) setCode(address common.Address, account *types.StateAccount) {
j.append(codeChange{account: address})
}

func (j *linearJournal) nonceChange(address common.Address, prev uint64) {
func (j *linearJournal) nonceChange(address common.Address, account *types.StateAccount, destructed, newContract bool) {
j.append(nonceChange{
account: address,
prev: prev,
prev: account.Nonce,
})
}

func (j *linearJournal) touchChange(address common.Address) {
func (j *linearJournal) touchChange(address common.Address, account *types.StateAccount, destructed, newContract bool) {
j.append(touchChange{
account: address,
})
Expand Down
39 changes: 27 additions & 12 deletions core/state/journal_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,22 @@ package state

import (
"github.com/ethereum/go-ethereum/common"
"github.com/holiman/uint256"
"github.com/ethereum/go-ethereum/core/types"
)

type journal interface {

// snapshot returns an identifier for the current revision of the state.
// The lifeycle of journalling is as follows:
// - snapshot() starts a 'scope'.
// - Tee method snapshot() may be called any number of times.
// - For each call to snapshot, there should be a corresponding call to end
// the scope via either of:
// - revertToSnapshot, which undoes the changes in the scope, or
// - discardSnapshot, which discards the ability to revert the changes in the scope.
// - This operation might merge the changes into the parent scope.
// If it does not merge the changes into the parent scope, it must create
// a new snapshot internally, in order to ensure that order of changes
// remains intact.
snapshot() int

// revertToSnapshot reverts all state changes made since the given revision.
Expand All @@ -16,6 +26,11 @@ type journal interface {
// reset clears the journal so it can be reused.
reset()

// DiscardSnapshot removes the snapshot with the given id; after calling this
// method, it is no longer possible to revert to that particular snapshot, the
// changes are considered part of the parent scope.
DiscardSnapshot(revid int)

// dirtyAccounts returns a list of all accounts modified in this journal
dirtyAccounts() []common.Address

Expand All @@ -34,12 +49,12 @@ type journal interface {
// createContract journals the creation of a new contract at addr.
// OBS: This method must not be applied twice, it assumes that the pre-state
// (i.e the rollback-state) is non-created.
createContract(addr common.Address)
createContract(addr common.Address, account *types.StateAccount)

// destruct journals the destruction of an account in the trie.
// OBS: This method must not be applied twice -- it always assumes that the
// pre-state (i.e the rollback-state) is non-destructed.
destruct(addr common.Address)
// pre-state (i.e the rollback-state) is non-destructed (and, for the purpose
// of EIP-XXX (TODO lookup), created in this tx).
destruct(addr common.Address, account *types.StateAccount)

// storageChange journals a change in the storage data related to addr.
// It records the key and previous value of the slot.
Expand All @@ -52,19 +67,19 @@ type journal interface {
// refundChange journals that the refund has been changed, recording the previous value.
refundChange(previous uint64)

// balanceChange journals tha the balance of addr has been changed, recording the previous value
balanceChange(addr common.Address, previous *uint256.Int)
// balanceChange journals that the balance of addr has been changed, recording the previous value
balanceChange(addr common.Address, account *types.StateAccount, destructed, newContract bool)

// JournalSetCode journals that the code of addr has been set.
// setCode journals that the code of addr has been set.
// OBS: This method must not be applied twice -- it always assumes that the
// pre-state (i.e the rollback-state) is "no code".
setCode(addr common.Address)
setCode(addr common.Address, account *types.StateAccount)

// nonceChange journals that the nonce of addr was changed, recording the previous value.
nonceChange(addr common.Address, prev uint64)
nonceChange(addr common.Address, account *types.StateAccount, destructed, newContract bool)

// touchChange journals that the account at addr was touched during execution.
touchChange(addr common.Address)
touchChange(addr common.Address, account *types.StateAccount, destructed, newContract bool)

// copy returns a deep-copied journal.
copy() journal
Expand Down
134 changes: 134 additions & 0 deletions core/state/journal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Copyright 2024 The go-ethereum Authors
// This file is part of the go-ethereum library.
//
// The go-ethereum library is free software: you can redistribute it and/or modify
// it 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 go-ethereum library is distributed in the hope that it 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 state provides a caching layer atop the Ethereum state trie.
package state

import (
"testing"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
"github.com/holiman/uint256"
)

func TestLinearJournalDirty(t *testing.T) {
testJournalDirty(t, newLinearJournal())
}

func TestSparseJournalDirty(t *testing.T) {
testJournalDirty(t, newSparseJournal())
}

// This test verifies some basics around journalling: the ability to
// deliver a dirty-set.
func testJournalDirty(t *testing.T, j journal) {
acc := &types.StateAccount{
Nonce: 1,
Balance: new(uint256.Int),
Root: common.Hash{},
CodeHash: nil,
}
{
j.nonceChange(common.Address{0x1}, acc, false, false)
if have, want := len(j.dirtyAccounts()), 1; have != want {
t.Errorf("wrong size of dirty accounts, have %v want %v", have, want)
}
}
{
j.storageChange(common.Address{0x2}, common.Hash{0x1}, common.Hash{0x1}, common.Hash{})
if have, want := len(j.dirtyAccounts()), 2; have != want {
t.Errorf("wrong size of dirty accounts, have %v want %v", have, want)
}
}
{ // The previous scopes should also be accounted for
j.snapshot()
if have, want := len(j.dirtyAccounts()), 2; have != want {
t.Errorf("wrong size of dirty accounts, have %v want %v", have, want)
}
}
}

func TestLinearJournalAccessList(t *testing.T) {
testJournalAccessList(t, newLinearJournal())
}

func TestSparseJournalAccessList(t *testing.T) {
testJournalAccessList(t, newSparseJournal())
}

func testJournalAccessList(t *testing.T, j journal) {
var statedb = &StateDB{}
statedb.accessList = newAccessList()
statedb.journal = j

{
// If the journal performs the rollback in the wrong order, this
// will cause a panic.
id := j.snapshot()
statedb.AddSlotToAccessList(common.Address{0x1}, common.Hash{0x4})
statedb.AddSlotToAccessList(common.Address{0x3}, common.Hash{0x4})
statedb.RevertToSnapshot(id)

}
{
id := j.snapshot()
statedb.AddAddressToAccessList(common.Address{0x2})
statedb.AddAddressToAccessList(common.Address{0x3})
statedb.AddAddressToAccessList(common.Address{0x4})
statedb.RevertToSnapshot(id)
if statedb.accessList.ContainsAddress(common.Address{0x2}) {
t.Fatal("should be missing")
}

}
}

func TestLinearJournalRefunds(t *testing.T) {
testJournalRefunds(t, newLinearJournal())
}

func TestSparseJournalRefunds(t *testing.T) {
testJournalRefunds(t, newSparseJournal())
}

func testJournalRefunds(t *testing.T, j journal) {
var statedb = &StateDB{}
statedb.accessList = newAccessList()
statedb.journal = j
zero := j.snapshot()
j.refundChange(0)
j.refundChange(1)
{
id := j.snapshot()
j.refundChange(2)
j.refundChange(3)
j.revertToSnapshot(id, statedb)
if have, want := statedb.refund, uint64(2); have != want {
t.Fatalf("have %d want %d", have, want)
}
}
{
id := j.snapshot()
j.refundChange(2)
j.refundChange(3)
j.DiscardSnapshot(id)
}
j.revertToSnapshot(zero, statedb)
if have, want := statedb.refund, uint64(0); have != want {
t.Fatalf("have %d want %d", have, want)
}
}
Loading

0 comments on commit bf1f7a8

Please sign in to comment.