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

chore: Upgrade arti_client to 0.24 #18

Merged
merged 17 commits into from
Nov 20, 2024

Conversation

binarybaron
Copy link
Contributor

@binarybaron binarybaron commented Nov 18, 2024

This PR:

  • Bumps the libp2p crate version to 0.53
  • Bumps the arti_client crate to 0.24
  • Removes support for runtime agnonstic use of this crate. We force users to use tokio + rustls
  • Adds a new TorTransport::from_client builder method to construct a transport from an already existing Arti client

Closes #17

@umgefahren
Copy link
Owner

Thank you for your effort!!! Can you tell me what you are using this for?

@umgefahren
Copy link
Owner

Thanks for your effort. However it looks like it no longer compiles with the version update. This likely to updates or movements. Could you resolve these problems?

@umgefahren umgefahren self-requested a review November 18, 2024 08:40
@binarybaron binarybaron marked this pull request as draft November 18, 2024 12:14
@binarybaron
Copy link
Contributor Author

Thanks for your effort. However it looks like it no longer compiles with the version update. This likely to updates or movements. Could you resolve these problems?

We are using this for our implementation of Monero <> Bitcoin atomic swaps.

I'm super hyped to use this crate, both for privacy and to allow users to listen on an onion address without having to expose any ports. Previously, we bundled a pre-compiled tor binary and routed libp2p over the socks5 proxy (and connected to the control port to manually create a hidden service) but using this'd be so much simpler! I only recently discovered arti and I love it!

I'm still trying to adapt the code to use the new arti_client API. Feel free to jump in and push to this branch if you want to. I haven't worked a lot with runtime agnostic before.

@umgefahren
Copy link
Owner

The approach of using this instead of a SOCKS5 proxy is indeed a good idea.
Since you are/would be the only user of this crate (to my knowledge) I wouldn't mind dropping support for either Tokio or Async-std. It was written that way to fit into rust-libp2p.
At the moment I don't have that much bandwidth to do the implementation myself but I'd be very happy to review your PR.

@umgefahren
Copy link
Owner

I don't know your use case that well but I want to note, that the security could be easily compromised by an incorrect use of identify, so bear that in mind.
Second note that Tor anonymity can be compromised: https://github.com/Attacks-on-Tor/Attacks-on-Tor/blob/master/Thirteen%20years%20of%20Tor%20Attacks.pdf
There are more secure alternatives out there, if you are interested: @hoprnet

@binarybaron binarybaron marked this pull request as ready for review November 19, 2024 15:56
Copy link
Owner

@umgefahren umgefahren left a comment

Choose a reason for hiding this comment

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

I don't understand some of your questions. Especially the ones you did in [Cargo.toml]. Could you clarify them?

Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
Cargo.toml Outdated
edition = "2021"
description = "Tor transport for libp2p"
repository = "https://github.com/hannes-furmans/libp2p-community-tor"
Copy link
Owner

Choose a reason for hiding this comment

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

This is not the url of this repository.

Cargo.toml Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
src/lib.rs Outdated
Comment on lines 58 to 70
//! ## Example (async-std + native-tls)
//! ```no_run
//! # use async_std_crate as async_std;
//! # use libp2p_core::Transport;
//! # async fn test_func() -> Result<(), Box<dyn std::error::Error>> {
//! let address = "/dns/www.torproject.org/tcp/1000".parse()?;
//! let mut transport = libp2p_community_tor::AsyncStdNativeTlsTorTransport::bootstrapped().await?;
//! // we have achieved tor connection
//! let _conn = transport.dial(address)?.await?;
//! # Ok(())
//! # }
//! # async_std::task::block_on(async { test_func().await.unwrap() });
//! ```
Copy link
Owner

Choose a reason for hiding this comment

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

Why do you remove the example?

@binarybaron
Copy link
Contributor Author

Sorry for the confusion. This is not ready for review yet.

I did a dirty proof-of-concept to get this compiling, and used Claude 3.5 for some of the changes. I definitely didn't intend to remove your copyright / parts of your documentation.

I'll ping you once this is ready :)

@umgefahren
Copy link
Owner

oh okay, no worries :)
I just reviewed it because you marked it ready to review

@umgefahren umgefahren marked this pull request as draft November 19, 2024 20:06
@binarybaron
Copy link
Contributor Author

oh okay, no worries :) I just reviewed it because you marked it ready to review

I know, my fault.

@binarybaron binarybaron marked this pull request as ready for review November 20, 2024 09:00
@binarybaron
Copy link
Contributor Author

@umgefahren This is ready for review.

Copy link
Owner

@umgefahren umgefahren left a comment

Choose a reason for hiding this comment

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

Just the little thing about the entry point. Otherwise: Thank you!

@@ -33,80 +33,54 @@
//! Use with caution and at your own risk. **Don't** just blindly advertise Tor without fully understanding what you
//! are dealing with.
//!
//! Main entrypoint of the crate: [`TorTransport`]
Copy link
Owner

Choose a reason for hiding this comment

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

I still think that's helpful :)

@umgefahren umgefahren enabled auto-merge (squash) November 20, 2024 09:55
@umgefahren umgefahren merged commit 75d2540 into umgefahren:main Nov 20, 2024
8 checks passed
@umgefahren
Copy link
Owner

Thank you for your work! May I ask why you didn't update to libp2p to 0.54.1?

@binarybaron
Copy link
Contributor Author

Thank you for your work! May I ask why you didn't update to libp2p to 0.54.1?

We are still on 0.53. Once we upgrade, I'll open a PR to upgrade this crate too.

binarybaron added a commit to UnstoppableSwap/core that referenced this pull request Nov 21, 2024
- Upgrade `sqlx` to `0.8`
- Use `[email protected]` in combination with [`libp2p-community-tor`](https://crates.io/crates/libp2p-community-tor/0.4.1). umgefahren/libp2p-tor#18 was required for this.
- Display spinner in GUI while Tor circuits are being established
- Remove unused dependencies (`once_cell`, `tauri-plugin-devtools`, `digest`, `hyper`, `itertools`, `erased_serde`)
- Bundle roboto font from npm registry
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.

Update to newer version of arti_client
2 participants