-
Notifications
You must be signed in to change notification settings - Fork 446
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: use keep-alive tag to reconnect to peers on startup #1278
fix: use keep-alive tag to reconnect to peers on startup #1278
Conversation
Instead of trying to connect to every peer in the peer store when we start a node, only connect to the peers that have been marked with a `keep-alive` tag.
src/connection-manager/index.ts
Outdated
|
||
for (const peer of await this.components.getPeerStore().all()) { | ||
const tags = await this.components.getPeerStore().getTags(peer.id) | ||
const hasKeepAlive = tags.filter(tag => tag.name === 'keep-alive').length > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you define and export the string as a constant? I think downstream users should tag via exported constants if possible to avoid typos and confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened this one: libp2p/js-libp2p-interfaces#263
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind you, we want to be careful about different modules reusing tag names - otherwise one module could decide a given peer is not important any more and remove the tag, a different module would then be rug-pulled and the bug would be quite hard to track down.
Instead of trying to connect to every peer in the peer store when we start a node, only connect to the peers that have been marked with a
keep-alive
tag.