-
Notifications
You must be signed in to change notification settings - Fork 37
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
js-peer: add ability to connect via peerID #210
base: main
Are you sure you want to change the base?
Conversation
js-peer/src/lib/libp2p.ts
Outdated
} | ||
|
||
// Step 2: Find the peer using DHT lookup with retries | ||
let event: QueryEvent | undefined; |
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.
Instead of using the DHT service, you should use libp2p.peerRouting.findPeer(peerId)
which abstracts from the specific peerRouting implementations. This way, it can benefit from both delegated routing and the DHT.
I would also reduce the scope of this PR and not introduce the kadDHT.
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.
See https://github.com/ipfs-shipyard/www-helia-identify/blob/main/src/index.ts#L59-L86 for how to simplify.
You probably don't even need the onPorgress
hook from findPeer
.
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.
alryt will implement and fix as desired
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.
Thanks for the PR, Neha.
Before a proper review:
- Can you please run prettier on the files you edited in this PR to format changes (I've opened js-peer: add prettier for consistent styling #211 to address this). A lot of changes in this PR are code style changes adding unnecessary noise that makes reviewing harder.
- Can you please remove any commented out code that isn't relevant for this.
Roger that! @2color |
Hey, @2color |
Hey, kindly review and provide guidance--
• Connection Handling:
• Transport Protocol Validation:
• Connection Flow:
• UI Improvements:
|
@2color any updates/guidance? |
Issue: #177
Aims:
Description:
1)findByPeerId function aims to find through PeerStore or DHT Lookup
2)DHT response and peerResponse as fallback
Fix needed:
peerInfo response is retrieving a list of mutiaddrs alongwith bootstrap addrs, however on clicking connect thru either of them, an error of
NoValidAddressesError: The dial request has no valid addresses
or"DHT lookup failed"
OUTPUT from PEERINFO obj(mainly thru Known Peer, not DHT)