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

chore: increase max sub topic size to 100 #1791

Merged
merged 6 commits into from
Jan 18, 2024

Conversation

fbarbu15
Copy link
Collaborator

@fbarbu15 fbarbu15 commented Jan 15, 2024

Problem

Fixes failed test caused by this nwaku change
Updated deprecated master images for nwaku and go-waku

Merge only after above PR is merged

Copy link

github-actions bot commented Jan 15, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 58.45 KB (0%) 1.2 s (0%) 307 ms (+65.84% 🔺) 1.5 s
Waku Simple Light Node 298.73 KB (0%) 6 s (0%) 735 ms (+40.16% 🔺) 6.8 s
ECIES encryption 31.97 KB (0%) 640 ms (0%) 221 ms (+34.03% 🔺) 861 ms
Symmetric encryption 31.96 KB (0%) 640 ms (0%) 269 ms (+2.91% 🔺) 909 ms
DNS discovery 106.65 KB (0%) 2.2 s (0%) 351 ms (-21.25% 🔽) 2.5 s
Privacy preserving protocols 145.01 KB (0%) 3 s (0%) 349 ms (-6.13% 🔽) 3.3 s
Light protocols 55.87 KB (0%) 1.2 s (0%) 107 ms (-54.49% 🔽) 1.3 s
History retrieval protocols 54.72 KB (0%) 1.1 s (0%) 263 ms (+31.96% 🔺) 1.4 s
Deterministic Message Hashing 5.92 KB (0%) 119 ms (0%) 51 ms (+5.18% 🔺) 169 ms

@fbarbu15 fbarbu15 marked this pull request as ready for review January 18, 2024 14:50
@fbarbu15 fbarbu15 requested a review from a team as a code owner January 18, 2024 14:50
@danisharora099 danisharora099 merged commit 13d3d70 into master Jan 18, 2024
11 of 12 checks passed
@danisharora099 danisharora099 deleted the chore/max-sub-topic-count branch January 18, 2024 15:52
@@ -85,14 +85,14 @@ jobs:
node_with_go_waku_master:
uses: ./.github/workflows/test-node.yml
with:
nim_wakunode_image: wakuorg/go-waku:latest
nim_wakunode_image: harbor.status.im/wakuorg/go-waku:latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

@fbarbu15 why did you move it to different repo for the image?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

harbor.status.im is the official repo for the nwaku and gowaku images and from there they are replicated on docker hub
But the replication had issues in the past and docker hub images were outdated.
So for the interop testing project I was instructed to use the images from harbor.status.im because it's more likely for them to be up to date.
That's why I updated them on js-waku side as well

@@ -219,11 +220,14 @@ describe("Waku Filter V2: Subscribe", function () {
});
});

it("Subscribe to 30 topics at once and receives messages", async function () {
const topicCount = 30;
it("Subscribe to 100 topics at once and receives messages", async function () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is better to split this test in two: one for legacy versions of nwaku with testing 30 topics and another for 0.24.0 with 100 topics.

In this way it will be less misleading considering the name of the test.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

like this? #1803

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.

None yet

3 participants