statesync: assert app version matches (#7463)

internal/statesync/reactor_test.go

@@ -29,9 +29,9 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
-var (
-	m = PrometheusMetrics(config.TestConfig().Instrumentation.Namespace)
-)
+var m = PrometheusMetrics(config.TestConfig().Instrumentation.Namespace)
+
+const testAppVersion = 9
 
 type reactorTestSuite struct {
 	reactor *Reactor
@@ -204,7 +204,7 @@ func TestReactor_Sync(t *testing.T) {
 
 	// app query returns valid state app hash
 	rts.connQuery.On("InfoSync", mock.Anything, proxy.RequestInfo).Return(&abci.ResponseInfo{
-		AppVersion:       9,
+		AppVersion:       testAppVersion,
 		LastBlockHeight:  snapshotHeight,
 		LastBlockAppHash: chain[snapshotHeight+1].AppHash,
 	}, nil)
@@ -798,6 +798,7 @@ func mockLB(t *testing.T, height int64, time time.Time, lastBlockID types.BlockI
 		LastBlockID: lastBlockID,
 		Time:        time,
 	})
+	header.Version.App = testAppVersion
 	require.NoError(t, err)
 	nextVals, nextPrivVals := factory.RandValidatorSet(3, 10)
 	header.ValidatorsHash = currentVals.Hash()

internal/statesync/syncer.go

@@ -348,12 +348,10 @@ func (s *syncer) Sync(ctx context.Context, snapshot *snapshot, chunks *chunkQueu
 		return sm.State{}, nil, err
 	}
 
-	// Verify app and update app version
-	appVersion, err := s.verifyApp(snapshot)
-	if err != nil {
+	// Verify app and app version
+	if err := s.verifyApp(snapshot, state.Version.Consensus.App); err != nil {
 		return sm.State{}, nil, err
 	}
-	state.Version.Consensus.App = appVersion
 
 	// Done! 🎉
 	s.logger.Info("Snapshot restored", "height", snapshot.Height, "format", snapshot.Format,
@@ -547,19 +545,27 @@ func (s *syncer) requestChunk(ctx context.Context, snapshot *snapshot, chunk uin
 	return nil
 }
 
-// verifyApp verifies the sync, checking the app hash and last block height. It returns the
-// app version, which should be returned as part of the initial state.
-func (s *syncer) verifyApp(snapshot *snapshot) (uint64, error) {
+// verifyApp verifies the sync, checking the app hash, last block height and app version
+func (s *syncer) verifyApp(snapshot *snapshot, appVersion uint64) error {
 	resp, err := s.connQuery.InfoSync(context.TODO(), proxy.RequestInfo)
 	if err != nil {
-		return 0, fmt.Errorf("failed to query ABCI app for appHash: %w", err)
+		return fmt.Errorf("failed to query ABCI app for appHash: %w", err)
+	}
+
+	// sanity check that the app version in the block matches the application's own record
+	// of its version
+	if resp.AppVersion != appVersion {
+		// An error here most like means that the app hasn't inplemented state sync
+		// or the Info call correctly
+		return fmt.Errorf("app version mismatch. Expected: %d, got: %d",
+			appVersion, resp.AppVersion)
 	}
 
 	if !bytes.Equal(snapshot.trustedAppHash, resp.LastBlockAppHash) {
 		s.logger.Error("appHash verification failed",
 			"expected", snapshot.trustedAppHash,
 			"actual", resp.LastBlockAppHash)
-		return 0, errVerifyFailed
+		return errVerifyFailed
 	}
 
 	if uint64(resp.LastBlockHeight) != snapshot.Height {
@@ -568,9 +574,9 @@ func (s *syncer) verifyApp(snapshot *snapshot) (uint64, error) {
 			"expected", snapshot.Height,
 			"actual", resp.LastBlockHeight,
 		)
-		return 0, errVerifyFailed
+		return errVerifyFailed
 	}
 
 	s.logger.Info("Verified ABCI app", "height", snapshot.Height, "appHash", snapshot.trustedAppHash)
-	return resp.AppVersion, nil
+	return nil
 }

internal/statesync/syncer_test.go

@@ -30,7 +30,7 @@ func TestSyncer_SyncAny(t *testing.T) {
 		Version: sm.Version{
 			Consensus: version.Consensus{
 				Block: version.BlockProtocol,
-				App:   0,
+				App:   testAppVersion,
 			},
 			Software: version.TMVersion,
 		},
@@ -178,7 +178,7 @@ func TestSyncer_SyncAny(t *testing.T) {
 		Index: 2, Chunk: []byte{1, 1, 2},
 	}).Once().Return(&abci.ResponseApplySnapshotChunk{Result: abci.ResponseApplySnapshotChunk_ACCEPT}, nil)
 	connQuery.On("InfoSync", mock.Anything, proxy.RequestInfo).Return(&abci.ResponseInfo{
-		AppVersion:       9,
+		AppVersion:       testAppVersion,
 		LastBlockHeight:  1,
 		LastBlockAppHash: []byte("app_hash"),
 	}, nil)
@@ -192,10 +192,7 @@ func TestSyncer_SyncAny(t *testing.T) {
 	require.Equal(t, map[uint32]int{0: 1, 1: 2, 2: 1}, chunkRequests)
 	chunkRequestsMtx.Unlock()
 
-	// The syncer should have updated the state app version from the ABCI info response.
 	expectState := state
-	expectState.Version.Consensus.App = 9
-
 	require.Equal(t, expectState, newState)
 	require.Equal(t, commit, lastCommit)
 
@@ -713,6 +710,8 @@ func TestSyncer_applyChunks_RejectSenders(t *testing.T) {
 
 func TestSyncer_verifyApp(t *testing.T) {
 	boom := errors.New("boom")
+	appVersion := uint64(9)
+	appVersionMismatchErr := errors.New("app version mismatch. Expected: 9, got: 2")
 	s := &snapshot{Height: 3, Format: 1, Chunks: 5, Hash: []byte{1, 2, 3}, trustedAppHash: []byte("app_hash")}
 
 	testcases := map[string]struct {
@@ -723,17 +722,22 @@ func TestSyncer_verifyApp(t *testing.T) {
 		"verified": {&abci.ResponseInfo{
 			LastBlockHeight:  3,
 			LastBlockAppHash: []byte("app_hash"),
-			AppVersion:       9,
+			AppVersion:       appVersion,
 		}, nil, nil},
+		"invalid app version": {&abci.ResponseInfo{
+			LastBlockHeight:  3,
+			LastBlockAppHash: []byte("app_hash"),
+			AppVersion:       2,
+		}, nil, appVersionMismatchErr},
 		"invalid height": {&abci.ResponseInfo{
 			LastBlockHeight:  5,
 			LastBlockAppHash: []byte("app_hash"),
-			AppVersion:       9,
+			AppVersion:       appVersion,
 		}, nil, errVerifyFailed},
 		"invalid hash": {&abci.ResponseInfo{
 			LastBlockHeight:  3,
 			LastBlockAppHash: []byte("xxx"),
-			AppVersion:       9,
+			AppVersion:       appVersion,
 		}, nil, errVerifyFailed},
 		"error": {nil, boom, boom},
 	}
@@ -749,16 +753,12 @@ func TestSyncer_verifyApp(t *testing.T) {
 			rts := setup(ctx, t, nil, nil, nil, 2)
 
 			rts.connQuery.On("InfoSync", mock.Anything, proxy.RequestInfo).Return(tc.response, tc.err)
-			version, err := rts.syncer.verifyApp(s)
+			err := rts.syncer.verifyApp(s, appVersion)
 			unwrapped := errors.Unwrap(err)
 			if unwrapped != nil {
 				err = unwrapped
 			}
-
 			require.Equal(t, tc.expectErr, err)
-			if err == nil {
-				require.Equal(t, tc.response.AppVersion, version)
-			}
 		})
 	}
 }