From cb74fd1bf66a6a64fa7fc372befa084356c9f8e9 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 6 Dec 2023 01:03:40 +0100 Subject: [PATCH] chore: address vanity nits in 08-wasm (code structure, godocs) (#5320) * chore: restructure mock_engine.go * chore: consolidate sdk.Msg codec registration * chore: adding godoc to WasmConfig * chore: move state storage code to store.go, rm ambiguous wasm.go and wasm_test.go files * chore: add more meat to mock engine godoc * chore: update storeAdapter godoc to use wasmvmtypes.KVStore * lint: yes * Update modules/light-clients/08-wasm/testing/mock_engine.go --- .../08-wasm/testing/mock_engine.go | 45 ++++--- modules/light-clients/08-wasm/types/codec.go | 6 - modules/light-clients/08-wasm/types/config.go | 3 + modules/light-clients/08-wasm/types/store.go | 53 ++++++++ .../light-clients/08-wasm/types/store_test.go | 118 +++++++++++++++++ modules/light-clients/08-wasm/types/wasm.go | 54 -------- .../light-clients/08-wasm/types/wasm_test.go | 124 ------------------ 7 files changed, 199 insertions(+), 204 deletions(-) delete mode 100644 modules/light-clients/08-wasm/types/wasm.go delete mode 100644 modules/light-clients/08-wasm/types/wasm_test.go diff --git a/modules/light-clients/08-wasm/testing/mock_engine.go b/modules/light-clients/08-wasm/testing/mock_engine.go index e15d0ca0cd6..6e6e985e65a 100644 --- a/modules/light-clients/08-wasm/testing/mock_engine.go +++ b/modules/light-clients/08-wasm/testing/mock_engine.go @@ -34,6 +34,31 @@ type ( sudoFn func(checksum 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) ) +// MockWasmEngine implements types.WasmEngine for testing purposes. One or multiple messages can be stubbed. +// Without a stub function a panic is thrown. +// ref: https://github.com/CosmWasm/wasmd/blob/v0.42.0/x/wasm/keeper/wasmtesting/mock_engine.go#L19 +type MockWasmEngine struct { + StoreCodeFn func(code wasmvm.WasmCode) (wasmvm.Checksum, error) + StoreCodeUncheckedFn func(code wasmvm.WasmCode) (wasmvm.Checksum, error) + InstantiateFn func(checksum 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) + MigrateFn func(checksum wasmvm.Checksum, env wasmvmtypes.Env, migrateMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) + GetCodeFn func(checksum wasmvm.Checksum) (wasmvm.WasmCode, error) + PinFn func(checksum wasmvm.Checksum) error + UnpinFn func(checksum wasmvm.Checksum) error + + // queryCallbacks contains a mapping of queryMsg field type name to callback function. + queryCallbacks map[string]queryFn + sudoCallbacks map[string]sudoFn + + // contracts contains a mapping of checksum to code. + storedContracts map[uint32][]byte +} + +// NewMockWasmEngine creates and returns a new instance of the mock wasmvm for testing purposes. +// Each callback method of the mock wasmvm can be overridden to assign specific functionality. +// Default functionality is assigned for StoreCode, StoreCodeUnchecked and GetCode. Both Pin and Unpin are implemented as no-op methods. +// All other callbacks stored in the query and sudo callback maps panic. Use RegisterQueryCallback and RegisterSudoCallback methods +// to assign expected behaviour for test cases. func NewMockWasmEngine() *MockWasmEngine { m := &MockWasmEngine{ queryCallbacks: map[string]queryFn{}, @@ -107,26 +132,6 @@ func (m *MockWasmEngine) RegisterSudoCallback(sudoMessage any, fn sudoFn) { m.sudoCallbacks[typeName] = fn } -// MockWasmEngine implements types.WasmEngine for testing purpose. One or multiple messages can be stubbed. -// Without a stub function a panic is thrown. -// ref: https://github.com/CosmWasm/wasmd/blob/v0.42.0/x/wasm/keeper/wasmtesting/mock_engine.go#L19 -type MockWasmEngine struct { - StoreCodeFn func(code wasmvm.WasmCode) (wasmvm.Checksum, error) - StoreCodeUncheckedFn func(code wasmvm.WasmCode) (wasmvm.Checksum, error) - InstantiateFn func(checksum 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) - MigrateFn func(checksum wasmvm.Checksum, env wasmvmtypes.Env, migrateMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) - GetCodeFn func(checksum wasmvm.Checksum) (wasmvm.WasmCode, error) - PinFn func(checksum wasmvm.Checksum) error - UnpinFn func(checksum wasmvm.Checksum) error - - // queryCallbacks contains a mapping of queryMsg field type name to callback function. - queryCallbacks map[string]queryFn - sudoCallbacks map[string]sudoFn - - // contracts contains a mapping of checksum to code. - storedContracts map[uint32][]byte -} - // StoreCode implements the WasmEngine interface. func (m *MockWasmEngine) StoreCode(code wasmvm.WasmCode) (wasmvm.Checksum, error) { if m.StoreCodeFn == nil { diff --git a/modules/light-clients/08-wasm/types/codec.go b/modules/light-clients/08-wasm/types/codec.go index 275809d98f7..e588477260f 100644 --- a/modules/light-clients/08-wasm/types/codec.go +++ b/modules/light-clients/08-wasm/types/codec.go @@ -26,13 +26,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) { registry.RegisterImplementations( (*sdk.Msg)(nil), &MsgStoreCode{}, - ) - registry.RegisterImplementations( - (*sdk.Msg)(nil), &MsgMigrateContract{}, - ) - registry.RegisterImplementations( - (*sdk.Msg)(nil), &MsgRemoveChecksum{}, ) diff --git a/modules/light-clients/08-wasm/types/config.go b/modules/light-clients/08-wasm/types/config.go index 7e947b3471e..da4ac531cd0 100644 --- a/modules/light-clients/08-wasm/types/config.go +++ b/modules/light-clients/08-wasm/types/config.go @@ -15,6 +15,9 @@ const ( defaultContractDebugMode = false ) +// WasmConfig defines configuration parameters for the 08-wasm wasm virtual machine instance. +// It includes the `dataDir` intended to be used for wasm blobs and internal caches, as well as a comma separated list +// of features or capabilities the user wishes to enable. A boolean flag is provided to enable debug mode. type WasmConfig struct { // DataDir is the directory for Wasm blobs and various caches DataDir string diff --git a/modules/light-clients/08-wasm/types/store.go b/modules/light-clients/08-wasm/types/store.go index c83f7aeab0b..9f6cbcb49b7 100644 --- a/modules/light-clients/08-wasm/types/store.go +++ b/modules/light-clients/08-wasm/types/store.go @@ -2,11 +2,13 @@ package types import ( "bytes" + "context" "errors" "io" "reflect" "strings" + wasmvm "github.com/CosmWasm/wasmvm" wasmvmtypes "github.com/CosmWasm/wasmvm/types" errorsmod "cosmossdk.io/errors" @@ -14,6 +16,8 @@ import ( storeprefix "cosmossdk.io/store/prefix" "cosmossdk.io/store/tracekv" storetypes "cosmossdk.io/store/types" + + "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm" ) var ( @@ -24,6 +28,50 @@ var ( substitutePrefix = []byte("substitute/") ) +// Checksum is a type alias used for wasm byte code checksums. +type Checksum = wasmvmtypes.Checksum + +// CreateChecksum creates a sha256 checksum from the given wasm code, it forwards the +// call to the wasmvm package. The code is checked for the following conditions: +// - code length is zero. +// - code length is less than 4 bytes (magic number length). +// - code does not start with the wasm magic number. +func CreateChecksum(code []byte) (Checksum, error) { + return wasmvm.CreateChecksum(code) +} + +// GetAllChecksums is a helper to get all checksums from the store. +// It returns an empty slice if no checksums are found +func GetAllChecksums(ctx context.Context) ([]Checksum, error) { + iterator, err := ibcwasm.Checksums.Iterate(ctx, nil) + if err != nil { + return nil, err + } + + keys, err := iterator.Keys() + if err != nil { + return nil, err + } + + checksums := []Checksum{} + for _, key := range keys { + checksums = append(checksums, key) + } + + return checksums, nil +} + +// HasChecksum returns true if the given checksum exists in the store and +// false otherwise. +func HasChecksum(ctx context.Context, checksum Checksum) bool { + found, err := ibcwasm.Checksums.Has(ctx, checksum) + if err != nil { + return false + } + + return found +} + // migrateClientWrappedStore combines two KVStores into one. // // Both stores are used for reads, but only the subjectStore is used for writes. For all operations, the key @@ -205,22 +253,27 @@ func newStoreAdapter(s storetypes.KVStore) *storeAdapter { return &storeAdapter{parent: s} } +// Get implements the wasmvmtypes.KVStore interface. func (s storeAdapter) Get(key []byte) []byte { return s.parent.Get(key) } +// Set implements the wasmvmtypes.KVStore interface. func (s storeAdapter) Set(key, value []byte) { s.parent.Set(key, value) } +// Delete implements the wasmvmtypes.KVStore interface. func (s storeAdapter) Delete(key []byte) { s.parent.Delete(key) } +// Iterator implements the wasmvmtypes.KVStore interface. func (s storeAdapter) Iterator(start, end []byte) wasmvmtypes.Iterator { return s.parent.Iterator(start, end) } +// ReverseIterator implements the wasmvmtypes.KVStore interface. func (s storeAdapter) ReverseIterator(start, end []byte) wasmvmtypes.Iterator { return s.parent.ReverseIterator(start, end) } diff --git a/modules/light-clients/08-wasm/types/store_test.go b/modules/light-clients/08-wasm/types/store_test.go index 78e5e8e29b2..7037ccbe0aa 100644 --- a/modules/light-clients/08-wasm/types/store_test.go +++ b/modules/light-clients/08-wasm/types/store_test.go @@ -4,6 +4,7 @@ import ( prefixstore "cosmossdk.io/store/prefix" storetypes "cosmossdk.io/store/types" + "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm" wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" @@ -11,6 +12,123 @@ import ( var invalidPrefix = []byte("invalid/") +func (suite *TypesTestSuite) TestGetChecksums() { + testCases := []struct { + name string + malleate func() + expResult func(checksums []types.Checksum) + }{ + { + "success: no contract stored.", + func() {}, + func(checksums []types.Checksum) { + suite.Require().Len(checksums, 0) + }, + }, + { + "success: default mock vm contract stored.", + func() { + suite.SetupWasmWithMockVM() + }, + func(checksums []types.Checksum) { + suite.Require().Len(checksums, 1) + expectedChecksum, err := types.CreateChecksum(wasmtesting.Code) + suite.Require().NoError(err) + suite.Require().Equal(expectedChecksum, checksums[0]) + }, + }, + { + "success: non-empty checksums", + func() { + suite.SetupWasmWithMockVM() + + err := ibcwasm.Checksums.Set(suite.chainA.GetContext(), types.Checksum("checksum")) + suite.Require().NoError(err) + }, + func(checksums []types.Checksum) { + suite.Require().Len(checksums, 2) + suite.Require().Contains(checksums, types.Checksum("checksum")) + }, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + tc.malleate() + + checksums, err := types.GetAllChecksums(suite.chainA.GetContext()) + suite.Require().NoError(err) + tc.expResult(checksums) + }) + } +} + +func (suite *TypesTestSuite) TestAddChecksum() { + suite.SetupWasmWithMockVM() + + checksums, err := types.GetAllChecksums(suite.chainA.GetContext()) + suite.Require().NoError(err) + // default mock vm contract is stored + suite.Require().Len(checksums, 1) + + checksum1 := types.Checksum("checksum1") + checksum2 := types.Checksum("checksum2") + err = ibcwasm.Checksums.Set(suite.chainA.GetContext(), checksum1) + suite.Require().NoError(err) + err = ibcwasm.Checksums.Set(suite.chainA.GetContext(), checksum2) + suite.Require().NoError(err) + + // Test adding the same checksum twice + err = ibcwasm.Checksums.Set(suite.chainA.GetContext(), checksum1) + suite.Require().NoError(err) + + checksums, err = types.GetAllChecksums(suite.chainA.GetContext()) + suite.Require().NoError(err) + suite.Require().Len(checksums, 3) + suite.Require().Contains(checksums, checksum1) + suite.Require().Contains(checksums, checksum2) +} + +func (suite *TypesTestSuite) TestHasChecksum() { + var checksum types.Checksum + + testCases := []struct { + name string + malleate func() + exprResult bool + }{ + { + "success: checksum exists", + func() { + checksum = types.Checksum("checksum") + err := ibcwasm.Checksums.Set(suite.chainA.GetContext(), checksum) + suite.Require().NoError(err) + }, + true, + }, + { + "success: checksum does not exist", + func() { + checksum = types.Checksum("non-existent-checksum") + }, + false, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupWasmWithMockVM() + + tc.malleate() + + result := types.HasChecksum(suite.chainA.GetContext(), checksum) + suite.Require().Equal(tc.exprResult, result) + }) + } +} + // TestMigrateClientWrappedStoreGetStore tests the getStore method of the migrateClientWrappedStore. func (suite *TypesTestSuite) TestMigrateClientWrappedStoreGetStore() { // calls suite.SetupWasmWithMockVM() and creates two clients with their respective stores diff --git a/modules/light-clients/08-wasm/types/wasm.go b/modules/light-clients/08-wasm/types/wasm.go deleted file mode 100644 index cfec1c03b2b..00000000000 --- a/modules/light-clients/08-wasm/types/wasm.go +++ /dev/null @@ -1,54 +0,0 @@ -package types - -import ( - "context" - - wasmvm "github.com/CosmWasm/wasmvm" - wasmvmtypes "github.com/CosmWasm/wasmvm/types" - - "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm" -) - -// Checksum is a type alias used for wasm byte code checksums. -type Checksum = wasmvmtypes.Checksum - -// CreateChecksum creates a sha256 checksum from the given wasm code, it forwards the -// call to the wasmvm package. The code is checked for the following conditions: -// - code length is zero. -// - code length is less than 4 bytes (magic number length). -// - code does not start with the wasm magic number. -func CreateChecksum(code []byte) (Checksum, error) { - return wasmvm.CreateChecksum(code) -} - -// GetAllChecksums is a helper to get all checksums from the store. -// It returns an empty slice if no checksums are found -func GetAllChecksums(ctx context.Context) ([]Checksum, error) { - iterator, err := ibcwasm.Checksums.Iterate(ctx, nil) - if err != nil { - return nil, err - } - - keys, err := iterator.Keys() - if err != nil { - return nil, err - } - - checksums := []Checksum{} - for _, key := range keys { - checksums = append(checksums, key) - } - - return checksums, nil -} - -// HasChecksum returns true if the given checksum exists in the store and -// false otherwise. -func HasChecksum(ctx context.Context, checksum Checksum) bool { - found, err := ibcwasm.Checksums.Has(ctx, checksum) - if err != nil { - return false - } - - return found -} diff --git a/modules/light-clients/08-wasm/types/wasm_test.go b/modules/light-clients/08-wasm/types/wasm_test.go deleted file mode 100644 index 65b9b6574c5..00000000000 --- a/modules/light-clients/08-wasm/types/wasm_test.go +++ /dev/null @@ -1,124 +0,0 @@ -package types_test - -import ( - "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm" - wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" - "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" -) - -func (suite *TypesTestSuite) TestGetChecksums() { - testCases := []struct { - name string - malleate func() - expResult func(checksums []types.Checksum) - }{ - { - "success: no contract stored.", - func() {}, - func(checksums []types.Checksum) { - suite.Require().Len(checksums, 0) - }, - }, - { - "success: default mock vm contract stored.", - func() { - suite.SetupWasmWithMockVM() - }, - func(checksums []types.Checksum) { - suite.Require().Len(checksums, 1) - expectedChecksum, err := types.CreateChecksum(wasmtesting.Code) - suite.Require().NoError(err) - suite.Require().Equal(expectedChecksum, checksums[0]) - }, - }, - { - "success: non-empty checksums", - func() { - suite.SetupWasmWithMockVM() - - err := ibcwasm.Checksums.Set(suite.chainA.GetContext(), types.Checksum("checksum")) - suite.Require().NoError(err) - }, - func(checksums []types.Checksum) { - suite.Require().Len(checksums, 2) - suite.Require().Contains(checksums, types.Checksum("checksum")) - }, - }, - } - - for _, tc := range testCases { - tc := tc - suite.Run(tc.name, func() { - tc.malleate() - - checksums, err := types.GetAllChecksums(suite.chainA.GetContext()) - suite.Require().NoError(err) - tc.expResult(checksums) - }) - } -} - -func (suite *TypesTestSuite) TestAddChecksum() { - suite.SetupWasmWithMockVM() - - checksums, err := types.GetAllChecksums(suite.chainA.GetContext()) - suite.Require().NoError(err) - // default mock vm contract is stored - suite.Require().Len(checksums, 1) - - checksum1 := types.Checksum("checksum1") - checksum2 := types.Checksum("checksum2") - err = ibcwasm.Checksums.Set(suite.chainA.GetContext(), checksum1) - suite.Require().NoError(err) - err = ibcwasm.Checksums.Set(suite.chainA.GetContext(), checksum2) - suite.Require().NoError(err) - - // Test adding the same checksum twice - err = ibcwasm.Checksums.Set(suite.chainA.GetContext(), checksum1) - suite.Require().NoError(err) - - checksums, err = types.GetAllChecksums(suite.chainA.GetContext()) - suite.Require().NoError(err) - suite.Require().Len(checksums, 3) - suite.Require().Contains(checksums, checksum1) - suite.Require().Contains(checksums, checksum2) -} - -func (suite *TypesTestSuite) TestHasChecksum() { - var checksum types.Checksum - - testCases := []struct { - name string - malleate func() - exprResult bool - }{ - { - "success: checksum exists", - func() { - checksum = types.Checksum("checksum") - err := ibcwasm.Checksums.Set(suite.chainA.GetContext(), checksum) - suite.Require().NoError(err) - }, - true, - }, - { - "success: checksum does not exist", - func() { - checksum = types.Checksum("non-existent-checksum") - }, - false, - }, - } - - for _, tc := range testCases { - tc := tc - suite.Run(tc.name, func() { - suite.SetupWasmWithMockVM() - - tc.malleate() - - result := types.HasChecksum(suite.chainA.GetContext(), checksum) - suite.Require().Equal(tc.exprResult, result) - }) - } -}