From db7fcf80cdcca798be2ab1738a6672516827f433 Mon Sep 17 00:00:00 2001 From: jay tseng Date: Tue, 8 Mar 2022 16:20:45 -0500 Subject: [PATCH] move app ver check into HandShake step, rollback default gen app version, and set kvstore/e2e app ver to 0 --- abci/example/kvstore/kvstore.go | 2 +- config/toml.go | 4 +--- internal/consensus/replay.go | 9 ++++++++- internal/consensus/replay_file.go | 2 +- internal/consensus/replay_test.go | 8 ++++---- node/node.go | 13 +------------ node/node_test.go | 2 +- test/e2e/app/app.go | 2 +- types/params.go | 2 +- 9 files changed, 19 insertions(+), 25 deletions(-) diff --git a/abci/example/kvstore/kvstore.go b/abci/example/kvstore/kvstore.go index d2b2d99fe..9338b575f 100644 --- a/abci/example/kvstore/kvstore.go +++ b/abci/example/kvstore/kvstore.go @@ -24,7 +24,7 @@ var ( stateKey = []byte("stateKey") kvPairPrefixKey = []byte("kvPairKey:") - ProtocolVersion uint64 = 0x1 + ProtocolVersion uint64 = 0x0 ) type State struct { diff --git a/config/toml.go b/config/toml.go index 44c1cfbff..0508f9e74 100644 --- a/config/toml.go +++ b/config/toml.go @@ -612,9 +612,7 @@ var testGenesisFmt = `{ "ed25519" ] }, - "version": { - "app_version": "1" - } + "version": {} }, "validators": [ { diff --git a/internal/consensus/replay.go b/internal/consensus/replay.go index 5d097df21..cf135a459 100644 --- a/internal/consensus/replay.go +++ b/internal/consensus/replay.go @@ -238,7 +238,7 @@ func (h *Handshaker) NBlocks() int { } // TODO: retry the handshake/replay if it fails ? -func (h *Handshaker) Handshake(ctx context.Context, appClient abciclient.Client) error { +func (h *Handshaker) Handshake(ctx context.Context, appClient abciclient.Client, genDocAppVer uint64) error { // Handshake is done via ABCI Info on the query conn. res, err := appClient.Info(ctx, proxy.RequestInfo) @@ -259,6 +259,13 @@ func (h *Handshaker) Handshake(ctx context.Context, appClient abciclient.Client) "protocol-version", res.AppVersion, ) + // Check app version is matched with the genesis file. + if res.AppVersion != genDocAppVer { + return fmt.Errorf("client app version (%d) is not matched with the genesis file (%d)", + res.AppVersion, + genDocAppVer) + } + // Only set the version if there is no existing state. if h.initialState.LastBlockHeight == 0 { h.initialState.Version.Consensus.App = res.AppVersion diff --git a/internal/consensus/replay_file.go b/internal/consensus/replay_file.go index 492d1d1ee..4d5281791 100644 --- a/internal/consensus/replay_file.go +++ b/internal/consensus/replay_file.go @@ -343,7 +343,7 @@ func newConsensusStateForReplay( handshaker := NewHandshaker(logger, stateStore, state, blockStore, eventBus, gdoc) - if err = handshaker.Handshake(ctx, proxyApp); err != nil { + if err = handshaker.Handshake(ctx, proxyApp, gdoc.ConsensusParams.Version.AppVersion); err != nil { return nil, err } diff --git a/internal/consensus/replay_test.go b/internal/consensus/replay_test.go index 2f6a531da..750f71ae1 100644 --- a/internal/consensus/replay_test.go +++ b/internal/consensus/replay_test.go @@ -782,7 +782,7 @@ func testHandshakeReplay( require.NotNil(t, proxyApp) t.Cleanup(func() { cancel(); proxyApp.Wait() }) - err = handshaker.Handshake(ctx, proxyApp) + err = handshaker.Handshake(ctx, proxyApp, genDoc.ConsensusParams.Version.AppVersion) if expectError { require.Error(t, err) return @@ -995,7 +995,7 @@ func TestHandshakePanicsIfAppReturnsWrongAppHash(t *testing.T) { assert.Panics(t, func() { h := NewHandshaker(logger, stateStore, state, store, eventBus, genDoc) - if err = h.Handshake(ctx, proxyApp); err != nil { + if err = h.Handshake(ctx, proxyApp, genDoc.ConsensusParams.Version.AppVersion); err != nil { t.Log(err) } }) @@ -1015,7 +1015,7 @@ func TestHandshakePanicsIfAppReturnsWrongAppHash(t *testing.T) { assert.Panics(t, func() { h := NewHandshaker(logger, stateStore, state, store, eventBus, genDoc) - if err = h.Handshake(ctx, proxyApp); err != nil { + if err = h.Handshake(ctx, proxyApp, genDoc.ConsensusParams.Version.AppVersion); err != nil { t.Log(err) } }) @@ -1270,7 +1270,7 @@ func TestHandshakeUpdatesValidators(t *testing.T) { proxyApp := proxy.New(client, logger, proxy.NopMetrics()) require.NoError(t, proxyApp.Start(ctx), "Error starting proxy app connections") - require.NoError(t, handshaker.Handshake(ctx, proxyApp), "error on abci handshake") + require.NoError(t, handshaker.Handshake(ctx, proxyApp, genDoc.ConsensusParams.Version.AppVersion), "error on abci handshake") // reload the state, check the validator set was updated state, err = stateStore.Load() diff --git a/node/node.go b/node/node.go index ab4d9e394..df4f5d39a 100644 --- a/node/node.go +++ b/node/node.go @@ -164,17 +164,6 @@ func makeNode( return nil, fmt.Errorf("error starting proxy app connections: %w", err) } - // Check app version is matched with the genesis file. - appInfo, err := proxyApp.Info(ctx, abci.RequestInfo{Version: ""}) - if err != nil { - return nil, fmt.Errorf("error query app info: %w", err) - } - if appInfo.AppVersion != genDoc.ConsensusParams.Version.AppVersion { - return nil, fmt.Errorf("error client app version (%d) is not matched with the genesis file (%d)", - appInfo.AppVersion, - genDoc.ConsensusParams.Version.AppVersion) - } - // EventBus and IndexerService must be started before the handshake because // we might need to index the txs of the replayed block as this might not have happened // when the node stopped last time (i.e. the node stopped or crashed after it saved the block @@ -238,7 +227,7 @@ func makeNode( if err := consensus.NewHandshaker( logger.With("module", "handshaker"), stateStore, state, blockStore, eventBus, genDoc, - ).Handshake(ctx, proxyApp); err != nil { + ).Handshake(ctx, proxyApp, genDoc.ConsensusParams.Version.AppVersion); err != nil { return nil, combineCloseError(err, makeCloser(closers)) } diff --git a/node/node_test.go b/node/node_test.go index 36c8f36b9..4115c9743 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -798,7 +798,7 @@ func TestNodeAppVersionNotMatched(t *testing.T) { require.Error(t, err) require.Equal(t, - fmt.Sprintf("error client app version (%d) is not matched with the genesis file (%d)", + fmt.Sprintf("error=\"client app version (%d) is not matched with the genesis file (%d)\" closerError=\"\"", kvstore.ProtocolVersion, genesisDoc.ConsensusParams.Version.AppVersion), err.Error()) diff --git a/test/e2e/app/app.go b/test/e2e/app/app.go index e0c080f87..2fc5255b7 100644 --- a/test/e2e/app/app.go +++ b/test/e2e/app/app.go @@ -105,7 +105,7 @@ func (app *Application) Info(req abci.RequestInfo) abci.ResponseInfo { return abci.ResponseInfo{ Version: version.ABCIVersion, - AppVersion: 1, + AppVersion: 0, LastBlockHeight: int64(app.state.Height), LastBlockAppHash: app.state.Hash, } diff --git a/types/params.go b/types/params.go index 1d728b55f..fc9c4aaad 100644 --- a/types/params.go +++ b/types/params.go @@ -123,7 +123,7 @@ func DefaultValidatorParams() ValidatorParams { func DefaultVersionParams() VersionParams { return VersionParams{ - AppVersion: 1, + AppVersion: 0, } }