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.

Sign up for GitHub

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

Make Trin compilable on Windows #739

Closed
wants to merge 1 commit into from
Closed

Make Trin compilable on Windows #739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
229 changes: 109 additions & 120 deletions Cargo.lock
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ parking_lot = "0.11.2"
portalnet = { path = "portalnet" }
prometheus_exporter = "0.8.4"
rand = "0.8.4"
reth-ipc = { version = "0.1.0", git = "https://github.com/paradigmxyz/reth.git"}
rlp = "0.5.0"
rocksdb = "0.18.0"
rpc = { path = "rpc"}
Expand All @@ -41,6 +40,9 @@ trin-utils = { path = "trin-utils" }
trin-validation = { path = "trin-validation" }
utp-rs = "0.1.0-alpha.4"

[target.'cfg(not(windows))'.dependencies]
reth-ipc = { version = "0.1.0", git = "https://github.com/paradigmxyz/reth.git"}

[dev-dependencies]
ethportal-peertest = { path = "ethportal-peertest" }
ureq = { version = "2.5.0", features = ["json"] }
Expand Down
6 changes: 3 additions & 3 deletions ethportal-peertest/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ hex = "0.4.3"
hyper = { version = "0.14", features = ["full"] }
jsonrpsee = {version="0.16.2", features = ["async-client", "client", "macros", "server"]}
rand = "0.8.4"
reth-ipc = { version = "0.1.0", git = "https://github.com/paradigmxyz/reth.git"}

rocksdb = "0.18.0"
serde_json = "1.0.89"
tempfile = "3.3.0"
Expand All @@ -36,5 +36,5 @@ trin-state = { path = "../trin-state" }
trin-utils = { path = "../trin-utils" }
ureq = { version = "2.5.0", features = ["json"] }

[target.'cfg(windows)'.dependencies]
uds_windows = "1.0.1"
[target.'cfg(not(windows))'.dependencies]
reth-ipc = { version = "0.1.0", git = "https://github.com/paradigmxyz/reth.git"}
1 change: 1 addition & 0 deletions ethportal-peertest/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(unix)]
pub mod constants;
pub mod scenarios;

Expand Down
3 changes: 3 additions & 0 deletions newsfragments/739.fixed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### fix windows compilation

add cfg and add a few panics to clearly define what is not supported by windows so it can compile
3 changes: 0 additions & 3 deletions portalnet/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,6 @@ validator = { version = "0.13.0", features = ["derive"] }
url = "2.3.1"
utp-rs = "0.1.0-alpha.4"

[target.'cfg(windows)'.dependencies]
uds_windows = "1.0.1"

[dev-dependencies]
env_logger = "0.9.0"
quickcheck = "1.0.3"
Expand Down
4 changes: 3 additions & 1 deletion rpc/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ ethportal-api = { path = "../ethportal-api"}
portalnet = { path = "../portalnet"}
trin-utils = { path = "../trin-utils"}
tokio = { version = "1.14.0", features = ["full"] }
reth-ipc = { version = "0.1.0", git = "https://github.com/paradigmxyz/reth.git"}
url = "2.3.1"
serde_json = "1.0.95"

[target.'cfg(not(windows))'.dependencies]
reth-ipc = { version = "0.1.0", git = "https://github.com/paradigmxyz/reth.git"}
11 changes: 11 additions & 0 deletions rpc/src/server_rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::{Discv5Api, HistoryNetworkApi, Web3Api};
use ethportal_api::types::jsonrpc::request::HistoryJsonRpcRequest;
use ethportal_api::{Discv5ApiServer, HistoryNetworkApiServer, Web3ApiServer};
use portalnet::discovery::Discovery;
#[cfg(unix)]
use reth_ipc::server::Builder as IpcServerBuilder;
use std::net::SocketAddr;
use std::path::Path;
Expand Down Expand Up @@ -31,6 +32,7 @@ impl JsonRpcServer {
Ok(handle)
}

#[cfg(unix)]
pub async fn run_ipc(
ipc_path: Box<dyn AsRef<Path>>,
discv5: Arc<Discovery>,
Expand All @@ -46,4 +48,13 @@ impl JsonRpcServer {
let handle = server.start(api).await?;
Ok(handle)
}

#[cfg(windows)]
pub async fn run_ipc(
_ipc_path: Box<dyn AsRef<Path>>,
_discv5: Arc<Discovery>,
_history_handler: mpsc::UnboundedSender<HistoryJsonRpcRequest>,
) -> anyhow::Result<ServerHandle> {
panic!("Windows doesn't support Unix Domain Sockets IPC, use --web3-transport http")
Copy link
Member

Choose a reason for hiding this comment

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

It seems Windows support unix domain sockets since 2018: https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/ and there is a rust crate for it?

Copy link
Member Author

Choose a reason for hiding this comment

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

We used to use that crate. Currently Reth uses tokio UnixStream so we can't use that crate. For tokio to add windows UDS it has to be added to MIO first I made a PR for it tokio-rs/mio#1667 but there is no telling when it will be merged if at all. This is a tokio windows UDS by Microsoft https://github.com/Azure/tokio-uds-windows but it hasn't been updated really in 5 years so it is hard to know if it is even maintained.

If we want we could maintain our own Windows Tokio UDS library I can have that ready by this week, but then we would also need to see if Reth would want to use that.

So yes Windows supports UDS (without pair), but it really isn't supported by any Rust libraries what suit Tokio.

So I think this is the best solution for now and maybe we can talk if maintaining a windows tokio UDS library and working with reth in the future. So my idea was merge this now and see if that is a overhead we would want to support in the future. (Unless we get lucky and get support from the Tokio team, but they seem uninterested)

Copy link
Member

Choose a reason for hiding this comment

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

I see, supporting windows at this stage is not a priority, so I don't think we should maintain our Windows Tokio UDS library.

Copy link
Member Author

@KolbyML KolbyML May 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think so either, which is why I don't think windows should support RPC via IPC

}
}
2 changes: 1 addition & 1 deletion tests/self_peertest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#[cfg(test)]
#[cfg(all(unix, test))]
mod test {
use std::{
net::{IpAddr, Ipv4Addr},
Expand Down