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

Handling minor versions messages of protocols #407

Closed
swcurran opened this issue Jan 30, 2020 · 12 comments
Closed

Handling minor versions messages of protocols #407

swcurran opened this issue Jan 30, 2020 · 12 comments
Assignees
Labels
question Further information is requested

Comments

@swcurran
Copy link
Member

Yesterday's Working Group discussions about the handling of minor versions of protocol message types suggests we need to formalize the rules we want Aries agents to follow to enable a generally simpler update process when protocols change. One where we can avoid the "Community Update Process" as much as possible.

How about this approach:

  1. Minor versions can only include the addition of new fields and the continued semantic meaning of existing fields. When a change violates those conditions, a new major version is required.

  2. An Aries agent must be able to handle the receipt of any minor version within a single major version. E.g. an agent that handles the 1.3 message should be able to handle any 1.x message.

  3. Any fields not recognized in a message are silently ignored. Thus, an agent that supports version 1.2 can receive a 1.4 message and process it using the 1.2 handling, ignoring any added v1.3 and v1.4 data items. Of course, that also means ignoring fields incorrectly put into the message.

  4. If an agent chooses to respond to a discover feature message, the agent should respond with the latest version that the agent fully supports (e.g. 1.2 in the previous example).

  5. When sending a response, an Aries agent should always respond with the latest version of the message that it supports.

Aside: Note that a message received and a message sent in response will use different versions, so there can be no such concept of responding to the "same version as was sent". For example, currently in RFC 0036, all the messages are v1.0 except Proposal which is at v1.1. There is no "0036 RFC v1.1 protocol".

Are those reasonable requirements? Is there something missing in this? Is this documented already in RFCs? I confess I didn't really search for it...

Thanks

@swcurran swcurran added the question Further information is requested label Jan 30, 2020
@dhh1128
Copy link
Contributor

dhh1128 commented Jan 31, 2020

Item 1 is already documented: https://github.com/hyperledger/aries-rfcs/blob/master/concepts/0003-protocols/semver.md

Item 2 is a reasonable requirement for agent makers to impose on themselves, but is not a reasonable requirement for the community to impose on anybody. We should use "SHOULD" when describing it.

Item 3 is an absolute requirement. It is strongly implied by our documented semver rules, and it is mentioned as an explicit requirement in RFC 0011 Decorators: "Entities that handle messages should treat all unrecognized fields as valid but meaningless, and decorators are no exception. Thus, software that doesn't recognize a decorator should ignore it." I suggest that we change this language from "should" to "MUST", and that we put the same rule in the semver doc, plus in the protocol RFC near where message types are formally described, and also in RFC 0067 DIDComm Best Practices.

Items 4 and 5 are explicitly discussed in the semver doc, under "Version Negotiation". It is more nuanced than the rules you suggest. I recommend reading that content and then seeing if you still want the rules as you proposed them.

@TelegramSam
Copy link
Contributor

I agree on point 2 with @dhh1128. Agents should handle newer dot releases by ignoring unknown fields per point 3, but requiring that they support older dot releases should not be a requirement.

@swcurran
Copy link
Member Author

swcurran commented Feb 3, 2020

@TelegramSam Could you explain why an agent need not be able to handle an earlier dot release of a message? They may respond with a later version of the message, but they should be able to accept and process the earlier version, shouldn't they? Without that, independently upgrading agents becomes very difficult. Am I missing something?

@dhh1128 - thanks for the notes. As I wrote that I knew that you would have a better handle on what is already there. I'll check out the semvar RFC and think about anything else that might be needed in the notes.

@dhh1128
Copy link
Contributor

dhh1128 commented Feb 3, 2020

Could you explain why an agent need not be able to handle an earlier dot release of a message? They may respond with a later version of the message, but they should be able to accept and process the earlier version, shouldn't they?

A number of HTTP-dependent processes are written for HTTP 1.1; if you ask them to interact per HTTP 1.0 rules, they decline. HTTP 1.1 introduced keep-alives (a new header, analogous to a new DIDComm field). This is an added field, so to speak, and would require just a minor update per our semver rules, because if you send such a header to an HTTP 1.0 server, the server doesn't complain; it just ignores it. But if the server sends back an HTTP 1.0 response and closes the socket, and it's dealing with a client that really wants 1.1, there can't be a requirement that the more advanced client like it or that it continue to interact in that degraded mode. Maybe it's a client that needs to reuse sockets, so the 1.0 featureset doesn't give it enough performance.

As another example, I believe (though I could be wrong) that it is common to implement JPEG support against version 1.02 of the standard, without ever implementing support for 1.0.

It has to be a valid behavior for someone to implement 1.1 support without also implementing 1.0 support. If I were writing a client, I would prefer implementing both in most cases, but we ought not to use the word "MUST" to mandate this behavior of all DIDComm talkers. Supporting 1.7 cannot mean you are required/guaranteed to support 1.6 (thought we hope that will often be the case).

@swcurran
Copy link
Member Author

swcurran commented Feb 3, 2020 via email

@swcurran
Copy link
Member Author

I was going through this to figure out if there were changes to be made. I would like to suggest that we take a couple of the sub-documents in the 0003-protocols folder out of this RFC as follows:

  • template.md is now in the root as 000-template-protocol.md and so should be removed
  • semvar.md should be it's own RFC - the things not obvious that were discussed above would be added to that.
  • uris.md should be put into the 0020-message-types RFC - which needs to be cleaned up to remove the DID resolution text
  • roles-participants-etc.md could be left as is or better, moved into the README.md
  • state-details.md - could be left as is or better, moved into the README.md

Any objections to make those changes. Happy to do a PR, but it's a lot and wanted to be sure that people didn't object.

@TelegramSam @dhh1128

@dhh1128
Copy link
Contributor

dhh1128 commented Mar 10, 2020

The first bullet is already done in #412, because it was suggested on a community call where 412 was discussed and made a precondition of 412 being accepted. It's a little more involved than just deleting template.md; a number of hyperlinks have to be updated, and the material about adopted messages needed a new home.

Other than that, I have no objection. (The breakout into separate docs is a pattern we haven't used a lot, which may be why Stephen says putting material into the main README.md is better. However, I'm not sure we have consensus on giant README.md's being better. I am reserving judgment. Whichever path is taken, I'll cheerfully vote to merge.)

@kdenhartog
Copy link
Contributor

Item 1 is already documented: https://github.com/hyperledger/aries-rfcs/blob/master/concepts/0003-protocols/semver.md

Item 2 is a reasonable requirement for agent makers to impose on themselves, but is not a reasonable requirement for the community to impose on anybody. We should use "SHOULD" when describing it.

Item 3 is an absolute requirement. It is strongly implied by our documented semver rules, and it is mentioned as an explicit requirement in RFC 0011 Decorators: "Entities that handle messages should treat all unrecognized fields as valid but meaningless, and decorators are no exception. Thus, software that doesn't recognize a decorator should ignore it." I suggest that we change this language from "should" to "MUST", and that we put the same rule in the semver doc, plus in the protocol RFC near where message types are formally described, and also in RFC 0067 DIDComm Best Practices.

Items 4 and 5 are explicitly discussed in the semver doc, under "Version Negotiation". It is more nuanced than the rules you suggest. I recommend reading that content and then seeing if you still want the rules as you proposed them.

I'm in favor of the described changes here.

I was going through this to figure out if there were changes to be made. I would like to suggest that we take a couple of the sub-documents in the 0003-protocols folder out of this RFC as follows:

* template.md is now in the root as 000-template-protocol.md and so should be removed

* semvar.md should be it's own RFC - the things not obvious that were discussed above would be added to that.

* uris.md should be put into the 0020-message-types RFC - which needs to be cleaned up to remove the DID resolution text

* roles-participants-etc.md could be left as is or better, moved into the README.md

* state-details.md - could be left as is or better, moved into the README.md

Any objections to make those changes. Happy to do a PR, but it's a lot and wanted to be sure that people didn't object.

@TelegramSam @dhh1128

I'm also aligned with these changes.

@swcurran
Copy link
Member Author

Question about this. What should happen if I receive a given minor version, I fully support that minor version, and I discover some unexpected fields in the message? My first reaction was to say that we should silently or with a problem-report tell the other side we are ignoring those fields. Is that correct?

@dhh1128
Copy link
Contributor

dhh1128 commented Aug 12, 2020

It is ALWAYS the case, no matter what versioning stuff is happening, that unexpected fields should be ignored. Sending a problem report might be a good best practice, but IMO is not required. (I can think of some security reasons not to send one, sometimes.)

@swcurran
Copy link
Member Author

OK - so my thought was correct. Sounds good thanks. I'll probably do a clarification PR to RFC 0003 to make that clearer.

@swcurran
Copy link
Member Author

Added PR #527 to address.

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

No branches or pull requests

4 participants