From e68a8b7804f09e86d049c1627b94283f0e4a6805 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 16 Sep 2024 12:18:50 +0200 Subject: [PATCH 1/3] feat(runtime(v2)): add sanity checks on store upgrades --- runtime/store.go | 42 ++++++++++++++++++++++++++++++++++++++++++ runtime/v2/store.go | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/runtime/store.go b/runtime/store.go index a6a2866fbeb3..dca6e9cd0c24 100644 --- a/runtime/store.go +++ b/runtime/store.go @@ -2,6 +2,8 @@ package runtime import ( "context" + "errors" + "fmt" "io" "cosmossdk.io/core/store" @@ -184,6 +186,11 @@ func KVStoreAdapter(store store.KVStore) storetypes.KVStore { // UpgradeStoreLoader is used to prepare baseapp with a fixed StoreLoader // pattern. This is useful for custom upgrade loading logic. func UpgradeStoreLoader(upgradeHeight int64, storeUpgrades *store.StoreUpgrades) baseapp.StoreLoader { + // sanity checks on store upgrades + if err := checkStoreUpgrade(storeUpgrades); err != nil { + panic(err) + } + return func(ms storetypes.CommitMultiStore) error { if upgradeHeight == ms.LastCommitID().Version+1 { // Check if the current commit version and upgrade height matches @@ -200,3 +207,38 @@ func UpgradeStoreLoader(upgradeHeight int64, storeUpgrades *store.StoreUpgrades) return baseapp.DefaultStoreLoader(ms) } } + +// checkStoreUpgrade performs sanity checks on the store upgrades +func checkStoreUpgrade(storeUpgrades *store.StoreUpgrades) error { + if storeUpgrades == nil { + return errors.New("store upgrades cannot be nil") + } + + // check for duplicates + var exists = make(map[string]bool) + for _, key := range storeUpgrades.Added { + if exists[key] { + return fmt.Errorf("store upgrade has duplicate key %s in added", key) + } + + if storeUpgrades.IsDeleted(key) { + return fmt.Errorf("store upgrade has key %s in both added and deleted", key) + } + + exists[key] = true + } + exists = make(map[string]bool) + for _, key := range storeUpgrades.Deleted { + if exists[key] { + return fmt.Errorf("store upgrade has duplicate key %s in deleted", key) + } + + if storeUpgrades.IsAdded(key) { + return fmt.Errorf("store upgrade has key %s in both added and deleted", key) + } + + exists[key] = true + } + + return nil +} diff --git a/runtime/v2/store.go b/runtime/v2/store.go index 5268033ad323..c18eae62701f 100644 --- a/runtime/v2/store.go +++ b/runtime/v2/store.go @@ -1,6 +1,7 @@ package runtime import ( + "errors" "fmt" "cosmossdk.io/core/store" @@ -69,6 +70,11 @@ func DefaultStoreLoader(store Store) error { // UpgradeStoreLoader upgrades the store if the upgrade height matches the current version, it is used as a replacement // for the DefaultStoreLoader when there are store upgrades func UpgradeStoreLoader(upgradeHeight int64, storeUpgrades *store.StoreUpgrades) StoreLoader { + // sanity checks on store upgrades + if err := checkStoreUpgrade(storeUpgrades); err != nil { + panic(err) + } + return func(store Store) error { latestVersion, err := store.GetLatestVersion() if err != nil { @@ -88,3 +94,38 @@ func UpgradeStoreLoader(upgradeHeight int64, storeUpgrades *store.StoreUpgrades) return DefaultStoreLoader(store) } } + +// checkStoreUpgrade performs sanity checks on the store upgrades +func checkStoreUpgrade(storeUpgrades *store.StoreUpgrades) error { + if storeUpgrades == nil { + return errors.New("store upgrades cannot be nil") + } + + // check for duplicates + var exists = make(map[string]bool) + for _, key := range storeUpgrades.Added { + if exists[key] { + return fmt.Errorf("store upgrade has duplicate key %s in added", key) + } + + if storeUpgrades.IsDeleted(key) { + return fmt.Errorf("store upgrade has key %s in both added and deleted", key) + } + + exists[key] = true + } + exists = make(map[string]bool) + for _, key := range storeUpgrades.Deleted { + if exists[key] { + return fmt.Errorf("store upgrade has duplicate key %s in deleted", key) + } + + if storeUpgrades.IsAdded(key) { + return fmt.Errorf("store upgrade has key %s in both added and deleted", key) + } + + exists[key] = true + } + + return nil +} From 7af497eb5bafd2df36e3ebf3150eb9002d223e46 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 17 Sep 2024 10:57:46 +0200 Subject: [PATCH 2/3] add tests --- runtime/store_test.go | 64 ++++++++++++++++++++++++++++++++++++ runtime/v2/store_test.go | 71 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100644 runtime/store_test.go create mode 100644 runtime/v2/store_test.go diff --git a/runtime/store_test.go b/runtime/store_test.go new file mode 100644 index 000000000000..229e218a9c22 --- /dev/null +++ b/runtime/store_test.go @@ -0,0 +1,64 @@ +package runtime + +import ( + "testing" + + corestore "cosmossdk.io/core/store" + "github.com/stretchr/testify/require" +) + +func TestCheckStoreUpgrade(t *testing.T) { + tests := []struct { + name string + storeUpgrades *corestore.StoreUpgrades + errMsg string + }{ + { + name: "Nil StoreUpgrades", + storeUpgrades: nil, + errMsg: "store upgrades cannot be nil", + }, + { + name: "Valid StoreUpgrades", + storeUpgrades: &corestore.StoreUpgrades{ + Added: []string{"store1", "store2"}, + Deleted: []string{"store3", "store4"}, + }, + }, + { + name: "Duplicate key in Added", + storeUpgrades: &corestore.StoreUpgrades{ + Added: []string{"store1", "store2", "store1"}, + Deleted: []string{"store3"}, + }, + errMsg: "store upgrade has duplicate key store1 in added", + }, + { + name: "Duplicate key in Deleted", + storeUpgrades: &corestore.StoreUpgrades{ + Added: []string{"store1"}, + Deleted: []string{"store2", "store3", "store2"}, + }, + errMsg: "store upgrade has duplicate key store2 in deleted", + }, + { + name: "Key in both Added and Deleted", + storeUpgrades: &corestore.StoreUpgrades{ + Added: []string{"store1", "store2"}, + Deleted: []string{"store2", "store3"}, + }, + errMsg: "store upgrade has key store2 in both added and deleted", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkStoreUpgrade(tt.storeUpgrades) + if tt.errMsg == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, tt.errMsg) + } + }) + } +} diff --git a/runtime/v2/store_test.go b/runtime/v2/store_test.go new file mode 100644 index 000000000000..64ad7e9cc274 --- /dev/null +++ b/runtime/v2/store_test.go @@ -0,0 +1,71 @@ +package runtime + +import ( + "testing" + + corestore "cosmossdk.io/core/store" +) + +func TestCheckStoreUpgrade(t *testing.T) { + tests := []struct { + name string + storeUpgrades *corestore.StoreUpgrades + wantErr bool + errMsg string + }{ + { + name: "Nil StoreUpgrades", + storeUpgrades: nil, + wantErr: true, + errMsg: "store upgrades cannot be nil", + }, + { + name: "Valid StoreUpgrades", + storeUpgrades: &corestore.StoreUpgrades{ + Added: []string{"store1", "store2"}, + Deleted: []string{"store3", "store4"}, + }, + wantErr: false, + }, + { + name: "Duplicate key in Added", + storeUpgrades: &corestore.StoreUpgrades{ + Added: []string{"store1", "store2", "store1"}, + Deleted: []string{"store3"}, + }, + wantErr: true, + errMsg: "store upgrade has duplicate key store1 in added", + }, + { + name: "Duplicate key in Deleted", + storeUpgrades: &corestore.StoreUpgrades{ + Added: []string{"store1"}, + Deleted: []string{"store2", "store3", "store2"}, + }, + wantErr: true, + errMsg: "store upgrade has duplicate key store2 in deleted", + }, + { + name: "Key in both Added and Deleted", + storeUpgrades: &corestore.StoreUpgrades{ + Added: []string{"store1", "store2"}, + Deleted: []string{"store2", "store3"}, + }, + wantErr: true, + errMsg: "store upgrade has key store2 in both added and deleted", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkStoreUpgrade(tt.storeUpgrades) + if (err != nil) != tt.wantErr { + t.Errorf("checkStoreUpgrade() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err != nil && err.Error() != tt.errMsg { + t.Errorf("checkStoreUpgrade() error message = %v, want %v", err.Error(), tt.errMsg) + } + }) + } +} From 6cf700d4c68fddd137bcd7450618957cbc4650b3 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Tue, 17 Sep 2024 12:35:59 +0200 Subject: [PATCH 3/3] lint --- runtime/store.go | 2 +- runtime/store_test.go | 3 ++- runtime/v2/store.go | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/runtime/store.go b/runtime/store.go index dca6e9cd0c24..298582087caf 100644 --- a/runtime/store.go +++ b/runtime/store.go @@ -215,7 +215,7 @@ func checkStoreUpgrade(storeUpgrades *store.StoreUpgrades) error { } // check for duplicates - var exists = make(map[string]bool) + exists := make(map[string]bool) for _, key := range storeUpgrades.Added { if exists[key] { return fmt.Errorf("store upgrade has duplicate key %s in added", key) diff --git a/runtime/store_test.go b/runtime/store_test.go index 229e218a9c22..a583a08d0391 100644 --- a/runtime/store_test.go +++ b/runtime/store_test.go @@ -3,8 +3,9 @@ package runtime import ( "testing" - corestore "cosmossdk.io/core/store" "github.com/stretchr/testify/require" + + corestore "cosmossdk.io/core/store" ) func TestCheckStoreUpgrade(t *testing.T) { diff --git a/runtime/v2/store.go b/runtime/v2/store.go index c18eae62701f..de05a27dfd12 100644 --- a/runtime/v2/store.go +++ b/runtime/v2/store.go @@ -102,7 +102,7 @@ func checkStoreUpgrade(storeUpgrades *store.StoreUpgrades) error { } // check for duplicates - var exists = make(map[string]bool) + exists := make(map[string]bool) for _, key := range storeUpgrades.Added { if exists[key] { return fmt.Errorf("store upgrade has duplicate key %s in added", key)