Merge pull request #2656 from nspcc-dev/imprv-sc-generator

cli: add manifest validness check where it's got from the user input

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"))

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))
+		m := manifest.NewManifest("MyContract") // no methods
-		checkError(t, "", "--manifest", manifestFile)
+		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(),

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
 }

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)
 	}

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

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 {

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 {

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 {