Skip to content

Commit

Permalink
chore: address some additional nits. (#6176)
Browse files Browse the repository at this point in the history
Make GetQueryPlugins and SetQueryPlugins as moved on the keeper private.
Rename var to match semantics.
Remove unecessary client state look-up in migrate contract.
  • Loading branch information
DimitrisJim authored Apr 17, 2024
1 parent 1c4b41f commit 4d3623d
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 25 deletions.
10 changes: 10 additions & 0 deletions modules/light-clients/08-wasm/keeper/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,13 @@ import sdk "github.com/cosmos/cosmos-sdk/types"
func (k Keeper) MigrateContractCode(ctx sdk.Context, clientID string, newChecksum, migrateMsg []byte) error {
return k.migrateContractCode(ctx, clientID, newChecksum, migrateMsg)
}

// GetQueryPlugins is a wrapper around k.getQueryPlugins to allow the method to be directly called in tests.
func (k Keeper) GetQueryPlugins() QueryPlugins {
return k.getQueryPlugins()
}

// SetQueryPlugins is a wrapper around k.setQueryPlugins to allow the method to be directly called in tests.
func (k *Keeper) SetQueryPlugins(plugins QueryPlugins) {
k.setQueryPlugins(plugins)
}
25 changes: 10 additions & 15 deletions modules/light-clients/08-wasm/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,18 @@ func (k Keeper) GetChecksums() collections.KeySet[[]byte] {
return k.checksums
}

// GetQueryPlugins returns the set query plugins.
func (k Keeper) GetQueryPlugins() QueryPlugins {
// getQueryPlugins returns the set query plugins.
func (k Keeper) getQueryPlugins() QueryPlugins {
return k.queryPlugins
}

// SetQueryPlugins sets the plugins.
func (k *Keeper) SetQueryPlugins(plugins QueryPlugins) {
func (k *Keeper) setQueryPlugins(plugins QueryPlugins) {
k.queryPlugins = plugins
}

func (k Keeper) newQueryHandler(ctx sdk.Context, callerID string) *queryHandler {
return newQueryHandler(ctx, k.GetQueryPlugins(), callerID)
return newQueryHandler(ctx, k.getQueryPlugins(), callerID)
}

// storeWasmCode stores the contract to the VM, pins the checksum in the VM's in memory cache and stores the checksum
Expand Down Expand Up @@ -142,32 +142,27 @@ func (k Keeper) storeWasmCode(ctx sdk.Context, code []byte, storeFn func(code wa
// migrateContractCode migrates the contract for a given light client to one denoted by the given new checksum. The checksum we
// are migrating to must first be stored using storeWasmCode and must not match the checksum currently stored for this light client.
func (k Keeper) migrateContractCode(ctx sdk.Context, clientID string, newChecksum, migrateMsg []byte) error {
wasmClientState, err := k.GetWasmClientState(ctx, clientID)
if err != nil {
return errorsmod.Wrap(err, "failed to retrieve wasm client state")
}
oldChecksum := wasmClientState.Checksum

clientStore := k.clientKeeper.ClientStore(ctx, clientID)
clientState, found := types.GetClientState(clientStore, k.cdc)
wasmClientState, found := types.GetClientState(clientStore, k.cdc)
if !found {
return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
}
oldChecksum := wasmClientState.Checksum

if !k.HasChecksum(ctx, newChecksum) {
return types.ErrWasmChecksumNotFound
}

if bytes.Equal(clientState.Checksum, newChecksum) {
return errorsmod.Wrapf(types.ErrWasmCodeExists, "new checksum (%s) is the same as current checksum (%s)", hex.EncodeToString(newChecksum), hex.EncodeToString(clientState.Checksum))
if bytes.Equal(wasmClientState.Checksum, newChecksum) {
return errorsmod.Wrapf(types.ErrWasmCodeExists, "new checksum (%s) is the same as current checksum (%s)", hex.EncodeToString(newChecksum), hex.EncodeToString(wasmClientState.Checksum))
}

// update the checksum, this needs to be done before the contract migration
// so that wasmMigrate can call the right code. Note that this is not
// persisted to the client store.
clientState.Checksum = newChecksum
wasmClientState.Checksum = newChecksum

err = k.WasmMigrate(ctx, clientStore, clientState, clientID, migrateMsg)
err := k.WasmMigrate(ctx, clientStore, wasmClientState, clientID, migrateMsg)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion modules/light-clients/08-wasm/keeper/keeper_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func NewKeeperWithVM(

// set query plugins to ensure there is a non-nil query plugin
// regardless of what options the user provides
keeper.SetQueryPlugins(NewDefaultQueryPlugins(queryRouter))
keeper.setQueryPlugins(NewDefaultQueryPlugins(queryRouter))

for _, opt := range opts {
opt.apply(keeper)
Expand Down
2 changes: 1 addition & 1 deletion modules/light-clients/08-wasm/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func (suite *KeeperTestSuite) TestMsgMigrateContract() {
func() {
msg = types.NewMsgMigrateContract(govAcc, ibctesting.InvalidID, newChecksum, []byte("{}"))
},
clienttypes.ErrClientTypeNotFound,
clienttypes.ErrClientNotFound,
},
{
"failure: vm returns error",
Expand Down
4 changes: 2 additions & 2 deletions modules/light-clients/08-wasm/keeper/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ func (f optsFn) apply(keeper *Keeper) {
// Missing fields will be filled with default queriers.
func WithQueryPlugins(plugins *QueryPlugins) Option {
return optsFn(func(k *Keeper) {
currentPlugins := k.GetQueryPlugins()
currentPlugins := k.getQueryPlugins()
newPlugins := currentPlugins.Merge(plugins)

k.SetQueryPlugins(newPlugins)
k.setQueryPlugins(newPlugins)
})
}
12 changes: 6 additions & 6 deletions modules/light-clients/08-wasm/light_client_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,8 +431,8 @@ func (l LightClientModule) RecoverClient(ctx sdk.Context, clientID, substituteCl

cdc := l.keeper.Codec()

clientStore := l.storeProvider.ClientStore(ctx, clientID)
clientState, found := types.GetClientState(clientStore, cdc)
subjectClientStore := l.storeProvider.ClientStore(ctx, clientID)
subjectClientState, found := types.GetClientState(subjectClientStore, cdc)
if !found {
return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID)
}
Expand All @@ -445,17 +445,17 @@ func (l LightClientModule) RecoverClient(ctx sdk.Context, clientID, substituteCl

// check that checksums of subject client state and substitute client state match
// changing the checksum is only allowed through the migrate contract RPC endpoint
if !bytes.Equal(clientState.Checksum, substituteClientState.Checksum) {
return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected checksums to be equal: expected %s, got %s", hex.EncodeToString(clientState.Checksum), hex.EncodeToString(substituteClientState.Checksum))
if !bytes.Equal(subjectClientState.Checksum, substituteClientState.Checksum) {
return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected checksums to be equal: expected %s, got %s", hex.EncodeToString(subjectClientState.Checksum), hex.EncodeToString(substituteClientState.Checksum))
}

store := types.NewMigrateClientWrappedStore(clientStore, substituteClientStore)
store := types.NewMigrateClientWrappedStore(subjectClientStore, substituteClientStore)

payload := types.SudoMsg{
MigrateClientStore: &types.MigrateClientStoreMsg{},
}

_, err = l.keeper.WasmSudo(ctx, clientID, store, clientState, payload)
_, err = l.keeper.WasmSudo(ctx, clientID, store, subjectClientState, payload)
return err
}

Expand Down

0 comments on commit 4d3623d

Please sign in to comment.