Merge pull request #1648 from nspcc-dev/backport-fixes-from-master

Backport fixes from master to 2.x

pkg/core/blockchain.go

@@ -1237,10 +1237,10 @@ func (bc *Blockchain) headerListLen() (n int) {
 	return
 }
 
-// GetTransaction returns a TX and its height by the given hash.
+// GetTransaction returns a TX and its height by the given hash. The height is MaxUint32 if tx is in the mempool.
 func (bc *Blockchain) GetTransaction(hash util.Uint256) (*transaction.Transaction, uint32, error) {
 	if tx, _, ok := bc.memPool.TryGetValue(hash); ok {
-		return tx, 0, nil // the height is not actually defined for memPool transaction. Not sure if zero is a good number in this case.
+		return tx, math.MaxUint32, nil // the height is not actually defined for memPool transaction.
 	}
 	return bc.dao.GetTransaction(hash)
 }

pkg/network/message.go

@@ -248,9 +248,6 @@ func (m *Message) Bytes() ([]byte, error) {
 	if err := m.Encode(w.BinWriter); err != nil {
 		return nil, err
 	}
-	if w.Err != nil {
-		return nil, w.Err
-	}
 	return w.Bytes(), nil
 }
 

pkg/network/server.go

@@ -266,7 +266,28 @@ func (s *Server) run() {
 				addr := drop.peer.PeerAddr().String()
 				if drop.reason == errIdenticalID {
 					s.discovery.RegisterBadAddr(addr)
-				} else if drop.reason != errAlreadyConnected {
+				} else if drop.reason == errAlreadyConnected {
+					// There is a race condition when peer can be disconnected twice for the this reason
+					// which can lead to no connections to peer at all. Here we check for such a possibility.
+					stillConnected := false
+					s.lock.RLock()
+					verDrop := drop.peer.Version()
+					addr := drop.peer.PeerAddr().String()
+					if verDrop != nil {
+						for peer := range s.peers {
+							ver := peer.Version()
+							// Already connected, drop this connection.
+							if ver != nil && ver.Nonce == verDrop.Nonce && peer.PeerAddr().String() == addr {
+								stillConnected = true
+							}
+						}
+					}
+					s.lock.RUnlock()
+					if !stillConnected {
+						s.discovery.UnregisterConnectedAddr(addr)
+						s.discovery.BackFill(addr)
+					}
+				} else {
 					s.discovery.UnregisterConnectedAddr(addr)
 					s.discovery.BackFill(addr)
 				}
@@ -866,10 +887,20 @@ func (s *Server) requestTx(hashes ...util.Uint256) {
 		return
 	}
 
-	msg := s.MkMsg(CMDGetData, payload.NewInventory(payload.TXType, hashes))
+	for i := 0; i <= len(hashes)/payload.MaxHashesCount; i++ {
+		start := i * payload.MaxHashesCount
+		stop := (i + 1) * payload.MaxHashesCount
+		if stop > len(hashes) {
+			stop = len(hashes)
+		}
+		if start == stop {
+			break
+		}
+		msg := s.MkMsg(CMDGetData, payload.NewInventory(payload.TXType, hashes[start:stop]))
 		// It's high priority because it directly affects consensus process,
 		// even though it's getdata.
 		s.broadcastHPMessage(msg)
+	}
 }
 
 // iteratePeersWithSendMsg sends given message to all peers using two functions

pkg/network/tcp_peer.go

@@ -178,6 +178,7 @@ func (p *TCPPeer) handleQueues() {
 	var p2pSkipCounter uint32
 	const p2pSkipDivisor = 4
 
+	var writeTimeout = time.Duration(p.server.chain.GetConfig().SecondsPerBlock) * time.Second
 	for {
 		var msg []byte
 
@@ -211,6 +212,10 @@ func (p *TCPPeer) handleQueues() {
 			case msg = <-p.sendQ:
 			}
 		}
+		err = p.conn.SetWriteDeadline(time.Now().Add(writeTimeout))
+		if err != nil {
+			break
+		}
 		_, err = p.conn.Write(msg)
 		if err != nil {
 			break

pkg/rpc/response/result/tx_raw_output.go

@@ -28,29 +28,27 @@ type TransactionMetadata struct {
 
 // NewTransactionOutputRaw returns a new ransactionOutputRaw object.
 func NewTransactionOutputRaw(tx *transaction.Transaction, header *block.Header, chain core.Blockchainer) TransactionOutputRaw {
-	// confirmations formula
-	confirmations := int(chain.BlockHeight() - header.Base.Index + 1)
-	return TransactionOutputRaw{
+	result := TransactionOutputRaw{
 		Transaction: tx,
 		TransactionMetadata: TransactionMetadata{
 			SysFee: chain.SystemFee(tx),
 			NetFee: chain.NetworkFee(tx),
-			Blockhash:     header.Hash(),
-			Confirmations: confirmations,
-			Timestamp:     header.Timestamp,
 		},
 	}
+	if header == nil {
+		return result
+	}
+	// confirmations formula
+	confirmations := int(chain.BlockHeight() - header.Base.Index + 1)
+	result.TransactionMetadata.Blockhash = header.Hash()
+	result.TransactionMetadata.Confirmations = confirmations
+	result.TransactionMetadata.Timestamp = header.Timestamp
+	return result
 }
 
 // MarshalJSON implements json.Marshaler interface.
 func (t TransactionOutputRaw) MarshalJSON() ([]byte, error) {
-	output, err := json.Marshal(TransactionMetadata{
-		SysFee:        t.SysFee,
-		NetFee:        t.NetFee,
-		Blockhash:     t.Blockhash,
-		Confirmations: t.Confirmations,
-		Timestamp:     t.Timestamp,
-	})
+	output, err := json.Marshal(t.TransactionMetadata)
 	if err != nil {
 		return nil, err
 	}
@@ -78,17 +76,6 @@ func (t *TransactionOutputRaw) UnmarshalJSON(data []byte) error {
 	if err != nil {
 		return err
 	}
-	t.SysFee = output.SysFee
-	t.NetFee = output.NetFee
-	t.Blockhash = output.Blockhash
-	t.Confirmations = output.Confirmations
-	t.Timestamp = output.Timestamp
-
-	transaction := new(transaction.Transaction)
-	err = json.Unmarshal(data, transaction)
-	if err != nil {
-		return err
-	}
-	t.Transaction = transaction
-	return nil
+	t.TransactionMetadata = *output
+	return json.Unmarshal(data, &t.Transaction)
 }

pkg/rpc/response/result/unclaimed.go

@@ -1,6 +1,9 @@
 package result
 
 import (
+	"errors"
+	"math"
+
 	"github.com/nspcc-dev/neo-go/pkg/core"
 	"github.com/nspcc-dev/neo-go/pkg/core/state"
 	"github.com/nspcc-dev/neo-go/pkg/util"
@@ -38,6 +41,9 @@ func NewUnclaimed(a *state.Account, chain core.Blockchainer) (*Unclaimed, error)
 		if err != nil {
 			return nil, err
 		}
+		if txHeight == math.MaxUint32 {
+			return nil, errors.New("wrong transaction stored in account data")
+		}
 		gen, sys, err := chain.CalculateClaimable(usb.Value, txHeight, blockHeight)
 		if err != nil {
 			return nil, err

pkg/rpc/server/server.go

@@ -5,6 +5,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"math"
 	"net"
 	"net/http"
 	"strconv"
@@ -1159,27 +1160,29 @@ func (s *Server) getStorage(ps request.Params) (interface{}, *response.Error) {
 }
 
 func (s *Server) getrawtransaction(reqParams request.Params) (interface{}, *response.Error) {
-	var resultsErr *response.Error
-	var results interface{}
-
-	if txHash, err := reqParams.Value(0).GetUint256(); err != nil {
-		resultsErr = response.ErrInvalidParams
-	} else if tx, height, err := s.chain.GetTransaction(txHash); err != nil {
+	txHash, err := reqParams.Value(0).GetUint256()
+	if err != nil {
+		return nil, response.ErrInvalidParams
+	}
+	tx, height, err := s.chain.GetTransaction(txHash)
+	if err != nil {
 		err = errors.Wrapf(err, "Invalid transaction hash: %s", txHash)
 		return nil, response.NewRPCError("Unknown transaction", err.Error(), err)
-	} else if reqParams.Value(1).GetBoolean() {
+	}
+	if reqParams.Value(1).GetBoolean() {
+		if height == math.MaxUint32 {
+			return result.NewTransactionOutputRaw(tx, nil, s.chain), nil
+		}
 		_header := s.chain.GetHeaderHash(int(height))
 		header, err := s.chain.GetHeader(_header)
 		if err != nil {
-			resultsErr = response.NewInvalidParamsError(err.Error(), err)
-		} else {
-			results = result.NewTransactionOutputRaw(tx, header, s.chain)
-		}
-	} else {
-		results = hex.EncodeToString(tx.Bytes())
+			return nil, response.NewInvalidParamsError(err.Error(), err)
 		}
+		return result.NewTransactionOutputRaw(tx, header, s.chain), nil
+
+	}
+	return hex.EncodeToString(tx.Bytes()), nil
 
-	return results, resultsErr
 }
 
 func (s *Server) getTransactionHeight(ps request.Params) (interface{}, *response.Error) {
@@ -1189,7 +1192,7 @@ func (s *Server) getTransactionHeight(ps request.Params) (interface{}, *response
 	}
 
 	_, height, err := s.chain.GetTransaction(h)
-	if err != nil {
+	if err != nil || height == math.MaxUint32 {
 		return nil, response.NewRPCError("unknown transaction", "", nil)
 	}
 

pkg/vm/cli/cli.go

@@ -213,8 +213,13 @@ func handleIP(c *ishell.Context) {
 		return
 	}
 	v := getVMFromContext(c)
-	ip, opcode := v.Context().CurrInstr()
+	ctx := v.Context()
+	if ctx.NextIP() < ctx.LenInstr() {
+		ip, opcode := v.Context().NextInstr()
 		c.Printf("instruction pointer at %d (%s)\n", ip, opcode)
+	} else {
+		c.Println("execution has finished")
+	}
 }
 
 func handleBreak(c *ishell.Context) {
@@ -224,6 +229,7 @@ func handleBreak(c *ishell.Context) {
 	v := getVMFromContext(c)
 	if len(c.Args) != 1 {
 		c.Err(errors.New("missing parameter <ip>"))
+		return
 	}
 	n, err := strconv.Atoi(c.Args[0])
 	if err != nil {
@@ -319,7 +325,6 @@ func runVMWithHandling(c *ishell.Context, v *vm.VM) {
 	err := v.Run()
 	if err != nil {
 		c.Err(err)
-		return
 	}
 	checkAndPrintVMState(c, v)
 }
@@ -331,16 +336,20 @@ func checkAndPrintVMState(c *ishell.Context, v *vm.VM) {
 	var message string
 	switch {
 	case v.HasFailed():
-		message = "FAILED"
+		message = "" // the error will be printed on return
 	case v.HasHalted():
 		message = v.Stack("estack")
 	case v.AtBreakpoint():
 		ctx := v.Context()
-		i, op := ctx.CurrInstr()
-		message = fmt.Sprintf("at breakpoint %d (%s)\n", i, op.String())
+		if ctx.NextIP() < ctx.LenInstr() {
+			i, op := ctx.NextInstr()
+			message = fmt.Sprintf("at breakpoint %d (%s)", i, op)
+		} else {
+			message = "execution has finished"
+		}
 	}
 	if message != "" {
-		c.Printf(message)
+		c.Println(message)
 	}
 }
 
@@ -412,8 +421,9 @@ func handleStepType(c *ishell.Context, stepType string) {
 	}
 	if err != nil {
 		c.Err(err)
-	}
+	} else {
 		handleIP(c)
+	}
 	changePrompt(c, v)
 }
 
@@ -426,8 +436,8 @@ func handleOps(c *ishell.Context) {
 }
 
 func changePrompt(c ishell.Actions, v *vm.VM) {
-	if v.Ready() && v.Context().IP()-1 >= 0 {
-		c.SetPrompt(fmt.Sprintf("NEO-GO-VM %d > ", v.Context().IP()-1))
+	if v.Ready() && v.Context().NextIP() >= 0 && v.Context().NextIP() < v.Context().LenInstr() {
+		c.SetPrompt(fmt.Sprintf("NEO-GO-VM %d > ", v.Context().NextIP()))
 	} else {
 		c.SetPrompt("NEO-GO-VM > ")
 	}

pkg/vm/context.go

@@ -143,6 +143,15 @@ func (c *Context) CurrInstr() (int, opcode.Opcode) {
 	return c.ip, opcode.Opcode(c.prog[c.ip])
 }
 
+// NextInstr returns the next instruction and opcode.
+func (c *Context) NextInstr() (int, opcode.Opcode) {
+	op := opcode.RET
+	if c.nextip < len(c.prog) {
+		op = opcode.Opcode(c.prog[c.nextip])
+	}
+	return c.nextip, op
+}
+
 // Copy returns an new exact copy of c.
 func (c *Context) Copy() *Context {
 	ctx := new(Context)

pkg/vm/vm.go

@@ -228,7 +228,7 @@ func (v *VM) AddBreakPoint(n int) {
 // instruction pointer.
 func (v *VM) AddBreakPointRel(n int) {
 	ctx := v.Context()
-	v.AddBreakPoint(ctx.ip + n)
+	v.AddBreakPoint(ctx.nextip + n)
 }
 
 // LoadFile loads a program from the given path, ready to execute it.