Validate before calculating checksum, move private functions at end, …

…add context to mockvm errors. (#5157) * Validate before calculating checksum, move private functions at end in vm.go, add context to mockvm errors. * panic with errors.
cosmos · Nov 22, 2023 · 4bb8edd · 4bb8edd
1 parent 9e203b8
commit 4bb8edd
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 54 deletions.
diff --git a/modules/light-clients/08-wasm/keeper/keeper.go b/modules/light-clients/08-wasm/keeper/keeper.go
@@ -101,6 +101,11 @@ func (Keeper) storeWasmCode(ctx sdk.Context, code []byte) ([]byte, error) {
 		}
 	}
 
+	// run the code through the wasm light client validation process
+	if err := types.ValidateWasmCode(code); err != nil {
+		return nil, errorsmod.Wrap(err, "wasm bytecode validation failed")
+	}
+
 	// Check to see if store already has checksum.
 	checksum, err := types.CreateChecksum(code)
 	if err != nil {
@@ -111,11 +116,6 @@ func (Keeper) storeWasmCode(ctx sdk.Context, code []byte) ([]byte, error) {
 		return nil, types.ErrWasmCodeExists
 	}
 
-	// run the code through the wasm light client validation process
-	if err := types.ValidateWasmCode(code); err != nil {
-		return nil, errorsmod.Wrap(err, "wasm bytecode validation failed")
-	}
-
 	// create the code in the vm
 	ctx.GasMeter().ConsumeGas(types.VMGasRegister.CompileCosts(len(code)), "Compiling wasm bytecode")
 	vmChecksum, err := ibcwasm.GetVM().StoreCode(code)

diff --git a/modules/light-clients/08-wasm/keeper/msg_server_test.go b/modules/light-clients/08-wasm/keeper/msg_server_test.go
@@ -56,7 +56,7 @@ func (suite *KeeperTestSuite) TestMsgStoreCode() {
 			func() {
 				msg = types.NewMsgStoreCode(signer, []byte{})
 			},
-			errors.New("Wasm bytes nil or empty"),
+			types.ErrWasmEmptyCode,
 		},
 		{
 			"fails with checksum",

diff --git a/modules/light-clients/08-wasm/testing/mock_engine.go b/modules/light-clients/08-wasm/testing/mock_engine.go
@@ -122,15 +122,15 @@ type MockWasmEngine struct {
 // StoreCode implements the WasmEngine interface.
 func (m *MockWasmEngine) StoreCode(code wasmvm.WasmCode) (wasmvm.Checksum, error) {
 	if m.StoreCodeFn == nil {
-		panic("mock engine is not properly initialized")
+		panic(errors.New("mock engine is not properly initialized: StoreCodeFn is nil"))
 	}
 	return m.StoreCodeFn(code)
 }
 
 // Instantiate implements the WasmEngine interface.
 func (m *MockWasmEngine) Instantiate(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) {
 	if m.InstantiateFn == nil {
-		panic("mock engine is not properly initialized")
+		panic(errors.New("mock engine is not properly initialized: InstantiateFn is nil"))
 	}
 	return m.InstantiateFn(checksum, env, info, initMsg, store, goapi, querier, gasMeter, gasLimit, deserCost)
 }
@@ -141,7 +141,7 @@ func (m *MockWasmEngine) Query(checksum wasmvm.Checksum, env wasmvmtypes.Env, qu
 
 	callbackFn, ok := m.queryCallbacks[msgTypeName]
 	if !ok {
-		panic(fmt.Errorf("no callback specified for %s", msgTypeName))
+		panic(fmt.Errorf("mock engine is not properly initialized: no callback specified for %s", msgTypeName))
 	}
 
 	return callbackFn(checksum, env, queryMsg, store, goapi, querier, gasMeter, gasLimit, deserCost)
@@ -150,7 +150,7 @@ func (m *MockWasmEngine) Query(checksum wasmvm.Checksum, env wasmvmtypes.Env, qu
 // Migrate implements the WasmEngine interface.
 func (m *MockWasmEngine) Migrate(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) {
 	if m.MigrateFn == nil {
-		panic("mock engine is not properly initialized")
+		panic(errors.New("mock engine is not properly initialized: MigrateFn is nil"))
 	}
 	return m.MigrateFn(checksum, env, migrateMsg, store, goapi, querier, gasMeter, gasLimit, deserCost)
 }
@@ -161,7 +161,7 @@ func (m *MockWasmEngine) Sudo(checksum wasmvm.Checksum, env wasmvmtypes.Env, sud
 
 	sudoFn, ok := m.sudoCallbacks[msgTypeName]
 	if !ok {
-		panic(fmt.Errorf("no callback specified for %s", msgTypeName))
+		panic(fmt.Errorf("mock engine is not properly initialized: no callback specified for %s", msgTypeName))
 	}
 
 	return sudoFn(checksum, env, sudoMsg, store, goapi, querier, gasMeter, gasLimit, deserCost)
@@ -170,23 +170,23 @@ func (m *MockWasmEngine) Sudo(checksum wasmvm.Checksum, env wasmvmtypes.Env, sud
 // GetCode implements the WasmEngine interface.
 func (m *MockWasmEngine) GetCode(checksum wasmvm.Checksum) (wasmvm.WasmCode, error) {
 	if m.GetCodeFn == nil {
-		panic("mock engine is not properly initialized")
+		panic(errors.New("mock engine is not properly initialized: GetCodeFn is nil"))
 	}
 	return m.GetCodeFn(checksum)
 }
 
 // Pin implements the WasmEngine interface.
 func (m *MockWasmEngine) Pin(checksum wasmvm.Checksum) error {
 	if m.PinFn == nil {
-		panic("mock engine is not properly initialized")
+		panic(errors.New("mock engine is not properly initialized: PinFn is nil"))
 	}
 	return m.PinFn(checksum)
 }
 
 // Unpin implements the WasmEngine interface.
 func (m *MockWasmEngine) Unpin(checksum wasmvm.Checksum) error {
 	if m.UnpinFn == nil {
-		panic("mock engine is not properly initialized")
+		panic(errors.New("mock engine is not properly initialized: UnpinFn is nil"))
 	}
 	return m.UnpinFn(checksum)
 }

diff --git a/modules/light-clients/08-wasm/types/vm.go b/modules/light-clients/08-wasm/types/vm.go
@@ -177,46 +177,6 @@ func wasmSudo[T ContractResult](ctx sdk.Context, cdc codec.BinaryCodec, clientSt
 	return result, nil
 }
 
-// validatePostExecutionClientState validates that the contract has not many any invalid modifications
-// to the client state during execution. It ensures that
-// - the client state is still present
-// - the client state can be unmarshaled successfully.
-// - the client state is of type *ClientState
-func validatePostExecutionClientState(clientStore storetypes.KVStore, cdc codec.BinaryCodec) (*ClientState, error) {
-	key := host.ClientStateKey()
-	_, ok := clientStore.(migrateClientWrappedStore)
-	if ok {
-		key = append(subjectPrefix, key...)
-	}
-
-	bz := clientStore.Get(key)
-	if len(bz) == 0 {
-		return nil, errorsmod.Wrap(ErrWasmInvalidContractModification, types.ErrClientNotFound.Error())
-	}
-
-	clientState, err := unmarshalClientState(cdc, bz)
-	if err != nil {
-		return nil, errorsmod.Wrap(ErrWasmInvalidContractModification, err.Error())
-	}
-
-	cs, ok := clientState.(*ClientState)
-	if !ok {
-		return nil, errorsmod.Wrapf(ErrWasmInvalidContractModification, "expected client state type %T, got %T", (*ClientState)(nil), clientState)
-	}
-
-	return cs, nil
-}
-
-// unmarshalClientState unmarshals the client state from the given bytes.
-func unmarshalClientState(cdc codec.BinaryCodec, bz []byte) (exported.ClientState, error) {
-	var clientState exported.ClientState
-	if err := cdc.UnmarshalInterface(bz, &clientState); err != nil {
-		return nil, err
-	}
-
-	return clientState, nil
-}
-
 // wasmMigrate migrate calls the migrate entry point of the contract with the given payload and returns the result.
 // wasmMigrate returns an error if:
 // - the contract migration returns an error
@@ -259,6 +219,46 @@ func wasmQuery[T ContractResult](ctx sdk.Context, clientStore storetypes.KVStore
 	return result, nil
 }
 
+// validatePostExecutionClientState validates that the contract has not many any invalid modifications
+// to the client state during execution. It ensures that
+// - the client state is still present
+// - the client state can be unmarshaled successfully.
+// - the client state is of type *ClientState
+func validatePostExecutionClientState(clientStore storetypes.KVStore, cdc codec.BinaryCodec) (*ClientState, error) {
+	key := host.ClientStateKey()
+	_, ok := clientStore.(migrateClientWrappedStore)
+	if ok {
+		key = append(subjectPrefix, key...)
+	}
+
+	bz := clientStore.Get(key)
+	if len(bz) == 0 {
+		return nil, errorsmod.Wrap(ErrWasmInvalidContractModification, types.ErrClientNotFound.Error())
+	}
+
+	clientState, err := unmarshalClientState(cdc, bz)
+	if err != nil {
+		return nil, errorsmod.Wrap(ErrWasmInvalidContractModification, err.Error())
+	}
+
+	cs, ok := clientState.(*ClientState)
+	if !ok {
+		return nil, errorsmod.Wrapf(ErrWasmInvalidContractModification, "expected client state type %T, got %T", (*ClientState)(nil), clientState)
+	}
+
+	return cs, nil
+}
+
+// unmarshalClientState unmarshals the client state from the given bytes.
+func unmarshalClientState(cdc codec.BinaryCodec, bz []byte) (exported.ClientState, error) {
+	var clientState exported.ClientState
+	if err := cdc.UnmarshalInterface(bz, &clientState); err != nil {
+		return nil, err
+	}
+
+	return clientState, nil
+}
+
 // getEnv returns the state of the blockchain environment the contract is running on
 func getEnv(ctx sdk.Context, contractAddr string) wasmvmtypes.Env {
 	chainID := ctx.BlockHeader().ChainID