Skip to content

Commit

Permalink
rpc/wallet/bumpfee: allow specifying output index
Browse files Browse the repository at this point in the history
Currently, bumpfee cannot be used to bump a single output transaction. This should be possible, as single output transactions are special in that the exact amount doesn't usually matter (the user is emptying a wallet or using coin control to move UTXOs).
  • Loading branch information
kallewoof committed Jan 15, 2019
1 parent cf0c67b commit cf076ba
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 19 deletions.
3 changes: 2 additions & 1 deletion src/interfaces/wallet.cpp
Expand Up @@ -268,12 +268,13 @@ class WalletImpl : public Wallet
bool createBumpTransaction(const uint256& txid,
const CCoinControl& coin_control,
CAmount total_fee,
int32_t reduce_output,
std::vector<std::string>& errors,
CAmount& old_fee,
CAmount& new_fee,
CMutableTransaction& mtx) override
{
return feebumper::CreateTransaction(&m_wallet, txid, coin_control, total_fee, errors, old_fee, new_fee, mtx) ==
return feebumper::CreateTransaction(&m_wallet, txid, coin_control, total_fee, reduce_output, errors, old_fee, new_fee, mtx) ==
feebumper::Result::OK;
}
bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(&m_wallet, mtx); }
Expand Down
1 change: 1 addition & 0 deletions src/interfaces/wallet.h
Expand Up @@ -152,6 +152,7 @@ class Wallet
virtual bool createBumpTransaction(const uint256& txid,
const CCoinControl& coin_control,
CAmount total_fee,
int32_t reduce_output,
std::vector<std::string>& errors,
CAmount& old_fee,
CAmount& new_fee,
Expand Down
2 changes: 1 addition & 1 deletion src/qt/walletmodel.cpp
Expand Up @@ -509,7 +509,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash)
CAmount old_fee;
CAmount new_fee;
CMutableTransaction mtx;
if (!m_wallet->createBumpTransaction(hash, coin_control, 0 /* totalFee */, errors, old_fee, new_fee, mtx)) {
if (!m_wallet->createBumpTransaction(hash, coin_control, 0 /* totalFee */, -1 /* reduce_output */, errors, old_fee, new_fee, mtx)) {
QMessageBox::critical(nullptr, tr("Fee bump error"), tr("Increasing transaction fee failed") + "<br />(" +
(errors.size() ? QString::fromStdString(errors[0]) : "") +")");
return false;
Expand Down
37 changes: 22 additions & 15 deletions src/wallet/feebumper.cpp
Expand Up @@ -75,7 +75,8 @@ bool TransactionCanBeBumped(const CWallet* wallet, const uint256& txid)
return res == feebumper::Result::OK;
}

Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoinControl& coin_control, CAmount total_fee, std::vector<std::string>& errors,
Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoinControl& coin_control,
CAmount total_fee, int32_t reduce_output, std::vector<std::string>& errors,
CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx)
{
auto locked_chain = wallet->chain().lock();
Expand All @@ -87,22 +88,28 @@ Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoin
return Result::INVALID_ADDRESS_OR_KEY;
}
const CWalletTx& wtx = it->second;
if (reduce_output < -1 || reduce_output >= int64_t(wtx.tx->vout.size())) {
errors.push_back(strprintf("Change output out of bounds [0..%zu]", wtx.tx->vout.size()-1));
return Result::INVALID_PARAMETER;
}

Result result = PreconditionChecks(*locked_chain, wallet, wtx, errors);
if (result != Result::OK) {
return result;
}

// figure out which output was change
// if there was no change output or multiple change outputs, fail
int nOutput = -1;
for (size_t i = 0; i < wtx.tx->vout.size(); ++i) {
if (wallet->IsChange(wtx.tx->vout[i])) {
if (nOutput != -1) {
errors.push_back("Transaction has multiple change outputs");
return Result::WALLET_ERROR;
int nOutput = reduce_output;
if (nOutput == -1) {
// figure out which output was change
// if there was no change output or multiple change outputs, fail
for (size_t i = 0; i < wtx.tx->vout.size(); ++i) {
if (wallet->IsChange(wtx.tx->vout[i])) {
if (nOutput != -1) {
errors.push_back("Transaction has multiple change outputs");
return Result::WALLET_ERROR;
}
nOutput = i;
}
nOutput = i;
}
}
if (nOutput == -1) {
Expand Down Expand Up @@ -161,11 +168,11 @@ Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoin
}

// Check that in all cases the new fee doesn't violate maxTxFee
if (new_fee > maxTxFee) {
errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)",
FormatMoney(new_fee), FormatMoney(maxTxFee)));
return Result::WALLET_ERROR;
}
if (new_fee > maxTxFee) {
errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)",
FormatMoney(new_fee), FormatMoney(maxTxFee)));
return Result::WALLET_ERROR;
}

// check that fee rate is higher than mempool's minimum fee
// (no point in bumping fee if we know that the new tx won't be accepted to the mempool)
Expand Down
1 change: 1 addition & 0 deletions src/wallet/feebumper.h
Expand Up @@ -33,6 +33,7 @@ Result CreateTransaction(const CWallet* wallet,
const uint256& txid,
const CCoinControl& coin_control,
CAmount total_fee,
int32_t reduce_output,
std::vector<std::string>& errors,
CAmount& old_fee,
CAmount& new_fee,
Expand Down
15 changes: 13 additions & 2 deletions src/wallet/rpcwallet.cpp
Expand Up @@ -3168,7 +3168,8 @@ static UniValue bumpfee(const JSONRPCRequest& request)
"\nBumps the fee of an opt-in-RBF transaction T, replacing it with a new transaction B.\n"
"An opt-in RBF transaction with the given txid must be in the wallet.\n"
"The command will pay the additional fee by decreasing (or perhaps removing) its change output.\n"
"If the change output is not big enough to cover the increased fee, the command will currently fail\n"
"If an explicit \"reduce_output\" has been picked, that will be decreased instead.\n"
"If the selected output is not big enough to cover the increased fee, the command will currently fail\n"
"instead of adding new inputs to compensate. (A future implementation could improve this.)\n"
"The command will fail if the wallet or mempool contains a transaction that spends one of T's outputs.\n"
"By default, the new fee will be calculated automatically using estimatesmartfee.\n"
Expand Down Expand Up @@ -3196,6 +3197,8 @@ static UniValue bumpfee(const JSONRPCRequest& request)
" \"UNSET\"\n"
" \"ECONOMICAL\"\n"
" \"CONSERVATIVE\""},
{"reduce_output", RPCArg::Type::NUM, /* opt */ true, /* default_val */ "", "Increase the fee by reducing the output value of the output at\n"
" the given index. NOTE: The recipient of the given output will receive a smaller amount!"},
},
"options"},
}}
Expand All @@ -3217,6 +3220,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)

// optional parameters
CAmount totalFee = 0;
int32_t reduce_output = -1;
CCoinControl coin_control;
coin_control.m_signal_bip125_rbf = true;
if (!request.params[1].isNull()) {
Expand All @@ -3227,6 +3231,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)
{"totalFee", UniValueType(UniValue::VNUM)},
{"replaceable", UniValueType(UniValue::VBOOL)},
{"estimate_mode", UniValueType(UniValue::VSTR)},
{"reduce_output", UniValueType(UniValue::VNUM)},
},
true, true);

Expand All @@ -3249,6 +3254,12 @@ static UniValue bumpfee(const JSONRPCRequest& request)
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter");
}
}
if (options.exists("reduce_output")) {
reduce_output = options["reduce_output"].get_int64();
if (reduce_output < 0) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid reduce_output parameter (cannot be negative)");
}
}
}

// Make sure the results are valid at least up to the most recent block
Expand All @@ -3264,7 +3275,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)
CAmount old_fee;
CAmount new_fee;
CMutableTransaction mtx;
feebumper::Result res = feebumper::CreateTransaction(pwallet, hash, coin_control, totalFee, errors, old_fee, new_fee, mtx);
feebumper::Result res = feebumper::CreateTransaction(pwallet, hash, coin_control, totalFee, reduce_output, errors, old_fee, new_fee, mtx);
if (res != feebumper::Result::OK) {
switch(res) {
case feebumper::Result::INVALID_ADDRESS_OR_KEY:
Expand Down

0 comments on commit cf076ba

Please sign in to comment.