Make sure no output have less value
ADD
WalletWasabi.Tests/UnitTests/WabiSabi/Client/CoinJoinClientTests.cs
+using System.Threading.Tasks;
+using NBitcoin;
+using WalletWasabi.Tests.Helpers;
+using WalletWasabi.WabiSabi.Client;
+using Xunit;
+
+namespace WalletWasabi.Tests.UnitTests.WabiSabi.Client;
+
+public class CoinJoinClientTests
+{
+ [Fact]
+ public void SanityCheckTest()
+ {
+ var output1 = new TxOut(Money.Coins(1), BitcoinFactory.CreateScript());
+ var output2 = new TxOut(Money.Coins(2), BitcoinFactory.CreateScript());
+ var output3 = new TxOut(Money.Coins(3), BitcoinFactory.CreateScript());
+ var output4 = new TxOut(Money.Coins(4), BitcoinFactory.CreateScript());
+
+ // Exact match (one expected)
+ Assert.True(CoinJoinClient.SanityCheck(
+ new[] { output1 },
+ new[] { output1, output2, output3, output4 }));
+
+ // Exact match (two expected)
+ Assert.True(CoinJoinClient.SanityCheck(
+ new[] { output2, output3 },
+ new[] { output1, output2, output3, output4 }));
+
+ // Missing output
+ Assert.False(CoinJoinClient.SanityCheck(
+ new[] { output2, output3 },
+ new[] { output1, output2, output4 }));
+
+ static TxOut AddSats(long sats, TxOut output) => new(output.Value + sats, output.ScriptPubKey);
+ static TxOut AddOneSat(TxOut output) => AddSats(1, output);
+ static TxOut SubOneSat(TxOut output) => AddSats(-1, output);
+
+ // More money in one output
+ Assert.True(CoinJoinClient.SanityCheck(
+ new[] { output2, output3 },
+ new[] { output1, AddOneSat(output2), output3, output4 }));
+
+ // More money in all output
+ Assert.True(CoinJoinClient.SanityCheck(
+ new[] { output2, output3 },
+ new[] { output1, AddOneSat(output2), AddOneSat(output3), output4 }));
+
+ // Same scriptpubkeys, same amount of money but outputs were manipulated
+ Assert.False(CoinJoinClient.SanityCheck(
+ new[] { output2, output3 },
+ new[] { output1, AddOneSat(output2), SubOneSat(output3), output4 }));
+ }
+}

MODIFY
WalletWasabi/WabiSabi/Client/CoinJoinClient.cs
WalletWasabi/WabiSabi/Client/CoinJoinClient.cs
@@ -499,22 +499,23 @@ public class CoinJoinClient
arenaRequestHandler));
}
- private bool SanityCheck(IEnumerable<TxOut> expectedOutputs, Transaction unsignedCoinJoinTransaction)
+ internal static bool SanityCheck(IEnumerable<TxOut> expectedOutputs, IEnumerable<TxOut> coinJoinOutputs)
{
- var coinJoinOutputs = unsignedCoinJoinTransaction.Outputs;
- var coinjoinOutputScripts = coinJoinOutputs.Select(x => x.ScriptPubKey);
- var expectedOutputScripts = expectedOutputs.Select(x => x.ScriptPubKey);
- var allMyScriptsAreThere = coinjoinOutputScripts.IsSuperSetOf(expectedOutputScripts);
+ bool AllExpectedScriptsArePresent() =>
+ coinJoinOutputs
+ .Select(x => x.ScriptPubKey)
+ .IsSuperSetOf(expectedOutputs.Select(x => x.ScriptPubKey));
- var expectedScriptHashSet = coinjoinOutputScripts.ToHashSet();
- var receivedMoney = coinJoinOutputs
- .Where(x => expectedScriptHashSet.Contains(x.ScriptPubKey))
- .Sum(x => x.Value);
+ bool AllOutputsHaveAtLeastTheExpectedValue() =>
+ coinJoinOutputs
+ .Join(
+ expectedOutputs,
+ x => x.ScriptPubKey,
+ x => x.ScriptPubKey,
+ (coinjoinOutput, expectedOutput) => coinjoinOutput.Value - expectedOutput.Value)
+ .All(x => x >= 0);
- var expectedMoneyToReceive = expectedOutputs.Sum(x => x.Value);
- var allMyMoneyIsThere = receivedMoney >= expectedMoneyToReceive;
-
- return allMyScriptsAreThere && allMyMoneyIsThere;
+ return AllExpectedScriptsArePresent() && AllOutputsHaveAtLeastTheExpectedValue();
}
private async Task SignTransactionAsync(
@@ -1122,7 +1123,7 @@ public class CoinJoinClient
// now when we identify as satoshi.
// In this scenario we should ban the coordinator and stop dealing with it.
// see more: https://github.com/zkSNACKs/WalletWasabi/issues/8171
- bool mustSignAllInputs = SanityCheck(outputTxOuts, unsignedCoinJoin.Transaction);
+ bool mustSignAllInputs = SanityCheck(outputTxOuts, unsignedCoinJoin.Transaction.Outputs);
if (!mustSignAllInputs)
{
roundState.LogInfo($"There are missing outputs. A subset of inputs will be signed.");

Make sure no output have less value