Skip to content
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

Improve wasm contract queries #22

Merged
merged 7 commits into from
Jan 15, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ require (
github.com/snikch/goodman v0.0.0-20171125024755-10e37e294daa
github.com/spf13/afero v1.2.2 // indirect
github.com/spf13/cobra v0.0.5
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.5.0
github.com/stretchr/testify v1.4.0
github.com/tendermint/go-amino v0.15.1
Expand Down
78 changes: 67 additions & 11 deletions x/wasm/client/cli/query.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package cli

import (
"encoding/base64"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"strconv"

flag "github.com/spf13/pflag"

"github.com/spf13/cobra"

"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -176,7 +180,7 @@ func GetCmdGetContractStateAll(cdc *codec.Codec) *cobra.Command {
return err
}

route := fmt.Sprintf("custom/%s/%s/%s", types.QuerierRoute, keeper.QueryGetContractState, addr.String())
route := fmt.Sprintf("custom/%s/%s/%s/%s", types.QuerierRoute, keeper.QueryGetContractState, addr.String(), keeper.QueryMethodContractStateAll)
res, _, err := cliCtx.Query(route)
if err != nil {
return err
Expand All @@ -188,24 +192,24 @@ func GetCmdGetContractStateAll(cdc *codec.Codec) *cobra.Command {
}

func GetCmdGetContractStateRaw(cdc *codec.Codec) *cobra.Command {
return &cobra.Command{
decoder := newArgDecoder(hex.DecodeString)
cmd := &cobra.Command{
Use: "raw [bech32_address] [key]",
Short: "Prints out internal state for key of a contract given its address",
Long: "Prints out internal state for of a contract given its address",
Args: cobra.ExactArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
RunE: func(_ *cobra.Command, args []string) error {
cliCtx := context.NewCLIContext().WithCodec(cdc)

addr, err := sdk.AccAddressFromBech32(args[0])
if err != nil {
return err
}
key := args[1]
if key == "" {
return errors.New("key must not be empty")
queryData, err := decoder.DecodeString(args[1])
if err != nil {
return err
}
route := fmt.Sprintf("custom/%s/%s/%s/%s", types.QuerierRoute, keeper.QueryGetContractState, addr.String(), keeper.QueryMethodContractStateRaw)
queryData := []byte(key) // todo: open question: encode into json???
res, _, err := cliCtx.QueryWithData(route, queryData)
if err != nil {
return err
Expand All @@ -214,15 +218,19 @@ func GetCmdGetContractStateRaw(cdc *codec.Codec) *cobra.Command {
return nil
},
}
decoder.RegisterFlags(cmd.PersistentFlags(), "key argument")
return cmd
}

func GetCmdGetContractStateSmart(cdc *codec.Codec) *cobra.Command {
return &cobra.Command{
decoder := newArgDecoder(asciiDecodeString)

cmd := &cobra.Command{
Use: "smart [bech32_address] [query]",
Short: "Calls contract with given address with query data and prints the returned result",
Long: "Calls contract with given address with query data and prints the returned result",
Args: cobra.ExactArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
RunE: func(_ *cobra.Command, args []string) error {
cliCtx := context.NewCLIContext().WithCodec(cdc)

addr, err := sdk.AccAddressFromBech32(args[0])
Expand All @@ -234,14 +242,62 @@ func GetCmdGetContractStateSmart(cdc *codec.Codec) *cobra.Command {
return errors.New("key must not be empty")
}
route := fmt.Sprintf("custom/%s/%s/%s/%s", types.QuerierRoute, keeper.QueryGetContractState, addr.String(), keeper.QueryMethodContractStateSmart)
var queryData []byte

queryData, err := decoder.DecodeString(args[1])
if err != nil {
return fmt.Errorf("decode query: %s", err)
}
res, _, err := cliCtx.QueryWithData(route, queryData)
if err != nil {
return err
}
// todo: decode response
fmt.Println(string(res))
return nil
},
}
decoder.RegisterFlags(cmd.PersistentFlags(), "query argument")
return cmd
}

type argumentDecoder struct {
// dec is the default decoder
dec func(string) ([]byte, error)
asciiF, hexF, b64F bool
}

func newArgDecoder(def func(string) ([]byte, error)) *argumentDecoder {
return &argumentDecoder{dec: def}
}

func (a *argumentDecoder) RegisterFlags(f *flag.FlagSet, argName string) {
f.BoolVar(&a.asciiF, "ascii", false, "ascii encoded "+argName)
f.BoolVar(&a.hexF, "hex", false, "hex encoded "+argName)
f.BoolVar(&a.b64F, "b64", false, "base64 encoded "+argName)
}

func (a *argumentDecoder) DecodeString(s string) ([]byte, error) {
alpe marked this conversation as resolved.
Show resolved Hide resolved
found := -1
for i, v := range []*bool{&a.asciiF, &a.hexF, &a.b64F} {
if !*v {
continue
}
if found != -1 {
return nil, errors.New("multiple decoding flags used")
}
found = i
}
switch found {
case 0:
return asciiDecodeString(s)
case 1:
return hex.DecodeString(s)
case 2:
return base64.StdEncoding.DecodeString(s)
default:
return a.dec(s)
}
}

func asciiDecodeString(s string) ([]byte, error) {
alpe marked this conversation as resolved.
Show resolved Hide resolved
return []byte(s), nil
}
33 changes: 24 additions & 9 deletions x/wasm/internal/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,19 +160,40 @@ func (k Keeper) Execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller
return types.CosmosResult(*res), nil
}

func (k Keeper) Query(ctx sdk.Context, contractAddr sdk.AccAddress, req []byte) (*wasmTypes.QueryResult, sdk.Error) {
// QuerySmart queries the smart contract itself.
func (k Keeper) QuerySmart(ctx sdk.Context, contractAddr sdk.AccAddress, req []byte) ([]types.Model, sdk.Error) {
ctx = ctx.WithGasMeter(sdk.NewGasMeter(k.queryGasLimit))

codeInfo, prefixStore, err := k.contractInstance(ctx, contractAddr)
if err != nil {
return nil, err
}
res, gasUsed, qErr := k.wasmer.Query(codeInfo.CodeHash, req, prefixStore, gasForContract(ctx))
queryResult, gasUsed, qErr := k.wasmer.Query(codeInfo.CodeHash, req, prefixStore, gasForContract(ctx))
alpe marked this conversation as resolved.
Show resolved Hide resolved
if qErr != nil {
return nil, types.ErrExecuteFailed(qErr)
}
consumeGas(ctx, gasUsed)
return res, nil
models := make([]types.Model, len(queryResult.Results))
for i := range queryResult.Results {
models[i] = types.Model{
Key: queryResult.Results[i].Key,
Value: string(queryResult.Results[i].Value),
}
}
return models, nil
}

// QueryRaw returns the contract's state for give key. For a `nil` key a `nil` result is returned.
func (k Keeper) QueryRaw(ctx sdk.Context, contractAddress sdk.AccAddress, key []byte) []types.Model {
if key == nil {
return nil
}
prefixStoreKey := types.GetContractStorePrefixKey(contractAddress)
prefixStore := prefix.NewStore(ctx.KVStore(k.storeKey), prefixStoreKey)
return []types.Model{{
Key: string(key),
Value: string(prefixStore.Get(key)),
}}
}

func (k Keeper) contractInstance(ctx sdk.Context, contractAddress sdk.AccAddress) (types.CodeInfo, prefix.Store, sdk.Error) {
alpe marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -231,12 +252,6 @@ func (k Keeper) GetContractState(ctx sdk.Context, contractAddress sdk.AccAddress
return prefixStore.Iterator(nil, nil)
}

func (k Keeper) getContractStateForKey(ctx sdk.Context, contractAddress sdk.AccAddress, key []byte) []byte {
prefixStoreKey := types.GetContractStorePrefixKey(contractAddress)
prefixStore := prefix.NewStore(ctx.KVStore(k.storeKey), prefixStoreKey)
return prefixStore.Get(key)
}

func (k Keeper) setContractState(ctx sdk.Context, contractAddress sdk.AccAddress, models []types.Model) {
prefixStoreKey := types.GetContractStorePrefixKey(contractAddress)
prefixStore := prefix.NewStore(ctx.KVStore(k.storeKey), prefixStoreKey)
Expand Down
18 changes: 6 additions & 12 deletions x/wasm/internal/keeper/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,33 +82,27 @@ func queryContractState(ctx sdk.Context, bech, queryMethod string, req abci.Requ
return nil, sdk.ErrUnknownRequest(err.Error())
}

var result interface{}
var resultData []types.Model
switch queryMethod {
case QueryMethodContractStateAll:
var state []types.Model
for iter := keeper.GetContractState(ctx, contractAddr); iter.Valid(); iter.Next() {
state = append(state, types.Model{
resultData = append(resultData, types.Model{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this

Key: string(iter.Key()),
Value: string(iter.Value()),
})
}
result = state
case QueryMethodContractStateRaw:
value := keeper.getContractStateForKey(ctx, contractAddr, req.Data)
result = []types.Model{{
Key: string(req.Data),
Value: string(value),
}}
resultData = keeper.QueryRaw(ctx, contractAddr, req.Data)
case QueryMethodContractStateSmart:
res, err := keeper.Query(ctx, contractAddr, req.Data)
res, err := keeper.QuerySmart(ctx, contractAddr, req.Data)
if err != nil {
return nil, err
}
result = res.Results
resultData = res
default:
return nil, sdk.ErrUnknownRequest("unsupported data query method for contract-state")
}
bz, err := json.MarshalIndent(result, "", " ")
bz, err := json.MarshalIndent(resultData, "", " ")
if err != nil {
return nil, sdk.ErrUnknownRequest(err.Error())
}
Expand Down
131 changes: 131 additions & 0 deletions x/wasm/internal/keeper/querier_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package keeper

import (
"encoding/json"
"io/ioutil"
"os"
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmwasm/wasmd/x/wasm/internal/types"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
)

func TestQueryContractState(t *testing.T) {
type model struct {
Key string `json:"key"`
Value string `json:"val"`
}

tempDir, err := ioutil.TempDir("", "wasm")
require.NoError(t, err)
defer os.RemoveAll(tempDir)
ctx, accKeeper, keeper := CreateTestInput(t, false, tempDir)

deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000))
topUp := sdk.NewCoins(sdk.NewInt64Coin("denom", 5000))
creator := createFakeFundedAccount(ctx, accKeeper, deposit.Add(deposit))
anyAddr := createFakeFundedAccount(ctx, accKeeper, topUp)

wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm")
require.NoError(t, err)

contractID, err := keeper.Create(ctx, creator, wasmCode)
require.NoError(t, err)

_, _, bob := keyPubAddr()
initMsg := InitMsg{
Verifier: anyAddr.String(),
Beneficiary: bob.String(),
}
initMsgBz, err := json.Marshal(initMsg)
require.NoError(t, err)

addr, err := keeper.Instantiate(ctx, creator, contractID, initMsgBz, deposit)
require.NoError(t, err)
require.Equal(t, "cosmos18vd8fpwxzck93qlwghaj6arh4p7c5n89uzcee5", addr.String())

contractModel := []types.Model{
{Key: "foo", Value: "bar"},
{Key: string([]byte{0x0, 0x1}), Value: string([]byte{0x2, 0x3})},
}
keeper.setContractState(ctx, addr, contractModel)
alpe marked this conversation as resolved.
Show resolved Hide resolved

q := NewQuerier(keeper)
specs := map[string]struct {
srcPath []string
srcReq abci.RequestQuery
expModelLen int
expModelContains []model
expErr sdk.Error
}{
"query all": {
srcPath: []string{QueryGetContractState, addr.String(), QueryMethodContractStateAll},
expModelLen: 3,
expModelContains: []model{
{Key: "foo", Value: "bar"},
{Key: string([]byte{0x0, 0x1}), Value: string([]byte{0x2, 0x3})},
},
},
"query raw key": {
alpe marked this conversation as resolved.
Show resolved Hide resolved
srcPath: []string{QueryGetContractState, addr.String(), QueryMethodContractStateRaw},
srcReq: abci.RequestQuery{Data: []byte("foo")},
expModelLen: 1,
expModelContains: []model{{Key: "foo", Value: "bar"}},
},
"query raw binary key": {
srcPath: []string{QueryGetContractState, addr.String(), QueryMethodContractStateRaw},
srcReq: abci.RequestQuery{Data: []byte{0x0, 0x1}},
expModelLen: 1,
expModelContains: []model{{Key: string([]byte{0x0, 0x1}), Value: string([]byte{0x2, 0x3})}},
},
"query smart": {
alpe marked this conversation as resolved.
Show resolved Hide resolved
srcPath: []string{QueryGetContractState, addr.String(), QueryMethodContractStateSmart},
srcReq: abci.RequestQuery{Data: []byte(`{"raw":{"key":"config"}}`)},
expModelLen: 1,
//expModelContains: []model{}, // stopping here as contract internals are not stable
},
"query unknown raw key": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't this return an empty slice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the simplest implementation but not necessarily the best. I was under the assumption that the KV store allows nil values so this result make sense but it does not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you are right, but I would rather let len(0) = deleted. Even if not strictly true, it gives contracts a way to mark things as deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a patch. And also returning non nil model slices.

srcPath: []string{QueryGetContractState, addr.String(), QueryMethodContractStateRaw},
srcReq: abci.RequestQuery{Data: []byte("unknown")},
expModelLen: 1,
expModelContains: []model{{Key: "unknown", Value: ""}},
},
"query empty raw key": {
srcPath: []string{QueryGetContractState, addr.String(), QueryMethodContractStateRaw},
expModelLen: 0,
},
"query raw with unknown address": {
srcPath: []string{QueryGetContractState, anyAddr.String(), QueryMethodContractStateRaw},
expModelLen: 0,
},
"query all with unknown address": {
srcPath: []string{QueryGetContractState, anyAddr.String(), QueryMethodContractStateAll},
expModelLen: 0,
},
"query smart with unknown address": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would assume all the "query with unknown address" would return the same types.ErrNotFound("contract") message

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree ErrNotFound would be nicer but the prefix.Store concatenates the prefix and key for the parent store so that it results in just another unknown key.
I would need to do a prefix scan on the parent to detect the issue. The previous implementation for all had the same "problem"

srcPath: []string{QueryGetContractState, anyAddr.String(), QueryMethodContractStateSmart},
expModelLen: 0,
expErr: types.ErrNotFound("contract"),
},
}

for msg, spec := range specs {
alpe marked this conversation as resolved.
Show resolved Hide resolved
t.Run(msg, func(t *testing.T) {
binResult, err := q(ctx, spec.srcPath, spec.srcReq)
require.Equal(t, spec.expErr, err)
// then
var r []model
if spec.expErr == nil {
require.NoError(t, json.Unmarshal(binResult, &r))
}
require.Len(t, r, spec.expModelLen)
// and in result set
for _, v := range spec.expModelContains {
assert.Contains(t, r, v)
}
})
}
}
2 changes: 1 addition & 1 deletion x/wasm/internal/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
// Model is a struct that holds a KV pair
type Model struct {
Key string `json:"key"`
Value string `json:"value"`
Value string `json:"val"`
}

// CodeInfo is data for the uploaded contract WASM code
Expand Down
2 changes: 1 addition & 1 deletion x/wasm/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ func assertContractList(t *testing.T, q sdk.Querier, ctx sdk.Context, addrs []st

type model struct {
Key string `json:"key"`
Value string `json:"value"`
Value string `json:"val"`
}

func assertContractState(t *testing.T, q sdk.Querier, ctx sdk.Context, addr sdk.AccAddress, expected state) {
Expand Down