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

feat(62/STATUS-Payload): adds the status protocol payload spec #612

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

0x-r4bbit
Copy link
Contributor

@0x-r4bbit 0x-r4bbit commented Sep 1, 2023

No description provided.

Copy link
Contributor

@rymnc rymnc left a comment

Choose a reason for hiding this comment

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

Phenomenal PR, thanks!!
left a few notes to cut down scope of description :)
boom-mind-blown (1)


This specification describes how the payload of each message in Status looks
like.
It is primarily centered around chat and chat-related use cases.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe refer to the status-1to1-chat rfc here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

}
```

`signature` is the bytes of the signed `SHA3-256` of the payload, signed with the key of the author of the message.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe in the security considerations we should note that this does not have sender anonymity

string chat_id = 6;

// The type of message (public/one-to-one/private-group-chat)
MessageType message_type = 7;
Copy link
Contributor

Choose a reason for hiding this comment

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

Seeing MessageType defined below, should we also do the same for ContentType?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +111 to +123
// Only local
SYSTEM_MESSAGE_GAP = 10;
CONTACT_REQUEST = 11;
DISCORD_MESSAGE = 12;
IDENTITY_VERIFICATION = 13;
// Only local
SYSTEM_MESSAGE_PINNED_MESSAGE = 14;
// Only local
SYSTEM_MESSAGE_MUTUAL_EVENT_SENT = 15;
// Only local
SYSTEM_MESSAGE_MUTUAL_EVENT_ACCEPTED = 16;
// Only local
SYSTEM_MESSAGE_MUTUAL_EVENT_REMOVED = 17;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have some more details about these types of messages?

Comment on lines +216 to +217
- Clients MUST choose a secure decoder
- Clients SHOULD strip metadata if present without parsing/decoding it
Copy link
Contributor

Choose a reason for hiding this comment

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

might be a silly question, but what constitutes a "secure decoder"? could we have an example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a silly question, and honestly, I have no idea.

| 1 | id | `string` | The id of the contact request |
| 2 | clock | `uint64` | Clock value of the message |

### CommunityRequestToJoinResponse
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicated across specs, see https://rfc.vac.dev/spec/56/#wire-format

| 5 | community_id | `bytes` | The id of the community |
| 6 | magnet_uri | `string` | The latest magnet uri of the community's archive torrent |

### CommunityRequestToLeave
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicated across specs, see https://rfc.vac.dev/spec/56/#wire-format

| 2 | id | `string` | The verification request id |


### CommunityCancelRequestToJoin
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicated across specs, see https://rfc.vac.dev/spec/56/#wire-format

Comment on lines +1049 to +1051
## Security Considerations

-
Copy link
Contributor

Choose a reason for hiding this comment

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

this inherits security considerations of the key-exchange mechanism, waku-relay, no sender anonymity, etc

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Please add a line making this explicit.

Comment on lines +1055 to +1072
### Version 0.5

Released [August 25, 2020](https://github.com/status-im/specs/commit/968fafff23cdfc67589b34dd64015de29aaf41f0)

- Added support for emoji reactions

### Version 0.4

Released [July 16, 2020](https://github.com/status-im/specs/commit/ad45cd5fed3c0f79dfa472253a404f670dd47396)

- Added support for images
- Added support for audio

### Version 0.3

Released [May 22, 2020](https://github.com/status-im/specs/commit/664dd1c9df6ad409e4c007fefc8c8945b8d324e8)

- Added language to include Waku in all relevant places
Copy link
Contributor

Choose a reason for hiding this comment

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

are these versions to be included? cc: @kaiserd

Copy link
Contributor

Choose a reason for hiding this comment

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

Not for raw RFCs. (For drafts, they are on the top of the RFC; see filter RFC)
We can remove that part.

If desired by Status, we can link to one previous status spec version though.

Copy link
Contributor

@kaiserd kaiserd left a comment

Choose a reason for hiding this comment

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

LGTM ❇️
Thank you very much for this!

Comment on lines +1049 to +1051
## Security Considerations

-
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Please add a line making this explicit.

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.

None yet

4 participants