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

fix: measure total message size #1643

Merged
merged 2 commits into from
Oct 26, 2023
Merged

Conversation

fryorcraken
Copy link
Collaborator

Network message limitations are imposed on the whole message, not just
the payload.

@github-actions
Copy link

github-actions bot commented Oct 10, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 83.85 KB (0%) 1.7 s (0%) 572 ms (+53.01% 🔺) 2.3 s
Waku Simple Light Node 264.27 KB (+0.02% 🔺) 5.3 s (+0.02% 🔺) 1.1 s (-23.78% 🔽) 6.4 s
ECIES encryption 79.46 KB (0%) 1.6 s (0%) 632 ms (+5.3% 🔺) 2.3 s
Symmetric encryption 79.47 KB (0%) 1.6 s (0%) 699 ms (+3.86% 🔺) 2.3 s
DNS discovery 111.22 KB (0%) 2.3 s (0%) 664 ms (-19.44% 🔽) 2.9 s
Privacy preserving protocols 131.77 KB (+0.01% 🔺) 2.7 s (+0.01% 🔺) 831 ms (+4.24% 🔺) 3.5 s
Light protocols 81.62 KB (+0.04% 🔺) 1.7 s (+0.04% 🔺) 490 ms (-9.16% 🔽) 2.2 s
History retrieval protocols 80.54 KB (0%) 1.7 s (0%) 375 ms (-27.44% 🔽) 2 s
Deterministic Message Hashing 5.65 KB (0%) 113 ms (0%) 87 ms (-13.8% 🔽) 200 ms

@fryorcraken fryorcraken force-pushed the fix/measure-total-message-size branch from f91c112 to d9b9709 Compare October 25, 2023 03:20
@fryorcraken fryorcraken marked this pull request as ready for review October 25, 2023 03:21
@fryorcraken fryorcraken requested a review from a team as a code owner October 25, 2023 03:21
@fryorcraken fryorcraken force-pushed the fix/measure-total-message-size branch 3 times, most recently from e7f04d6 to 3a0e2c7 Compare October 25, 2023 03:47
Network message limitations are imposed on the whole message, not just
the payload.
@fryorcraken fryorcraken force-pushed the fix/measure-total-message-size branch from 3a0e2c7 to 559082e Compare October 25, 2023 06:15
@fryorcraken
Copy link
Collaborator Author

@fbarbu15 I removed the "exact 1MB message size" tests because they are actually hard to setup as the protobuf boilerplate needs to be taken in account and not sure how useful.

Copy link
Collaborator

@danisharora099 danisharora099 left a comment

Choose a reason for hiding this comment

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

nice spot!

@weboko
Copy link
Collaborator

weboko commented Oct 26, 2023

@fryorcraken , is it that you noticed or there was an issue with that particular case?

@weboko weboko merged commit b7dc3d7 into master Oct 26, 2023
@weboko weboko deleted the fix/measure-total-message-size branch October 26, 2023 11:14
@fryorcraken
Copy link
Collaborator Author

@fryorcraken , is it that you noticed or there was an issue with that particular case?

I noticed the problem because I did change in the area for my previous PR and I recently read the RFC about the Waku Network and hence noticed the discrepancy.

danisharora099 pushed a commit that referenced this pull request Nov 2, 2023
Network message limitations are imposed on the whole message, not just
the payload.

Co-authored-by: Sasha <118575614+weboko@users.noreply.github.com>
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.

3 participants