From 6cc6a6c5bff60d6851808d66826b8f50d056b8ef Mon Sep 17 00:00:00 2001 From: mmsqe Date: Mon, 7 Oct 2024 15:59:29 +0800 Subject: [PATCH] fix(baseapp): align block header when query with latest height (#21003) --- CHANGELOG.md | 1 + baseapp/abci.go | 52 +++++++++++++++---- baseapp/baseapp_test.go | 109 ++++++++++++++++++++++++++++++++++------ 3 files changed, 137 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f86a2748fdce..db1e8bed256c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i * (sims) [#21952](https://github.com/cosmos/cosmos-sdk/pull/21952) Use liveness matrix for validator sign status in sims * (sims) [#21906](https://github.com/cosmos/cosmos-sdk/pull/21906) Skip sims test when running dry on validators * (cli) [#21919](https://github.com/cosmos/cosmos-sdk/pull/21919) Query address-by-acc-num by account_id instead of id. +* (baseapp) [#21003](https://github.com/cosmos/cosmos-sdk/pull/21003) Align block header when query with latest height. ### API Breaking Changes diff --git a/baseapp/abci.go b/baseapp/abci.go index dd091294d563..15646e438ab6 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -143,7 +143,7 @@ func (app *BaseApp) Info(_ *abci.InfoRequest) (*abci.InfoResponse, error) { lastCommitID := app.cms.LastCommitID() appVersion := InitialAppVersion if lastCommitID.Version > 0 { - ctx, err := app.CreateQueryContext(lastCommitID.Version, false) + ctx, err := app.CreateQueryContextWithCheckHeader(lastCommitID.Version, false, false) if err != nil { return nil, fmt.Errorf("failed creating query context: %w", err) } @@ -1222,6 +1222,12 @@ func checkNegativeHeight(height int64) error { // CreateQueryContext creates a new sdk.Context for a query, taking as args // the block height and whether the query needs a proof or not. func (app *BaseApp) CreateQueryContext(height int64, prove bool) (sdk.Context, error) { + return app.CreateQueryContextWithCheckHeader(height, prove, true) +} + +// CreateQueryContextWithCheckHeader creates a new sdk.Context for a query, taking as args +// the block height, whether the query needs a proof or not, and whether to check the header or not. +func (app *BaseApp) CreateQueryContextWithCheckHeader(height int64, prove, checkHeader bool) (sdk.Context, error) { if err := checkNegativeHeight(height); err != nil { return sdk.Context{}, err } @@ -1245,12 +1251,7 @@ func (app *BaseApp) CreateQueryContext(height int64, prove bool) (sdk.Context, e ) } - // when a client did not provide a query height, manually inject the latest - if height == 0 { - height = lastBlockHeight - } - - if height <= 1 && prove { + if height > 0 && height <= 1 && prove { return sdk.Context{}, errorsmod.Wrap( sdkerrors.ErrInvalidRequest, @@ -1258,6 +1259,38 @@ func (app *BaseApp) CreateQueryContext(height int64, prove bool) (sdk.Context, e ) } + var header *cmtproto.Header + isLatest := height == 0 + for _, state := range []*state{ + app.checkState, + app.finalizeBlockState, + } { + if state != nil { + // branch the commit multi-store for safety + h := state.Context().BlockHeader() + if isLatest { + lastBlockHeight = qms.LatestVersion() + } + if !checkHeader || !isLatest || isLatest && h.Height == lastBlockHeight { + header = &h + break + } + } + } + + if header == nil { + return sdk.Context{}, + errorsmod.Wrapf( + sdkerrors.ErrInvalidHeight, + "header height in all state context is not latest height (%d)", lastBlockHeight, + ) + } + + // when a client did not provide a query height, manually inject the latest + if isLatest { + height = lastBlockHeight + } + cacheMS, err := qms.CacheMultiStoreWithVersion(height) if err != nil { return sdk.Context{}, @@ -1275,10 +1308,10 @@ func (app *BaseApp) CreateQueryContext(height int64, prove bool) (sdk.Context, e ChainID: app.chainID, Height: height, }). - WithBlockHeader(app.checkState.Context().BlockHeader()). + WithBlockHeader(*header). WithBlockHeight(height) - if height != lastBlockHeight { + if !isLatest { rms, ok := app.cms.(*rootmulti.Store) if ok { cInfo, err := rms.GetCommitInfo(height) @@ -1287,7 +1320,6 @@ func (app *BaseApp) CreateQueryContext(height int64, prove bool) (sdk.Context, e } } } - return ctx, nil } diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 60e474b851e4..9ca9669b6224 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -696,26 +696,26 @@ func TestBaseAppPostHandler(t *testing.T) { require.NotContains(t, suite.logBuffer.String(), "panic recovered in runTx") } +type mockABCIListener struct { + ListenCommitFn func(context.Context, abci.CommitResponse, []*storetypes.StoreKVPair) error +} + +func (m mockABCIListener) ListenFinalizeBlock(_ context.Context, _ abci.FinalizeBlockRequest, _ abci.FinalizeBlockResponse) error { + return nil +} + +func (m *mockABCIListener) ListenCommit(ctx context.Context, commit abci.CommitResponse, pairs []*storetypes.StoreKVPair) error { + return m.ListenCommitFn(ctx, commit, pairs) +} + // Test and ensure that invalid block heights always cause errors. // See issues: // - https://github.com/cosmos/cosmos-sdk/issues/11220 // - https://github.com/cosmos/cosmos-sdk/issues/7662 func TestABCI_CreateQueryContext(t *testing.T) { t.Parallel() + app := getQueryBaseapp(t) - db := coretesting.NewMemDB() - name := t.Name() - app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil) - - _, err := app.FinalizeBlock(&abci.FinalizeBlockRequest{Height: 1}) - require.NoError(t, err) - _, err = app.Commit() - require.NoError(t, err) - - _, err = app.FinalizeBlock(&abci.FinalizeBlockRequest{Height: 2}) - require.NoError(t, err) - _, err = app.Commit() - require.NoError(t, err) testCases := []struct { name string height int64 @@ -724,7 +724,7 @@ func TestABCI_CreateQueryContext(t *testing.T) { expErr bool }{ {"valid height", 2, 2, true, false}, - {"valid height with different initial height", 2, 1, true, false}, + {"valid height with different initial height", 2, 1, true, true}, {"future height", 10, 10, true, true}, {"negative height, prove=true", -1, -1, true, true}, {"negative height, prove=false", -1, -1, false, true}, @@ -738,7 +738,11 @@ func TestABCI_CreateQueryContext(t *testing.T) { }) require.NoError(t, err) } - ctx, err := app.CreateQueryContext(tc.height, tc.prove) + height := tc.height + if tc.height > tc.headerHeight { + height = 0 + } + ctx, err := app.CreateQueryContext(height, tc.prove) if tc.expErr { require.Error(t, err) } else { @@ -749,6 +753,81 @@ func TestABCI_CreateQueryContext(t *testing.T) { } } +func TestABCI_CreateQueryContextWithCheckHeader(t *testing.T) { + t.Parallel() + app := getQueryBaseapp(t) + var height int64 = 2 + var headerHeight int64 = 1 + + testCases := []struct { + checkHeader bool + expErr bool + }{ + {true, true}, + {false, false}, + } + + for _, tc := range testCases { + t.Run("valid height with different initial height", func(t *testing.T) { + _, err := app.InitChain(&abci.InitChainRequest{ + InitialHeight: headerHeight, + }) + require.NoError(t, err) + ctx, err := app.CreateQueryContextWithCheckHeader(0, true, tc.checkHeader) + if tc.expErr { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, height, ctx.BlockHeight()) + } + }) + } +} + +func TestABCI_CreateQueryContext_Before_Set_CheckState(t *testing.T) { + t.Parallel() + + db := coretesting.NewMemDB() + name := t.Name() + var height int64 = 2 + var headerHeight int64 = 1 + + t.Run("valid height with different initial height", func(t *testing.T) { + app := baseapp.NewBaseApp(name, log.NewTestLogger(t), db, nil) + + _, err := app.FinalizeBlock(&abci.FinalizeBlockRequest{Height: 1}) + require.NoError(t, err) + _, err = app.Commit() + require.NoError(t, err) + + _, err = app.FinalizeBlock(&abci.FinalizeBlockRequest{Height: 2}) + require.NoError(t, err) + + var queryCtx *sdk.Context + var queryCtxErr error + app.SetStreamingManager(storetypes.StreamingManager{ + ABCIListeners: []storetypes.ABCIListener{ + &mockABCIListener{ + ListenCommitFn: func(context.Context, abci.CommitResponse, []*storetypes.StoreKVPair) error { + qCtx, qErr := app.CreateQueryContext(0, true) + queryCtx = &qCtx + queryCtxErr = qErr + return nil + }, + }, + }, + }) + _, err = app.Commit() + require.NoError(t, err) + require.NoError(t, queryCtxErr) + require.Equal(t, height, queryCtx.BlockHeight()) + _, err = app.InitChain(&abci.InitChainRequest{ + InitialHeight: headerHeight, + }) + require.NoError(t, err) + }) +} + func TestSetMinGasPrices(t *testing.T) { minGasPrices := sdk.DecCoins{sdk.NewInt64DecCoin("stake", 5000)} suite := NewBaseAppSuite(t, baseapp.SetMinGasPrices(minGasPrices.String()))