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

cmd, consensus, core, eth, tests: return memorized state error #26671

Closed
wants to merge 1 commit into from
Closed
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
5 changes: 3 additions & 2 deletions cmd/evm/internal/t8ntool/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,9 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig,
if chainConfig.IsByzantium(vmContext.BlockNumber) {
statedb.Finalise(true)
} else {
root = statedb.IntermediateRoot(chainConfig.IsEIP158(vmContext.BlockNumber)).Bytes()
// the ignored error will eventually be captured by commit.
hash, _ := statedb.IntermediateRoot(chainConfig.IsEIP158(vmContext.BlockNumber))
root = hash.Bytes()
}

// Create a new receipt for the transaction, storing the intermediate root and
Expand Down Expand Up @@ -231,7 +233,6 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig,

txIndex++
}
statedb.IntermediateRoot(chainConfig.IsEIP158(vmContext.BlockNumber))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you drop this?

Copy link
Member Author

Choose a reason for hiding this comment

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

// Add mining reward? (-1 means rewards are disabled)
if miningReward >= 0 {
// Add mining reward. The mining reward may be `0`, which only makes a difference in the cases
Expand Down
7 changes: 5 additions & 2 deletions cmd/evm/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,11 @@ func runCmd(ctx *cli.Context) error {
output, leftOverGas, stats, err := timedExec(bench, execFunc)

if ctx.Bool(DumpFlag.Name) {
statedb.Commit(true)
statedb.IntermediateRoot(true)
_, err := statedb.Commit(true)
if err != nil {
fmt.Println("failed to commit state changes: ", err)
os.Exit(1)
}
fmt.Println(string(statedb.Dump(nil)))
}

Expand Down
19 changes: 15 additions & 4 deletions cmd/evm/staterunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,21 @@ func stateTestCmd(ctx *cli.Context) error {
_, s, err := test.Run(st, cfg, false)
// print state root for evmlab tracing
if s != nil {
root := s.IntermediateRoot(false)
result.Root = &root
if ctx.Bool(MachineFlag.Name) {
fmt.Fprintf(os.Stderr, "{\"stateRoot\": \"%#x\"}\n", root)
root, err := s.IntermediateRoot(false)
if err != nil {
// Test failed(corrupted state shouldn't actually
// occur during tests though), mark as so and dump
// any state to aid debugging.
result.Pass, result.Error = false, err.Error()
if ctx.Bool(DumpFlag.Name) && s != nil {
dump := s.RawDump(nil)
result.State = &dump
}
} else {
result.Root = &root
if ctx.Bool(MachineFlag.Name) {
fmt.Fprintf(os.Stderr, "{\"stateRoot\": \"%#x\"}\n", root)
}
}
}
if err != nil {
Expand Down
12 changes: 9 additions & 3 deletions consensus/beacon/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,9 +341,6 @@ func (beacon *Beacon) Finalize(chain consensus.ChainHeaderReader, header *types.
amount = amount.Mul(amount, big.NewInt(params.GWei))
state.AddBalance(w.Address, amount)
}
// The block reward is no longer handled here. It's done by the
// external consensus engine.
header.Root = state.IntermediateRoot(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

You removed the IntermediateRoot from Finalize. Finalize is called from FinaliseAndAssemble, where you added a call to IntermediateRoot after the call to Finalize.

Finalize is however also called from state_processor.go:L95, and there you have not added any corresponding call to intermediate root. How come?

Copy link
Member Author

Choose a reason for hiding this comment

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

In state_processor.go:L95, all this function should do is:

  • apply the transactions inside of the block on top on the provided stateDB
  • apply the post-process ethereum rules, e.g. pow block rewards, uncle rules, withdrawals, etc
  • return the generated receipts and logs
    and that all.

header.Root = state.IntermediateRoot(true) is just to derive the hash of the mutated stateDB
and assign it to header. In case of ``state_processor.go:L95`, the block is already generated
and retrieved from the network, we don't need to re-generate/assign the new hash to the header.
What's more, the header is just a copied from the block, it's noop.

In block insertion, the real hash comparison is happened in block validation,

	// Validate the state root against the received state root and throw
	// an error if they don't match.
	root, err := statedb.IntermediateRoot(v.config.IsEIP158(header.Number))
	if err != nil {
		return fmt.Errorf("failed to derive state root %v", err)
	}
	if header.Root != root {
		return fmt.Errorf("invalid merkle root (remote: %x local: %x)", header.Root, root)
	}

Copy link
Member Author

Choose a reason for hiding this comment

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

In summary, the behavior is identical with old code, but just emphasize

  • for block insertion, Finalize is only used to apply extra post-processing rules(e.g. block rewards, withdrawal, etc) but do nothing else.
  • for block generation, FinalizeAndAssemble is required to both do Finalize work and also assemble a block locally, deriving the state hash from the stateDB and wrap it in a block.

}

// FinalizeAndAssemble implements consensus.Engine, setting the final state and
Expand All @@ -367,6 +364,15 @@ func (beacon *Beacon) FinalizeAndAssemble(chain consensus.ChainHeaderReader, hea
}
// Finalize and assemble the block.
beacon.Finalize(chain, header, state, txs, uncles, withdrawals)

// Assign the final state root to header.
root, err := state.IntermediateRoot(true)
if err != nil {
return nil, err
}
header.Root = root

// Assemble and return the final block.
return types.NewBlockWithWithdrawals(header, txs, uncles, receipts, withdrawals, trie.NewStackTrie(nil)), nil
}

Expand Down
14 changes: 9 additions & 5 deletions consensus/clique/clique.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,8 +569,6 @@ func (c *Clique) Prepare(chain consensus.ChainHeaderReader, header *types.Header
// rewards given.
func (c *Clique) Finalize(chain consensus.ChainHeaderReader, header *types.Header, state *state.StateDB, txs []*types.Transaction, uncles []*types.Header, withdrawals []*types.Withdrawal) {
// No block rewards in PoA, so the state remains as is and uncles are dropped
header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number))
header.UncleHash = types.CalcUncleHash(nil)
}

// FinalizeAndAssemble implements consensus.Engine, ensuring no uncles are set,
Expand All @@ -579,11 +577,17 @@ func (c *Clique) FinalizeAndAssemble(chain consensus.ChainHeaderReader, header *
if len(withdrawals) > 0 {
return nil, errors.New("clique does not support withdrawals")
}

// Finalize block
// Finalize block.
c.Finalize(chain, header, state, txs, uncles, nil)

// Assemble and return the final block for sealing
// Assign the final state root to header.
root, err := state.IntermediateRoot(chain.Config().IsEIP158(header.Number))
if err != nil {
return nil, err
}
header.Root = root

// Assemble and return the final block for sealing.
return types.NewBlock(header, txs, nil, receipts, trie.NewStackTrie(nil)), nil
}

Expand Down
10 changes: 5 additions & 5 deletions consensus/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,16 @@ type Engine interface {
// rules of a particular engine. The changes are executed inline.
Prepare(chain ChainHeaderReader, header *types.Header) error

// Finalize runs any post-transaction state modifications (e.g. block rewards)
// but does not assemble the block.
// Finalize runs any post-transaction state modifications (e.g. block rewards
// or process withdrawals) but does not assemble the block.
//
// Note: The block header and state database might be updated to reflect any
// consensus rules that happen at finalization (e.g. block rewards).
// Note: The state database might be updated to reflect any consensus rules
// that happen at finalization.
Finalize(chain ChainHeaderReader, header *types.Header, state *state.StateDB, txs []*types.Transaction,
uncles []*types.Header, withdrawals []*types.Withdrawal)

// FinalizeAndAssemble runs any post-transaction state modifications (e.g. block
// rewards) and assembles the final block.
// rewards or process withdrawals) and assembles the final block.
//
// Note: The block header and state database might be updated to reflect any
// consensus rules that happen at finalization (e.g. block rewards).
Expand Down
14 changes: 10 additions & 4 deletions consensus/ethash/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,6 @@ func (ethash *Ethash) Prepare(chain consensus.ChainHeaderReader, header *types.H
func (ethash *Ethash) Finalize(chain consensus.ChainHeaderReader, header *types.Header, state *state.StateDB, txs []*types.Transaction, uncles []*types.Header, withdrawals []*types.Withdrawal) {
// Accumulate any block and uncle rewards and commit the final state root
accumulateRewards(chain.Config(), state, header, uncles)
header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number))
}

// FinalizeAndAssemble implements consensus.Engine, accumulating the block and
Expand All @@ -612,10 +611,17 @@ func (ethash *Ethash) FinalizeAndAssemble(chain consensus.ChainHeaderReader, hea
if len(withdrawals) > 0 {
return nil, errors.New("ethash does not support withdrawals")
}

// Finalize block
// Finalize block.
ethash.Finalize(chain, header, state, txs, uncles, nil)
// Header seems complete, assemble into a block and return

// Assign the final state root to header.
root, err := state.IntermediateRoot(chain.Config().IsEIP158(header.Number))
if err != nil {
return nil, err
}
header.Root = root

// Header seems complete, assemble into a block and return.
return types.NewBlock(header, txs, uncles, receipts, trie.NewStackTrie(nil)), nil
}

Expand Down
6 changes: 5 additions & 1 deletion core/block_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,11 @@ func (v *BlockValidator) ValidateState(block *types.Block, statedb *state.StateD
}
// Validate the state root against the received state root and throw
// an error if they don't match.
if root := statedb.IntermediateRoot(v.config.IsEIP158(header.Number)); header.Root != root {
root, err := statedb.IntermediateRoot(v.config.IsEIP158(header.Number))
if err != nil {
return fmt.Errorf("failed to derive state root %v", err)
}
if header.Root != root {
return fmt.Errorf("invalid merkle root (remote: %x local: %x)", header.Root, root)
}
return nil
Expand Down
6 changes: 5 additions & 1 deletion core/chain_makers.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,12 @@ func makeHeader(chain consensus.ChainReader, parent *types.Block, state *state.S
} else {
time = parent.Time() + 10 // block time is fixed at 10 seconds
}
root, err := state.IntermediateRoot(chain.Config().IsEIP158(parent.Number()))
if err != nil {
panic(err) // error shouldn't occur in tests.
}
header := &types.Header{
Root: state.IntermediateRoot(chain.Config().IsEIP158(parent.Number())),
Root: root,
ParentHash: parent.Hash(),
Coinbase: parent.Coinbase(),
Difficulty: engine.CalcDifficulty(chain, time, &types.Header{
Expand Down
47 changes: 22 additions & 25 deletions core/state/state_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ func (s Storage) String() (str string) {
for key, value := range s {
str += fmt.Sprintf("%X : %X\n", key, value)
}

return
}

Expand All @@ -54,7 +53,6 @@ func (s Storage) Copy() Storage {
for key, value := range s {
cpy[key] = value
}

return cpy
}

Expand All @@ -73,8 +71,12 @@ type stateObject struct {
// DB error.
// State objects are used by the consensus core and VM which are
// unable to deal with database-level errors. Any error that occurs
// during a database read is memoized here and will eventually be returned
// by StateDB.Commit.
// during a database read is memoized here and will eventually be
// returned by StateDB.Commit. Specially, this error is used to
// represent a failed operation of:
// - storage trie read
// - contract code read
// - trie node decode
dbErr error

// Write caches.
Expand All @@ -86,7 +88,7 @@ type stateObject struct {
dirtyStorage Storage // Storage entries that have been modified in the current transaction execution

// Cache flags.
// When an object is marked suicided it will be delete from the trie
// When an object is marked suicided it will be deleted from the trie
// during the "update" phase of the state transition.
dirtyCode bool // true if the code was updated
suicided bool
Expand Down Expand Up @@ -282,6 +284,10 @@ func (s *stateObject) finalise(prefetch bool) {
// It will return nil if the trie has not been loaded and no changes have been
// made. An error will be returned if the trie can't be loaded/updated correctly.
func (s *stateObject) updateTrie(db Database) (Trie, error) {
// Short circuit if any the previous database failure is memorized.
if s.dbErr != nil {
return nil, s.dbErr
}
// Make sure all dirty slots are finalized into the pending storage area
s.finalise(false) // Don't prefetch anymore, pull directly if need be
if len(s.pendingStorage) == 0 {
Expand All @@ -298,7 +304,6 @@ func (s *stateObject) updateTrie(db Database) (Trie, error) {
)
tr, err := s.getTrie(db)
if err != nil {
s.setError(err)
return nil, err
}
// Insert all the pending updates into the trie
Expand All @@ -313,15 +318,13 @@ func (s *stateObject) updateTrie(db Database) (Trie, error) {
var v []byte
if (value == common.Hash{}) {
if err := tr.TryDelete(key[:]); err != nil {
s.setError(err)
return nil, err
}
s.db.StorageDeleted += 1
} else {
// Encoding []byte cannot fail, ok to ignore the error.
v, _ = rlp.EncodeToBytes(common.TrimLeftZeroes(value[:]))
if err := tr.TryUpdate(key[:], v); err != nil {
s.setError(err)
return nil, err
}
s.db.StorageUpdated += 1
Expand All @@ -348,35 +351,35 @@ func (s *stateObject) updateTrie(db Database) (Trie, error) {
return tr, nil
}

// UpdateRoot sets the trie root to the current root hash of. An error
// will be returned if trie root hash is not computed correctly.
func (s *stateObject) updateRoot(db Database) {
// updateRoot sets the trie root to the current root hash. An error
// will be returned if case any error occurs because of failed database
// reads.
func (s *stateObject) updateRoot(db Database) error {
tr, err := s.updateTrie(db)
if err != nil {
s.setError(fmt.Errorf("updateRoot (%x) error: %w", s.address, err))
return
return err
}
// If nothing changed, don't bother with hashing anything
if tr == nil {
return
return nil
}
// Track the amount of time wasted on hashing the storage trie
if metrics.EnabledExpensive {
defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now())
}
s.data.Root = tr.Hash()
return nil
}

// commitTrie submits the storage changes into the storage trie and re-computes
// the root. Besides, all trie changes will be collected in a nodeset and returned.
// the root. Storage trie changes will be wrapped in nodeset and be returned.
// The error will be non-nil if any error occurs during trie commit operation
// or memorized in stateObject because of failed database reads.
func (s *stateObject) commitTrie(db Database) (*trie.NodeSet, error) {
tr, err := s.updateTrie(db)
if err != nil {
return nil, err
}
if s.dbErr != nil {
return nil, s.dbErr
}
// If nothing changed, don't bother with committing anything
if tr == nil {
return nil, nil
Expand Down Expand Up @@ -437,6 +440,7 @@ func (s *stateObject) deepCopy(db *StateDB) *stateObject {
stateObject.suicided = s.suicided
stateObject.dirtyCode = s.dirtyCode
stateObject.deleted = s.deleted
stateObject.dbErr = s.dbErr
return stateObject
}

Expand Down Expand Up @@ -521,10 +525,3 @@ func (s *stateObject) Balance() *big.Int {
func (s *stateObject) Nonce() uint64 {
return s.data.Nonce
}

// Value is never called, but must be present to allow stateObject to be used
// as a vm.Account interface that also satisfies the vm.ContractRef
// interface. Interfaces are awesome.
func (s *stateObject) Value() *big.Int {
panic("Value on stateObject should never be called")
}
Loading