Skip to content

Commit

Permalink
More strict coinjoin parameters (WalletWasabi#13239)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
lontivero authored Jul 9, 2024
1 parent b628b15 commit 1b39499
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 21 deletions.
9 changes: 7 additions & 2 deletions WalletWasabi.Daemon/Config.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,14 @@ [ nameof(AbsoluteMinInputCount)] = (

public bool EnableGpu => GetEffectiveValue<BoolValue, bool>(nameof(EnableGpu));
public string CoordinatorIdentifier => GetEffectiveValue<StringValue, string>(nameof(CoordinatorIdentifier));
public decimal MaxCoordinationFeeRate => GetEffectiveValue<DecimalValue, decimal>(nameof(MaxCoordinationFeeRate));
public decimal MaxCoordinationFeeRate => decimal.Min(
GetEffectiveValue<DecimalValue, decimal>(nameof(MaxCoordinationFeeRate)),
Constants.AbsoluteMaxCoordinationFeeRate);
public decimal MaxCoinjoinMiningFeeRate => GetEffectiveValue<DecimalValue, decimal>(nameof(MaxCoinjoinMiningFeeRate));
public int AbsoluteMinInputCount => GetEffectiveValue<IntValue, int>(nameof(AbsoluteMinInputCount));
public int AbsoluteMinInputCount => int.Max(
GetEffectiveValue<IntValue, int>(nameof(AbsoluteMinInputCount)),
Constants.AbsoluteMinInputCount);

public ServiceConfiguration ServiceConfiguration { get; }

public static string DataDir { get; } = GetStringValue(
Expand Down
2 changes: 1 addition & 1 deletion WalletWasabi.Daemon/Global.cs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ private void RegisterCoinJoinComponents()
Tor.Http.IHttpClient roundStateUpdaterHttpClient = CoordinatorHttpClientFactory.NewHttpClient(Mode.SingleCircuitPerLifetime, RoundStateUpdaterCircuit);
HostedServices.Register<RoundStateUpdater>(() => 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<CoinJoinManager>(() => new CoinJoinManager(WalletManager, HostedServices.Get<RoundStateUpdater>(), CoordinatorHttpClientFactory, HostedServices.Get<WasabiSynchronizer>(), coinJoinConfiguration, CoinPrison), "CoinJoin Manager");
}

Expand Down
2 changes: 1 addition & 1 deletion WalletWasabi.Tests/Helpers/WabiSabiFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions WalletWasabi.Tests/UnitTests/Bases/ConfigManagerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ 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));

// 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);
Expand Down
4 changes: 2 additions & 2 deletions WalletWasabi/Helpers/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
35 changes: 23 additions & 12 deletions WalletWasabi/WabiSabi/Client/CoinJoin/Client/CoinJoinClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

0 comments on commit 1b39499

Please sign in to comment.