From bcc2980179f39eea4825df72ad882274086d912e Mon Sep 17 00:00:00 2001
From: Péter Szilágyi <peterke@gmail.com>
Date: Fri, 15 May 2015 02:14:21 +0300
Subject: eth/downloader: check sync after failed attacks

---
 eth/downloader/downloader.go      |  6 +++++-
 eth/downloader/downloader_test.go | 24 ++++++++++++++++++------
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go
index c90d289df..30be0dde5 100644
--- a/eth/downloader/downloader.go
+++ b/eth/downloader/downloader.go
@@ -268,8 +268,12 @@ out:
 			// Insert all the new hashes, but only continue if got something useful
 			inserts := d.queue.Insert(hashPack.hashes)
 			if inserts == 0 && !done {
+				glog.V(logger.Debug).Infof("Peer (%s) responded with stale hashes\n", activePeer.id)
+				d.queue.Reset()
+
 				return ErrBadPeer
-			} else if !done {
+			}
+			if !done {
 				activePeer.getHashes(hash)
 				continue
 			}
diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go
index 46538d6b4..7888bd1e0 100644
--- a/eth/downloader/downloader_test.go
+++ b/eth/downloader/downloader_test.go
@@ -342,14 +342,13 @@ func TestNonExistingParentAttack(t *testing.T) {
 // loop indefinitely.
 func TestRepeatingHashAttack(t *testing.T) {
 	// Create a valid chain, but drop the last link
-	hashes := createHashes(0, 1000)
+	hashes := createHashes(0, blockCacheLimit)
 	blocks := createBlocksFromHashes(hashes)
-
-	hashes = hashes[:len(hashes)-1]
+	forged := hashes[:len(hashes)-1]
 
 	// Try and sync with the malicious node
-	tester := newTester(t, hashes, blocks)
-	tester.newPeer("attack", big.NewInt(10000), hashes[0])
+	tester := newTester(t, forged, blocks)
+	tester.newPeer("attack", big.NewInt(10000), forged[0])
 
 	errc := make(chan error)
 	go func() {
@@ -365,14 +364,21 @@ func TestRepeatingHashAttack(t *testing.T) {
 			t.Fatalf("synchronisation succeeded")
 		}
 	}
+	// Ensure that a valid chain can still pass sync
+	tester.hashes = hashes
+	tester.newPeer("valid", big.NewInt(20000), hashes[0])
+	if err := tester.sync("valid", hashes[0]); err != nil {
+		t.Fatalf("failed to synchronise blocks: %v", err)
+	}
 }
 
 // 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)
+	hashes := createHashes(0, blockCacheLimit)
 	blocks := createBlocksFromHashes(hashes)
+	origin := hashes[len(hashes)/2]
 
 	hashes[len(hashes)/2] = unknownHash
 
@@ -382,4 +388,10 @@ func TestNonExistingBlockAttack(t *testing.T) {
 	if err := tester.sync("attack", hashes[0]); err != errPeersUnavailable {
 		t.Fatalf("synchronisation error mismatch: have %v, want %v", err, errPeersUnavailable)
 	}
+	// Ensure that a valid chain can still pass sync
+	hashes[len(hashes)/2] = origin
+	tester.newPeer("valid", big.NewInt(20000), hashes[0])
+	if err := tester.sync("valid", hashes[0]); err != nil {
+		t.Fatalf("failed to synchronise blocks: %v", err)
+	}
 }
-- 
cgit