From 15732580eba1d5ae7f3bb3696883cba73304f614 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 22 Aug 2022 13:26:12 +0300 Subject: [PATCH 1/3] smartcontract: improve manifest validness errors It should be clear from error what's wrong with ABI (specify bad method/event/parameter identifier). --- pkg/smartcontract/manifest/abi.go | 7 ++++--- pkg/smartcontract/manifest/manifest.go | 3 ++- pkg/smartcontract/manifest/parameter.go | 3 ++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/smartcontract/manifest/abi.go b/pkg/smartcontract/manifest/abi.go index 15f89daf4..3027e5c97 100644 --- a/pkg/smartcontract/manifest/abi.go +++ b/pkg/smartcontract/manifest/abi.go @@ -2,6 +2,7 @@ package manifest import ( "errors" + "fmt" "sort" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" @@ -53,12 +54,12 @@ func (a *ABI) GetEvent(name string) *Event { // IsValid checks ABI consistency and correctness. func (a *ABI) IsValid() error { if len(a.Methods) == 0 { - return errors.New("ABI contains no methods") + return errors.New("no methods") } for i := range a.Methods { err := a.Methods[i].IsValid() if err != nil { - return err + return fmt.Errorf("method %q/%d: %w", a.Methods[i].Name, len(a.Methods[i].Parameters), err) } } if len(a.Methods) > 1 { @@ -92,7 +93,7 @@ func (a *ABI) IsValid() error { for i := range a.Events { err := a.Events[i].IsValid() if err != nil { - return err + return fmt.Errorf("event %q/%d: %w", a.Events[i].Name, len(a.Events[i].Parameters), err) } } if len(a.Events) > 1 { diff --git a/pkg/smartcontract/manifest/manifest.go b/pkg/smartcontract/manifest/manifest.go index e0b968772..ba391a657 100644 --- a/pkg/smartcontract/manifest/manifest.go +++ b/pkg/smartcontract/manifest/manifest.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "errors" + "fmt" "math" ojson "github.com/nspcc-dev/go-ordered-json" @@ -104,7 +105,7 @@ func (m *Manifest) IsValid(hash util.Uint160) error { } err = m.ABI.IsValid() if err != nil { - return err + return fmt.Errorf("ABI: %w", err) } err = Groups(m.Groups).AreValid(hash) if err != nil { diff --git a/pkg/smartcontract/manifest/parameter.go b/pkg/smartcontract/manifest/parameter.go index 88445adde..3e7424522 100644 --- a/pkg/smartcontract/manifest/parameter.go +++ b/pkg/smartcontract/manifest/parameter.go @@ -2,6 +2,7 @@ package manifest import ( "errors" + "fmt" "sort" "github.com/nspcc-dev/neo-go/pkg/smartcontract" @@ -75,7 +76,7 @@ func (p Parameters) AreValid() error { for i := range p { err := p[i].IsValid() if err != nil { - return err + return fmt.Errorf("parameter #%d/%q: %w", i, p[i].Name, err) } } if len(p) < 2 { From 40315fe0922ad0d6cf4dfc47d4ac59686fc32614 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 22 Aug 2022 13:41:57 +0300 Subject: [PATCH 2/3] cli: check manifest for validness if it's got from user input And adjust the test case along the way, unnamed arguments are not allowed for valid manifest. --- cli/smartcontract/generate.go | 12 ++++----- cli/smartcontract/generate_test.go | 40 +++++++++++++++-------------- cli/smartcontract/manifest.go | 11 ++++++-- cli/smartcontract/smart_contract.go | 2 +- 4 files changed, 36 insertions(+), 29 deletions(-) diff --git a/cli/smartcontract/generate.go b/cli/smartcontract/generate.go index ba33dfffb..44d6f6f02 100644 --- a/cli/smartcontract/generate.go +++ b/cli/smartcontract/generate.go @@ -43,7 +43,11 @@ func contractGenerateWrapper(ctx *cli.Context) error { if err := cmdargs.EnsureNone(ctx); err != nil { return err } - m, _, err := readManifest(ctx.String("manifest")) + h, err := util.Uint160DecodeStringLE(strings.TrimPrefix(ctx.String("hash"), "0x")) + if err != nil { + return cli.NewExitError(fmt.Errorf("invalid contract hash: %w", err), 1) + } + m, _, err := readManifest(ctx.String("manifest"), h) if err != nil { return cli.NewExitError(fmt.Errorf("can't read contract manifest: %w", err), 1) } @@ -59,13 +63,7 @@ func contractGenerateWrapper(ctx *cli.Context) error { return cli.NewExitError(fmt.Errorf("can't parse config file: %w", err), 1) } } - cfg.Manifest = m - - h, err := util.Uint160DecodeStringLE(strings.TrimPrefix(ctx.String("hash"), "0x")) - if err != nil { - return cli.NewExitError(fmt.Errorf("invalid contract hash: %w", err), 1) - } cfg.Hash = h f, err := os.Create(ctx.String("out")) diff --git a/cli/smartcontract/generate_test.go b/cli/smartcontract/generate_test.go index 64629c695..e7f57371e 100644 --- a/cli/smartcontract/generate_test.go +++ b/cli/smartcontract/generate_test.go @@ -66,13 +66,6 @@ func TestGenerate(t *testing.T) { }, ReturnType: smartcontract.BoolType, }, - manifest.Method{ - Name: "emptyName", - Parameters: []manifest.Parameter{ - manifest.NewParameter("", smartcontract.MapType), - }, - ReturnType: smartcontract.AnyType, - }, manifest.Method{ Name: "searchStorage", Parameters: []manifest.Parameter{ @@ -193,11 +186,6 @@ func OtherTypes(ctr interop.Hash160, tx interop.Hash256, sig interop.Signature, return neogointernal.CallWithToken(Hash, "otherTypes", int(contract.All), ctr, tx, sig, data).(bool) } -// EmptyName invokes ` + "`emptyName`" + ` method of contract. -func EmptyName(arg0 map[string]interface{}) interface{} { - return neogointernal.CallWithToken(Hash, "emptyName", int(contract.All), arg0).(interface{}) -} - // SearchStorage invokes ` + "`searchStorage`" + ` method of contract. func SearchStorage(ctx storage.Context) iterator.Iterator { return neogointernal.CallWithToken(Hash, "searchStorage", int(contract.All), ctx).(iterator.Iterator) @@ -288,27 +276,41 @@ func TestGenerate_Errors(t *testing.T) { err := app.Run(append([]string{"", "generate-wrapper"}, args...)) require.True(t, strings.Contains(err.Error(), msg), "got: %v", err) } + t.Run("invalid hash", func(t *testing.T) { + checkError(t, "invalid contract hash", "--hash", "xxx") + }) t.Run("missing manifest argument", func(t *testing.T) { - checkError(t, errNoManifestFile.Error()) + checkError(t, errNoManifestFile.Error(), "--hash", util.Uint160{}.StringLE()) }) t.Run("missing manifest file", func(t *testing.T) { - checkError(t, "can't read contract manifest", "--manifest", "notexists") + checkError(t, "can't read contract manifest", "--manifest", "notexists", "--hash", util.Uint160{}.StringLE()) + }) + t.Run("empty manifest", func(t *testing.T) { + manifestFile := filepath.Join(t.TempDir(), "invalid.json") + require.NoError(t, os.WriteFile(manifestFile, []byte("[]"), os.ModePerm)) + checkError(t, "json: cannot unmarshal array into Go value of type manifest.Manifest", "--manifest", manifestFile, "--hash", util.Uint160{}.StringLE()) }) t.Run("invalid manifest", func(t *testing.T) { manifestFile := filepath.Join(t.TempDir(), "invalid.json") - require.NoError(t, os.WriteFile(manifestFile, []byte("[]"), os.ModePerm)) - checkError(t, "", "--manifest", manifestFile) + m := manifest.NewManifest("MyContract") // no methods + rawManifest, err := json.Marshal(m) + require.NoError(t, err) + require.NoError(t, os.WriteFile(manifestFile, rawManifest, os.ModePerm)) + checkError(t, "ABI: no methods", "--manifest", manifestFile, "--hash", util.Uint160{}.StringLE()) }) manifestFile := filepath.Join(t.TempDir(), "manifest.json") m := manifest.NewManifest("MyContract") + m.ABI.Methods = append(m.ABI.Methods, manifest.Method{ + Name: "method0", + Offset: 0, + ReturnType: smartcontract.AnyType, + Safe: true, + }) rawManifest, err := json.Marshal(m) require.NoError(t, err) require.NoError(t, os.WriteFile(manifestFile, rawManifest, os.ModePerm)) - t.Run("invalid hash", func(t *testing.T) { - checkError(t, "invalid contract hash", "--manifest", manifestFile, "--hash", "xxx") - }) t.Run("missing config", func(t *testing.T) { checkError(t, "can't read config file", "--manifest", manifestFile, "--hash", util.Uint160{}.StringLE(), diff --git a/cli/smartcontract/manifest.go b/cli/smartcontract/manifest.go index 1f4eca2a2..498e66f77 100644 --- a/cli/smartcontract/manifest.go +++ b/cli/smartcontract/manifest.go @@ -11,6 +11,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/smartcontract/manifest" "github.com/nspcc-dev/neo-go/pkg/smartcontract/nef" + "github.com/nspcc-dev/neo-go/pkg/util" "github.com/urfave/cli" ) @@ -29,7 +30,7 @@ func manifestAddGroup(ctx *cli.Context) error { } mPath := ctx.String("manifest") - m, _, err := readManifest(mPath) + m, _, err := readManifest(mPath, util.Uint160{}) if err != nil { return cli.NewExitError(fmt.Errorf("can't read contract manifest: %w", err), 1) } @@ -89,7 +90,10 @@ func readNEFFile(filename string) (*nef.File, []byte, error) { return &nefFile, f, nil } -func readManifest(filename string) (*manifest.Manifest, []byte, error) { +// readManifest unmarshalls manifest got from the provided filename and checks +// it for validness against the provided contract hash. If empty hash is specified +// then no hash-related manifest groups check is performed. +func readManifest(filename string, hash util.Uint160) (*manifest.Manifest, []byte, error) { if len(filename) == 0 { return nil, nil, errNoManifestFile } @@ -104,5 +108,8 @@ func readManifest(filename string) (*manifest.Manifest, []byte, error) { if err != nil { return nil, nil, err } + if err := m.IsValid(hash); err != nil { + return nil, nil, fmt.Errorf("manifest is invalid: %w", err) + } return m, manifestBytes, nil } diff --git a/cli/smartcontract/smart_contract.go b/cli/smartcontract/smart_contract.go index e260306aa..a61222ecb 100644 --- a/cli/smartcontract/smart_contract.go +++ b/cli/smartcontract/smart_contract.go @@ -922,7 +922,7 @@ func contractDeploy(ctx *cli.Context) error { return cli.NewExitError(err, 1) } - m, manifestBytes, err := readManifest(ctx.String("manifest")) + m, manifestBytes, err := readManifest(ctx.String("manifest"), util.Uint160{}) if err != nil { return cli.NewExitError(fmt.Errorf("failed to read manifest file: %w", err), 1) } From fb8a3973f18a12fe26d5f9a07f26df75949f6086 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 22 Aug 2022 13:50:47 +0300 Subject: [PATCH 3/3] smartcontract: remove empty method parameter handling It's prohibited by the manifest validness checker, thus should not be supported by bindings generator. --- pkg/smartcontract/binding/generate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/smartcontract/binding/generate.go b/pkg/smartcontract/binding/generate.go index 5ac2f2e00..b38802a21 100644 --- a/pkg/smartcontract/binding/generate.go +++ b/pkg/smartcontract/binding/generate.go @@ -201,7 +201,7 @@ func templateFromManifest(cfg Config) (contractTmpl, error) { for i := range m.Parameters { name := m.Parameters[i].Name if name == "" { - name = fmt.Sprintf("arg%d", i) + return ctr, fmt.Errorf("manifest ABI method %q/%d: parameter #%d is unnamed", m.Name, len(m.Parameters), i) } var typeStr string