From 1b3949931d1323e2a9918e9376cb00fc68b8efbb Mon Sep 17 00:00:00 2001 From: Lucas Ontivero Date: Tue, 9 Jul 2024 14:13:38 -0300 Subject: [PATCH] More strict coinjoin parameters (#13239) * Reduce absolute max coordination fee to 0.005 and absolute min input count to 5 * Do not sign coinjoins where I am the only one. --- WalletWasabi.Daemon/Config.cs | 9 +++-- WalletWasabi.Daemon/Global.cs | 2 +- WalletWasabi.Tests/Helpers/WabiSabiFactory.cs | 2 +- .../UnitTests/Bases/ConfigManagerTests.cs | 4 +-- WalletWasabi/Helpers/Constants.cs | 4 +-- .../Client/CoinJoin/Client/CoinJoinClient.cs | 35 ++++++++++++------- .../CoinJoin/Manager/CoinJoinManager.cs | 2 +- 7 files changed, 37 insertions(+), 21 deletions(-) diff --git a/WalletWasabi.Daemon/Config.cs b/WalletWasabi.Daemon/Config.cs index 3dc6e7b8b5f..096b06bfde8 100644 --- a/WalletWasabi.Daemon/Config.cs +++ b/WalletWasabi.Daemon/Config.cs @@ -193,9 +193,14 @@ [ nameof(AbsoluteMinInputCount)] = ( public bool EnableGpu => GetEffectiveValue(nameof(EnableGpu)); public string CoordinatorIdentifier => GetEffectiveValue(nameof(CoordinatorIdentifier)); - public decimal MaxCoordinationFeeRate => GetEffectiveValue(nameof(MaxCoordinationFeeRate)); + public decimal MaxCoordinationFeeRate => decimal.Min( + GetEffectiveValue(nameof(MaxCoordinationFeeRate)), + Constants.AbsoluteMaxCoordinationFeeRate); public decimal MaxCoinjoinMiningFeeRate => GetEffectiveValue(nameof(MaxCoinjoinMiningFeeRate)); - public int AbsoluteMinInputCount => GetEffectiveValue(nameof(AbsoluteMinInputCount)); + public int AbsoluteMinInputCount => int.Max( + GetEffectiveValue(nameof(AbsoluteMinInputCount)), + Constants.AbsoluteMinInputCount); + public ServiceConfiguration ServiceConfiguration { get; } public static string DataDir { get; } = GetStringValue( diff --git a/WalletWasabi.Daemon/Global.cs b/WalletWasabi.Daemon/Global.cs index e34f66a08be..e6acbc669c5 100644 --- a/WalletWasabi.Daemon/Global.cs +++ b/WalletWasabi.Daemon/Global.cs @@ -400,7 +400,7 @@ private void RegisterCoinJoinComponents() Tor.Http.IHttpClient roundStateUpdaterHttpClient = CoordinatorHttpClientFactory.NewHttpClient(Mode.SingleCircuitPerLifetime, RoundStateUpdaterCircuit); HostedServices.Register(() => new RoundStateUpdater(TimeSpan.FromSeconds(10), new WabiSabiHttpApiClient(roundStateUpdaterHttpClient)), "Round info updater"); - var coinJoinConfiguration = new CoinJoinConfiguration(Config.CoordinatorIdentifier, Config.MaxCoordinationFeeRate, Config.MaxCoinjoinMiningFeeRate, Config.AbsoluteMinInputCount); + var coinJoinConfiguration = new CoinJoinConfiguration(Config.CoordinatorIdentifier, Config.MaxCoordinationFeeRate, Config.MaxCoinjoinMiningFeeRate, Config.AbsoluteMinInputCount, AllowSoloCoinjoining: false); HostedServices.Register(() => new CoinJoinManager(WalletManager, HostedServices.Get(), CoordinatorHttpClientFactory, HostedServices.Get(), coinJoinConfiguration, CoinPrison), "CoinJoin Manager"); } diff --git a/WalletWasabi.Tests/Helpers/WabiSabiFactory.cs b/WalletWasabi.Tests/Helpers/WabiSabiFactory.cs index e47b19f8f54..cf90958d95d 100644 --- a/WalletWasabi.Tests/Helpers/WabiSabiFactory.cs +++ b/WalletWasabi.Tests/Helpers/WabiSabiFactory.cs @@ -341,7 +341,7 @@ public static CoinJoinClient CreateTestCoinJoinClient( outputProvider, roundStateUpdater, coinSelector, - new CoinJoinConfiguration("CoinJoinCoordinatorIdentifier", 0.3m, 150.0m, 1), + new CoinJoinConfiguration("CoinJoinCoordinatorIdentifier", 0.3m, 150.0m, 1, AllowSoloCoinjoining: true), new LiquidityClueProvider(), TimeSpan.Zero, TimeSpan.Zero, diff --git a/WalletWasabi.Tests/UnitTests/Bases/ConfigManagerTests.cs b/WalletWasabi.Tests/UnitTests/Bases/ConfigManagerTests.cs index 220ffb7753e..e6a9f164c74 100644 --- a/WalletWasabi.Tests/UnitTests/Bases/ConfigManagerTests.cs +++ b/WalletWasabi.Tests/UnitTests/Bases/ConfigManagerTests.cs @@ -42,7 +42,7 @@ public async Task CheckFileChangeTestAsync() // Change coordination fee rate. { // Double coordination fee rate. - config.CoordinationFeeRate = new CoordinationFeeRate(rate: 0.006m); + config.CoordinationFeeRate = new CoordinationFeeRate(rate: 0.005m); // Change should be detected. Assert.True(ConfigManager.CheckFileChange(configPath, config)); @@ -50,7 +50,7 @@ public async Task CheckFileChangeTestAsync() // Now store and check that JSON is as expected. config.ToFile(); - string expectedFileContents = GetVanillaConfigString(coordinationFeeRate: 0.006m); + string expectedFileContents = GetVanillaConfigString(coordinationFeeRate: 0.005m); string actualFileContents = ReadAllTextAndNormalize(configPath); Assert.Equal(expectedFileContents, actualFileContents); diff --git a/WalletWasabi/Helpers/Constants.cs b/WalletWasabi/Helpers/Constants.cs index e80e18f1e0f..3a25316da9a 100644 --- a/WalletWasabi/Helpers/Constants.cs +++ b/WalletWasabi/Helpers/Constants.cs @@ -68,8 +68,8 @@ public static class Constants public const decimal DefaultMaxCoordinationFeeRate = 0.0m; public const decimal DefaultMaxCoinJoinMiningFeeRate = 150.0m; public const int DefaultAbsoluteMinInputCount = 21; - public const int AbsoluteMinInputCount = 2; - public const decimal AbsoluteMaxCoordinationFeeRate = 0.01m; + public const int AbsoluteMinInputCount = 5; + public const decimal AbsoluteMaxCoordinationFeeRate = 0.005m; public const string AlphaNumericCharacters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; public const string CapitalAlphaNumericCharacters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; diff --git a/WalletWasabi/WabiSabi/Client/CoinJoin/Client/CoinJoinClient.cs b/WalletWasabi/WabiSabi/Client/CoinJoin/Client/CoinJoinClient.cs index c8165d36d66..3af50a2581f 100644 --- a/WalletWasabi/WabiSabi/Client/CoinJoin/Client/CoinJoinClient.cs +++ b/WalletWasabi/WabiSabi/Client/CoinJoin/Client/CoinJoinClient.cs @@ -876,21 +876,32 @@ void OnOutputRegistrationErrors(DependencyGraphTaskScheduler.OutputRegistrationE // now when we identify as satoshi. // In this scenario we should ban the coordinator and stop dealing with it. // see more: https://github.com/WalletWasabi/WalletWasabi/issues/8171 - bool mustSignAllInputs = SanityCheck(outputTxOuts, unsignedCoinJoin.Transaction.Outputs); - if (!mustSignAllInputs) + var isItSoloCoinjoin = signingState.Inputs.Count() == registeredAliceClients.Length; + var isItForbiddenSoloCoinjoining = isItSoloCoinjoin && !CoinJoinConfiguration.AllowSoloCoinjoining; + if (isItForbiddenSoloCoinjoining) { - roundState.LogInfo($"There are missing outputs. A subset of inputs will be signed."); + roundState.LogInfo($"I am the only one in that coinjoin."); } - else + bool allMyOutputsArePresent = SanityCheck(outputTxOuts, unsignedCoinJoin.Transaction.Outputs); + + if (!allMyOutputsArePresent) { - // Assert that the effective fee rate is at least what was agreed on. - // Otherwise, coordinator could take some of the mining fees for itself. - // There is a tolerance because before constructing the transaction only an estimation can be computed. - mustSignAllInputs = signingState.EffectiveFeeRate.FeePerK.Satoshi > signingState.Parameters.MiningFeeRate.FeePerK.Satoshi * 0.90; - if (!mustSignAllInputs) - { - roundState.LogInfo($"Effective fee rate of the transaction is lower than expected. A subset of inputs will be signed."); - } + roundState.LogInfo($"There are missing outputs."); + } + + // Assert that the effective fee rate is at least what was agreed on. + // Otherwise, coordinator could take some of the mining fees for itself. + // There is a tolerance because before constructing the transaction only an estimation can be computed. + var isCoordinatorTakingExtraFees = signingState.EffectiveFeeRate.FeePerK.Satoshi <= signingState.Parameters.MiningFeeRate.FeePerK.Satoshi * 0.90; + if (isCoordinatorTakingExtraFees) + { + roundState.LogInfo($"Effective fee rate of the transaction is lower than expected."); + } + + var mustSignAllInputs = !isItForbiddenSoloCoinjoining && allMyOutputsArePresent && !isCoordinatorTakingExtraFees; + if (!mustSignAllInputs) + { + roundState.LogInfo($"A subset of inputs will be signed. "); } // Send signature. diff --git a/WalletWasabi/WabiSabi/Client/CoinJoin/Manager/CoinJoinManager.cs b/WalletWasabi/WabiSabi/Client/CoinJoin/Manager/CoinJoinManager.cs index 89a5b8c7750..dd39dab9d1d 100644 --- a/WalletWasabi/WabiSabi/Client/CoinJoin/Manager/CoinJoinManager.cs +++ b/WalletWasabi/WabiSabi/Client/CoinJoin/Manager/CoinJoinManager.cs @@ -783,4 +783,4 @@ private record CoinJoinClientStateHolder(CoinJoinClientState CoinJoinClientState private record UiBlockedStateHolder(bool NeedRestart, bool StopWhenAllMixed, bool OverridePlebStop, IWallet OutputWallet); } -public record CoinJoinConfiguration(string CoordinatorIdentifier, decimal MaxCoordinationFeeRate, decimal MaxCoinJoinMiningFeeRate, int AbsoluteMinInputCount); +public record CoinJoinConfiguration(string CoordinatorIdentifier, decimal MaxCoordinationFeeRate, decimal MaxCoinJoinMiningFeeRate, int AbsoluteMinInputCount, bool AllowSoloCoinjoining);