Add DidComm Discovery Protocol Query/Disclosure message #71
Conversation
| var acceptProfiles []string | ||
| for _, p := range r.packers { | ||
| profiles := p.GetSupportedProfiles() | ||
| acceptProfiles = append(acceptProfiles, profiles...) |
There was a problem hiding this comment.
Is it a chance that two packers would have profiles with the same name and we would return duplicate entries?
There was a problem hiding this comment.
It shouldn't be that way; otherwise, the first packer that matches the envelope would handle the pack()
protocol/accept.go
Outdated
|
|
||
| const ( | ||
| // ProtocolVersionV1 is a V1 version of the protocol used in the accept header | ||
| ProtocolVersionV1 AcceptProtocolVersion = "iden3comm/v1" |
There was a problem hiding this comment.
IMHO V duplicates the Version part. I would rename it either to ProtocolV1 or ProtocolVersion1.
packers/anoncrypt.go
Outdated
| supportedAlgorithms := p.getSupportedAlgorithms() | ||
| algSupported := len(parsedProfile.AcceptAnoncryptAlgorithms) == 0 | ||
| if !algSupported { | ||
| for _, alg := range parsedProfile.AcceptAnoncryptAlgorithms { | ||
| for _, supportedAlg := range supportedAlgorithms { | ||
| if string(alg) == supportedAlg { | ||
| algSupported = true | ||
| break | ||
| } | ||
| } | ||
| if algSupported { | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return algSupported |
There was a problem hiding this comment.
a little less indentation and state variables
| supportedAlgorithms := p.getSupportedAlgorithms() | |
| algSupported := len(parsedProfile.AcceptAnoncryptAlgorithms) == 0 | |
| if !algSupported { | |
| for _, alg := range parsedProfile.AcceptAnoncryptAlgorithms { | |
| for _, supportedAlg := range supportedAlgorithms { | |
| if string(alg) == supportedAlg { | |
| algSupported = true | |
| break | |
| } | |
| } | |
| if algSupported { | |
| break | |
| } | |
| } | |
| } | |
| return algSupported | |
| if len(parsedProfile.AcceptJwsAlgorithms) == 0 { | |
| return true | |
| } | |
| supportedAlgorithms := p.getSupportedAlgorithms() | |
| for _, alg := range parsedProfile.AcceptJwsAlgorithms { | |
| for _, supportedAlg := range supportedAlgorithms { | |
| if string(alg) == supportedAlg { | |
| return true | |
| } | |
| } | |
| } | |
| return false |
protocol/accept.go
Outdated
|
|
||
| // AcceptProfile is a struct that represents the accept header | ||
| type AcceptProfile struct { | ||
| ProtocolVersion AcceptProtocolVersion |
There was a problem hiding this comment.
I'm a little confused about naming. Correct me if I'm wrong.
The AcceptProtocolVersion type denotes the version of the protocol. Maybe it would be better to name this type ProtocolVersion instead of AcceptProtocolVersion. Moreover... as the package name is protocol, better to name this type just Version. Then it could be used in other packages as protocol.Version instead of protocol.ProtocolVersion.
On the other side, the field name ProtocolVersion stands for version that is accepted by this profile. So the field name is good candidate to be named as AcceptedVersion instead of ProtocolVersion.
protocol/accept.go
Outdated
| ) | ||
|
|
||
| // AcceptJwzAlgorithms is a type of accepted proving algorithms | ||
| type AcceptJwzAlgorithms string |
There was a problem hiding this comment.
I have the same confusion here as with the protocol. The type stands for Algorithm name. I don't see the reason to prepend the type name with Accept prefix. In the field name of this type it is OK to name the field AcceptJwsAlgorithms of type []JwsAlgorithms
And the same with AcceptAnoncryptAlgorithms.
There was a problem hiding this comment.
I tried to emphasize that this type is used in the Accept header, but you might be right; this algorithm could be a generic one.
No description provided.