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

Use Tor executable from install dir #4505

Merged
merged 26 commits into from Oct 13, 2020
Merged

Use Tor executable from install dir #4505

merged 26 commits into from Oct 13, 2020

Conversation

molnard
Copy link
Collaborator

@molnard molnard commented Oct 6, 2020

Ready to review!

Instead of copy/update/execute Tor binaries from datadir, Wasabi will just run Tor from the installation dir like it does with HWI and bitcoind.

The first step of this #4501

@molnard molnard changed the title Mov Tor executable Use Tor executable from install dir Oct 6, 2020
@molnard molnard marked this pull request as draft October 6, 2020 15:32
@molnard
Copy link
Collaborator Author

molnard commented Oct 7, 2020

@yahiheb can you figure out something for CF? We cannot touch that file so I would say suppress the error somehow or disable for that folder/file.

@molnard
Copy link
Collaborator Author

molnard commented Oct 8, 2020

@yahiheb
Copy link
Collaborator

yahiheb commented Oct 8, 2020

no, what's up with that?

It describes how to delete those carriage return. https://github.com/koalaman/shellcheck/wiki/SC1017#rationale

Or another (complicated) solution is to add a .shellcheck.yaml file to the root of the branch and to configure it to disable SC1017 https://github.com/koalaman/shellcheck/blob/master/shellcheck.1.md#rc-files

@molnard
Copy link
Collaborator Author

molnard commented Oct 9, 2020

We cannot change the file, it is delivered by Tor, so that is not an option.

@kiminuo
Copy link
Collaborator

kiminuo commented Oct 9, 2020

@molnard digests.txt file was removed and is no longer checked. Could you please say why it was useful before and why it is no longer useful now?

@molnard
Copy link
Collaborator Author

molnard commented Oct 12, 2020

@molnard digests.txt file was removed and is no longer checked. Could you please say why it was useful before and why it is no longer useful now?

It was used to check if the tor binaries need to be updated in the data folder. Now we are just running the one came with the installation so it is always the newest and don't need to manage Tor executable in the datadir.

@molnard
Copy link
Collaborator Author

molnard commented Oct 12, 2020

Tested on win, mac and linux.

@molnard molnard marked this pull request as ready for review October 12, 2020 11:55
@molnard
Copy link
Collaborator Author

molnard commented Oct 12, 2020

I was not able to fix CF issues those should be handled here: #4531

@molnard molnard added this to Backlog in Valhalla via automation Oct 12, 2020
@molnard molnard moved this from Backlog to Review in progress in Valhalla Oct 12, 2020
@nopara73 nopara73 merged commit c29958b into zkSNACKs:master Oct 13, 2020
Valhalla automation moved this from Review in progress to Done Oct 13, 2020
@@ -17,34 +18,18 @@ public class TorSettings
/// <param name="distributionFolderPath">Full path to folder containing Tor installation files.</param>
public TorSettings(string dataDir, string logFilePath, string distributionFolderPath)
{
TorDir = Path.Combine(dataDir, "tor");
TorBinaryDir = Path.Combine(TorDir, "Tor");
var torBinary = MicroserviceHelpers.GetBinaryPath(Path.Combine("Tor", "tor"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be a parameter of TorSettings too. TorSettings was meant to be purely data wrapper class that does not "find paths to something".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Valhalla
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants