-
Notifications
You must be signed in to change notification settings - Fork 587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add mockVM for wasm clients testing #4804
Changes from 1 commit
877c0df
ed161e4
d43af71
2559a5b
e5187b4
842719d
cde7797
ef6b6ee
323b6ae
b1b62d1
c458d23
c088c77
6dbe416
af729ee
db819b0
4055830
23114ae
98087c2
2541e6c
7e51364
f3c57b9
7bdda21
d9b8f06
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
package mock | ||
|
||
import ( | ||
wasmvm "github.com/CosmWasm/wasmvm" | ||
wasmvmtypes "github.com/CosmWasm/wasmvm/types" | ||
|
||
"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" | ||
) | ||
|
||
var _ types.WasmEngine = &MockWasmEngine{} | ||
colin-axner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// MockWasmEngine implements types.WasmEngine for testing purpose. One or multiple messages can be stubbed. | ||
// Without a stub function a panic is thrown. | ||
type MockWasmEngine struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://github.com/CosmWasm/wasmd/blob/main/x/wasm/keeper/wasmtesting/mock_engine.go#L19, also very similar to how we mock ibc apps There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we can add this link in the code? Will be nice as an easy reference for making changes in the future. |
||
StoreCodeFn func(codeID wasmvm.WasmCode) (wasmvm.Checksum, error) | ||
InstantiateFn func(codeID wasmvm.Checksum, env wasmvmtypes.Env, info wasmvmtypes.MessageInfo, initMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) | ||
QueryFn func(codeID wasmvm.Checksum, env wasmvmtypes.Env, queryMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) ([]byte, uint64, error) | ||
SudoFn func(codeID wasmvm.Checksum, env wasmvmtypes.Env, sudoMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) | ||
GetCodeFn func(codeID wasmvm.Checksum) (wasmvm.WasmCode, error) | ||
PinFn func(checksum wasmvm.Checksum) error | ||
} | ||
|
||
func (m *MockWasmEngine) StoreCode(codeID wasmvm.WasmCode) (wasmvm.Checksum, error) { | ||
if m.StoreCodeFn == nil { | ||
panic("mock engine is not properly initialized") | ||
} | ||
return m.StoreCodeFn(codeID) | ||
} | ||
|
||
func (m *MockWasmEngine) Instantiate(codeID wasmvm.Checksum, env wasmvmtypes.Env, info wasmvmtypes.MessageInfo, initMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some of these funcs are only expected to perform a single action (initialize the contract or not), so I think a simple override of functionality makes sense for those |
||
if m.InstantiateFn == nil { | ||
panic("mock engine is not properly initialized") | ||
} | ||
return m.InstantiateFn(codeID, env, info, initMsg, store, goapi, querier, gasMeter, gasLimit, deserCost) | ||
} | ||
|
||
func (m *MockWasmEngine) Query(codeID wasmvm.Checksum, env wasmvmtypes.Env, queryMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) ([]byte, uint64, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other funcs like query are expected to handle a subset of queries, so we may actually want to make the subset actions overrideable, not the entire query fn itself I could remove the QueryFn field and replace with overrides for each subset action, then the QueryFn would use code magic to determine what the request is and then delegate to the appropriate response handler? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we utilize a Maybe expose a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happy to leave as is for the initial run and see if we can make improvements later 👍 |
||
if m.QueryFn == nil { | ||
panic("mock engine is not properly initialized") | ||
} | ||
return m.QueryFn(codeID, env, queryMsg, store, goapi, querier, gasMeter, gasLimit, deserCost) | ||
} | ||
|
||
func (m *MockWasmEngine) Sudo(codeID wasmvm.Checksum, env wasmvmtypes.Env, sudoMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) { | ||
if m.SudoFn == nil { | ||
panic("mock engine is not properly initialized") | ||
} | ||
return m.SudoFn(codeID, env, sudoMsg, store, goapi, querier, gasMeter, gasLimit, deserCost) | ||
} | ||
|
||
func (m *MockWasmEngine) GetCode(codeID wasmvm.Checksum) (wasmvm.WasmCode, error) { | ||
if m.GetCodeFn == nil { | ||
panic("mock engine is not properly initialized") | ||
} | ||
Comment on lines
+60
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a default of a hashmap maintaining code ids might be a nice default. |
||
return m.GetCodeFn(codeID) | ||
} | ||
|
||
func (m *MockWasmEngine) Pin(checksum wasmvm.Checksum) error { | ||
if m.PinFn == nil { | ||
panic("mock engine is not properly initialized") | ||
} | ||
Comment on lines
+68
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder maybe some of these fns can have defualts, for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so, I decided to hold off for now until we run into the reason they need a default. I attempted to bring in the existing default functionality for initialize and status to the mock engine, but it requires exposing access to a cdc and other fields the suite has, so I decided it was better to handle that later when we have more of the testing pattern defined |
||
return m.PinFn(checksum) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -281,6 +281,7 @@ func NewSimApp( | |
traceStore io.Writer, | ||
loadLatest bool, | ||
appOpts servertypes.AppOptions, | ||
mockVM wasmtypes.WasmEngine, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. quick hack, didn't want to spend so much time on this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could maybe just add an additional constructor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will leave as is for now, I figure once we get time to reduce the redundant simapps we will solve this issue, but also happy to see a followup with a nicer solution |
||
baseAppOptions ...func(*baseapp.BaseApp), | ||
) *SimApp { | ||
encodingConfig := makeEncodingConfig() | ||
|
@@ -488,7 +489,12 @@ func NewSimApp( | |
MemoryCacheSize: uint32(math.Pow(2, 8)), | ||
ContractDebugMode: false, | ||
} | ||
app.WasmClientKeeper = wasmkeeper.NewKeeperWithConfig(appCodec, keys[wasmtypes.StoreKey], authtypes.NewModuleAddress(govtypes.ModuleName).String(), wasmConfig) | ||
if mockVM != nil { | ||
// NOTE: mockVM is used for testing purposes only! | ||
app.WasmClientKeeper = wasmkeeper.NewKeeperWithVM(appCodec, keys[wasmtypes.StoreKey], authtypes.NewModuleAddress(govtypes.ModuleName).String(), mockVM) | ||
Comment on lines
+469
to
+471
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a bit icky but I think I can live with it, we'll need this for the E2Es to work I guess! |
||
} else { | ||
app.WasmClientKeeper = wasmkeeper.NewKeeperWithConfig(appCodec, keys[wasmtypes.StoreKey], authtypes.NewModuleAddress(govtypes.ModuleName).String(), wasmConfig) | ||
} | ||
|
||
// IBC Fee Module keeper | ||
app.IBCFeeKeeper = ibcfeekeeper.NewKeeper( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,13 @@ package types_test | |
import ( | ||
"crypto/sha256" | ||
"encoding/base64" | ||
"encoding/json" | ||
"errors" | ||
"time" | ||
|
||
wasmvm "github.com/CosmWasm/wasmvm" | ||
wasmvmtypes "github.com/CosmWasm/wasmvm/types" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
|
||
"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" | ||
|
@@ -89,11 +94,10 @@ func (suite *TypesTestSuite) TestStatusGrandpa() { | |
} | ||
} | ||
|
||
func (suite *TypesTestSuite) TestStatusTendermint() { | ||
func (suite *TypesTestSuite) TestStatus() { | ||
var ( | ||
path *ibctesting.Path | ||
clientState *types.ClientState | ||
tmClientState *tmtypes.ClientState | ||
path *ibctesting.Path | ||
clientState *types.ClientState | ||
) | ||
|
||
testCases := []struct { | ||
|
@@ -109,54 +113,58 @@ func (suite *TypesTestSuite) TestStatusTendermint() { | |
{ | ||
"client is frozen", | ||
func() { | ||
tmClientState.FrozenHeight = clienttypes.NewHeight(0, 1) | ||
|
||
wasmData, err := suite.chainA.Codec.MarshalInterface(tmClientState) | ||
suite.Require().NoError(err) | ||
|
||
clientState.Data = wasmData | ||
path.EndpointA.SetClientState(clientState) | ||
suite.mockVM.QueryFn = func(codeID wasmvm.Checksum, env wasmvmtypes.Env, queryMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) ([]byte, uint64, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is super nice, one thing that might be useful for readability is a function alias, but editors will auto fill this stuff for you so I don't feel super strongly about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't feel strongly either, so I'll let it be a followup if we so chose |
||
resp, err := json.Marshal(&mockStatusResult{ | ||
Status: exported.Frozen, | ||
}) | ||
suite.Require().NoError(err) | ||
gasUsed := uint64(10) // TODO | ||
return resp, gasUsed, nil | ||
} | ||
}, | ||
exported.Frozen, | ||
}, | ||
{ | ||
"client status without consensus state", | ||
"client status is expired", | ||
func() { | ||
latestHeight := clientState.LatestHeight.Increment().(clienttypes.Height) | ||
tmClientState.LatestHeight = latestHeight | ||
|
||
wasmData, err := suite.chainA.Codec.MarshalInterface(tmClientState) | ||
suite.Require().NoError(err) | ||
|
||
clientState.Data = wasmData | ||
clientState.LatestHeight = latestHeight | ||
path.EndpointA.SetClientState(clientState) | ||
suite.mockVM.QueryFn = func(codeID wasmvm.Checksum, env wasmvmtypes.Env, queryMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) ([]byte, uint64, error) { | ||
resp, err := json.Marshal(&mockStatusResult{ | ||
Status: exported.Expired, | ||
}) | ||
suite.Require().NoError(err) | ||
gasUsed := uint64(10) // TODO | ||
return resp, gasUsed, nil | ||
} | ||
}, | ||
exported.Expired, | ||
}, | ||
{ | ||
"client status is expired", | ||
"client status is unknown: vm returns an error", | ||
func() { | ||
suite.coordinator.IncrementTimeBy(tmClientState.TrustingPeriod) | ||
suite.mockVM.QueryFn = func(codeID wasmvm.Checksum, env wasmvmtypes.Env, queryMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) ([]byte, uint64, error) { | ||
return nil, 0, errors.New("client status not implemented") | ||
} | ||
}, | ||
exported.Expired, | ||
}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
suite.Run(tc.name, func() { | ||
suite.SetupWasmTendermint() | ||
suite.SetupWasmWithMockVM() | ||
|
||
path = ibctesting.NewPath(suite.chainA, suite.chainB) | ||
|
||
clientConfig, ok := path.EndpointA.ClientConfig.(*ibctesting.TendermintConfig) | ||
suite.Require().True(ok) | ||
clientConfig.IsWasmClient = true // TODO | ||
path.EndpointA.ClientConfig = clientConfig | ||
|
||
suite.coordinator.SetupClients(path) | ||
|
||
clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) | ||
clientState = path.EndpointA.GetClientState().(*types.ClientState) | ||
|
||
var cs exported.ClientState | ||
err := suite.chainA.Codec.UnmarshalInterface(clientState.Data, &cs) | ||
suite.Require().NoError(err) | ||
tmClientState = cs.(*tmtypes.ClientState) | ||
|
||
tc.malleate() | ||
|
||
status := clientState.Status(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec()) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package types | ||
|
||
/* | ||
This file is to allow for unexported functions and fields to be accessible to the testing package. | ||
*/ | ||
|
||
type StatusResult struct { | ||
statusResult | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this check doesn't work with mock vm when you have other testing also uses the wasmvm, so I decided to remove it, especially since we are going to manage global assignment internally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH I like this quite a bit, even though we're still dealing with global variables, removing the scope at which we assign them can never be a bad thing.