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

Lower the size of maximum waku message to 125KiB #4955

Open
1 task done
osmaczko opened this issue Mar 20, 2024 · 9 comments
Open
1 task done

Lower the size of maximum waku message to 125KiB #4955

osmaczko opened this issue Mar 20, 2024 · 9 comments

Comments

@osmaczko
Copy link
Contributor

osmaczko commented Mar 20, 2024

Problem

As of now, the maximum waku message size is 1MiB. According to waku specs, the absolute maximum size should be 150KiB and the recommended one is 4KiB.

EDIT:
Even though waku specifies the maximum to 150KiB, we shouldn't go higher than 125KiB.

libp2p (which Waku is built on top of) is in the process of being optimised by ProtocolLabs and others for messages up to 125kb in size, as part of the work to implement Ethereum's DankSharding (EIP-4844)

status-im/status-desktop#9923

Implementation

diff --git a/wakuv2/common/const.go b/wakuv2/common/const.go
index a7ec19b6f..7dbbe2053 100644
--- a/wakuv2/common/const.go
+++ b/wakuv2/common/const.go
@@ -34,8 +34,8 @@ const (
 
     EnvelopeHeaderLength = 20
 
-    MaxMessageSize        = uint32(10 * 1024 * 1024) // maximum accepted size of a message.
-    DefaultMaxMessageSize = uint32(1 << 20)          // DefaultMaximumMessageSize is 1mb.
+    MaxMessageSize        = uint32(125 * 1024) // Maximum accepted size of a message is 150KiB.
+    DefaultMaxMessageSize = uint32(4 * 1024)   // DefaultMaximumMessageSize is 4KiB.
 
     ExpirationCycle   = time.Second
     TransmissionCycle = 300 * time.Millisecond

Notes

This change will result in the segmentation of most messages. The segmentation mechanism is designed to reconstruct messages with any segment size, ensuring backward compatibility with changes such as this one. Segmentation was introduced in version 2.27.0. Versions prior to this will not be able to process segmented messages.

Blockers

  • status-web must implement segmentation in order to handle segmented messages
@osmaczko
Copy link
Contributor Author

@richard-ramos, @cammellos, can you think of any other factors that should be considered before introducing this change?

@osmaczko
Copy link
Contributor Author

CC: @John-44, @jrainville, @felicio

@richard-ramos
Copy link
Member

Something to keep in mind that maximum is not applied to the WakuMessage itself but to the Gossipsub RPC message and that protobuffer contains announcements regarding the pubsub topics they wish to subscribe to or unsubscribe from, and the waku messages are wrapped in a Message protobuffer which includes also the pubsub topic on which a message is being published.

This means that the logic for segmenting messages must do so in values less than 150kb, since we need to take into account these other field, as well as the extra bytes that are added due to protobuffer serialization.

@richard-ramos
Copy link
Member

If this line is removed,

node.WithMaxMsgSize(1024 * 1024),
go-waku will use the default maximum of 150kb. Meaning that all the constants that are defined in status-go can be removed.

@osmaczko
Copy link
Contributor Author

This means that the logic for segmenting messages must do so in values less than 150kb, since we need to take into account these other field, as well as the extra bytes that are added due to protobuffer serialization.

Thanks @richard-ramos. I has been already taken into account:

func (s *MessageSender) segmentMessage(newMessage *types.NewMessage) ([]*types.NewMessage, error) {
// We set the max message size to 3/4 of the allowed message size, to leave
// room for segment message metadata.
newMessages, err := segmentMessage(newMessage, int(s.transport.MaxMessageSize()/4*3))
s.logger.Debug("message segmented", zap.Int("segments", len(newMessages)))
return newMessages, err
}

I believe 37.5KiB (25% from 150) for metadata is more than enough 🤔

@cammellos
Copy link
Member

@osmaczko I would not introduce it for the time being, in order to avoid unknowns, unless really necessary, since we have a tight release schedule

@jrainville
Copy link
Member

@osmaczko I would not introduce it for the time being, in order to avoid unknowns, unless really necessary, since we have a tight release schedule

We just discussed the same in our stand up 😄

We agree, I set it for 2.29 so that Web has time to adapt their code to support chunks and also so that we have enough time to test it.

@osmaczko osmaczko changed the title Lower the size of maximum waku message to 150KiB Lower the size of maximum waku message to 125KiB Apr 16, 2024
@jrainville jrainville removed the blocked label May 3, 2024
@cammellos
Copy link
Member

@fryorcraken what do you think about this? I think this might be adding a bit too much fuel to the fire, and might make it debugging message reliablity harder, what do you think?

@jrainville
Copy link
Member

For what it's worth, it would mostly affect community descriptions and image messages, since normal messages are smaller than that limit.
However, I'm totally fine to push this to the next milestone if you guys prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Iteration Backlog
Development

No branches or pull requests

5 participants