From af529b8c828a53a37e598281ebf6130a31d89082 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/iavl/store_test.go | 2 +- store/rootmulti/store_test.go | 12 ++++++------ 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 627045e2f2f4..85e695a35008 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,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). diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index f9cdfcb71438..d7b8c4b7fbd3 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -108,7 +108,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 a44cc1d531b3..5429f853d948 100644 --- a/store/iavl/store.go +++ b/store/iavl/store.go @@ -109,7 +109,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 &Store{tree: &immutableTree{&iavl.ImmutableTree{}}}, nil + 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/iavl/store_test.go b/store/iavl/store_test.go index f1cd586e36a0..5c120f290194 100644 --- a/store/iavl/store_test.go +++ b/store/iavl/store_test.go @@ -127,7 +127,7 @@ func TestGetImmutable(t *testing.T) { require.Nil(t, err) _, err = store.GetImmutable(cID.Version + 1) - require.NoError(t, err) + require.Error(t, err) newStore, err := store.GetImmutable(cID.Version - 1) require.NoError(t, err) diff --git a/store/rootmulti/store_test.go b/store/rootmulti/store_test.go index fd706dd3c987..3bb006019764 100644 --- a/store/rootmulti/store_test.go +++ b/store/rootmulti/store_test.go @@ -85,9 +85,9 @@ 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.NoError(t, err) + require.Error(t, err) // require a valid version can be cache-loaded cms, err := ms.CacheMultiStoreWithVersion(cID.Version) @@ -499,7 +499,7 @@ func TestMultiStore_Pruning(t *testing.T) { saved []int64 }{ {"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, 8, 9}, []int64{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}}, @@ -519,12 +519,12 @@ func TestMultiStore_Pruning(t *testing.T) { for _, v := range tc.saved { _, err := ms.CacheMultiStoreWithVersion(v) - require.NoError(t, err, "expected error when loading height: %d", v) + require.NoError(t, err, "expected no error when loading height: %d", v) } for _, v := range tc.deleted { _, err := ms.CacheMultiStoreWithVersion(v) - require.NoError(t, err, "expected error when loading height: %d", v) + require.Error(t, err, "expected error when loading height: %d", v) } }) } @@ -560,7 +560,7 @@ func TestMultiStore_PruningRestart(t *testing.T) { for _, v := range pruneHeights { _, err := ms.CacheMultiStoreWithVersion(v) - require.NoError(t, err, "expected error when loading height: %d", v) + require.Error(t, err, "expected error when loading height: %d", v) } }