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

WIP: Tor support #117

Open
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@freehodl
Copy link

freehodl commented Jan 11, 2019

This is my first ever pull request so please let me know what I'm doing wrong.

This change adds a button to the node launcher GUI that:

  1. Downloads Tor
  2. Installs Tor
  3. Adds a pre-configured torrc file
  4. Writes Tor configuration lines to lnd.conf and bitcoin.conf

This change only works on Windows so far.

I'm not a Tor expert and so security review is still needed.

@PierreRochard

This comment has been minimized.

Copy link
Collaborator

PierreRochard commented Jan 11, 2019

Excellent! Thank you!

@PierreRochard
Copy link
Collaborator

PierreRochard left a comment

I love where this is going! It'll be safer to write to the configuration files using the ConfigurationFile class https://github.com/PierreRochard/node-launcher/blob/master/node_launcher/services/configuration_file.py

Show resolved Hide resolved node_launcher/node_set/setup_tor.py Outdated
Show resolved Hide resolved node_launcher/node_set/node_set.py Outdated
Show resolved Hide resolved node_launcher/node_set/setup_tor.py
Show resolved Hide resolved node_launcher/node_set/setup_tor.py Outdated
self.node_set = node_set

def edit_bitcoin_conf(self):
f = open(str(BITCOIN_CONF_PATH[OPERATING_SYSTEM]) , 'a')

This comment has been minimized.

@PierreRochard

PierreRochard Jan 17, 2019

Collaborator

No longer need f

def edit_bitcoin_conf(self):
f = open(str(BITCOIN_CONF_PATH[OPERATING_SYSTEM]) , 'a')
self.node_set.bitcoin.file['proxy'] = '127.0.0.1:9050'
self.node_set.bitcoin.file['listen'] = '1'

This comment has been minimized.

@PierreRochard

PierreRochard Jan 17, 2019

Collaborator
Suggested change Beta
self.node_set.bitcoin.file['listen'] = '1'
self.node_set.bitcoin.file['listen'] = True
def edit_lnd_conf(self):
f = open(str(LND_CONF_PATH[OPERATING_SYSTEM]), 'a')
self.node_set.lnd.file['listen'] = 'localhost'
self.node_set.lnd.file[' ']

This comment has been minimized.

@PierreRochard

PierreRochard Jan 17, 2019

Collaborator

Not needed

f = open(str(LND_CONF_PATH[OPERATING_SYSTEM]), 'a')
self.node_set.lnd.file['listen'] = 'localhost'
self.node_set.lnd.file[' ']
self.node_set.lnd.file['tor.active'] ='1'

This comment has been minimized.

@PierreRochard

PierreRochard Jan 17, 2019

Collaborator

Same for the rest

Suggested change Beta
self.node_set.lnd.file['tor.active'] ='1'
self.node_set.lnd.file['tor.active'] = True
@PierreRochard

This comment has been minimized.

Copy link
Collaborator

PierreRochard commented Jan 17, 2019

Excellent progress @freehodl! I'll get it working on macos and linux and will contribute those pieces asap

@freehodl

This comment has been minimized.

Copy link
Author

freehodl commented Jan 19, 2019

Thanks @PierreRochard ! I'll get these changes made asap. I've already started work on the macos piece so I can take care of that if you want to focus on linux?

@willcl-ark

This comment has been minimized.

Copy link

willcl-ark commented Jan 23, 2019

I love where this is going! It'll be safer to write to the configuration files using the ConfigurationFile class https://github.com/PierreRochard/node-launcher/blob/master/node_launcher/services/configuration_file.py

Just as a point of interest more than anything else at this stage, you might like to check out Core's plan to split the configuration files in two; a read-write one which the GUI can effectively modify (bitcoin-conf-rw) and one which the daemon/GUI won't be able to modify (bitcoin.conf):

bitcoin/bitcoin#12833

It would only require minimal changes for this project though I think as we are intentionally modifying those files (settings) on the user's behalf.

@PierreRochard

This comment has been minimized.

Copy link
Collaborator

PierreRochard commented Jan 23, 2019

Great catch @willcl-ark, I'll keep a close eye on that, doesn't look like it'll be a big change on our end

@PierreRochard

This comment has been minimized.

Copy link
Collaborator

PierreRochard commented Jan 23, 2019

Thanks @freehodl! I'll try this out on Linux this weekend

@freehodl freehodl force-pushed the freehodl:master branch from d6bb904 to 5d9bcea Jan 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment