Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] clarify pickup protocol live mode delivery and acknowledge messages #105

Conversation

JamesKEbert
Copy link
Collaborator

As discussed initially at the DIDComm UG 20240408.

This PR makes clarifications to the Message Pickup v3 protocol, clarifying that when in live mode, delivery and acknowledge messages are still required. This requirement is critical to prevent messages silently never being delivered. If for instance, the mediator thinks the message has been sent over a websocket, but the recipient is not listening to that socket any more, the mediator will think the message was delivered successfully, when in fact it was not (without delivery & acknowledgement).

This is also illustrated and discussed here: hyperledger/aries-rfcs#760

A similar clarification should be made to the pickup v2 protocol in the Aries RFCs repo, including potentially this clarification (not exactly relevant to v3, as the line in question was removed in v3).

cc: @TelegramSam @genaris

@dbluhm
Copy link
Member

dbluhm commented Apr 22, 2024

This is a good clarification. Removing ambiguity both from the specification of the protocol and from the outcome of a message delivery is the right way to go, IMO. However, I think this would be considered a bit more than just a clarification, at least in terms of version of the specification. Existing v3 implementations would not necessarily be compatible, though I think this may be somewhat asymmetric. It will impact mediators more than their clients. For instance, the DIDComm Demo (https://demo.didcomm.org) would behave "correctly" whether the message was delivered as a delivery message or the raw message. The mediator it talks to, on the other hand, must be updated to send the live message wrapped in a delivery message.

In other words, I think before this clarification, "reasonable implementers" (i.e. me and my team lol) are likely to have interpreted it such that messages would be delivered without being wrapped in a delivery message. It seems like a version change should be necessary because of this.

@dbluhm
Copy link
Member

dbluhm commented Apr 22, 2024

It is also a bit unfortunate that this would result in a roughly 33% message bloat on each message delivered live to the client.

@JamesKEbert
Copy link
Collaborator Author

In other words, I think before this clarification, "reasonable implementers" (i.e. me and my team lol) are likely to have interpreted it such that messages would be delivered without being wrapped in a delivery message. It seems like a version change should be necessary because of this.

I could see that being a fair position @dbluhm--one of my worries would be creating a v4 version that would support both DIDComm v1 & v2 (as pickup v2 also needs to be clarified for DIDComm v1), or the creation of a v4 and v5. It feels a little bit clunky/confusing. But perhaps I'm just missing a clean way to do a version change.

It is also a bit unfortunate that this would result in a roughly 33% message bloat on each message delivered live to the client.

This is true--we had this debate (with you too IIRC), and we eventually decided the explicit nature was worth the additional cost. This line was even mentioned in V2 of the protocol (not sure why this line was removed in V3 though):
This method of delivery does incur an encoding cost, but is much simpler to implement and a more robust interaction.

@JamesKEbert
Copy link
Collaborator Author

I'm open to additional discussion on the delivery wrapping vs grabbing an identifier from the message to be delivered (via a hash or other method). Either way though, we need to be able to explicitly acknowledge delivery of messages.

@JamesKEbert
Copy link
Collaborator Author

Discussed on the Aries WG call 20240424 (which reformatted is a palindrome - 42424 😆)

Our discussion opted to make a new major release (4.0) for DIDComm v2, and a minor release (2.1) for DIDComm v1. We could have just made the clarification, but it will make it much clearer who has updated to the correct flow with explicit version changes. I'll make the relevant PRs for that approach soon.

Additionally, we discussed briefly the 33% message bloat issue, and Sam mentioned that this should be removed/adjusted/considered at the DIDComm layer in the next DIDComm iteration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants