Use different AliceClients for every coin #7608

molnard · 2022-03-22T15:30:32Z

Fixes:

AliceClients were disposed right after the ConnectionConfirmation phase.
Same Tor identity was used for all the AliceClients.

2022-03-21 22:22:10.196 [20] ERROR      CoinJoinManager.ExecuteAsync (187)      Wallet: `Test0312` - Coinjoin client failed with exception: Exception: WalletWasabi.Tor.Socks5.Exceptions.TorCircuitExpiredException: Exception of type 'WalletWasabi.Tor.Socks5.Exceptions.TorCircuitExpiredException' was thrown.
   at WalletWasabi.Tor.Socks5.Pool.TorHttpPool.ObtainFreeConnectionAsync(HttpRequestMessage request, ICircuit circuit, CancellationToken token) in WalletWasabi\Tor\Socks5\Pool\TorHttpPool.cs:line 236
   at WalletWasabi.Tor.Socks5.Pool.TorHttpPool.SendAsync(HttpRequestMessage request, ICircuit circuit, CancellationToken cancellationToken) in WalletWasabi\Tor\Socks5\Pool\TorHttpPool.cs:line 115
   at WalletWasabi.Tor.Http.TorHttpClient.SendAsync(HttpRequestMessage request, CancellationToken token) in WalletWasabi\Tor\Http\TorHttpClient.cs:line 79
   at WalletWasabi.Tor.Http.TorHttpClient.SendAsync(HttpMethod method, String relativeUri, HttpContent content, CancellationToken token) in WalletWasabi\Tor\Http\TorHttpClient.cs:line 66
   at WalletWasabi.WabiSabi.Client.WabiSabiHttpApiClient.SendWithRetriesAsync(RemoteAction action, String jsonString, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\WabiSabiHttpApiClient.cs:line 77
   at WalletWasabi.WabiSabi.Client.WabiSabiHttpApiClient.SendWithRetriesAsync[TRequest](RemoteAction action, TRequest request, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\WabiSabiHttpApiClient.cs:line 121
   at WalletWasabi.WabiSabi.Client.WabiSabiHttpApiClient.SendAndReceiveAsync[TRequest](RemoteAction action, TRequest request, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\WabiSabiHttpApiClient.cs:line 133
   at WalletWasabi.WabiSabi.Client.ArenaClient.ReadyToSignAsync(uint256 roundId, Guid aliceId, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\ArenaClient.cs:line 208
   at WalletWasabi.WabiSabi.Client.AliceClient.ReadyToSignAsync(CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\AliceClient.cs:line 239
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.<>c__DisplayClass45_0.<<ReadyToSignAsync>b__0>d.MoveNext() in WalletWasabi\WabiSabi\Client\CoinJoinClient.cs:line 293
--- End of stack trace from previous location ---
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.ReadyToSignAsync(IEnumerable`1 aliceClients, DateTimeOffset readyToSignEndTime, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\CoinJoinClient.cs:line 297
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.ProceedWithOutputRegistrationPhaseAsync(uint256 roundId, ImmutableArray`1 registeredAliceClients, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\CoinJoinClient.cs:line 520
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.StartRoundAsync(IEnumerable`1 smartCoins, RoundState roundState, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\CoinJoinClient.cs:line 165
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.StartCoinJoinAsync(IEnumerable`1 coinCandidates, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\CoinJoinClient.cs:line 128
   at WalletWasabi.WabiSabi.Client.CoinJoinManager.ExecuteAsync(CancellationToken stoppingToken) in WalletWasabi\WabiSabi\Client\CoinJoinManager.cs:line 164
2022-03-21 22:22:14.288 [38] ERROR      CoinJoinManager.ExecuteAsync (187)      Wallet: `TestThink` - Coinjoin client failed with exception: Exception: WalletWasabi.Tor.Socks5.Exceptions.TorCircuitExpiredException: Exception of type 'WalletWasabi.Tor.Socks5.Exceptions.TorCircuitExpiredException' was thrown.
   at WalletWasabi.Tor.Socks5.Pool.TorHttpPool.ObtainFreeConnectionAsync(HttpRequestMessage request, ICircuit circuit, CancellationToken token) in WalletWasabi\Tor\Socks5\Pool\TorHttpPool.cs:line 236
   at WalletWasabi.Tor.Socks5.Pool.TorHttpPool.SendAsync(HttpRequestMessage request, ICircuit circuit, CancellationToken cancellationToken) in WalletWasabi\Tor\Socks5\Pool\TorHttpPool.cs:line 115
   at WalletWasabi.Tor.Http.TorHttpClient.SendAsync(HttpRequestMessage request, CancellationToken token) in WalletWasabi\Tor\Http\TorHttpClient.cs:line 79
   at WalletWasabi.Tor.Http.TorHttpClient.SendAsync(HttpMethod method, String relativeUri, HttpContent content, CancellationToken token) in WalletWasabi\Tor\Http\TorHttpClient.cs:line 66
   at WalletWasabi.WabiSabi.Client.WabiSabiHttpApiClient.SendWithRetriesAsync(RemoteAction action, String jsonString, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\WabiSabiHttpApiClient.cs:line 77
   at WalletWasabi.WabiSabi.Client.WabiSabiHttpApiClient.SendWithRetriesAsync[TRequest](RemoteAction action, TRequest request, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\WabiSabiHttpApiClient.cs:line 121
   at WalletWasabi.WabiSabi.Client.WabiSabiHttpApiClient.SendAndReceiveAsync[TRequest](RemoteAction action, TRequest request, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\WabiSabiHttpApiClient.cs:line 133
   at WalletWasabi.WabiSabi.Client.ArenaClient.ReadyToSignAsync(uint256 roundId, Guid aliceId, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\ArenaClient.cs:line 208
   at WalletWasabi.WabiSabi.Client.AliceClient.ReadyToSignAsync(CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\AliceClient.cs:line 239
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.<>c__DisplayClass45_0.<<ReadyToSignAsync>b__0>d.MoveNext() in WalletWasabi\WabiSabi\Client\CoinJoinClient.cs:line 293
--- End of stack trace from previous location ---
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.ReadyToSignAsync(IEnumerable`1 aliceClients, DateTimeOffset readyToSignEndTime, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\CoinJoinClient.cs:line 297
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.ProceedWithOutputRegistrationPhaseAsync(uint256 roundId, ImmutableArray`1 registeredAliceClients, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\CoinJoinClient.cs:line 520
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.StartRoundAsync(IEnumerable`1 smartCoins, RoundState roundState, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\CoinJoinClient.cs:line 165
   at WalletWasabi.WabiSabi.Client.CoinJoinClient.StartCoinJoinAsync(IEnumerable`1 coinCandidates, CancellationToken cancellationToken) in WalletWasabi\WabiSabi\Client\CoinJoinClient.cs:line 128
   at WalletWasabi.WabiSabi.Client.CoinJoinManager.ExecuteAsync(CancellationToken stoppingToken) in WalletWasabi\WabiSabi\Client\CoinJoinManager.cs:line 164

lontivero · 2022-03-22T16:26:15Z

WalletWasabi/WabiSabi/Client/CoinJoinClient.cs

 			try
 			{
+				personCircuit = HttpClientFactory.NewHttpClientWithPersonCircuit(out Tor.Http.IHttpClient httpClient);


nothingmuch · 2022-03-22T16:28:22Z

wow could it be? is it exactly the thing i warned about repeatedly?

what a farce this project is.

lontivero · 2022-03-22T16:29:15Z

WalletWasabi/WabiSabi/Client/CoinJoinClient.cs

@@ -141,25 +141,28 @@ public async Task<CoinJoinResult> StartRoundAsync(IEnumerable<SmartCoin> smartCo
 	{
 		var roundId = roundState.Id;

-		ImmutableArray<AliceClient> registeredAliceClients = ImmutableArray<AliceClient>.Empty;
+		ImmutableArray<(AliceClient AliceClient, PersonCircuit PersonCircuit)> registeredAliceClientAndCircuits = ImmutableArray<(AliceClient, PersonCircuit)>.Empty;


The noise/signal ratio of this line is close to infinite.

I was thinking of adding PersonCircuit to AliceClient but it won't be wise because of the abstractions. So the cleanest cut is to handle them outside.

molnard · 2022-03-22T16:40:13Z

wow could it be? is it exactly the thing i warned about repeatedly?

This was exactly the right time to fix this.

molnard · 2022-03-23T10:16:06Z

(1) The most important change in this PR is the line:

https://github.com/zkSNACKs/WalletWasabi/pull/7608/files#diff-b4b0ed52d80da8067250690ccb013b4ba124c66ddbe1969236ac568759630031L563

The circuit was disposed of right after the connection confirmation.

(2) The second change was to use separate Tor identities for every coin.

Use different AliceClient for every coin

e45b462

pull-request-size bot added the size/M label Mar 22, 2022

molnard requested review from yahiheb, adamPetho and kiminuo March 22, 2022 15:59

molnard added the priority label Mar 22, 2022

lontivero reviewed Mar 22, 2022

View reviewed changes

lontivero approved these changes Mar 22, 2022

View reviewed changes

molnard merged commit 1b491be into zkSNACKs:master Mar 22, 2022

molnard deleted the aliceclientfix branch March 22, 2022 16:34

molnard mentioned this pull request Mar 22, 2022

Client do not try to participate in blame round #7541

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use different AliceClients for every coin #7608

Use different AliceClients for every coin #7608

molnard commented Mar 22, 2022 •

edited

lontivero Mar 22, 2022

nothingmuch commented Mar 22, 2022

lontivero Mar 22, 2022

molnard Mar 22, 2022

molnard commented Mar 22, 2022 •

edited

molnard commented Mar 23, 2022

Use different AliceClients for every coin #7608

Use different AliceClients for every coin #7608

Conversation

molnard commented Mar 22, 2022 • edited

lontivero Mar 22, 2022

Choose a reason for hiding this comment

nothingmuch commented Mar 22, 2022

lontivero Mar 22, 2022

Choose a reason for hiding this comment

molnard Mar 22, 2022

Choose a reason for hiding this comment

molnard commented Mar 22, 2022 • edited

molnard commented Mar 23, 2022

molnard commented Mar 22, 2022 •

edited

molnard commented Mar 22, 2022 •

edited