eth/downloader: add a test for detecting missing blocks

2 files changed, 24 insertions, 8 deletions

diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go
index 3da606aef..c90d289df 100644
--- a/eth/downloader/downloader.go
+++ b/eth/downloader/downloader.go
@@ -2,7 +2,6 @@ package downloader
 
 import (
 	"errors"
-	"fmt"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -398,8 +397,7 @@ out:
 				// and all failed throw an error
 				if d.queue.InFlight() == 0 {
 					d.queue.Reset()
-
-					return fmt.Errorf("%v peers available = %d. total peers = %d. hashes needed = %d", errPeersUnavailable, len(idlePeers), d.peers.Len(), d.queue.Pending())
+					return errPeersUnavailable
 				}
 
 			} else if d.queue.InFlight() == 0 {
diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go
index 1bba3ca51..46538d6b4 100644
--- a/eth/downloader/downloader_test.go
+++ b/eth/downloader/downloader_test.go
@@ -106,11 +106,12 @@ func (dl *downloadTester) getHashes(hash common.Hash) error {
 
 func (dl *downloadTester) getBlocks(id string) func([]common.Hash) error {
 	return func(hashes []common.Hash) error {
-		blocks := make([]*types.Block, len(hashes))
-		for i, hash := range hashes {
-			blocks[i] = dl.blocks[hash]
+		blocks := make([]*types.Block, 0, len(hashes))
+		for _, hash := range hashes {
+			if block, ok := dl.blocks[hash]; ok {
+				blocks = append(blocks, block)
+			}
 		}
-
 		go dl.downloader.DeliverBlocks(id, blocks)
 
 		return nil
@@ -341,7 +342,7 @@ func TestNonExistingParentAttack(t *testing.T) {
 // loop indefinitely.
 func TestRepeatingHashAttack(t *testing.T) {
 	// Create a valid chain, but drop the last link
-	hashes := createHashes(1000, 1)
+	hashes := createHashes(0, 1000)
 	blocks := createBlocksFromHashes(hashes)
 
 	hashes = hashes[:len(hashes)-1]
@@ -365,3 +366,20 @@ func TestRepeatingHashAttack(t *testing.T) {
 		}
 	}
 }
+
+// Tests that if a malicious peers returns a non-existent block hash, it should
+// eventually time out and the sync reattempted.
+func TestNonExistingBlockAttack(t *testing.T) {
+	// Create a valid chain, but forge the last link
+	hashes := createHashes(0, 10)
+	blocks := createBlocksFromHashes(hashes)
+
+	hashes[len(hashes)/2] = unknownHash
+
+	// Try and sync with the malicious node and check that it fails
+	tester := newTester(t, hashes, blocks)
+	tester.newPeer("attack", big.NewInt(10000), hashes[0])
+	if err := tester.sync("attack", hashes[0]); err != errPeersUnavailable {
+		t.Fatalf("synchronisation error mismatch: have %v, want %v", err, errPeersUnavailable)
+	}
+}