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

Tor: Configurable ports #11997

Merged

Conversation

kiminuo
Copy link
Collaborator

@kiminuo kiminuo commented Nov 26, 2023

This PR allows to change WW default ports assigned to the bundled Tor.

As a consequence, it would be possible to run WW instances like this:

dotnet build && dotnet run --framework net8.0 -- --datadir="C:\w1" --torSocksPort=35000 --torControlPort=35001

That is useful for tests and useful for advanced scenarios for advanced users.

@@ -54,7 +54,7 @@ public async Task<ExitCode> RunAsync(Func<Task> afterStarting)
return ExitCode.Ok;
}

if (AppConfig.MustCheckSingleInstance)
if (AppConfig.MustCheckSingleInstance && !AppConfig.Arguments.Contains("--disableSingleInstanceCheck"))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would remove this as it does not feel 'finished'.

Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

I was able to run two WW instances separately.

Once closing Wasabi the crash reporter shows up but I guess this is expected because of the argument --disableSingleInstanceCheck in the command to run.

2024-01-10 19:01:13.618 [1] CRITICAL    Program.Main (75)       System.NullReferenceException: Object reference not set to an instance of an object.
   at WalletWasabi.Services.SingleInstanceChecker.Dispose() in WalletWasabi\Services\SingleInstanceChecker.cs:line 219
   at WalletWasabi.Daemon.WasabiApplication.BeforeStopping() in WalletWasabi.Daemon\WasabiAppBuilder.cs:line 104
   at WalletWasabi.Daemon.WasabiApplication.RunAsync(Func`1 afterStarting) in WalletWasabi.Daemon\WasabiAppBuilder.cs:line 83
   at WalletWasabi.Fluent.Desktop.WasabiAppExtensions.RunAsGuiAsync(WasabiApplication app) in WalletWasabi.Fluent.Desktop\Program.cs:line 147
   at WalletWasabi.Fluent.Desktop.Program.Main(String[] args) in WalletWasabi.Fluent.Desktop\Program.cs:line 63

@MaxHillebrand
Copy link
Collaborator

Does this help running Wasabi on torified OS?

@kristapsk
Copy link
Collaborator

kristapsk commented Jan 10, 2024

How about --torSocksPort=auto and --torControlPort=auto, where it finds first free ports to use? We use such logic in JoinMarket for tests. https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/2978b1824555038cb2a393ae06c1b237377f9b35/src/jmbase/support.py#L346-L359

@kiminuo
Copy link
Collaborator Author

kiminuo commented Jan 10, 2024

Once closing Wasabi the crash reporter shows up but I guess this is expected because of the argument --disableSingleInstanceCheck in the command to run.

I want to remove that parameter. I'm not happy with the implementation of that part just yet. So yes, that's somewhat expected because this PR combines two features:

  • Configuring Tor ports
  • Running two WW instances

Does this help running Wasabi on torified OS?

It's a step towards that future. There is more work to do if we want to support1 those OSes.

Footnotes

  1. Not sure where on a roadmap it is.

@kiminuo kiminuo force-pushed the feature/2023-11-26-Tor-configurable-ports branch from 176567e to 882fec9 Compare January 10, 2024 19:49
@kiminuo kiminuo changed the title [PoC] Tor: Configurable ports Tor: Configurable ports Jan 10, 2024
@kiminuo kiminuo marked this pull request as ready for review January 10, 2024 19:50
@kiminuo
Copy link
Collaborator Author

kiminuo commented Jan 10, 2024

How about --torSocksPort=auto and --torControlPort=auto, where it finds first free ports to use? We use such logic in JoinMarket for tests. https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/2978b1824555038cb2a393ae06c1b237377f9b35/src/jmbase/support.py#L346-L359

It sounds good but I'm not sure if we want to spend time on it. The idea of this PR was that having Tor ports configurable adds some value because you can:

  • run integration tests on different ports than the default ones Wasabi uses
  • it may allow to use a pre-set Tor instance in one's system (for advanced users)
  • it may help us to support Torrified OSes in the future (where we know we need it (AFAIK)).

Supporting auto can be probably added, maybe easily, maybe it's a bit more involved. If you want to spend time on, it would be great, I want to focus on filter processing right now which I find more important at the moment.

@kristapsk
Copy link
Collaborator

Supporting auto can be probably added, maybe easily, maybe it's a bit more involved. If you want to spend time on, it would be great, I want to focus on filter processing right now which I find more important at the moment.

Maybe need to ask ChatGPT to translate that Python code from JoinMarket to C#. :) I will probably try, but also don't want to spend too much time on this.

@kristapsk
Copy link
Collaborator

Supporting auto can be probably added, maybe easily, maybe it's a bit more involved. If you want to spend time on, it would be great, I want to focus on filter processing right now which I find more important at the moment.

Maybe need to ask ChatGPT to translate that Python code from JoinMarket to C#. :) I will probably try, but also don't want to spend too much time on this.

This is suggestion from ChatGPT.

using System;
using System.Collections.Generic;
using System.Net;
using System.Net.Sockets;

public class PortUtils
{
    public static List<int> GetFreeTcpPorts(int numPorts)
    {
        List<int> ports = new List<int>();
        List<Socket> sockets = new List<Socket>();

        for (int i = 0; i < numPorts; i++)
        {
            Socket s = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
            s.Bind(new IPEndPoint(IPAddress.Any, 0));
            s.Listen(1);
            ports.Add(((IPEndPoint)s.LocalEndPoint).Port);
            sockets.Add(s);
        }

        foreach (Socket s in sockets)
        {
            s.Close();
        }

        return ports;
    }
}

Copy link
Collaborator

@adamPetho adamPetho left a comment

Choose a reason for hiding this comment

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

tACK, code LGTM

@kiminuo kiminuo merged commit 7d1c6f3 into WalletWasabi:master Jan 11, 2024
6 of 7 checks passed
@kiminuo kiminuo deleted the feature/2023-11-26-Tor-configurable-ports branch January 11, 2024 20:58
@nyxnor nyxnor mentioned this pull request Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants