From de1dc1644dfe0bf7388197bb874a745a5c02bbb0 Mon Sep 17 00:00:00 2001
From: George Tankersley <george.tankersley@gmail.com>
Date: Mon, 1 Jun 2020 13:33:18 -0400
Subject: [PATCH] zcash: fix blacklist behavior

Previously, the blacklist would never be retried since we queued the
wrong list of addresses. New logic also drops peers from the blacklist
if they've been continuously retried without success for a while.
---
 zcash/address_book.go | 27 ++++++++++++++++++++++++++-
 zcash/client.go       | 15 ++++++++++++---
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/zcash/address_book.go b/zcash/address_book.go
index 7196f5c..d78c740 100644
--- a/zcash/address_book.go
+++ b/zcash/address_book.go
@@ -123,8 +123,22 @@ func (bk *AddressBook) Blacklist(s PeerKey) {
 	}
 }
 
-// Redeem removes an address from the blacklist.
+// Redeem removes an address from the blacklist and adds it to the peer list.
 func (bk *AddressBook) Redeem(s PeerKey) {
+	bk.addrState.Lock()
+	if addr, ok := bk.blacklist[s]; ok {
+		delete(bk.blacklist, s)
+		addr.lastUpdate = time.Now()
+		bk.peers[s] = addr
+	}
+	bk.addrState.Unlock()
+
+	// Wake anyone who was waiting on us to receive an address.
+	bk.addrRecvCond.Broadcast()
+}
+
+// DropFromBlacklist removes an address from the blacklist.
+func (bk *AddressBook) DropFromBlacklist(s PeerKey) {
 	bk.addrState.Lock()
 	defer bk.addrState.Unlock()
 
@@ -179,6 +193,17 @@ func (bk *AddressBook) enqueueAddrs(addrQueue *chan *Address) {
 	}
 }
 
+// enqueueBlacklist puts all of our blacklisted peers into a channel.
+func (bk *AddressBook) enqueueBlacklist(addrQueue *chan *Address) {
+	bk.addrState.RLock()
+	defer bk.addrState.RUnlock()
+
+	*addrQueue = make(chan *Address, len(bk.blacklist))
+	for _, v := range bk.blacklist {
+		*addrQueue <- v
+	}
+}
+
 // WaitForAddresses waits for n addresses to be received and their initial
 // connection attempts to resolve. There is no escape if that does not happen -
 // this is intended for test runners or goroutines with a timeout.
diff --git a/zcash/client.go b/zcash/client.go
index 593a148..2d387b3 100644
--- a/zcash/client.go
+++ b/zcash/client.go
@@ -53,6 +53,9 @@ var (
 
 	// The amount of space we allocate to keep things moving smoothly.
 	incomingAddressBufferSize = 1024
+
+	// The amount of time a peer can spend on the blacklist before we forget about it entirely.
+	blacklistDropTime = 3 * 24 * time.Hour
 )
 
 // Seeder contains all of the state and configuration needed to request addresses from Zcash peers and present them to a DNS provider.
@@ -443,7 +446,7 @@ func (s *Seeder) RetryBlacklist() {
 	var wg sync.WaitGroup
 
 	// XXX lil awkward to allocate a channel whose size we can't determine without a lock here
-	s.addrBook.enqueueAddrs(&blacklistQueue)
+	s.addrBook.enqueueBlacklist(&blacklistQueue)
 
 	for i := 0; i < crawlerGoroutineCount; i++ {
 		wg.Add(1)
@@ -460,12 +463,18 @@ func (s *Seeder) RetryBlacklist() {
 
 				if err != nil {
 					// Connection failed. Peer remains blacklisted.
+					if time.Since(next.lastUpdate) > blacklistDropTime {
+						// If we've been retrying for a while, forget about this peer entirely.
+						// This would deadlock if enqueueBlacklist still held the RLock.
+						s.addrBook.DropFromBlacklist(next.asPeerKey())
+					}
 					continue
 				}
 
 				s.DisconnectPeer(next.asPeerKey())
 
-				// This would deadlock if enqueueAddrs still held the RLock.
+				// Remove the peer from the blacklist and add it back to the address book.
+				// This would deadlock if enqueueBlacklist still held the RLock.
 				s.addrBook.Redeem(next.asPeerKey())
 			}
 			wg.Done()
@@ -515,5 +524,5 @@ func (s *Seeder) testBlacklist(pk PeerKey) {
 
 // testRedeen adds a peer to the blacklist directly, for testing.
 func (s *Seeder) testRedeem(pk PeerKey) {
-	s.addrBook.Redeem(pk)
+	s.addrBook.DropFromBlacklist(pk)
 }