From e2ad31975f2ede592912b86346b5ebf055c9e05f Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Fri, 19 Apr 2024 16:25:32 +0200 Subject: [PATCH] 08-wasm: rename client store (#6187) * rename client store * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * review comment * fix * more fixing --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .../08-wasm/internal/types/store.go | 66 +++++++++---------- .../08-wasm/internal/types/store_test.go | 36 +++++----- .../08-wasm/keeper/contract_keeper.go | 2 +- .../08-wasm/light_client_module.go | 2 +- 4 files changed, 53 insertions(+), 53 deletions(-) diff --git a/modules/light-clients/08-wasm/internal/types/store.go b/modules/light-clients/08-wasm/internal/types/store.go index a35d57995a2..11ffd559d94 100644 --- a/modules/light-clients/08-wasm/internal/types/store.go +++ b/modules/light-clients/08-wasm/internal/types/store.go @@ -14,24 +14,24 @@ import ( var ( _ wasmvmtypes.KVStore = &StoreAdapter{} - _ storetypes.KVStore = &MergedClientStore{} + _ storetypes.KVStore = &ClientRecoveryStore{} SubjectPrefix = []byte("subject/") SubstitutePrefix = []byte("substitute/") ) -// MergedClientStore combines two KVStores into one. +// ClientRecoveryStore combines two KVStores into one. // // Both stores are used for reads, but only the subjectStore is used for writes. For all operations, the key // is checked to determine which types to use and must be prefixed with either "subject/" or "substitute/" accordingly. // If the key is not prefixed with either "subject/" or "substitute/", a default action is taken (e.g. no-op for Set/Delete). -type MergedClientStore struct { +type ClientRecoveryStore struct { subjectStore storetypes.KVStore substituteStore storetypes.KVStore } -// NewMergedClientStore retusn a new instance of a MergedClientStore -func NewMergedClientStore(subjectStore, substituteStore storetypes.KVStore) MergedClientStore { +// NewClientRecoveryStore returns a new instance of a ClientRecoveryStore +func NewClientRecoveryStore(subjectStore, substituteStore storetypes.KVStore) ClientRecoveryStore { if subjectStore == nil { panic(errors.New("subjectStore must not be nil")) } @@ -39,7 +39,7 @@ func NewMergedClientStore(subjectStore, substituteStore storetypes.KVStore) Merg panic(errors.New("substituteStore must not be nil")) } - return MergedClientStore{ + return ClientRecoveryStore{ subjectStore: subjectStore, substituteStore: substituteStore, } @@ -48,10 +48,10 @@ func NewMergedClientStore(subjectStore, substituteStore storetypes.KVStore) Merg // Get implements the storetypes.KVStore interface. It allows reads from both the subjectStore and substituteStore. // // Get will return an empty byte slice if the key is not prefixed with either "subject/" or "substitute/". -func (ws MergedClientStore) Get(key []byte) []byte { +func (s ClientRecoveryStore) Get(key []byte) []byte { prefix, key := SplitPrefix(key) - store, found := ws.GetStore(prefix) + store, found := s.GetStore(prefix) if !found { // return a nil byte slice as KVStore.Get() does by default return []byte(nil) @@ -63,10 +63,10 @@ func (ws MergedClientStore) Get(key []byte) []byte { // Has implements the storetypes.KVStore interface. It allows reads from both the subjectStore and substituteStore. // // Note: contracts do not have access to the Has method, it is only implemented here to satisfy the storetypes.KVStore interface. -func (ws MergedClientStore) Has(key []byte) bool { +func (s ClientRecoveryStore) Has(key []byte) bool { prefix, key := SplitPrefix(key) - store, found := ws.GetStore(prefix) + store, found := s.GetStore(prefix) if !found { // return false as value when types is not found return false @@ -78,40 +78,40 @@ func (ws MergedClientStore) Has(key []byte) bool { // Set implements the storetypes.KVStore interface. It allows writes solely to the subjectStore. // // Set will no-op if the key is not prefixed with "subject/". -func (ws MergedClientStore) Set(key, value []byte) { +func (s ClientRecoveryStore) Set(key, value []byte) { prefix, key := SplitPrefix(key) if !bytes.Equal(prefix, SubjectPrefix) { return // no-op } - ws.subjectStore.Set(key, value) + s.subjectStore.Set(key, value) } // Delete implements the storetypes.KVStore interface. It allows deletions solely to the subjectStore. // // Delete will no-op if the key is not prefixed with "subject/". -func (ws MergedClientStore) Delete(key []byte) { +func (s ClientRecoveryStore) Delete(key []byte) { prefix, key := SplitPrefix(key) if !bytes.Equal(prefix, SubjectPrefix) { return // no-op } - ws.subjectStore.Delete(key) + s.subjectStore.Delete(key) } // Iterator implements the storetypes.KVStore interface. It allows iteration over both the subjectStore and substituteStore. // // Iterator will return a closed iterator if the start or end keys are not prefixed with either "subject/" or "substitute/". -func (ws MergedClientStore) Iterator(start, end []byte) storetypes.Iterator { +func (s ClientRecoveryStore) Iterator(start, end []byte) storetypes.Iterator { prefixStart, start := SplitPrefix(start) prefixEnd, end := SplitPrefix(end) if !bytes.Equal(prefixStart, prefixEnd) { - return ws.closedIterator() + return s.closedIterator() } - store, found := ws.GetStore(prefixStart) + store, found := s.GetStore(prefixStart) if !found { - return ws.closedIterator() + return s.closedIterator() } return store.Iterator(start, end) @@ -120,35 +120,35 @@ func (ws MergedClientStore) Iterator(start, end []byte) storetypes.Iterator { // ReverseIterator implements the storetypes.KVStore interface. It allows iteration over both the subjectStore and substituteStore. // // ReverseIterator will return a closed iterator if the start or end keys are not prefixed with either "subject/" or "substitute/". -func (ws MergedClientStore) ReverseIterator(start, end []byte) storetypes.Iterator { +func (s ClientRecoveryStore) ReverseIterator(start, end []byte) storetypes.Iterator { prefixStart, start := SplitPrefix(start) prefixEnd, end := SplitPrefix(end) if !bytes.Equal(prefixStart, prefixEnd) { - return ws.closedIterator() + return s.closedIterator() } - store, found := ws.GetStore(prefixStart) + store, found := s.GetStore(prefixStart) if !found { - return ws.closedIterator() + return s.closedIterator() } return store.ReverseIterator(start, end) } // GetStoreType implements the storetypes.KVStore interface, it is implemented solely to satisfy the interface. -func (ws MergedClientStore) GetStoreType() storetypes.StoreType { - return ws.substituteStore.GetStoreType() +func (s ClientRecoveryStore) GetStoreType() storetypes.StoreType { + return s.substituteStore.GetStoreType() } // CacheWrap implements the storetypes.KVStore interface, it is implemented solely to satisfy the interface. -func (ws MergedClientStore) CacheWrap() storetypes.CacheWrap { - return cachekv.NewStore(ws) +func (s ClientRecoveryStore) CacheWrap() storetypes.CacheWrap { + return cachekv.NewStore(s) } // CacheWrapWithTrace implements the storetypes.KVStore interface, it is implemented solely to satisfy the interface. -func (ws MergedClientStore) CacheWrapWithTrace(w io.Writer, tc storetypes.TraceContext) storetypes.CacheWrap { - return cachekv.NewStore(tracekv.NewStore(ws, w, tc)) +func (s ClientRecoveryStore) CacheWrapWithTrace(w io.Writer, tc storetypes.TraceContext) storetypes.CacheWrap { + return cachekv.NewStore(tracekv.NewStore(s, w, tc)) } // getStore returns the types to be used for the given key and a boolean flag indicating if that types was found. @@ -156,11 +156,11 @@ func (ws MergedClientStore) CacheWrapWithTrace(w io.Writer, tc storetypes.TraceC // the substituteStore is returned. // // If the key is not prefixed with either "subject/" or "substitute/", a nil types is returned and the boolean flag is false. -func (ws MergedClientStore) GetStore(prefix []byte) (storetypes.KVStore, bool) { +func (s ClientRecoveryStore) GetStore(prefix []byte) (storetypes.KVStore, bool) { if bytes.Equal(prefix, SubjectPrefix) { - return ws.subjectStore, true + return s.subjectStore, true } else if bytes.Equal(prefix, SubstitutePrefix) { - return ws.substituteStore, true + return s.substituteStore, true } return nil, false @@ -168,9 +168,9 @@ func (ws MergedClientStore) GetStore(prefix []byte) (storetypes.KVStore, bool) { // closedIterator returns an iterator that is always closed, used when Iterator() or ReverseIterator() is called // with an invalid prefix or start/end key. -func (ws MergedClientStore) closedIterator() storetypes.Iterator { +func (s ClientRecoveryStore) closedIterator() storetypes.Iterator { // Create a dummy iterator that is always closed right away. - it := ws.subjectStore.Iterator([]byte{0}, []byte{1}) + it := s.subjectStore.Iterator([]byte{0}, []byte{1}) it.Close() return it diff --git a/modules/light-clients/08-wasm/internal/types/store_test.go b/modules/light-clients/08-wasm/internal/types/store_test.go index 6debb6ff2cf..f6dd0f9ee41 100644 --- a/modules/light-clients/08-wasm/internal/types/store_test.go +++ b/modules/light-clients/08-wasm/internal/types/store_test.go @@ -64,8 +64,8 @@ func setupTestingApp() (ibctesting.TestingApp, map[string]json.RawMessage) { return app, app.DefaultGenesis() } -// TestMergedClientStoreGetStore tests the GetStore method of the MergedClientStore. -func (suite *TypesTestSuite) TestMergedClientStoreGetStore() { +// TestClientRecoveryStoreGetStore tests the GetStore method of the ClientRecoveryStore. +func (suite *TypesTestSuite) TestClientRecoveryStoreGetStore() { subjectStore, substituteStore := suite.GetSubjectAndSubstituteStore() testCases := []struct { @@ -98,7 +98,7 @@ func (suite *TypesTestSuite) TestMergedClientStoreGetStore() { for _, tc := range testCases { tc := tc suite.Run(tc.name, func() { - wrappedStore := internaltypes.NewMergedClientStore(subjectStore, substituteStore) + wrappedStore := internaltypes.NewClientRecoveryStore(subjectStore, substituteStore) store, found := wrappedStore.GetStore(tc.prefix) @@ -156,8 +156,8 @@ func (suite *TypesTestSuite) TestSplitPrefix() { } } -// TestMergedClientStoreGet tests the Get method of the MergedClientStore. -func (suite *TypesTestSuite) TestMergedClientStoreGet() { +// TestClientRecoveryStoreGet tests the Get method of the ClientRecoveryStore. +func (suite *TypesTestSuite) TestClientRecoveryStoreGet() { subjectStore, substituteStore := suite.GetSubjectAndSubstituteStore() testCases := []struct { @@ -189,7 +189,7 @@ func (suite *TypesTestSuite) TestMergedClientStoreGet() { for _, tc := range testCases { tc := tc suite.Run(tc.name, func() { - wrappedStore := internaltypes.NewMergedClientStore(subjectStore, substituteStore) + wrappedStore := internaltypes.NewClientRecoveryStore(subjectStore, substituteStore) prefixedKey := tc.prefix prefixedKey = append(prefixedKey, tc.key...) @@ -207,8 +207,8 @@ func (suite *TypesTestSuite) TestMergedClientStoreGet() { } } -// TestMergedClientStoreSet tests the Set method of the MergedClientStore. -func (suite *TypesTestSuite) TestMergedClientStoreSet() { +// TestClientRecoveryStoreSet tests the Set method of the ClientRecoveryStore. +func (suite *TypesTestSuite) TestClientRecoveryStoreSet() { testCases := []struct { name string prefix []byte @@ -233,7 +233,7 @@ func (suite *TypesTestSuite) TestMergedClientStoreSet() { tc := tc suite.Run(tc.name, func() { subjectStore, substituteStore := suite.GetSubjectAndSubstituteStore() - wrappedStore := internaltypes.NewMergedClientStore(subjectStore, substituteStore) + wrappedStore := internaltypes.NewClientRecoveryStore(subjectStore, substituteStore) prefixedKey := tc.prefix prefixedKey = append(prefixedKey, tc.key...) @@ -257,8 +257,8 @@ func (suite *TypesTestSuite) TestMergedClientStoreSet() { } } -// TestMergedClientStoreDelete tests the Delete method of the MergedClientStore. -func (suite *TypesTestSuite) TestMergedClientStoreDelete() { +// TestClientRecoveryStoreDelete tests the Delete method of the ClientRecoveryStore. +func (suite *TypesTestSuite) TestClientRecoveryStoreDelete() { var ( mockStoreKey = []byte("mock-key") mockStoreValue = []byte("mock-value") @@ -293,7 +293,7 @@ func (suite *TypesTestSuite) TestMergedClientStoreDelete() { subjectStore.Set(mockStoreKey, mockStoreValue) substituteStore.Set(mockStoreKey, mockStoreValue) - wrappedStore := internaltypes.NewMergedClientStore(subjectStore, substituteStore) + wrappedStore := internaltypes.NewClientRecoveryStore(subjectStore, substituteStore) prefixedKey := tc.prefix prefixedKey = append(prefixedKey, tc.key...) @@ -315,8 +315,8 @@ func (suite *TypesTestSuite) TestMergedClientStoreDelete() { } } -// TestMergedClientStoreIterators tests the Iterator/ReverseIterator methods of the MergedClientStore. -func (suite *TypesTestSuite) TestMergedClientStoreIterators() { +// TestClientRecoveryStoreIterators tests the Iterator/ReverseIterator methods of the ClientRecoveryStore. +func (suite *TypesTestSuite) TestClientRecoveryStoreIterators() { subjectStore, substituteStore := suite.GetSubjectAndSubstituteStore() testCases := []struct { @@ -364,7 +364,7 @@ func (suite *TypesTestSuite) TestMergedClientStoreIterators() { for _, tc := range testCases { tc := tc suite.Run(tc.name, func() { - wrappedStore := internaltypes.NewMergedClientStore(subjectStore, substituteStore) + wrappedStore := internaltypes.NewClientRecoveryStore(subjectStore, substituteStore) prefixedKeyStart := tc.prefixStart prefixedKeyStart = append(prefixedKeyStart, tc.start...) @@ -383,7 +383,7 @@ func (suite *TypesTestSuite) TestMergedClientStoreIterators() { } } -func (suite *TypesTestSuite) TestNewMergedClientStore() { +func (suite *TypesTestSuite) TestNewClientRecoveryStore() { subjectStore, substituteStore := suite.GetSubjectAndSubstituteStore() testCases := []struct { @@ -420,11 +420,11 @@ func (suite *TypesTestSuite) TestNewMergedClientStore() { expPass := !tc.expPanic if expPass { suite.Require().NotPanics(func() { - internaltypes.NewMergedClientStore(subjectStore, substituteStore) + internaltypes.NewClientRecoveryStore(subjectStore, substituteStore) }) } else { suite.Require().Panics(func() { - internaltypes.NewMergedClientStore(subjectStore, substituteStore) + internaltypes.NewClientRecoveryStore(subjectStore, substituteStore) }) } }) diff --git a/modules/light-clients/08-wasm/keeper/contract_keeper.go b/modules/light-clients/08-wasm/keeper/contract_keeper.go index 0d66523625a..18810aef285 100644 --- a/modules/light-clients/08-wasm/keeper/contract_keeper.go +++ b/modules/light-clients/08-wasm/keeper/contract_keeper.go @@ -216,7 +216,7 @@ func (k Keeper) WasmQuery(ctx sdk.Context, clientID string, clientStore storetyp // - the client state is of type *ClientState func validatePostExecutionClientState(clientStore storetypes.KVStore, cdc codec.BinaryCodec) (*types.ClientState, error) { key := host.ClientStateKey() - _, ok := clientStore.(internaltypes.MergedClientStore) + _, ok := clientStore.(internaltypes.ClientRecoveryStore) if ok { key = append(internaltypes.SubjectPrefix, key...) } diff --git a/modules/light-clients/08-wasm/light_client_module.go b/modules/light-clients/08-wasm/light_client_module.go index 6b6ce45725d..1467a6834ed 100644 --- a/modules/light-clients/08-wasm/light_client_module.go +++ b/modules/light-clients/08-wasm/light_client_module.go @@ -450,7 +450,7 @@ func (l LightClientModule) RecoverClient(ctx sdk.Context, clientID, substituteCl return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected checksums to be equal: expected %s, got %s", hex.EncodeToString(subjectClientState.Checksum), hex.EncodeToString(substituteClientState.Checksum)) } - store := internaltypes.NewMergedClientStore(subjectClientStore, substituteClientStore) + store := internaltypes.NewClientRecoveryStore(subjectClientStore, substituteClientStore) payload := types.SudoMsg{ MigrateClientStore: &types.MigrateClientStoreMsg{},