Skip to content

Clean up dependencies and features #146

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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
Contributor

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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
new feature New feature or request
Projects
None yet
2 participants