-
Notifications
You must be signed in to change notification settings - Fork 992
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
[WIP] P2P: initial I2P support #2932
Conversation
Adds necessary configuration bits to enable I2P in various modes, unused for now but will soon. Also add peer address wrappers for I2P so those addresses can be understood and exchanged between peers. Doesn't compile yet as serde needs to be supported in i2p-rs first.
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.
Fixes references for non-`Copy` `PeerAddr`, other minor fixups for return types, and removing unneeded functions.
macosx test failed, but looks unrelated to the changes in this PR:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great works, some minor comments
@chisa0a if you don't mind I can remove extra clone calls |
Sounds good to me. I can also take care of them when working on the other refactors, either way. Many thanks for the review! |
config/src/comments.rs
Outdated
#mode = \"Enabled\" | ||
|
||
#[server.p2p_config.i2p_mode.i2p_config] | ||
#autostart = true/false - start I2P automatically |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More admin-friendly config:
# Start I2P automatically
# autostart = true
# Exclusively connect through I2P, or also use TCP/IP
# exclusive = true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed this before the most recent push of updates. Can add with next round of changes.
We are interested only in behaviour of Stream, so it sounds right to model it as a trait. It allows also reuse implementation of existing traits (Read, Write) as is without wrapping. It simplifies client code because it can work directly with TcpStream and I2pStream. At the same time it brings some complexity - one more type parameter for some functions and some boilerplate code to implement trait for references.
Represent Stream as a trait
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.
Fixes references for non-`Copy` `PeerAddr`, other minor fixups for return types, and removing unneeded functions.
We are interested only in behaviour of Stream, so it sounds right to model it as a trait. It allows also reuse implementation of existing traits (Read, Write) as is without wrapping. It simplifies client code because it can work directly with TcpStream and I2pStream. At the same time it brings some complexity - one more type parameter for some functions and some boilerplate code to implement trait for references.
Adds `set_read_timeout` and `set_write_timeout` handlers to the Stream trait. Requires updates in i2p-rs for calling the functions on I2pStream.
Adds I2P thread handle to deal with any potential errors when joining on shutdown.
Various small fixups for rebase onto master at commit ea1d14d.
Changes the i2p-rs crate to the fork hosted on the mimblewimble org repo.
} | ||
} | ||
|
||
fn start_i2pd() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see us stopping i2pd. Is that intentional?
Closing due to lack of activity. |
Adds initial support for I2P P2P connections. Includes @ignopeverell's initial work supporting I2P, finishes a large amount of the remaining work for E2E I2P connections, and rebases onto the
milestone/2.x.x
branch.Supports connecting to an I2P seeding peer via the
List
seeding option in the config. Other config peer lists (peers_allow
,peers_deny
) also support I2P peers. Further testing is needed to see how other peer lists handle I2P peers.Due to the size of this PR, will split remaining work into separate PRs building on top of this one.
Remaining work:
i2pd
automatically (if not found locally)Changes have been tested on Mainnet and Floonet, with successful I2P connections on both. Currently, I2P peer connections have to be manually verified through
i2pd
user interface.Seeding peer was able to sync exclusively from I2P seeder, get a TCP peer list from I2P seeder, and sync from TCP peers.
I've added a wiki entry for initial setup to aid with testing, and as WIP documentation for connecting Grin nodes over I2P (thanks for the suggestion @Kargakis).
Includes unit tests for de/serializing I2P peers from/to the config. Tests for creating I2P streams require a running I2P node that supports SAM connections.