Skip to content

Conversation

jonatack
Copy link
Member

@jonatack jonatack commented Aug 6, 2021

This pull addresses #22634 (comment) and various user feedback/questions, updates the -onlynet documentation in doc/i2p.md and doc/tor.md per #22651 (src/init.cpp is already fine) and fills in some missing I2P unit test coverage.

Note: this PR depends in part on whether #22651 is merged in order to propose the correct -onlynet documentation (it is currently aligned with the change in #22651), so that PR should be decided or merged first.

@jonatack jonatack force-pushed the i2p-doc-updates-august-2021 branch 2 times, most recently from ee86daa to 8d51dd9 Compare August 6, 2021 10:11
@fanquake fanquake added the Docs label Aug 6, 2021
@jonatack jonatack changed the title doc: improve i2p.md about router options/versions and -onlynet doc, test: improve i2p/tor docs and i2p reachable unit tests Aug 6, 2021
@jonatack jonatack force-pushed the i2p-doc-updates-august-2021 branch from 8d51dd9 to e3a5814 Compare August 6, 2021 22:59
@jonatack jonatack marked this pull request as ready for review August 6, 2021 22:59
@ghost
Copy link

ghost commented Aug 8, 2021

Concept ACK on improving the doc.

@ghost
Copy link

ghost commented Aug 8, 2021

Should we add information about ports in this doc? #22634 (comment)

@jonatack
Copy link
Member Author

jonatack commented Aug 8, 2021

Your suggestion? Feel free to propose a text.

@ghost
Copy link

ghost commented Aug 8, 2021

Looks like we need all ports open for outbound: https://www.reddit.com/r/i2p/comments/fzwwd0/confused_on_what_ports_to_open/fn76ly9

And for inbound users can follow everything mentioned here: https://geti2p.net/en/faq#ports

or later recommended)
- [i2p-zero](https://github.com/i2p-zero/i2p-zero)
- [other alternatives](https://en.wikipedia.org/wiki/I2P#Routers)

Copy link
Contributor

@Rspigler Rspigler Aug 8, 2021

Choose a reason for hiding this comment

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

For the official i2prouter implementation in Java, visit their download page and follow the instructions for your Operating System.

Once installed, open a terminal window and type i2prouter start. Then visit http://127.0.0.1:7657/configclients in your browser to enable SAM. On the left side of the page, there should be a green light next to "Shared Clients".

Edit: Fixed with @RandyMcMillan suggestions

Copy link
Contributor

Choose a reason for hiding this comment

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

If the user has successfully config'd to this point - a link will take the user to the correct page.

Maybe add a link for convenience?

Then visit 127.0.0.1:7657/configclients

Choose a reason for hiding this comment

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

L09

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not clear to me if a change is being requested here, and what, precisely. Install how-to seems a bit redundant with the existing docs of the various implementations.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think adding the above at Line 23 (insert new line first) would be helpful. I agree instructions for every implementation would be overkill, but I was completely new to i2p, and I had to do a bit of digging to figure out how to set it up. I found the wording a bit confusing (I thought I had to install a router and install SAM). The documentation on the website is mostly about setting up a browser. I think adding instructions for users for the reference implementation could be helpful in increasing i2p nodes. The tor docs include a bit of instructions as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. I agree it might be helpful, but I don't know if those instructions are platform agnostic. We would also need to maintain it / keep it up to date. Most people I talk to are using i2pd (admittedly perhaps because it is very easy to install), so if we explain how to install i2p-router, we should probably also do it for i2pd. Sounds like a dedicated "How to Install an I2P Router" section.

In any case, feel free to propose this separately. I would rather not make this pull larger.

@Rspigler
Copy link
Contributor

Rspigler commented Aug 8, 2021

@prayank23 I don't believe that is correct. I am behind a router/firewall I don't control right now (haven't forwarded any ports) and I am able to successfully create an i2p address (ugmw4zuyhmrm7anfctjamcjgi32daor4ulqdrxtn66owgn2oyebq.b32.i2p:0) and connect out to other i2p nodes (although they are not very long lasting connections, and I have no inbound connections despite setting i2pacceptincoming=1).

That being said, I also have no incoming tor connections to my HS (kwn76yxlsaucsbywm5sy4q5ebo6atao5escejchwjkzu2l5lwkkkzdqd.onion:8333) which I know doesn't require any port forwarding, so I'm not sure what's going on. I have been running my node for many hours.

bitcoin.conf:

listen=1
assumevalid=0
dbcache=1000
addresstype=bech32
avoidpartialspends=1
changetype=bech32
maxapsfee=0.01
i2psam=127.0.0.1:7656
debug=tor
debug=i2p
i2pacceptincoming=1

In my debug file, I see that my node is trying to connect to lots of i2p nodes, but I have tons of connection timeout errors.

Edit: Maybe you could try to connect to me?

@ghost
Copy link

ghost commented Aug 8, 2021

Edit: Maybe you could try to connect to me?

Sure. Will experiment tomorrow. Will ping you in IRC. It's 4 AM here right now 😴

@jonatack
Copy link
Member Author

jonatack commented Aug 9, 2021

@Rspigler I manually connected successfully to both of your addresses.

Edit: still connected but they both have really high ping times of 11 to 60 seconds versus under 1.5 seconds for all the others.

@Rspigler
Copy link
Contributor

Rspigler commented Aug 9, 2021

I see your inbound for both i2p and tor, so I think we should be good, unless other users start reporting high ping times for these networks with v22.0.

Thanks for helping!

@ghost
Copy link

ghost commented Aug 9, 2021

I see your inbound for both i2p and tor, so I think we should be good, unless other users start reporting high ping times for these networks with v22.0.

Things that you should try:

  1. Check firewall status: sudo ufw status verbose
  2. Deny incoming traffic, restart and see if i2p works fine in Core for incoming peers
  3. Block some ports for outgoing and see if i2p works for outbound peers in Core
  4. Results for ss nlt and nmap

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

ACK e3a5814

nit:

  1. Would be great if we can add things suggested by @Rspigler in #22648

  2. And information about ports or at least this link: https://geti2p.net/en/faq#ports

I have added both things in BlockchainCommons/Learning-Bitcoin-from-the-Command-Line#408

For ports I tried experimenting with allow/deny in ufw and see if it affects outgoing and incoming connections in Bitcoin Core.

TL;DR:

Ports required by i2p:

1. Outbound (Internet facing): a random port between 9000 and 31000 is selected however its better if all ports are open and it doesn't affect your security. You can check firewall status using `sudo ufw status verbose` which shouldn't deny outgoing by default.

2. Inbound (Internet facing): Optional

Local ports: https://geti2p.net/en/faq#ports

If something looks incorrect feel free to comment :)

@Rspigler
Copy link
Contributor

a random port between 9000 and 31000

I can confirm this

@jonatack
Copy link
Member Author

Thanks @prayank23! And thanks for the suggestions. I'd rather not copy information here that is already in https://geti2p.net/en/faq#ports, as it would add maintainance here and could go out of date.

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK e3a5814

@Rspigler
Copy link
Contributor

ACK e3a5814

@jonatack jonatack mentioned this pull request Aug 19, 2021
@jonatack jonatack force-pushed the i2p-doc-updates-august-2021 branch from e3a5814 to 0175977 Compare August 19, 2021 14:29
@jonatack
Copy link
Member Author

Updated git diff e3a5814 0175977 per review feedback (thanks!) and corrected the name of the first I2P router from i2p-router to i2prouter (I2P Router), which is also more similar to the second router i2pd (I2P Daemon).

@Rspigler
Copy link
Contributor

Re-ACK 0175977

Copy link
Contributor

@vasild vasild left a comment

Choose a reason for hiding this comment

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

ACK 0175977

@ghost
Copy link

ghost commented Aug 20, 2021

reACK 0175977

Changes since last review: #22648 (comment)

@laanwj laanwj merged commit 7740ebc into bitcoin:master Aug 26, 2021
@laanwj
Copy link
Member

laanwj commented Aug 26, 2021

Sorry for merging this before #22651, -onlynet edge-case behavior is not personally clear to me and it wasn't clear to me that the documentation here depends on code that isn't merged yet. The other documentation changes looked sane to me and all the ACKs here were kind of confusing me that it was ready for merge.

Warning: if you use -onlynet with values other than onion, and
the -onion or -proxy option is set, then outgoing onion
connections will still be made; use -noonion or -onion=0 to
disable outbound onion connections in this case.
Copy link
Member

@laanwj laanwj Aug 26, 2021

Choose a reason for hiding this comment

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

I think I would personally prefer #22651 to fix this behavior instead of having to describe this long special case exception at all. Can't we make -onlynet=ipv4 (or any onlynet not involving -onlynet=onion) do as one would expect, ignore onions, even if there happens to be a proxy for them? I think this is the most expected behavior, to have proxy setting be orthogonal to what networks to connect out on.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@laanwj laanwj Aug 26, 2021

Choose a reason for hiding this comment

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

A counter-example: if I specify -onlynet=onion (with the intent of only connecting to onions), I'd be bummed if it still connected to ipv4 and ipv6 nodes because a proxy is specified for them indirectly through -proxy=127.0.0.1:9050.

Copy link

@ghost ghost Aug 26, 2021

Choose a reason for hiding this comment

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

I think I would personally prefer #22651 to fix this behavior

It can be done in a follow up PR. Will be great if we can do it in the same PR.

Would also need to fix port used for Windows by default: #22651 (comment)

Can't we make -onlynet=ipv4 (or any onlynet not involving -onlynet=onion) do as one would expect, ignore onions, even if there happens to be a proxy for them?

Yes. According to my understanding this is how things would work: Consider Alice wants to use Tor proxy for outbound connections, however connect only with ipv4 nodes. Bob wants to use Tor proxy for outbound connections, he is okay with connecting to any nodes (onion, ipv4 etc.)

So Alice can use onlynet=ipv4 and proxy=127.0.0.1:9050. Bob will use proxy=127.0.0.1:9050

Copy link
Member Author

Choose a reason for hiding this comment

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

At least the onlynet docs in src/init.cpp, doc/tor.md and doc/i2p.md are now all aligned.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we make -onlynet=ipv4 ... do as one would expect ...

Done in #22834.

laanwj pushed a commit that referenced this pull request Aug 26, 2021
Github-Pull: #22648
Rebased-From: b87a9c4

Tree-SHA512: 3567e6dbfb7c3f410ea84d3f005c86be0283f92e1be1bc2a7ce93240a0b8460b985b3cec5873a19eade52dc1d78ca383c99e023d0bb4247048ae7e43b47522a5
fujicoin added a commit to fujicoin/fujicoin-22.0 that referenced this pull request Aug 27, 2021
Github-Pull: bitcoin/bitcoin#22648
Rebased-From: b87a9c4d13329a6236124d4b93580c4df8107b32

Tree-SHA512: 3567e6dbfb7c3f410ea84d3f005c86be0283f92e1be1bc2a7ce93240a0b8460b985b3cec5873a19eade52dc1d78ca383c99e023d0bb4247048ae7e43b47522a5
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 28, 2021
gwillen pushed a commit to gwillen/elements that referenced this pull request Jul 27, 2022
Github-Pull: bitcoin/bitcoin#22648
Rebased-From: b87a9c4

Tree-SHA512: 3567e6dbfb7c3f410ea84d3f005c86be0283f92e1be1bc2a7ce93240a0b8460b985b3cec5873a19eade52dc1d78ca383c99e023d0bb4247048ae7e43b47522a5
gwillen pushed a commit to gwillen/elements that referenced this pull request Aug 1, 2022
Github-Pull: bitcoin/bitcoin#22648
Rebased-From: b87a9c4

Tree-SHA512: 3567e6dbfb7c3f410ea84d3f005c86be0283f92e1be1bc2a7ce93240a0b8460b985b3cec5873a19eade52dc1d78ca383c99e023d0bb4247048ae7e43b47522a5
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants