From 16c5ba20f298b7d5e1e826813c2b3c6e77511339 Mon Sep 17 00:00:00 2001 From: Turbolay Date: Tue, 21 May 2024 14:33:22 -0600 Subject: [PATCH 1/6] Fix infinite loop in FetchTransactionsAsync in case of missing txs --- .../Controllers/BlockchainController.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/WalletWasabi.Backend/Controllers/BlockchainController.cs b/WalletWasabi.Backend/Controllers/BlockchainController.cs index 2d54438ab9..32b8e5c0aa 100644 --- a/WalletWasabi.Backend/Controllers/BlockchainController.cs +++ b/WalletWasabi.Backend/Controllers/BlockchainController.cs @@ -210,6 +210,20 @@ private async Task FetchTransactionsAsync(uint256[] txIds, Cancel { txIdsRetrieve[kvp.Key].TrySetResult(kvp.Value); } + + var stillMissing = txIdsRetrieve.Where(x => !rpcBatch.ContainsKey(x.Key)).ToList(); + if (stillMissing.Count > 0) + { + List exceptions = new(); + foreach (KeyValuePair> txStillMissingWithTcs in stillMissing) + { + var ex = new InvalidOperationException($"Transaction {txStillMissingWithTcs.Key} wasn't found."); + txStillMissingWithTcs.Value.SetException(ex); + exceptions.Add(ex); + } + + throw new AggregateException(exceptions); + } } Transaction[] result = new Transaction[requestCount]; From 862131171eb2a40797d28b5fdb2b74bd26fadfab Mon Sep 17 00:00:00 2001 From: Kiminuo Date: Wed, 22 May 2024 10:52:00 +0200 Subject: [PATCH 2/6] Different --- .../Controllers/BlockchainController.cs | 44 +++++++++++-------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/WalletWasabi.Backend/Controllers/BlockchainController.cs b/WalletWasabi.Backend/Controllers/BlockchainController.cs index 32b8e5c0aa..e5fd801d48 100644 --- a/WalletWasabi.Backend/Controllers/BlockchainController.cs +++ b/WalletWasabi.Backend/Controllers/BlockchainController.cs @@ -177,6 +177,7 @@ public async Task GetTransactionsAsync([FromQuery, Required] IEnu /// /// Fetches transactions from cache if possible and missing transactions are fetched using RPC. /// + /// If RPC client succeeds in getting some transactions but not all. private async Task FetchTransactionsAsync(uint256[] txIds, CancellationToken cancellationToken) { int requestCount = txIds.Length; @@ -204,6 +205,7 @@ private async Task FetchTransactionsAsync(uint256[] txIds, Cancel { // Ask to get missing transactions over RPC. IEnumerable txs = await RpcClient.GetRawTransactionsAsync(txIdsRetrieve.Keys, cancellationToken).ConfigureAwait(false); + Dictionary rpcBatch = txs.ToDictionary(x => x.GetHash(), x => x); foreach (KeyValuePair kvp in rpcBatch) @@ -211,17 +213,10 @@ private async Task FetchTransactionsAsync(uint256[] txIds, Cancel txIdsRetrieve[kvp.Key].TrySetResult(kvp.Value); } - var stillMissing = txIdsRetrieve.Where(x => !rpcBatch.ContainsKey(x.Key)).ToList(); - if (stillMissing.Count > 0) + // RPC client does not throw if a transaction is missing, so we need to account for this case. + if (txs.Count() < txIdsRetrieve.Count) { - List exceptions = new(); - foreach (KeyValuePair> txStillMissingWithTcs in stillMissing) - { - var ex = new InvalidOperationException($"Transaction {txStillMissingWithTcs.Key} wasn't found."); - txStillMissingWithTcs.Value.SetException(ex); - exceptions.Add(ex); - } - + IReadOnlyList exceptions = MarkNotFinishedTasksAsFailed(txIdsRetrieve); throw new AggregateException(exceptions); } } @@ -241,18 +236,29 @@ private async Task FetchTransactionsAsync(uint256[] txIds, Cancel { if (txIdsRetrieve.Count > 0) { - // It's necessary to always set a result to the task completion sources. Otherwise, cache can get corrupted. - Exception ex = new InvalidOperationException("Failed to get the transaction."); - foreach ((uint256 txid, TaskCompletionSource tcs) in txIdsRetrieve) + MarkNotFinishedTasksAsFailed(txIdsRetrieve); + } + } + + IReadOnlyList MarkNotFinishedTasksAsFailed(Dictionary> txIdsRetrieve) + { + IReadOnlyList? exceptions = null; + + // It's necessary to always set a result to the task completion sources. Otherwise, cache can get corrupted. + foreach ((uint256 txid, TaskCompletionSource tcs) in txIdsRetrieve) + { + if (!tcs.Task.IsCompleted) { - if (!tcs.Task.IsCompleted) - { - // Prefer new cache requests to try again rather than getting the exception. The window is small though. - Cache.Remove(txid); - tcs.SetException(ex); - } + exceptions ??= new List(); + + // Prefer new cache requests to try again rather than getting the exception. The window is small though. + Exception e = new InvalidOperationException($"Failed to get the transaction '{txid}'."); + Cache.Remove(txid); + tcs.SetException(e); } } + + return exceptions ?? []; } } From 6ea595bfaa6029be06a7c0d0d49892f6abf2ab01 Mon Sep 17 00:00:00 2001 From: Kiminuo Date: Wed, 22 May 2024 18:43:52 +0200 Subject: [PATCH 3/6] feedback --- WalletWasabi.Backend/Controllers/BlockchainController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WalletWasabi.Backend/Controllers/BlockchainController.cs b/WalletWasabi.Backend/Controllers/BlockchainController.cs index e5fd801d48..38776f5fb5 100644 --- a/WalletWasabi.Backend/Controllers/BlockchainController.cs +++ b/WalletWasabi.Backend/Controllers/BlockchainController.cs @@ -214,7 +214,7 @@ private async Task FetchTransactionsAsync(uint256[] txIds, Cancel } // RPC client does not throw if a transaction is missing, so we need to account for this case. - if (txs.Count() < txIdsRetrieve.Count) + if (rpcBatch.Count < txIdsRetrieve.Count) { IReadOnlyList exceptions = MarkNotFinishedTasksAsFailed(txIdsRetrieve); throw new AggregateException(exceptions); From 5985d7f0a4c28254580b60a37b61d32a2003ac97 Mon Sep 17 00:00:00 2001 From: Kiminuo Date: Wed, 22 May 2024 21:11:54 +0200 Subject: [PATCH 4/6] Feedback --- WalletWasabi.Backend/Controllers/BlockchainController.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/WalletWasabi.Backend/Controllers/BlockchainController.cs b/WalletWasabi.Backend/Controllers/BlockchainController.cs index 38776f5fb5..60ffab1856 100644 --- a/WalletWasabi.Backend/Controllers/BlockchainController.cs +++ b/WalletWasabi.Backend/Controllers/BlockchainController.cs @@ -242,17 +242,18 @@ private async Task FetchTransactionsAsync(uint256[] txIds, Cancel IReadOnlyList MarkNotFinishedTasksAsFailed(Dictionary> txIdsRetrieve) { - IReadOnlyList? exceptions = null; + List? exceptions = null; // It's necessary to always set a result to the task completion sources. Otherwise, cache can get corrupted. foreach ((uint256 txid, TaskCompletionSource tcs) in txIdsRetrieve) { if (!tcs.Task.IsCompleted) { - exceptions ??= new List(); + exceptions ??= new(); // Prefer new cache requests to try again rather than getting the exception. The window is small though. Exception e = new InvalidOperationException($"Failed to get the transaction '{txid}'."); + exceptions.Add(e); Cache.Remove(txid); tcs.SetException(e); } From b7cb91985dec4bc33a720d5d908bca15c65fdec0 Mon Sep 17 00:00:00 2001 From: Turbolay Date: Wed, 22 May 2024 19:07:52 -0600 Subject: [PATCH 5/6] Fix Cache removal --- WalletWasabi.Backend/Controllers/BlockchainController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WalletWasabi.Backend/Controllers/BlockchainController.cs b/WalletWasabi.Backend/Controllers/BlockchainController.cs index 60ffab1856..b95f2e0f88 100644 --- a/WalletWasabi.Backend/Controllers/BlockchainController.cs +++ b/WalletWasabi.Backend/Controllers/BlockchainController.cs @@ -254,7 +254,7 @@ IReadOnlyList MarkNotFinishedTasksAsFailed(Dictionary Date: Wed, 22 May 2024 19:08:05 -0600 Subject: [PATCH 6/6] Fix usage --- .../Controllers/BlockchainController.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/WalletWasabi.Backend/Controllers/BlockchainController.cs b/WalletWasabi.Backend/Controllers/BlockchainController.cs index b95f2e0f88..8e65eaeb1a 100644 --- a/WalletWasabi.Backend/Controllers/BlockchainController.cs +++ b/WalletWasabi.Backend/Controllers/BlockchainController.cs @@ -510,11 +510,19 @@ private async Task> BuildUn private async Task ComputeUnconfirmedTransactionChainItemAsync(uint256 currentTxId, IEnumerable mempoolHashes, CancellationToken cancellationToken) { - var currentTx = (await FetchTransactionsAsync([currentTxId], cancellationToken).ConfigureAwait(false)).FirstOrDefault() ?? throw new InvalidOperationException("Tx not found"); + var currentTx = (await FetchTransactionsAsync([currentTxId], cancellationToken).ConfigureAwait(false)).First(); var txsToFetch = currentTx.Inputs.Select(input => input.PrevOut.Hash).Distinct().ToArray(); - var parentTxs = await FetchTransactionsAsync(txsToFetch, cancellationToken).ConfigureAwait(false); + Transaction[] parentTxs; + try + { + parentTxs = await FetchTransactionsAsync(txsToFetch, cancellationToken).ConfigureAwait(false); + } + catch(AggregateException ex) + { + throw new InvalidOperationException($"Some transactions part of the chain were not found: {ex}"); + } // Get unconfirmed parents and children var unconfirmedParents = parentTxs.Where(x => mempoolHashes.Contains(x.GetHash())).ToHashSet();