From 7d33e8a8bbc0ae5613d757eb520ff2ba512593bc Mon Sep 17 00:00:00 2001
From: Andrei Smirnov <andrei@obol.tech>
Date: Mon, 10 Aug 2026 12:37:29 +0200
Subject: [PATCH] cluster: dedup definition peers by peer ID

Reject cluster definitions containing operators whose ENRs encode the
same public key. Peers previously deduplicated operators by ENR string
only, so distinct ENRs sharing a key (and thus a peer ID) passed
verification and collapsed the peer index map built during DKG setup,
causing an index out-of-range panic in newFrostP2P.

category: bug
ticket: none

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 cluster/cluster_test.go | 56 +++++++++++++++++++++++++++++++++++++++++
 cluster/definition.go   | 16 ++++++------
 2 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/cluster/cluster_test.go b/cluster/cluster_test.go
index d7142a9b2..17cfd031e 100644
--- a/cluster/cluster_test.go
+++ b/cluster/cluster_test.go
@@ -11,10 +11,12 @@ import (
 	"strings"
 	"testing"
 
+	k1 "github.com/decred/dcrd/dcrec/secp256k1/v4"
 	"github.com/stretchr/testify/require"
 
 	"github.com/obolnetwork/charon/cluster"
 	"github.com/obolnetwork/charon/eth2util"
+	"github.com/obolnetwork/charon/eth2util/enr"
 	"github.com/obolnetwork/charon/testutil"
 )
 
@@ -317,6 +319,60 @@ func TestDefinitionPeers(t *testing.T) {
 	}
 }
 
+func TestDefinitionPeersDuplicatePeerID(t *testing.T) {
+	newENR := func(key *k1.PrivateKey, opts ...enr.Option) string {
+		record, err := enr.New(key, opts...)
+		require.NoError(t, err)
+
+		return record.String()
+	}
+
+	dupKey, err := k1.GeneratePrivateKey()
+	require.NoError(t, err)
+
+	// Two distinct ENR strings encoding the same public key, so the same peer ID.
+	dupENR1 := newENR(dupKey)
+	dupENR2 := newENR(dupKey, enr.WithTCP(3610))
+	require.NotEqual(t, dupENR1, dupENR2)
+
+	uniqueENR := func() string {
+		key, err := k1.GeneratePrivateKey()
+		require.NoError(t, err)
+
+		return newENR(key)
+	}
+
+	tests := []struct {
+		name string
+		enrs []string
+	}{
+		{
+			name: "duplicate peer ids with distinct enrs",
+			enrs: []string{uniqueENR(), dupENR1, dupENR2, uniqueENR()},
+		},
+		{
+			name: "duplicate peer ids at tail",
+			enrs: []string{uniqueENR(), uniqueENR(), dupENR1, dupENR2},
+		},
+		{
+			name: "duplicate identical enrs",
+			enrs: []string{uniqueENR(), dupENR1, dupENR1, uniqueENR()},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var def cluster.Definition
+			for _, e := range tt.enrs {
+				def.Operators = append(def.Operators, cluster.Operator{ENR: e})
+			}
+
+			_, err := def.Peers()
+			require.ErrorContains(t, err, "definition contains duplicate peer ids")
+		})
+	}
+}
+
 // TestV1x11SafeSignatures tests that v1.11 supports variable-length signatures (Safe multisig).
 func TestV1x11SafeSignatures(t *testing.T) {
 	r := rand.New(rand.NewSource(1))
diff --git a/cluster/definition.go b/cluster/definition.go
index afa6eaf9b..9458d1312 100644
--- a/cluster/definition.go
+++ b/cluster/definition.go
@@ -386,14 +386,10 @@ func validateSignatureLength(version string, sig []byte, fieldName string) error
 func (d Definition) Peers() ([]p2p.Peer, error) {
 	var resp []p2p.Peer
 
-	dedup := make(map[string]bool)
-	for i, operator := range d.Operators {
-		if dedup[operator.ENR] {
-			return nil, errors.New("definition contains duplicate peer enrs", z.Str("enr", operator.ENR))
-		}
-
-		dedup[operator.ENR] = true
+	// Dedup by peer ID (not ENR string) since distinct ENRs can encode the same public key.
+	dedup := make(map[peer.ID]bool)
 
+	for i, operator := range d.Operators {
 		record, err := enr.Parse(operator.ENR)
 		if err != nil {
 			return nil, errors.Wrap(err, "decode enr", z.Str("enr", operator.ENR))
@@ -404,6 +400,12 @@ func (d Definition) Peers() ([]p2p.Peer, error) {
 			return nil, err
 		}
 
+		if dedup[p.ID] {
+			return nil, errors.New("definition contains duplicate peer ids", z.Str("enr", operator.ENR), z.Str("peer", p.Name))
+		}
+
+		dedup[p.ID] = true
+
 		resp = append(resp, p)
 	}