From f1007d611d072aed0f10889ca43fbcdb2fbc9d62 Mon Sep 17 00:00:00 2001 From: "Hayim.Shaul@ibm.com" Date: Tue, 4 Aug 2026 14:02:30 +0000 Subject: [PATCH 1/2] fix(network): fill same-MSP endorser slots with distinct identities SelectEndorsersForMSPSets sampled one configured endorser per required MSP ID of a candidate set without excluding identities already picked for an earlier slot of that same set. A namespace endorsement policy requiring two signers from one MSP - e.g. AND(Org1MSP.member, Org1MSP.member), whose principal set inquire.SatisfiedBy() reports as ["Org1MSP", "Org1MSP"] - could therefore be satisfied by asking a single endorser to sign twice, collapsing the intended 2-of-N-within-org guarantee onto a single point of trust. Per-slot sampling now excludes identities already selected for the same candidate set, so every returned identity is distinct and the result length always matches the chosen candidate set. A set requiring more distinct signers from an MSP than that MSP has configured endorsers is no longer satisfiable and is skipped, leaving the caller to try the remaining candidate sets before failing - correctness over availability, as elsewhere in this selector. Duplicate entries in the configured endorser list now collapse to a single candidate: such a duplicate denotes one endorser providing one endorsement, so counting it twice both skewed the uniform random pick and overstated how many distinct endorsers an MSP actually offers. Signed-off-by: Hayim.Shaul@ibm.com --- docs/configuration.md | 6 +- docs/services/network-fabric.md | 4 +- docs/services/network-fabricx.md | 9 ++- .../fabric/endorsement/fsc/selection.go | 68 +++++++++++++---- .../fabric/endorsement/fsc/selection_test.go | 74 +++++++++++++++++++ 5 files changed, 142 insertions(+), 19 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 436a4aeca7..c85adfa11e 100644 --- a/docs/configuration.md +++ b/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. diff --git a/docs/services/network-fabric.md b/docs/services/network-fabric.md index 5224653f30..2b88d710e6 100644 --- a/docs/services/network-fabric.md +++ b/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. diff --git a/docs/services/network-fabricx.md b/docs/services/network-fabricx.md index 584e22b3e7..f44639044d 100644 --- a/docs/services/network-fabricx.md +++ b/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 diff --git a/token/services/network/fabric/endorsement/fsc/selection.go b/token/services/network/fabric/endorsement/fsc/selection.go index f9466b8ca4..cefedc4d2a 100644 --- a/token/services/network/fabric/endorsement/fsc/selection.go +++ b/token/services/network/fabric/endorsement/fsc/selection.go @@ -19,7 +19,19 @@ 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 +// 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: 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. func SelectEndorsersForMSPSets(configured []view.Identity, mspOf func(view.Identity) (string, error), candidates [][]string) ([]view.Identity, error) { if len(candidates) == 0 { @@ -27,6 +39,7 @@ func SelectEndorsersForMSPSets(configured []view.Identity, mspOf func(view.Ident } 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,54 @@ 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) } 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 { + if selected, ok := selectDistinctForMSPSet(byMSP, candidates[idx]); ok { return selected, nil } } - 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(skipped...), 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 ok=false if some slot cannot be filled with a +// still-unused endorser, 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, 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, false + } + id := available[rand.Intn(len(available))] + used[string(id)] = struct{}{} + selected = append(selected, id) + } + + return selected, true +} diff --git a/token/services/network/fabric/endorsement/fsc/selection_test.go b/token/services/network/fabric/endorsement/fsc/selection_test.go index 192a4f5602..23a42da2fc 100644 --- a/token/services/network/fabric/endorsement/fsc/selection_test.go +++ b/token/services/network/fabric/endorsement/fsc/selection_test.go @@ -123,6 +123,80 @@ 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") + }) + + 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 { From 11f84e9cb40fabb5f843a70f7f94662526f040cb Mon Sep 17 00:00:00 2001 From: AkramBitar Date: Tue, 11 Aug 2026 13:40:02 +0000 Subject: [PATCH 2/2] fix(network): narrow distinctness godoc and surface failing MSP in error selectDistinctForMSPSet now returns ([]view.Identity, string, bool): the string carries the first MSP ID whose pool was exhausted, so the caller can include it in the error message. The error now reads: MSP [Org1MSP] requires 2 distinct endorser(s) but only 1 configured making operator configuration errors immediately diagnosable. The SelectEndorsersForMSPSets godoc now says "distinct by identity bytes" rather than just "distinct", to match what the code actually enforces (byte equality) and distinguish it from the key-equality check in endorserForThresholdRule. The existing test for the unsatisfiable single-endorser case gains three new assertions to pin the MSP name and counts in the error text. All 14 subtests pass; make checks is clean on the affected files. Signed-off-by: Akram Bitar Signed-off-by: AkramBitar --- .../fabric/endorsement/fsc/selection.go | 42 ++++++++++++------- .../fabric/endorsement/fsc/selection_test.go | 4 ++ 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/token/services/network/fabric/endorsement/fsc/selection.go b/token/services/network/fabric/endorsement/fsc/selection.go index cefedc4d2a..a7f527b274 100644 --- a/token/services/network/fabric/endorsement/fsc/selection.go +++ b/token/services/network/fabric/endorsement/fsc/selection.go @@ -22,17 +22,17 @@ import ( // 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: 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. +// 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. +// 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") @@ -58,27 +58,41 @@ func SelectEndorsersForMSPSets(configured []view.Identity, mspOf func(view.Ident byMSP[mspID] = append(byMSP[mspID], id) } + var setFailures []error for _, idx := range rand.Perm(len(candidates)) { - if selected, ok := selectDistinctForMSPSet(byMSP, candidates[idx]); ok { + 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) with a distinct endorser per required signer", 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 ok=false if some slot cannot be filled with a -// still-unused endorser, meaning the set is not coverable by distinct endorsers. +// 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 +// 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, bool) { +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 { @@ -90,12 +104,12 @@ func selectDistinctForMSPSet(byMSP map[string][]view.Identity, requiredMSPIDs [] } } if len(available) == 0 { - return nil, false + return nil, mspID, false } id := available[rand.Intn(len(available))] used[string(id)] = struct{}{} selected = append(selected, id) } - return selected, true + return selected, "", true } diff --git a/token/services/network/fabric/endorsement/fsc/selection_test.go b/token/services/network/fabric/endorsement/fsc/selection_test.go index 23a42da2fc..3cd74be336 100644 --- a/token/services/network/fabric/endorsement/fsc/selection_test.go +++ b/token/services/network/fabric/endorsement/fsc/selection_test.go @@ -152,6 +152,10 @@ func TestSelectEndorsersForMSPSets(t *testing.T) { 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) {