Fix message representation for signing (CosmWasm#658)

* Introduce RawContractMessage type

* Add json signbytes test for proposals

* No assumptions on MsgIBCSend.data content

* Smart query uses RawContractMessage

* Revert method signature change to be consistent

* Review comment

* Update after discussions

(cherry picked from commit dfba139)

docs/proto/proto-docs.md

@@ -617,7 +617,7 @@ MsgIBCSend
 | `channel` | [string](#string) |  | the channel by which the packet will be sent |
 | `timeout_height` | [uint64](#uint64) |  | Timeout height relative to the current block height. The timeout is disabled when set to 0. |
 | `timeout_timestamp` | [uint64](#uint64) |  | Timeout timestamp (in nanoseconds) relative to the current block timestamp. The timeout is disabled when set to 0. |
-| `data` | [bytes](#bytes) |  | data is the payload to transfer |
+| `data` | [bytes](#bytes) |  | Data is the payload to transfer. We must not make assumption what format or content is in here. |
 
 
 

proto/cosmwasm/wasm/v1/ibc.proto

@@ -20,8 +20,9 @@ message MsgIBCSend {
   uint64 timeout_timestamp = 5
       [ (gogoproto.moretags) = "yaml:\"timeout_timestamp\"" ];
 
-  // data is the payload to transfer
-  bytes data = 6 [ (gogoproto.casttype) = "encoding/json.RawMessage" ];
+  // Data is the payload to transfer. We must not make assumption what format or
+  // content is in here.
+  bytes data = 6;
 }
 
 // MsgIBCCloseChannel port and channel need to be owned by the contract

proto/cosmwasm/wasm/v1/proposal.proto

@@ -42,7 +42,7 @@ message InstantiateContractProposal {
   // Label is optional metadata to be stored with a constract instance.
   string label = 6;
   // Msg json encoded message to be passed to the contract on instantiation
-  bytes msg = 7;
+  bytes msg = 7 [ (gogoproto.casttype) = "RawContractMessage" ];
   // Funds coins that are transferred to the contract on instantiation
   repeated cosmos.base.v1beta1.Coin funds = 8 [
     (gogoproto.nullable) = false,
@@ -63,7 +63,7 @@ message MigrateContractProposal {
   // CodeID references the new WASM code
   uint64 code_id = 5 [ (gogoproto.customname) = "CodeID" ];
   // Msg json encoded message to be passed to the contract on migration
-  bytes msg = 6;
+  bytes msg = 6 [ (gogoproto.casttype) = "RawContractMessage" ];
 }
 
 // UpdateAdminProposal gov proposal content type to set an admin for a contract.

proto/cosmwasm/wasm/v1/query.proto

@@ -154,14 +154,14 @@ message QuerySmartContractStateRequest {
   // address is the address of the contract
   string address = 1;
   // QueryData contains the query data passed to the contract
-  bytes query_data = 2;
+  bytes query_data = 2 [ (gogoproto.casttype) = "RawContractMessage" ];
 }
 
 // QuerySmartContractStateResponse is the response type for the
 // Query/SmartContractState RPC method
 message QuerySmartContractStateResponse {
   // Data contains the json data returned from the smart contract
-  bytes data = 1 [ (gogoproto.casttype) = "encoding/json.RawMessage" ];
+  bytes data = 1 [ (gogoproto.casttype) = "RawContractMessage" ];
 }
 
 // QueryCodeRequest is the request type for the Query/Code RPC method

proto/cosmwasm/wasm/v1/tx.proto

@@ -55,7 +55,7 @@ message MsgInstantiateContract {
   // Label is optional metadata to be stored with a contract instance.
   string label = 4;
   // Msg json encoded message to be passed to the contract on instantiation
-  bytes msg = 5;
+  bytes msg = 5 [ (gogoproto.casttype) = "RawContractMessage" ];
   // Funds coins that are transferred to the contract on instantiation
   repeated cosmos.base.v1beta1.Coin funds = 6 [
     (gogoproto.nullable) = false,
@@ -77,7 +77,7 @@ message MsgExecuteContract {
   // Contract is the address of the smart contract
   string contract = 2;
   // Msg json encoded message to be passed to the contract
-  bytes msg = 3;
+  bytes msg = 3 [ (gogoproto.casttype) = "RawContractMessage" ];
   // Funds coins that are transferred to the contract on execution
   repeated cosmos.base.v1beta1.Coin funds = 5 [
     (gogoproto.nullable) = false,
@@ -100,7 +100,7 @@ message MsgMigrateContract {
   // CodeID references the new WASM code
   uint64 code_id = 3 [ (gogoproto.customname) = "CodeID" ];
   // Msg json encoded message to be passed to the contract on migration
-  bytes msg = 4;
+  bytes msg = 4 [ (gogoproto.casttype) = "RawContractMessage" ];
 }
 
 // MsgMigrateContractResponse returns contract migration result data.

proto/cosmwasm/wasm/v1/types.proto

@@ -117,7 +117,7 @@ message ContractCodeHistoryEntry {
   uint64 code_id = 2 [ (gogoproto.customname) = "CodeID" ];
   // Updated Tx position when the operation was executed.
   AbsoluteTxPosition updated = 3;
-  bytes msg = 4 [ (gogoproto.casttype) = "encoding/json.RawMessage" ];
+  bytes msg = 4 [ (gogoproto.casttype) = "RawContractMessage" ];
 }
 
 // AbsoluteTxPosition is a unique transaction position that allows for global

x/wasm/client/rest/gov.go

@@ -86,7 +86,7 @@ func (s InstantiateProposalJsonReq) Content() govtypes.Content {
 		Admin:       s.Admin,
 		CodeID:      s.Code,
 		Label:       s.Label,
-		Msg:         s.Msg,
+		Msg:         types.RawContractMessage(s.Msg),
 		Funds:       s.Funds,
 	}
 }
@@ -135,7 +135,7 @@ func (s MigrateProposalJsonReq) Content() govtypes.Content {
 		Description: s.Description,
 		Contract:    s.Contract,
 		CodeID:      s.Code,
-		Msg:         s.Msg,
+		Msg:         types.RawContractMessage(s.Msg),
 		RunAs:       s.RunAs,
 	}
 }

x/wasm/keeper/handler_plugin_encoders_test.go

@@ -1,7 +1,6 @@
 package keeper
 
 import (
-	"encoding/json"
 	"testing"
 
 	"github.com/CosmWasm/wasmd/x/wasm/keeper/wasmtesting"
@@ -34,7 +33,7 @@ func TestEncoding(t *testing.T) {
 	valAddr2 := make(sdk.ValAddress, 32)
 	valAddr2[1] = 123
 
-	jsonMsg := json.RawMessage(`{"foo": 123}`)
+	jsonMsg := types.RawContractMessage(`{"foo": 123}`)
 
 	bankMsg := &banktypes.MsgSend{
 		FromAddress: addr2.String(),

x/wasm/keeper/keeper_test.go

@@ -321,7 +321,7 @@ func TestInstantiate(t *testing.T) {
 		Operation: types.ContractCodeHistoryOperationTypeInit,
 		CodeID:    codeID,
 		Updated:   types.NewAbsoluteTxPosition(ctx),
-		Msg:       json.RawMessage(initMsgBz),
+		Msg:       initMsgBz,
 	}}
 	assert.Equal(t, exp, keepers.WasmKeeper.GetContractHistory(ctx, gotContractAddr))
 

x/wasm/keeper/legacy_querier.go

@@ -88,35 +88,37 @@ func queryContractState(ctx sdk.Context, bech, queryMethod string, data []byte,
 		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, bech)
 	}
 
-	var resultData []types.Model
 	switch queryMethod {
 	case QueryMethodContractStateAll:
+		resultData := make([]types.Model, 0)
 		// this returns a serialized json object (which internally encoded binary fields properly)
 		for iter := keeper.GetContractState(ctx, contractAddr); iter.Valid(); iter.Next() {
 			resultData = append(resultData, types.Model{
 				Key:   iter.Key(),
 				Value: iter.Value(),
 			})
 		}
-		if resultData == nil {
-			resultData = make([]types.Model, 0)
+		bz, err := json.Marshal(resultData)
+		if err != nil {
+			return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
 		}
+		return bz, nil
 	case QueryMethodContractStateRaw:
 		// this returns the raw data from the state, base64-encoded
 		return keeper.QueryRaw(ctx, contractAddr, data), nil
 	case QueryMethodContractStateSmart:
 		// we enforce a subjective gas limit on all queries to avoid infinite loops
 		ctx = ctx.WithGasMeter(sdk.NewGasMeter(gasLimit))
+		msg := types.RawContractMessage(data)
+		if err := msg.ValidateBasic(); err != nil {
+			return nil, sdkerrors.Wrap(err, "json msg")
+		}
 		// this returns raw bytes (must be base64-encoded)
-		return keeper.QuerySmart(ctx, contractAddr, data)
+		bz, err := keeper.QuerySmart(ctx, contractAddr, msg)
+		return bz, err
 	default:
 		return nil, sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, queryMethod)
 	}
-	bz, err := json.Marshal(resultData)
-	if err != nil {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
-	}
-	return bz, nil
 }
 
 func queryCodeList(ctx sdk.Context, keeper types.ViewKeeper) ([]types.CodeInfoResponse, error) {

x/wasm/keeper/legacy_querier_test.go

@@ -3,13 +3,13 @@ package keeper
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"testing"
 
 	"github.com/CosmWasm/wasmd/x/wasm/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	sdkErrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -60,7 +60,7 @@ func TestLegacyQueryContractState(t *testing.T) {
 		// if success and expSmartRes is not set, we parse into []types.Model and compare (all state)
 		expModelLen      int
 		expModelContains []types.Model
-		expErr           *sdkErrors.Error
+		expErr           error
 	}{
 		"query all": {
 			srcPath:     []string{QueryGetContractState, addr.String(), QueryMethodContractStateAll},
@@ -93,7 +93,7 @@ func TestLegacyQueryContractState(t *testing.T) {
 		"query smart with invalid json": {
 			srcPath: []string{QueryGetContractState, addr.String(), QueryMethodContractStateSmart},
 			srcReq:  abci.RequestQuery{Data: []byte(`not a json string`)},
-			expErr:  types.ErrQueryFailed,
+			expErr:  types.ErrInvalid,
 		},
 		"query non-existent raw key": {
 			srcPath: []string{QueryGetContractState, addr.String(), QueryMethodContractStateRaw},
@@ -120,6 +120,7 @@ func TestLegacyQueryContractState(t *testing.T) {
 		},
 		"query smart with unknown address": {
 			srcPath:     []string{QueryGetContractState, anyAddr.String(), QueryMethodContractStateSmart},
+			srcReq:      abci.RequestQuery{Data: []byte(`{}`)},
 			expModelLen: 0,
 			expErr:      types.ErrNotFound,
 		},
@@ -129,7 +130,7 @@ func TestLegacyQueryContractState(t *testing.T) {
 		t.Run(msg, func(t *testing.T) {
 			binResult, err := q(ctx, spec.srcPath, spec.srcReq)
 			// require.True(t, spec.expErr.Is(err), "unexpected error")
-			require.True(t, spec.expErr.Is(err), err)
+			require.True(t, errors.Is(err, spec.expErr), err)
 
 			// if smart query, check custom response
 			if spec.srcPath[2] != QueryMethodContractStateAll {

x/wasm/keeper/querier.go

@@ -164,6 +164,9 @@ func (q grpcQuerier) SmartContractState(c context.Context, req *types.QuerySmart
 	if req == nil {
 		return nil, status.Error(codes.InvalidArgument, "empty request")
 	}
+	if err := req.QueryData.ValidateBasic(); err != nil {
+		return nil, status.Error(codes.InvalidArgument, "invalid query data")
+	}
 	contractAddr, err := sdk.AccAddressFromBech32(req.Address)
 	if err != nil {
 		return nil, err

x/wasm/keeper/querier_test.go

@@ -3,13 +3,14 @@ package keeper
 import (
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"io/ioutil"
 	"testing"
 	"time"
 
-	"github.com/CosmWasm/wasmd/x/wasm/keeper/wasmtesting"
-	"github.com/CosmWasm/wasmd/x/wasm/types"
 	cosmwasm "github.com/CosmWasm/wasmvm"
 	wasmvmtypes "github.com/CosmWasm/wasmvm/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -19,6 +20,9 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/tendermint/tendermint/libs/log"
+
+	"github.com/CosmWasm/wasmd/x/wasm/keeper/wasmtesting"
+	"github.com/CosmWasm/wasmd/x/wasm/types"
 )
 
 func TestQueryAllContractState(t *testing.T) {
@@ -120,7 +124,7 @@ func TestQuerySmartContractState(t *testing.T) {
 		srcAddr  sdk.AccAddress
 		srcQuery *types.QuerySmartContractStateRequest
 		expResp  string
-		expErr   *sdkErrors.Error
+		expErr   error
 	}{
 		"query smart": {
 			srcQuery: &types.QuerySmartContractStateRequest{Address: contractAddr, QueryData: []byte(`{"verifier":{}}`)},
@@ -132,7 +136,7 @@ func TestQuerySmartContractState(t *testing.T) {
 		},
 		"query smart with invalid json": {
 			srcQuery: &types.QuerySmartContractStateRequest{Address: contractAddr, QueryData: []byte(`not a json string`)},
-			expErr:   types.ErrQueryFailed,
+			expErr:   status.Error(codes.InvalidArgument, "invalid query data"),
 		},
 		"query smart with unknown address": {
 			srcQuery: &types.QuerySmartContractStateRequest{Address: RandomBech32AccountAddress(t), QueryData: []byte(`{"verifier":{}}`)},
@@ -142,7 +146,7 @@ func TestQuerySmartContractState(t *testing.T) {
 	for msg, spec := range specs {
 		t.Run(msg, func(t *testing.T) {
 			got, err := q.SmartContractState(sdk.WrapSDKContext(ctx), spec.srcQuery)
-			require.True(t, spec.expErr.Is(err), "but got %+v", err)
+			require.True(t, errors.Is(err, spec.expErr), "but got %+v", err)
 			if spec.expErr != nil {
 				return
 			}
@@ -187,7 +191,8 @@ func TestQuerySmartContractPanics(t *testing.T) {
 			// when
 			q := Querier(keepers.WasmKeeper)
 			got, err := q.SmartContractState(sdk.WrapSDKContext(ctx), &types.QuerySmartContractStateRequest{
-				Address: contractAddr.String(),
+				Address:   contractAddr.String(),
+				QueryData: types.RawContractMessage("{}"),
 			})
 			require.True(t, spec.expErr.Is(err), "got error: %+v", err)
 			assert.Nil(t, got)

x/wasm/keeper/query_plugins.go

@@ -468,7 +468,11 @@ func WasmQuerier(k wasmQueryKeeper) func(ctx sdk.Context, request *wasmvmtypes.W
 			if err != nil {
 				return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, request.Smart.ContractAddr)
 			}
-			return k.QuerySmart(ctx, addr, request.Smart.Msg)
+			msg := types.RawContractMessage(request.Smart.Msg)
+			if err := msg.ValidateBasic(); err != nil {
+				return nil, sdkerrors.Wrap(err, "json msg")
+			}
+			return k.QuerySmart(ctx, addr, msg)
 		case request.Raw != nil:
 			addr, err := sdk.AccAddressFromBech32(request.Raw.ContractAddr)
 			if err != nil {

x/wasm/keeper/query_plugins_test.go

@@ -444,7 +444,7 @@ func TestContractInfoWasmQuerier(t *testing.T) {
 type mockWasmQueryKeeper struct {
 	GetContractInfoFn func(ctx sdk.Context, contractAddress sdk.AccAddress) *types.ContractInfo
 	QueryRawFn        func(ctx sdk.Context, contractAddress sdk.AccAddress, key []byte) []byte
-	QuerySmartFn      func(ctx sdk.Context, contractAddr sdk.AccAddress, req []byte) ([]byte, error)
+	QuerySmartFn      func(ctx sdk.Context, contractAddr sdk.AccAddress, req types.RawContractMessage) ([]byte, error)
 	IsPinnedCodeFn    func(ctx sdk.Context, codeID uint64) bool
 }
 

x/wasm/relay_pingpong_test.go

@@ -390,7 +390,7 @@ type startGame struct {
 	MaxValue uint64 `json:"max_value,omitempty"`
 }
 
-func (g startGame) GetBytes() json.RawMessage {
+func (g startGame) GetBytes() wasmtypes.RawContractMessage {
 	b, err := json.Marshal(g)
 	if err != nil {
 		panic(err)

x/wasm/relay_test.go

@@ -442,7 +442,7 @@ type startTransfer struct {
 	Timeout         uint64
 }
 
-func (g startTransfer) GetBytes() json.RawMessage {
+func (g startTransfer) GetBytes() types.RawContractMessage {
 	b, err := json.Marshal(g)
 	if err != nil {
 		panic(err)