Skip to content
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

Added Onion address support #35

Closed
wants to merge 1 commit into from

Conversation

sdbondi
Copy link
Contributor

@sdbondi sdbondi commented Nov 28, 2019

Support for parsing /onion and /onion3 addresses.

Unfortunately I had to make data-encoding a dependency because
multibase added an extra (padding?) byte to the byte representation
when decoding from BASE32.

This go implementation uses go std lib base32 encoding, not multibase so perhaps this is fine.

There are probably a few issues such as whether it's acceptable to have the onion encoding code in this repo. If so, suggestions welcome :)

Progress on #3

@sdbondi
Copy link
Contributor Author

sdbondi commented Dec 2, 2019

@dignifiedquire @Stebalien @tomaka Hey :) Any comments on this PR?

@sdbondi sdbondi force-pushed the add-onion branch 2 times, most recently from b5de2f1 to 3e45ea2 Compare December 10, 2019 04:54
Support for parsing `/onion` and `/onion3` addresses.

Unfortunately I had to make `data-encoding` a dependency because
`multibase` added an extra (padding?) byte to the byte representation
when decoding from BASE32.
@mark-stopka
Copy link

So I guess this was not merged in? Is this project still alive?

@Stebalien
Copy link
Member

There's nobody actively maintaining it, unfortunately. cc @vmx if you have some time to revive this.

@mark-stopka
Copy link

@vmx, would it be possible to add a community maintainer who would perhaps be more active on this?

@vmx
Copy link
Member

vmx commented Jul 10, 2020

@mark-stopka an actively maintained version of multiaddr is part of the libp2p project: https://github.com/libp2p/rust-libp2p/tree/68587ee180c076e6b83a4d3bb3193dcc9a656b59/misc/multiaddr that one even has onion address support already merge (libp2p/rust-libp2p#1354).

@mark-stopka
Copy link

I am aware of that, but I was under the impression that it was a fork of necessity to accelerate development and the idea was to backport the features back into this upstream library...

See libp2p/rust-libp2p#758 for reference...

I would be willing to commit the required resources by hiring a 3rd party developer to work on it, but I need to be sure, that upstream will accept the backports or transfer the maintenance...

@vmx
Copy link
Member

vmx commented Jul 10, 2020

I am aware of that, but I was under the impression that it was a fork of necessity to accelerate development and the idea was to backport the features back into this upstream library...

That's also my impression and we did that with multihash already.

I currently maintain most of the rust multiformats repos as most of them are tightly related to IPLD (which is what I actually work on). Though multiaddr is kind of separate and I currently don't have the bandwidth to maintain it properly.

I would be willing to commit the required resources by hiring a 3rd party developer to work on it, but I need to be sure, that upstream will accept the backports or transfer the maintenance...

Thanks for the offer. I think next steps would be to talk to the libp2p maintainers to see if there is interest to have an upstream version of multiaddr which isn't under their full control, but under shared control.

@tomaka How does it sound to you to move multiaddr out of libp2p again? I can help on the initial work and make sure that things are smooth, so that there is as little additional work for the libp2p team (there will of course be some work, like testing for regression, but I hope to make this as smooth as possible, as I did with rust-multihash).

As I know little about multiaddr details, it would be great if someone from the libp2p could then overlook the future developments (the way you would if someone would open an PR on the libp2p version of multiaddr today).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants