Skip to content

Commit

Permalink
- Fix failure to generate all possible combinations (#3132)
Browse files Browse the repository at this point in the history
* - Fix failure to generate all possible combinations

Bug happens when the endorsement policy is set to k-out-of-n where (n - k) > 1.
For example, when there are 6 organizations and the endorsement policy is set
to majority (i.e. 4-out-of-6). The combinations calculated by chooseKoutOfN()
in original code are as below.
	[0 1 2 5], [0 1 2 5], [0 1 2 5], [0 1 3 5],
	[0 1 3 5], [0 1 4 5], [0 2 3 5], [0 2 3 5],
	[0 2 4 5], [0 3 4 5], [1 2 3 5], [1 2 3 5],
	[1 2 4 5], [1 3 4 5], [2 3 4 5]

Obviously, the function fails to find out all the possible combinations. Some of
 the combinations are overrided by child's call. This bug would trigger below
error sometimes when the alive peers are not within the above combinations.

	[discovery] chaincodeQuery -> ERRO 13a Failed constructing descriptor for
	chaincode chaincodes:<name:"XXXX" > ,: no peer combination can satisfy the
	endorsement policy

To fix it, the last line of choose() should be changed to pass a copy of
"currentSubGroup" to the recursive call instead, i.e.

	choose(n, targetAmount, i+1, append(make([]int, 0), currentSubGroup...), subGroups)

After applying the fix, the resulting combinations generated should be corrected
as below.
	[0 1 2 3], [0 1 2 4], [0 1 2 5], [0 1 3 4],
	[0 1 3 5], [0 1 4 5], [0 2 3 4], [0 2 3 5],
	[0 2 4 5], [0 3 4 5], [1 2 3 4], [1 2 3 5],
	[1 2 4 5], [1 3 4 5], [2 3 4 5]

Signed-off-by: Tim <96273851+timtim-git@users.noreply.github.com>

* - Fix failure to generate all possible combinations

Bug happens when the endorsement policy is set to k-out-of-n where (n - k) > 1.
For example, when there are 6 organizations and the endorsement policy is set
to majority (i.e. 4-out-of-6). The combinations calculated by chooseKoutOfN()
in original code are as below.
	[0 1 2 5], [0 1 2 5], [0 1 2 5], [0 1 3 5],
	[0 1 3 5], [0 1 4 5], [0 2 3 5], [0 2 3 5],
	[0 2 4 5], [0 3 4 5], [1 2 3 5], [1 2 3 5],
	[1 2 4 5], [1 3 4 5], [2 3 4 5]

Obviously, the function fails to find out all the possible combinations. Some of
 the combinations are overrided by child's call. This bug would trigger below
error sometimes when the alive peers are not within the above combinations.

	[discovery] chaincodeQuery -> ERRO 13a Failed constructing descriptor for
	chaincode chaincodes:<name:"XXXX" > ,: no peer combination can satisfy the
	endorsement policy

To fix it, the last line of choose() should be changed to pass a copy of
"currentSubGroup" to the recursive call instead, i.e.

	choose(n, targetAmount, i+1, append(make([]int, 0), currentSubGroup...), subGroups)

After applying the fix, the resulting combinations generated should be corrected
as below.
	[0 1 2 3], [0 1 2 4], [0 1 2 5], [0 1 3 4],
	[0 1 3 5], [0 1 4 5], [0 2 3 4], [0 2 3 5],
	[0 2 4 5], [0 3 4 5], [1 2 3 4], [1 2 3 5],
	[1 2 4 5], [1 3 4 5], [2 3 4 5]

Signed-off-by: Tim <96273851+timtim-git@users.noreply.github.com>

* - Update choose_test.go

Signed-off-by: Tim <96273851+timtim-git@users.noreply.github.com>

* - Fix build error

Signed-off-by: Tim <96273851+timtim-git@users.noreply.github.com>
(cherry picked from commit 21a17ee)
  • Loading branch information
timtim-git authored and mergify-bot committed Dec 23, 2021
1 parent a67a7cb commit e2d0193
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
9 changes: 8 additions & 1 deletion common/graph/choose.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,14 @@ func choose(n int, targetAmount int, i int, currentSubGroup []int, subGroups *or
return
}
// We either pick the current element
choose(n, targetAmount, i+1, append(currentSubGroup, i), subGroups)
choose(n, targetAmount, i+1, concatInts(currentSubGroup, i), subGroups)
// Or don't pick it
choose(n, targetAmount, i+1, currentSubGroup, subGroups)
}

func concatInts(a []int, elements ...int) []int {
var res []int
res = append(res, a...)
res = append(res, elements...)
return res
}
30 changes: 30 additions & 0 deletions common/graph/choose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ SPDX-License-Identifier: Apache-2.0
package graph

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -24,3 +25,32 @@ func TestCombinationsExceed(t *testing.T) {
// N < K returns false
assert.False(t, CombinationsExceed(20, 30, 0))
}

func TestChooseKoutOfN(t *testing.T) {
expectedSets := indiceSets{
&indiceSet{[]int{0, 1, 2, 3}},
&indiceSet{[]int{0, 1, 2, 4}},
&indiceSet{[]int{0, 1, 2, 5}},
&indiceSet{[]int{0, 1, 3, 4}},
&indiceSet{[]int{0, 1, 3, 5}},
&indiceSet{[]int{0, 1, 4, 5}},
&indiceSet{[]int{0, 2, 3, 4}},
&indiceSet{[]int{0, 2, 3, 5}},
&indiceSet{[]int{0, 2, 4, 5}},
&indiceSet{[]int{0, 3, 4, 5}},
&indiceSet{[]int{1, 2, 3, 4}},
&indiceSet{[]int{1, 2, 3, 5}},
&indiceSet{[]int{1, 2, 4, 5}},
&indiceSet{[]int{1, 3, 4, 5}},
&indiceSet{[]int{2, 3, 4, 5}},
}
require.Equal(t, indiceSetsToStrings(expectedSets), indiceSetsToStrings(chooseKoutOfN(6, 4)))
}

func indiceSetsToStrings(sets indiceSets) []string {
var res []string
for _, set := range sets {
res = append(res, fmt.Sprintf("%v", set.indices))
}
return res
}

0 comments on commit e2d0193

Please sign in to comment.