From efeee5ef790955261aff63097271f95e6b582911 Mon Sep 17 00:00:00 2001 From: Likhita Polavarapu <78951027+likhita-809@users.noreply.github.com> Date: Thu, 17 Nov 2022 17:04:04 +0530 Subject: [PATCH] fix: Return error instead of empty values when pruned height is queried (#13896) * fix: return err instead of empty values when pruned height is queried * fix tests * fix more tests * final fixes * add changelog --- CHANGELOG.md | 1 + baseapp/baseapp_test.go | 2 +- store/iavl/store.go | 2 +- store/rootmulti/store_test.go | 31 +++++++++++++++---------------- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 460f023b78d9..11b246b8a464 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* [#13896](https://github.com/cosmos/cosmos-sdk/pull/13896) Queries on pruned height returns error instead of empty values. * (deps) Bump Tendermint version to [v0.34.23](https://github.com/tendermint/tendermint/releases/tag/v0.34.23). * (deps) Bump IAVL version to [v0.19.4](https://github.com/cosmos/iavl/releases/tag/v0.19.4). * (snapshot) [#13400](https://github.com/cosmos/cosmos-sdk/pull/13400) Fix snapshot checksum issue in golang 1.19. diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 174a0d378b8a..2764e1292224 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -117,7 +117,7 @@ func TestLoadVersionPruning(t *testing.T) { for _, v := range []int64{1, 2, 4} { _, err = app.cms.CacheMultiStoreWithVersion(v) - require.NoError(t, err) + require.Error(t, err) } for _, v := range []int64{3, 5, 6, 7} { diff --git a/store/iavl/store.go b/store/iavl/store.go index 6314d670475f..7244530a21f4 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -116,7 +116,7 @@ func UnsafeNewStore(tree *iavl.MutableTree) *Store { // Any mutable operations executed will result in a panic. func (st *Store) GetImmutable(version int64) (*Store, error) { if !st.VersionExists(version) { - return nil, errors.New("version mismatch on immutable IAVL tree; version does not exist. Version has either been pruned, or is for a future block height") + return &Store{tree: &immutableTree{&iavl.ImmutableTree{}}}, fmt.Errorf("version mismatch on immutable IAVL tree; version does not exist. Version has either been pruned, or is for a future block height") } iTree, err := st.tree.GetImmutable(version) diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index 3476c8e5689f..5a17d3c8f762 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -87,7 +87,7 @@ func TestCacheMultiStoreWithVersion(t *testing.T) { cID := ms.Commit() require.Equal(t, int64(1), cID.Version) - // require no failure when given an invalid or pruned version + // require error when given an invalid or pruned version _, err = ms.CacheMultiStoreWithVersion(cID.Version + 1) require.Error(t, err) @@ -535,11 +535,11 @@ func TestMultiStore_Pruning(t *testing.T) { deleted []int64 saved []int64 }{ - {"prune nothing", 10, pruningtypes.NewPruningOptions(pruningtypes.PruningNothing), nil, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}, - {"prune everything", 12, pruningtypes.NewPruningOptions(pruningtypes.PruningEverything), []int64{1, 2, 3, 4, 5, 6, 7}, []int64{8, 9, 10, 11, 12}}, - {"prune some; no batch", 10, pruningtypes.NewCustomPruningOptions(2, 1), []int64{1, 2, 3, 4, 6, 5, 7}, []int64{8, 9, 10}}, - {"prune some; small batch", 10, pruningtypes.NewCustomPruningOptions(2, 3), []int64{1, 2, 3, 4, 5, 6}, []int64{7, 8, 9, 10}}, - {"prune some; large batch", 10, pruningtypes.NewCustomPruningOptions(2, 11), nil, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}, + {"prune nothing", 10, types.PruneNothing, nil, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}, + {"prune everything", 10, types.PruneEverything, []int64{1, 2, 3, 4, 5, 6, 7}, []int64{8, 9, 10}}, + {"prune some; no batch", 10, types.NewPruningOptions(2, 3, 1), []int64{1, 2, 4, 5, 7}, []int64{3, 6, 8, 9, 10}}, + {"prune some; small batch", 10, types.NewPruningOptions(2, 3, 3), []int64{1, 2, 4, 5}, []int64{3, 6, 7, 8, 9, 10}}, + {"prune some; large batch", 10, types.NewPruningOptions(2, 3, 11), nil, []int64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}, } for _, tc := range testCases { @@ -552,18 +552,14 @@ func TestMultiStore_Pruning(t *testing.T) { ms.Commit() } - for _, v := range tc.deleted { - // Ensure async pruning is done - checkErr := func() bool { - _, err := ms.CacheMultiStoreWithVersion(v) - return err != nil - } - require.Eventually(t, checkErr, 1*time.Second, 10*time.Millisecond, "expected error when loading height: %d", v) + for _, v := range tc.saved { + _, err := ms.CacheMultiStoreWithVersion(v) + require.NoError(t, err, "expected no error when loading height: %d", v) } for _, v := range tc.saved { _, err := ms.CacheMultiStoreWithVersion(v) - require.NoError(t, err, "expected no error when loading height: %d", v) + require.Error(t, err, "expected error when loading height: %d", v) } }) } @@ -641,8 +637,11 @@ func TestMultiStore_PruningRestart(t *testing.T) { // commit one more block and ensure the heights have been pruned ms.Commit() - actualHeightToPrune = ms.pruningManager.GetPruningHeight(ms.LatestVersion()) - require.Equal(t, int64(8), actualHeightToPrune) + for _, v := range pruneHeights { + _, err := ms.CacheMultiStoreWithVersion(v) + require.Error(t, err, "expected error when loading height: %d", v) + } +} func TestSetInitialVersion(t *testing.T) { db := coretesting.NewMemDB()