From 9f69522ff5e0ed934cae8ef27a3e418f96ac7436 Mon Sep 17 00:00:00 2001
From: Anna Shaleva <shaleva.ann@nspcc.ru>
Date: Wed, 12 Jul 2023 08:33:26 +0300
Subject: [PATCH] neorpc: adjust SignerWithWitness scopes parsing

Ensure that Scopes can be properly parsed not only from the string
representation, but also from a single byte. transaction.Signer
is not affected (checked against the C# implementation), only
RPC-related signer scopes are allowed to be unmarshalled from byte.

Close #3059.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
---
 pkg/core/transaction/witness_scope.go      |  14 +++
 pkg/core/transaction/witness_scope_test.go |  62 +++++++++++++
 pkg/neorpc/types.go                        |  32 ++++++-
 pkg/services/rpcsrv/params/param_test.go   | 102 ++++++++++++++++-----
 4 files changed, 185 insertions(+), 25 deletions(-)

diff --git a/pkg/core/transaction/witness_scope.go b/pkg/core/transaction/witness_scope.go
index aa67eb33a..c1cf2b7d8 100644
--- a/pkg/core/transaction/witness_scope.go
+++ b/pkg/core/transaction/witness_scope.go
@@ -3,6 +3,7 @@ package transaction
 //go:generate stringer -type=WitnessScope -linecomment -output=witness_scope_string.go
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strings"
 )
@@ -29,6 +30,19 @@ const (
 	Global WitnessScope = 0x80
 )
 
+// ScopesFromByte converts byte to a set of WitnessScopes and performs validity
+// check.
+func ScopesFromByte(b byte) (WitnessScope, error) {
+	var res = WitnessScope(b)
+	if (res&Global != 0) && (res&(None|CalledByEntry|CustomContracts|CustomGroups|Rules) != 0) {
+		return 0, errors.New("Global scope can not be combined with other scopes")
+	}
+	if res&^(None|CalledByEntry|CustomContracts|CustomGroups|Rules|Global) != 0 {
+		return 0, fmt.Errorf("invalid scope %d", res)
+	}
+	return res, nil
+}
+
 // ScopesFromString converts string of comma-separated scopes to a set of scopes
 // (case-sensitive). String can combine several scopes, e.g. be any of: 'Global',
 // 'CalledByEntry,CustomGroups' etc. In case of an empty string an error will be
diff --git a/pkg/core/transaction/witness_scope_test.go b/pkg/core/transaction/witness_scope_test.go
index 42a672e68..aea706a08 100644
--- a/pkg/core/transaction/witness_scope_test.go
+++ b/pkg/core/transaction/witness_scope_test.go
@@ -1,6 +1,7 @@
 package transaction
 
 import (
+	"strconv"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -51,3 +52,64 @@ func TestScopesFromString(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, CalledByEntry|CustomGroups|CustomContracts, s)
 }
+
+func TestScopesFromByte(t *testing.T) {
+	testCases := []struct {
+		in         byte
+		expected   WitnessScope
+		shouldFail bool
+	}{
+		{
+			in:       0,
+			expected: None,
+		},
+		{
+			in:       1,
+			expected: CalledByEntry,
+		},
+		{
+			in:       16,
+			expected: CustomContracts,
+		},
+		{
+			in:       32,
+			expected: CustomGroups,
+		},
+		{
+			in:       64,
+			expected: Rules,
+		},
+		{
+			in:       128,
+			expected: Global,
+		},
+		{
+			in:       17,
+			expected: CalledByEntry | CustomContracts,
+		},
+		{
+			in:       48,
+			expected: CustomContracts | CustomGroups,
+		},
+		{
+			in:         128 + 1, // Global can't be combined with others.
+			shouldFail: true,
+		},
+		{
+			in:         2, // No such scope.
+			shouldFail: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(strconv.Itoa(int(tc.in)), func(t *testing.T) {
+			actual, err := ScopesFromByte(tc.in)
+			if tc.shouldFail {
+				require.Error(t, err, tc.in)
+			} else {
+				require.NoError(t, err, tc.in)
+				require.Equal(t, tc.expected, actual, tc.in)
+			}
+		})
+	}
+}
diff --git a/pkg/neorpc/types.go b/pkg/neorpc/types.go
index 40db91878..bbe7b690a 100644
--- a/pkg/neorpc/types.go
+++ b/pkg/neorpc/types.go
@@ -82,7 +82,7 @@ type (
 // DisallowUnknownFields JSON marshaller setting.
 type signerWithWitnessAux struct {
 	Account            string                    `json:"account"`
-	Scopes             transaction.WitnessScope  `json:"scopes"`
+	Scopes             json.RawMessage           `json:"scopes"`
 	AllowedContracts   []util.Uint160            `json:"allowedcontracts,omitempty"`
 	AllowedGroups      []*keys.PublicKey         `json:"allowedgroups,omitempty"`
 	Rules              []transaction.WitnessRule `json:"rules,omitempty"`
@@ -92,9 +92,13 @@ type signerWithWitnessAux struct {
 
 // MarshalJSON implements the json.Marshaler interface.
 func (s *SignerWithWitness) MarshalJSON() ([]byte, error) {
+	sc, err := s.Scopes.MarshalJSON()
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal scopes: %w", err)
+	}
 	signer := &signerWithWitnessAux{
 		Account:            s.Account.StringLE(),
-		Scopes:             s.Scopes,
+		Scopes:             sc,
 		AllowedContracts:   s.AllowedContracts,
 		AllowedGroups:      s.AllowedGroups,
 		Rules:              s.Rules,
@@ -118,9 +122,31 @@ func (s *SignerWithWitness) UnmarshalJSON(data []byte) error {
 	if err != nil {
 		return fmt.Errorf("not a signer: %w", err)
 	}
+	var (
+		jStr   string
+		jByte  byte
+		scopes transaction.WitnessScope
+	)
+	if len(aux.Scopes) != 0 {
+		if err := json.Unmarshal(aux.Scopes, &jStr); err == nil {
+			scopes, err = transaction.ScopesFromString(jStr)
+			if err != nil {
+				return fmt.Errorf("failed to retrieve scopes from string: %w", err)
+			}
+		} else {
+			err := json.Unmarshal(aux.Scopes, &jByte)
+			if err != nil {
+				return fmt.Errorf("failed to unmarshal scopes from byte: %w", err)
+			}
+			scopes, err = transaction.ScopesFromByte(jByte)
+			if err != nil {
+				return fmt.Errorf("failed to retrieve scopes from byte: %w", err)
+			}
+		}
+	}
 	s.Signer = transaction.Signer{
 		Account:          acc,
-		Scopes:           aux.Scopes,
+		Scopes:           scopes,
 		AllowedContracts: aux.AllowedContracts,
 		AllowedGroups:    aux.AllowedGroups,
 		Rules:            aux.Rules,
diff --git a/pkg/services/rpcsrv/params/param_test.go b/pkg/services/rpcsrv/params/param_test.go
index af7c0bbc1..dfe0d2f3f 100644
--- a/pkg/services/rpcsrv/params/param_test.go
+++ b/pkg/services/rpcsrv/params/param_test.go
@@ -243,38 +243,96 @@ func TestGetWitness(t *testing.T) {
 	require.NoError(t, err)
 
 	testCases := []struct {
-		raw      string
-		expected neorpc.SignerWithWitness
+		raw        string
+		expected   neorpc.SignerWithWitness
+		shouldFail bool
 	}{
-		{`{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569"}`, neorpc.SignerWithWitness{
-			Signer: transaction.Signer{
-				Account: accountHash,
-				Scopes:  transaction.None,
-			}},
+		{
+			raw: `{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569"}`,
+			expected: neorpc.SignerWithWitness{
+				Signer: transaction.Signer{
+					Account: accountHash,
+					Scopes:  transaction.None,
+				},
+			},
 		},
-		{`{"account": "NYxb4fSZVKAz8YsgaPK2WkT3KcAE9b3Vag", "scopes": "Global"}`, neorpc.SignerWithWitness{
-			Signer: transaction.Signer{
-				Account: addrHash,
-				Scopes:  transaction.Global,
-			}},
+		{
+			raw: `{"account": "NYxb4fSZVKAz8YsgaPK2WkT3KcAE9b3Vag", "scopes": "Global"}`,
+			expected: neorpc.SignerWithWitness{
+				Signer: transaction.Signer{
+					Account: addrHash,
+					Scopes:  transaction.Global,
+				},
+			},
 		},
-		{`{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": "Global"}`, neorpc.SignerWithWitness{
-			Signer: transaction.Signer{
-				Account: accountHash,
-				Scopes:  transaction.Global,
-			}},
+		{
+			raw: `{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": "Global"}`,
+			expected: neorpc.SignerWithWitness{
+				Signer: transaction.Signer{
+					Account: accountHash,
+					Scopes:  transaction.Global,
+				},
+			},
+		},
+		{
+			raw: `{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": 128}`,
+			expected: neorpc.SignerWithWitness{
+				Signer: transaction.Signer{
+					Account: accountHash,
+					Scopes:  transaction.Global,
+				},
+			},
+		},
+		{
+			raw: `{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": 0}`,
+			expected: neorpc.SignerWithWitness{
+				Signer: transaction.Signer{
+					Account: accountHash,
+					Scopes:  transaction.None,
+				},
+			},
+		},
+		{
+			raw: `{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": 1}`,
+			expected: neorpc.SignerWithWitness{
+				Signer: transaction.Signer{
+					Account: accountHash,
+					Scopes:  transaction.CalledByEntry,
+				},
+			},
+		},
+		{
+			raw: `{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": 17}`,
+			expected: neorpc.SignerWithWitness{
+				Signer: transaction.Signer{
+					Account: accountHash,
+					Scopes:  transaction.CalledByEntry | transaction.CustomContracts,
+				},
+			},
+		},
+		{
+			raw:        `{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": 178}`,
+			shouldFail: true,
+		},
+		{
+			raw:        `{"account": "0xcadb3dc2faa3ef14a13b619c9a43124755aa2569", "scopes": 2}`,
+			shouldFail: true,
 		},
 	}
 
 	for _, tc := range testCases {
 		p := Param{RawMessage: json.RawMessage(tc.raw)}
 		actual, err := p.GetSignerWithWitness()
-		require.NoError(t, err)
-		require.Equal(t, tc.expected, actual)
+		if tc.shouldFail {
+			require.Error(t, err, tc.raw)
+		} else {
+			require.NoError(t, err, tc.raw)
+			require.Equal(t, tc.expected, actual)
 
-		actual, err = p.GetSignerWithWitness() // valid second invocation.
-		require.NoError(t, err)
-		require.Equal(t, tc.expected, actual)
+			actual, err = p.GetSignerWithWitness() // valid second invocation.
+			require.NoError(t, err, tc.raw)
+			require.Equal(t, tc.expected, actual)
+		}
 	}
 }