Defend too-long-mempool-chain attack.

fixes #904
fixes #936

WalletWasabi.Backend/Controllers/ChaumianCoinJoinController.cs

@@ -1,7 +1,9 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 using NBitcoin;
+using NBitcoin.Protocol;
 using NBitcoin.RPC;
+using Newtonsoft.Json.Linq;
 using Nito.AsyncEx;
 using System;
 using System.Collections.Generic;
@@ -12,6 +14,7 @@
 using WalletWasabi.Backend.Models.Requests;
 using WalletWasabi.Backend.Models.Responses;
 using WalletWasabi.Crypto;
+using WalletWasabi.Helpers;
 using WalletWasabi.Logging;
 using WalletWasabi.Models;
 using WalletWasabi.Models.ChaumianCoinJoin;
@@ -178,10 +181,14 @@ public async Task<IActionResult> PostInputsAsync([FromBody]InputsRequest request
 							{
 								return BadRequest("Provided input is neither confirmed, nor is from an unconfirmed coinjoin.");
 							}
-							// After 24 unconfirmed cj in the mempool dont't let unconfirmed coinjoin to be registered.
-							if (await Coordinator.IsUnconfirmedCoinJoinLimitReachedAsync())
+
+							// Check if mempool would accept a fake transaction created with the registered inputs.
+							// This will catch ascendant/descendant count and size limits for example.
+							var result = await RpcClient.TestMempoolAcceptAsync(new Coin(inputProof.Input.ToOutPoint(), getTxOutResponse.TxOut));
+
+							if (!result.accept)
 							{
-								return BadRequest("Provided input is from an unconfirmed coinjoin, but the maximum number of unconfirmed coinjoins is reached.");
+								return BadRequest($"Provided input is from an unconfirmed coinjoin, but a limit is reached: {result.rejectReason}");
 							}
 						}
 
@@ -261,7 +268,7 @@ public async Task<IActionResult> PostInputsAsync([FromBody]InputsRequest request
 					// Progress round if needed.
 					if (round.CountAlices() >= round.AnonymitySet)
 					{
-						await round.RemoveAlicesIfInputsSpentAsync();
+						await round.RemoveAlicesIfAnInputRefusedByMempoolAsync();
 
 						if (round.CountAlices() >= round.AnonymitySet)
 						{
@@ -330,7 +337,7 @@ public async Task<IActionResult> PostConfirmationAsync([FromQuery]string uniqueI
 						// Progress round if needed.
 						if (round.AllAlices(AliceState.ConnectionConfirmed))
 						{
-							IEnumerable<Alice> alicesToBan = await round.RemoveAlicesIfInputsSpentAsync(); // So ban only those who confirmed participation, yet spent their inputs.
+							IEnumerable<Alice> alicesToBan = await round.RemoveAlicesIfAnInputRefusedByMempoolAsync(); // So ban only those who confirmed participation, yet spent their inputs.
 
 							if (alicesToBan.Any())
 							{

WalletWasabi/Extensions/RPCClientExtensions.cs

@@ -1,8 +1,10 @@
+using Newtonsoft.Json.Linq;
 using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
 using System.Threading.Tasks;
+using WalletWasabi.Helpers;
 using WalletWasabi.Logging;
 using WalletWasabi.Models;
 
@@ -124,5 +126,33 @@ public static async Task<RPCResponse> TestMempoolAcceptAsync(this RPCClient rpc,
 
 			return resp;
 		}
+
+		/// <returns>(allowed, reject-reason)</returns>
+		public static async Task<(bool accept, string rejectReason)> TestMempoolAcceptAsync(this RPCClient rpc, Coin coin)
+		{
+			// Check if mempool would accept a fake transaction created with the registered inputs.
+			// This will catch ascendant/descendant count and size limits for example.
+			var fakeTransaction = rpc.Network.CreateTransaction();
+			fakeTransaction.Inputs.Add(new TxIn(coin.Outpoint));
+			Money fakeOutputValue = NBitcoinHelpers.TakeAReasonableFee(coin.TxOut.Value);
+			fakeTransaction.Outputs.Add(fakeOutputValue, new Key());
+			RPCResponse testMempoolAcceptResponse = await rpc.TestMempoolAcceptAsync(allowHighFees: true, fakeTransaction);
+
+			JToken first = testMempoolAcceptResponse.Result[0];
+			bool allowed = first["allowed"].Value<bool>();
+
+			var rejectedReason = string.Empty;
+			if (!allowed)
+			{
+				string rejected = first["reject-reason"].Value<string>();
+
+				if (!(rejected.Contains("mandatory-script-verify-flag-failed", StringComparison.OrdinalIgnoreCase)
+					|| rejected.Contains("non-mandatory-script-verify-flag", StringComparison.OrdinalIgnoreCase)))
+				{
+					return (false, rejected);
+				}
+			}
+			return (true, "");
+		}
 	}
 }

WalletWasabi/Helpers/NBitcoinHelpers.cs

@@ -37,5 +37,23 @@ public static BitcoinAddress ParseBitcoinAddress(string address)
 				}
 			}
 		}
+
+		public static Money TakeAReasonableFee(Money outputValue)
+		{
+			Money fee = Money.Coins(0.002m);
+			var remaining = Money.Zero;
+
+			while (true)
+			{
+				remaining = outputValue - fee;
+				if (remaining > Money.Coins(0.00001m))
+				{
+					break;
+				}
+				fee = fee.Percentange(50);
+			}
+
+			return remaining;
+		}
 	}
 }

WalletWasabi/Models/ChaumianCoinJoin/CcjRound.cs

@@ -1,5 +1,6 @@
 using NBitcoin;
 using NBitcoin.RPC;
+using Newtonsoft.Json.Linq;
 using Nito.AsyncEx;
 using System;
 using System.Collections.Generic;
@@ -427,7 +428,7 @@ public async Task ExecuteNextPhaseAsync(CcjRoundPhase expectedPhase, Money feePe
 									{
 										// Only abort if less than two one Alice is registered.
 										// Don't ban anyone, it's ok if they lost connection.
-										await RemoveAlicesIfInputsSpentAsync();
+										await RemoveAlicesIfAnInputRefusedByMempoolAsync();
 										int aliceCountAfterInputRegistrationTimeout = CountAlices();
 										if (aliceCountAfterInputRegistrationTimeout < 2)
 										{
@@ -451,7 +452,7 @@ public async Task ExecuteNextPhaseAsync(CcjRoundPhase expectedPhase, Money feePe
 										// THEN connection confirmation will go with 2 alices in every round
 										// Therefore Alices those didn't confirm, nor requested dsconnection should be banned:
 										IEnumerable<Alice> alicesToBan1 = GetAlicesBy(AliceState.InputsRegistered);
-										IEnumerable<Alice> alicesToBan2 = await RemoveAlicesIfInputsSpentAsync(); // So ban only those who confirmed participation, yet spent their inputs.
+										IEnumerable<Alice> alicesToBan2 = await RemoveAlicesIfAnInputRefusedByMempoolAsync(); // So ban only those who confirmed participation, yet spent their inputs.
 
 										IEnumerable<OutPoint> inputsToBan = alicesToBan1.SelectMany(x => x.Inputs).Select(y => y.Outpoint).Concat(alicesToBan2.SelectMany(x => x.Inputs).Select(y => y.Outpoint)).Distinct();
 
@@ -851,9 +852,10 @@ public int RemoveAlicesBy(AliceState state)
 			return numberOfRemovedAlices;
 		}
 
-		public async Task<IEnumerable<Alice>> RemoveAlicesIfInputsSpentAsync()
+		public async Task<IEnumerable<Alice>> RemoveAlicesIfAnInputRefusedByMempoolAsync()
 		{
 			var alicesRemoved = new List<Alice>();
+			var key = new Key();
 
 			using (RoundSynchronizerLock.Lock())
 			{
@@ -864,12 +866,13 @@ public async Task<IEnumerable<Alice>> RemoveAlicesIfInputsSpentAsync()
 
 				foreach (Alice alice in Alices)
 				{
-					foreach (OutPoint input in alice.Inputs.Select(y => y.Outpoint))
+					foreach (Coin input in alice.Inputs)
 					{
-						GetTxOutResponse getTxOutResponse = await RpcClient.GetTxOutAsync(input.Hash, (int)input.N, includeMempool: true);
+						// Check if mempool would accept a fake transaction created with the registered inputs.
+						// This will catch ascendant/descendant count and size limits for example.
+						var result = await RpcClient.TestMempoolAcceptAsync(input);
 
-						// Check if inputs are unspent.
-						if (getTxOutResponse is null)
+						if (!result.accept)
 						{
 							alicesRemoved.Add(alice);
 							Alices.Remove(alice);

WalletWasabi/Services/CcjCoordinator.cs

@@ -19,7 +19,6 @@ public class CcjCoordinator : IDisposable
 		private List<CcjRound> Rounds { get; }
 		private AsyncLock RoundsListLock { get; }
 
-		private List<uint256> UnconfirmedCoinJoins { get; }
 		private List<uint256> CoinJoins { get; }
 		public string CoinJoinsFilePath => Path.Combine(FolderPath, $"CoinJoins{Network}.txt");
 		private AsyncLock CoinJoinsLock { get; }
@@ -49,7 +48,6 @@ public CcjCoordinator(Network network, string folderPath, RPCClient rpc, CcjRoun
 			RoundsListLock = new AsyncLock();
 
 			CoinJoins = new List<uint256>();
-			UnconfirmedCoinJoins = new List<uint256>();
 			CoinJoinsLock = new AsyncLock();
 
 			Directory.CreateDirectory(FolderPath);
@@ -83,10 +81,6 @@ public CcjCoordinator(Network network, string folderPath, RPCClient rpc, CcjRoun
 							uint256 txHash = new uint256(line);
 							RPCResponse getRawTransactionResponse = RpcClient.SendCommand(RPCOperations.getrawtransaction, txHash.ToString(), true);
 							CoinJoins.Add(txHash);
-							if (getRawTransactionResponse.Result.Value<int>("confirmations") <= 0)
-							{
-								UnconfirmedCoinJoins.Add(txHash);
-							}
 						}
 						catch (Exception ex)
 						{
@@ -348,40 +342,6 @@ public int GetCoinJoinCount()
 			return CoinJoins.Count;
 		}
 
-		public async Task<bool> IsUnconfirmedCoinJoinLimitReachedAsync()
-		{
-			using (await CoinJoinsLock.LockAsync())
-			{
-				if (UnconfirmedCoinJoins.Count < 24)
-				{
-					return false;
-				}
-				foreach (var cjHash in UnconfirmedCoinJoins.ToArray())
-				{
-					try
-					{
-						var txInfo = await RpcClient.GetRawTransactionInfoAsync(cjHash);
-
-						// if confirmed remove only from unconfirmed
-						if (txInfo.Confirmations > 0)
-						{
-							UnconfirmedCoinJoins.Remove(cjHash);
-						}
-					}
-					catch (Exception ex)
-					{
-						// If aborted remove from everywhere (should not happen normally).
-						UnconfirmedCoinJoins.Remove(cjHash);
-						CoinJoins.Remove(cjHash);
-						await File.WriteAllLinesAsync(CoinJoinsFilePath, CoinJoins.Select(x => x.ToString()));
-						Logger.LogWarning<CcjCoordinator>(ex);
-					}
-				}
-
-				return UnconfirmedCoinJoins.Count >= 24;
-			}
-		}
-
 		#region IDisposable Support
 
 		private volatile bool _disposedValue = false; // To detect redundant calls