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

Updated electrum config to support socks5 & validate_domain #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

i5hi
Copy link
Contributor

@i5hi i5hi commented Sep 20, 2024

No description provided.

@i5hi
Copy link
Contributor Author

i5hi commented Sep 20, 2024

validate_domain may not be required since its in ElectrumUrl ?

Copy link
Contributor

@RCasatta RCasatta left a comment

Choose a reason for hiding this comment

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

Sorry I forgot about this one...

I think we need some refactors tough, there are some issues:

  • some parameters are passed in new, some in build_client...
  • the comment about retry value
  • We can probably use directly electrum_client::Config instead of having ElectrumOptions

We can probably go ahead with this (after fixing the retry) and think about the other issues later

@@ -113,7 +119,10 @@ impl Debug for ElectrumClient {

#[derive(Default)]
pub struct ElectrumOptions {
timeout: Option<u8>,
pub timeout: Option<u8>,
pub retry: u8,
Copy link
Contributor

Choose a reason for hiding this comment

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

This as default would be 0, but in electrum_client::Config the default is 1 (not sure what happens with 0)

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.

2 participants