Add p2p protocol support

[jacobheun]

This is needed so that we can switch to using p2p instead of ipfs by default.

Replaces #69, this just rebases that and updates tests from the latest master changes.
Closes #68.

[jacobheun]

The old PR has been sitting for a while, it would be great to get this merged and released soon @victorb @vmx.

[jacobheun]

@victorb @vmx any update on getting this released soon? Can I help with anything to get this done?

[vmx]

Thanks for pinging. I was waiting for @victorb. But as he seems busy and I found out that I also can do releases I'll just do it.

[vmx]

I've done a release. It should probably have been a 6.1.0 and not a 6.0.1, but as it's already out now, there's not really a point doing another one.

[jacobheun]

Woo, thank you!

[alanshaw]

Oh no, I think this might have broken all the js-ipfs browser tests 😱

[jacobheun]

@alanshaw I think the transports are doing filtering badly, they're looking specifically for /ipfs/ which is causing issues. I'm looking at getting them all upgraded to multiaddr 6.

This may cause an issue fragmenting the network though, as old nodes won't be able to interpret the p2p addresses. I'm looking into how we can make this graceful.

[alanshaw]

I will double check and I'm available if you need any reviews, or want to split the load...

[vmx]

I know too little about this. If there's a revert needed, let me know.

[jacobheun]

I hit this testing the daemon too, the node couldnt start because tcp filtered out the p2p addresses.

[alanshaw]

It's weird I'm only seeing this in the browser tests for js-ipfs...

[alanshaw]

Ok so in js-ipfs tests, libp2p-websocket-star is only being asked to filter out:

['/ip4/127.0.0.1/tcp/0/p2p/QmRdfAJKnQUQZkUe7UDgJVBLbepwSnZNF96ER3ahr2iio1']

Which doesn't match WebSocketStar in mafmt. I had originally figured it was an issue here so updated to read:

const WebSocketStar = or(
  and(WebSockets, base('p2p-websocket-star'), base('p2p')),
  and(WebSocketsSecure, base('p2p-websocket-star'), base('p2p')),
  and(WebSockets, base('p2p-websocket-star'), base('ipfs')),
  and(WebSocketsSecure, base('p2p-websocket-star'), base('ipfs')),
  and(WebSockets, base('p2p-websocket-star')),
  and(WebSocketsSecure, base('p2p-websocket-star'))
)

...which may still be necessary, but isn't the only problem.

Anyway, I need to head home and get some food, but back on it later.

---

We really need to get better at understanding the potential repercussions of releases especially at this fundamental level.

@vmx I think in this case we might be ok and not need a rollback. Looking at the dependency list for js-ipfs right now:

All still <=5.

[jacobheun]

I think one of the big problems is that the /ipfs/ proto is hardcoded in a lot of the transport logic. So if it gets an address with /p2p/ it doesn't do anything with it, or does the wrong thing (peer discovery in websocket-star is an example where this would break). The migration path for this is messy. I think ideally we'd be able to handle /p2p/ but expose /ipfs/ to give nodes some time to upgrade. Then switch over to exposing /p2p/ at a specified date.

I think right now the best thing to do is to rollback the change. This will give us time to think through the right way to handle this change without risking any unnecessary issues. Right now a lot of the js-ipfs 0.34 release dependencies are starting to move to multiaddr 6, so it will create problems there.

@alanshaw if you agree, I think we should go ahead and rollback. I can put together a better impact assessment for the change and plan for getting this out.

[jacobheun]

The protocol table order is currently the big issue, I believe. Right now p2p is set as the preferred because it comes later. This causes the addresses to get turned into p2p addresses. Flipping those would allow us to support p2p, but still expose ipfs as our primary, so it's more backwards compatible.

  [421, Protocols.lengthPrefixedVarSize, 'ipfs'],
  // preferred name for 421 (added below ipfs, p2p takes precedence during table population)
  [421, Protocols.lengthPrefixedVarSize, 'p2p'],

The transports should likely be updated to check for the code rather than the proto name.

[alanshaw]

I can confirm swapping these around fixes the browser tests

[jacobheun]

I'll get a PR for this change and will continue looking into the upgrade path for the transports.

[jacobheun]

Created #77 to resolve this.