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

Feature/org channels #151

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Feature/org channels #151

wants to merge 2 commits into from

Conversation

cammellos
Copy link
Contributor

@cammellos cammellos commented Sep 17, 2020

Draft for org channels

@cammellos cammellos self-assigned this Sep 17, 2020
string ens_name = 1;
string profile_picture = 2; // not a string, more complex data structure, but you get the point
string display_name = 3;
string description = 4; // this is actually unique to an organisation, and chatMessageIdentity might want to be updated, or similar

Choose a reason for hiding this comment

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

Not sure how extensive this needs to be; I imagine it can't be updated for channels that have already been created later. Other information we might want for a sorted and categorized view of chats would be:

  • date created
  • category tags
  • color code (for basic on brand styling)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can add new fields as we implement new functionalities, this is always a non-breaking change (we can't remove fields).
So for things like "Category tags"/Color code, unless we use them in the MVP, no need to add them at this stage and can always be added at a later stage (older client will ignore them, but that's expected as they would not know what to do with those fields, unless we implement the handling from the get go)

Choose a reason for hiding this comment

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

Let me think on when we would want to add these. If the data is available, Desktop client likely starts handling them as soon as possible as they're looking for a more visual and rich way to explore community chats and other (more visual real estate, lower discovery risks wrt store policies)

Given that the public key of the organisation is known, information about the channels and members can be fetched by listening to a discovery topic derived by the public key:

```
Does not matter which topic for now, as long as it does not clash with public chats.
Copy link
Contributor

@oskarth oskarth Sep 23, 2020

Choose a reason for hiding this comment

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

How can this be assured? Or is this more of a soft guarantee ("don't use a topic that corresponds to known ASCII string (e.g.)"?

What happens if someone else decided to use that topic in the network? (I assume nothing but some extra BW, just to make sure)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So a filter would be a combination of topic + encryption key.
For this channels we should not re-use the same algorithm for calculating those, mainly for bw issues.
If there's a topic only clash (different encryption keys), that's not an issue as we would have two different filters handling this.
If there's both a topic and encryption key clash (that's completely avoidable on our side), then we might start having issues, but chat-id is in the message so I believe we can still differentiate them.

Comment on lines +133 to +142
This grant MAY be attached to each message so that client can optimistically process
messages coming from this user even though the most up-to-date `OrganistationDescription` has not been fetched.

(The idea here is that we don't want to have issue with ordering of messages, for example
if I joined channel `x`, and I fetch messages from the mailserver, I might see a post from a new
user `y` before I have fetched the org description, so I don't know whether the user
is actually allowed to post. By using a grant, I can optimistically process their messages,
knowing that at some point between my latest version of the memberships and what I haven't fetched
yet, they have been added to the org. In case they have been removed afterward, I can go back and delete those
messages, but in most cases I should be fine.
Copy link
Member

Choose a reason for hiding this comment

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

I like this solution, I have a few questions.

  • When will new user y know to stop sending messages with a grant?
  • Because a grant is more available than an OrganisationDescription, is the disadvantage of using a grant all the time the additional payload to the community message over the single source OrganisationDescription? ie why not always use grants?
  • Do grants expire?

Copy link
Contributor Author

@cammellos cammellos Dec 21, 2020

Choose a reason for hiding this comment

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

When will new user y know to stop sending messages with a grant?

There's no easy way to know, so I would have the client keep grant as long as it's not invalid, but say 7 days should be plenty for now for example.

Because a grant is more available than an OrganisationDescription, is the disadvantage of using a grant all the time the additional payload to the community message over the single source OrganisationDescription? ie why not always use grants?

The issue with grants is that they are not completely authoritative and that they only reflect partial updates, so clients should only optimistically act on them, and revert if necessary.
If we were to always use grants, then it would be easy to have inconsistent state among clients (Which is always possible, but we should minimize it).

So you could see a OrganisationDescription as a checkpoint, and grants as patches essentially. Eventually we want this to be consolidated in a single OrganisationdDescription, which will "squash" the history, so that we don't waste too much bandwidth.

It is essentially a mixture between a purely event driven protocol (private group chats), which is very bandwidth heavy but has little loss of information/room for state inconsistencies, and a purely materialized view, which has high loss of information, room for state inconsistencies, but low bandwidth.

Do grants expire?

No, there's no mechanism for it

Choose a reason for hiding this comment

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

Why not sending an OrganisationDescription message every time that an update occurs (in addition to 24 hrs period)? This way, as soon as a new member joins, all the clients get informed via the new advertised OrganisationDescription.
I understand the bandwidth concern, but if the new member is to send multiple messages as soon as she joins, then the additional grant information, attached to each message, is already causing an access bandwidth which on aggregate MAY cause more bandwidth consumption than having the organization owner send one singleOrganisationDescription per member insertion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not sending an OrganisationDescription message every time that an update occurs

Sorry, it's not specified, but that's the implemented behavior (on top of resending), will update the docs

We'd still need the grant I think in case clients have partial history, as the admin might not always be online and it's possible they are not able to retrieve historic messages.

@oskarth
Copy link
Contributor

oskarth commented Mar 25, 2021

What's the state of this PR?

Co-authored-by: Franck Royer <franck@status.im>

If an organisation access is set to `ON_REQUEST` a user MUST send a
message `OrganisationRequestJoin` if they want to join the organisation.
A `chat_id` MAY also be specified if they are interested in joining a particular channel

Choose a reason for hiding this comment

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

is the association of chat_ids and subscribed members stored in the members field of OrganisationDescription? if not, I think it needs to be stored somewhere? this info would be necessary to enforce per channel access control

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.

6 participants