decentralized-identity / didcomm-demo

In browser DIDComm v2 demo.
http://demo.didcomm.org/
MIT License
13 stars 5 forks source link

Double checking the structure of the encoded services #2

Open FabioPinheiro opened 9 months ago

FabioPinheiro commented 9 months ago

Here a example of the did created did:peer:2.Vz6MkfFh6mePrDSEW5VH9teoibsMG5JrvMovH3o3kYCq84Fnq.Ez6LSmZebuvzRL1XAysKRAjCW4ecq4LE2AawXieHF5sXyG9NU.SeyJ0IjoiZG0iLCJzIjp7InVyaSI6ImRpZDpwZWVyOjIuRXo2TFNlR1NCeTdOUGQ5NHRHaHV0RzNwUlNSa3VVRTFVSlduNU1Rd1dFZmtGd1dLNC5WejZNa280WFl5c3l5dHZZM3hiaEY1UWE3WndGcXVHUldwUXp0Z0ZxQm40TnNnc05aLlNXM3NpZENJNkltUnRJaXdpY3lJNkltaDBkSEJ6T2k4dlp6WnpkSGcwYzJveWFDNWxlR1ZqZFhSbExXRndhUzUxY3kxbFlYTjBMVEV1WVcxaGVtOXVZWGR6TG1OdmJTOVFjbTlrTDIxbGMzTmhaMlVpTENKeUlqcGJYU3dpWVNJNld5SmthV1JqYjIxdEwzWXlJaXdpWkdsa1kyOXRiUzloYVhBeU8yVnVkajF5Wm1NeE9TSmRmU3g3SW5RaU9pSmtiU0lzSW5NaU9pSjNjem92TDJSa2JTMHlNREkxTXpjeU5qSXdMblZ6TFdWaGMzUXRNUzVsYkdJdVlXMWhlbTl1WVhkekxtTnZiUzkzY3lJc0luSWlPbHRkTENKaElqcGJJbVJwWkdOdmJXMHZkaklpTENKa2FXUmpiMjF0TDJGcGNESTdaVzUyUFhKbVl6RTVJbDE5WFEiLCJhY2NlcHQiOlsiZGlkY29tbS92MiJdfX0

The service of the did:peer decodes to:

{
  "t": "dm",
  "s": {
    "uri": "did:peer:2.Ez6LSeGSBy7NPd94tGhutG3pRSRkuUE1UJWn5MQwWEfkFwWK4.Vz6Mko4XYysyytvY3xbhF5Qa7ZwFquGRWpQztgFqBn4NsgsNZ.SW3sidCI6ImRtIiwicyI6Imh0dHBzOi8vZzZzdHg0c2oyaC5leGVjdXRlLWFwaS51cy1lYXN0LTEuYW1hem9uYXdzLmNvbS9Qcm9kL21lc3NhZ2UiLCJyIjpbXSwiYSI6WyJkaWRjb21tL3YyIiwiZGlkY29tbS9haXAyO2Vudj1yZmMxOSJdfSx7InQiOiJkbSIsInMiOiJ3czovL2RkbS0yMDI1MzcyNjIwLnVzLWVhc3QtMS5lbGIuYW1hem9uYXdzLmNvbS93cyIsInIiOltdLCJhIjpbImRpZGNvbW0vdjIiLCJkaWRjb21tL2FpcDI7ZW52PXJmYzE5Il19XQ",
    "accept": ["didcomm/v2"]
  }
}

The service of the Mediator decodes to:

[
  {
    "t": "dm",
    "s": "https://g6stx4sj2h.execute-api.us-east-1.amazonaws.com/Prod/message",
    "r": [],
    "a": [
      "didcomm/v2",
      "didcomm/aip2;env=rfc19"
    ]
  },
  {
    "t": "dm",
    "s": "ws://ddm-2025372620.us-east-1.elb.amazonaws.com/ws",
    "r": [],
    "a": [
      "didcomm/v2",
      "didcomm/aip2;env=rfc19"
    ]
  }
]

My question is if the structure valid according to the specs? https://identity.foundation/peer-did-method-spec/

FabioPinheiro commented 9 months ago

Here a example from the did:peer specs {"t":"dm","s":"https://example.com/endpoint","r":["did:example:somemediator#somekey"],"a":["didcomm/v2","didcomm/aip2;env=rfc587"]}

It's using a flat structure: the field serviceEndpoint is not a json object with with routingKeys accept fields inside.

The did:peer specs is still following a early draft of the DIDComm specs.

dbluhm commented 9 months ago

Yeah, we noted this too. The didcomm-rust library that we're using a WASM build of for the demo expects the serviceEndpoint to be an object while the didcomm-python library we're using on the mediator end expects the flat service structure.

I think the structure that we should consider correct is the one outlined in the DIDComm v2 spec: https://identity.foundation/didcomm-messaging/spec/v2.0/#service-endpoint

We could potentially keep the flat structure and then expect on resolution that the did:peer:2 service endpoints are mapped onto the expected structure for DIDComm v2 but I get the feeling this would be needlessly complicated and we should just update the examples in the did:peer:2 spec to match the DIDComm v2 spec.

Given the discrepancies between the two libraries interacting in this demo, we've implemented a transform step on both ends to turn the value into what the library is expecting but we intend to push for updates to the didcomm-python library so we can be consistent.

Thoughts, @TelegramSam?

FabioPinheiro commented 9 months ago

I'm in favor of updating the specs of did:peer. But we also need to update the libs.

But you are using both structure formats at the same time. I see the method transformOldServiceStyleToNew in the did resolver of this. Which make even harder to make everything interoperable.

I'm changing my implementation of did:peer to integrate this demo with mine https://did.fmgp.app/. But I don't want to lose interoperability with other integrations.

dbluhm commented 9 months ago

Yeah, of course, I don't want this demo to cause a propagation of spec-divergent implementations -- we're not necessarily the right ones to implement library changes in all cases but we have scheduled as a high priority for our current sprint to raise issues in as many libraries that we can find and to bring up the topic in the DIDComm UG meeting and any other relevant venues.

We plan to update our mediator as soon as we can as well.

TheTechmage commented 7 months ago

Hey @FabioPinheiro, apologies that as long as it has, but I've corrected the DID used by the mediator to follow more in-line with the spec. I'm still working to update the underlying libraries used in various locations, but with #66 merged in, we should no longer be sending out the invalid DID services.

Does the new did address your concerns?

FabioPinheiro commented 7 months ago

We were also trying to track the changes of did:peer:2 in https://github.com/decentralized-identity/peer-did-method-spec/issues/64