Multiple services are encoded within did:peer:2 incorrectly

[spivachuk]

According to the recent clarification in Peer DID Method Specification, multiple services must be encoded individually:

When encoding multiple services, they MUST be encoded individually and concatenated together. The services MUST NOT be encoded as a JSON list.

Please also see the example in the end of the section. The clarification was made in decentralized-identity/peer-did-method-spec@49a7c72.

However, in the latest release of Aries Framework JavaScript (v0.4.2) multiple services are encoded within did:peer:2 all together as a JSON array:
https://github.com/openwallet-foundation/agent-framework-javascript/blob/v0.4.2/packages/core/src/modules/dids/methods/peer/peerDidNumAlgo2.ts#L141

This should be fixed in accordance with the latest specification.

[swcurran]

FYI @dbluhm

[dbluhm]

Another note on did:peer:2 resolution. Here's an example doc resolved by Credo:

{
  "@context": [
    "https://w3id.org/did/v1"
  ],
  "id": "did:peer:2.Vz6Mkh95XxQsoAvJBrDUc6UfuwjTZZaeuPzPVDoK76EpwCpQ9.SeyJpZCI6IiNkaWRjb21tLTAiLCJ0IjoiZGlkLWNvbW11bmljYXRpb24iLCJwcmlvcml0eSI6MCwicmVjaXBpZW50S2V5cyI6WyIja2V5LTEiXSwiciI6W10sInMiOiJodHRwOi8vYWNhcHk6MzAwMCJ9",
  "service": [
    {
      "id": "did:peer:2.Vz6Mkh95XxQsoAvJBrDUc6UfuwjTZZaeuPzPVDoK76EpwCpQ9.SeyJpZCI6IiNkaWRjb21tLTAiLCJ0IjoiZGlkLWNvbW11bmljYXRpb24iLCJwcmlvcml0eSI6MCwicmVjaXBpZW50S2V5cyI6WyIja2V5LTEiXSwiciI6W10sInMiOiJodHRwOi8vYWNhcHk6MzAwMCJ9#did-communication-0",
      "type": "did-communication",
      "priority": 0,
      "recipientKeys": [
        "#key-1"
      ],
      "routingKeys": [],
      "serviceEndpoint": "http://acapy:3000"
    }
  ],
  "authentication": [
    {
      "id": "did:peer:2.Vz6Mkh95XxQsoAvJBrDUc6UfuwjTZZaeuPzPVDoK76EpwCpQ9.SeyJpZCI6IiNkaWRjb21tLTAiLCJ0IjoiZGlkLWNvbW11bmljYXRpb24iLCJwcmlvcml0eSI6MCwicmVjaXBpZW50S2V5cyI6WyIja2V5LTEiXSwiciI6W10sInMiOiJodHRwOi8vYWNhcHk6MzAwMCJ9#6Mkh95XxQsoAvJBrDUc6UfuwjTZZaeuPzPVDoK76EpwCpQ9",
      "type": "Ed25519VerificationKey2018",
      "controller": "did:peer:2.Vz6Mkh95XxQsoAvJBrDUc6UfuwjTZZaeuPzPVDoK76EpwCpQ9.SeyJpZCI6IiNkaWRjb21tLTAiLCJ0IjoiZGlkLWNvbW11bmljYXRpb24iLCJwcmlvcml0eSI6MCwicmVjaXBpZW50S2V5cyI6WyIja2V5LTEiXSwiciI6W10sInMiOiJodHRwOi8vYWNhcHk6MzAwMCJ9",
      "publicKeyBase58": "3gpVNAdMqNoijiduQui56duZk1P3z798XnQBFxrvHbcm"
    }
  ]
}

Note the id of the embedded authentication vm; the specification now dictates that the id should be #key-1.
FYI @genaris I discovered this as a result of the interop testing I messaged you about.

[TimoGlastra]

Hmm changing this will mean we'll break interop with all previous versions of AFJ that supported did:peer as they won't be able to resolve the did peer anymore.

@genaris have you thought about this? How can we best remain interoperable with outdated versions of AFJ?

TBH -- I think we should start supporting did:peer:4, but not necessarily change the did:peer:2 creation process at this point, if it will cause breakages in older implementations not aware of these changes.

I don't want that when we update our infrastructure to 0.5.0 to break interoperability with all afj wallets. It seems easier (and less prone to breakages) if ACA-Py would support both the old and new way, and maybe in the future we change it (like the multi-step community coordinated updates). @dbluhm does ACA-Py support the old did:peer:2 encoding, where multiple services are encoded together, and the key ids are different?

We haven't had interoperability between ACA-Py and AFJ for DIDExchange, but it's important to not break interoperability between AFJ-0.5 with AFJ-0.3/0.4 to gain interop with ACA-Py

[dbluhm]

did:peer:2 support was only recently added to ACA-Py and we did it (basically) after the specification updates. So ACA-Py currently only supports the new ids. The question of backwards compatibility is definitely a tricky one. I'm personally content to focus on did:peer:4 interop between ACA-Py and Credo/AFJ. I have an open PR with some fixes to ACA-Py to clear the final hurdles for OOB+DIDExchange with did:peer:4 between ACA-Py and Credo: hyperledger/aries-cloudagent-python#2748

[TimoGlastra]

That makes sense!

[swcurran]

@dbluhm -- can we (or perhaps did you?) enable ACA-Py to accept/process Credo/AFJ peer:did:2 DIDs, even if they don't match the spec? E.g. to compensate and process DIDs that use the format used in AFJ 0.3.0/0.4.0?

Agree on not breaking Credo 0.5.0/AFJ 0.3.0 interop and focusing on did:peer:4.

[dbluhm]

@swcurran I have not. It's definitely possible. I think it's a bit worse for DIDComm v2 than it would be for v1, since v2 uses verification method DID URLs as the key identifiers instead of just the base58 encoded key itself. However, even if we added support for resolving the old style did:peer:2, I don't think that would buy us DIDExchange interop with any AFJ/Credo version less than the upcoming 0.5.0 since support for did:peer:2 in DIDExchange is recently added by this PR: #1550

So I'm not sure it would buy us much. We'd have to add did:peer:1 support, I think, if we wanted that.

[swcurran]

Thanks for the assessment. Not sure I understand your last statement in your comment We'd have to add did:peer:1 support, I think, if we wanted that.. We have receiving did:peer:1 support in ACA-Py.

[dbluhm]

Yep, we receive did:peer:1, but AFJ/Credo < 0.5.0 wouldn't be able to receive a did:peer:2 or 4. Before #1550, only did:peer:1 was accepted by AFJ. So ACA-Py would have to support sending a did:peer:1 in order to successfully complete a DID Exchange with AFJ < 0.5.0.

[genaris]

Hmm changing this will mean we'll break interop with all previous versions of AFJ that supported did:peer as they won't be able to resolve the did peer anymore.

@genaris have you thought about this? How can we best remain interoperable with outdated versions of AFJ?

TBH -- I think we should start supporting did:peer:4, but not necessarily change the did:peer:2 creation process at this point, if it will cause breakages in older implementations not aware of these changes.

I don't want that when we update our infrastructure to 0.5.0 to break interoperability with all afj wallets. It seems easier (and less prone to breakages) if ACA-Py would support both the old and new way, and maybe in the future we change it (like the multi-step community coordinated updates). @dbluhm does ACA-Py support the old did:peer:2 encoding, where multiple services are encoded together, and the key ids are different?

We haven't had interoperability between ACA-Py and AFJ for DIDExchange, but it's important to not break interoperability between AFJ-0.5 with AFJ-0.3/0.4 to gain interop with ACA-Py

I agree with all of you regarding on giving the priority to did:peer:4, which will be the one we'll use by default since next major release and likely the one we'll use for DIDComm V2.

But I'm not too sure that a fix on the did:peer:2 creation would break interop with AFJ <= 0.4.2. In the in the entries loop in didToNumAlgo2DidDocument I see that it supports both array and single object for each service entry and it supports multiple individual service entries, so it should parse them and add each to the resulting DID Document. I did a quick test and seem to succeed, so I'll look a bit further so maybe a fix can fit all the cases without forcing ACA-Py to accept a legacy/undocumented encoding method.

In regards to the relative id that @dbluhm mentions, I have to look at it but I guess it does not affect the interop with other agents, does it? I mean, we are not usually exchanging decoded DID Documents, or are we?

[TimoGlastra]

But I'm not too sure that a fix on the did:peer:2 creation would break interop with AFJ <= 0.4.2

Okay that is good to hear. One change we should then probably make (for the future) is that we allow dids in the oob invitation services entry that we don't support, so that it would be possible to create an invite including did:peer:1, and did:peer:4 for backwards compatibility. This allows the wallet to adapt the did:peer protocol used to what is in the invitation and what is supports.

For now it's probably best to use a service object and let the inviter adapt to the peer did used in the request (i think you already implemented that right @genaris?)

[dbluhm]

In regards to the relative id that @dbluhm mentions, I have to look at it but I guess it does not affect the interop with other agents, does it? I mean, we are not usually exchanging decoded DID Documents, or are we?

The issue I ran into is that the DIDComm service needs to reference the verification method ID in its recipientKeys list (when using did-communication service endpoint type). This would be #key-N from ACA-Py's perspective but #6Mk... from Credo's. So Credo was failing to look up that ID at some point in the document parsing.

[genaris]

The issue I ran into is that the DIDComm service needs to reference the verification method ID in its recipientKeys list (when using did-communication service endpoint type). This would be #key-N from ACA-Py's perspective but #6Mk... from Credo's. So Credo was failing to look up that ID at some point in the document parsing.

Unlike the multiple services encoding, I think this will indeed require to change not only did:peer:2 creation but also parsing.

However, up to AFJ 0.4.2, AFAIK the only place where we are storing a did using this algorithm is in invitationDid of ConnectionRecord, which is based on the calculation on did:peer:2 from Out Of Band Invitation services (and used to look for existing connections for a given OOB Invitation). This is done here and, as funny as it sounds, due to not having fixed the "FIXME" notes, it seems it would not be affected by this change, as it is encoding keys using did:key rather than using references from decoded DID Document.

What do you think @TimoGlastra ? Am I missing anything here that can represent an issue when a "fixed" 0.5 agent interacts with an outdated one?

[TimoGlastra]

as funny as it sounds, due to not having fixed the "FIXME" notes, it seems it would not be affected by this change

Ah interesting. Weird that these were never fixed. But then we should be good yes! 🙌