Skip to content

Commit

Permalink
feat(keeper)!: use <pkgpath>.<expr> syntax for qeval; <pkgpath>:<path…
Browse files Browse the repository at this point in the history
…> for qrender (#2382)

This is a switch that was discussed with @leohhhn and @moul. It switches
the current syntax for qeval, which requires a newline as a separator,
to use a dot `.` instead (ie. `gno.land/r/demo/users.MyFunction(123)`).
For qrender, this is switched to a colon `:`, like the gnoweb render:
`gno.land/r/demo/users:u/morgan`.

BREAKING CHANGE: current qeval and qrender calls using the RPC endpoints
will have to be changed. No changes are required for gnoclient users.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
- [x] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>

---------

Co-authored-by: Manfred Touron <94029+moul@users.noreply.github.com>
Co-authored-by: Leon Hudak <33522493+leohhhn@users.noreply.github.com>
  • Loading branch information
3 people authored Jun 20, 2024
1 parent d2d34eb commit 032d422
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 89 deletions.
10 changes: 5 additions & 5 deletions docs/gno-tooling/cli/gnokey.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ gnokey query {QUERY_PATH}
| `bank/balances/{ADDRESS}` | Returns balances of an account. | `gnokey query bank/balances/g1jg8mtutu9khhfwc4nxmuhcpftf0pajdhfvsqf5` |
| `vm/qfuncs` | Returns public facing function signatures as JSON. | `gnokey query vm/qfuncs --data "gno.land/r/demo/boards"` |
| `vm/qfile` | Returns the file bytes, or list of files if directory. | `gnokey query vm/qfile --data "gno.land/r/demo/boards"` |
| `vm/qrender` | Calls .Render(path) in readonly mode. | `gnokey query vm/qrender --data "gno.land/r/demo/boards"` |
| `vm/qeval` | Evaluates any expression in readonly mode and returns the results. | `gnokey query vm/qeval --data "gno.land/r/demo/boards GetBoardIDFromName("my_board")"` |
| `vm/qrender` | Calls .Render(path) in readonly mode. | `gnokey query vm/qrender --data "gno.land/r/demo/boards:"` |
| `vm/qeval` | Evaluates any expression in readonly mode and returns the results. | `gnokey query vm/qeval --data "gno.land/r/demo/boards.GetBoardIDFromName("my_board")"` |
| `vm/store` | (not yet supported) Fetches items from the store. | - |
| `vm/package` | (not yet supported) Fetches a package's files. | - |

Expand Down Expand Up @@ -205,16 +205,16 @@ gnokey maketx call \
> unsigned.tx
```

:::warning `call` is a state-changing message
:::warning `call` is a state-changing message

All exported functions, including `Render()`, can be called in two main ways:
`call` and [`query vm/qeval`](#query).

With `call`, any state change that happened in the function being called will be
applied and persisted in on the blockchain, and the gas used for this call will
be subtracted from the caller balance.
be subtracted from the caller balance.

As opposed to this, an ABCI query, such as `vm/qeval` will not persist state
As opposed to this, an ABCI query, such as `vm/qeval` will not persist state
changes and does not cost gas, only evaluating the expression in read-only mode.

:::
Expand Down
11 changes: 4 additions & 7 deletions examples/gno.land/r/demo/boards/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ your `ACCOUNT_ADDR` and `KEYNAME`

Instead of editing `gno.land/genesis/genesis_balances.txt`, a more general solution (with more steps)
is to run a local "faucet" and use the web browser to add $GNOT. (This can be done at any time.)
See this page: https://github.com/gnolang/gno/blob/master/gno.land/cmd/gnofaucet/README.md
See this page: https://github.com/gnolang/gno/blob/master/gno.land/cmd/gnofaucet/README.md

### Start the `gnoland` node.

Expand Down Expand Up @@ -97,8 +97,7 @@ Interactive documentation: https://test3.gno.land/r/demo/boards?help&__func=Crea
Next, query for the permanent board ID by querying (you need this to create a new post):

```bash
./build/gnokey query "vm/qeval" -data "gno.land/r/demo/boards
GetBoardIDFromName(\"BOARDNAME\")" -remote localhost:26657
./build/gnokey query "vm/qeval" -data 'gno.land/r/demo/boards.GetBoardIDFromName("BOARDNAME")' -remote localhost:26657
```

### Create a post of a board with a smart contract call.
Expand All @@ -120,8 +119,7 @@ Interactive documentation: https://test3.gno.land/r/demo/boards?help&__func=Crea
Interactive documentation: https://test3.gno.land/r/demo/boards?help&__func=CreateReply

```bash
./build/gnokey query "vm/qrender" -data "gno.land/r/demo/boards
BOARDNAME/1" -remote localhost:26657
./build/gnokey query "vm/qrender" -data "gno.land/r/demo/boards:BOARDNAME/1" -remote localhost:26657
```

### Render page with optional path expression.
Expand All @@ -130,8 +128,7 @@ The contents of `https://gno.land/r/demo/boards:` and `https://gno.land/r/demo/b
the `Render(path string)` function like so:

```bash
./build/gnokey query "vm/qrender" -data "gno.land/r/demo/boards
gnolang"
./build/gnokey query "vm/qrender" -data "gno.land/r/demo/boards:gnolang"
```
## View the board in the browser.

Expand Down
12 changes: 6 additions & 6 deletions gno.land/cmd/gnoland/testdata/gnokey_simulate.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,19 @@ stdout '"sequence": "1"'
gnokey maketx call -pkgpath gno.land/r/hello -func SetName -args John -gas-wanted 2000000 -gas-fee 1000000ugnot -broadcast -chainid tendermint_test -simulate test test1
gnokey query auth/accounts/$USER_ADDR_test1
stdout '"sequence": "2"'
gnokey query vm/qeval --data "gno.land/r/hello\nHello()"
gnokey query vm/qeval --data "gno.land/r/hello.Hello()"
stdout 'Hello, John!'
# -simulate only
gnokey maketx call -pkgpath gno.land/r/hello -func SetName -args Paul -gas-wanted 2000000 -gas-fee 1000000ugnot -broadcast -chainid tendermint_test -simulate only test1
gnokey query auth/accounts/$USER_ADDR_test1
stdout '"sequence": "2"'
gnokey query vm/qeval --data "gno.land/r/hello\nHello()"
gnokey query vm/qeval --data "gno.land/r/hello.Hello()"
stdout 'Hello, John!'
# -simulate skip
gnokey maketx call -pkgpath gno.land/r/hello -func SetName -args George -gas-wanted 2000000 -gas-fee 1000000ugnot -broadcast -chainid tendermint_test -simulate skip test1
gnokey query auth/accounts/$USER_ADDR_test1
stdout '"sequence": "3"'
gnokey query vm/qeval --data "gno.land/r/hello\nHello()"
gnokey query vm/qeval --data "gno.land/r/hello.Hello()"
stdout 'Hello, George!'

# attempt calling hello.Grumpy (always panics).
Expand All @@ -52,19 +52,19 @@ stdout 'Hello, George!'
! gnokey maketx call -pkgpath gno.land/r/hello -func Grumpy -gas-wanted 2000000 -gas-fee 1000000ugnot -broadcast -chainid tendermint_test -simulate test test1
gnokey query auth/accounts/$USER_ADDR_test1
stdout '"sequence": "3"'
gnokey query vm/qeval --data "gno.land/r/hello\nHello()"
gnokey query vm/qeval --data "gno.land/r/hello.Hello()"
stdout 'Hello, George!'
# -simulate only
! gnokey maketx call -pkgpath gno.land/r/hello -func Grumpy -gas-wanted 2000000 -gas-fee 1000000ugnot -broadcast -chainid tendermint_test -simulate only test1
gnokey query auth/accounts/$USER_ADDR_test1
stdout '"sequence": "3"'
gnokey query vm/qeval --data "gno.land/r/hello\nHello()"
gnokey query vm/qeval --data "gno.land/r/hello.Hello()"
stdout 'Hello, George!'
# -simulate skip
! gnokey maketx call -pkgpath gno.land/r/hello -func Grumpy -gas-wanted 2000000 -gas-fee 1000000ugnot -broadcast -chainid tendermint_test -simulate skip test1
gnokey query auth/accounts/$USER_ADDR_test1
stdout '"sequence": "4"'
gnokey query vm/qeval --data "gno.land/r/hello\nHello()"
gnokey query vm/qeval --data "gno.land/r/hello.Hello()"
stdout 'Hello, George!'

-- test/test.gno --
Expand Down
4 changes: 2 additions & 2 deletions gno.land/genesis/genesis_txs.jsonl

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions gno.land/pkg/gnoclient/client_queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (c *Client) Render(pkgPath string, args string) (string, *ctypes.ResultABCI
}

path := "vm/qrender"
data := []byte(fmt.Sprintf("%s\n%s", pkgPath, args))
data := []byte(fmt.Sprintf("%s:%s", pkgPath, args))

qres, err := c.RPCClient.ABCIQuery(path, data)
if err != nil {
Expand All @@ -113,7 +113,7 @@ func (c *Client) QEval(pkgPath string, expression string) (string, *ctypes.Resul
}

path := "vm/qeval"
data := []byte(fmt.Sprintf("%s\n%s", pkgPath, expression))
data := []byte(fmt.Sprintf("%s.%s", pkgPath, expression))

qres, err := c.RPCClient.ABCIQuery(path, data)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions gno.land/pkg/gnoweb/gnoweb.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func handlerRealmAlias(logger *slog.Logger, app gotuna.App, cfg *Config, rlmpath
}
rlmname := strings.TrimPrefix(rlmfullpath, "gno.land/r/")
qpath := "vm/qrender"
data := []byte(fmt.Sprintf("%s\n%s", rlmfullpath, querystr))
data := []byte(fmt.Sprintf("%s:%s", rlmfullpath, querystr))
res, err := makeRequest(logger, cfg, qpath, data)
if err != nil {
writeError(logger, w, fmt.Errorf("gnoweb failed to query gnoland: %w", err))
Expand Down Expand Up @@ -323,7 +323,7 @@ func handleRealmRender(logger *slog.Logger, app gotuna.App, cfg *Config, w http.
return
}
qpath := "vm/qrender"
data := []byte(fmt.Sprintf("%s\n%s", rlmpath, querystr))
data := []byte(fmt.Sprintf("%s:%s", rlmpath, querystr))
res, err := makeRequest(logger, cfg, qpath, data)
if err != nil {
// XXX hack
Expand Down
48 changes: 30 additions & 18 deletions gno.land/pkg/sdk/vm/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,12 @@ func (vh vmHandler) queryStore(ctx sdk.Context, req abci.RequestQuery) (res abci
// queryRender calls .Render(<path>) in readonly mode.
func (vh vmHandler) queryRender(ctx sdk.Context, req abci.RequestQuery) (res abci.ResponseQuery) {
reqData := string(req.Data)
reqParts := strings.Split(reqData, "\n")
if len(reqParts) != 2 {
panic("expected two lines in query input data")
dot := strings.IndexByte(reqData, ':')
if dot < 0 {
panic("expected <pkgpath>:<path> syntax in query input data")
}
pkgPath := reqParts[0]
path := reqParts[1]

pkgPath, path := reqData[:dot], reqData[dot+1:]
expr := fmt.Sprintf("Render(%q)", path)
result, err := vh.vm.QueryEvalString(ctx, pkgPath, expr)
if err != nil {
Expand All @@ -168,12 +168,7 @@ func (vh vmHandler) queryRender(ctx sdk.Context, req abci.RequestQuery) (res abc

// queryFuncs returns public facing function signatures as JSON.
func (vh vmHandler) queryFuncs(ctx sdk.Context, req abci.RequestQuery) (res abci.ResponseQuery) {
reqData := string(req.Data)
reqParts := strings.Split(reqData, "\n")
if len(reqParts) != 1 {
panic("expected one line in query input data")
}
pkgPath := reqParts[0]
pkgPath := string(req.Data)
fsigs, err := vh.vm.QueryFuncs(ctx, pkgPath)
if err != nil {
res = sdk.ABCIResponseQueryFromError(err)
Expand All @@ -185,13 +180,7 @@ func (vh vmHandler) queryFuncs(ctx sdk.Context, req abci.RequestQuery) (res abci

// queryEval evaluates any expression in readonly mode and returns the results.
func (vh vmHandler) queryEval(ctx sdk.Context, req abci.RequestQuery) (res abci.ResponseQuery) {
reqData := string(req.Data)
reqParts := strings.Split(reqData, "\n")
if len(reqParts) != 2 {
panic("expected two lines in query input data")
}
pkgPath := reqParts[0]
expr := reqParts[1]
pkgPath, expr := parseQueryEvalData(string(req.Data))
result, err := vh.vm.QueryEval(ctx, pkgPath, expr)
if err != nil {
res = sdk.ABCIResponseQueryFromError(err)
Expand All @@ -201,6 +190,29 @@ func (vh vmHandler) queryEval(ctx sdk.Context, req abci.RequestQuery) (res abci.
return
}

// parseQueryEval parses the input string of vm/qeval. It takes the first dot
// after the first slash (if any) to separe the pkgPath and the expr.
// For instance, in gno.land/r/realm.MyFunction(), gno.land/r/realm is the
// pkgPath,and MyFunction() is the expr.
func parseQueryEvalData(data string) (pkgPath, expr string) {
slash := strings.IndexByte(data, '/')
if slash >= 0 {
pkgPath += data[:slash]
data = data[slash:]
}
dot := strings.IndexByte(data, '.')
if dot < 0 {
panic(panicInvalidQueryEvalData)
}
pkgPath += data[:dot]
expr = data[dot+1:]
return
}

const (
panicInvalidQueryEvalData = "expected <pkgpath>.<expression> syntax in query input data"
)

// queryFile returns the file bytes, or list of files if directory.
// if file, res.Value is []byte("file").
// if dir, res.Value is []byte("dir").
Expand Down
86 changes: 39 additions & 47 deletions gno.land/pkg/sdk/vm/handler_test.go
Original file line number Diff line number Diff line change
@@ -1,58 +1,50 @@
package vm

/*
import (
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/require"
"github.com/gnolang/gno/tm2/pkg/amino"
abci "github.com/gnolang/gno/tm2/pkg/bft/abci/types"
bft "github.com/gnolang/gno/tm2/pkg/bft/types"
"github.com/gnolang/gno/tm2/pkg/sdk"
tu "github.com/gnolang/gno/tm2/pkg/sdk/testutils"
"github.com/gnolang/gno/tm2/pkg/std"
"github.com/stretchr/testify/assert"
)

func TestInvalidMsg(t *testing.T) {
h := NewHandler(BankKeeper{})
res := h.Process(sdk.NewContext(nil, &bft.Header{}, false, nil), tu.NewTestMsg())
require.False(t, res.IsOK())
require.True(t, strings.Contains(res.Log, "unrecognized bank message type"))
}
func TestBalances(t *testing.T) {
env := setupTestEnv()
h := NewHandler(env.bank)
req := abci.RequestQuery{
Path: fmt.Sprintf("bank/%s", QueryBalance),
Data: []byte{},
func Test_parseQueryEvalData(t *testing.T) {
t.Parallel()
tt := []struct {
input string
pkgpath string
expr string
}{
{
"gno.land/r/realm.Expression()",
"gno.land/r/realm",
"Expression()",
},
{
"a.b/c/d.e",
"a.b/c/d",
"e",
},
{
"a.b.c.d.e/c/d.e",
"a.b.c.d.e/c/d",
"e",
},
{
"abcde/c/d.e",
"abcde/c/d",
"e",
},
}
for _, tc := range tt {
path, expr := parseQueryEvalData(tc.input)
assert.Equal(t, tc.pkgpath, path)
assert.Equal(t, tc.expr, expr)
}
}

res := h.Query(env.ctx, req)
require.NotNil(t, res.Error)
_, _, addr := tu.KeyTestPubAddr()
req.Data = amino.MustMarshalJSON(NewQueryBalanceParams(addr))
res = h.Query(env.ctx, req)
require.Nil(t, res.Error) // the account does not exist, no error returned anyway
require.NotNil(t, res)
var coins std.Coins
require.NoError(t, amino.UnmarshalJSON(res.Data, &coins))
require.True(t, coins.IsZero())
func Test_parseQueryEval_panic(t *testing.T) {
t.Parallel()

acc := env.acck.NewAccountWithAddress(env.ctx, addr)
acc.SetCoins(std.NewCoins(std.NewCoin("foo", 10)))
env.acck.SetAccount(env.ctx, acc)
res = h.Query(env.ctx, req)
require.Nil(t, res.Error)
require.NotNil(t, res)
require.NoError(t, amino.UnmarshalJSON(res.Data, &coins))
require.True(t, coins.AmountOf("foo") == 10)
assert.PanicsWithValue(t, panicInvalidQueryEvalData, func() {
parseQueryEvalData("gno.land/r/demo/users")
})
}
*/

0 comments on commit 032d422

Please sign in to comment.