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

I2P: updates for connections over I2P #2

Open
wants to merge 10 commits into
base: i2p
Choose a base branch
from

Conversation

chisa0a
Copy link

@chisa0a chisa0a commented Jun 1, 2019

Adds remaining work for connecting nodes over I2P, and starting i2pd automatically.

Some work remains (seeding over I2P, creating reverse connections, etc.), but I wanted to submit this PR for review given the upcoming version upgrade.


name: Pull Request
about: Pull Request checklist
title: ''
labels: ''
assignees: ''


If your PR is a work in progress, please feel free to create it and include a [WIP] tag in the PR name. We encourage everyone to PR early and often so that other developers know what you're working on.

Before submitting your PR for final review, please ensure that it:

  • Includes a proper description of what problems the PR addresses, as well as a detailed explanation as to what it changes
  • Explains whether/how the change is consensus breaking or breaks existing client functionality
  • Contains unit tests exercising new/changed functionality
  • Fully considers the potential impact of the change on other parts of the system
  • Describes how you've tested the change (e.g. against Floonet, etc)
  • Updates any documentation that's affected by the PR

chisa0a added 10 commits May 15, 2019 21:22
Convert use of panic! to return result + error.

Adds PeerAddrType enum for converting address type to/from raw u8.
Adds I2P to fullnode capabilities, modifies tests accordingly.

Other rustfmt changes.
Shutdown I2P streams gracefully on stop_state.
Includes `i2p_mode` and I2P peer examples in the default config file.
Adds implementation to start i2pd automatically when I2P support enabled.
#be specified as follows:
#seeds = [\"192.168.0.1:3414\",\"192.168.0.2:3414\"]
#be specified as follows (same for other peer types):
#For TCP addresses:

Choose a reason for hiding this comment

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

These are IP addresses, TCP is how the packages are transmitted between the addresses in the network. If I2P and TCP are mutually exclusive protocols and that's what you want to call out here then maybe use "TCP/IP" instead of "TCP"?


#[server.p2p_config.i2p_mode.i2p_config]
#autostart = true/false - start I2P automatically
#exclusive = true/false - exclusively connect through I2P, or also use TCP

Choose a reason for hiding this comment

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

TCP/IP instead of TCP?

#mode = \"Enabled\"

#[server.p2p_config.i2p_mode.i2p_config]
#autostart = true/false - start I2P automatically
Copy link

@0xmichalis 0xmichalis Jun 12, 2019

Choose a reason for hiding this comment

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

all configuration options here will not be valid if uncommented, maybe try to make it as simple as possible to get started when uncommenting them so:

# start I2P automatically
# autostart = true
# exclusively connect through I2P, or also use TCP/IP
# exclusive = true
# address of local I2P server
# addr = \"127.0.0.1:7656\"

.spawn()
.expect("i2pd failed to start. Ensure i2pd is on your system path.");
// allow i2pd a moment to startup before attempting SAM connection
std::thread::sleep(std::time::Duration::from_millis(100));

Choose a reason for hiding this comment

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

Can't you retry the SAM connection for some time instead of relying on the sleep?

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.

2 participants