Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[VDG] Fix less option sends the same amount #10332

Merged
merged 8 commits into from Mar 30, 2023
Merged

[VDG] Fix less option sends the same amount #10332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4235ee0
fix text
soosr Mar 22, 2023
85b71f8
add GetAmount() method
soosr Mar 22, 2023
baed8d6
move check
soosr Mar 22, 2023
d43c3da
fix the case when the suggestions has the same amount as the original…
soosr Mar 22, 2023
090ca63
cleanup
soosr Mar 22, 2023
f87617e
better wording
soosr Mar 22, 2023
5866065
only remove the change avoidance suggestions
soosr Mar 22, 2023
f1a6c86
Update WalletWasabi.Fluent/ViewModels/Wallets/Send/ChangeAvoidanceSug…
soosr Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 23 additions & 26 deletions WalletWasabi.Fluent/ViewModels/Wallets/Send/ChangeAvoidanceSuggestionViewModel.cs
View file
Open in desktop
@@ -1,7 +1,6 @@
using NBitcoin;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -18,7 +17,7 @@ public partial class ChangeAvoidanceSuggestionViewModel : SuggestionViewModel
{
[AutoNotify] private string _amount;
[AutoNotify] private decimal _amountFiat;
[AutoNotify] private string? _differenceFiat;
[AutoNotify] private string? _differenceFiatText;

public ChangeAvoidanceSuggestionViewModel(
decimal originalAmount,
Expand All @@ -27,26 +26,34 @@ public partial class ChangeAvoidanceSuggestionViewModel : SuggestionViewModel
{
TransactionResult = transactionResult;

var totalAmount = transactionResult.CalculateDestinationAmount();
var totalAmount = GetAmount();
var total = totalAmount.ToDecimal(MoneyUnit.BTC);

var fiatTotal = total * fiatExchangeRate;

_amountFiat = fiatTotal;

var fiatOriginal = originalAmount * fiatExchangeRate;
var fiatDifference = fiatTotal - fiatOriginal;

_differenceFiat = (fiatDifference > 0
? $"{fiatDifference.ToUsd()} More"
: $"{Math.Abs(fiatDifference).ToUsd()} Less")
.Replace("(", "").Replace(")", "");

_amountFiat = fiatTotal;
_differenceFiatText = GetDifferenceFiatText(fiatDifference);
_amount = $"{totalAmount.ToFormattedString()} BTC";
}

public BuildTransactionResult TransactionResult { get; }

public Money GetAmount()
{
return TransactionResult.CalculateDestinationAmount();
}

private string GetDifferenceFiatText(decimal fiatDifference)
{
return fiatDifference switch
{
> 0 => $"{fiatDifference.ToUsd()} More",
< 0 => $"{Math.Abs(fiatDifference).ToUsd()} Less",
_ => "Same Amount"
soosr marked this conversation as resolved.
Show resolved Hide resolved
};
}

public static async IAsyncEnumerable<ChangeAvoidanceSuggestionViewModel> GenerateSuggestionsAsync(
TransactionInfo transactionInfo,
Wallet wallet,
Expand All @@ -62,8 +69,6 @@ public partial class ChangeAvoidanceSuggestionViewModel : SuggestionViewModel
maxInputCount,
cancellationToken);

HashSet<Money> foundSolutionsByAmount = new();

await foreach (IEnumerable<SmartCoin> selection in selectionsTask.ConfigureAwait(false))
{
BuildTransactionResult? transaction = null;
Expand All @@ -85,18 +90,10 @@ await foreach (IEnumerable<SmartCoin> selection in selectionsTask.ConfigureAwait

if (transaction is not null)
{
Money destinationAmount = transaction.CalculateDestinationAmount();

// If BnB solutions become the same transaction somehow, do not show the same suggestion twice.
if (!foundSolutionsByAmount.Contains(destinationAmount))
{
foundSolutionsByAmount.Add(destinationAmount);

yield return new ChangeAvoidanceSuggestionViewModel(
transactionInfo.Amount.ToDecimal(MoneyUnit.BTC),
transaction,
usdExchangeRate);
}
yield return new ChangeAvoidanceSuggestionViewModel(
transactionInfo.Amount.ToDecimal(MoneyUnit.BTC),
transaction,
usdExchangeRate);
}
}
}
Expand Down
19 changes: 19 additions & 0 deletions WalletWasabi.Fluent/ViewModels/Wallets/Send/PrivacySuggestionsFlyoutViewModel.cs
View file
Open in desktop
Expand Up @@ -6,7 +6,9 @@
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using DynamicData;
using WalletWasabi.Blockchain.TransactionBuilding;
using WalletWasabi.Fluent.Extensions;
using WalletWasabi.Fluent.Helpers;
using WalletWasabi.Logging;
using WalletWasabi.Wallets;
Expand Down Expand Up @@ -90,6 +92,23 @@

await foreach (var suggestion in suggestions)
{
var changeAvoidanceSuggestions = Suggestions.OfType<ChangeAvoidanceSuggestionViewModel>().ToArray();
var newSuggestionAmount = suggestion.GetAmount();

// If BnB solutions become the same transaction somehow, do not show the same suggestion twice.
if (changeAvoidanceSuggestions.Any(x => x.GetAmount() == newSuggestionAmount))
{
continue;
}

// If BnB solution has the same amount as the original transaction, only suggest that one and stop searching.
if (newSuggestionAmount == transaction.CalculateDestinationAmount())
{
Suggestions.RemoveMany(changeAvoidanceSuggestions);
Suggestions.Add(suggestion);
return;
}

Check warning on line 111 in WalletWasabi.Fluent/ViewModels/Wallets/Send/PrivacySuggestionsFlyoutViewModel.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

❌ New issue: Complex Method 

BuildPrivacySuggestionsAsync has a cyclomatic complexity of 10, threshold = 9
Suggestions.Add(suggestion);
}
}
Expand Down
2 changes: 1 addition & 1 deletion WalletWasabi.Fluent/Views/Wallets/Send/ChangeAvoidanceSuggestionControl.axaml
View file
Open in desktop
Expand Up @@ -19,7 +19,7 @@

<TextBlock TextWrapping="Wrap" Text="Avoid Change by sending:" TextAlignment="Center" />
<StackPanel Spacing="10">
<TextBlock Text="{Binding DifferenceFiat}" FontSize="20" HorizontalAlignment="Center"
<TextBlock Text="{Binding DifferenceFiatText}" FontSize="20" HorizontalAlignment="Center"
Foreground="{DynamicResource SystemAccentColor}" />
<TextBlock Text="{Binding Amount}" FontSize="12" HorizontalAlignment="Center" Opacity="0.6" />
<TextBlock Text="{Binding AmountFiat, Converter={x:Static converters:MoneyConverters.ToUsd}}" FontSize="12" HorizontalAlignment="Center" Opacity="0.6" />
Expand Down