Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

chore: use libp2p 0.28.x #3019

Merged
merged 7 commits into from
Jun 8, 2020
Merged

chore: use libp2p 0.28.x #3019

merged 7 commits into from
Jun 8, 2020

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented May 4, 2020

This PR updates libp2p usage to 0.28.x.

The biggest impact in js-ipfs is the deprecation of PeerInfo, which creates breaking changes in several parts of the libp2p API.

For more information you can check the libp2p migration guide and release notes:

Needs:

"ipfs-block-service": "^0.17.1",
"ipfs-core-utils": "^0.2.3",
"ipfs-http-client": "^44.1.0",
"ipfs-http-client": "44.0.3",
Copy link
Member Author

@vasco-santos vasco-santos May 28, 2020

Choose a reason for hiding this comment

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

This needs to be changed! When I rebased this PR with the js-ipfs repo changes from the last week, I started getting this error:

Error: Cannot find module 'ipfs-http-client/src/dht/find-peer'
Require stack:
- /Users/vsantos/work/pl/gh/ipfs/js-ipfs/node_modules/libp2p-delegated-peer-routing/src/index.js
- /Users/vsantos/work/pl/gh/ipfs/js-ipfs/packages/ipfs/src/cli/daemon.js

From my analysis so far, using the "ipfs-http-client": "^44.1.0" will install ipfs-http-client in js-ipfs/packages/ipfs/node_modules/ipfs-http-client as a link to its folder, which should be correct! However, the require in libp2p/js-libp2p-delegated-peer-routing/master/src/index.js#L4 does not work with the above error and the tests will not start to run. If I put this fixed version, the dependency will be installed in js-ipfs/node_modules/ipfs-http-client and the above error will not happen. I am not familiar with lerna dependency mgt and I could not understand yet the issue.

@achingbrain @hugomrdias do you have any idea on the issue?

Copy link
Member

Choose a reason for hiding this comment

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

For posterity, the problem here was that the delegate-* modules depend on a module that resolves to one in the /packages folder, which other modules in that folder also depend on. So lerna creates symlinks for those modules but then fails to hoist the symlink to the root node_modules folder.

This is a bit of an edge case as our dependency tree is a little wonky in this case but it also seems like a bug in lerna. The short-term solution is to add the delegate-* modules to the nohoist list in lerna.json.

@vasco-santos
Copy link
Member Author

Except for the issue regarding not finding the ipfs-http-client dependency described in the review above, everything else should be ready. I copied the ipfs-http-client dependency for manual testing, and it seems that everything else is good and ready for a review

cc @achingbrain @hugomrdias

await ipfsA.swarm.connect(ipfsB.peerId.addresses[0])
// Add peers to addressBook
ipfsA.libp2p.peerStore.addressBook.set(ipfsB.libp2p.peerId, ipfsB.libp2p.multiaddrs)
ipfsB.libp2p.peerStore.addressBook.set(ipfsA.libp2p.peerId, ipfsA.libp2p.multiaddrs)
Copy link
Member

Choose a reason for hiding this comment

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

Will this work when ipfsA or ipfsB are go-ipfs nodes?

Copy link
Member

Choose a reason for hiding this comment

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

This is why the ipfs-http-client tests are failing in CI..

@vasco-santos vasco-santos marked this pull request as ready for review June 4, 2020 16:08
@vasco-santos vasco-santos force-pushed the chore/use-libp2p-0.28 branch 5 times, most recently from 5196859 to bceed1b Compare June 4, 2020 20:27
@vasco-santos
Copy link
Member Author

FYI CI is green now, this is pending a dht fix libp2p/js-libp2p-kad-dht#194 to be merged/released and the final release of js-libp2p.

@achingbrain achingbrain merged commit 463ec66 into master Jun 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants