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

[Qt] Add checkbox in the GUI to opt-in to RBF when creating a transaction #9592

Merged
merged 3 commits into from Mar 17, 2017
Merged

[Qt] Add checkbox in the GUI to opt-in to RBF when creating a transaction #9592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
568c05a
Allow to opt-into RBF when creating a transaction
jonasschnelli Dec 3, 2015
838a58e
[Qt] Add simple optin-RBF checkbox and confirmation info
jonasschnelli Apr 5, 2016
c4e4792
[Qt] Change RBF checkbox to reflect -walletrbf setting
ryanofsky Jan 19, 2017
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
14 changes: 12 additions & 2 deletions src/qt/forms/sendcoinsdialog.ui
View file
Open in desktop
Expand Up @@ -1158,6 +1158,16 @@
</item>
</layout>
</item>
<item>
<widget class="QCheckBox" name="optInRBF">
<property name="text">
<string>Request Replace-By-Fee</string>
</property>
<property name="toolTip">
<string>Indicates that the sender may wish to replace this transaction with a new one paying higher fees (prior to being confirmed).</string>
</property>
</widget>
</item>
</layout>
</widget>
</item>
Expand All @@ -1168,8 +1178,8 @@
</property>
<property name="sizeHint" stdset="0">
<size>
<width>800</width>
<height>1</height>
<width>40</width>
<height>5</height>
</size>
</property>
</spacer>
Expand Down
10 changes: 10 additions & 0 deletions src/qt/sendcoinsdialog.cpp
View file
Open in desktop
Expand Up @@ -112,6 +112,7 @@ SendCoinsDialog::SendCoinsDialog(const PlatformStyle *_platformStyle, QWidget *p
ui->groupCustomFee->button((int)std::max(0, std::min(1, settings.value("nCustomFeeRadio").toInt())))->setChecked(true);
ui->customFee->setValue(settings.value("nTransactionFee").toLongLong());
ui->checkBoxMinimumFee->setChecked(settings.value("fPayOnlyMinFee").toBool());
ui->optInRBF->setCheckState(model->getDefaultWalletRbf() ? Qt::Checked : Qt::Unchecked);
minimizeFeeSection(settings.value("fFeeSectionMinimized").toBool());
}

Expand Down Expand Up @@ -245,6 +246,8 @@ void SendCoinsDialog::on_sendButton_clicked()
else
ctrl.nConfirmTarget = 0;

ctrl.signalRbf = ui->optInRBF->isChecked();

prepareStatus = model->prepareTransaction(currentTransaction, &ctrl);

// process prepareStatus and on error generate message shown to user
Expand Down Expand Up @@ -324,6 +327,13 @@ void SendCoinsDialog::on_sendButton_clicked()
questionString.append(QString("<span style='font-size:10pt;font-weight:normal;'><br />(=%2)</span>")
.arg(alternativeUnits.join(" " + tr("or") + "<br />")));

if (ui->optInRBF->isChecked())
{
questionString.append("<hr /><span>");
questionString.append(tr("This transaction signals replaceability (optin-RBF)."));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this is likely to confuse users. Why must we say so here? (The alternative case seems much more of a liability...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonasschnelli or others, you should comment if you think the "This transaction signals replaceability" text is useful, otherwise I'm fine with removing it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know whats best here.
Somehow I agree with @luke-jr that it would probably be better the label non-RBF transactions (something like "this transaction signals to be final"), but meh.

We also need to respect that RBF is deployed as a new feature and maybe users are expecting to see wether the new features is enabled or not.

So, no strong opinion. The PRs current solution seems acceptable to me.

questionString.append("</span>");
}

SendConfirmationDialog confirmationDialog(tr("Confirm send coins"),
questionString.arg(formatted.join("<br />")), SEND_CONFIRM_DELAY, this);
confirmationDialog.exec();
Expand Down
5 changes: 5 additions & 0 deletions src/qt/walletmodel.cpp
View file
Open in desktop
Expand Up @@ -706,3 +706,8 @@ int WalletModel::getDefaultConfirmTarget() const
{
return nTxConfirmTarget;
}

bool WalletModel::getDefaultWalletRbf() const
{
return fWalletRbf;
}
2 changes: 2 additions & 0 deletions src/qt/walletmodel.h
View file
Open in desktop
Expand Up @@ -213,6 +213,8 @@ class WalletModel : public QObject

int getDefaultConfirmTarget() const;

bool getDefaultWalletRbf() const;

private:
CWallet *wallet;
bool fHaveWatchOnly;
Expand Down
4 changes: 4 additions & 0 deletions src/wallet/coincontrol.h
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@
#define BITCOIN_WALLET_COINCONTROL_H

#include "primitives/transaction.h"
#include "wallet/wallet.h"

/** Coin Control Features. */
class CCoinControl
Expand All @@ -24,6 +25,8 @@ class CCoinControl
CFeeRate nFeeRate;
//! Override the default confirmation target, 0 = use default
int nConfirmTarget;
//! Signal BIP-125 replace by fee.
bool signalRbf;

CCoinControl()
{
Expand All @@ -40,6 +43,7 @@ class CCoinControl
nFeeRate = CFeeRate(0);
fOverrideFeeRate = false;
nConfirmTarget = 0;
signalRbf = fWalletRbf;
}

bool HasSelected() const
Expand Down
3 changes: 2 additions & 1 deletion src/wallet/wallet.cpp
View file
Open in desktop
Expand Up @@ -2566,9 +2566,10 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
// to avoid conflicting with other possible uses of nSequence,
// and in the spirit of "smallest posible change from prior
// behavior."
bool rbf = coinControl ? coinControl->signalRbf : fWalletRbf;
for (const auto& coin : setCoins)
txNew.vin.push_back(CTxIn(coin.first->GetHash(),coin.second,CScript(),
std::numeric_limits<unsigned int>::max() - (fWalletRbf ? 2 : 1)));
std::numeric_limits<unsigned int>::max() - (rbf ? 2 : 1)));

// Fill in dummy signatures for fee calculation.
int nIn = 0;
Expand Down