Merge pull request #2846 from nspcc-dev/fix-getblockbyindex-req

network: do not allow to request invalid block count

pkg/network/blockqueue.go

@@ -136,10 +136,12 @@ func (bq *blockQueue) putBlock(block *block.Block) error {
 	return nil
 }
 
-func (bq *blockQueue) lastQueued() uint32 {
+// lastQueued returns the index of the last queued block and the queue's capacity
+// left.
+func (bq *blockQueue) lastQueued() (uint32, int) {
 	bq.queueLock.RLock()
 	defer bq.queueLock.RUnlock()
-	return bq.lastQ
+	return bq.lastQ, blockCacheSize - bq.len
 }
 
 func (bq *blockQueue) discard() {

pkg/network/blockqueue_test.go

@@ -22,7 +22,9 @@ func TestBlockQueue(t *testing.T) {
 	for i := 3; i < 5; i++ {
 		assert.NoError(t, bq.putBlock(blocks[i]))
 	}
-	assert.Equal(t, uint32(0), bq.lastQueued())
+	last, capLeft := bq.lastQueued()
+	assert.Equal(t, uint32(0), last)
+	assert.Equal(t, blockCacheSize-2, capLeft)
 	// nothing should be put into the blockchain
 	assert.Equal(t, uint32(0), chain.BlockHeight())
 	assert.Equal(t, 2, bq.length())
@@ -31,7 +33,9 @@ func TestBlockQueue(t *testing.T) {
 		assert.NoError(t, bq.putBlock(blocks[i]))
 	}
 	// but they're still not put into the blockchain, because bq isn't running
-	assert.Equal(t, uint32(4), bq.lastQueued())
+	last, capLeft = bq.lastQueued()
+	assert.Equal(t, uint32(4), last)
+	assert.Equal(t, blockCacheSize-4, capLeft)
 	assert.Equal(t, uint32(0), chain.BlockHeight())
 	assert.Equal(t, 4, bq.length())
 	// block with too big index is dropped
@@ -40,14 +44,18 @@ func TestBlockQueue(t *testing.T) {
 	go bq.run()
 	// run() is asynchronous, so we need some kind of timeout anyway and this is the simplest one
 	assert.Eventually(t, func() bool { return chain.BlockHeight() == 4 }, 4*time.Second, 100*time.Millisecond)
-	assert.Equal(t, uint32(4), bq.lastQueued())
+	last, capLeft = bq.lastQueued()
+	assert.Equal(t, uint32(4), last)
+	assert.Equal(t, blockCacheSize, capLeft)
 	assert.Equal(t, 0, bq.length())
 	assert.Equal(t, uint32(4), chain.BlockHeight())
 	// put some old blocks
 	for i := 1; i < 5; i++ {
 		assert.NoError(t, bq.putBlock(blocks[i]))
 	}
-	assert.Equal(t, uint32(4), bq.lastQueued())
+	last, capLeft = bq.lastQueued()
+	assert.Equal(t, uint32(4), last)
+	assert.Equal(t, blockCacheSize, capLeft)
 	assert.Equal(t, 0, bq.length())
 	assert.Equal(t, uint32(4), chain.BlockHeight())
 	// unexpected blocks with run() active
@@ -65,7 +73,9 @@ func TestBlockQueue(t *testing.T) {
 	assert.NoError(t, bq.putBlock(blocks[5]))
 	// run() is asynchronous, so we need some kind of timeout anyway and this is the simplest one
 	assert.Eventually(t, func() bool { return chain.BlockHeight() == 8 }, 4*time.Second, 100*time.Millisecond)
-	assert.Equal(t, uint32(8), bq.lastQueued())
+	last, capLeft = bq.lastQueued()
+	assert.Equal(t, uint32(8), last)
+	assert.Equal(t, blockCacheSize-1, capLeft)
 	assert.Equal(t, 1, bq.length())
 	assert.Equal(t, uint32(8), chain.BlockHeight())
 	bq.discard()

pkg/network/payload/getblockbyindex.go

@@ -1,7 +1,7 @@
 package payload
 
 import (
-	"errors"
+	"fmt"
 
 	"github.com/nspcc-dev/neo-go/pkg/io"
 )
@@ -25,7 +25,7 @@ func (d *GetBlockByIndex) DecodeBinary(br *io.BinReader) {
 	d.IndexStart = br.ReadU32LE()
 	d.Count = int16(br.ReadU16LE())
 	if d.Count < -1 || d.Count == 0 || d.Count > MaxHeadersAllowed {
-		br.Err = errors.New("invalid block count")
+		br.Err = fmt.Errorf("invalid block count: %d", d.Count)
 	}
 }
 

pkg/network/server.go

@@ -1247,13 +1247,15 @@ func (s *Server) handleGetAddrCmd(p Peer) error {
 // 2. Send requests for chunk in increasing order.
 // 3. After all requests have been sent, request random height.
 func (s *Server) requestBlocks(bq Blockqueuer, p Peer) error {
-	h := bq.BlockHeight()
+	pl := getRequestBlocksPayload(p, bq.BlockHeight(), &s.lastRequestedBlock)
-	pl := getRequestBlocksPayload(p, h, &s.lastRequestedBlock)
+	lq, capLeft := s.bQueue.lastQueued()
-	lq := s.bQueue.lastQueued()
+	if capLeft == 0 {
-	if lq > pl.IndexStart {
+		// No more blocks will fit into the queue.
-		c := int16(h + blockCacheSize - lq)
+		return nil
-		if c < payload.MaxHashesCount {
+	}
-			pl.Count = c
+	if lq >= pl.IndexStart {
+		if capLeft < payload.MaxHashesCount {
+			pl.Count = int16(capLeft)
 		}
 		pl.IndexStart = lq + 1
 	}