Manifest and script checks

pkg/smartcontract/manifest/manifest.go

@@ -82,10 +82,10 @@ func (m *Manifest) IsValid(hash util.Uint160) error {
 	for _, g := range m.Groups {
 		err = g.IsValid(hash)
 		if err != nil {
-			break
+			return err
 		}
 	}
-	return err
+	return Permissions(m.Permissions).AreValid()
 }
 
 // ToStackItem converts Manifest to stackitem.Item.

pkg/smartcontract/manifest/manifest_test.go

@@ -147,6 +147,17 @@ func TestIsValid(t *testing.T) {
 	})
 	m.ABI.Events = m.ABI.Events[:1]
 
+	m.Permissions = append(m.Permissions, *NewPermission(PermissionHash, util.Uint160{1, 2, 3}))
+	t.Run("valid, with permissions", func(t *testing.T) {
+		require.NoError(t, m.IsValid(contractHash))
+	})
+
+	m.Permissions = append(m.Permissions, *NewPermission(PermissionHash, util.Uint160{1, 2, 3}))
+	t.Run("invalid, with permissions", func(t *testing.T) {
+		require.Error(t, m.IsValid(contractHash))
+	})
+	m.Permissions = m.Permissions[:1]
+
 	t.Run("with groups", func(t *testing.T) {
 		m.Groups = make([]Group, 3)
 		pks := make([]*keys.PrivateKey, 3)

pkg/smartcontract/manifest/parameter.go

@@ -85,14 +85,22 @@ func (p Parameters) AreValid() error {
 	for i := range p {
 		names[i] = p[i].Name
 	}
-	sort.Strings(names)
-	for i := range names {
+	if stringsHaveDups(names) {
+		return errors.New("duplicate parameter name")
+	}
+	return nil
+}
+
+// stringsHaveDups checks given set of strings for duplicates. It modifies the slice given!
+func stringsHaveDups(strings []string) bool {
+	sort.Strings(strings)
+	for i := range strings {
 		if i == 0 {
 			continue
 		}
-		if names[i] == names[i-1] {
-			return errors.New("duplicate parameter name")
+		if strings[i] == strings[i-1] {
+			return true
 		}
 	}
-	return nil
+	return false
 }

pkg/smartcontract/manifest/permission.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"sort"
 
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
 	"github.com/nspcc-dev/neo-go/pkg/util"
@@ -36,6 +37,9 @@ type Permission struct {
 	Methods  WildStrings    `json:"methods"`
 }
 
+// Permissions is just an array of Permission.
+type Permissions []Permission
+
 type permissionAux struct {
 	Contract PermissionDesc `json:"contract"`
 	Methods  WildStrings    `json:"methods"`
@@ -85,6 +89,82 @@ func (d *PermissionDesc) Group() *keys.PublicKey {
 	return d.Value.(*keys.PublicKey)
 }
 
+// IsValid checks if Permission is correct.
+func (p *Permission) IsValid() error {
+	for i := range p.Methods.Value {
+		if p.Methods.Value[i] == "" {
+			return errors.New("empty method name")
+		}
+	}
+	if len(p.Methods.Value) < 2 {
+		return nil
+	}
+	names := make([]string, len(p.Methods.Value))
+	copy(names, p.Methods.Value)
+	if stringsHaveDups(names) {
+		return errors.New("duplicate method names")
+	}
+	return nil
+}
+
+// AreValid checks each Permission and ensures there are no duplicates.
+func (ps Permissions) AreValid() error {
+	for i := range ps {
+		err := ps[i].IsValid()
+		if err != nil {
+			return err
+		}
+	}
+	if len(ps) < 2 {
+		return nil
+	}
+	contracts := make([]PermissionDesc, 0, len(ps))
+	for i := range ps {
+		contracts = append(contracts, ps[i].Contract)
+	}
+	sort.Slice(contracts, func(i, j int) bool {
+		if contracts[i].Type < contracts[j].Type {
+			return true
+		}
+		if contracts[i].Type != contracts[j].Type {
+			return false
+		}
+		switch contracts[i].Type {
+		case PermissionHash:
+			return contracts[i].Hash().Less(contracts[j].Hash())
+		case PermissionGroup:
+			return contracts[i].Group().Cmp(contracts[j].Group()) < 0
+		}
+		return false
+	})
+	for i := range contracts {
+		if i == 0 {
+			continue
+		}
+		j := i - 1
+		if contracts[i].Type != contracts[j].Type {
+			continue
+		}
+		var bad bool
+		switch contracts[i].Type {
+		case PermissionWildcard:
+			bad = true
+		case PermissionHash:
+			if contracts[i].Hash() == contracts[j].Hash() {
+				bad = true
+			}
+		case PermissionGroup:
+			if contracts[i].Group().Cmp(contracts[j].Group()) == 0 {
+				bad = true
+			}
+		}
+		if bad {
+			return errors.New("duplicate contracts")
+		}
+	}
+	return nil
+}
+
 // IsAllowed checks if method is allowed to be executed.
 func (p *Permission) IsAllowed(hash util.Uint160, m *Manifest, method string) bool {
 	switch p.Contract.Type {

pkg/smartcontract/manifest/permission_test.go

@@ -21,6 +21,62 @@ func TestNewPermission(t *testing.T) {
 	require.Panics(t, func() { NewPermission(PermissionGroup, util.Uint160{}) })
 }
 
+func TestPermissionIsValid(t *testing.T) {
+	p := Permission{}
+	require.NoError(t, p.IsValid())
+
+	p.Methods.Add("")
+	require.Error(t, p.IsValid())
+
+	p.Methods.Value = nil
+	p.Methods.Add("qwerty")
+	require.NoError(t, p.IsValid())
+
+	p.Methods.Add("poiuyt")
+	require.NoError(t, p.IsValid())
+
+	p.Methods.Add("qwerty")
+	require.Error(t, p.IsValid())
+}
+
+func TestPermissionsAreValid(t *testing.T) {
+	p := Permissions{}
+	require.NoError(t, p.AreValid())
+
+	p = append(p, Permission{Methods: WildStrings{Value: []string{""}}})
+	require.Error(t, p.AreValid())
+
+	p = p[:0]
+	p = append(p, *NewPermission(PermissionHash, util.Uint160{1, 2, 3}))
+	require.NoError(t, p.AreValid())
+
+	priv0, err := keys.NewPrivateKey()
+	require.NoError(t, err)
+	priv1, err := keys.NewPrivateKey()
+	require.NoError(t, err)
+
+	p = append(p, *NewPermission(PermissionGroup, priv0.PublicKey()))
+	require.NoError(t, p.AreValid())
+
+	p = append(p, *NewPermission(PermissionGroup, priv1.PublicKey()))
+	require.NoError(t, p.AreValid())
+
+	p = append(p, *NewPermission(PermissionWildcard))
+	require.NoError(t, p.AreValid())
+
+	p = append(p, *NewPermission(PermissionHash, util.Uint160{3, 2, 1}))
+	require.NoError(t, p.AreValid())
+
+	p = append(p, *NewPermission(PermissionWildcard))
+	require.Error(t, p.AreValid())
+
+	p = append(p[:len(p)-1], *NewPermission(PermissionHash, util.Uint160{1, 2, 3}))
+	require.Error(t, p.AreValid())
+
+	p = append(p[:len(p)-1], *NewPermission(PermissionGroup, priv0.PublicKey()))
+	require.Error(t, p.AreValid())
+}
+
 func TestPermission_MarshalJSON(t *testing.T) {
 	t.Run("wildcard", func(t *testing.T) {
 		expected := NewPermission(PermissionWildcard)