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

go/hidden_path_srv/internal/hpcfgreq/handler.go, line 29 at r1 (raw file):

)

type hpCfgReqHandler struct {

this can simply be called handler.

go/hidden_path_srv/internal/hpcfgreq/handler.go, line 37 at r1 (raw file):

// NewCfgReqHandler returns a hidden path configuration request handler
func NewCfgReqHandler(groups []*hiddenpath.Group, ia addr.IA) infra.Handler {

I think NewHandler would be enough, for callers it would now be hpcfgreq.NewCfgReqHandler, buthpcfgreq.NewHandler should be descriptive enough.

go/hidden_path_srv/internal/hpcfgreq/handler.go, line 89 at r1 (raw file):

	}

	reply := &path_mgmt.HPCfgReply{Cfgs: h.groupMap[snetPeer.IA]}

Hm so from the if condition above it seems that snetPeer.IA is always equal to h.localIA so I wonder why we even need a map? couldn't we just store the list for our IA?

go/hidden_path_srv/internal/hpcfgreq/handler_test.go, line 43 at r1 (raw file):

	as110 = xtest.MustParseAS("ff00:0:110")
	as111 = xtest.MustParseAS("ff00:0:111")
	ia110 = xtest.MustParseIA("1-ff00:0:110")

You could reuse this:

	ia110 = xtest.MustParseIA("1-ff00:0:110")
	ia111 = xtest.MustParseIA("1-ff00:0:111")
	as110 = ia110.A
	as111 = ia111.A

go/hidden_path_srv/internal/hpcfgreq/handler_test.go, line 69 at r1 (raw file):

	Id: hiddenpath.GroupId{
		OwnerAS: as111,
		Suffix:  0xacdc,

👍

go/hidden_path_srv/internal/hpcfgreq/handler_test.go, line 97 at r1 (raw file):

			peer := &snet.Addr{
				IA:   ia110,
				Host: addr.NewSVCUDPAppAddr(addr.SvcHPS),

Nit: It would not be the HPS but rather sciond or something that requests. I think you could leave the host part empty for this test.

Reviewable status: 1 of 3 files reviewed, 4 unresolved discussions (waiting on @lukedirtwalker)

go/hidden_path_srv/internal/hpcfgreq/handler.go, line 29 at r1 (raw file):

Done.

go/hidden_path_srv/internal/hpcfgreq/handler.go, line 37 at r1 (raw file):

Done.

go/hidden_path_srv/internal/hpcfgreq/handler.go, line 89 at r1 (raw file):

True. I added the if-condition later and forgot to remove the map.

go/hidden_path_srv/internal/hpcfgreq/handler_test.go, line 43 at r1 (raw file):

Done.

go/hidden_path_srv/internal/hpcfgreq/handler_test.go, line 69 at r1 (raw file):

:)

Reviewed 2 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved