hyperledger / aries-rfcs

Hyperledger Aries is infrastructure for blockchain-rooted, peer-to-peer interactions
https://hyperledger.github.io/aries-rfcs/
Apache License 2.0
326 stars 217 forks source link

Special case of threading in didexchange #0023 ? #817

Closed Patrik-Stas closed 8 months ago

Patrik-Stas commented 8 months ago

Hi, I have a question about thread branching and its particular usage in did-exchange https://github.com/hyperledger/aries-rfcs/blob/main/features/0023-did-exchange/README.md

So according to the general https://github.com/hyperledger/aries-rfcs/blob/main/concepts/0008-message-id-and-threading/README.md it sounds like (and was always my understanding) that pthread is used to "bootstrap" new subprotocols from some other message (which might be part of some thread). My understanding was that only the first message in the new subprotocol needs to refer to its parent message via pthread.

However, did-exchange explicitly states that the https://didcomm.org/didexchange/1.0/complete (eg. being 2nd message in protocol sent by invitation receiver) needs to specify pthid again:

The pthid is required in this message, and must be identical to the pthid used in the request message.

swcurran commented 8 months ago

It’s not intended to be a special case, but to remind DID Exchange implementers of how threading is supposed to work. A DID Exchange “usually” (more on that below) starts from an OOB invitation message (a different protocol), so the OOB thid is the pthid in a DID Exchange request message, as the DID Exchange is the “child” protocol to the OOB. So, OOB and DID Exchange are just following the intention of 0008 in that scenario.

That said, I realize as I type this that DID Exchange can use an implicit invitation — e.g. if a party has published a DIDComm service in a published DID, anyone can use that to send a DID Exchange request message. In that case, there is no OOB invitation, and therefore no pthid needed in either the request or complete message.

Perhaps a clarification is needed.

genaris commented 8 months ago

That said, I realize as I type this that DID Exchange can use an implicit invitation — e.g. if a party has published a DIDComm service in a published DID, anyone can use that to send a DID Exchange request message. In that case, there is no OOB invitation, and therefore no pthid needed in either the request or complete message.

When dealing with implicit invitations, pthid is still used:

When a request responds to an implicit invitation, its ~thread.pthid MUST contain a DID URL that resolves to the specific service on a DID document that contains the invitation.

swcurran commented 8 months ago

Nice @genaris — I was only writing and thinking and not actually looking at the RFC (as I should have been). Thanks for the correction. So it does seem to be correct.

Patrik-Stas commented 8 months ago

Thanks @swcurran and @genaris. However I think I didn't get fully my point across - I understand reason for pthid in ttps://didcomm.org/didexchange/1.0/request message type. What I don't understand is why is required that https://didcomm.org/didexchange/1.0/complete also needs to contain pthread. It seems like redundancy, given the complete message links to the "subprotocol didexchange conversation" by specifying thid (generated upon creating ttps://didcomm.org/didexchange/1.0/request message).

I would suspect that pthid would be only used in a first message of subprotocol (here being request msg type), but not necessary all subsequent messages within that subprotocol.

swcurran commented 8 months ago

Got it — sorry for the tangent. It’s interesting (perhaps good?) and kind of suprising that it is called — but perhaps it is wrong? By calling it out, the behaviour is clear. The wording in 0008 is a bit ambiguous, so that it is not clear if the pthid should continue to be used in the messages or not. I don’t recall the specific call out was made in DID Exchange, and you are right, I’m not seeing why it would be needed/important. I would kind of expect it to be there — that the thread decorator would be moved from message to message in a protocol, but not would it would have to be there.

TelegramSam commented 8 months ago

Well, It seemed like a good idea at the time. That was added given feedback.

genaris commented 8 months ago

After looking at the history (and watching a few minutes of 2020-03-11 Aries WG call recording, starting from 59:30), it looks like this mandatory pthid in complete message has been added with the intention of using it for connection reuse:

The complete message may also be used to reuse existing connections. When an out of band invitation is received containing a public DID for which the invitee already has a connection, the invitee may jump directly to the complete message in the protocol sent over the existing connection. The pthid passed in the complete message allows the inviter to correlate the invitation with the identified existing connection and then invoke any protocols desired based on that context.

TODO: Should this message be called continue instead of complete? Does combining the ack for new connections and the continue for reuse make sense?

I guess the pthid was necessary because this 'standalone' complete message would create a new thread by its own.

Later on, this functionality went to RFC 0434's handshake-reuse messages.

swcurran commented 8 months ago

Good detective work, @genaris !! So that might be why that line is there. However, I don’t think we can remove the line and so make it optional to have or not have the pthid in the message. That would be the definition of a semver version change and require we change the protocol version, and I don’t think it is worth doing that.

So the answer is that it is a possibly unfortunate evolution of the RFC as it was initially debated, but it is there now and should be left as it is defined. When we next make a version change, we can consider removing the line, but the issue is not important enough to generated the overhead of a version change.

Hopefully that is enough information to close the issue.

Patrik-Stas commented 8 months ago

Thank you all for engaging here, especially @genaris for all the effort too dig this out, wow! 🚀

Though this is not ideal and while I'd be happy to remove pthid in complete message, I recognize everyone's now looking rather ahead at didcomm v2 rather than making minor but technically braking changes here.

However it's good to have understanding why the RFC was defined the way it is (and I am not missing something important). Again, thanks everyone.