fix(network): fill same-MSP endorser slots with distinct identities

docs/configuration.md

@@ -181,8 +181,10 @@ token:
               #   - `all`: contact all the endorsers listed below.
               #   - `namespace`: fetch the real endorsement policy of the token namespace and contact a
               #     random subset of the endorsers below that satisfies it. On Fabric, the policy is
-              #     obtained via service discovery; on FabricX, via the query service. Endorsement fails
-              #     if the endorsers below cannot satisfy the namespace's policy.
+              #     obtained via service discovery; on FabricX, via the query service. A policy requiring
+              #     several signers from the same MSP is satisfied with that many *distinct* endorsers of
+              #     that MSP, never the same endorser twice. Endorsement fails if the endorsers below
+              #     cannot satisfy the namespace's policy.
               policy:
                 type: 1outn
               # A list of FSC node identifiers that must be contacted to obtain the endorsement.

docs/services/network-fabric.md

@@ -157,7 +157,9 @@ The set of endorsers to contact is selected by `fsc_endorsement.policy.type`:
   discovery (`Channel.Chaincode(namespace).Discover()`) and contact a random subset of
   the configured endorsers that satisfies it. Discovery already returns the required
   MSPs; if none of the configured endorsers can cover them, endorsement fails with an
-  error rather than falling back to a weaker policy. See
+  error rather than falling back to a weaker policy. Where the policy requires more than
+  one signer from the same MSP, each of those signer slots is filled with a *distinct*
+  configured endorser, never the same endorser twice. See
   [FabricX FSC Endorsement Service](network-fabricx.md#fsc-endorsement-service) for
   the equivalent (query-service-based) mechanism on FabricX.
 

docs/services/network-fabricx.md

@@ -244,6 +244,12 @@ On FabricX, the namespace's endorsement policy is fetched via the query service'
   onto MSP IDs, and a random subset of the configured `endorsers` that jointly satisfies
   it is selected — for example, given an `OR(Org1MSP, Org2MSP)` policy, either a
   configured Org1 endorser or a configured Org2 endorser is contacted, chosen at random.
+  A policy may require *several* signers from the same MSP (e.g.
+  `AND(Org1MSP.member, Org1MSP.member)`, meant to protect against a single misbehaving
+  endorser within that organization): each such signer slot is filled with a *distinct*
+  configured endorser of that MSP, so the policy's guarantee is never collapsed onto one
+  endorser signing twice. Duplicate entries in `endorsers` denote the same endorser and
+  count once.
 - **`threshold_rule`** (a single raw public key + signature scheme, *not* a k-of-n
   group despite the name): there is no MSP principal to satisfy, so the policy instead
   names one specific signer directly by key. The configured `endorsers` are searched for
@@ -258,7 +264,8 @@ On FabricX, the namespace's endorsement policy is fetched via the query service'
 The following are treated as hard errors, since correctness takes priority over
 availability:
 - for `msp_rule`: none of the configured `endorsers` can satisfy the namespace's policy
-  (e.g. no configured endorser belongs to a required MSP);
+  (e.g. no configured endorser belongs to a required MSP, or an MSP has fewer distinct
+  configured endorsers than the number of signers the policy requires from it);
 - for `threshold_rule`: the scheme is not `ECDSA`; the public key cannot be parsed; zero
   configured endorsers' identities carry the policy's key; or more than one *distinct*
   configured identity does (a duplicate entry for the same endorser is not an error; two

token/services/network/fabric/endorsement/fsc/selection.go

@@ -19,14 +19,27 @@ import (
 // that set, one random configured endorser belonging to it. mspOf resolves a configured
 // identity to the MSP ID it belongs to.
 //
-// It returns an error if none of the candidate sets can be fully covered by the
-// configured endorsers.
+// A candidate set may list the same MSP ID more than once: a policy such as
+// AND(Org1MSP.member, Org1MSP.member) requires that many *distinct* signers from that MSP,
+// which is exactly the property protecting against a single misbehaving endorser within the
+// organization. Every returned identity is therefore distinct by identity bytes: a slot is
+// never filled with an identity already selected for an earlier slot of the same set, and
+// duplicate entries in configured collapse to a single candidate endorser. The result always
+// has exactly as many elements as the chosen candidate set.
+//
+// A candidate set requiring more distinct signers from an MSP than there are distinct
+// configured endorsers in it cannot be satisfied and is skipped, like one naming an MSP with
+// no configured endorser at all.
+//
+// It returns an error if none of the candidate sets can be fully covered by distinct
+// configured endorsers. The error names the first MSP that blocked each candidate set.
 func SelectEndorsersForMSPSets(configured []view.Identity, mspOf func(view.Identity) (string, error), candidates [][]string) ([]view.Identity, error) {
 	if len(candidates) == 0 {
 		return nil, errors.Errorf("no candidate MSP set to satisfy the namespace endorsement policy")
 	}
 	var skipped []error
 	byMSP := make(map[string][]view.Identity)
+	seen := make(map[string]struct{}, len(configured))
 	for _, id := range configured {
 		mspID, err := mspOf(id)
 		if err != nil {
@@ -35,29 +48,68 @@ func SelectEndorsersForMSPSets(configured []view.Identity, mspOf func(view.Ident
 
 			continue
 		}
+		if _, ok := seen[string(id)]; ok {
+			// configured may legitimately list the same endorser twice; it still provides a
+			// single endorsement, so bucket it once.
+			continue
+		}
+		seen[string(id)] = struct{}{}
 
 		byMSP[mspID] = append(byMSP[mspID], id)
 	}
 
+	var setFailures []error
 	for _, idx := range rand.Perm(len(candidates)) {
-		requiredMSPIDs := candidates[idx]
-		selected := make([]view.Identity, 0, len(requiredMSPIDs))
-		satisfied := true
-		for _, mspID := range requiredMSPIDs {
-			pool := byMSP[mspID]
-			if len(pool) == 0 {
-				satisfied = false
-
-				break
-			}
-			selected = append(selected, pool[rand.Intn(len(pool))])
-		}
-		if satisfied {
+		selected, failedMSP, ok := selectDistinctForMSPSet(byMSP, candidates[idx])
+		if ok {
 			return selected, nil
 		}
+		required := 0
+		for _, id := range candidates[idx] {
+			if id == failedMSP {
+				required++
+			}
+		}
+		available := len(byMSP[failedMSP])
+		setFailures = append(setFailures, errors.Errorf("MSP [%s] requires %d distinct endorser(s) but only %d configured", failedMSP, required, available))
 	}
 
-	return nil, errors.Join(errors.Errorf("no configured endorser covers any of the [%d] policy-satisfying MSP set(s)", len(candidates)),
+	return nil, errors.Join(
+		errors.Errorf("no configured endorser covers any of the [%d] policy-satisfying MSP set(s) with a distinct endorser per required signer", len(candidates)),
+		errors.Join(setFailures...),
 		errors.Join(skipped...),
-		errors.Errorf("failed to resolve MSP"))
+		errors.Errorf("failed to resolve MSP"),
+	)
+}
+
+// selectDistinctForMSPSet fills one slot per entry of requiredMSPIDs with a random
+// configured endorser of that MSP, never reusing an identity already selected for an
+// earlier slot of the same set. It returns (selected, "", true) on success, or
+// (nil, failedMSP, false) where failedMSP is the first MSP ID whose pool was exhausted,
+// meaning the set is not coverable by distinct endorsers.
+//
+// A greedy per-slot pick is complete here: every slot requiring a given MSP ID draws from
+// the same pool, so the only way this fails is that some MSP ID appears in requiredMSPIDs
+// more times than that MSP has distinct configured endorsers — genuinely unsatisfiable
+// whatever the order of the picks. No backtracking is needed.
+func selectDistinctForMSPSet(byMSP map[string][]view.Identity, requiredMSPIDs []string) ([]view.Identity, string, bool) {
+	used := make(map[string]struct{}, len(requiredMSPIDs))
+	selected := make([]view.Identity, 0, len(requiredMSPIDs))
+	for _, mspID := range requiredMSPIDs {
+		pool := byMSP[mspID]
+		available := make([]view.Identity, 0, len(pool))
+		for _, id := range pool {
+			if _, ok := used[string(id)]; !ok {
+				available = append(available, id)
+			}
+		}
+		if len(available) == 0 {
+			return nil, mspID, false
+		}
+		id := available[rand.Intn(len(available))]
+		used[string(id)] = struct{}{}
+		selected = append(selected, id)
+	}
+
+	return selected, "", true
 }

token/services/network/fabric/endorsement/fsc/selection_test.go

@@ -123,6 +123,84 @@ func TestSelectEndorsersForMSPSets(t *testing.T) {
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), "failed to resolve MSP")
 	})
+
+	t.Run("same MSP required twice is filled with two distinct endorsers", func(t *testing.T) {
+		configured := []view.Identity{org1Endorser1, org1Endorser2}
+		candidates := [][]string{{"Org1MSP", "Org1MSP"}}
+
+		// Selection is randomized, so a single run proves little; a duplicate must never be
+		// observed, however many times the selection is repeated.
+		firstSlot := make(map[string]bool)
+		secondSlot := make(map[string]bool)
+		for range 200 {
+			selected, err := fsc.SelectEndorsersForMSPSets(configured, mspOf, candidates)
+			require.NoError(t, err)
+			require.Len(t, selected, 2)
+			require.False(t, selected[0].Equal(selected[1]), "the same endorser [%s] filled both required Org1MSP slots", selected[0])
+			firstSlot[selected[0].String()] = true
+			secondSlot[selected[1].String()] = true
+		}
+		// Both endorsers must be reachable in both slots: no fixed-order bias.
+		assert.Len(t, firstSlot, 2)
+		assert.Len(t, secondSlot, 2)
+	})
+
+	t.Run("same MSP required twice with a single endorser is unsatisfiable", func(t *testing.T) {
+		configured := []view.Identity{org1Endorser1}
+
+		_, err := fsc.SelectEndorsersForMSPSets(configured, mspOf, [][]string{{"Org1MSP", "Org1MSP"}})
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "no configured endorser covers")
+		// Error must name the short MSP and its required-vs-available counts.
+		assert.Contains(t, err.Error(), "Org1MSP")
+		assert.Contains(t, err.Error(), "requires 2")
+		assert.Contains(t, err.Error(), "only 1 configured")
+	})
+
+	t.Run("candidate not coverable by distinct endorsers falls through to the next one", func(t *testing.T) {
+		configured := []view.Identity{org1Endorser1, org2Endorser1}
+		candidates := [][]string{{"Org1MSP", "Org1MSP"}, {"Org2MSP"}}
+
+		// Candidate order is randomized, so repeat to cover the case where the
+		// non-coverable set is drawn first.
+		for range 20 {
+			selected, err := fsc.SelectEndorsersForMSPSets(configured, mspOf, candidates)
+			require.NoError(t, err)
+			require.Len(t, selected, 1)
+			assert.Equal(t, org2Endorser1.String(), selected[0].String())
+		}
+	})
+
+	t.Run("duplicate configured entries count as a single endorser", func(t *testing.T) {
+		configured := []view.Identity{org1Endorser1, org1Endorser1}
+
+		selected, err := fsc.SelectEndorsersForMSPSets(configured, mspOf, [][]string{{"Org1MSP"}})
+		require.NoError(t, err)
+		require.Len(t, selected, 1)
+		assert.Equal(t, org1Endorser1.String(), selected[0].String())
+
+		_, err = fsc.SelectEndorsersForMSPSets(configured, mspOf, [][]string{{"Org1MSP", "Org1MSP"}})
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "no configured endorser covers")
+	})
+
+	t.Run("mixed candidate with a repeated and a single MSP", func(t *testing.T) {
+		configured := []view.Identity{org1Endorser1, org1Endorser2, org2Endorser1}
+		candidates := [][]string{{"Org1MSP", "Org1MSP", "Org2MSP"}}
+
+		for range 20 {
+			selected, err := fsc.SelectEndorsersForMSPSets(configured, mspOf, candidates)
+			require.NoError(t, err)
+			require.Len(t, selected, 3)
+			assert.ElementsMatch(t, []string{"Org1MSP", "Org1MSP", "Org2MSP"}, mspIDsOf(t, mspOf, selected))
+			assert.ElementsMatch(
+				t,
+				[]string{org1Endorser1.String(), org1Endorser2.String(), org2Endorser1.String()},
+				[]string{selected[0].String(), selected[1].String(), selected[2].String()},
+			)
+		}
+	})
 }
 
 func mspIDsOf(t *testing.T, mspOf func(view.Identity) (string, error), ids []view.Identity) []string {