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

fix: interface core test swarm peers with webrtc does not dial the correct address #3633

Conversation

vasco-santos
Copy link
Member

Error in CI: https://travis-ci.com/github/ipfs/js-ipfs/jobs/500525900

In the past webrtc did not proper support multiple listeners. This basically mean that if trying to listen on multiple addresses the webrtc sdp offers could end up with wrong signal server address throwing random errors in CI.

libp2p/js-libp2p-webrtc-star#330 fixed this and now a libp2p node allows multiple listeners. When a peer with multiple webrtc-star listeners attempts to dial another peer via a star signalling server, the signal server of the target peers is identified among the listeners and used for starting the dial.

In the test changed in this PR (note that ports might change):

nodeA: ['/ip4/127.0.0.1/tcp/53466/ws/p2p-webrtcstar/p2p/QmPqSo3Kqf9XtWG6g3WKVrx2PZ2q3BoFZssVjixaweABNW']
nodeB: [
          '/ip4/127.0.0.1/tcp/53466/ws/p2p-webrtc-star/p2p/QmNLojvkVdRTD2bmy7Yzpc7vUBv6bUDEpamrgQeRwPQAg1',
          '/ip4/127.0.0.1/tcp/53467/ws/p2p-webrtc-star/p2p/QmNLojvkVdRTD2bmy7Yzpc7vUBv6bUDEpamrgQeRwPQAg1'
]

and the test did:

await nodeA.swarm.connect(nodeB.peerId.addresses[isBrowser ? 1 : 0])

We were using nodeA to dial nodeB with an address of a signalServer it does not use, which obviously failed. The solution here is to either have nodeB dial nodeA address (has same signalServer), or we need to do in the test a match function to understand what address of nodeB we should use (with the same signalServer).

The error in CI is poor and webrtc-star should validate we actually have the listener for the signalServer and throw a proper error if not instead of the Cannot read property 'listener' of undefined. This will be fixed shortly in a new PR

@vasco-santos vasco-santos force-pushed the fix/interface-core-test-swarm-peers-with-webrtc branch from 2d4d001 to e66a651 Compare April 22, 2021 20:41
Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

LGTM.

Could you please update the libp2p-webrtc dep version wherever it appears in package.json files to make sure we always get the right patch version with the fix, then this is good to go.

@vasco-santos
Copy link
Member Author

Could you please update the libp2p-webrtc dep version wherever it appears in package.json files to make sure we always get the right patch version with the fix, then this is good to go.

done

@achingbrain achingbrain merged commit 5cf6f5b into chore/update-buffer-deps Apr 23, 2021
@achingbrain achingbrain deleted the fix/interface-core-test-swarm-peers-with-webrtc branch April 23, 2021 14:21
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