Add maxMessageSize option to pubsub #634
Conversation
|
Thanks! Will integrate in Waku once merged. |
libp2p/protocols/pubsub/pubsub.nim
Outdated
| @@ -106,6 +106,7 @@ type | |||
| anonymize*: bool # if we omit fromPeer and seqno from RPC messages we send | |||
| subscriptionValidator*: SubscriptionValidator # callback used to validate subscriptions | |||
| topicsHigh*: int # the maximum number of topics a peer is allowed to subscribe to | |||
| maxRecvMessageSize*: int | |||
There was a problem hiding this comment.
this needs documentation: unless all nodes in the mesh need the same message size constraint, the nodes that have a higher setting will be descored / disconnected because they'll be sending invalid messages.
I'm not sure the peer is the right granularity for this setting due to the above - instead, it seems better to set it per topic? Since the application has to know the payload kind on each topic, it will also be able to set a reasonable limit.
There was a problem hiding this comment.
this needs documentation
👍
it seems better to set it per topic
That would not be the right place to do it, we cannot "peek" to see which topics is it on
Instead, the application should add a validator per-topic, and reject too big messages here. We'll still read too-big-for-topic messages, but this way it won't be relayed at least
| @@ -106,6 +106,11 @@ type | |||
| anonymize*: bool # if we omit fromPeer and seqno from RPC messages we send | |||
| subscriptionValidator*: SubscriptionValidator # callback used to validate subscriptions | |||
| topicsHigh*: int # the maximum number of topics a peer is allowed to subscribe to | |||
| maxRecvMessageSize*: int ##\ | |||
| ## the maximum raw message size we'll globally allow | |||
| ## for finer tuning, check message size on topic validator | |||
There was a problem hiding this comment.
still needs a deeper explanation, for the risk of getting descored on networks with different limits
There was a problem hiding this comment.
But note that this limit doesn't apply to sending message, even with a low maxRecvMessageSize, you could still send big messages
|
Menduist
changed the title
| @@ -538,6 +546,7 @@ proc init*[PubParams: object | bool]( | |||
| sign: bool = true, | |||
| msgIdProvider: MsgIdProvider = defaultMsgIdProvider, | |||
| subscriptionValidator: SubscriptionValidator = nil, | |||
| maxMessageSize: int = 1024 * 1024, | |||
There was a problem hiding this comment.
where is the 1mb limit from? we used to have 64 kb - ideally we'd use the same limit as "Defaults" in other impls
There was a problem hiding this comment.
The spec half-says it, and it's the default in go-libp2p.
I would have kept it to 64 kB to avoid breaking changes, but both nim-waku and nimbus wants 1mb, so let's harmonize with the other libp2ps :)
No description provided.