perf(consensus/state): Change finalizeCommit to use applyVerifiedBloc… (

#39) (#46)

* perf(consensus/state): Change finalizeCommit to use applyVerifiedBlock (backport cometbft#2928) (cometbft#2945)

Simplest component of cometbft#2854

We already run ValidateBlock in finalizeCommit, so this PR removes one
extra redundant call by using ApplyVerifiedBlock. (The other call is
also redundant, but that likely requires a more complex caching strategy
as noted in cometbft#2854 to remedy)

From my understanding of these benchmarks, at Osmosis 150 validators,
this should be saving ~13ms of execution time per block.

---

#### PR checklist

- [X] Tests written/updated - Theres no test to update, as its
impossible to reach the difference in codepaths!
- [X] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request cometbft#2928 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Dev Ojha <[email protected]>
Co-authored-by: Anton Kaliaev <[email protected]>

* fix merge conflict

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Dev Ojha <[email protected]>
Co-authored-by: Anton Kaliaev <[email protected]>
(cherry picked from commit 6fe6ee1)

Co-authored-by: Adam Tucker <[email protected]>

.changelog/unreleased/improvements/2928-remove-redundant-verifyblock-call-in-finalize-commit.md

@@ -0,0 +1,2 @@
+- `[consensus/state]` Remove a redundant `VerifyBlock` call in `FinalizeCommit`
+  ([\#2928](https://github.com/cometbft/cometbft/pull/2928))

blocksync/reactor.go

@@ -426,7 +426,7 @@ FOR_LOOP:
 
 			// TODO: same thing for app - but we would need a way to
 			// get the hash without persisting the state
-			state, err = bcR.blockExec.ApplyVerifiedBlock(state, firstID, first)
+			state, _, err = bcR.blockExec.ApplyVerifiedBlock(state, firstID, first)
 			if err != nil {
 				// TODO This is bad, are we zombie?
 				panic(fmt.Sprintf("Failed to process committed block (%d:%X): %v", first.Height, first.Hash(), err))

consensus/state.go

@@ -1701,13 +1701,13 @@ func (cs *State) finalizeCommit(height int64) {
 	stateCopy := cs.state.Copy()
 
 	// Execute and commit the block, update and save the state, and update the mempool.
+	// We use apply verified block here because we have verified the block in this function already.
 	// NOTE The block.AppHash wont reflect these txs until the next block.
 	var (
 		err          error
 		retainHeight int64
 	)
-
-	stateCopy, retainHeight, err = cs.blockExec.ApplyBlock(
+	stateCopy, retainHeight, err = cs.blockExec.ApplyVerifiedBlock(
 		stateCopy,
 		types.BlockID{
 			Hash:          block.Hash(),

state/execution.go

@@ -182,9 +182,8 @@ func (blockExec *BlockExecutor) ValidateBlock(state State, block *types.Block) e
 // ApplyVerifiedBlock does the same as `ApplyBlock`, but skips verification.
 func (blockExec *BlockExecutor) ApplyVerifiedBlock(
 	state State, blockID types.BlockID, block *types.Block,
-) (State, error) {
-	newState, _, err := blockExec.applyBlock(state, blockID, block)
-	return newState, err
+) (State, int64, error) {
+	return blockExec.applyBlock(state, blockID, block)
 }
 
 // ApplyBlock validates the block against the state, executes it against the app,