First refactoring and test TransactionFactory
ADD
WalletWasabi.Tests/UnitTests/TransactionFactoryTests.cs
+using NBitcoin;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using WalletWasabi.Exceptions;
+using WalletWasabi.KeyManagement;
+using WalletWasabi.Models;
+using WalletWasabi.Models.TransactionBuilding;
+using WalletWasabi.Services;
+using Xunit;
+
+namespace WalletWasabi.Tests.UnitTests
+{
+ public class TransactionFactoryTests
+ {
+ [Fact]
+ public void InsufficientBalanceTest()
+ {
+ var transactionFactory = CreateDefaultTransactionFactory(new[]{
+ ("Pablo", 0, 0.01m, confirmed: true, anonymitySet: 1),
+ ("Jean", 1, 0.02m, confirmed: true, anonymitySet: 1)
+ });
+
+ var amount = Money.Coins(100m);
+ var payment = new PaymentIntent(new Key().ScriptPubKey, amount);
+
+ var ex = Assert.Throws<InsufficientBalanceException>(() => transactionFactory.BuildTransaction(payment, new FeeRate(2m)));
+
+ Assert.Equal(ex.Minimum, amount);
+ Assert.Equal(ex.Actual, transactionFactory.Coins.Select(x => x.Amount).Sum());
+ }
+
+ [Fact]
+ public void TooMuchFeePaid()
+ {
+ var transactionFactory = CreateDefaultTransactionFactory(new[]{
+ ("Pablo", 0, 0.0001m, confirmed: true, anonymitySet: 1),
+ });
+
+ var payment = new PaymentIntent(new Key().ScriptPubKey, MoneyRequest.CreateAllRemaining(subtractFee: true));
+
+ var result = transactionFactory.BuildTransaction(payment, new FeeRate(44.25m));
+ var output = Assert.Single(result.OuterWalletOutputs);
+ Assert.Equal(result.Fee, output.Amount); // edge case! paid amout equal to paid fee
+
+ var ex = Assert.Throws<InvalidOperationException>(() => transactionFactory.BuildTransaction(payment, new FeeRate(50m)));
+ Assert.StartsWith("The transaction fee is more than twice the transaction amount", ex.Message);
+ }
+
+ [Fact]
+ public void SelectMostPrivateCoin()
+ {
+ var transactionFactory = CreateDefaultTransactionFactory(new[]{
+ ("Maria", 0, 0.08m, confirmed: true, anonymitySet: 50),
+ ("Joseph", 1, 0.16m, confirmed: true, anonymitySet: 200),
+ });
+
+ // There is a 0.8 coin with AS=100. However it selects the most private one with AS= 200
+ var destination = new Key().ScriptPubKey;
+ var payment = new PaymentIntent(destination, Money.Coins(0.07m), label: new SmartLabel("Sophie"));
+ var feeRate = new FeeRate(2m);
+ var result = transactionFactory.BuildTransaction(payment, feeRate);
+
+ Assert.True(result.Signed);
+ var spentCoin = Assert.Single(result.SpentCoins);
+ Assert.Equal(Money.Coins(0.16m), spentCoin.Amount);
+ Assert.Equal(200, spentCoin.AnonymitySet);
+ Assert.False(result.SpendsUnconfirmed);
+ var tx = result.Transaction.Transaction;
+ Assert.Equal(2, tx.Outputs.Count());
+
+ var changeCoin = Assert.Single(result.InnerWalletOutputs);
+ Assert.True(changeCoin.HdPubKey.IsInternal);
+ Assert.Equal("Sophie", changeCoin.Label.ToString()); // Shouldn't this say: "Sophie, Joseph"????
+ }
+
+ [Fact]
+ public void SelectMostPrivateCoins()
+ {
+ var transactionFactory = CreateDefaultTransactionFactory(new[]{
+ ("Pablo", 0, 0.01m, confirmed: true, anonymitySet: 1),
+ ("Jean", 1, 0.02m, confirmed: true, anonymitySet: 1),
+ ("Daniel", 2, 0.04m, confirmed: true, anonymitySet: 100),
+ ("Maria", 3, 0.08m, confirmed: true, anonymitySet: 50),
+ ("Joseph", 4, 0.16m, confirmed: true, anonymitySet: 200),
+ });
+
+ // Select more than one coins
+ var payment = new PaymentIntent(new Key().ScriptPubKey, Money.Coins(0.17m));
+ var feeRate = new FeeRate(2m);
+ var result = transactionFactory.BuildTransaction(payment, feeRate);
+
+ Assert.True(result.Signed);
+ Assert.Equal(2, result.SpentCoins.Count());
+ var spentCoin200 = Assert.Single(result.SpentCoins, x => x.AnonymitySet == 200);
+ var spentCoin100 = Assert.Single(result.SpentCoins, x => x.AnonymitySet == 100);
+
+ Assert.Equal(Money.Coins(0.16m), spentCoin200.Amount);
+ Assert.Equal(Money.Coins(0.04m), spentCoin100.Amount);
+
+ Assert.False(result.SpendsUnconfirmed);
+ var tx = result.Transaction.Transaction;
+ Assert.Equal(2, tx.Outputs.Count());
+ }
+
+ [Fact]
+ public void SelectSameScriptPubKeyCoins()
+ {
+ var transactionFactory = CreateDefaultTransactionFactory(new[]{
+ ("Pablo", 0, 0.01m, confirmed: false, anonymitySet: 50),
+ ("Jean", 1, 0.02m, confirmed: false, anonymitySet: 1),
+ ("Daniel", 1, 0.04m, confirmed: true, anonymitySet: 1),
+ ("Maria", 2, 0.08m, confirmed: true, anonymitySet: 100),
+ });
+
+ // Selecting 0.08 + 0.04 should be enough but it has to select 0.02 too because it is the same address
+ var payment = new PaymentIntent(new Key().ScriptPubKey, Money.Coins(0.1m), label: new SmartLabel("Sophie"));
+ var feeRate = new FeeRate(2m);
+ var result = transactionFactory.BuildTransaction(payment, feeRate);
+
+ Assert.True(result.Signed);
+ Assert.Equal(4, result.SpentCoins.Count());
+ Assert.Equal(Money.Coins(0.15m), result.SpentCoins.Select(x => x.Amount).Sum());
+
+ var tx = result.Transaction.Transaction;
+ // it must select the unconfirm coin even when the anonymity set is lower
+ Assert.True(result.SpendsUnconfirmed);
+ Assert.Equal(2, tx.Outputs.Count());
+ }
+
+
+ [Fact]
+ public void CustomChangeScript()
+ {
+ var transactionFactory = CreateDefaultTransactionFactory(new[]{
+ ("Maria", 0, 1m, confirmed: true, anonymitySet: 100),
+ });
+
+ // Selecting 0.08 + 0.04 should be enough but it has to select 0.02 too because it is the same address
+ var destination = new Key().ScriptPubKey;
+ var changeDestination = new Key().ScriptPubKey;
+ var payment = new PaymentIntent(new[]{
+ new DestinationRequest(destination, Money.Coins(0.1m)),
+ new DestinationRequest(changeDestination, MoneyRequest.CreateChange(subtractFee: true))
+ });
+ var feeRate = new FeeRate(2m);
+ var result = transactionFactory.BuildTransaction(payment, feeRate);
+
+ Assert.True(result.Signed);
+ Assert.Single(result.SpentCoins);
+ Assert.Equal(Money.Coins(1m), result.SpentCoins.Select(x => x.Amount).Sum());
+
+ var tx = result.Transaction.Transaction;
+ Assert.Equal(2, tx.Outputs.Count());
+
+ var changeOutput = Assert.Single(tx.Outputs, x => x.ScriptPubKey == changeDestination);
+ Assert.Null(transactionFactory.KeyManager.GetKeyForScriptPubKey(changeOutput.ScriptPubKey));
+ Assert.Equal(Money.Coins(0.9m), changeOutput.Value + result.Fee);
+ }
+
+ [Fact]
+ public void SubtractFeeFromSpecificOutput()
+ {
+ var transactionFactory = CreateDefaultTransactionFactory(new[]{
+ ("Maria", 0, 1m, confirmed: true, anonymitySet: 100),
+ });
+
+ // Selecting 0.08 + 0.04 should be enough but it has to select 0.02 too because it is the same address
+ var destination1 = new Key().ScriptPubKey;
+ var destination2 = new Key().ScriptPubKey;
+ var destination3 = new Key().ScriptPubKey;
+ var payment = new PaymentIntent(new[]{
+ new DestinationRequest(destination1, Money.Coins(0.3m)),
+ new DestinationRequest(destination2, Money.Coins(0.3m), subtractFee: true),
+ new DestinationRequest(destination3, Money.Coins(0.3m)),
+ });
+ var feeRate = new FeeRate(2m);
+ var result = transactionFactory.BuildTransaction(payment, feeRate);
+
+ Assert.True(result.Signed);
+ var spentCoin = Assert.Single(result.SpentCoins);
+ Assert.Equal(Money.Coins(1m), spentCoin.Amount);
+
+ var tx = result.Transaction.Transaction;
+ Assert.Equal(4, tx.Outputs.Count());
+
+ var destination2Output = Assert.Single(tx.Outputs, x => x.ScriptPubKey == destination2);
+ Assert.Equal(Money.Coins(0.3m), destination2Output.Value + result.Fee);
+
+ var changeOutput = Assert.Single(tx.Outputs, x => transactionFactory.KeyManager.GetKeyForScriptPubKey(x.ScriptPubKey) != null);
+ Assert.Equal(Money.Coins(0.1m), changeOutput.Value);
+ }
+
+ [Fact]
+ public void SendWholeCoins()
+ {
+ var transactionFactory = CreateDefaultTransactionFactory(new[]{
+ ("Maria", 0, 0.5m, confirmed: false, anonymitySet: 1),
+ ("Joseph", 1, 0.4m, confirmed: true, anonymitySet: 10),
+ ("Eve", 2, 0.3m, confirmed: false, anonymitySet: 40),
+ ("Julio", 3, 0.2m, confirmed: true, anonymitySet: 100),
+ });
+
+ // Selecting 0.08 + 0.04 should be enough but it has to select 0.02 too because it is the same address
+ var destination = new Key().ScriptPubKey;
+ var payment = new PaymentIntent(new[]{
+ new DestinationRequest(destination, MoneyRequest.CreateAllRemaining(subtractFee:true))
+ });
+ var feeRate = new FeeRate(2m);
+ var result = transactionFactory.BuildTransaction(payment, feeRate);
+
+ Assert.True(result.Signed);
+ Assert.Equal(Money.Coins(1.4m), result.SpentCoins.Select(x => x.Amount).Sum());
+
+ var tx = result.Transaction.Transaction;
+ Assert.Single(tx.Outputs);
+ }
+
+ private TransactionFactory CreateDefaultTransactionFactory(
+ IEnumerable<(string Label, int KeyIndex, decimal Amount, bool Confirmed, int AnonymitySet)> coins,
+ bool allowUnconfirmed = true)
+ {
+ var password = "blahblahblah";
+ var keyManager = KeyManager.CreateNew(out var _, password);
+ keyManager.AssertCleanKeysIndexed();
+
+ var keys = keyManager.GetKeys().Take(10).ToArray();
+ var scoins = coins.Select(x => Coin(x.Label, keys[x.KeyIndex], x.Amount, x.Confirmed, x.AnonymitySet));
+ return new TransactionFactory(Network.Main, keyManager, scoins, password, allowUnconfirmed);
+ }
+
+ private static SmartCoin Coin(string label, HdPubKey pubKey, decimal amount, bool confirmed = true, int anonymitySet = 1)
+ {
+ var randomIndex = new Func<int>(() => new Random().Next(0, 200));
+ var height = confirmed ? new Height(randomIndex()) : Height.Mempool;
+ var slabel = new SmartLabel(label);
+ var spentOutput = new[]{
+ new TxoRef(RandomUtils.GetUInt256(), (uint)randomIndex())
+ };
+ return new SmartCoin(RandomUtils.GetUInt256(), (uint)randomIndex(), pubKey.P2wpkhScript, Money.Coins(amount), spentOutput, height, false, anonymitySet, false, slabel, pubKey: pubKey);
+ }
+ }
+}

MODIFY
WalletWasabi/Services/WalletService.cs
WalletWasabi/Services/WalletService.cs
@@ -889,55 +889,6 @@ namespace WalletWasabi.Services
bool allowUnconfirmed = false,
IEnumerable<TxoRef> allowedInputs = null)
{
- password ??= ""; // Correction.
- payments = Guard.NotNull(nameof(payments), payments);
-
- long totalAmount = payments.TotalAmount.Satoshi;
- if (totalAmount < 0 || totalAmount > Constants.MaximumNumberOfSatoshis)
- {
- throw new ArgumentOutOfRangeException($"{nameof(payments)}.{nameof(payments.TotalAmount)} sum cannot be smaller than 0 or greater than {Constants.MaximumNumberOfSatoshis}.");
- }
-
- // Get allowed coins to spend.
- List<SmartCoin> allowedSmartCoinInputs; // Inputs that can be used to build the transaction.
- if (allowedInputs != null) // If allowedInputs are specified then select the coins from them.
- {
- if (!allowedInputs.Any())
- {
- throw new ArgumentException($"{nameof(allowedInputs)} is not null, but empty.");
- }
-
- allowedSmartCoinInputs = allowUnconfirmed
- ? Coins.Where(x => !x.Unavailable && allowedInputs.Any(y => y.TransactionId == x.TransactionId && y.Index == x.Index)).ToList()
- : Coins.Where(x => !x.Unavailable && x.Confirmed && allowedInputs.Any(y => y.TransactionId == x.TransactionId && y.Index == x.Index)).ToList();
-
- // Add those that have the same script, because common ownership is already exposed.
- // But only if the user didn't click the "max" button. In this case he'd send more money than what he'd think.
- if (payments.ChangeStrategy != ChangeStrategy.AllRemainingCustom)
- {
- var allScripts = allowedSmartCoinInputs.Select(x => x.ScriptPubKey).ToHashSet();
- foreach (var coin in Coins.Where(x => !x.Unavailable && !allowedSmartCoinInputs.Any(y => x.TransactionId == y.TransactionId && x.Index == y.Index)))
- {
- if (!(allowUnconfirmed || coin.Confirmed))
- {
- continue;
- }
-
- if (allScripts.Contains(coin.ScriptPubKey))
- {
- allowedSmartCoinInputs.Add(coin);
- }
- }
- }
- }
- else
- {
- allowedSmartCoinInputs = allowUnconfirmed ? Coins.Where(x => !x.Unavailable).ToList() : Coins.Where(x => !x.Unavailable && x.Confirmed).ToList();
- }
-
- // Get and calculate fee
- Logger.LogInfo("Calculating dynamic transaction fee...");
-
FeeRate feeRate;
if (feeStrategy.Type == FeeStrategyType.Target)
{
@@ -952,200 +903,8 @@ namespace WalletWasabi.Services
throw new NotSupportedException(feeStrategy.Type.ToString());
}
- TransactionBuilder builder = Network.CreateTransactionBuilder();
- builder.SetCoinSelector(new SmartCoinSelector(allowedSmartCoinInputs));
- builder.AddCoins(allowedSmartCoinInputs.Select(c => c.GetCoin()));
-
- foreach (var request in payments.Requests.Where(x => x.Amount.Type == MoneyRequestType.Value))
- {
- var amountRequest = request.Amount;
-
- builder.Send(request.Destination, amountRequest.Amount);
- if (amountRequest.SubtractFee)
- {
- builder.SubtractFees();
- }
- }
-
- HdPubKey changeHdPubKey = null;
-
- if (payments.TryGetCustomRequest(out DestinationRequest custChange))
- {
- var changeScript = custChange.Destination.ScriptPubKey;
- changeHdPubKey = KeyManager.GetKeyForScriptPubKey(changeScript);
-
- var changeStrategy = payments.ChangeStrategy;
- if (changeStrategy == ChangeStrategy.Custom)
- {
- builder.SetChange(changeScript);
- }
- else if (changeStrategy == ChangeStrategy.AllRemainingCustom)
- {
- builder.SendAllRemaining(changeScript);
- }
- else
- {
- throw new NotSupportedException(payments.ChangeStrategy.ToString());
- }
- }
- else
- {
- KeyManager.AssertCleanKeysIndexed(isInternal: true);
- KeyManager.AssertLockedInternalKeysIndexed(14);
- changeHdPubKey = KeyManager.GetKeys(KeyState.Clean, true).RandomElement();
-
- builder.SetChange(changeHdPubKey.P2wpkhScript);
- }
-
- builder.SendEstimatedFees(feeRate);
-
- var psbt = builder.BuildPSBT(false);
-
- var spentCoins = psbt.Inputs.Select(txin => allowedSmartCoinInputs.First(y => y.GetOutPoint() == txin.PrevOut)).ToArray();
-
- var realToSend = payments.Requests
- .Select(t =>
- (label: t.Label,
- destination: t.Destination,
- amount: psbt.Outputs.FirstOrDefault(o => o.ScriptPubKey == t.Destination.ScriptPubKey)?.Value))
- .Where(i => i.amount != null);
-
- if (!psbt.TryGetFee(out var fee))
- {
- throw new InvalidOperationException("Impossible to get the fees of the PSBT, this should never happen.");
- }
- Logger.LogInfo($"Fee: {fee.Satoshi} Satoshi.");
-
- var vSize = builder.EstimateSize(psbt.GetOriginalTransaction(), true);
- Logger.LogInfo($"Estimated tx size: {vSize} vbytes.");
-
- // Do some checks
- Money totalSendAmountNoFee = realToSend.Sum(x => x.amount);
- if (totalSendAmountNoFee == Money.Zero)
- {
- throw new InvalidOperationException("The amount after subtracting the fee is too small to be sent.");
- }
- Money totalSendAmount = totalSendAmountNoFee + fee;
-
- Money totalOutgoingAmountNoFee;
- if (changeHdPubKey is null)
- {
- totalOutgoingAmountNoFee = totalSendAmountNoFee;
- }
- else
- {
- totalOutgoingAmountNoFee = realToSend.Where(x => !changeHdPubKey.ContainsScript(x.destination.ScriptPubKey)).Sum(x => x.amount);
- }
- decimal totalOutgoingAmountNoFeeDecimal = totalOutgoingAmountNoFee.ToDecimal(MoneyUnit.BTC);
- // Cannot divide by zero, so use the closest number we have to zero.
- decimal totalOutgoingAmountNoFeeDecimalDivisor = totalOutgoingAmountNoFeeDecimal == 0 ? decimal.MinValue : totalOutgoingAmountNoFeeDecimal;
- decimal feePc = (100 * fee.ToDecimal(MoneyUnit.BTC)) / totalOutgoingAmountNoFeeDecimalDivisor;
-
- if (feePc > 1)
- {
- Logger.LogInfo($"The transaction fee is {totalOutgoingAmountNoFee:0.#}% of your transaction amount.{Environment.NewLine}"
- + $"Sending:\t {totalSendAmount.ToString(fplus: false, trimExcessZero: true)} BTC.{Environment.NewLine}"
- + $"Fee:\t\t {fee.Satoshi} Satoshi.");
- }
- if (feePc > 100)
- {
- throw new InvalidOperationException($"The transaction fee is more than twice the transaction amount: {feePc:0.#}%.");
- }
-
- if (spentCoins.Any(u => !u.Confirmed))
- {
- Logger.LogInfo("Unconfirmed transaction is spent.");
- }
-
- // Build the transaction
- Logger.LogInfo("Signing transaction...");
- // It must be watch only, too, because if we have the key and also hardware wallet, we do not care we can sign.
-
- Transaction tx = null;
- if (KeyManager.IsWatchOnly)
- {
- tx = psbt.GetGlobalTransaction();
- }
- else
- {
- IEnumerable<ExtKey> signingKeys = KeyManager.GetSecrets(password, spentCoins.Select(x => x.ScriptPubKey).ToArray());
- builder = builder.AddKeys(signingKeys.ToArray());
- builder.SignPSBT(psbt);
- psbt.Finalize();
- tx = psbt.ExtractTransaction();
-
- var checkResults = builder.Check(tx).ToList();
- if (!psbt.TryGetEstimatedFeeRate(out FeeRate actualFeeRate))
- {
- throw new InvalidOperationException("Impossible to get the fee rate of the PSBT, this should never happen.");
- }
-
- // Manually check the feerate, because some inaccuracy is possible.
- var sb1 = feeRate.SatoshiPerByte;
- var sb2 = actualFeeRate.SatoshiPerByte;
- if (Math.Abs(sb1 - sb2) > 2) // 2s/b inaccuracy ok.
- {
- // So it'll generate a transactionpolicy error thrown below.
- checkResults.Add(new NotEnoughFundsPolicyError("Fees different than expected"));
- }
- if (checkResults.Count > 0)
- {
- throw new InvalidTxException(tx, checkResults);
- }
- }
-
- if (KeyManager.MasterFingerprint is HDFingerprint fp)
- {
- foreach (var coin in spentCoins)
- {
- var rootKeyPath = new RootedKeyPath(fp, coin.HdPubKey.FullKeyPath);
- psbt.AddKeyPath(coin.HdPubKey.PubKey, rootKeyPath, coin.ScriptPubKey);
- }
- }
-
- var label = SmartLabel.Merge(payments.Requests.Select(x => x.Label));
- var outerWalletOutputs = new List<SmartCoin>();
- var innerWalletOutputs = new List<SmartCoin>();
- for (var i = 0U; i < tx.Outputs.Count; i++)
- {
- TxOut output = tx.Outputs[i];
- var anonset = (tx.GetAnonymitySet(i) + spentCoins.Min(x => x.AnonymitySet)) - 1; // Minus 1, because count own only once.
- var foundKey = KeyManager.GetKeyForScriptPubKey(output.ScriptPubKey);
- var coin = new SmartCoin(tx.GetHash(), i, output.ScriptPubKey, output.Value, tx.Inputs.ToTxoRefs().ToArray(), Height.Unknown, tx.RBF, anonset, isLikelyCoinJoinOutput: false, pubKey: foundKey);
- label = SmartLabel.Merge(label, coin.Label); // foundKey's label is already added to the coinlabel.
-
- if (foundKey is null)
- {
- outerWalletOutputs.Add(coin);
- }
- else
- {
- innerWalletOutputs.Add(coin);
- }
- }
-
- foreach (var coin in outerWalletOutputs.Concat(innerWalletOutputs))
- {
- var foundPaymentRequest = payments.Requests.FirstOrDefault(x => x.Destination.ScriptPubKey == coin.ScriptPubKey);
-
- // If change then we concatenate all the labels.
- if (foundPaymentRequest is null) // Then it's autochange.
- {
- coin.Label = label;
- }
- else
- {
- coin.Label = SmartLabel.Merge(coin.Label, foundPaymentRequest.Label);
- }
-
- var foundKey = KeyManager.GetKeyForScriptPubKey(coin.ScriptPubKey);
- foundKey?.SetLabel(coin.Label); // The foundkeylabel has already been added previously, so no need to concatenate.
- }
-
- Logger.LogInfo($"Transaction is successfully built: {tx.GetHash()}.");
- var sign = !KeyManager.IsWatchOnly;
- var spendsUnconfirmed = spentCoins.Any(c => !c.Confirmed);
- return new BuildTransactionResult(new SmartTransaction(tx, Height.Unknown), psbt, spendsUnconfirmed, sign, fee, feePc, outerWalletOutputs, innerWalletOutputs, spentCoins);
+ var builder = new TransactionFactory(Network, KeyManager, Coins, password, allowUnconfirmed);
+ return builder.BuildTransaction(payments, feeRate, allowedInputs);
}
public void RenameLabel(SmartCoin coin, SmartLabel newLabel)

First refactoring and test TransactionFactory