From db60bbad5425c0aaecbc6f197b78681f47de05e5 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 23 Feb 2022 12:17:12 +0100 Subject: [PATCH] statesync: assert app version matches (backport #7856) (#7885) --- statesync/stateprovider.go | 5 +++++ statesync/syncer.go | 35 ++++++++++++++++++++++------------- statesync/syncer_test.go | 28 ++++++++++++++++------------ test/e2e/app/app.go | 5 ++++- test/e2e/runner/setup.go | 2 ++ 5 files changed, 49 insertions(+), 26 deletions(-) diff --git a/statesync/stateprovider.go b/statesync/stateprovider.go index 7a1f78e86..ad6036468 100644 --- a/statesync/stateprovider.go +++ b/statesync/stateprovider.go @@ -19,6 +19,7 @@ import ( rpchttp "github.com/tendermint/tendermint/rpc/client/http" sm "github.com/tendermint/tendermint/state" "github.com/tendermint/tendermint/types" + "github.com/tendermint/tendermint/version" ) //go:generate mockery --case underscore --name StateProvider @@ -155,6 +156,10 @@ func (s *lightClientStateProvider) State(ctx context.Context, height uint64) (sm return sm.State{}, err } + state.Version = tmstate.Version{ + Consensus: currentLightBlock.Version, + Software: version.TMCoreSemVer, + } state.LastBlockHeight = lastLightBlock.Height state.LastBlockTime = lastLightBlock.Time state.LastBlockID = lastLightBlock.Commit.BlockID diff --git a/statesync/syncer.go b/statesync/syncer.go index f0342692d..115b98749 100644 --- a/statesync/syncer.go +++ b/statesync/syncer.go @@ -301,12 +301,10 @@ func (s *syncer) Sync(snapshot *snapshot, chunks *chunkQueue) (sm.State, *types. 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, @@ -476,25 +474,36 @@ func (s *syncer) requestChunk(snapshot *snapshot, chunk uint32) { })) } -// 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(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 likely 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 { - s.logger.Error("ABCI app reported unexpected last block height", - "expected", snapshot.Height, "actual", resp.LastBlockHeight) - return 0, errVerifyFailed + s.logger.Error( + "ABCI app reported unexpected last block height", + "expected", snapshot.Height, + "actual", resp.LastBlockHeight, + ) + return errVerifyFailed } s.logger.Info("Verified ABCI app", "height", snapshot.Height, "appHash", snapshot.trustedAppHash) - return resp.AppVersion, nil + return nil } diff --git a/statesync/syncer_test.go b/statesync/syncer_test.go index 4fd74a794..4dabe7288 100644 --- a/statesync/syncer_test.go +++ b/statesync/syncer_test.go @@ -26,6 +26,8 @@ import ( "github.com/tendermint/tendermint/version" ) +const testAppVersion = 9 + // Sets up a basic syncer that can be used to test OfferSnapshot requests func setupOfferSyncer(t *testing.T) (*syncer, *proxymocks.AppConnSnapshot) { connQuery := &proxymocks.AppConnQuery{} @@ -51,7 +53,7 @@ func TestSyncer_SyncAny(t *testing.T) { Version: tmstate.Version{ Consensus: tmversion.Consensus{ Block: version.BlockProtocol, - App: 0, + App: testAppVersion, }, Software: version.TMCoreSemVer, }, @@ -184,7 +186,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", proxy.RequestInfo).Return(&abci.ResponseInfo{ - AppVersion: 9, + AppVersion: testAppVersion, LastBlockHeight: 1, LastBlockAppHash: []byte("app_hash"), }, nil) @@ -198,9 +200,7 @@ func TestSyncer_SyncAny(t *testing.T) { assert.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 assert.Equal(t, expectState, newState) assert.Equal(t, commit, lastCommit) @@ -613,6 +613,8 @@ func TestSyncer_applyChunks_RejectSenders(t *testing.T) { func TestSyncer_verifyApp(t *testing.T) { boom := errors.New("boom") + const appVersion = 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 { @@ -623,17 +625,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}, } @@ -648,15 +655,12 @@ func TestSyncer_verifyApp(t *testing.T) { syncer := newSyncer(*cfg, log.NewNopLogger(), connSnapshot, connQuery, stateProvider, "") connQuery.On("InfoSync", proxy.RequestInfo).Return(tc.response, tc.err) - version, err := syncer.verifyApp(s) + err := syncer.verifyApp(s, appVersion) unwrapped := errors.Unwrap(err) if unwrapped != nil { err = unwrapped } - assert.Equal(t, tc.expectErr, err) - if err == nil { - assert.Equal(t, tc.response.AppVersion, version) - } + require.Equal(t, tc.expectErr, err) }) } } diff --git a/test/e2e/app/app.go b/test/e2e/app/app.go index c9ac3f30d..c812b0f15 100644 --- a/test/e2e/app/app.go +++ b/test/e2e/app/app.go @@ -15,9 +15,12 @@ import ( "github.com/tendermint/tendermint/version" ) +const appVersion = 1 + // Application is an ABCI application for use by end-to-end tests. It is a // simple key/value store for strings, storing data in memory and persisting // to disk as JSON, taking state sync snapshots if requested. + type Application struct { abci.BaseApplication logger log.Logger @@ -99,7 +102,7 @@ func NewApplication(cfg *Config) (*Application, error) { func (app *Application) Info(req abci.RequestInfo) abci.ResponseInfo { return abci.ResponseInfo{ Version: version.ABCIVersion, - AppVersion: 1, + AppVersion: appVersion, LastBlockHeight: int64(app.state.Height), LastBlockAppHash: app.state.Hash, } diff --git a/test/e2e/runner/setup.go b/test/e2e/runner/setup.go index 92902b01b..e1c82d589 100644 --- a/test/e2e/runner/setup.go +++ b/test/e2e/runner/setup.go @@ -202,6 +202,8 @@ func MakeGenesis(testnet *e2e.Testnet) (types.GenesisDoc, error) { ConsensusParams: types.DefaultConsensusParams(), InitialHeight: testnet.InitialHeight, } + // set the app version to 1 + genesis.ConsensusParams.Version.AppVersion = 1 for validator, power := range testnet.Validators { genesis.Validators = append(genesis.Validators, types.GenesisValidator{ Name: validator.Name,