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

Clean up dependencies and features #146

Open
LLFourn opened this issue Sep 12, 2024 · 1 comment · May be fixed by #148
Open

Clean up dependencies and features #146

LLFourn opened this issue Sep 12, 2024 · 1 comment · May be fixed by #148
Assignees
Labels
new feature New feature or request

Comments

@LLFourn
Copy link

LLFourn commented Sep 12, 2024

There is way too much feature gating logic to me.

  1. We shouldn't have a method like new_proxy_ssl, just have a single method that starts an electrum TLS connection given a TcpStream. Of course then we have a convenience method to do the initial connection to a url string if they don't want proxies. But if they do then it's fine to have them set up the proxy connection themselves.
  2. Do we even need openssl as a dependency at all? If someone wants to use socks5 then can depend on openssl themelves or use one of the many pure rust socks5 client libraries like socks5-client. We can have an example showing how to do tor. IMO it'd be better to try and use socks5-client rather than openssl. For TLS we can just be opinionated and use rustls.

This seems like something you would want to do anyway @oleonardolima for arti?

I'll turn this into an issue that I think we should tackle first.

Originally posted by @LLFourn in #138 (comment)

@oleonardolima
Copy link
Collaborator

Yes, I do agree with both (1) and (2), there are also issues with other features such s proxy previously mentioned by tnull, refer to #91, which is basically mandatory and we can't use the client with --no-default-features.

Indeed, with arti-client I do need the client to receive a data stream (e.g. TcpStream, arti-client::DataStream), and was planning on such refactoring. It also requires being async, which detangling all existing features will make it easy to have an async version.

Thanks for opening the issue, I moved the #138 back to draft for now.

@oleonardolima oleonardolima added the new feature New feature or request label Sep 13, 2024
@oleonardolima oleonardolima self-assigned this Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
2 participants