Implement missing protocols

[Zollerboy1]

This PR implements all protocols listed in https://github.com/multiformats/multiaddr/blob/master/protocols.csv.

Presumably, this PR closes #97.

[umgefahren]

I would like to see that merged especially since we need ip6zone

[umgefahren]

What's the status on this? @jxs

[jxs]

Hi @umgefahren sorry I missed this notification, and I don't seem to have permissions for this repo, I can ask to, meanwhile @Zollerboy1 sorry for the delay, but can we separate this into two PR's one implementing the missing protocols and the other re-ordering them? As it is it's very hard to review.
Thanks!

[Zollerboy1]

Hey @jxs, of course, I can do that. I'm on vacation right now, so I probably cannot work on this until next week though.

[Zollerboy1]

Hey @jxs, I finally came around to do this, it should be much easier to review now.

I have the code with the reordered protocols in another branch and would create a new PR when (or if) this one gets merged.

Currently, the CI is failing because the new rust-check-cfg feature in Rust 1.80 complains about the use of #[cfg(nightly)] in the test file. I wasn't sure how exactly you'd want to solve this, but this should probably be changed to #[cfg(feature = "nightly")] or similar.

[umgefahren]

Currently, the CI is failing because the new rust-check-cfg feature in Rust 1.80 complains about the use of #[cfg(nightly)] in the test file. I wasn't sure how exactly you'd want to solve this, but this should probably be changed to #[cfg(feature = "nightly")] or similar.

I don't think adding an extra feature that only pertains to a test is a good idea. Using this cfg is actually not that uncommon. I would suggest you add the lint exception:

[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(nightly)'] }

It might seem like a dirty fix, but it's a widely used workaround:

• cfg(nightly) exception in Cargo.toml
• cfg(nightly) in Rust files

It's also one of the suggested solutions by rust itself (and it's more efficient then using a build.rs file).

[umgefahren]

I made a quick PR: #112

[jxs]

Hi @Zollerboy1 can you pull the latest changes so we can make CI pass?
Thanks!

[umgefahren]

Hi @Zollerboy1 can you pull the latest changes so we can make CI pass? Thanks!

It passes now.

[jxs]

Hi! And sorry for the delay overal LGTM, just one question

[jxs]

why do we need replacing these characters?

[Zollerboy1]

The garlic protocols are very weird in that there is almost no documentation of them anywhere. But apparently, garlic64 doesn't use classic Base64 encoding, but a similar encoding where + is replaced with - and / with ~. I don't know why. But to use multibase::Base::Base64.decode on the string, we have to have a proper Base64 encoding, so I'm just replacing these characters. You can see in the multiaddr implementations in Go (https://github.com/multiformats/go-multiaddr/blob/414c602b9282d609668bd58c90c800468816d9f4/transcoders.go#L270-L271) and Java (https://github.com/multiformats/java-multiaddr/blob/ffdc5320b4ccb5d5e490b6bce72a8da5dbe775ae/src/main/java/io/ipfs/multiaddr/Protocol.java#L231) that they're doing the same.

[jxs]

thanks for taking the time to explain and provide examples!

[jxs]

LGTM Thanks! 🚀

[jxs]

thanks for taking the time to explain and provide examples!