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.

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

Refactor coin selection to leverage NBitcoin #2116

Merged
merged 41 commits into from Aug 19, 2019
Merged
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
6d80c1b
Refactor coin selection to leverage NBitcoin
NicolasDorier Aug 13, 2019
6fdd38d
Use Guard to check ctor parameter
lontivero Aug 15, 2019
ac9f252
Merge branch 'master' into pr/2116-refactor-coin-selection-to-leverag…
nopara73 Aug 16, 2019
f4642f5
codemaid + fix nullcheck
nopara73 Aug 16, 2019
9c2928a
use property
nopara73 Aug 16, 2019
b1399e5
Set SmartCoinsByOutpoint
nopara73 Aug 16, 2019
b33bbc1
Restore an original line for readability.
nopara73 Aug 16, 2019
96b914b
Improve comment.
nopara73 Aug 16, 2019
e8d0a8c
Use != null for readability
nopara73 Aug 16, 2019
1bc0a4e
Invert if (so it'll be closer to how we used in the original code.)
nopara73 Aug 16, 2019
efdbd7e
Introduce PaymentIntent instead of operation.
nopara73 Aug 16, 2019
f58ee19
When sending, do not create change if not needed
NicolasDorier Aug 16, 2019
e568f06
Send proper error message if we send a too small amount
NicolasDorier Aug 16, 2019
fa40150
Fix change labeling.
nopara73 Aug 16, 2019
d5d0a75
Merge branch 'refactor/coinselection' of https://github.com/NicolasDo…
nopara73 Aug 16, 2019
ad739f0
Use calculated property.
nopara73 Aug 16, 2019
7c7b598
fix typo
nopara73 Aug 16, 2019
d6ceb00
Improve grammar
NicolasDorier Aug 17, 2019
f9bd25e
Use NBitcoin SendAllRemaining instead of SendAll
NicolasDorier Aug 17, 2019
5bb0c48
bump NBitcoin
NicolasDorier Aug 17, 2019
9479142
Merge branch 'master' into pr/2116-refactor-coin-selection-to-leverag…
nopara73 Aug 17, 2019
7f6c168
fix fee percent calculation
nopara73 Aug 17, 2019
6772692
Make sure there's difference between change and all remaining.
nopara73 Aug 17, 2019
db3fca4
Remove leftover numbering comments
nopara73 Aug 17, 2019
6705cfa
Introduce FeeStrategy
nopara73 Aug 17, 2019
0e9e8ea
Make sure to add to "allowedcoins" even if they're not selected.
nopara73 Aug 17, 2019
0f3b168
fix off by one bug, make sure with the "max" button we cannot acciden…
nopara73 Aug 17, 2019
68b4b37
Implement address reuse consolidation
nopara73 Aug 17, 2019
bb98c40
Fix some datadir issues
nopara73 Aug 17, 2019
bc5a813
Don't address reuse in test
nopara73 Aug 17, 2019
592409a
Improve labeling
nopara73 Aug 17, 2019
ee8886a
Fix naming
nopara73 Aug 17, 2019
4d3be06
fix naming in other places, too
nopara73 Aug 17, 2019
83aa0cc
Use FeeStrategy.TwentyMinutesConfirmationTargetStrategy
nopara73 Aug 17, 2019
23ccb13
target -> rate
nopara73 Aug 17, 2019
ce81960
Improve exception message.
nopara73 Aug 18, 2019
5942c5d
Remove ToArray
nopara73 Aug 18, 2019
dd81c48
Improve readability
nopara73 Aug 18, 2019
c07befc
Fix checkresult
nopara73 Aug 18, 2019
3b7d977
Make fee check explicit on WasabiWallet side
NicolasDorier Aug 18, 2019
f53c454
satoshi -> sat
nopara73 Aug 19, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -26,6 +26,7 @@
using WalletWasabi.Hwi.Models;
using WalletWasabi.KeyManagement;
using WalletWasabi.Models;
using WalletWasabi.Models.TransactionBuilding;
using WalletWasabi.Services;

namespace WalletWasabi.Gui.Controls.WalletExplorer
@@ -268,11 +269,14 @@ public SendTabViewModel(WalletViewModel walletViewModel, bool isTransactionBuild
return;
}

var script = address.ScriptPubKey;
var amount = Money.Zero;
if (!IsMax)
MoneyRequest moneyRequest;
if (IsMax)
{
if (!Money.TryParse(AmountText, out amount) || amount == Money.Zero)
moneyRequest = MoneyRequest.CreateAllRemaining();
}
else
{
if (!Money.TryParse(AmountText, out Money amount) || amount == Money.Zero)
{
SetWarningMessage($"Invalid amount.");
return;
@@ -283,6 +287,7 @@ public SendTabViewModel(WalletViewModel walletViewModel, bool isTransactionBuild
SetWarningMessage("Looks like you want to spend a whole coin. Try Max button instead.");
return;
}
moneyRequest = MoneyRequest.Create(amount);
}

if (FeeRate is null || FeeRate.SatoshiPerByte < 1)
@@ -292,11 +297,10 @@ public SendTabViewModel(WalletViewModel walletViewModel, bool isTransactionBuild
}

bool useCustomFee = !IsSliderFeeUsed;
FeeRate feeRate = FeeRate;
var feeStrategy = FeeStrategy.CreateFromFeeRate(FeeRate);

var label = Label;
var operation = new WalletService.Operation(script, amount, label);

var intent = new PaymentIntent(address, moneyRequest, label);
try
{
MainWindowViewModel.Instance.StatusBar.TryAddStatus(StatusBarStatus.DequeuingSelectedCoins);
@@ -331,7 +335,7 @@ public SendTabViewModel(WalletViewModel walletViewModel, bool isTransactionBuild
return;
}

BuildTransactionResult result = await Task.Run(() => Global.WalletService.BuildTransaction(Password, new[] { operation }, feeTarget: null, feeRate: feeRate, allowUnconfirmed: true, allowedInputs: selectedCoinReferences));
BuildTransactionResult result = await Task.Run(() => Global.WalletService.BuildTransaction(Password, intent, feeStrategy, allowUnconfirmed: true, allowedInputs: selectedCoinReferences));

if (IsTransactionBuilder)
{
@@ -673,7 +677,7 @@ private void SetFeesAndTexts()
}
else if (feeTarget == Constants.SevenDaysConfirmationTarget)
{
ConfirmationExpectedText = $"two weeks™";
ConfirmationExpectedText = "one week";
}
else if (feeTarget == -1)
{
@@ -11,6 +11,7 @@
using System.Runtime.InteropServices;
using System.Threading.Tasks;
using WalletWasabi.Models;
using WalletWasabi.Models.TransactionBuilding;

namespace WalletWasabi.Gui.Controls.WalletExplorer
{

Large diffs are not rendered by default.

@@ -76,11 +76,12 @@ public static BitcoinWitPubKeyAddress GetCoordinatorAddress(Network network)
}
}

public const int BigFileReadWriteBufferSize = 1 * 1024 * 1024;

public const int TwentyMinutesConfirmationTarget = 2;
public const int OneDayConfirmationTarget = 144;
public const int SevenDaysConfirmationTarget = 1008;

public const int BigFileReadWriteBufferSize = 1 * 1024 * 1024;

public const int DefaultTorSocksPort = 9050;
public const int DefaultTorBrowserSocksPort = 9150;
public const int DefaultTorControlPort = 9051;
@@ -0,0 +1,43 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace WalletWasabi.Helpers
{
public class LabelBuilder
{
public List<string> Labels { get; } = new List<string>();

public LabelBuilder(params string[] labels)
{
labels = labels ?? new string[] { };
foreach (var label in labels)
{
Add(label);
}
}

public void Add(string label)
{
var parts = Guard.Correct(label).Split(',', StringSplitOptions.RemoveEmptyEntries);
foreach (var corrected in parts.Select(Guard.Correct))
{
if (corrected == "")

This comment has been minimized.

Copy link
@lontivero

lontivero Aug 19, 2019

Collaborator

I think this is not possible.

This comment has been minimized.

Copy link
@nopara73

nopara73 Aug 19, 2019

Collaborator

Guard.Correct corrects everything to "". Whitespaces, nulls. So it should be possible.

{
return;
}

if (!Labels.Contains(label))
{
Labels.Add(label);
}
}
}

public override string ToString()
{
return string.Join(", ", Labels);
}
}
}
@@ -1,6 +1,7 @@
using NBitcoin;
using Newtonsoft.Json;
using System;
using System.Linq;
using WalletWasabi.Helpers;
using WalletWasabi.JsonConverters;

@@ -114,6 +115,19 @@ public void SetKeyState(KeyState state, KeyManager kmToFile = null)

public BitcoinScriptAddress GetP2shOverP2wpkhAddress(Network network) => P2wpkhScript.GetScriptAddress(network);

public bool ContainsScript(Script scriptPubKey)
{
var scripts = new[]
{
P2pkScript,
P2pkhScript,
P2wpkhScript,
P2shOverP2wpkhScript
};

return scripts.Contains(scriptPubKey);
}

#region Equality

public override bool Equals(object obj) => obj is HdPubKey pubKey && this == pubKey;
@@ -329,7 +329,6 @@ private void Create(uint256 transactionId, uint index, Script scriptPubKey, Mone
ScriptPubKey = Guard.NotNull(nameof(scriptPubKey), scriptPubKey);
Amount = Guard.NotNull(nameof(amount), amount);
Height = height;
Label = Guard.Correct(label);
SpentOutputs = Guard.NotNullOrEmpty(nameof(spentOutputs), spentOutputs);
IsReplaceable = replaceable;
AnonymitySet = Guard.InRangeAndNotNull(nameof(anonymitySet), anonymitySet, 1, int.MaxValue);
@@ -343,6 +342,9 @@ private void Create(uint256 transactionId, uint index, Script scriptPubKey, Mone

HdPubKey = pubKey;

var labelBuilder = new LabelBuilder(label, pubKey?.Label);
Label = labelBuilder.ToString();

SetConfirmed();
SetUnspent();
SetIsBanned();
@@ -2,7 +2,7 @@
using System.Collections.Generic;
using WalletWasabi.Helpers;

namespace WalletWasabi.Models
namespace WalletWasabi.Models.TransactionBuilding
{
public class BuildTransactionResult
{
@@ -0,0 +1,13 @@
using System;
using System.Collections.Generic;
using System.Text;

namespace WalletWasabi.Models.TransactionBuilding
{
public enum ChangeStrategy
{
Auto,
Custom,
AllRemainingCustom
}
}
@@ -0,0 +1,34 @@
using NBitcoin;
using System;
using System.Collections.Generic;
using System.Text;
using WalletWasabi.Helpers;

namespace WalletWasabi.Models.TransactionBuilding
{
public class DestinationRequest
{
public IDestination Destination { get; }
public MoneyRequest Amount { get; }
public string Label { get; }

public DestinationRequest(Script scriptPubKey, Money amount, bool subtractFee = false, string label = "") : this(scriptPubKey, MoneyRequest.Create(amount, subtractFee), label)
{
}

public DestinationRequest(Script scriptPubKey, MoneyRequest amount, string label = "") : this(scriptPubKey.GetDestination(), amount, label)
{
}

public DestinationRequest(IDestination destination, Money amount, bool subtractFee = false, string label = "") : this(destination, MoneyRequest.Create(amount, subtractFee), label)
{
}

public DestinationRequest(IDestination destination, MoneyRequest amount, string label = "")
{
Destination = Guard.NotNull(nameof(destination), destination);
Amount = Guard.NotNull(nameof(amount), amount);
Label = Guard.Correct(label);
}
}
}
@@ -0,0 +1,79 @@
using NBitcoin;
using System;
using System.Collections.Generic;
using System.Text;
using WalletWasabi.Helpers;

namespace WalletWasabi.Models.TransactionBuilding
{
public class FeeStrategy
{
private int _target;
private FeeRate _rate;

public FeeStrategyType Type { get; }

public int Target
{
get
{
if (Type != FeeStrategyType.Target)
{
throw new NotSupportedException($"Cannot get {nameof(Target)} with {nameof(FeeStrategyType)} {Type}.");
}
return _target;
}
}

public FeeRate Rate
{
get
{
if (Type != FeeStrategyType.Rate)
{
throw new NotSupportedException($"Cannot get {nameof(Rate)} with {nameof(FeeStrategyType)} {Type}.");
}
return _rate;
}
}

public static FeeStrategy TwentyMinutesConfirmationTargetStrategy { get; } = CreateFromConfirmationTarget(Constants.TwentyMinutesConfirmationTarget);
public static FeeStrategy OneDayConfirmationTargetStrategy { get; } = CreateFromConfirmationTarget(Constants.OneDayConfirmationTarget);
public static FeeStrategy SevenDaysConfirmationTargetStrategy { get; } = CreateFromConfirmationTarget(Constants.SevenDaysConfirmationTarget);

public static FeeStrategy CreateFromConfirmationTarget(int confirmationTarget) => new FeeStrategy(FeeStrategyType.Target, confirmationTarget: confirmationTarget, feeRate: null);

public static FeeStrategy CreateFromFeeRate(FeeRate feeRate) => new FeeStrategy(FeeStrategyType.Rate, confirmationTarget: null, feeRate: feeRate);

private FeeStrategy(FeeStrategyType type, int? confirmationTarget, FeeRate feeRate)
{
Type = type;
if (type == FeeStrategyType.Rate)
{
if (confirmationTarget != null)
{
throw new ArgumentException($"{nameof(confirmationTarget)} must be null.");
}
Guard.NotNull(nameof(feeRate), feeRate);
if (feeRate < new FeeRate(1m))
{
throw new ArgumentOutOfRangeException(nameof(feeRate), feeRate, "Cannot be less than 1 sat/byte.");
}
_rate = feeRate;
}
else if (type == FeeStrategyType.Target)
{
if (feeRate != null)
{
throw new ArgumentException($"{nameof(feeRate)} must be null.");
}
Guard.NotNull(nameof(confirmationTarget), confirmationTarget);
_target = Guard.InRangeAndNotNull(nameof(confirmationTarget), confirmationTarget.Value, Constants.TwentyMinutesConfirmationTarget, Constants.SevenDaysConfirmationTarget);
}
else
{
throw new NotSupportedException(type.ToString());
}
}
}
}
@@ -0,0 +1,12 @@
using System;
using System.Collections.Generic;
using System.Text;

namespace WalletWasabi.Models.TransactionBuilding
{
public enum FeeStrategyType
{
Target,
Rate
}
}
@@ -0,0 +1,50 @@
using NBitcoin;
using System;
using System.Collections.Generic;
using System.Text;

namespace WalletWasabi.Models.TransactionBuilding
{
public class MoneyRequest
{
public static MoneyRequest Create(Money amount, bool subtractFee = false) => new MoneyRequest(amount, MoneyRequestType.Value, subtractFee);

public static MoneyRequest CreateChange(bool subtractFee = true) => new MoneyRequest(null, MoneyRequestType.Change, subtractFee);

public static MoneyRequest CreateAllRemaining(bool subtractFee = true) => new MoneyRequest(null, MoneyRequestType.AllRemaining, subtractFee);

public Money Amount { get; }
public MoneyRequestType Type { get; }
public bool SubtractFee { get; }

private MoneyRequest(Money amount, MoneyRequestType type, bool subtractFee)
{
if (type == MoneyRequestType.AllRemaining || type == MoneyRequestType.Change)
{
if (amount != null)
{
throw new ArgumentException($"{nameof(amount)} must be null.");
}
}
else if (type == MoneyRequestType.Value)
{
if (amount is null)
{
throw new ArgumentNullException($"{nameof(amount)} cannot be null.");
}
else if (amount <= Money.Zero)
{
throw new ArgumentOutOfRangeException($"{nameof(amount)} must be positive.");
}
}
else
{
throw new NotSupportedException($"{nameof(type)} is not supported: {type}.");
}

Amount = amount;
Type = type;
SubtractFee = subtractFee;
}
}
}
@@ -0,0 +1,13 @@
using System;
using System.Collections.Generic;
using System.Text;

namespace WalletWasabi.Models.TransactionBuilding
{
public enum MoneyRequestType
{
Value,
Change,
AllRemaining
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.