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

Fix tor onion string conversion #29

Closed
wants to merge 2 commits into from

Conversation

david415
Copy link
Contributor

added code for onion multiaddr string conversion; added a unit test as well. please review.

- changed Tor onion multiaddr protocol size spec to 96 bits not 80 because 16 bits are for the port number
- added onion protocol case to addressBytesToString
- added an onion test to TestStringToBytes to test that multiaddr to string conversion is correct
@ianopolous
Copy link
Member

See discussion on similar PR: #21

@ianopolous
Copy link
Member

Don't forget the protocols.csv :-) then we're equivalent.

@david415
Copy link
Contributor Author

david415 commented Aug 5, 2016

please see my comments in
#21

  • the onion multiaddr should contain both the onion address and the port number. This makes sense since both peices of addressing info are logically at the same transportation level UNLIKE a IP address and TCP port which are located at different layers of the network stack.

@david415
Copy link
Contributor Author

david415 commented Aug 5, 2016

then merge #21 and we're done with this conversation and we can gradually move on to discussing the more interesting parts of the Tor integration

@jbenet
Copy link
Member

jbenet commented Aug 6, 2016

Thanks very much for this, and for hashing out this stuff. great to hear Tor's port is part of the addr, that it's always a stream, and that it works without a TCP stack. I will merge #21

@jbenet
Copy link
Member

jbenet commented Sep 16, 2016

This has been addressed in #21 -- thank you very much for championing this, @david415 @ianopolous

@jbenet jbenet closed this Sep 16, 2016
marten-seemann pushed a commit to marten-seemann/go-multiaddr that referenced this pull request Feb 25, 2021
…github.com/libp2p/go-maddr-filter-0.0.5

Bump github.com/libp2p/go-maddr-filter from 0.0.4 to 0.0.5
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.

None yet

3 participants