uuid6 / uuid6-ietf-draft

Next Generation UUID Formats
https://datatracker.ietf.org/doc/draft-peabody-dispatch-new-uuid-format/
187 stars 11 forks source link

Typo: Proposed UUIDv8 implementation #46

Closed BaerMitUmlaut closed 2 years ago

BaerMitUmlaut commented 2 years ago

The proposed UUIDv8 implementation (section 4.5.4) contains a couple typos:

  1. Set 12-bit time_or_node to the 12 least significant bits from the timestamp
    • Typo, the referenced field is called time_or_seq
  2. If the state was unavailable (e.g., non-existent or corrupted) or the timestamp is greater than the current timestamp; set the 12-bit clock sequence value (seq_or_node) to 0
    • seq_or_node contains only 8 bits, not 12

Just as general feedback: UUIDv8 feels oddly specific to me when it should be the most flexible. Why must timestamp_48 be set to 0 for 32 bit timestamps? These bits could be used for a sequence counter instead. It is also odd that timestamp and sequence counter are required, however the node according to the spec is optional and can be filled with more timestamp and sequence counter data. I would have expected that UUIDv8 would just contain (in this order) timestamp, sequence counter and node, with each having a variable length greater than zero and the version and variant bits being fixed. I don't really understand why it needs to be more specific than that.

kyzer-davis commented 2 years ago

I believe I fixed those in Draft 02 which is published. However the point is moot since I planned on relaxing UUIDv8 for Draft 03 based on this comment: https://github.com/uuid6/uuid6-ietf-draft/issues/31#issuecomment-899594662

BaerMitUmlaut commented 2 years ago

The first typo is fixed, the second one isn't:

  1. If the state was unavailable (e.g., non-existent or corrupted) or the timestamp is greater than the current timestamp; set the 12 bit 8 bit clock sequence value (seq_or_node) to 0

Since that field probably won't exist anymore in the next draft, feel free to close this. Looks like you already adressed exactly what I was wondering about in that comment, looking forward to draft 03👍 Oh and fyi the README is still linking to draft 01.

kyzer-davis commented 2 years ago

Yup, I just received the power to merge PRs so the README is updated!