combinator: avoid returning duplicate paths

[matzf]

Duplicate paths can occur when multiple combinations of different path
segments result in identical sequences of path interfaces path after
applying short-cuts.

The old behaviour was to return all paths found. The rationale for this
was that from a crypto point of view the paths are different; this made
sense with the old path header format, where shortcuts would include a
verify-only hop field that was really separate from the effectively used
hop fields.
Now with the new path header format, the sequence of the hop fields for
such duplicate paths is really identical, the only differences are the
segment IDs and the MACs. While it's still conceivable that an AS could
misbehave and change the forwarding behaviour based on the segment IDs
or MACs, this would be really an exceptional case.
Thus, the new behaviour of the combinator is to return only one path for
each unique sequence of path interfaces found. If there are multiple
ways to construct the same sequence of path interfaces from the
available path segments, the construction with latest expiration time
will be returned.
A flag allows to still return all identical paths, so that an application
could diagnose broken paths due to such a misbehaving AS described
above. This flag is not currently exposed through sciond at the moment.

To keep this change simple, duplicates are filtered in a separate
post-processing step. In a future change, duplicates could/should be
avoided directly by reducing the available options in the graph, as we
could potentially create a large number of duplicates in wide network
topologies.

---

This change is 

[marcfrei]

Reviewed 5 of 5 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved

[scrye]

Reviewed 3 of 5 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @matzf)

a discussion (no related file):
Some typos in the commit message:

• rational -> rationale
• realy -> really
• identicaly -> identical

---

---

go/lib/infra/modules/combinator/combinator.go, line 110 at r2 (raw file):

// available options in the graph, as we could potentially create a large
// number of duplicates in wide network topologies.
func filterDuplicates(paths []Path) []Path {

Add unit tests for filterDuplicates. Use the export file pattern to unit test an unexported function. See the below for an example:

scion/go/pkg/cs/trust/grpc/export_test.go

Lines 15 to 22 in ae63a60

	package grpc
	var (
	RequestToChainQuery = requestToChainQuery
	RequestToTRCQuery = requestToTRCQuery
	ChainsToResponse = chainsToResponse
	TRCToResponse = trcToResponse
	)

---

go/lib/infra/modules/combinator/combinator.go, line 111 at r2 (raw file):

// number of duplicates in wide network topologies.
func filterDuplicates(paths []Path) []Path {

Remove newline.

[scrye]

Reviewed 1 of 5 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @matzf)

[matzf]

Reviewable status: 3 of 7 files reviewed, 3 unresolved discussions (waiting on @marcfrei and @scrye)

a discussion (no related file):

Previously, scrye (Sergiu Costea) wrote…

Some typos in the commit message:

• rational -> rationale
• realy -> really
• identicaly -> identical

Done.

---

---

go/lib/infra/modules/combinator/combinator.go, line 110 at r2 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Add unit tests for filterDuplicates. Use the export file pattern to unit test an unexported function. See the below for an example:

scion/go/pkg/cs/trust/grpc/export_test.go

Lines 15 to 22 in ae63a60

	package grpc
	var (
	RequestToChainQuery = requestToChainQuery
	RequestToTRCQuery = requestToTRCQuery
	ChainsToResponse = chainsToResponse
	TRCToResponse = trcToResponse
	)

Done.

---

go/lib/infra/modules/combinator/combinator.go, line 111 at r2 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Remove newline.

Ok.

[scrye]

Reviewed 4 of 4 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matzf)

---

go/lib/infra/modules/combinator/combinator_test.go, line 596 at r3 (raw file):

		Expected []uint32
	}{
		{

Add tests for nil input slice and empty input slice.

[matzf]

Reviewable status: 6 of 7 files reviewed, 1 unresolved discussion (waiting on @scrye)

---

go/lib/infra/modules/combinator/combinator_test.go, line 596 at r3 (raw file):

Previously, scrye (Sergiu Costea) wrote…

Add tests for nil input slice and empty input slice.

Ok, done.

[scrye]

Reviewed 1 of 1 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved

[matzf]

/rebase

[scrye]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @matzf)