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

Editing after review #29

Merged
merged 5 commits into from
Aug 9, 2024
Merged

Editing after review #29

merged 5 commits into from
Aug 9, 2024

Conversation

felixlinker
Copy link
Contributor

With this PR I address my comments on editing in this review: https://mailarchive.ietf.org/arch/msg/cose/b7ZLpC3U5CNhQscCi6ErDvFk9kg/

Some reasoning.

  • I replaced the terms "latest" and "previous" with "newer" and "older." This is in-line with RFC 9126 and emphasizes that consistency proofs about the past can be generated.
  • I fixed what I believed to be errors at the beginning of 5.2.1. There, it said "previous" root (I believe correct would be just any root you wish to prove inclusion for) and "tree-size-1" (which I believe should be "tree-size").
  • I removed an ambiguous occurrence of the word "signature." I think the text is clearer without it.

@@ -408,13 +408,13 @@ The cbor representation of a consistency proof for RFC9162_SHA256 is:
~~~~ cddl
consistency-proof = bstr .cbor [

Copy link
Contributor

Choose a reason for hiding this comment

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

I also believe it should be possible for a consistency proof to show consistency between two arbitrary points in the log. So the 'older' / 'newer' frazing seems like an improvement to me also

Copy link
Collaborator

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

I think the first sentence could be clearer, but the rest of these changes are improvements.

Copy link
Collaborator

@SteveLasker SteveLasker left a comment

Choose a reason for hiding this comment

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

LGTM, with, or without the nit suggestion at line 302

@OR13 OR13 merged commit 9e71356 into cose-wg:main Aug 9, 2024
1 check passed
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.

5 participants