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

Conversation

rjl493456442
Copy link
Member

@rjl493456442 rjl493456442 commented Feb 13, 2023

This PR bubble up the memorized database failure in stateDB and report it in critical places(e.g. block validation).

@rjl493456442 rjl493456442 marked this pull request as ready for review February 13, 2023 08:25
@holiman
Copy link
Contributor

holiman commented Feb 17, 2023

I need to look a bit more to grokk everything, but one thing we need to be careful about is that

  • Lookups caused by (potentially malicious) eth/snap trie requests must not trigger memorised error
  • Lookups caused by prefetcher must not trigger memorised error
  • And same for lookups caused due to tracing, e.g historical roots

@@ -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.

@@ -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.

@rjl493456442
Copy link
Member Author

I need to look a bit more to grokk everything, but one thing we need to be careful about is that

  • Lookups caused by (potentially malicious) eth/snap trie requests must not trigger memorised error
  • Lookups caused by prefetcher must not trigger memorised error
  • And same for lookups caused due to tracing, e.g historical roots

Yep, very good question, and I will elaborate the rules of memorised error first so we can be quite
confident that these situations won't happen for sure.

What's the memorised error in stateDB

StateDB is used by EVM to access ethereum state and cache mutations inside. However EVM is
not capable for handling database error(e.g. state is corrupted and can't return the desired state),
so any error that occurs during a database read will be memorised in stateDB and returns a default
value for EVM's state read requests instead, specifically:

  • nil for getStateObject if state is corrupted
  • common.Hash{} for GetCommittedState if state is corrupted
  • nil for Code if state is corrupted
  • 0 for CodeSize if state is corrupted

Only these four types of errors will be memorised in stateDB.

How memorised error will be used

memorised error is only used when the stateDB needs to be rehashed, or when the stateDB mutations
is submitted to the database. Otherwise, it's simply ignored and stateDB is just returning wrong data
(the default state value) for state queries.

Specifically, memorised error is only checked in these functions:

  • stateDB.IntermediateRoot:
    In this function, all errors memorised in each dirty state object(for failed storage slot, contract code
    state access) and the error memorised in stateDB(for failed state object access) will be checked and
    returned.
  • stateDB.Commit:
    In this function, all errors memorised in each dirty state object(for failed storage slot, contract code
    state access) and the error memorised in stateDB(for failed state object access) will be checked and
    returned.
  • stateDB.StorageTrie:
    In this function, a storage trie is requested for serving GetProof, StorageRangeAt. If the state is
    corrupted, an error will be returned directly

State isolation

There are actually two types of stateDb we can classify:

  • StateDB for chain progression
  • StateDB for state query
    Theoretically, it's possible that there is a shared stateDb instance and the state data is indeed corrupted,
    some state queries touch the corrupted part which lead to a memorised state error and eventually abort
    the chain progression. But fortunately, all the stateDB instance for both chain progression and state query
    are all created on top of the shared trie database. It means:

Lookups caused by (potentially malicious) eth/snap trie requests must not trigger memorised error
The error is only memorised in the stateDb instance which is created for serving this particular requests

Lookups caused by prefetcher must not trigger memorised error
The error is only memorised in the stateDb instance which is created for pre-execution

And same for lookups caused due to tracing, e.g historical roots
The error is only memorised in the stateDb instance which is created for tracing

@rjl493456442
Copy link
Member Author

Needs to fix CI failures

@karalabe
Copy link
Member

Closing in favor of #26870

@karalabe karalabe closed this Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants