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.

Sign up for GitHub

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

consensus: improve consensus engine definition #26871

Merged
merged 1 commit into from
Mar 16, 2023
Merged

consensus: improve consensus engine definition #26871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 7 additions & 8 deletions consensus/beacon/consensus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,8 @@ func (beacon *Beacon) Prepare(chain consensus.ChainHeaderReader, header *types.H
return nil
}

// Finalize implements consensus.Engine, setting the final state on the header
// Finalize implements consensus.Engine and processes withdrawals on top.
func (beacon *Beacon) Finalize(chain consensus.ChainHeaderReader, header *types.Header, state *state.StateDB, txs []*types.Transaction, uncles []*types.Header, withdrawals []*types.Withdrawal) {
// Finalize is different with Prepare, it can be used in both block generation
// and verification. So determine the consensus rules by header type.
if !beacon.IsPoSHeader(header) {
beacon.ethone.Finalize(chain, header, state, txs, uncles, nil)
return
Expand All @@ -341,16 +339,12 @@ 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)
// No block reward which is issued by consensus layer instead.
}

// FinalizeAndAssemble implements consensus.Engine, setting the final state and
// assembling the block.
func (beacon *Beacon) FinalizeAndAssemble(chain consensus.ChainHeaderReader, header *types.Header, state *state.StateDB, txs []*types.Transaction, uncles []*types.Header, receipts []*types.Receipt, withdrawals []*types.Withdrawal) (*types.Block, error) {
// FinalizeAndAssemble is different with Prepare, it can be used in both block
// generation and verification. So determine the consensus rules by header type.
if !beacon.IsPoSHeader(header) {
return beacon.ethone.FinalizeAndAssemble(chain, header, state, txs, uncles, receipts, nil)
}
Expand All @@ -367,6 +361,11 @@ 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.
header.Root = state.IntermediateRoot(true)

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

Expand Down
14 changes: 7 additions & 7 deletions consensus/clique/clique.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,12 +565,10 @@ func (c *Clique) Prepare(chain consensus.ChainHeaderReader, header *types.Header
return nil
}

// Finalize implements consensus.Engine, ensuring no uncles are set, nor block
// rewards given.
// Finalize implements consensus.Engine. There is no post-transaction
// consensus rules in clique, do nothing here.
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)
// No block rewards in PoA, so the state remains as is
}

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

// 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.
header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number))

// 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
View file
Open in desktop
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 (e.g. block rewards).
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
11 changes: 6 additions & 5 deletions consensus/ethash/consensus.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,12 +598,10 @@ func (ethash *Ethash) Prepare(chain consensus.ChainHeaderReader, header *types.H
return nil
}

// Finalize implements consensus.Engine, accumulating the block and uncle rewards,
// setting the final state on the header
// Finalize implements consensus.Engine, accumulating the block and uncle rewards.
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
// Accumulate any block and uncle rewards
accumulateRewards(chain.Config(), state, header, uncles)
header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit unsure about this.
In blockchain insert, we call state_processor Process, which calls Finalize: https://github.com/ethereum/go-ethereum/blob/master/core/state_processor.go#L95

I agree that we need to set the header.Root, but call here to IntermediateRoot also modifies the state (?) by clearing empties.

I think it still works out, though, because in blockchain, we later call if err := bc.validator.ValidateState, which also calls IntermediateRoot. So we do it, but a bit later.

I'm not sure how the meters defined here are affected: https://github.com/ethereum/go-ethereum/blob/master/core/blockchain.go#L1764 . Maybe we have to move something there?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that we need to set the header.Root

I think in block processing, we don't need to mutate header at all. The case is we receive a block generated by block producer, the action here is to apply all transactions inside and then ensure the transition is the same as described by the block.

which also calls IntermediateRoot. So we do it, but a bit later.

Yes, it's the step for state transition checking. We compute the root hash of local stateDB, ensure the root is same as the one in block header advertised by the block producer.

I'm not sure how the meters defined here are affected: https://github.com/ethereum/go-ethereum/blob/master/core/blockchain.go#L1764 . Maybe we have to move something there?

Let me check.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that we need to set the header.Root

I think in block processing, we don't need to mutate header at all.

Yeah, typo from me. I meant: I agree that we do not need to set the header.Root. So I agred with that part of the change

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure how the meters defined here are affected: https://github.com/ethereum/go-ethereum/blob/master/core/blockchain.go#L1764 . Maybe we have to move something there?

You are right, I have to fix the metrics.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, I will deploy it on benchmark nodes to see the metrics work or not.

}

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

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

// Assign the final state root to header.
header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number))

// Header seems complete, assemble into a block and return
return types.NewBlock(header, txs, uncles, receipts, trie.NewStackTrie(nil)), nil
}
Expand Down
6 changes: 2 additions & 4 deletions core/block_validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,8 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error {
return nil
}

// ValidateState validates the various changes that happen after a state
// transition, such as amount of used gas, the receipt roots and the state root
// itself. ValidateState returns a database batch if the validation was a success
// otherwise nil and an error is returned.
// ValidateState validates the various changes that happen after a state transition,
// such as amount of used gas, the receipt roots and the state root itself.
func (v *BlockValidator) ValidateState(block *types.Block, statedb *state.StateDB, receipts types.Receipts, usedGas uint64) error {
header := block.Header()
if block.GasUsed() != usedGas {
Expand Down
53 changes: 27 additions & 26 deletions core/blockchain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1752,44 +1752,45 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals, setHead bool)
}

// Process block using the parent state as reference point
substart := time.Now()
pstart := time.Now()
receipts, logs, usedGas, err := bc.processor.Process(block, statedb, bc.vmConfig)
if err != nil {
bc.reportBlock(block, receipts, err)
atomic.StoreUint32(&followupInterrupt, 1)
return it.index, err
}
ptime := time.Since(pstart)

// Update the metrics touched during block processing
accountReadTimer.Update(statedb.AccountReads) // Account reads are complete, we can mark them
storageReadTimer.Update(statedb.StorageReads) // Storage reads are complete, we can mark them
accountUpdateTimer.Update(statedb.AccountUpdates) // Account updates are complete, we can mark them
storageUpdateTimer.Update(statedb.StorageUpdates) // Storage updates are complete, we can mark them
snapshotAccountReadTimer.Update(statedb.SnapshotAccountReads) // Account reads are complete, we can mark them
snapshotStorageReadTimer.Update(statedb.SnapshotStorageReads) // Storage reads are complete, we can mark them
triehash := statedb.AccountHashes + statedb.StorageHashes // Save to not double count in validation
trieproc := statedb.SnapshotAccountReads + statedb.AccountReads + statedb.AccountUpdates
trieproc += statedb.SnapshotStorageReads + statedb.StorageReads + statedb.StorageUpdates

blockExecutionTimer.Update(time.Since(substart) - trieproc - triehash)

// Validate the state using the default validator
substart = time.Now()
vstart := time.Now()
if err := bc.validator.ValidateState(block, statedb, receipts, usedGas); err != nil {
bc.reportBlock(block, receipts, err)
atomic.StoreUint32(&followupInterrupt, 1)
return it.index, err
}
proctime := time.Since(start)

// Update the metrics touched during block validation
accountHashTimer.Update(statedb.AccountHashes) // Account hashes are complete, we can mark them
storageHashTimer.Update(statedb.StorageHashes) // Storage hashes are complete, we can mark them
blockValidationTimer.Update(time.Since(substart) - (statedb.AccountHashes + statedb.StorageHashes - triehash))
vtime := time.Since(vstart)
proctime := time.Since(start) // processing + validation

// Update the metrics touched during block processing and validation
accountReadTimer.Update(statedb.AccountReads) // Account reads are complete(in processing)
storageReadTimer.Update(statedb.StorageReads) // Storage reads are complete(in processing)
snapshotAccountReadTimer.Update(statedb.SnapshotAccountReads) // Account reads are complete(in processing)
snapshotStorageReadTimer.Update(statedb.SnapshotStorageReads) // Storage reads are complete(in processing)
accountUpdateTimer.Update(statedb.AccountUpdates) // Account updates are complete(in validation)
storageUpdateTimer.Update(statedb.StorageUpdates) // Storage updates are complete(in validation)
accountHashTimer.Update(statedb.AccountHashes) // Account hashes are complete(in validation)
storageHashTimer.Update(statedb.StorageHashes) // Storage hashes are complete(in validation)
triehash := statedb.AccountHashes + statedb.StorageHashes // The time spent on tries hashing
trieUpdate := statedb.AccountUpdates + statedb.StorageUpdates // The time spent on tries update
trieRead := statedb.SnapshotAccountReads + statedb.AccountReads // The time spent on account read
trieRead += statedb.SnapshotStorageReads + statedb.StorageReads // The time spent on storage read
blockExecutionTimer.Update(ptime - trieRead) // The time spent on EVM processing
blockValidationTimer.Update(vtime - (triehash + trieUpdate)) // The time spent on block validation

// Write the block to the chain and get the status.
substart = time.Now()
var status WriteStatus
var (
wstart = time.Now()
status WriteStatus
)
if !setHead {
// Don't set the head, only insert the block
err = bc.writeBlockWithState(block, receipts, statedb)
Expand All @@ -1804,9 +1805,9 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals, setHead bool)
accountCommitTimer.Update(statedb.AccountCommits) // Account commits are complete, we can mark them
storageCommitTimer.Update(statedb.StorageCommits) // Storage commits are complete, we can mark them
snapshotCommitTimer.Update(statedb.SnapshotCommits) // Snapshot commits are complete, we can mark them
triedbCommitTimer.Update(statedb.TrieDBCommits) // Triedb commits are complete, we can mark them
triedbCommitTimer.Update(statedb.TrieDBCommits) // Trie database commits are complete, we can mark them

blockWriteTimer.Update(time.Since(substart) - statedb.AccountCommits - statedb.StorageCommits - statedb.SnapshotCommits - statedb.TrieDBCommits)
blockWriteTimer.Update(time.Since(wstart) - statedb.AccountCommits - statedb.StorageCommits - statedb.SnapshotCommits - statedb.TrieDBCommits)
blockInsertTimer.UpdateSince(start)

// Report the import stats before returning the various results
Expand Down