From 473cc37a75cc356d5fc376a56dcfeda0f7dfd48b Mon Sep 17 00:00:00 2001
From: Svarog <feanorr@gmail.com>
Date: Tue, 4 Aug 2020 11:07:21 +0300
Subject: [PATCH] [NOD-1224] Fix duplicate connections and duplicate blocks
 bugs (#842)

* [NOD-1224] Make block already existing a ruleError

* [NOD-1224] Remove block from pendingBlocks list only after it was processed

* [NOD-1224] AddToPeers should have a Write Lock, not Read Lock

* [NOD-1224] Check for unrequested before processing
---
 blockdag/process.go                            | 6 ++++--
 protocol/common/common.go                      | 5 +++--
 protocol/flowcontext/network.go                | 4 ++--
 protocol/flows/blockrelay/handle_relay_invs.go | 7 +++++--
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/blockdag/process.go b/blockdag/process.go
index 5e1815646..b452d309f 100644
--- a/blockdag/process.go
+++ b/blockdag/process.go
@@ -6,9 +6,10 @@ package blockdag
 
 import (
 	"fmt"
+	"time"
+
 	"github.com/kaspanet/kaspad/dagconfig"
 	"github.com/pkg/errors"
-	"time"
 
 	"github.com/kaspanet/kaspad/util"
 	"github.com/kaspanet/kaspad/util/daghash"
@@ -162,7 +163,8 @@ func (dag *BlockDAG) processBlockNoLock(block *util.Block, flags BehaviorFlags)
 
 	// The block must not already exist in the DAG.
 	if dag.IsInDAG(blockHash) && !wasBlockStored {
-		return false, false, errors.Errorf("already have block %s", blockHash)
+		str := fmt.Sprintf("already have block %s", blockHash)
+		return false, false, ruleError(ErrDuplicateBlock, str)
 	}
 
 	// The block must not already exist as an orphan.
diff --git a/protocol/common/common.go b/protocol/common/common.go
index 5e7d28da7..03061da14 100644
--- a/protocol/common/common.go
+++ b/protocol/common/common.go
@@ -1,8 +1,9 @@
 package common
 
 import (
-	"github.com/pkg/errors"
 	"time"
+
+	"github.com/pkg/errors"
 )
 
 // DefaultTimeout is the default duration to wait for enqueuing/dequeuing
@@ -10,4 +11,4 @@ import (
 const DefaultTimeout = 30 * time.Second
 
 // ErrPeerWithSameIDExists signifies that a peer with the same ID already exist.
-var ErrPeerWithSameIDExists = errors.New("ready with the same ID already exists")
+var ErrPeerWithSameIDExists = errors.New("ready peer with the same ID already exists")
diff --git a/protocol/flowcontext/network.go b/protocol/flowcontext/network.go
index ce18b2da6..3d644150d 100644
--- a/protocol/flowcontext/network.go
+++ b/protocol/flowcontext/network.go
@@ -21,8 +21,8 @@ func (f *FlowContext) ConnectionManager() *connmanager.ConnectionManager {
 
 // AddToPeers marks this peer as ready and adds it to the ready peers list.
 func (f *FlowContext) AddToPeers(peer *peerpkg.Peer) error {
-	f.peersMutex.RLock()
-	defer f.peersMutex.RUnlock()
+	f.peersMutex.Lock()
+	defer f.peersMutex.Unlock()
 
 	if _, ok := f.peers[peer.ID()]; ok {
 		return errors.Wrapf(common.ErrPeerWithSameIDExists, "peer with ID %s already exists", peer.ID())
diff --git a/protocol/flows/blockrelay/handle_relay_invs.go b/protocol/flows/blockrelay/handle_relay_invs.go
index 1b9f6b255..bc47902ac 100644
--- a/protocol/flows/blockrelay/handle_relay_invs.go
+++ b/protocol/flows/blockrelay/handle_relay_invs.go
@@ -141,16 +141,19 @@ func (flow *handleRelayInvsFlow) requestBlocks(requestQueue *hashesQueueSet) err
 
 		block := util.NewBlock(msgBlock)
 		blockHash := block.Hash()
+
 		if _, ok := pendingBlocks[*blockHash]; !ok {
 			return protocolerrors.Errorf(true, "got unrequested block %s", block.Hash())
 		}
-		delete(pendingBlocks, *blockHash)
-		flow.SharedRequestedBlocks().remove(blockHash)
 
 		err = flow.processAndRelayBlock(requestQueue, block)
 		if err != nil {
 			return err
 		}
+
+		delete(pendingBlocks, *blockHash)
+		flow.SharedRequestedBlocks().remove(blockHash)
+
 	}
 	return nil
 }