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

Split CommunityDescription message (and all messages too big) #12188

Closed
MishkaRogachev opened this issue Sep 18, 2023 · 11 comments · Fixed by status-im/status-go#4285
Closed
Assignees
Labels
backend-team bug Something isn't working E:Desktop Scalability For Mass Market Issues related to make the app scalable for thousands of user at the same time refactor to analyze
Milestone

Comments

@MishkaRogachev
Copy link
Contributor

MishkaRogachev commented Sep 18, 2023

Description

At the moment, the CommunityDescription message is too large, both in terms of meaningfulness and size in bytes. In this experiment, we found that the size of this message for Status CCs reaches 2 mb.

This PR is a temporary measure until this feature is implemented. When switching to split messages I think we should remove this optimisation, but in the list of channel members we should not send the members themselves, but only their IDs.

@MishkaRogachev
Copy link
Contributor Author

MishkaRogachev commented Sep 18, 2023

Proposition

I propose to split this message into several messages broken down by meaning, but not just chunks:

  • CommunityDescription (chat identity, intro, outro, tags and etc)
  • CommumnityMembers: (members & ban list)
  • CommunityChannels (chats & categories)
  • CommunityTokens (tokens & collectibles)

I think permissions should be scattered between these categories.

@richard-ramos it will be great if we can store just one last message of each type for each community. Then we can broadcast those messages periodically in case they are got lost.

@osmaczko @mprakhov @cammellos WDYT?

@John-44 @iurimatias @jrainville I think we need this before releasing the MVP due to the risk of losing compatibility

@osmaczko
Copy link
Contributor

I like the idea.

The primary challenge I foresee is how to manage situations where one of the messages is lost. We need to determine whether we should wait for all partitions or if we can apply them separately to modify the state of the community.

Additionally, this change will enable encryption for both CommunityMembers and CommunityChannels. This ensures that they are not publicly accessible for closed communities.

I believe we need to implement this before releasing the MVP to avoid the risk of losing compatibility.

I agree. Ideally, we should refrain from supporting the legacy CommunityDescription for the sake of simplicity.

@caybro
Copy link
Member

caybro commented Sep 18, 2023

Good idea; I also thought if it would be feasible to just compress/uncompress the description; it's plain text anyway, should be able to reach a high ratio

@John-44
Copy link

John-44 commented Sep 18, 2023

@John-44 @iurimatias @jrainville I think we need this before releasing the MVP due to the risk of losing compatibility

After 0.15 is out we won't be able to make non-backwards compatible breaking changes. So @iurimatias @jrainville does this need to go on the list of must-have (release blocking) features for 0.15?

@jrainville
Copy link
Member

After 0.15 is out we won't be able to make non-backwards compatible breaking changes. So @iurimatias @jrainville does this need to go on the list of must-have (release blocking) features for 0.15?

I would say so. For 0.14 we will have a small fix that makes it possible for the Status CC community to continue working, but if there was a very big community that has lots of channels with permissions and lots of members, then we'll have the same issue happen again (the description will exceed the max number of bytes).

So yes, this is something that needs to be done before the "breaking change freeze".

@jrainville jrainville added this to the 0.15 milestone Sep 18, 2023
@jrainville jrainville added E:Desktop Scalability For Mass Market Issues related to make the app scalable for thousands of user at the same time refactor labels Sep 18, 2023
@richard-ramos
Copy link
Member

If a refactor is going to be done for community members, please do include the following:

Community members are stored like this in the community description:

map<string,CommunityMember> members = 2;

with the string key containing a 130char string with the member public key ("0x" + 128 hex characters). We should use instead the 33 bytes compressed version with no "0x" prefix to save space.

@richard-ramos
Copy link
Member

Also keep in mind that in the future the maximum size of a message might be reduced dramatically from 1MB to maybe 150kb as described in here, so we should consider looking at storing this type of large control messages outside of waku. (i.e. Codex / IPFS / Swarm / Torrent?)

@richard-ramos
Copy link
Member

@richard-ramos it will be great if we can store just one last message of each type for each community. Then we can broadcast those messages periodically in case they are got lost.

Storing only the last message of a type is not supported in the Store protocol, but you can achieve similar results via a store query:

  1. Send each message type on a separate content topic instead of only in the community topic
  2. Create a function similar to this: https://github.com/status-im/status-go/blob/develop/wakuv2/waku.go#L1164-L1180 but remove the Start and End time, pass the specific content topic you're interested into and the following option: store.WithPaging(false, 1) to order messages DESC and just return the first message.

@cammellos
Copy link
Member

@richard-ramos it will be great if we can store just one last message of each type for each community. Then we can broadcast those messages periodically in case they are got lost.

Storing only the last message of a type is not supported in the Store protocol, but you can achieve similar results via a store query:

  1. Send each message type on a separate content topic instead of only in the community topic
  2. Create a function similar to this: https://github.com/status-im/status-go/blob/develop/wakuv2/waku.go#L1164-L1180 but remove the Start and End time, pass the specific content topic you're interested into and the following option: store.WithPaging(false, 1) to order messages DESC and just return the first message.

This can't be relied on though, since anyone can publish on this topic, so you would still have to fetch until you find the one you care about, unless I am mistaking.
Good suggestion on the public key, I believe bytes is not supported as a key on protobuf, but we should move towards something on those lines.

@MishkaRogachev not sure I would go that way, the message fundamentally doesn't scale as the member list is fundamentally unbound.
I think in this cases it should be broken down at the transport layer rather. (or we can find application layer solutions, but we need to assume that CommumnityMembers will exceed 1 MB eventually.

@John-44
Copy link

John-44 commented Sep 19, 2023

Also keep in mind that in the future the maximum size of a message might be reduced dramatically from 1MB to maybe 150kb as described in here, so we should consider looking at storing this type of large control messages outside of waku. (i.e. Codex / IPFS / Swarm / Torrent?)

We will also be increasing the maximum size of binary messages that can be sent over waku via #9923 and when we do this we could perhaps extend this to split non-binary messages as well.

Agree that it's a good idea to impose a max message size at the Waku protocol level, and then at the Status protocol level we can get larger message sizes than we have today via schemes like the one described in the above issue.

@osmaczko
Copy link
Contributor

@jrainville we may want to put it back to 0.15 if this release that guarantees backward compatibility.

Other than size issue, there is an issue with data availability, CommunityDescription should not share private channels.

@jrainville jrainville changed the title Split CommunityDescription message Split CommunityDescription message (and all messages too big) Nov 6, 2023
osmaczko added a commit to status-im/status-go that referenced this issue Nov 10, 2023
osmaczko added a commit to status-im/status-go that referenced this issue Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend-team bug Something isn't working E:Desktop Scalability For Mass Market Issues related to make the app scalable for thousands of user at the same time refactor to analyze
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants