Merge pull request #1751 from nspcc-dev/microfixes

core: microfixes

pkg/core/blockchain.go

@@ -1390,7 +1390,6 @@ var (
 	ErrTxTooBig          = errors.New("too big transaction")
 	ErrMemPoolConflict   = errors.New("invalid transaction due to conflicts with the memory pool")
 	ErrInvalidScript     = errors.New("invalid script")
-	ErrTxInvalidWitnessNum = errors.New("number of signers doesn't match witnesses")
 	ErrInvalidAttribute  = errors.New("invalid attribute")
 )
 
@@ -1767,7 +1766,7 @@ func (bc *Blockchain) initVerificationVM(ic *interop.Context, hash util.Uint160,
 		if err != nil {
 			return fmt.Errorf("%w: %v", ErrInvalidVerification, err)
 		}
-		v.LoadScriptWithFlags(witness.VerificationScript, callflag.ReadStates)
+		v.LoadScriptWithFlags(witness.VerificationScript, callflag.ReadOnly)
 	} else {
 		cs, err := ic.GetContract(hash)
 		if err != nil {
@@ -1778,7 +1777,7 @@ func (bc *Blockchain) initVerificationVM(ic *interop.Context, hash util.Uint160,
 			return ErrInvalidVerificationContract
 		}
 		initMD := cs.Manifest.ABI.GetMethod(manifest.MethodInit, 0)
-		v.LoadScriptWithHash(cs.NEF.Script, hash, callflag.ReadStates)
+		v.LoadScriptWithHash(cs.NEF.Script, hash, callflag.ReadOnly)
 		v.Context().NEF = &cs.NEF
 		v.Jump(v.Context(), md.Offset)
 
@@ -1856,9 +1855,6 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa
 // not yet added into any block.
 // Golang implementation of VerifyWitnesses method in C# (https://github.com/neo-project/neo/blob/master/neo/SmartContract/Helper.cs#L87).
 func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *block.Block, isPartialTx bool) error {
-	if len(t.Signers) != len(t.Scripts) {
-		return fmt.Errorf("%w: %d vs %d", ErrTxInvalidWitnessNum, len(t.Signers), len(t.Scripts))
-	}
 	interopCtx := bc.newInteropContext(trigger.Verification, bc.dao, block, t)
 	gasLimit := t.NetworkFee - int64(t.Size())*bc.FeePerByte()
 	if bc.P2PSigExtensionsEnabled() {

pkg/core/blockchain_test.go

@@ -2,8 +2,10 @@ package core
 
 import (
 	"errors"
+	"fmt"
 	"math/big"
 	"math/rand"
+	"strings"
 	"testing"
 	"time"
 
@@ -439,10 +441,6 @@ func TestVerifyTx(t *testing.T) {
 		err := bc.PoolTx(tx2)
 		require.True(t, errors.Is(err, ErrMemPoolConflict))
 	})
-	t.Run("NotEnoughWitnesses", func(t *testing.T) {
-		tx := bc.newTestTx(h, testScript)
-		checkErr(t, ErrTxInvalidWitnessNum, tx)
-	})
 	t.Run("InvalidWitnessHash", func(t *testing.T) {
 		tx := bc.newTestTx(h, testScript)
 		require.NoError(t, accs[0].SignTx(tx))
@@ -1199,7 +1197,6 @@ func TestIsTxStillRelevant(t *testing.T) {
 		require.NoError(t, bc.AddBlock(bc.newBlock()))
 		require.True(t, bc.IsTxStillRelevant(tx3, nil, false))
 	})
-	/* // neo-project/neo#2289
 	t.Run("contract witness check fails", func(t *testing.T) {
 		src := fmt.Sprintf(`package verify
 		import (
@@ -1230,7 +1227,6 @@ func TestIsTxStillRelevant(t *testing.T) {
 		require.NoError(t, bc.AddBlock(bc.newBlock()))
 		require.False(t, bc.IsTxStillRelevant(tx, mp, false))
 	})
-	*/
 }
 
 func TestMemPoolRemoval(t *testing.T) {

pkg/core/interop_neo_test.go

@@ -280,8 +280,8 @@ func createVMAndContractState(t *testing.T) (*vm.VM, *state.Contract, *interop.C
 func createVMAndTX(t *testing.T) (*vm.VM, *transaction.Transaction, *interop.Context, *Blockchain) {
 	script := []byte{byte(opcode.PUSH1), byte(opcode.RET)}
 	tx := transaction.New(netmode.UnitTestNet, script, 0)
-
 	tx.Signers = []transaction.Signer{{Account: util.Uint160{1, 2, 3, 4}}}
+	tx.Scripts = []transaction.Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}}
 	chain := newTestChain(t)
 	d := dao.NewSimple(storage.NewMemoryStore(), netmode.UnitTestNet, chain.config.StateRootInHeader)
 	context := chain.newInteropContext(trigger.Application, d, nil, tx)

pkg/core/native/name_service.go

@@ -75,8 +75,8 @@ var (
 	// Lookahead is not supported by Go, but it is simple `(?=.{3,255}$)`,
 	// so we check name length explicitly.
 	nameRegex = regexp.MustCompile("^([a-z0-9]{1,62}\\.)+[a-z][a-z0-9]{0,15}$")
-	ipv4Regex = regexp.MustCompile("^(2(5[0-5]|[0-4]\\d))|1?\\d{1,2}(\\.((2(5[0-5]|[0-4]\\d))|1?\\d{1,2})){3}$")
+	ipv4Regex = regexp.MustCompile("^(?:(?:25[0-5]|2[0-4][0-9]|1[0-9]{2}|[1-9][0-9]|[0-9])\\.){3}(?:25[0-5]|2[0-4][0-9]|1[0-9]{2}|[1-9][0-9]|[0-9])$")
-	ipv6Regex = regexp.MustCompile("^([a-f0-9A-F]{1,4}:){7}[a-f0-9A-F]{1,4}$")
+	ipv6Regex = regexp.MustCompile("(?:^)(([0-9a-f]{1,4}:){7,7}[0-9a-f]{1,4}|([0-9a-f]{1,4}:){1,7}:|([0-9a-f]{1,4}:){1,6}:[0-9a-f]{1,4}|([0-9a-f]{1,4}:){1,5}(:[0-9a-f]{1,4}){1,2}|([0-9a-f]{1,4}:){1,4}(:[0-9a-f]{1,4}){1,3}|([0-9a-f]{1,4}:){1,3}(:[0-9a-f]{1,4}){1,4}|([0-9a-f]{1,4}:){1,2}(:[0-9a-f]{1,4}){1,5}|[0-9a-f]{1,4}:((:[0-9a-f]{1,4}){1,6})|:((:[0-9a-f]{1,4}){1,7}|:))$")
 	rootRegex = regexp.MustCompile("^[a-z][a-z0-9]{0,15}$")
 )
 
@@ -429,7 +429,7 @@ func (n *NameService) setRecord(ic *interop.Context, args []stackitem.Item) stac
 	name := toName(args[0])
 	rt := toRecordType(args[1])
 	data := toString(args[2])
-	n.checkName(rt, data)
+	checkName(rt, data)
 
 	domain := toDomain(name)
 	token, _, err := n.tokenState(ic.DAO, []byte(domain))
@@ -447,7 +447,7 @@ func (n *NameService) setRecord(ic *interop.Context, args []stackitem.Item) stac
 	return stackitem.Null{}
 }
 
-func (n *NameService) checkName(rt RecordType, name string) {
+func checkName(rt RecordType, name string) {
 	var valid bool
 	switch rt {
 	case RecordTypeA:
@@ -604,17 +604,17 @@ func (s *nameState) FromStackItem(item stackitem.Item) error {
 		return err
 	}
 	elems := item.Value().([]stackitem.Item)
-	if len(elems) < 5 {
+	if len(elems) < 4 {
 		return errors.New("invalid stack item")
 	}
-	bi, err := elems[3].TryInteger()
+	bi, err := elems[2].TryInteger()
 	if err != nil || !bi.IsUint64() {
 		return errors.New("invalid stack item")
 	}
 
-	_, isNull := elems[4].(stackitem.Null)
+	_, isNull := elems[3].(stackitem.Null)
 	if !isNull {
-		bs, err := elems[4].TryBytes()
+		bs, err := elems[3].TryBytes()
 		if err != nil {
 			return err
 		}

pkg/core/native/name_service_test.go

@@ -30,3 +30,59 @@ func TestParseDomain(t *testing.T) {
 	_, ok := domainFromString("nodots")
 	require.False(t, ok)
 }
+
+func TestNameService_CheckName(t *testing.T) {
+	// tests are got from the C# implementation
+	testCases := []struct {
+		Type       RecordType
+		Name       string
+		ShouldFail bool
+	}{
+		{Type: RecordTypeA, Name: "0.0.0.0"},
+		{Type: RecordTypeA, Name: "10.10.10.10"},
+		{Type: RecordTypeA, Name: "255.255.255.255"},
+		{Type: RecordTypeA, Name: "192.168.1.1"},
+		{Type: RecordTypeA, Name: "1a", ShouldFail: true},
+		{Type: RecordTypeA, Name: "256.0.0.0", ShouldFail: true},
+		{Type: RecordTypeA, Name: "01.01.01.01", ShouldFail: true},
+		{Type: RecordTypeA, Name: "00.0.0.0", ShouldFail: true},
+		{Type: RecordTypeA, Name: "0.0.0.-1", ShouldFail: true},
+		{Type: RecordTypeA, Name: "0.0.0.0.1", ShouldFail: true},
+		{Type: RecordTypeA, Name: "11111111.11111111.11111111.11111111", ShouldFail: true},
+		{Type: RecordTypeA, Name: "11111111.11111111.11111111.11111111", ShouldFail: true},
+		{Type: RecordTypeA, Name: "ff.ff.ff.ff", ShouldFail: true},
+		{Type: RecordTypeA, Name: "0.0.256", ShouldFail: true},
+		{Type: RecordTypeA, Name: "0.0.0", ShouldFail: true},
+		{Type: RecordTypeA, Name: "0.257", ShouldFail: true},
+		{Type: RecordTypeA, Name: "1.1", ShouldFail: true},
+		{Type: RecordTypeA, Name: "257", ShouldFail: true},
+		{Type: RecordTypeA, Name: "1", ShouldFail: true},
+		{Type: RecordTypeAAAA, Name: "2001:db8::8:800:200c:417a"},
+		{Type: RecordTypeAAAA, Name: "ff01::101"},
+		{Type: RecordTypeAAAA, Name: "::1"},
+		{Type: RecordTypeAAAA, Name: "::"},
+		{Type: RecordTypeAAAA, Name: "2001:db8:0:0:8:800:200c:417a"},
+		{Type: RecordTypeAAAA, Name: "ff01:0:0:0:0:0:0:101"},
+		{Type: RecordTypeAAAA, Name: "0:0:0:0:0:0:0:1"},
+		{Type: RecordTypeAAAA, Name: "0:0:0:0:0:0:0:0"},
+		{Type: RecordTypeAAAA, Name: "2001:DB8::8:800:200C:417A", ShouldFail: true},
+		{Type: RecordTypeAAAA, Name: "FF01::101", ShouldFail: true},
+		{Type: RecordTypeAAAA, Name: "fF01::101", ShouldFail: true},
+		{Type: RecordTypeAAAA, Name: "2001:DB8:0:0:8:800:200C:417A", ShouldFail: true},
+		{Type: RecordTypeAAAA, Name: "FF01:0:0:0:0:0:0:101", ShouldFail: true},
+		{Type: RecordTypeAAAA, Name: "::ffff:1.01.1.01", ShouldFail: true},
+		{Type: RecordTypeAAAA, Name: "2001:DB8:0:0:8:800:200C:4Z", ShouldFail: true},
+		{Type: RecordTypeAAAA, Name: "::13.1.68.3", ShouldFail: true},
+	}
+	for _, testCase := range testCases {
+		if testCase.ShouldFail {
+			require.Panics(t, func() {
+				checkName(testCase.Type, testCase.Name)
+			})
+		} else {
+			require.NotPanics(t, func() {
+				checkName(testCase.Type, testCase.Name)
+			})
+		}
+	}
+}

pkg/core/native_name_service_test.go

@@ -159,7 +159,6 @@ func TestRegisterAndRenew(t *testing.T) {
 
 	props := stackitem.NewMap()
 	props.Add(stackitem.Make("name"), stackitem.Make("neo.com"))
-	props.Add(stackitem.Make("description"), stackitem.Make(""))
 	props.Add(stackitem.Make("expiration"), stackitem.Make(expectedExpiration))
 	testNameServiceInvoke(t, bc, "properties", props, "neo.com")
 	testNameServiceInvoke(t, bc, "balanceOf", 1, testchain.CommitteeScriptHash())
@@ -206,7 +205,7 @@ func TestSetGetRecord(t *testing.T) {
 	testNameServiceInvoke(t, bc, "getRecord", "1.2.3.4", "neo.com", int64(native.RecordTypeA))
 	testNameServiceInvoke(t, bc, "setRecord", stackitem.Null{}, "neo.com", int64(native.RecordTypeA), "1.2.3.4")
 	testNameServiceInvoke(t, bc, "getRecord", "1.2.3.4", "neo.com", int64(native.RecordTypeA))
-	testNameServiceInvoke(t, bc, "setRecord", stackitem.Null{}, "neo.com", int64(native.RecordTypeAAAA), "2001:0000:1F1F:0000:0000:0100:11A0:ADDF")
+	testNameServiceInvoke(t, bc, "setRecord", stackitem.Null{}, "neo.com", int64(native.RecordTypeAAAA), "2001:0000:1f1f:0000:0000:0100:11a0:addf")
 	testNameServiceInvoke(t, bc, "setRecord", stackitem.Null{}, "neo.com", int64(native.RecordTypeCNAME), "nspcc.ru")
 	testNameServiceInvoke(t, bc, "setRecord", stackitem.Null{}, "neo.com", int64(native.RecordTypeTXT), "sometext")
 

pkg/core/state/nonfungible.go

@@ -15,7 +15,6 @@ import (
 type NFTTokenState struct {
 	Owner util.Uint160
 	Name  string
-	Description string
 }
 
 // NFTAccountState represents state of nonfunglible account.
@@ -35,7 +34,6 @@ func (s *NFTTokenState) ToStackItem() stackitem.Item {
 	return stackitem.NewStruct([]stackitem.Item{
 		stackitem.NewByteArray(owner.BytesBE()),
 		stackitem.NewByteArray([]byte(s.Name)),
-		stackitem.NewByteArray([]byte(s.Description)),
 	})
 }
 
@@ -47,7 +45,7 @@ func (s *NFTTokenState) EncodeBinary(w *io.BinWriter) {
 // FromStackItem converts stackitem to NFTTokenState.
 func (s *NFTTokenState) FromStackItem(item stackitem.Item) error {
 	arr, ok := item.Value().([]stackitem.Item)
-	if !ok || len(arr) < 3 {
+	if !ok || len(arr) < 2 {
 		return errors.New("invalid stack item")
 	}
 
@@ -63,14 +61,9 @@ func (s *NFTTokenState) FromStackItem(item stackitem.Item) error {
 	if err != nil {
 		return err
 	}
-	desc, err := stackitem.ToString(arr[2])
-	if err != nil {
-		return err
-	}
 
 	s.Owner = owner
 	s.Name = name
-	s.Description = desc
 	return nil
 }
 
@@ -89,10 +82,6 @@ func (s *NFTTokenState) ToMap() *stackitem.Map {
 			Key:   stackitem.NewByteArray([]byte("name")),
 			Value: stackitem.NewByteArray([]byte(s.Name)),
 		},
-		{
-			Key:   stackitem.NewByteArray([]byte("description")),
-			Value: stackitem.NewByteArray([]byte(s.Description)),
-		},
 	})
 }
 

pkg/core/state/nonfungible_test.go

@@ -24,7 +24,6 @@ func TestNFTTokenState_Serializable(t *testing.T) {
 		s := &NFTTokenState{
 			Owner: random.Uint160(),
 			Name:  "random name",
-			Description: "random description",
 		}
 		id := s.ID()
 		actual := new(NFTTokenState)
@@ -42,8 +41,6 @@ func TestNFTTokenState_Serializable(t *testing.T) {
 			{"invalid owner uint160", newStruct("123", "name", "desc")},
 			{"invalid name",
 				newStruct(random.Uint160().BytesBE(), []byte{0x80}, "desc")},
-			{"invalid description",
-				newStruct(random.Uint160().BytesBE(), "name", []byte{0x80})},
 		}
 
 		for _, tc := range errCases {
@@ -61,7 +58,6 @@ func TestNFTTokenState_ToMap(t *testing.T) {
 	s := &NFTTokenState{
 		Owner: random.Uint160(),
 		Name:  "random name",
-		Description: "random description",
 	}
 	m := s.ToMap()
 
@@ -69,10 +65,6 @@ func TestNFTTokenState_ToMap(t *testing.T) {
 	i := m.Index(stackitem.Make("name"))
 	require.True(t, i < len(elems))
 	require.Equal(t, []byte("random name"), elems[i].Value.Value())
-
-	i = m.Index(stackitem.Make("description"))
-	require.True(t, i < len(elems))
-	require.Equal(t, []byte("random description"), elems[i].Value.Value())
 }
 
 func TestNFTAccountState_Serializable(t *testing.T) {

pkg/core/transaction/transaction.go

@@ -31,6 +31,9 @@ const (
 	DummyVersion = 255
 )
 
+// ErrInvalidWitnessNum returns when the number of witnesses does not match signers.
+var ErrInvalidWitnessNum = errors.New("number of signers doesn't match witnesses")
+
 // Transaction is a process recorded in the NEO blockchain.
 type Transaction struct {
 	// The trading version which is currently 0.
@@ -170,7 +173,11 @@ func (t *Transaction) DecodeBinary(br *io.BinReader) {
 	if br.Err != nil {
 		return
 	}
-	br.ReadArray(&t.Scripts)
+	br.ReadArray(&t.Scripts, len(t.Signers))
+	if len(t.Signers) != len(t.Scripts) {
+		br.Err = fmt.Errorf("%w: %d vs %d", ErrInvalidWitnessNum, len(t.Signers), len(t.Scripts))
+		return
+	}
 
 	// Create the hash of the transaction at decode, so we dont need
 	// to do it anymore.

pkg/core/transaction/transaction_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/nspcc-dev/neo-go/pkg/config/netmode"
 	"github.com/nspcc-dev/neo-go/pkg/encoding/fixedn"
 	"github.com/nspcc-dev/neo-go/pkg/util"
+	"github.com/nspcc-dev/neo-go/pkg/vm/opcode"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -77,6 +78,7 @@ func TestNew(t *testing.T) {
 	script := []byte{0x51}
 	tx := New(netmode.UnitTestNet, script, 1)
 	tx.Signers = []Signer{{Account: util.Uint160{1, 2, 3}}}
+	tx.Scripts = []Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}}
 	assert.Equal(t, int64(1), tx.SystemFee)
 	assert.Equal(t, script, tx.Script)
 	// Update hash fields to match tx2 that is gonna autoupdate them on decode.
@@ -90,6 +92,7 @@ func TestNewTransactionFromBytes(t *testing.T) {
 	tx := New(netmode.UnitTestNet, script, 1)
 	tx.NetworkFee = 123
 	tx.Signers = []Signer{{Account: util.Uint160{1, 2, 3}}}
+	tx.Scripts = []Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}}
 	data, err := testserdes.EncodeBinary(tx)
 	require.NoError(t, err)
 
@@ -118,6 +121,15 @@ func TestDecodingTXWithNoScript(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestDecodingTxWithInvalidWitnessesNumber(t *testing.T) {
+	tx := New(netmode.UnitTestNet, []byte{byte(opcode.RET)}, 1)
+	tx.Signers = []Signer{{Account: util.Uint160{1, 2, 3}}}
+	tx.Scripts = []Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}, {InvocationScript: []byte{}, VerificationScript: []byte{}}}
+	data, err := testserdes.EncodeBinary(tx)
+	require.NoError(t, err)
+	require.True(t, errors.Is(testserdes.DecodeBinary(data, new(Transaction)), ErrInvalidWitnessNum))
+}
+
 func TestUnmarshalNeoFSTX(t *testing.T) {
 	txjson := []byte(`
 {

pkg/network/message_test.go

@@ -306,6 +306,7 @@ func newDummyBlock(height uint32, txCount int) *block.Block {
 func newDummyTx() *transaction.Transaction {
 	tx := transaction.New(netmode.UnitTestNet, random.Bytes(100), 123)
 	tx.Signers = []transaction.Signer{{Account: random.Uint160()}}
+	tx.Scripts = []transaction.Witness{{InvocationScript: []byte{}, VerificationScript: []byte{}}}
 	tx.Size()
 	tx.Hash()
 	return tx

pkg/network/payload/extensible.go

@@ -65,7 +65,7 @@ func (e *Extensible) decodeBinaryUnsigned(r *io.BinReader) {
 	e.ValidBlockStart = r.ReadU32LE()
 	e.ValidBlockEnd = r.ReadU32LE()
 	r.ReadBytes(e.Sender[:])
-	e.Data = r.ReadVarBytes(maxExtensibleDataSize)
+	e.Data = r.ReadVarBytes(MaxSize)
 }
 
 // DecodeBinary implements io.Serializable.

pkg/network/payload/extensible_test.go

@@ -41,6 +41,14 @@ func TestExtensible_Serializable(t *testing.T) {
 			err := testserdes.DecodeBinary(append(unsigned, 42), new(Extensible))
 			require.True(t, errors.Is(err, errInvalidPadding))
 		})
+		t.Run("too large data size", func(t *testing.T) {
+			expected.Data = make([]byte, MaxSize+1)
+			w := io.NewBufBinWriter()
+			expected.encodeBinaryUnsigned(w.BinWriter)
+			unsigned = w.Bytes()
+			err := testserdes.DecodeBinary(unsigned, new(Extensible))
+			require.NotNil(t, err)
+		})
 	})
 }
 

pkg/network/payload/notary_request.go

@@ -142,9 +142,6 @@ func (r *P2PNotaryRequest) isValid() error {
 	if len(r.FallbackTransaction.Signers) != 2 {
 		return errors.New("fallback transaction should have two signers")
 	}
-	if len(r.FallbackTransaction.Scripts) != 2 {
-		return errors.New("fallback transaction should have dummy Notary witness and valid witness for the second signer")
-	}
 	if len(r.FallbackTransaction.Scripts[0].InvocationScript) != 66 ||
 		len(r.FallbackTransaction.Scripts[0].VerificationScript) != 0 ||
 		!bytes.HasPrefix(r.FallbackTransaction.Scripts[0].InvocationScript, []byte{byte(opcode.PUSHDATA1), 64}) {

pkg/services/notary/notary.go

@@ -339,9 +339,6 @@ func (n *Notary) verifyIncompleteWitnesses(tx *transaction.Transaction, nKeys ui
 	if len(tx.Signers) < 2 {
 		return Unknown, 0, nil, errors.New("transaction should have at least 2 signers")
 	}
-	if len(tx.Signers) != len(tx.Scripts) {
-		return Unknown, 0, nil, fmt.Errorf("transaction should have %d witnesses attached (completed + dummy)", len(tx.Signers))
-	}
 	if !tx.HasSigner(n.Config.Chain.GetNotaryContractScriptHash()) {
 		return Unknown, 0, nil, fmt.Errorf("P2PNotary contract should be a signer of the transaction")
 	}

pkg/services/notary/notary_test.go

@@ -94,12 +94,6 @@ func TestVerifyIncompleteRequest(t *testing.T) {
 				Scripts: []transaction.Witness{{}},
 			},
 		},
-		"signers count and witnesses count mismatch": {
-			tx: &transaction.Transaction{
-				Signers: []transaction.Signer{{Account: notaryContractHash}, {}},
-				Scripts: []transaction.Witness{{}, {}, {}},
-			},
-		},
 		"missing Notary witness": {
 			tx: &transaction.Transaction{
 				Signers: []transaction.Signer{{Account: acc1.GetScriptHash()}, {Account: acc2.GetScriptHash()}},