Validator duplication tests

integration_testing/connection_controller_test.go

@@ -12,7 +12,6 @@ import (
 
 func TestConnectionControllerNonValidator(t *testing.T) {
 	require := require.New(t)
-	_ = require
 
 	dbDir1 := getDirectory(t)
 	defer os.RemoveAll(dbDir1)
@@ -66,7 +65,6 @@ func TestConnectionControllerNonValidator(t *testing.T) {
 
 func TestConnectionControllerValidator(t *testing.T) {
 	require := require.New(t)
-	_ = require
 
 	dbDir1 := getDirectory(t)
 	defer os.RemoveAll(dbDir1)
@@ -125,7 +123,6 @@ func TestConnectionControllerValidator(t *testing.T) {
 
 func TestConnectionControllerHandshakeDataErrors(t *testing.T) {
 	require := require.New(t)
-	_ = require
 
 	dbDir1 := getDirectory(t)
 	dbDir2 := getDirectory(t)
@@ -261,30 +258,27 @@ func TestConnectionControllerHandshakeDataErrors(t *testing.T) {
 
 func TestConnectionControllerHandshakeTimeouts(t *testing.T) {
 	require := require.New(t)
-	_ = require
 
 	dbDir1 := getDirectory(t)
-	dbDir2 := getDirectory(t)
 	defer os.RemoveAll(dbDir1)
-	defer os.RemoveAll(dbDir2)
-
 	config1 := generateConfig(t, 18000, dbDir1, 10)
 	config1.SyncType = lib.NodeSyncTypeBlockSync
-	config2 := generateConfig(t, 18001, dbDir2, 10)
-	config2.SyncType = lib.NodeSyncTypeBlockSync
-
 	node1 := cmd.NewNode(config1)
-	node2 := cmd.NewNode(config2)
 	node1.Params.UserAgent = "Node1"
 	node1.Params.ProtocolVersion = lib.ProtocolVersion2
 	// Set version negotiation timeout to 0 to make sure that the node will be disconnected
 	node1.Params.VersionNegotiationTimeout = 0
+	node1 = startNode(t, node1)
+	defer node1.Stop()
+
+	dbDir2 := getDirectory(t)
+	defer os.RemoveAll(dbDir2)
+	config2 := generateConfig(t, 18001, dbDir2, 10)
+	config2.SyncType = lib.NodeSyncTypeBlockSync
+	node2 := cmd.NewNode(config2)
 	node2.Params.UserAgent = "Node2"
 	node2.Params.ProtocolVersion = lib.ProtocolVersion2
-
-	node1 = startNode(t, node1)
 	node2 = startNode(t, node2)
-	defer node1.Stop()
 	defer node2.Stop()
 
 	cc := node1.Server.GetConnectionController()
@@ -312,6 +306,100 @@ func TestConnectionControllerHandshakeTimeouts(t *testing.T) {
 	t.Logf("Test #2 passed | Successfuly disconnected node after verack exchange timeout")
 }
 
+func TestConnectionControllerValidatorDuplication(t *testing.T) {
+	require := require.New(t)
+
+	dbDir1 := getDirectory(t)
+	defer os.RemoveAll(dbDir1)
+	config1 := generateConfig(t, 18000, dbDir1, 10)
+	config1.SyncType = lib.NodeSyncTypeBlockSync
+	node1 := cmd.NewNode(config1)
+	node1.Params.UserAgent = "Node1"
+	node1.Params.ProtocolVersion = lib.ProtocolVersion2
+	node1 = startNode(t, node1)
+	defer node1.Stop()
+
+	// Create a validator Node2
+	dbDir2 := getDirectory(t)
+	defer os.RemoveAll(dbDir2)
+	config2 := generateConfig(t, 18001, dbDir2, 10)
+	config2.SyncType = lib.NodeSyncTypeBlockSync
+	blsPriv2, err := bls.NewPrivateKey()
+	require.NoError(err)
+	config2.PosValidatorSeed = blsPriv2.ToString()
+	node2 := cmd.NewNode(config2)
+	node2.Params.UserAgent = "Node2"
+	node2.Params.ProtocolVersion = lib.ProtocolVersion2
+	node2 = startNode(t, node2)
+
+	// Create a duplicate validator Node3
+	dbDir3 := getDirectory(t)
+	defer os.RemoveAll(dbDir3)
+	config3 := generateConfig(t, 18002, dbDir3, 10)
+	config3.SyncType = lib.NodeSyncTypeBlockSync
+	config3.PosValidatorSeed = blsPriv2.ToString()
+	node3 := cmd.NewNode(config3)
+	node3.Params.UserAgent = "Node3"
+	node3.Params.ProtocolVersion = lib.ProtocolVersion2
+	node3 = startNode(t, node3)
+
+	// Create validator connection from Node1 to Node2 and from Node1 to Node3
+	cc := node1.Server.GetConnectionController()
+	require.NoError(cc.CreateValidatorConnection(node2.Listeners[0].Addr().String(), blsPriv2.PublicKey()))
+	// This should fail out right because Node3 has a duplicate public key.
+	require.Error(cc.CreateValidatorConnection(node3.Listeners[0].Addr().String(), blsPriv2.PublicKey()))
+	waitForValidatorConnection(t, node1, node2)
+	waitForNonValidatorInboundConnection(t, node2, node1)
+
+	// Now create an outbound connection from Node3 to Node1, which should pass handshake, but then fail because
+	// Node1 already has a validator connection to Node2 with the same public key.
+	cc3 := node3.Server.GetConnectionController()
+	require.NoError(cc3.CreateNonValidatorOutboundConnection(node1.Listeners[0].Addr().String()))
+	waitForEmptyRemoteNodeIndexer(t, node3)
+	waitForCountRemoteNodeIndexer(t, node1, 1, 1, 0, 0)
+	t.Logf("Test #1 passed | Successfuly rejected duplicate validator connection with inbound/outbound validators")
+
+	node3.Stop()
+	node2.Stop()
+	waitForEmptyRemoteNodeIndexer(t, node1)
+
+	// Create two more validators Node4, Node5 with duplicate public keys
+	dbDir4 := getDirectory(t)
+	defer os.RemoveAll(dbDir4)
+	config4 := generateConfig(t, 18003, dbDir4, 10)
+	config4.SyncType = lib.NodeSyncTypeBlockSync
+	blsPriv4, err := bls.NewPrivateKey()
+	require.NoError(err)
+	config4.PosValidatorSeed = blsPriv4.ToString()
+	node4 := cmd.NewNode(config4)
+	node4.Params.UserAgent = "Node4"
+	node4.Params.ProtocolVersion = lib.ProtocolVersion2
+	node4 = startNode(t, node4)
+	defer node4.Stop()
+
+	dbDir5 := getDirectory(t)
+	defer os.RemoveAll(dbDir5)
+	config5 := generateConfig(t, 18004, dbDir5, 10)
+	config5.SyncType = lib.NodeSyncTypeBlockSync
+	config5.PosValidatorSeed = blsPriv4.ToString()
+	node5 := cmd.NewNode(config5)
+	node5.Params.UserAgent = "Node5"
+	node5.Params.ProtocolVersion = lib.ProtocolVersion2
+	node5 = startNode(t, node5)
+	defer node5.Stop()
+
+	// Create validator connections from Node4 to Node1 and from Node5 to Node1
+	cc4 := node4.Server.GetConnectionController()
+	require.NoError(cc4.CreateNonValidatorOutboundConnection(node1.Listeners[0].Addr().String()))
+	waitForValidatorConnection(t, node1, node4)
+	waitForNonValidatorOutboundConnection(t, node4, node1)
+	cc5 := node5.Server.GetConnectionController()
+	require.NoError(cc5.CreateNonValidatorOutboundConnection(node1.Listeners[0].Addr().String()))
+	waitForEmptyRemoteNodeIndexer(t, node5)
+	waitForCountRemoteNodeIndexer(t, node1, 1, 1, 0, 0)
+	t.Logf("Test #2 passed | Successfuly rejected duplicate validator connection with multiple outbound validators")
+}
+
 func waitForValidatorConnection(t *testing.T, node1 *cmd.Node, node2 *cmd.Node) {
 	userAgentN1 := node1.Params.UserAgent
 	userAgentN2 := node2.Params.UserAgent
@@ -390,6 +478,20 @@ func waitForEmptyRemoteNodeIndexer(t *testing.T, node1 *cmd.Node) {
 	waitForCondition(t, fmt.Sprintf("Waiting for Node (%s) to disconnect from all RemoteNodes", userAgentN1), n1ValidatedN2)
 }
 
+func waitForCountRemoteNodeIndexer(t *testing.T, node1 *cmd.Node, allCount int, validatorCount int,
+	nonValidatorOutboundCount int, nonValidatorInboundCount int) {
+
+	userAgentN1 := node1.Params.UserAgent
+	rnManagerN1 := node1.Server.GetConnectionController().GetRemoteNodeManager()
+	n1ValidatedN2 := func() bool {
+		if true != checkRemoteNodeIndexerCount(rnManagerN1, allCount, validatorCount, nonValidatorOutboundCount, nonValidatorInboundCount) {
+			return false
+		}
+		return true
+	}
+	waitForCondition(t, fmt.Sprintf("Waiting for Node (%s) to have appropriate RemoteNodes counts", userAgentN1), n1ValidatedN2)
+}
+
 func checkRemoteNodeIndexerUserAgent(manager *lib.RemoteNodeManager, userAgent string, validator bool,
 	nonValidatorOutbound bool, nonValidatorInbound bool) bool {
 
@@ -409,6 +511,25 @@ func checkRemoteNodeIndexerUserAgent(manager *lib.RemoteNodeManager, userAgent s
 	return true
 }
 
+func checkRemoteNodeIndexerCount(manager *lib.RemoteNodeManager, allCount int, validatorCount int,
+	nonValidatorOutboundCount int, nonValidatorInboundCount int) bool {
+
+	if allCount != manager.GetAllRemoteNodes().Count() {
+		return false
+	}
+	if validatorCount != manager.GetValidatorIndex().Count() {
+		return false
+	}
+	if nonValidatorOutboundCount != manager.GetNonValidatorOutboundIndex().Count() {
+		return false
+	}
+	if nonValidatorInboundCount != manager.GetNonValidatorInboundIndex().Count() {
+		return false
+	}
+
+	return true
+}
+
 func checkRemoteNodeIndexerEmpty(manager *lib.RemoteNodeManager) bool {
 	if manager.GetAllRemoteNodes().Count() != 0 {
 		return false

lib/remote_node_manager.go

@@ -96,11 +96,20 @@ func (manager *RemoteNodeManager) removeRemoteNodeFromIndexer(rn *RemoteNode) {
 
 	indexer := manager.remoteNodeIndexer
 	indexer.GetAllRemoteNodes().Remove(rn.GetId())
-	if rn.GetValidatorPublicKey() != nil {
-		indexer.GetValidatorIndex().Remove(rn.GetValidatorPublicKey().Serialize())
-	}
 	indexer.GetNonValidatorOutboundIndex().Remove(rn.GetId())
 	indexer.GetNonValidatorInboundIndex().Remove(rn.GetId())
+
+	// Try to evict the remote node from the validator index. If the remote node is not a validator, then there is nothing to do.
+	if rn.GetValidatorPublicKey() == nil {
+		return
+	}
+	// Only remove from the validator index if the fetched remote node is the same as the one we are trying to remove.
+	// Otherwise, we could have a fun edge-case where a duplicated validator connection ends up removing an
+	// existing validator connection from the index.
+	fetchedRn, ok := indexer.GetValidatorIndex().Get(rn.GetValidatorPublicKey().Serialize())
+	if ok && fetchedRn.GetId() == rn.GetId() {
+		indexer.GetValidatorIndex().Remove(rn.GetValidatorPublicKey().Serialize())
+	}
 }
 
 func (manager *RemoteNodeManager) SendMessage(rn *RemoteNode, desoMessage DeSoMessage) error {
@@ -120,6 +129,10 @@ func (manager *RemoteNodeManager) CreateValidatorConnection(netAddr *wire.NetAdd
 		return fmt.Errorf("RemoteNodeManager.CreateValidatorConnection: netAddr or public key is nil")
 	}
 
+	if _, ok := manager.GetValidatorIndex().Get(publicKey.Serialize()); ok {
+		return fmt.Errorf("RemoteNodeManager.CreateValidatorConnection: RemoteNode already exists for public key: %v", publicKey)
+	}
+
 	remoteNode := manager.newRemoteNode(publicKey)
 	if err := remoteNode.DialPersistentOutboundConnection(netAddr); err != nil {
 		return errors.Wrapf(err, "RemoteNodeManager.CreateValidatorConnection: Problem calling DialPersistentOutboundConnection "+