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

feat: allow passing of multiple ENR URLs to DNS Discovery & dial multiple peers in parallel #1379

Merged
merged 4 commits into from
Jun 8, 2023

Conversation

danisharora099
Copy link
Collaborator

This PR addresses point 1 at #1326

@github-actions
Copy link

github-actions bot commented May 24, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 29.76 KB (+1.08% 🔺) 596 ms (+1.08% 🔺) 669 ms (-8.83% 🔽) 1.3 s
Waku Simple Light Node 297.36 KB (+0.13% 🔺) 6 s (+0.13% 🔺) 2.6 s (+26.93% 🔺) 8.6 s
ECIES encryption 28.54 KB (0%) 571 ms (0%) 761 ms (+3.68% 🔺) 1.4 s
Symmetric encryption 28.55 KB (0%) 572 ms (0%) 537 ms (-9.7% 🔽) 1.2 s
DNS discovery 108.47 KB (+0.02% 🔺) 2.2 s (+0.02% 🔺) 1.7 s (+4.29% 🔺) 3.8 s
Privacy preserving protocols 135.51 KB (+0.23% 🔺) 2.8 s (+0.23% 🔺) 1.5 s (-2.96% 🔽) 4.2 s
Light protocols 31.8 KB (+0.95% 🔺) 636 ms (+0.95% 🔺) 690 ms (-5.57% 🔽) 1.4 s
History retrieval protocols 30.42 KB (+1.01% 🔺) 609 ms (+1.01% 🔺) 575 ms (-5.78% 🔽) 1.2 s
Deterministic Message Hashing 5.78 KB (0%) 116 ms (0%) 180 ms (-22.93% 🔽) 295 ms

@@ -32,7 +32,7 @@ export interface Options {
/**
* ENR URL to use for DNS discovery
*/
enrUrl: string;
enrUrls: string[];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can do string | string[]?

Copy link
Collaborator

@fryorcraken fryorcraken left a comment

Choose a reason for hiding this comment

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

I agree with @weboko to make API still accept string as well as string[]

@fryorcraken
Copy link
Collaborator

instead of enable I'd use allow inPR title (for changelog)

Also, if you do string[] then it's a breaking change (must have ! in title) but with string|string[] it's not

@danisharora099 danisharora099 changed the title feat: enable passing of multiple ENR URLs to DNS Discovery feat: allow passing of multiple ENR URLs to DNS Discovery Jun 3, 2023
@fryorcraken fryorcraken force-pushed the feat/several-enrtree-multiaddr branch from 1153394 to b71930c Compare June 6, 2023 06:46
* ensure discovered peers are dialed in parallel

* cap parallel dials

* drop connection to bootstrap peer if >set connected

* switch to american english

* improve promises and error logging
@weboko weboko changed the title feat: allow passing of multiple ENR URLs to DNS Discovery feat: allow passing of multiple ENR URLs to DNS Discovery & dial multiple peers in parallel Jun 8, 2023
@weboko weboko merged commit f32d7d9 into master Jun 8, 2023
@weboko weboko deleted the feat/several-enrtree-multiaddr branch June 8, 2023 12:26
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