From 12fed0ed5313c5323154a8d11378e74b937a3e05 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Thu, 12 May 2022 10:36:48 +0200
Subject: [PATCH] blocksync: validate block before persisting it (backport
 #8493) (#8496)

---
 CHANGELOG_PENDING.md             |  2 ++
 internal/blocksync/v0/reactor.go | 54 +++++++++++++++++---------------
 2 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md
index 930c90adb..cb413f09e 100644
--- a/CHANGELOG_PENDING.md
+++ b/CHANGELOG_PENDING.md
@@ -25,3 +25,5 @@ Special thanks to external contributors on this release:
 ### IMPROVEMENTS
 
 ### BUG FIXES
+
+- [blocksync] [\#8496](https://github.com/tendermint/tendermint/pull/8496) validate block against state before persisting it to disk (@cmwaters)
diff --git a/internal/blocksync/v0/reactor.go b/internal/blocksync/v0/reactor.go
index 552dcbda5..1fb3ad29d 100644
--- a/internal/blocksync/v0/reactor.go
+++ b/internal/blocksync/v0/reactor.go
@@ -544,8 +544,15 @@ FOR_LOOP:
 			// first.Hash() doesn't verify the tx contents, so MakePartSet() is
 			// currently necessary.
 			err := state.Validators.VerifyCommitLight(chainID, firstID, first.Height, second.LastCommit)
+
+			if err == nil {
+				// validate the block before we persist it
+				err = r.blockExec.ValidateBlock(state, first)
+			}
+
+			// If either of the checks failed we log the error and request for a new block
+			// at that height
 			if err != nil {
-				err = fmt.Errorf("invalid last commit: %w", err)
 				r.Logger.Error(
 					err.Error(),
 					"last_commit", second.LastCommit,
@@ -570,37 +577,34 @@ FOR_LOOP:
 				}
 
 				continue FOR_LOOP
-			} else {
-				r.pool.PopRequest()
+			}
 
-				// TODO: batch saves so we do not persist to disk every block
-				r.store.SaveBlock(first, firstParts, second.LastCommit)
+			r.pool.PopRequest()
 
-				var err error
+			// TODO: batch saves so we do not persist to disk every block
+			r.store.SaveBlock(first, firstParts, second.LastCommit)
 
-				// TODO: Same thing for app - but we would need a way to get the hash
-				// without persisting the state.
-				state, err = r.blockExec.ApplyBlock(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))
-				}
+			// TODO: Same thing for app - but we would need a way to get the hash
+			// without persisting the state.
+			state, err = r.blockExec.ApplyBlock(state, firstID, first)
+			if err != nil {
+				panic(fmt.Sprintf("failed to process committed block (%d:%X): %v", first.Height, first.Hash(), err))
+			}
 
-				r.metrics.RecordConsMetrics(first)
+			r.metrics.RecordConsMetrics(first)
 
-				blocksSynced++
+			blocksSynced++
 
-				if blocksSynced%100 == 0 {
-					lastRate = 0.9*lastRate + 0.1*(100/time.Since(lastHundred).Seconds())
-					r.Logger.Info(
-						"block sync rate",
-						"height", r.pool.height,
-						"max_peer_height", r.pool.MaxPeerHeight(),
-						"blocks/s", lastRate,
-					)
+			if blocksSynced%100 == 0 {
+				lastRate = 0.9*lastRate + 0.1*(100/time.Since(lastHundred).Seconds())
+				r.Logger.Info(
+					"block sync rate",
+					"height", r.pool.height,
+					"max_peer_height", r.pool.MaxPeerHeight(),
+					"blocks/s", lastRate,
+				)
 
-					lastHundred = time.Now()
-				}
+				lastHundred = time.Now()
 			}
 
 			continue FOR_LOOP