-
Notifications
You must be signed in to change notification settings - Fork 486
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
Cannot send transaction #2304
Comments
Do you have a huge amount of coins selected? We changed to NBitcoin's transaction builder instead of using our homegrown transaction building and I remember that in 2018 Stratis was complaining about very similar issues during the same scenario. |
Number of utxo was reasonable,
For testing purposes,
how will wasabi handle having it's internet cut off just before or
during the transaction building? Will it cache and rebroadcast as soon as
it can connect again?
…On Thu., Sep. 19, 2019, 06:47 nopara73, ***@***.***> wrote:
Do you have a huge amount of coins selected? We changed to NBitcoin's
transaction builder instead of using our homegrown transaction building and
I remember that in 2018 Stratis was complaining about very similar issues
during the same scenario.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2304>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAC643AVOD4IUKRTAAZEBN3QKNKFXANCNFSM4IYCSNFQ>
.
|
No, if building the tx fails, then it never gets to broadcasting (nor serializing or similar.) Let me try to reproduce. |
@lontivero @NicolasDorier Do you have any theories? |
For the record this part of the code gets stuck (based on @dmp1ce's logs) // Get and calculate fee
Logger.LogInfo("Calculating dynamic transaction fee...");
FeeRate feeRate;
if (feeStrategy.Type == FeeStrategyType.Target)
{
feeRate = Synchronizer.GetFeeRate(feeStrategy.Target);
}
else if (feeStrategy.Type == FeeStrategyType.Rate)
{
feeRate = feeStrategy.Rate;
}
else
{
throw new NotSupportedException(feeStrategy.Type.ToString());
}
var smartCoinsByOutpoint = allowedSmartCoinInputs.ToDictionary(s => s.GetOutPoint());
TransactionBuilder builder = Network.CreateTransactionBuilder();
builder.SetCoinSelector(new SmartCoinSelector(smartCoinsByOutpoint));
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 => smartCoinsByOutpoint[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."); |
Try with two out (change) Sending MAX worked |
Thanks, I played around with the amounts, no difference. (Although I noticed when I did larger amounts building took like half a second longer than with smaller amounts, but that's probably irrelevant here.) |
@lontivero Can you play around with it on Linux? Maybe it's OS specific issue (although I doubt that, because this is pure C# code.) |
I should have taken a screenshot. I'll try to reproduce later. For now I can tell you it was about 10 to 20 available inputs, all private. The address I was sending to was a P2SH address, the address started with a "3". I had the fee slider set to fastest, closest to the lightning bolt. For the amount I was sending, I think it would have used at least two inputs, but it could have been more depending on the building algorithm. |
@nopara73 I started reviewing Wasabi code first, I cannot see how this could happen. Next I reviewed NBitcoin's TransactionBuilder code looking for loops and retries with no obvious termination conditions (I found one but I was not able to make it iterate forever). Finally I created a test that created hundreds of transactions with lots of coins paying random fee and spending random amounts in different ways. That test was running for almost an hour and never failed not got stuck. The only thing that rest to test is the PSBT part. |
I cannot make it fail. |
I was sending to an address I had probably sent to in the past. I'm not sure if there was supposed to be a privacy warning about that. I wondered if it was a GUI issue, but the GUI was responsive. I was able to close Wasabi normally. I tried three times, and each attempt took longer than five minutes to build transaction. I never waited longer than five minutes for the send. I tried a slightly higher fee on the slider on one attempt. |
@MaxHillebrand Credit should go to Samourai Wallet, which does this already. |
I never had any case of the TransactionBuilder having problem creating the transaction. My guess is that it might come from the SmartCoinSelector. |
I think I got it! The following line call the coin selection algorithm many many times. It seems there is a loop somewhere in NBitcoin. |
Yes, I go it. Here we have a way to reproduce it:
TestNetWallet.json
You will see the transaction takes forever to be built and that the coin selection I was focused on getting a reproducible scenario. I will continue debugging this issue but I am sure this is a NBitcoin bug @NicolasDorier |
Fixing this should go onto Wasabi's |
The problem is our There is a UT in this NBitcoin repo PR MetacoSA/NBitcoin#745 where the problem is reproducible. |
In a meeting we decided that @lontivero will try to fix it, but he is confused by NBitcoin code, so it's unlikely he'll succeed. We'll wait for @NicolasDorier to come around and try to fix it for a few days, if it won't happen, then we roll out a hotfix that dumbs down the coin selection algorithm so NBitcoin becomes satisfied with it. |
Don't rush on my account. I can wait for the proper fix. Thanks! |
I have fixed it. Could you test my PR? #2334 |
Tested #2334 with |
Fixed. Tomorrow we'll do a non-notifying release with the fix. |
General Description
I cannot send a transaction because Wasabi is trying to "Calculating dynamic transaction fee".
I select "All Private" and click the button to send my transaction. The send button says "Sending Transaction..." and the bottom status message says "Building transaction..." but doesn't progress beyond this point. A thread on my CPU also goes to 100% for the process
wassabee
.I notice in the logs the last line says "Calculating dynamic transaction fee..."
Operating System
My operating system is Arch Linux using version 1.1.9 of Wasabi.
Logs
Logs
Wasabi Version
I installed Wasabi from the release
.deb
using the AUR script here: https://aur.archlinux.org/packages/wasabi-wallet-binThe text was updated successfully, but these errors were encountered: