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

Small fix to the attestation signature language #135

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

cwarny
Copy link

@cwarny cwarny commented Mar 29, 2022

Fixing a small inconsistency in the spec regarding what needs signing in the attestation field of a message

spec/spec.md Outdated Show resolved Hide resolved
@cwarny
Copy link
Author

cwarny commented Mar 29, 2022

Pushed a few more small changes

Co-authored-by: Ted Thibodeau Jr <[email protected]>
@OR13
Copy link
Contributor

OR13 commented Aug 17, 2022

There are conflicts

2. The object ****MAY**** have a `replies` property, and if present its value ****MUST**** be an array of *Message Result Objects*{#message-results-objects}, which are constructed as follows:
1. The object ****MUST**** have a `messageId` property, and its value ****MUST**** be the stringified [Version 1 CID](https://docs.ipfs.io/concepts/content-addressing/#identifier-formats) of the associated message in the [*Request Object*](#request-object) from which it was received.
2. The object ****MUST**** have a `status` property, and its value ****MUST**** be an object composed of the following properties:
- The status object ****MUST**** have a `code` property, and its value ****MUST**** be an integer set to the [HTTP Status Code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status) appropriate for the status of the response.
- The status object ****MAY**** have a `message` property, and if present its value ****MUST**** be a string that describes a terse summary of the status. It is ****recommended**** that the implementer set the message text to the standard title of the HTTP Status Code, when a title/message has already been defined for that code.
- The status object ****MAY**** have a `text` property, and if present its value ****MUST**** be a string that describes a terse summary of the status. It is ****recommended**** that the implementer set the message text to the standard title of the HTTP Status Code, when a title/message has already been defined for that code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- The status object ****MAY**** have a `text` property, and if present its value ****MUST**** be a string that describes a terse summary of the status. It is ****recommended**** that the implementer set the message text to the standard title of the HTTP Status Code, when a title/message has already been defined for that code.
- The status object ****MAY**** have a `text` property, and if present its value ****MUST**** be a string that describes a terse summary of the status. It is ****RECOMMENDED**** that the implementer set the message text to the standard title of the HTTP Status Code, when a title/message has already been defined for that code.

@@ -490,12 +490,12 @@ Responses from Interface method invocations are JSON objects that ****MUST**** b
1. The object ****MUST**** include an `requestId` property, and its value ****MUST**** be the [[spec:rfc4122]] UUID Version 4 string from the `requestId` property of the [*Request Object*](#request-object) it is in response to.
2. The object ****MAY**** have a `status` property if an error is produced from a general request-related issue, and if present its value ****MUST**** be an object composed of the following properties:
- The status object ****MUST**** have a `code` property, and its value ****MUST**** be an integer set to the [HTTP Status Code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status) appropriate for the status of the response.
- The status object ****MAY**** have a `message` property, and if present its value ****MUST**** be a string that describes a terse summary of the status. It is ****recommended**** that the implementer set the message text to the standard title of the HTTP Status Code, when a title/message has already been defined for that code.
- The status object ****MAY**** have a `text` property, and if present its value ****MUST**** be a string that describes a terse summary of the status. It is ****recommended**** that the implementer set the message text to the standard title of the HTTP Status Code, when a title/message has already been defined for that code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- The status object ****MAY**** have a `text` property, and if present its value ****MUST**** be a string that describes a terse summary of the status. It is ****recommended**** that the implementer set the message text to the standard title of the HTTP Status Code, when a title/message has already been defined for that code.
- The status object ****MAY**** have a `text` property, and if present its value ****MUST**** be a string that describes a terse summary of the status. It is ****RECOMMENDED**** that the implementer set the message text to the standard title of the HTTP Status Code, when a title/message has already been defined for that code.

@andorsk
Copy link
Collaborator

andorsk commented Mar 22, 2023

@cwarny are you willing to make these fixes?

@andorsk andorsk added the artifact: spec intended for the actual specification label Mar 22, 2023
@andorsk
Copy link
Collaborator

andorsk commented Mar 22, 2023

Assigning you for now and we can move it off later if you are unavailable.

@andorsk andorsk added the stage: motion to close to close out in 7 days. label Jun 28, 2023
@andorsk
Copy link
Collaborator

andorsk commented Jun 28, 2023

motion to close this unless there is an intent to fix conflicts in the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
artifact: spec intended for the actual specification stage: motion to close to close out in 7 days.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants