openwallet-foundation/credo-ts

Agent endpoints and DidComm services priority

Closed this issue · 5 comments

AlexanderShenshin commented

Currently, AFJ agent with 2 endpoints specified in config creates a Did doc where second service endpoint has higher priority by default.

The reasons are:

  • DidDoc.didCommServices sorts result by highest priority value (I think it's a perfectly fine approach)
  • Did doc creation is using endpoint index in array as a value for service priority (see implementation)

It seems not very obvious and in my case I figured this out when I saw that all messages for agent are sent to WS endpoint instead of HTTP.

I also recall that in older AFJ version we had transport priority implementation that was explicit, but worked in some opposite way.

@TimoGlastra, what do you think? Should this be changed or it makes sense to keep it as it is?

genaris commented

I agree that it is not obvious so at least it should be clarified in docs and code comments so it is immediately seen when inspecting the property.

That said, I think it would be more intuitive to order them the other way around. Such change in behaviour for a config parameter I guess would be considered a breaking change so we can leave it for 0.5.0. I'd like to add also a way of specifying multiple endpoints for mediation records, where a priority strategy will be needed as well, so it could be a convenient milestone to do both.

  • 👍1
TimoGlastra commented

DidDoc.didCommServices sorts result by highest priority value (I think it's a perfectly fine approach)

I think this is incorrect. A lower priority value, means it has a higher priority. So priority 0 (default) has a higher priority that priority 2.

I think we should fix this, as I indeed noticed that our wallet now also prioritises the WS transport for sending messages, over the HTTP transport.

  • 👍1
TimoGlastra commented

From RFC 0067:

The agent is creating a new message to a connection and will use the default did-communication service definition for preparation of a message. In this case, the default did-communication service definition would be used by resolving the lowest priority service definition from the connections DID document.

genaris commented

From RFC 0067:

The agent is creating a new message to a connection and will use the default did-communication service definition for preparation of a message. In this case, the default did-communication service definition would be used by resolving the lowest priority service definition from the connections DID document.

This is a nice finding! It seems that both ACA-Py and AFGo are following the ascending order, so definitely it is not right in AFJ.

So if I understand correctly:

  • Endpoints configuration is fine, as it is ordering from highest priority to lowest. We just need to clarify in the docs this fact
  • Did Doc creation based on this configuration is also fine (priority 0 for first one, 1 for second, etc.)
  • DIDComm service lookup must be fixed in order to sort ascending
  • As this is a fix, no breaking change will be introduced

Am I right?

TimoGlastra commented

Well, sort of. The fix has breaking changes as the order of these services matters in the creation of did:peer:1 creation. And the old did to new did document process used .didCommServices, so the order was changed from the original did document.

I think for now, we should keep this 'incorrect' behaviour. As this algorithm will completely change with the update to did:peer:2 and did:peer:3

  • 👍2