Merge branch 'fix/memleak-protocol' of github.com:iotaledger/iota-cor…

…e into fix/memleak-protocol
iotaledger · Feb 28, 2024 · 076f3f7 · 076f3f7
2 parents cf3b65d + 636ded4
commit 076f3f7
Show file tree

Hide file tree

Showing 10 changed files with 156 additions and 111 deletions.
diff --git a/pkg/network/p2p/autopeering/autopeering.go b/pkg/network/p2p/autopeering/autopeering.go
@@ -3,6 +3,7 @@ package autopeering
 import (
 	"context"
 	"fmt"
+	"math/rand"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -183,19 +184,38 @@ func (m *Manager) discoveryLoop() {
 	}
 }
 
+func randomSubset[T any](slice []T, n int) []T {
+	if n >= len(slice) {
+		rand.Shuffle(len(slice), func(i, j int) {
+			slice[i], slice[j] = slice[j], slice[i]
+		})
+
+		return slice
+	}
+
+	subset := make([]T, n)
+	indices := rand.Perm(len(slice)) // Get a slice of random unique indices
+	for i := 0; i < n; i++ {
+		subset[i] = slice[indices[i]]
+	}
+
+	return subset
+}
+
 func (m *Manager) discoverAndDialPeers() {
 	autopeeringNeighbors := m.networkManager.AutopeeringNeighbors()
 	peersToFind := m.maxPeers - len(autopeeringNeighbors)
 	if peersToFind == 0 {
-		m.logger.LogDebugf("%d autopeering peers connected, not discovering new ones. (max %d)", len(autopeeringNeighbors), m.maxPeers)
+		m.logger.LogDebugf("%d autopeering neighbors connected, not discovering new ones. (max %d)", len(autopeeringNeighbors), m.maxPeers)
 		return
 	}
 
 	if peersToFind < 0 {
-		m.logger.LogDebugf("Too many autopeering peers connected %d, disconnecting some", -peersToFind)
-		for i := peersToFind; i < 0; i++ {
-			if err := m.networkManager.DropNeighbor(autopeeringNeighbors[i].Peer().ID); err != nil {
-				m.logger.LogDebugf("Failed to disconnect neighbor %s", autopeeringNeighbors[i].Peer().ID)
+		neighborsToDrop := randomSubset(autopeeringNeighbors, -peersToFind)
+		m.logger.LogDebugf("Too many autopeering neighbors connected %d, disconnecting some", len(neighborsToDrop))
+		for _, peer := range neighborsToDrop {
+			if err := m.networkManager.DropNeighbor(peer.Peer().ID); err != nil {
+				m.logger.LogDebugf("Failed to disconnect neighbor %s", peer.Peer().ID)
 			}
 		}
 
@@ -205,7 +225,7 @@ func (m *Manager) discoverAndDialPeers() {
 	findCtx, cancel := context.WithTimeout(m.ctx, 10*time.Second)
 	defer cancel()
 
-	m.logger.LogDebugf("%d autopeering peers connected. Discovering new peers for namespace %s", len(autopeeringNeighbors), m.namespace)
+	m.logger.LogDebugf("%d autopeering neighbors connected. Discovering new peers for namespace %s", len(autopeeringNeighbors), m.namespace)
 	peerChan, err := m.routingDiscovery.FindPeers(findCtx, m.namespace)
 	if err != nil {
 		m.logger.LogWarnf("Failed to find peers: %s", err)

diff --git a/pkg/protocol/commitment.go b/pkg/protocol/commitment.go
@@ -299,7 +299,8 @@ func (c *Commitment) deriveChain(parent *Commitment) func() {
 			return currentChain
 		}
 
-		// if we are not the main child of our parent, we spawn a new chain
+		// If we are not the main child of our parent, we spawn a new chain.
+		// Here we basically move commitments to a new chain if there's a fork.
 		if c != mainChild {
 			if currentChain == nil || currentChain == parentChain {
 				currentChain = c.commitments.protocol.Chains.newChain()
@@ -309,9 +310,10 @@ func (c *Commitment) deriveChain(parent *Commitment) func() {
 			return currentChain
 		}
 
-		// if we are the main child of our parent, and our chain is not the parent chain (that we are supposed to
-		// inherit), then we evict our current chain (we will spawn a new one if we ever change back to not being the
-		// main child)
+		// If we are the main child of our parent, and our chain is not the parent chain,
+		// then we inherit the parent chain and evict the current one.
+		// We will spawn a new one if we ever change back to not being the main child.
+		// Here we basically move commitments to the parent chain.
 		if currentChain != nil && currentChain != parentChain {
 			currentChain.IsEvicted.Trigger()
 		}

diff --git a/pkg/tests/confirmation_state_test.go b/pkg/tests/confirmation_state_test.go
@@ -79,7 +79,7 @@ func TestConfirmationFlags(t *testing.T) {
 		testsuite.WithProtocolParameters(ts.API.ProtocolParameters()),
 		testsuite.WithLatestCommitment(genesisCommitment),
 		testsuite.WithLatestFinalizedSlot(0),
-		testsuite.WithChainID(genesisCommitment.MustID()),
+		testsuite.WithMainChainID(genesisCommitment.MustID()),
 		testsuite.WithStorageCommitments([]*iotago.Commitment{genesisCommitment}),
 		testsuite.WithSybilProtectionCommittee(0, expectedCommittee),
 		testsuite.WithSybilProtectionOnlineCommittee(lo.Return1(lo.Return1(nodeA.Protocol.Engines.Main.Get().SybilProtection.SeatManager().CommitteeInSlot(1)).GetSeat(nodeA.Validator.AccountID))),

diff --git a/pkg/tests/protocol_engine_rollback_test.go b/pkg/tests/protocol_engine_rollback_test.go
@@ -116,7 +116,7 @@ func TestProtocol_EngineRollbackFinalization(t *testing.T) {
 			testsuite.WithProtocolParameters(ts.API.ProtocolParameters()),
 			testsuite.WithLatestCommitment(genesisCommitment),
 			testsuite.WithLatestFinalizedSlot(0),
-			testsuite.WithChainID(genesisCommitment.MustID()),
+			testsuite.WithMainChainID(genesisCommitment.MustID()),
 			testsuite.WithStorageCommitments([]*iotago.Commitment{genesisCommitment}),
 
 			testsuite.WithSybilProtectionCommittee(0, expectedCommittee),
@@ -303,7 +303,7 @@ func TestProtocol_EngineRollbackNoFinalization(t *testing.T) {
 			testsuite.WithProtocolParameters(ts.API.ProtocolParameters()),
 			testsuite.WithLatestCommitment(genesisCommitment),
 			testsuite.WithLatestFinalizedSlot(0),
-			testsuite.WithChainID(genesisCommitment.MustID()),
+			testsuite.WithMainChainID(genesisCommitment.MustID()),
 			testsuite.WithStorageCommitments([]*iotago.Commitment{genesisCommitment}),
 
 			testsuite.WithSybilProtectionCommittee(0, expectedCommittee),
@@ -497,7 +497,7 @@ func TestProtocol_EngineRollbackNoFinalizationLastSlot(t *testing.T) {
 			testsuite.WithProtocolParameters(ts.API.ProtocolParameters()),
 			testsuite.WithLatestCommitment(genesisCommitment),
 			testsuite.WithLatestFinalizedSlot(0),
-			testsuite.WithChainID(genesisCommitment.MustID()),
+			testsuite.WithMainChainID(genesisCommitment.MustID()),
 			testsuite.WithStorageCommitments([]*iotago.Commitment{genesisCommitment}),
 
 			testsuite.WithSybilProtectionCommittee(0, expectedCommittee),
@@ -691,7 +691,7 @@ func TestProtocol_EngineRollbackNoFinalizationBeforePointOfNoReturn(t *testing.T
 			testsuite.WithProtocolParameters(ts.API.ProtocolParameters()),
 			testsuite.WithLatestCommitment(genesisCommitment),
 			testsuite.WithLatestFinalizedSlot(0),
-			testsuite.WithChainID(genesisCommitment.MustID()),
+			testsuite.WithMainChainID(genesisCommitment.MustID()),
 			testsuite.WithStorageCommitments([]*iotago.Commitment{genesisCommitment}),
 
 			testsuite.WithSybilProtectionCommittee(0, expectedCommittee),