Skip to content

Commit

Permalink
Refactor 08-wasm with improvements after 02-client refactor (#6088)
Browse files Browse the repository at this point in the history
* tests: move tests from client state to light client module for 08-wasm.

* Remove usage of globals from 08-wasm (#6103)

* Move functionality from client state methods to light client module methods.

 - Yank all statements and delete functions as needed.
 - Make any required functions from types/ public.
 - Add the vm in the 08-wasm keeper.
 - Update documentation to include documentation of deleted functions.

* Remove global VM, introspection to retrieve client identifier.

 - Refactor signatures to explicitly pass vm, client identifier for time being.
 - Refactor tests to conform with new signatures.
 - Replace occurences of `ibcwasm.GetVM` in other keeper functions.

* Move migrateContract from clientState to keeper method.

- Matches moves for other functions.
- Move is required to address future circular dependency for state management (keeper would import types and types would import keeper)
- Rm migrate_contract(test).go files.

* Move state management to keeper.

- Remove globals for state (Checksums)
- Move access functions from types to keeper, move tests.
- Update calls using global to instead go through keeper.

* Make InitializePinnedCodes a method on the keeper.

* Remove pending todo in test for migrate contract.

This previously was testing a function that didn't set the client state after calling the contract (invoking through the keeper
always sets a client state after contract invocation). Hence, even if we don't set explicitly in the migrateFn callback, we _still_ get
a client state in the end.

* Move vm entrypoint functions in keeper/ package.

* feat: move vm entry points to keeper.

* chore: simplify signatures.

* test: add amended tests for contact keeper.

* refactor: move querier to keeper package.

* lint: you'll never fix me.

* refactor: remove queryRouter global variable

 - Move into the func for creating new plugins, pass it to acceptStargateQuery directly. This is probably what wasmd did (see they don't accept a query router now?).
   Since we don't use the get elsewhere, this was the smallest diff for a globa.
 - Preemptively ran make lint-fix, gotcha linting crybaby.

* refactor: remove global for query plugins.

* nit: rename to queryPlugins.

* refactor: remove QueryPluginsI interface.

* refactor: make queryHandler a private type.

* Make migrateContractCode a private function, clean up uneeded export in export_test.go for types/.

* refactor: Move vm entrypoints to keeper.

- Remove generic types and do unmarshalling in light client methods.
- Move all functions onto keeper and adjust call sites.

* chore: address some testing todos. Move testing unmarshal failure case to light client tests.

* chore: additional tiny nits.

- Consistently use wasmClientKeeper as local var name.
- Reorg keeper fields slightly.
- Rm 'vm.go' which was already empty.

* chore: restructure definitions to make diff more readable.

* d'oh: rm vm arg from instantiate.

* nit: Space out keeper fields. Remove TODO for method name.

* chore: address remaining TODOs.

* nit: don't hold ref to schema after building it.

* Update modules/light-clients/08-wasm/types/gas_register.go

Co-authored-by: Damian Nolan <[email protected]>

* lint: shut up

---------

Co-authored-by: Damian Nolan <[email protected]>
  • Loading branch information
DimitrisJim and damiannolan authored Apr 15, 2024
1 parent 89fdb1e commit ebabc1c
Show file tree
Hide file tree
Showing 42 changed files with 2,325 additions and 2,863 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
wasmvmtypes "github.com/CosmWasm/wasmvm/v2/types"

"github.com/cosmos/cosmos-sdk/baseapp"
sdk "github.com/cosmos/cosmos-sdk/types"
)

type WasmEngine interface {
Expand Down Expand Up @@ -123,8 +122,3 @@ type QueryRouter interface {
// if not found
Route(path string) baseapp.GRPCQueryHandler
}

type QueryPluginsI interface {
// HandleQuery will route the query to the correct plugin and return the result
HandleQuery(ctx sdk.Context, caller string, request wasmvmtypes.QueryRequest) ([]byte, error)
}
77 changes: 0 additions & 77 deletions modules/light-clients/08-wasm/internal/ibcwasm/wasm.go

This file was deleted.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package types_test
package keeper_test

import (
"encoding/json"
Expand All @@ -15,7 +15,7 @@ import (
localhost "github.com/cosmos/ibc-go/v8/modules/light-clients/09-localhost"
)

func (suite *TypesTestSuite) TestWasmInstantiate() {
func (suite *KeeperTestSuite) TestWasmInstantiate() {
testCases := []struct {
name string
malleate func()
Expand Down Expand Up @@ -165,17 +165,19 @@ func (suite *TypesTestSuite) TestWasmInstantiate() {
tc := tc
suite.Run(tc.name, func() {
suite.SetupWasmWithMockVM()
checksum := storeWasmCode(suite, wasmtesting.Code)

tc.malleate()

initMsg := types.InstantiateMessage{
ClientState: clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), wasmtesting.MockTendermitClientState),
ConsensusState: clienttypes.MustMarshalConsensusState(suite.chainA.App.AppCodec(), wasmtesting.MockTendermintClientConsensusState),
Checksum: suite.checksum,
Checksum: checksum,
}

clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), defaultWasmClientID)
err := types.WasmInstantiate(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, &types.ClientState{Checksum: suite.checksum}, initMsg)
wasmClientKeeper := GetSimApp(suite.chainA).WasmClientKeeper
err := wasmClientKeeper.WasmInstantiate(suite.chainA.GetContext(), defaultWasmClientID, clientStore, &types.ClientState{Checksum: checksum}, initMsg)

expPass := tc.expError == nil
if expPass {
Expand All @@ -187,7 +189,7 @@ func (suite *TypesTestSuite) TestWasmInstantiate() {
}
}

func (suite *TypesTestSuite) TestWasmMigrate() {
func (suite *KeeperTestSuite) TestWasmMigrate() {
testCases := []struct {
name string
malleate func()
Expand Down Expand Up @@ -305,6 +307,7 @@ func (suite *TypesTestSuite) TestWasmMigrate() {
tc := tc
suite.Run(tc.name, func() {
suite.SetupWasmWithMockVM()
_ = storeWasmCode(suite, wasmtesting.Code)

endpoint := wasmtesting.NewWasmEndpoint(suite.chainA)
err := endpoint.CreateClient()
Expand All @@ -313,7 +316,8 @@ func (suite *TypesTestSuite) TestWasmMigrate() {
tc.malleate()

clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), defaultWasmClientID)
err = types.WasmMigrate(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, &types.ClientState{}, defaultWasmClientID, []byte("{}"))
wasmClientKeeper := GetSimApp(suite.chainA).WasmClientKeeper
err = wasmClientKeeper.WasmMigrate(suite.chainA.GetContext(), clientStore, &types.ClientState{}, defaultWasmClientID, []byte("{}"))

expPass := tc.expError == nil
if expPass {
Expand All @@ -325,7 +329,7 @@ func (suite *TypesTestSuite) TestWasmMigrate() {
}
}

func (suite *TypesTestSuite) TestWasmQuery() {
func (suite *KeeperTestSuite) TestWasmQuery() {
var payload types.QueryMsg

testCases := []struct {
Expand Down Expand Up @@ -368,21 +372,13 @@ func (suite *TypesTestSuite) TestWasmQuery() {
},
types.ErrWasmContractCallFailed,
},
{
"failure: response fails to unmarshal",
func() {
suite.mockVM.RegisterQueryCallback(types.StatusMsg{}, func(_ wasmvm.Checksum, _ wasmvmtypes.Env, _ []byte, _ wasmvm.KVStore, _ wasmvm.GoAPI, _ wasmvm.Querier, _ wasmvm.GasMeter, _ uint64, _ wasmvmtypes.UFraction) (*wasmvmtypes.QueryResult, uint64, error) {
return &wasmvmtypes.QueryResult{Ok: []byte("invalid json")}, wasmtesting.DefaultGasUsed, nil
})
},
types.ErrWasmInvalidResponseData,
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupWasmWithMockVM()
_ = storeWasmCode(suite, wasmtesting.Code)

endpoint := wasmtesting.NewWasmEndpoint(suite.chainA)
err := endpoint.CreateClient()
Expand All @@ -398,7 +394,8 @@ func (suite *TypesTestSuite) TestWasmQuery() {

tc.malleate()

res, err := types.WasmQuery[types.StatusResult](suite.chainA.GetContext(), clientStore, wasmClientState, payload)
wasmClientKeeper := GetSimApp(suite.chainA).WasmClientKeeper
res, err := wasmClientKeeper.WasmQuery(suite.chainA.GetContext(), endpoint.ClientID, clientStore, wasmClientState, payload)

expPass := tc.expError == nil
if expPass {
Expand All @@ -411,7 +408,7 @@ func (suite *TypesTestSuite) TestWasmQuery() {
}
}

func (suite *TypesTestSuite) TestWasmSudo() {
func (suite *KeeperTestSuite) TestWasmSudo() {
var payload types.SudoMsg

testCases := []struct {
Expand Down Expand Up @@ -487,15 +484,6 @@ func (suite *TypesTestSuite) TestWasmSudo() {
},
types.ErrWasmAttributesNotAllowed,
},
{
"failure: response fails to unmarshal",
func() {
suite.mockVM.RegisterSudoCallback(types.UpdateStateMsg{}, func(_ wasmvm.Checksum, _ wasmvmtypes.Env, _ []byte, _ wasmvm.KVStore, _ wasmvm.GoAPI, _ wasmvm.Querier, _ wasmvm.GasMeter, _ uint64, _ wasmvmtypes.UFraction) (*wasmvmtypes.ContractResult, uint64, error) {
return &wasmvmtypes.ContractResult{Ok: &wasmvmtypes.Response{Data: []byte("invalid json")}}, wasmtesting.DefaultGasUsed, nil
})
},
types.ErrWasmInvalidResponseData,
},
{
"failure: invalid clientstate type",
func() {
Expand Down Expand Up @@ -561,6 +549,7 @@ func (suite *TypesTestSuite) TestWasmSudo() {
tc := tc
suite.Run(tc.name, func() {
suite.SetupWasmWithMockVM()
_ = storeWasmCode(suite, wasmtesting.Code)

endpoint := wasmtesting.NewWasmEndpoint(suite.chainA)
err := endpoint.CreateClient()
Expand All @@ -576,7 +565,8 @@ func (suite *TypesTestSuite) TestWasmSudo() {

tc.malleate()

res, err := types.WasmSudo[types.UpdateStateResult](suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, wasmClientState, payload)
wasmClientKeeper := GetSimApp(suite.chainA).WasmClientKeeper
res, err := wasmClientKeeper.WasmSudo(suite.chainA.GetContext(), endpoint.ClientID, clientStore, wasmClientState, payload)

expPass := tc.expError == nil
if expPass {
Expand Down
8 changes: 8 additions & 0 deletions modules/light-clients/08-wasm/keeper/export_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package keeper

import sdk "github.com/cosmos/cosmos-sdk/types"

// MigrateContractCode is a wrapper around k.migrateContractCode to allow the method to be directly called in tests.
func (k Keeper) MigrateContractCode(ctx sdk.Context, clientID string, newChecksum, migrateMsg []byte) error {
return k.migrateContractCode(ctx, clientID, newChecksum, migrateMsg)
}
9 changes: 4 additions & 5 deletions modules/light-clients/08-wasm/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm"
"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types"
)

// InitGenesis initializes the 08-wasm module's state from a provided genesis
// state.
func (k Keeper) InitGenesis(ctx sdk.Context, gs types.GenesisState) error {
storeFn := func(code wasmvm.WasmCode, _ uint64) (wasmvm.Checksum, uint64, error) {
checksum, err := ibcwasm.GetVM().StoreCodeUnchecked(code)
checksum, err := k.GetVM().StoreCodeUnchecked(code)
return checksum, 0, err
}

Expand All @@ -28,16 +27,16 @@ func (k Keeper) InitGenesis(ctx sdk.Context, gs types.GenesisState) error {

// ExportGenesis returns the 08-wasm module's exported genesis. This includes the code
// for all contracts previously stored.
func (Keeper) ExportGenesis(ctx sdk.Context) types.GenesisState {
checksums, err := types.GetAllChecksums(ctx)
func (k Keeper) ExportGenesis(ctx sdk.Context) types.GenesisState {
checksums, err := k.GetAllChecksums(ctx)
if err != nil {
panic(err)
}

// Grab code from wasmVM and add to genesis state.
var genesisState types.GenesisState
for _, checksum := range checksums {
code, err := ibcwasm.GetVM().GetCode(checksum)
code, err := k.GetVM().GetCode(checksum)
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion modules/light-clients/08-wasm/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
suite.Require().NoError(err)

var storedHashes []string
checksums, err := types.GetAllChecksums(suite.chainA.GetContext())
checksums, err := GetSimApp(suite.chainA).WasmClientKeeper.GetAllChecksums(suite.chainA.GetContext())
suite.Require().NoError(err)

for _, hash := range checksums {
Expand Down
11 changes: 5 additions & 6 deletions modules/light-clients/08-wasm/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,13 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
sdkquery "github.com/cosmos/cosmos-sdk/types/query"

"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm"
"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types"
)

var _ types.QueryServer = (*Keeper)(nil)

// Code implements the Query/Code gRPC method
func (Keeper) Code(goCtx context.Context, req *types.QueryCodeRequest) (*types.QueryCodeResponse, error) {
func (k Keeper) Code(goCtx context.Context, req *types.QueryCodeRequest) (*types.QueryCodeResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}
Expand All @@ -31,11 +30,11 @@ func (Keeper) Code(goCtx context.Context, req *types.QueryCodeRequest) (*types.Q
}

// Only return checksums we previously stored, not arbitrary checksums that might be stored via e.g Wasmd.
if !types.HasChecksum(sdk.UnwrapSDKContext(goCtx), checksum) {
if !k.HasChecksum(sdk.UnwrapSDKContext(goCtx), checksum) {
return nil, status.Error(codes.NotFound, errorsmod.Wrap(types.ErrWasmChecksumNotFound, req.Checksum).Error())
}

code, err := ibcwasm.GetVM().GetCode(checksum)
code, err := k.GetVM().GetCode(checksum)
if err != nil {
return nil, status.Error(codes.NotFound, errorsmod.Wrap(types.ErrWasmChecksumNotFound, req.Checksum).Error())
}
Expand All @@ -46,10 +45,10 @@ func (Keeper) Code(goCtx context.Context, req *types.QueryCodeRequest) (*types.Q
}

// Checksums implements the Query/Checksums gRPC method. It returns a list of hex encoded checksums stored.
func (Keeper) Checksums(goCtx context.Context, req *types.QueryChecksumsRequest) (*types.QueryChecksumsResponse, error) {
func (k Keeper) Checksums(goCtx context.Context, req *types.QueryChecksumsRequest) (*types.QueryChecksumsResponse, error) {
checksums, pageRes, err := sdkquery.CollectionPaginate(
goCtx,
ibcwasm.Checksums,
k.GetChecksums(),
req.Pagination,
func(key []byte, value collections.NoValue) (string, error) {
return hex.EncodeToString(key), nil
Expand Down
Loading

0 comments on commit ebabc1c

Please sign in to comment.