Merge pull request #3523 from nspcc-dev/fix-null-handling-in-manifest

Fix null handling in manifest

cli/smartcontract/testdata/verify.manifest.json

@@ -1 +1 @@
-{"name":"verify","abi":{"methods":[{"name":"verify","offset":0,"parameters":[],"returntype":"Boolean","safe":false},{"name":"onNEP17Payment","offset":5,"parameters":[{"name":"from","type":"ByteArray"},{"name":"amount","type":"Integer"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false}],"events":[{"name":"Hello world!","parameters":[{"name":"args","type":"Array"}]}]},"groups":[],"permissions":[{"contract":"*","methods":"*"}],"supportedstandards":[],"trusts":[],"extra":null}
+{"name":"verify","abi":{"methods":[{"name":"verify","offset":0,"parameters":[],"returntype":"Boolean","safe":false},{"name":"onNEP17Payment","offset":5,"parameters":[{"name":"from","type":"ByteArray"},{"name":"amount","type":"Integer"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false}],"events":[{"name":"Hello world!","parameters":[{"name":"args","type":"Array"}]}]},"groups":[],"permissions":[{"contract":"*","methods":"*"}],"supportedstandards":[],"trusts":[],"extra":null,"features":{}}

cli/smartcontract/testdata/verifyrpc/verify.manifest.json

@@ -4,6 +4,7 @@
    "supportedstandards" : [],
    "name" : "verify",
    "trusts" : [],
+   "features": {},
    "permissions" : [
       {
          "methods" : "*",

pkg/core/interop/runtime/ext_test.go

@@ -713,6 +713,9 @@ func TestSystemRuntimeNotify_HFBasilisk(t *testing.T) {
 
 	m := &manifest.Manifest{
 		Name:     "ctr",
+		Features: json.RawMessage("{}"),
+		Groups:   []manifest.Group{},
+		Trusts:   manifest.WildPermissionDescs{Wildcard: true},
 		ABI: manifest.ABI{
 			Methods: []manifest.Method{
 				{

pkg/core/native/management_neotest_test.go

@@ -53,6 +53,9 @@ func TestManagement_DeployUpdate_HFBasilisk(t *testing.T) {
 
 	m := &manifest.Manifest{
 		Name:     "ctr",
+		Features: json.RawMessage("{}"),
+		Groups:   []manifest.Group{},
+		Trusts:   manifest.WildPermissionDescs{Wildcard: true},
 		ABI: manifest.ABI{
 			Methods: []manifest.Method{
 				{
@@ -88,6 +91,9 @@ func TestManagement_CallInTheSameBlock(t *testing.T) {
 
 	m := &manifest.Manifest{
 		Name:     "ctr",
+		Features: json.RawMessage("{}"),
+		Groups:   []manifest.Group{},
+		Trusts:   manifest.WildPermissionDescs{Wildcard: true},
 		ABI: manifest.ABI{
 			Methods: []manifest.Method{
 				{

pkg/core/state/contract_test.go

@@ -58,6 +58,7 @@ func TestContractStateToFromSI(t *testing.T) {
 
 		t.Run("preserve wildcard trusts", func(t *testing.T) {
 			contract.Manifest.Trusts.Value = nil
+			contract.Manifest.Trusts.Wildcard = true
 			require.True(t, contract.Manifest.Trusts.IsWildcard())
 			actual := new(Contract)
 			item, err := contract.ToStackItem()

pkg/smartcontract/manifest/container.go

@@ -17,6 +17,7 @@ type WildStrings struct {
 // WildPermissionDescs represents a PermissionDescriptor set which can be a wildcard.
 type WildPermissionDescs struct {
 	Value    []PermissionDesc
+	Wildcard bool
 }
 
 // Contains checks if v is in the container.
@@ -49,13 +50,16 @@ func (c *WildPermissionDescs) Contains(v PermissionDesc) bool {
 func (c *WildStrings) IsWildcard() bool { return c.Value == nil }
 
 // IsWildcard returns true iff the container is a wildcard.
-func (c *WildPermissionDescs) IsWildcard() bool { return c.Value == nil }
+func (c *WildPermissionDescs) IsWildcard() bool { return c.Wildcard }
 
 // Restrict transforms the container into an empty one.
 func (c *WildStrings) Restrict() { c.Value = []string{} }
 
 // Restrict transforms the container into an empty one.
-func (c *WildPermissionDescs) Restrict() { c.Value = []PermissionDesc{} }
+func (c *WildPermissionDescs) Restrict() {
+	c.Value = []PermissionDesc{}
+	c.Wildcard = false
+}
 
 // Add adds v to the container.
 func (c *WildStrings) Add(v string) { c.Value = append(c.Value, v) }
@@ -93,7 +97,8 @@ func (c *WildStrings) UnmarshalJSON(data []byte) error {
 
 // UnmarshalJSON implements the json.Unmarshaler interface.
 func (c *WildPermissionDescs) UnmarshalJSON(data []byte) error {
-	if !bytes.Equal(data, []byte(`"*"`)) {
+	c.Wildcard = bytes.Equal(data, []byte(`"*"`))
+	if !c.Wildcard {
 		us := []PermissionDesc{}
 		if err := json.Unmarshal(data, &us); err != nil {
 			return err

pkg/smartcontract/manifest/container_test.go

@@ -24,6 +24,9 @@ func TestContainer_Restrict(t *testing.T) {
 	t.Run("PermissionDesc", func(t *testing.T) {
 		check := func(t *testing.T, u PermissionDesc) {
 			c := new(WildPermissionDescs)
+			require.False(t, c.IsWildcard())
+			require.False(t, c.Contains(u))
+			c.Wildcard = true
 			require.True(t, c.IsWildcard())
 			require.True(t, c.Contains(u))
 			c.Restrict()

pkg/smartcontract/manifest/group.go

@@ -40,6 +40,9 @@ func (g *Group) IsValid(h util.Uint160) error {
 // AreValid checks for groups correctness and uniqueness.
 // If the contract hash is empty, then hash-related checks are omitted.
 func (g Groups) AreValid(h util.Uint160) error {
+	if g == nil {
+		return errors.New("null groups")
+	}
 	if !h.Equals(util.Uint160{}) {
 		for i := range g {
 			err := g[i].IsValid(h)

pkg/smartcontract/manifest/group_test.go

@@ -19,6 +19,10 @@ func TestGroupJSONInOut(t *testing.T) {
 }
 
 func TestGroupsAreValid(t *testing.T) {
+	var gps Groups
+
+	require.Error(t, gps.AreValid(util.Uint160{})) // null
+
 	h := util.Uint160{42, 42, 42}
 	priv, err := keys.NewPrivateKey()
 	require.NoError(t, err)
@@ -29,7 +33,8 @@ func TestGroupsAreValid(t *testing.T) {
 	gcorrect := Group{pub, priv.Sign(h.BytesBE())}
 	gcorrect2 := Group{pub2, priv2.Sign(h.BytesBE())}
 	gincorrect := Group{pub, priv.Sign(h.BytesLE())}
-	gps := Groups{gcorrect}
+
+	gps = Groups{gcorrect}
 	require.NoError(t, gps.AreValid(h))
 
 	gps = Groups{gincorrect}

pkg/smartcontract/manifest/manifest.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"strings"
 
 	ojson "github.com/nspcc-dev/go-ordered-json"
 	"github.com/nspcc-dev/neo-go/pkg/util"
@@ -24,6 +25,8 @@ const (
 	NEP11Payable = "NEP-11-Payable"
 	// NEP17Payable represents the name of contract interface which can receive NEP-17 tokens.
 	NEP17Payable = "NEP-17-Payable"
+
+	emptyFeatures = "{}"
 )
 
 // Manifest represens contract metadata.
@@ -53,7 +56,7 @@ func NewManifest(name string) *Manifest {
 			Methods: []Method{},
 			Events:  []Event{},
 		},
-		Features:           json.RawMessage("{}"),
+		Features:           json.RawMessage(emptyFeatures),
 		Groups:             []Group{},
 		Permissions:        []Permission{},
 		SupportedStandards: []string{},
@@ -107,10 +110,23 @@ func (m *Manifest) IsValid(hash util.Uint160, checkSize bool) error {
 	if err != nil {
 		return fmt.Errorf("ABI: %w", err)
 	}
+
+	if strings.Map(func(c rune) rune {
+		switch c {
+		case ' ', '\n', '\t', '\r': // Strip all JSON whitespace.
+			return -1
+		}
+		return c
+	}, string(m.Features)) != emptyFeatures { // empty struct should be left.
+		return errors.New("invalid features")
+	}
 	err = Groups(m.Groups).AreValid(hash)
 	if err != nil {
 		return err
 	}
+	if m.Trusts.Value == nil && !m.Trusts.Wildcard {
+		return errors.New("invalid (null?) trusts")
+	}
 	if len(m.Trusts.Value) > 1 {
 		hashes := make([]PermissionDesc, len(m.Trusts.Value))
 		copy(hashes, m.Trusts.Value)
@@ -235,7 +251,7 @@ func (m *Manifest) FromStackItem(item stackitem.Item) error {
 	if str[2].Type() != stackitem.MapT || str[2].(*stackitem.Map).Len() != 0 {
 		return errors.New("invalid Features stackitem")
 	}
-	m.Features = json.RawMessage("{}")
+	m.Features = json.RawMessage(emptyFeatures)
 	if str[3].Type() != stackitem.ArrayT {
 		return errors.New("invalid SupportedStandards stackitem type")
 	}
@@ -265,7 +281,7 @@ func (m *Manifest) FromStackItem(item stackitem.Item) error {
 		m.Permissions[i] = *p
 	}
 	if _, ok := str[6].(stackitem.Null); ok {
-		m.Trusts = WildPermissionDescs{Value: nil} // wildcard by default
+		m.Trusts = WildPermissionDescs{Value: nil, Wildcard: true} // wildcard by default
 	} else {
 		if str[6].Type() != stackitem.ArrayT {
 			return errors.New("invalid Trusts stackitem type")

pkg/smartcontract/manifest/manifest_test.go

@@ -144,6 +144,24 @@ func TestIsValid(t *testing.T) {
 		require.NoError(t, m.IsValid(contractHash, true))
 	})
 
+	t.Run("invalid, no features", func(t *testing.T) {
+		m.Features = nil
+		require.Error(t, m.IsValid(contractHash, true))
+	})
+	m.Features = json.RawMessage(emptyFeatures)
+
+	t.Run("invalid, bad features", func(t *testing.T) {
+		m.Features = json.RawMessage(`{ "v" : true}`)
+		require.Error(t, m.IsValid(contractHash, true))
+	})
+	m.Features = json.RawMessage(emptyFeatures)
+
+	t.Run("valid, features with spaces", func(t *testing.T) {
+		m.Features = json.RawMessage("{ \t\n\r }")
+		require.NoError(t, m.IsValid(contractHash, true))
+	})
+	m.Features = json.RawMessage(emptyFeatures)
+
 	m.ABI.Events = append(m.ABI.Events, Event{
 		Name:       "itHappened",
 		Parameters: []Parameter{},
@@ -194,6 +212,13 @@ func TestIsValid(t *testing.T) {
 	})
 	m.SupportedStandards = m.SupportedStandards[:1]
 
+	t.Run("invalid, no trusts", func(t *testing.T) {
+		m.Trusts.Value = nil
+		m.Trusts.Wildcard = false
+		require.Error(t, m.IsValid(contractHash, true))
+	})
+	m.Trusts.Restrict()
+
 	d := PermissionDesc{Type: PermissionHash, Value: util.Uint160{1, 2, 3}}
 	m.Trusts.Add(d)
 	t.Run("valid, with trust", func(t *testing.T) {