Skip to content

Commit

Permalink
08-wasm: rename client store (cosmos#6187)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
Carlos Rodriguez and coderabbitai[bot] authored Apr 19, 2024
1 parent 247068b commit e2ad319
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 53 deletions.
66 changes: 33 additions & 33 deletions modules/light-clients/08-wasm/internal/types/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,32 @@ 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"))
}
if substituteStore == nil {
panic(errors.New("substituteStore must not be nil"))
}

return MergedClientStore{
return ClientRecoveryStore{
subjectStore: subjectStore,
substituteStore: substituteStore,
}
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -120,57 +120,57 @@ 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.
// If the key is prefixed with "subject/", the subjectStore is returned. If the key is prefixed with "substitute/",
// 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
}

// 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
Expand Down
36 changes: 18 additions & 18 deletions modules/light-clients/08-wasm/internal/types/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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...)
Expand All @@ -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
Expand All @@ -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...)
Expand All @@ -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")
Expand Down Expand Up @@ -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...)
Expand All @@ -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 {
Expand Down Expand Up @@ -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...)
Expand All @@ -383,7 +383,7 @@ func (suite *TypesTestSuite) TestMergedClientStoreIterators() {
}
}

func (suite *TypesTestSuite) TestNewMergedClientStore() {
func (suite *TypesTestSuite) TestNewClientRecoveryStore() {
subjectStore, substituteStore := suite.GetSubjectAndSubstituteStore()

testCases := []struct {
Expand Down Expand Up @@ -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)
})
}
})
Expand Down
2 changes: 1 addition & 1 deletion modules/light-clients/08-wasm/keeper/contract_keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}
Expand Down
2 changes: 1 addition & 1 deletion modules/light-clients/08-wasm/light_client_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down

0 comments on commit e2ad319

Please sign in to comment.