diff --git a/docs/gno-tooling/cli/gnokey.md b/docs/gno-tooling/cli/gnokey.md index 8abd0722229..8479e9c112d 100644 --- a/docs/gno-tooling/cli/gnokey.md +++ b/docs/gno-tooling/cli/gnokey.md @@ -171,13 +171,14 @@ gnokey maketx addpkg \ #### **SignBroadcast Options** -| Name | Type | Description | -|--------------|---------|--------------------------------------------------------------------------| -| `gas-wanted` | Int64 | The maximum amount of gas to use for the transaction. | -| `gas-fee` | String | The gas fee to pay for the transaction. | -| `memo` | String | Any descriptive text. | -| `broadcast` | Boolean | Broadcasts the transaction. | -| `chainid` | String | Defines the chainid to sign for (should only be used with `--broadcast`) | +| Name | Type | Description | +|--------------|---------|----------------------------------------------------------------------------------------| +| `gas-wanted` | Int64 | The maximum amount of gas to use for the transaction. | +| `gas-fee` | String | The gas fee to pay for the transaction. | +| `memo` | String | Any descriptive text. | +| `broadcast` | Boolean | Broadcasts the transaction. | +| `chainid` | String | The chainid to sign for (should only be used with `--broadcast`) | +| `simulate` | String | One of `test` (default), `skip` or `only` (should only be used with `--broadcast`)[^1] | #### **makeTx AddPackage Options** @@ -208,13 +209,14 @@ gnokey maketx call \ #### **SignBroadcast Options** -| Name | Type | Description | -|--------------|---------|------------------------------------------------------------------| -| `gas-wanted` | Int64 | The maximum amount of gas to use for the transaction. | -| `gas-fee` | String | The gas fee to pay for the transaction. | -| `memo` | String | Any descriptive text. | -| `broadcast` | Boolean | Broadcasts the transaction. | -| `chainid` | String | The chainid to sign for (should only be used with `--broadcast`) | +| Name | Type | Description | +|--------------|---------|----------------------------------------------------------------------------------------| +| `gas-wanted` | Int64 | The maximum amount of gas to use for the transaction. | +| `gas-fee` | String | The gas fee to pay for the transaction. | +| `memo` | String | Any descriptive text. | +| `broadcast` | Boolean | Broadcasts the transaction. | +| `chainid` | String | The chainid to sign for (should only be used with `--broadcast`) | +| `simulate` | String | One of `test` (default), `skip` or `only` (should only be used with `--broadcast`)[^1] | #### **makeTx Call Options** @@ -246,13 +248,14 @@ gnokey maketx send \ #### **SignBroadcast Options** -| Name | Type | Description | -|--------------|---------|-------------------------------------------------------| -| `gas-wanted` | Int64 | The maximum amount of gas to use for the transaction. | -| `gas-fee` | String | The gas fee to pay for the transaction. | -| `memo` | String | Any descriptive text. | -| `broadcast` | Boolean | Broadcasts the transaction. | -| `chainid` | String | The chainid to sign for (implies `--broadcast`) | +| Name | Type | Description | +|--------------|---------|----------------------------------------------------------------------------------------| +| `gas-wanted` | Int64 | The maximum amount of gas to use for the transaction. | +| `gas-fee` | String | The gas fee to pay for the transaction. | +| `memo` | String | Any descriptive text. | +| `broadcast` | Boolean | Broadcasts the transaction. | +| `chainid` | String | The chainid to sign for (should only be used with `--broadcast`) | +| `simulate` | String | One of `test` (default), `skip` or `only` (should only be used with `--broadcast`)[^1] | #### **makeTx Send Options** @@ -302,3 +305,7 @@ Broadcast a signed document with the following command. ```bash gnokey broadcast {signed transaction file document} ``` + +[^1]: `only` simulates the transaction as a "dry run" (ie. without committing to + the chain), `test` performs simulation and, if successful, commits the + transaction, `skip` skips simulation entirely and commits directly. diff --git a/gno.land/cmd/gnoland/testdata/addpkg_invalid.txtar b/gno.land/cmd/gnoland/testdata/addpkg_invalid.txtar new file mode 100644 index 00000000000..5cfd48bf2ea --- /dev/null +++ b/gno.land/cmd/gnoland/testdata/addpkg_invalid.txtar @@ -0,0 +1,21 @@ +# test for add package; ensuring type checker catches invalid code. + +# start a new node +gnoland start + +# add bar package located in $WORK directory as gno.land/r/foobar/bar +! gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/foobar/bar -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1 + +# check error message +! stdout .+ +stderr 'as string value in return statement' +stderr '"std" imported and not used' + +-- bar.gno -- +package bar + +import "std" + +func Render(path string) string { + return 89 +} diff --git a/gno.land/cmd/gnoland/testdata/gnokey_simulate.txtar b/gno.land/cmd/gnoland/testdata/gnokey_simulate.txtar new file mode 100644 index 00000000000..dab238a6122 --- /dev/null +++ b/gno.land/cmd/gnoland/testdata/gnokey_simulate.txtar @@ -0,0 +1,93 @@ +# test for gnokey maketx -simulate options, and how they return any errors + +loadpkg gno.land/r/hello $WORK/hello + +# start a new node +gnoland start + +# Initial state: assert that sequence == 0. +gnokey query auth/accounts/$USER_ADDR_test1 +stdout '"sequence": "0"' + +# attempt adding the "test" package. +# the package has a syntax error; simulation should catch this ahead of time and prevent the tx. +# -simulate test +! gnokey maketx addpkg -pkgdir $WORK/test -pkgpath gno.land/r/test -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -simulate test test1 +gnokey query auth/accounts/$USER_ADDR_test1 +stdout '"sequence": "0"' +# -simulate only +! gnokey maketx addpkg -pkgdir $WORK/test -pkgpath gno.land/r/test -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -simulate only test1 +gnokey query auth/accounts/$USER_ADDR_test1 +stdout '"sequence": "0"' +# -simulate skip +! gnokey maketx addpkg -pkgdir $WORK/test -pkgpath gno.land/r/test -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test -simulate skip test1 +gnokey query auth/accounts/$USER_ADDR_test1 +stdout '"sequence": "1"' + +# attempt calling hello.SetName correctly. +# -simulate test and skip should do it successfully, -simulate only should not. +# -simulate test +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()" +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()" +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()" +stdout 'Hello, George!' + +# attempt calling hello.Grumpy (always panics). +# all calls should fail, however -test skip should increase the account sequence. +# none should change the name (ie. panic rollbacks). +# -simulate test +! 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()" +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()" +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()" +stdout 'Hello, George!' + +-- test/test.gno -- +package test + +func Render(path string) string { + return 89 +} + +-- hello/hello.gno -- +package hello + +var name = "Ringo" + +func SetName(newName string) { + name = newName +} + +func Hello() string { + return "Hello, " + name + "!" +} + +func Grumpy() string { + name = "SCOUNDREL" + panic("YOU MAY NOT GREET ME, " + name) +} diff --git a/gno.land/cmd/gnoland/testdata/issue_1786.txtar b/gno.land/cmd/gnoland/testdata/issue_1786.txtar index a14f06cf9b9..44ea17674c9 100644 --- a/gno.land/cmd/gnoland/testdata/issue_1786.txtar +++ b/gno.land/cmd/gnoland/testdata/issue_1786.txtar @@ -5,7 +5,7 @@ loadpkg gno.land/r/demo/wugnot gnoland start # add contract -gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/demo/proxywugnot -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1 +gnokey maketx addpkg -pkgdir $WORK -pkgpath gno.land/r/demo/proxywugnot -gas-fee 1000000ugnot -gas-wanted 4000000 -broadcast -chainid=tendermint_test test1 stdout OK! # approve wugnot to `proxywugnot ≈ g1fndyg0we60rdfchyy5dwxzkfmhl5u34j932rg3` @@ -24,7 +24,7 @@ stdout '10000 uint64' # unwrap 500 wugnot gnokey maketx call -pkgpath gno.land/r/demo/proxywugnot -func ProxyUnwrap -args 500 -gas-fee 1000000ugnot -gas-wanted 2000000 -broadcast -chainid=tendermint_test test1 -# XXX without patching anything it will panic +# XXX without patching anything it will panic # panic msg: insufficient coins error # XXX with pathcing only wugnot.gnot it will panic # panic msg: RealmSendBanker can only send from the realm package address "g1fndyg0we60rdfchyy5dwxzkfmhl5u34j932rg3", but got "g1pf6dv9fjk3rn0m4jjcne306ga4he3mzmupfjl6" @@ -46,8 +46,6 @@ import ( "std" "gno.land/r/demo/wugnot" - - "gno.land/p/demo/ufmt" pusers "gno.land/p/demo/users" ) @@ -73,7 +71,6 @@ func ProxyUnwrap(wugnotAmount uint64) { if wugnotAmount == 0 { return } - userOldWugnot := wugnot.BalanceOf(pusers.AddressOrName(std.GetOrigCaller())) // SEND WUGNOT: USER -> PROXY_WUGNOT wugnot.TransferFrom(pusers.AddressOrName(std.GetOrigCaller()), pusers.AddressOrName(std.CurrentRealm().Addr()), wugnotAmount) @@ -84,4 +81,4 @@ func ProxyUnwrap(wugnotAmount uint64) { // SEND GNOT: PROXY_WUGNOT -> USER banker := std.GetBanker(std.BankerTypeRealmSend) banker.SendCoins(std.CurrentRealm().Addr(), std.GetOrigCaller(), std.Coins{{"ugnot", int64(wugnotAmount)}}) -} \ No newline at end of file +} diff --git a/gno.land/pkg/gnoclient/client_txs.go b/gno.land/pkg/gnoclient/client_txs.go index a61fa9a892d..e306d737ede 100644 --- a/gno.land/pkg/gnoclient/client_txs.go +++ b/gno.land/pkg/gnoclient/client_txs.go @@ -2,7 +2,6 @@ package gnoclient import ( "github.com/gnolang/gno/gno.land/pkg/sdk/vm" - "github.com/gnolang/gno/gnovm/pkg/transpiler" "github.com/gnolang/gno/tm2/pkg/amino" ctypes "github.com/gnolang/gno/tm2/pkg/bft/rpc/core/types" "github.com/gnolang/gno/tm2/pkg/crypto" @@ -145,11 +144,6 @@ func (c *Client) Run(cfg BaseTxCfg, msgs ...MsgRun) (*ctypes.ResultBroadcastTxCo caller := c.Signer.Info().GetAddress() - // Transpile and validate Gno syntax - if err = transpiler.TranspileAndCheckMempkg(msg.Package); err != nil { - return nil, err - } - msg.Package.Name = "main" msg.Package.Path = "" @@ -263,11 +257,6 @@ func (c *Client) AddPackage(cfg BaseTxCfg, msgs ...MsgAddPackage) (*ctypes.Resul caller := c.Signer.Info().GetAddress() - // Transpile and validate Gno syntax - if err = transpiler.TranspileAndCheckMempkg(msg.Package); err != nil { - return nil, err - } - // Unwrap syntax sugar to vm.MsgCall slice vmMsgs = append(vmMsgs, std.Msg(vm.MsgAddPackage{ Creator: caller, diff --git a/gno.land/pkg/gnoclient/integration_test.go b/gno.land/pkg/gnoclient/integration_test.go index ace9022e35d..985ec0f8d53 100644 --- a/gno.land/pkg/gnoclient/integration_test.go +++ b/gno.land/pkg/gnoclient/integration_test.go @@ -246,7 +246,6 @@ func TestRunSingle_Integration(t *testing.T) { fileBody := `package main import ( - "std" "gno.land/p/demo/ufmt" "gno.land/r/demo/tests" ) @@ -305,7 +304,6 @@ func TestRunMultiple_Integration(t *testing.T) { fileBody1 := `package main import ( - "std" "gno.land/p/demo/ufmt" "gno.land/r/demo/tests" ) @@ -319,7 +317,6 @@ func main() { fileBody2 := `package main import ( - "std" "gno.land/p/demo/ufmt" "gno.land/r/demo/deep/very/deep" ) diff --git a/gno.land/pkg/keyscli/addpkg.go b/gno.land/pkg/keyscli/addpkg.go index b04c39398ce..9cac68c4c79 100644 --- a/gno.land/pkg/keyscli/addpkg.go +++ b/gno.land/pkg/keyscli/addpkg.go @@ -7,7 +7,6 @@ import ( "github.com/gnolang/gno/gno.land/pkg/sdk/vm" gno "github.com/gnolang/gno/gnovm/pkg/gnolang" - "github.com/gnolang/gno/gnovm/pkg/transpiler" "github.com/gnolang/gno/tm2/pkg/amino" "github.com/gnolang/gno/tm2/pkg/commands" "github.com/gnolang/gno/tm2/pkg/crypto/keys" @@ -102,12 +101,6 @@ func execMakeAddPkg(cfg *MakeAddPkgCfg, args []string, io commands.IO) error { panic(fmt.Sprintf("found an empty package %q", cfg.PkgPath)) } - // transpile and validate syntax - err = transpiler.TranspileAndCheckMempkg(memPkg) - if err != nil { - panic(err) - } - // parse gas wanted & fee. gaswanted := cfg.RootCfg.GasWanted gasfee, err := std.ParseCoin(cfg.RootCfg.GasFee) diff --git a/gno.land/pkg/keyscli/run.go b/gno.land/pkg/keyscli/run.go index dc4c6d1d28d..9ae6132c8f3 100644 --- a/gno.land/pkg/keyscli/run.go +++ b/gno.land/pkg/keyscli/run.go @@ -9,7 +9,6 @@ import ( "github.com/gnolang/gno/gno.land/pkg/sdk/vm" gno "github.com/gnolang/gno/gnovm/pkg/gnolang" - "github.com/gnolang/gno/gnovm/pkg/transpiler" "github.com/gnolang/gno/tm2/pkg/amino" "github.com/gnolang/gno/tm2/pkg/commands" "github.com/gnolang/gno/tm2/pkg/crypto/keys" @@ -109,11 +108,7 @@ func execMakeRun(cfg *MakeRunCfg, args []string, cmdio commands.IO) error { if memPkg.IsEmpty() { panic(fmt.Sprintf("found an empty package %q", memPkg.Path)) } - // transpile and validate syntax - err = transpiler.TranspileAndCheckMempkg(memPkg) - if err != nil { - panic(err) - } + memPkg.Name = "main" // Set to empty; this will be automatically set by the VM keeper. memPkg.Path = "" diff --git a/gno.land/pkg/sdk/vm/builtins.go b/gno.land/pkg/sdk/vm/builtins.go index 63062847e01..368ada6ff82 100644 --- a/gno.land/pkg/sdk/vm/builtins.go +++ b/gno.land/pkg/sdk/vm/builtins.go @@ -16,7 +16,7 @@ func (vm *VMKeeper) initBuiltinPackagesAndTypes(store gno.Store) { // NOTE: native functions/methods added here must be quick operations, // or account for gas before operation. // TODO: define criteria for inclusion, and solve gas calculations. - getPackage := func(pkgPath string) (pn *gno.PackageNode, pv *gno.PackageValue) { + getPackage := func(pkgPath string, newStore gno.Store) (pn *gno.PackageNode, pv *gno.PackageValue) { // otherwise, built-in package value. // first, load from filepath. stdlibPath := filepath.Join(vm.stdlibsDir, pkgPath) @@ -34,7 +34,7 @@ func (vm *VMKeeper) initBuiltinPackagesAndTypes(store gno.Store) { PkgPath: "gno.land/r/stdlibs/" + pkgPath, // PkgPath: pkgPath, Output: os.Stdout, - Store: store, + Store: newStore, }) defer m2.Release() return m2.RunMemPackage(memPkg, true) diff --git a/gno.land/pkg/sdk/vm/errors.go b/gno.land/pkg/sdk/vm/errors.go index 64778f8b467..132d98b7ecd 100644 --- a/gno.land/pkg/sdk/vm/errors.go +++ b/gno.land/pkg/sdk/vm/errors.go @@ -1,6 +1,11 @@ package vm -import "github.com/gnolang/gno/tm2/pkg/errors" +import ( + "strings" + + "github.com/gnolang/gno/tm2/pkg/errors" + "go.uber.org/multierr" +) // for convenience: type abciError struct{} @@ -13,11 +18,21 @@ type ( InvalidPkgPathError struct{ abciError } InvalidStmtError struct{ abciError } InvalidExprError struct{ abciError } + TypeCheckError struct { + abciError + Errors []string + } ) func (e InvalidPkgPathError) Error() string { return "invalid package path" } func (e InvalidStmtError) Error() string { return "invalid statement" } func (e InvalidExprError) Error() string { return "invalid expression" } +func (e TypeCheckError) Error() string { + var bld strings.Builder + bld.WriteString("invalid gno package; type check errors:\n") + bld.WriteString(strings.Join(e.Errors, "\n")) + return bld.String() +} func ErrInvalidPkgPath(msg string) error { return errors.Wrap(InvalidPkgPathError{}, msg) @@ -30,3 +45,12 @@ func ErrInvalidStmt(msg string) error { func ErrInvalidExpr(msg string) error { return errors.Wrap(InvalidExprError{}, msg) } + +func ErrTypeCheck(err error) error { + var tce TypeCheckError + errs := multierr.Errors(err) + for _, err := range errs { + tce.Errors = append(tce.Errors, err.Error()) + } + return errors.NewWithData(tce).Stacktrace() +} diff --git a/gno.land/pkg/sdk/vm/gas_test.go b/gno.land/pkg/sdk/vm/gas_test.go index 75d13aa6c5d..9f4ae1a6678 100644 --- a/gno.land/pkg/sdk/vm/gas_test.go +++ b/gno.land/pkg/sdk/vm/gas_test.go @@ -86,7 +86,7 @@ func TestAddPkgDeliverTxFailed(t *testing.T) { gasDeliver := gctx.GasMeter().GasConsumed() assert.False(t, res.IsOK()) - assert.Equal(t, int64(17989), gasDeliver) + assert.Equal(t, int64(2231), gasDeliver) } // Not enough gas for a failed transaction. @@ -98,7 +98,7 @@ func TestAddPkgDeliverTxFailedNoGas(t *testing.T) { ctx = ctx.WithMode(sdk.RunTxModeDeliver) simulate = false - tx.Fee.GasWanted = 17988 + tx.Fee.GasWanted = 2230 gctx := auth.SetGasMeter(simulate, ctx, tx.Fee.GasWanted) var res sdk.Result @@ -116,7 +116,7 @@ func TestAddPkgDeliverTxFailedNoGas(t *testing.T) { assert.True(t, abort) assert.False(t, res.IsOK()) gasCheck := gctx.GasMeter().GasConsumed() - assert.Equal(t, int64(17989), gasCheck) + assert.Equal(t, int64(2231), gasCheck) } else { t.Errorf("should panic") } diff --git a/gno.land/pkg/sdk/vm/keeper.go b/gno.land/pkg/sdk/vm/keeper.go index c032ca4b7db..e7757235020 100644 --- a/gno.land/pkg/sdk/vm/keeper.go +++ b/gno.land/pkg/sdk/vm/keeper.go @@ -160,6 +160,11 @@ func (vm *VMKeeper) AddPackage(ctx sdk.Context, msg MsgAddPackage) (err error) { return ErrInvalidPkgPath("reserved package name: " + pkgPath) } + // Validate Gno syntax and type check. + if err := gno.TypeCheckMemPackage(memPkg, gnostore); err != nil { + return ErrTypeCheck(err) + } + // Pay deposit from creator. pkgAddr := gno.DerivePkgAddr(pkgPath) @@ -336,6 +341,11 @@ func (vm *VMKeeper) Run(ctx sdk.Context, msg MsgRun) (res string, err error) { return "", ErrInvalidPkgPath(err.Error()) } + // Validate Gno syntax and type check. + if err = gno.TypeCheckMemPackage(memPkg, gnostore); err != nil { + return "", ErrTypeCheck(err) + } + // Send send-coins to pkg from caller. err = vm.bank.SendCoins(ctx, caller, pkgAddr, send) if err != nil { @@ -616,6 +626,9 @@ func (vm *VMKeeper) QueryFile(ctx sdk.Context, filepath string) (res string, err return memFile.Body, nil } else { memPkg := store.GetMemPackage(dirpath) + if memPkg == nil { + return "", fmt.Errorf("package %q is not available", dirpath) // TODO: XSS protection + } for i, memfile := range memPkg.Files { if i > 0 { res += "\n" diff --git a/gno.land/pkg/sdk/vm/keeper_test.go b/gno.land/pkg/sdk/vm/keeper_test.go index bd8762b9831..9d74a855a61 100644 --- a/gno.land/pkg/sdk/vm/keeper_test.go +++ b/gno.land/pkg/sdk/vm/keeper_test.go @@ -31,8 +31,6 @@ func TestVMKeeperAddPackage(t *testing.T) { Name: "test.gno", Body: `package test -import "std" - func Echo() string { return "hello world" }`, @@ -413,8 +411,6 @@ func TestNumberOfArgsError(t *testing.T) { Name: "test.gno", Body: `package test -import "std" - func Echo(msg string) string { return "echo:"+msg }`, diff --git a/gno.land/pkg/sdk/vm/package.go b/gno.land/pkg/sdk/vm/package.go index 01fad3284e3..b2e7fbecfc4 100644 --- a/gno.land/pkg/sdk/vm/package.go +++ b/gno.land/pkg/sdk/vm/package.go @@ -20,4 +20,5 @@ var Package = amino.RegisterPackage(amino.NewPackage( InvalidPkgPathError{}, "InvalidPkgPathError", InvalidStmtError{}, "InvalidStmtError", InvalidExprError{}, "InvalidExprError", + TypeCheckError{}, "TypeCheckError", )) diff --git a/gnovm/pkg/gnolang/go2gno.go b/gnovm/pkg/gnolang/go2gno.go index 8ca985352a7..5c74621c973 100644 --- a/gnovm/pkg/gnolang/go2gno.go +++ b/gnovm/pkg/gnolang/go2gno.go @@ -35,12 +35,16 @@ import ( "go/ast" "go/parser" "go/token" + "go/types" "os" "reflect" "strconv" + "strings" "github.com/davecgh/go-spew/spew" "github.com/gnolang/gno/tm2/pkg/errors" + "github.com/gnolang/gno/tm2/pkg/std" + "go.uber.org/multierr" ) func MustReadFile(path string) *FileNode { @@ -102,7 +106,10 @@ func MustParseExpr(expr string) Expr { func ParseFile(filename string, body string) (fn *FileNode, err error) { // Use go parser to parse the body. fs := token.NewFileSet() - f, err := parser.ParseFile(fs, filename, body, parser.ParseComments|parser.DeclarationErrors) + // TODO(morgan): would be nice to add parser.SkipObjectResolution as we don't + // seem to be using its features, but this breaks when testing (specifically redeclaration tests). + const parseOpts = parser.ParseComments | parser.DeclarationErrors + f, err := parser.ParseFile(fs, filename, body, parseOpts) if err != nil { return nil, err } @@ -469,6 +476,104 @@ func Go2Gno(fs *token.FileSet, gon ast.Node) (n Node) { } } +//---------------------------------------- +// type checking (using go/types) + +// MemPackageGetter implements the GetMemPackage() method. It is a subset of +// [Store], separated for ease of testing. +type MemPackageGetter interface { + GetMemPackage(path string) *std.MemPackage +} + +// TypeCheckMemPackage performs type validation and checking on the given +// mempkg. To retrieve dependencies, it uses getter. +// +// The syntax checking is performed entirely using Go's go/types package. +func TypeCheckMemPackage(mempkg *std.MemPackage, getter MemPackageGetter) error { + var errs error + imp := &gnoImporter{ + getter: getter, + cache: map[string]gnoImporterResult{}, + cfg: &types.Config{ + Error: func(err error) { + errs = multierr.Append(errs, err) + }, + }, + } + imp.cfg.Importer = imp + + _, err := imp.parseCheckMemPackage(mempkg) + // prefer to return errs instead of err: + // err will generally contain only the first error encountered. + if errs != nil { + return errs + } + return err +} + +type gnoImporterResult struct { + pkg *types.Package + err error +} + +type gnoImporter struct { + getter MemPackageGetter + cache map[string]gnoImporterResult + cfg *types.Config +} + +// Unused, but satisfies the Importer interface. +func (g *gnoImporter) Import(path string) (*types.Package, error) { + return g.ImportFrom(path, "", 0) +} + +type importNotFoundError string + +func (e importNotFoundError) Error() string { return "import not found: " + string(e) } + +// ImportFrom returns the imported package for the given import +// path when imported by a package file located in dir. +func (g *gnoImporter) ImportFrom(path, _ string, _ types.ImportMode) (*types.Package, error) { + if pkg, ok := g.cache[path]; ok { + return pkg.pkg, pkg.err + } + mpkg := g.getter.GetMemPackage(path) + if mpkg == nil { + err := importNotFoundError(path) + g.cache[path] = gnoImporterResult{err: err} + return nil, err + } + result, err := g.parseCheckMemPackage(mpkg) + g.cache[path] = gnoImporterResult{pkg: result, err: err} + return result, err +} + +func (g *gnoImporter) parseCheckMemPackage(mpkg *std.MemPackage) (*types.Package, error) { + fset := token.NewFileSet() + files := make([]*ast.File, 0, len(mpkg.Files)) + var errs error + for _, file := range mpkg.Files { + if !strings.HasSuffix(file.Name, ".gno") || + endsWith(file.Name, []string{"_test.gno", "_filetest.gno"}) { + continue // skip spurious file. + } + + const parseOpts = parser.ParseComments | parser.DeclarationErrors | parser.SkipObjectResolution + f, err := parser.ParseFile(fset, file.Name, file.Body, parseOpts) + if err != nil { + errs = multierr.Append(errs, err) + continue + } + + files = append(files, f) + } + if errs != nil { + return nil, errs + } + + return g.cfg.Check(mpkg.Path, fset, files, nil) +} + //---------------------------------------- // utility methods diff --git a/gnovm/pkg/gnolang/go2gno_test.go b/gnovm/pkg/gnolang/go2gno_test.go index 920c5acd9c8..995ca3e8c0e 100644 --- a/gnovm/pkg/gnolang/go2gno_test.go +++ b/gnovm/pkg/gnolang/go2gno_test.go @@ -4,7 +4,10 @@ import ( "fmt" "testing" + "github.com/gnolang/gno/tm2/pkg/std" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/multierr" ) func TestParseForLoop(t *testing.T) { @@ -25,3 +28,308 @@ func main(){ fmt.Printf("AST:\n%#v\n\n", n) fmt.Printf("AST.String():\n%s\n", n.String()) } + +type mockPackageGetter []*std.MemPackage + +func (mi mockPackageGetter) GetMemPackage(path string) *std.MemPackage { + for _, pkg := range mi { + if pkg.Path == path { + return pkg + } + } + return nil +} + +type mockPackageGetterCounts struct { + mockPackageGetter + counts map[string]int +} + +func (mpg mockPackageGetterCounts) GetMemPackage(path string) *std.MemPackage { + mpg.counts[path]++ + return mpg.mockPackageGetter.GetMemPackage(path) +} + +func TestTypeCheckMemPackage(t *testing.T) { + t.Parallel() + + // if len(ss) > 0, then multierr.Errors must decompose it in errors, and + // each error in order must contain the associated string. + errContains := func(s0 string, ss ...string) func(*testing.T, error) { + return func(t *testing.T, err error) { + t.Helper() + errs := multierr.Errors(err) + if len(errs) == 0 { + t.Errorf("expected an error, got nil") + return + } + want := len(ss) + 1 + if len(errs) != want { + t.Errorf("expected %d errors, got %d", want, len(errs)) + return + } + assert.ErrorContains(t, errs[0], s0) + for idx, err := range errs[1:] { + assert.ErrorContains(t, err, ss[idx]) + } + } + } + + type testCase struct { + name string + pkg *std.MemPackage + getter MemPackageGetter + check func(*testing.T, error) + } + tt := []testCase{ + { + "Simple", + &std.MemPackage{ + Name: "hello", + Path: "gno.land/p/demo/hello", + Files: []*std.MemFile{ + { + Name: "hello.gno", + Body: ` + package hello + type S struct{} + func A() S { return S{} } + func B() S { return A() }`, + }, + }, + }, + nil, + nil, + }, + { + "WrongReturn", + &std.MemPackage{ + Name: "hello", + Path: "gno.land/p/demo/hello", + Files: []*std.MemFile{ + { + Name: "hello.gno", + Body: ` + package hello + type S struct{} + func A() S { return S{} } + func B() S { return 11 }`, + }, + }, + }, + nil, + errContains("cannot use 11"), + }, + { + "ParseError", + &std.MemPackage{ + Name: "hello", + Path: "gno.land/p/demo/hello", + Files: []*std.MemFile{ + { + Name: "hello.gno", + Body: ` + package hello! + func B() int { return 11 }`, + }, + }, + }, + nil, + errContains("found '!'"), + }, + { + "MultiError", + &std.MemPackage{ + Name: "main", + Path: "gno.land/p/demo/main", + Files: []*std.MemFile{ + { + Name: "hello.gno", + Body: ` + package main + func main() { + _, _ = 11 + return 88, 88 + }`, + }, + }, + }, + nil, + errContains("assignment mismatch", "too many return values"), + }, + { + "TestsIgnored", + &std.MemPackage{ + Name: "hello", + Path: "gno.land/p/demo/hello", + Files: []*std.MemFile{ + { + Name: "hello.gno", + Body: ` + package hello + func B() int { return 11 }`, + }, + { + Name: "hello_test.gno", + Body: `This is not valid Gno code, but it doesn't matter because test + files are not checked.`, + }, + }, + }, + nil, + nil, + }, + { + "ImportFailed", + &std.MemPackage{ + Name: "hello", + Path: "gno.land/p/demo/hello", + Files: []*std.MemFile{ + { + Name: "hello.gno", + Body: ` + package hello + import "std" + func Hello() std.Address { return "hello" }`, + }, + }, + }, + mockPackageGetter{}, + errContains("import not found: std"), + }, + { + "ImportSucceeded", + &std.MemPackage{ + Name: "hello", + Path: "gno.land/p/demo/hello", + Files: []*std.MemFile{ + { + Name: "hello.gno", + Body: ` + package hello + import "std" + func Hello() std.Address { return "hello" }`, + }, + }, + }, + mockPackageGetter{ + &std.MemPackage{ + Name: "std", + Path: "std", + Files: []*std.MemFile{ + { + Name: "std.gno", + Body: ` + package std + type Address string`, + }, + }, + }, + }, + nil, + }, + { + "ImportBadIdent", + &std.MemPackage{ + Name: "hello", + Path: "gno.land/p/demo/hello", + Files: []*std.MemFile{ + { + Name: "hello.gno", + Body: ` + package hello + import "std" + func Hello() std.Address { return "hello" }`, + }, + }, + }, + mockPackageGetter{ + &std.MemPackage{ + Name: "a_completely_different_identifier", + Path: "std", + Files: []*std.MemFile{ + { + Name: "std.gno", + Body: ` + package a_completely_different_identifier + type Address string`, + }, + }, + }, + }, + errContains("undefined: std", "a_completely_different_identifier and not used"), + }, + } + + cacheMpg := mockPackageGetterCounts{ + mockPackageGetter{ + &std.MemPackage{ + Name: "bye", + Path: "bye", + Files: []*std.MemFile{ + { + Name: "bye.gno", + Body: ` + package bye + import "std" + func Bye() std.Address { return "bye" }`, + }, + }, + }, + &std.MemPackage{ + Name: "std", + Path: "std", + Files: []*std.MemFile{ + { + Name: "std.gno", + Body: ` + package std + type Address string`, + }, + }, + }, + }, + make(map[string]int), + } + + tt = append(tt, testCase{ + "ImportWithCache", + // This test will make use of the importer's internal cache for package `std`. + &std.MemPackage{ + Name: "hello", + Path: "gno.land/p/demo/hello", + Files: []*std.MemFile{ + { + Name: "hello.gno", + Body: ` + package hello + import ( + "std" + "bye" + ) + func Hello() std.Address { return bye.Bye() }`, + }, + }, + }, + cacheMpg, + func(t *testing.T, err error) { + t.Helper() + require.NoError(t, err) + assert.Equal(t, map[string]int{"std": 1, "bye": 1}, cacheMpg.counts) + }, + }) + + for _, tc := range tt { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + err := TypeCheckMemPackage(tc.pkg, tc.getter) + if tc.check == nil { + assert.NoError(t, err) + } else { + tc.check(t, err) + } + }) + } +} diff --git a/gnovm/pkg/gnolang/gonative_test.go b/gnovm/pkg/gnolang/gonative_test.go index d023624a758..42729b43699 100644 --- a/gnovm/pkg/gnolang/gonative_test.go +++ b/gnovm/pkg/gnolang/gonative_test.go @@ -15,7 +15,7 @@ import ( // and the odd index items are corresponding package values. func gonativeTestStore(args ...interface{}) Store { store := NewStore(nil, nil, nil) - store.SetPackageGetter(func(pkgPath string) (*PackageNode, *PackageValue) { + store.SetPackageGetter(func(pkgPath string, _ Store) (*PackageNode, *PackageValue) { for i := 0; i < len(args)/2; i++ { pn := args[i*2].(*PackageNode) pv := args[i*2+1].(*PackageValue) @@ -90,11 +90,12 @@ func main() { n := MustParseFile("main.go", c) m.RunFiles(n) m.RunMain() + // weird `+` is used to place a space, without having editors strip it away. assert.Equal(t, `A: 1 B: 0 C: 0 -D: -`, out.String()) +D: `+` +`, string(out.Bytes())) } func TestGoNativeDefine3(t *testing.T) { @@ -132,7 +133,7 @@ func main() { assert.Equal(t, `A: 1 B: 0 C: 0 -D: +D: `+` `, out.String()) } diff --git a/gnovm/pkg/gnolang/store.go b/gnovm/pkg/gnolang/store.go index d15976ec262..7fc54c12b0f 100644 --- a/gnovm/pkg/gnolang/store.go +++ b/gnovm/pkg/gnolang/store.go @@ -3,6 +3,7 @@ package gnolang import ( "fmt" "reflect" + "slices" "strconv" "strings" @@ -11,8 +12,13 @@ import ( "github.com/gnolang/gno/tm2/pkg/store" ) -// return nil if package doesn't exist. -type PackageGetter func(pkgPath string) (*PackageNode, *PackageValue) +// PackageGetter specifies how the store may retrieve packages which are not +// already in its cache. PackageGetter should return nil when the requested +// package does not exist. store should be used to run the machine, or otherwise +// call any methods which may call store.GetPackage; avoid using any "global" +// store as the one passed to the PackageGetter may be a fork of that (ie. +// the original is not meant to be written to). +type PackageGetter func(pkgPath string, store Store) (*PackageNode, *PackageValue) // inject natives into a new or loaded package (value and node) type PackageInjector func(store Store, pn *PackageNode) @@ -78,8 +84,8 @@ type defaultStore struct { go2gnoStrict bool // if true, native->gno type conversion must be registered. // transient - opslog []StoreOp // for debugging and testing. - current map[string]struct{} // for detecting import cycles. + opslog []StoreOp // for debugging and testing. + current []string } func NewStore(alloc *Allocator, baseStore, iavlStore store.Store) *defaultStore { @@ -93,7 +99,6 @@ func NewStore(alloc *Allocator, baseStore, iavlStore store.Store) *defaultStore baseStore: baseStore, iavlStore: iavlStore, go2gnoStrict: true, - current: make(map[string]struct{}), } InitStoreCaches(ds) return ds @@ -109,13 +114,14 @@ func (ds *defaultStore) SetPackageGetter(pg PackageGetter) { // Gets package from cache, or loads it from baseStore, or gets it from package getter. func (ds *defaultStore) GetPackage(pkgPath string, isImport bool) *PackageValue { - // detect circular imports if isImport { - if _, exists := ds.current[pkgPath]; exists { - panic(fmt.Sprintf("import cycle detected: %q", pkgPath)) + if slices.Contains(ds.current, pkgPath) { + panic(fmt.Sprintf("import cycle detected: %q (through %v)", pkgPath, ds.current)) } - ds.current[pkgPath] = struct{}{} - defer delete(ds.current, pkgPath) + ds.current = append(ds.current, pkgPath) + defer func() { + ds.current = ds.current[:len(ds.current)-1] + }() } // first, check cache. oid := ObjectIDFromPkgPath(pkgPath) @@ -160,7 +166,7 @@ func (ds *defaultStore) GetPackage(pkgPath string, isImport bool) *PackageValue } // otherwise, fetch from pkgGetter. if ds.pkgGetter != nil { - if pn, pv := ds.pkgGetter(pkgPath); pv != nil { + if pn, pv := ds.pkgGetter(pkgPath, ds); pv != nil { // e.g. tests/imports_tests loads example/gno.land/r/... realms. // if pv.IsRealm() { // panic("realm packages cannot be gotten from pkgGetter") @@ -532,20 +538,41 @@ func (ds *defaultStore) AddMemPackage(memPkg *std.MemPackage) { ds.iavlStore.Set(pathkey, bz) } +// GetMemPackage retrieves the MemPackage at the given path. +// It returns nil if the package could not be found. func (ds *defaultStore) GetMemPackage(path string) *std.MemPackage { + return ds.getMemPackage(path, false) +} + +func (ds *defaultStore) getMemPackage(path string, isRetry bool) *std.MemPackage { pathkey := []byte(backendPackagePathKey(path)) bz := ds.iavlStore.Get(pathkey) if bz == nil { - panic(fmt.Sprintf( - "missing package at path %s", string(pathkey))) + // If this is the first try, attempt using GetPackage to retrieve the + // package, first. GetPackage can leverage pkgGetter, which in most + // implementations works by running Machine.RunMemPackage with save = true, + // which would add the package to the store after running. + // Some packages may never be persisted, thus why we only attempt this twice. + if !isRetry { + if pv := ds.GetPackage(path, false); pv != nil { + return ds.getMemPackage(path, true) + } + } + return nil } var memPkg *std.MemPackage amino.MustUnmarshal(bz, &memPkg) return memPkg } +// GetMemFile retrieves the MemFile with the given name, contained in the +// MemPackage at the given path. It returns nil if the file or the package +// do not exist. func (ds *defaultStore) GetMemFile(path string, name string) *std.MemFile { memPkg := ds.GetMemPackage(path) + if memPkg == nil { + return nil + } memFile := memPkg.GetFile(name) return memFile } @@ -585,9 +612,6 @@ func (ds *defaultStore) ClearObjectCache() { ds.alloc.Reset() ds.cacheObjects = make(map[ObjectID]Object) // new cache. ds.opslog = nil // new ops log. - if len(ds.current) > 0 { - ds.current = make(map[string]struct{}) - } ds.SetCachePackage(Uverse()) } @@ -607,7 +631,6 @@ func (ds *defaultStore) Fork() Store { nativeStore: ds.nativeStore, go2gnoStrict: ds.go2gnoStrict, opslog: nil, // new ops log. - current: make(map[string]struct{}), } ds2.SetCachePackage(Uverse()) return ds2 diff --git a/gnovm/pkg/transpiler/transpiler.go b/gnovm/pkg/transpiler/transpiler.go index a87a336125e..cc49aac4c78 100644 --- a/gnovm/pkg/transpiler/transpiler.go +++ b/gnovm/pkg/transpiler/transpiler.go @@ -16,10 +16,7 @@ import ( "strconv" "strings" - "go.uber.org/multierr" "golang.org/x/tools/go/ast/astutil" - - "github.com/gnolang/gno/tm2/pkg/std" ) const ( @@ -123,44 +120,6 @@ func GetTranspileFilenameAndTags(gnoFilePath string) (targetFilename, tags strin return } -func TranspileAndCheckMempkg(mempkg *std.MemPackage) error { - gofmt := "gofmt" - - tmpDir, err := os.MkdirTemp("", mempkg.Name) - if err != nil { - return err - } - defer os.RemoveAll(tmpDir) //nolint: errcheck - - var errs error - for _, mfile := range mempkg.Files { - if !strings.HasSuffix(mfile.Name, ".gno") { - continue // skip spurious file. - } - res, err := Transpile(mfile.Body, "gno,tmp", mfile.Name) - if err != nil { - errs = multierr.Append(errs, err) - continue - } - tmpFile := filepath.Join(tmpDir, mfile.Name) - err = os.WriteFile(tmpFile, []byte(res.Translated), 0o644) - if err != nil { - errs = multierr.Append(errs, err) - continue - } - err = TranspileVerifyFile(tmpFile, gofmt) - if err != nil { - errs = multierr.Append(errs, err) - continue - } - } - - if errs != nil { - return fmt.Errorf("transpile package: %w", errs) - } - return nil -} - func Transpile(source string, tags string, filename string) (*transpileResult, error) { fset := token.NewFileSet() f, err := parser.ParseFile(fset, filename, source, parser.ParseComments) diff --git a/gnovm/stdlibs/io/io.gno b/gnovm/stdlibs/io/io.gno index e1c8de731b6..bdbab135140 100644 --- a/gnovm/stdlibs/io/io.gno +++ b/gnovm/stdlibs/io/io.gno @@ -476,28 +476,29 @@ func (l *LimitedReader) Read(p []byte) (n int, err error) { return } -// NewSectionReader returns a SectionReader that reads from r +// NewSectionReader returns a [SectionReader] that reads from r // starting at offset off and stops with EOF after n bytes. func NewSectionReader(r ReaderAt, off int64, n int64) *SectionReader { var remaining int64 - const maxInt64 = 1<<63 - 1 - if off <= maxInt64-n { + const maxint64 = 1<<63 - 1 + if off <= maxint64-n { remaining = n + off } else { // Overflow, with no way to return error. - // Assume we can read up to an offset of `1<<63 - 1` bytes in this case. - remaining = maxInt64 + // Assume we can read up to an offset of 1<<63 - 1. + remaining = maxint64 } - return &SectionReader{r, off, off, remaining} + return &SectionReader{r, off, off, remaining, n} } // SectionReader implements Read, Seek, and ReadAt on a section -// of an underlying ReaderAt. +// of an underlying [ReaderAt]. type SectionReader struct { - r ReaderAt - base int64 + r ReaderAt // constant after creation + base int64 // constant after creation off int64 - limit int64 + limit int64 // constant after creation + n int64 // constant after creation } func (s *SectionReader) Read(p []byte) (n int, err error) { @@ -536,7 +537,7 @@ func (s *SectionReader) Seek(offset int64, whence int) (int64, error) { } func (s *SectionReader) ReadAt(p []byte, off int64) (n int, err error) { - if off < 0 || off >= s.limit-s.base { + if off < 0 || off >= s.Size() { return 0, EOF } off += s.base @@ -554,6 +555,14 @@ func (s *SectionReader) ReadAt(p []byte, off int64) (n int, err error) { // Size returns the size of the section in bytes. func (s *SectionReader) Size() int64 { return s.limit - s.base } +// Outer returns the underlying [ReaderAt] and offsets for the section. +// +// The returned values are the same that were passed to [NewSectionReader] +// when the [SectionReader] was created. +func (s *SectionReader) Outer() (r ReaderAt, off int64, n int64) { + return s.r, s.base, s.n +} + // An OffsetWriter maps writers at offset base to offset base + off in the underlying writer. type OffsetWriter struct { w WriterAt diff --git a/gnovm/tests/files/import6.gno b/gnovm/tests/files/import6.gno index 8c974ea1893..da5dbfbd3b2 100644 --- a/gnovm/tests/files/import6.gno +++ b/gnovm/tests/files/import6.gno @@ -7,4 +7,4 @@ func main() { } // Error: -// github.com/gnolang/gno/_test/c2/c2.gno:1: import cycle detected: "github.com/gnolang/gno/_test/c1" +// github.com/gnolang/gno/_test/c2/c2.gno:1: import cycle detected: "github.com/gnolang/gno/_test/c1" (through [github.com/gnolang/gno/_test/c1 github.com/gnolang/gno/_test/c2]) diff --git a/gnovm/tests/imports.go b/gnovm/tests/imports.go index 43d0969c007..d5541fb0554 100644 --- a/gnovm/tests/imports.go +++ b/gnovm/tests/imports.go @@ -63,7 +63,7 @@ const ( // NOTE: this isn't safe, should only be used for testing. func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Writer, mode importMode) (store gno.Store) { - getPackage := func(pkgPath string) (pn *gno.PackageNode, pv *gno.PackageValue) { + getPackage := func(pkgPath string, newStore gno.Store) (pn *gno.PackageNode, pv *gno.PackageValue) { if pkgPath == "" { panic(fmt.Sprintf("invalid zero package path in testStore().pkgGetter")) } @@ -84,7 +84,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri m2 := gno.NewMachineWithOptions(gno.MachineOptions{ PkgPath: "test", Output: stdout, - Store: store, + Store: newStore, Context: ctx, }) // pkg := gno.NewPackageNode(gno.Name(memPkg.Name), memPkg.Path, nil) @@ -97,7 +97,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri // if stdlibs package is preferred , try to load it first. if mode == ImportModeStdlibsOnly || mode == ImportModeStdlibsPreferred { - pn, pv = loadStdlib(rootDir, pkgPath, store, stdout) + pn, pv = loadStdlib(rootDir, pkgPath, newStore, stdout) if pn != nil { return } @@ -381,7 +381,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri // if native package is preferred, try to load stdlibs/* as backup. if mode == ImportModeNativePreferred { - pn, pv = loadStdlib(rootDir, pkgPath, store, stdout) + pn, pv = loadStdlib(rootDir, pkgPath, newStore, stdout) if pn != nil { return } @@ -400,7 +400,7 @@ func TestStore(rootDir, filesPath string, stdin io.Reader, stdout, stderr io.Wri m2 := gno.NewMachineWithOptions(gno.MachineOptions{ PkgPath: "test", Output: stdout, - Store: store, + Store: newStore, Context: ctx, }) pn, pv = m2.RunMemPackage(memPkg, true) diff --git a/tm2/pkg/crypto/keys/client/broadcast.go b/tm2/pkg/crypto/keys/client/broadcast.go index 423714b2141..c088c63d19f 100644 --- a/tm2/pkg/crypto/keys/client/broadcast.go +++ b/tm2/pkg/crypto/keys/client/broadcast.go @@ -21,6 +21,10 @@ type BroadcastCfg struct { // internal tx *std.Tx + // Set by SignAndBroadcastHandler, similar to DryRun. + // If true, simulation is attempted but not printed; + // the result is only returned in case of an error. + testSimulate bool } func NewBroadcastCmd(rootCfg *BaseCfg, io commands.IO) *commands.Command { @@ -81,6 +85,8 @@ func execBroadcast(cfg *BroadcastCfg, args []string, io commands.IO) error { io.Println("OK!") io.Println("GAS WANTED:", res.DeliverTx.GasWanted) io.Println("GAS USED: ", res.DeliverTx.GasUsed) + io.Println("HEIGHT: ", res.Height) + io.Println("EVENTS: ", string(res.DeliverTx.EncodeEvents())) } return nil } @@ -91,7 +97,7 @@ func BroadcastHandler(cfg *BroadcastCfg) (*ctypes.ResultBroadcastTxCommit, error } remote := cfg.RootCfg.Remote - if remote == "" || remote == "y" { + if remote == "" { return nil, errors.New("missing remote url") } @@ -105,8 +111,15 @@ func BroadcastHandler(cfg *BroadcastCfg) (*ctypes.ResultBroadcastTxCommit, error return nil, err } - if cfg.DryRun { - return SimulateTx(cli, bz) + // Both for DryRun and testSimulate, we perform simulation. + // However, DryRun always returns here, while in case of success + // testSimulate continues onto broadcasting the transaction. + if cfg.DryRun || cfg.testSimulate { + res, err := SimulateTx(cli, bz) + hasError := err != nil || res.CheckTx.IsErr() || res.DeliverTx.IsErr() + if cfg.DryRun || hasError { + return res, err + } } bres, err := cli.BroadcastTxCommit(bz) diff --git a/tm2/pkg/crypto/keys/client/maketx.go b/tm2/pkg/crypto/keys/client/maketx.go index 603be59396c..2afccf9141c 100644 --- a/tm2/pkg/crypto/keys/client/maketx.go +++ b/tm2/pkg/crypto/keys/client/maketx.go @@ -20,7 +20,25 @@ type MakeTxCfg struct { Memo string Broadcast bool - ChainID string + // Valid options are SimulateTest, SimulateSkip or SimulateOnly. + Simulate string + ChainID string +} + +// These are the valid options for MakeTxConfig.Simulate. +const ( + SimulateTest = "test" + SimulateSkip = "skip" + SimulateOnly = "only" +) + +func (c *MakeTxCfg) Validate() error { + switch c.Simulate { + case SimulateTest, SimulateSkip, SimulateOnly: + default: + return fmt.Errorf("invalid simulate option: %q", c.Simulate) + } + return nil } func NewMakeTxCmd(rootCfg *BaseCfg, io commands.IO) *commands.Command { @@ -71,14 +89,24 @@ func (c *MakeTxCfg) RegisterFlags(fs *flag.FlagSet) { &c.Broadcast, "broadcast", false, - "sign and broadcast", + "sign, simulate and broadcast", + ) + + fs.StringVar( + &c.Simulate, + "simulate", + "test", + `select how to simulate the transaction (only useful with --broadcast); valid options are + - test: attempts simulating the transaction, and if successful performs broadcasting (default) + - skip: avoids performing transaction simulation + - only: avoids broadcasting transaction (ie. dry run)`, ) fs.StringVar( &c.ChainID, "chainid", "dev", - "chainid to sign for (only useful if --broadcast)", + "chainid to sign for (only useful with --broadcast)", ) } @@ -139,6 +167,9 @@ func SignAndBroadcastHandler( bopts := &BroadcastCfg{ RootCfg: baseopts, tx: &tx, + + DryRun: cfg.Simulate == SimulateOnly, + testSimulate: cfg.Simulate == SimulateTest, } return BroadcastHandler(bopts) @@ -150,6 +181,10 @@ func ExecSignAndBroadcast( tx std.Tx, io commands.IO, ) error { + if err := cfg.Validate(); err != nil { + return err + } + baseopts := cfg.RootCfg // query account diff --git a/tm2/pkg/crypto/keys/client/query.go b/tm2/pkg/crypto/keys/client/query.go index e44bb796b9d..f4b65adebc0 100644 --- a/tm2/pkg/crypto/keys/client/query.go +++ b/tm2/pkg/crypto/keys/client/query.go @@ -91,7 +91,7 @@ func execQuery(cfg *QueryCfg, args []string, io commands.IO) error { func QueryHandler(cfg *QueryCfg) (*ctypes.ResultABCIQuery, error) { remote := cfg.RootCfg.Remote - if remote == "" || remote == "y" { + if remote == "" { return nil, errors.New("missing remote url") }