Merge #18312: wallet: remove deprecated fee bumping by totalFee

c3857c5fcb wallet: remove CreateTotalBumpTransaction() (Jon Atack)
4a0b27bb01 wallet: remove totalfee from createBumpTransaction() (Jon Atack)
e347cfa9a7 rpc: remove deprecated totalFee arg from RPC bumpfee (Jon Atack)
bd05f96d79 test: delete wallet_bumpfee_totalfee_deprecation.py (Jon Atack)
a6d1ab8caa test: update bumpfee testing from totalFee to fee_rate (Jon Atack)

Pull request description:

  Since 0.19, fee-bumping using `totalFee` was deprecated in #15996 and replaced by `fee_rate` in #16727. This changeset removes it.

ACKs for top commit:
  laanwj:
    ACK c3857c5fcb

Tree-SHA512: c1bb15d664baf4d2dea06981f36384af02057d125c51fcbc8640b9d5563532187c7b84aa952f7b575255a88ce383ed4d7495bec920a47b05b6fc0d432dce1f00
This commit is contained in:
Wladimir J. van der Laan 2020-03-26 18:30:52 +01:00
commit 694f4cbd78
No known key found for this signature in database
GPG Key ID: 1E4AED62986CD25D
9 changed files with 43 additions and 262 deletions

View File

@ -253,19 +253,12 @@ public:
}
bool createBumpTransaction(const uint256& txid,
const CCoinControl& coin_control,
CAmount total_fee,
std::vector<std::string>& errors,
CAmount& old_fee,
CAmount& new_fee,
CMutableTransaction& mtx) override
{
if (total_fee > 0) {
return feebumper::CreateTotalBumpTransaction(m_wallet.get(), txid, coin_control, total_fee, errors, old_fee, new_fee, mtx) ==
feebumper::Result::OK;
} else {
return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) ==
feebumper::Result::OK;
}
return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) == feebumper::Result::OK;
}
bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }
bool commitBumpTransaction(const uint256& txid,

View File

@ -155,7 +155,6 @@ public:
//! Create bump transaction.
virtual bool createBumpTransaction(const uint256& txid,
const CCoinControl& coin_control,
CAmount total_fee,
std::vector<std::string>& errors,
CAmount& old_fee,
CAmount& new_fee,

View File

@ -482,7 +482,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, 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;

View File

@ -60,7 +60,7 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet
static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, std::vector<std::string>& errors) {
// 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)
// This may occur if the user set FeeRate, TotalFee or paytxfee too low, if fallbackfee is too low, or, perhaps,
// This may occur if the user set fee_rate or paytxfee too low, if fallbackfee is too low, or, perhaps,
// in a rare situation where the mempool minimum fee increased significantly since the fee estimation just a
// moment earlier. In this case, we report an error to the user, who may adjust the fee.
CFeeRate minMempoolFeeRate = wallet.chain().mempoolMinFee();
@ -150,132 +150,6 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
return res == feebumper::Result::OK;
}
Result CreateTotalBumpTransaction(const CWallet* wallet, const uint256& txid, const CCoinControl& coin_control, CAmount total_fee, std::vector<std::string>& errors,
CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx)
{
new_fee = total_fee;
auto locked_chain = wallet->chain().lock();
LOCK(wallet->cs_wallet);
errors.clear();
auto it = wallet->mapWallet.find(txid);
if (it == wallet->mapWallet.end()) {
errors.push_back("Invalid or non-wallet transaction id");
return Result::INVALID_ADDRESS_OR_KEY;
}
const CWalletTx& wtx = it->second;
Result result = PreconditionChecks(*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;
}
nOutput = i;
}
}
if (nOutput == -1) {
errors.push_back("Transaction does not have a change output");
return Result::WALLET_ERROR;
}
// Calculate the expected size of the new transaction.
int64_t txSize = GetVirtualTransactionSize(*(wtx.tx));
const int64_t maxNewTxSize = CalculateMaximumSignedTxSize(*wtx.tx, wallet);
if (maxNewTxSize < 0) {
errors.push_back("Transaction contains inputs that cannot be signed");
return Result::INVALID_ADDRESS_OR_KEY;
}
// calculate the old fee and fee-rate
isminefilter filter = wallet->GetLegacyScriptPubKeyMan() && wallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
old_fee = wtx.GetDebit(filter) - wtx.tx->GetValueOut();
CFeeRate nOldFeeRate(old_fee, txSize);
// The wallet uses a conservative WALLET_INCREMENTAL_RELAY_FEE value to
// future proof against changes to network wide policy for incremental relay
// fee that our node may not be aware of.
CFeeRate nodeIncrementalRelayFee = wallet->chain().relayIncrementalFee();
CFeeRate walletIncrementalRelayFee = CFeeRate(WALLET_INCREMENTAL_RELAY_FEE);
if (nodeIncrementalRelayFee > walletIncrementalRelayFee) {
walletIncrementalRelayFee = nodeIncrementalRelayFee;
}
CAmount minTotalFee = nOldFeeRate.GetFee(maxNewTxSize) + nodeIncrementalRelayFee.GetFee(maxNewTxSize);
if (total_fee < minTotalFee) {
errors.push_back(strprintf("Insufficient totalFee, must be at least %s (oldFee %s + incrementalFee %s)",
FormatMoney(minTotalFee), FormatMoney(nOldFeeRate.GetFee(maxNewTxSize)), FormatMoney(nodeIncrementalRelayFee.GetFee(maxNewTxSize))));
return Result::INVALID_PARAMETER;
}
CAmount requiredFee = GetRequiredFee(*wallet, maxNewTxSize);
if (total_fee < requiredFee) {
errors.push_back(strprintf("Insufficient totalFee (cannot be less than required fee %s)",
FormatMoney(requiredFee)));
return Result::INVALID_PARAMETER;
}
// Check that in all cases the new fee doesn't violate maxTxFee
const CAmount max_tx_fee = wallet->m_default_max_tx_fee;
if (new_fee > max_tx_fee) {
errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than -maxtxfee %s)",
FormatMoney(new_fee), FormatMoney(max_tx_fee)));
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)
// This may occur if the user set TotalFee or paytxfee too low, if fallbackfee is too low, or, perhaps,
// in a rare situation where the mempool minimum fee increased significantly since the fee estimation just a
// moment earlier. In this case, we report an error to the user, who may use total_fee to make an adjustment.
CFeeRate minMempoolFeeRate = wallet->chain().mempoolMinFee();
CFeeRate nNewFeeRate = CFeeRate(total_fee, maxNewTxSize);
if (nNewFeeRate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) {
errors.push_back(strprintf(
"New fee rate (%s) is lower than the minimum fee rate (%s) to get into the mempool -- "
"the totalFee value should be at least %s to add transaction",
FormatMoney(nNewFeeRate.GetFeePerK()),
FormatMoney(minMempoolFeeRate.GetFeePerK()),
FormatMoney(minMempoolFeeRate.GetFee(maxNewTxSize))));
return Result::WALLET_ERROR;
}
// Now modify the output to increase the fee.
// If the output is not large enough to pay the fee, fail.
CAmount nDelta = new_fee - old_fee;
assert(nDelta > 0);
mtx = CMutableTransaction{*wtx.tx};
CTxOut* poutput = &(mtx.vout[nOutput]);
if (poutput->nValue < nDelta) {
errors.push_back("Change output is too small to bump the fee");
return Result::WALLET_ERROR;
}
// If the output would become dust, discard it (converting the dust to fee)
poutput->nValue -= nDelta;
if (poutput->nValue <= GetDustThreshold(*poutput, GetDiscardRate(*wallet))) {
wallet->WalletLogPrintf("Bumping fee and discarding dust output\n");
new_fee += poutput->nValue;
mtx.vout.erase(mtx.vout.begin() + nOutput);
}
// Mark new tx not replaceable, if requested.
if (!coin_control.m_signal_bip125_rbf.get_value_or(wallet->m_signal_rbf)) {
for (auto& input : mtx.vin) {
if (input.nSequence < 0xfffffffe) input.nSequence = 0xfffffffe;
}
}
return Result::OK;
}
Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<std::string>& errors,
CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx)
{

View File

@ -28,16 +28,6 @@ enum class Result
//! Return whether transaction can be bumped.
bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid);
//! Create bumpfee transaction based on total amount.
Result CreateTotalBumpTransaction(const CWallet* wallet,
const uint256& txid,
const CCoinControl& coin_control,
CAmount total_fee,
std::vector<std::string>& errors,
CAmount& old_fee,
CAmount& new_fee,
CMutableTransaction& mtx);
//! Create bumpfee transaction based on feerate estimates.
Result CreateRateBumpTransaction(CWallet& wallet,
const uint256& txid,

View File

@ -3342,12 +3342,11 @@ 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 reducing change outputs or adding inputs when necessary. It may add a new change output if one does not already exist.\n"
"If `totalFee` (DEPRECATED) is given, adding inputs is not supported, so there must be a single change output that is big enough or it will fail.\n"
"All inputs in the original transaction will be included in the replacement transaction.\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"
"The user can specify a confirmation target for estimatesmartfee.\n"
"Alternatively, the user can specify totalFee (DEPRECATED), or fee_rate (" + CURRENCY_UNIT + " per kB) for the new transaction .\n"
"Alternatively, the user can specify a fee_rate (" + CURRENCY_UNIT + " per kB) for the new transaction.\n"
"At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n"
"returned by getnetworkinfo) to enter the node's mempool.\n",
{
@ -3355,11 +3354,7 @@ static UniValue bumpfee(const JSONRPCRequest& request)
{"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "",
{
{"confTarget", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"},
{"totalFee", RPCArg::Type::NUM, /* default */ "fallback to 'confTarget'", "Total fee (NOT feerate) to pay, in satoshis. (DEPRECATED)\n"
" In rare cases, the actual fee paid might be slightly higher than the specified\n"
" totalFee if the tx change output has to be removed because it is too close to\n"
" the dust threshold."},
{"fee_rate", RPCArg::Type::NUM, /* default */ "fallback to 'confTarget'", "FeeRate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n"
{"fee_rate", RPCArg::Type::NUM, /* default */ "fall back to 'confTarget'", "fee rate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n"
" Specify a fee rate instead of relying on the built-in fee estimator.\n"
"Must be at least 0.0001 " + CURRENCY_UNIT + " per kB higher than the current transaction fee rate.\n"},
{"replaceable", RPCArg::Type::BOOL, /* default */ "true", "Whether the new transaction should still be\n"
@ -3400,7 +3395,6 @@ static UniValue bumpfee(const JSONRPCRequest& request)
CCoinControl coin_control;
coin_control.fAllowWatchOnly = pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS);
// optional parameters
CAmount totalFee = 0;
coin_control.m_signal_bip125_rbf = true;
if (!request.params[1].isNull()) {
@ -3408,26 +3402,15 @@ static UniValue bumpfee(const JSONRPCRequest& request)
RPCTypeCheckObj(options,
{
{"confTarget", UniValueType(UniValue::VNUM)},
{"totalFee", UniValueType(UniValue::VNUM)},
{"fee_rate", UniValueType(UniValue::VNUM)},
{"replaceable", UniValueType(UniValue::VBOOL)},
{"estimate_mode", UniValueType(UniValue::VSTR)},
},
true, true);
if (options.exists("confTarget") && (options.exists("totalFee") || options.exists("fee_rate"))) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget can't be set with totalFee or fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.");
} else if (options.exists("fee_rate") && options.exists("totalFee")) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "fee_rate can't be set along with totalFee.");
if (options.exists("confTarget") && options.exists("fee_rate")) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget can't be set with fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.");
} else if (options.exists("confTarget")) { // TODO: alias this to conf_target
coin_control.m_confirm_target = ParseConfirmTarget(options["confTarget"], pwallet->chain().estimateMaxBlocks());
} else if (options.exists("totalFee")) {
if (!pwallet->chain().rpcEnableDeprecated("totalFee")) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "totalFee argument has been deprecated and will be removed in 0.20. Please use -deprecatedrpc=totalFee to continue using this argument until removal.");
}
totalFee = options["totalFee"].get_int64();
if (totalFee <= 0) {
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid totalFee %s (must be greater than 0)", FormatMoney(totalFee)));
}
} else if (options.exists("fee_rate")) {
CFeeRate fee_rate(AmountFromValue(options["fee_rate"]));
if (fee_rate <= CFeeRate(0)) {
@ -3460,13 +3443,8 @@ static UniValue bumpfee(const JSONRPCRequest& request)
CAmount new_fee;
CMutableTransaction mtx;
feebumper::Result res;
if (totalFee > 0) {
// Targeting total fee bump. Requires a change output of sufficient size.
res = feebumper::CreateTotalBumpTransaction(pwallet, hash, coin_control, totalFee, errors, old_fee, new_fee, mtx);
} else {
// Targeting feerate bump.
res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx);
}
// Targeting feerate bump.
res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx);
if (res != feebumper::Result::OK) {
switch(res) {
case feebumper::Result::INVALID_ADDRESS_OR_KEY:
@ -4196,7 +4174,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
},
{"replaceable", RPCArg::Type::BOOL, /* default */ "wallet default", "Marks this transaction as BIP125 replaceable.\n"
" Allows this transaction to be replaced by a transaction with higher fees"},
{"conf_target", RPCArg::Type::NUM, /* default */ "Fallback to wallet's confirmation target", "Confirmation target (in blocks)"},
{"conf_target", RPCArg::Type::NUM, /* default */ "fall back to wallet's confirmation target (txconfirmtarget)", "Confirmation target (in blocks)"},
{"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n"
" \"UNSET\"\n"
" \"ECONOMICAL\"\n"

View File

@ -182,7 +182,6 @@ BASE_SCRIPTS = [
'rpc_bind.py --nonloopback',
'mining_basic.py',
'wallet_bumpfee.py',
'wallet_bumpfee_totalfee_deprecation.py',
'wallet_implicitsegwit.py',
'rpc_named_arguments.py',
'wallet_listsinceblock.py',

View File

@ -30,6 +30,13 @@ from test_framework.util import (
WALLET_PASSPHRASE = "test"
WALLET_PASSPHRASE_TIMEOUT = 3600
# Fee rates (in BTC per 1000 vbytes)
INSUFFICIENT = 0.00001000
ECONOMICAL = 0.00050000
NORMAL = 0.00100000
HIGH = 0.00500000
TOO_HIGH = 1.00000000
class BumpFeeTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 2
@ -37,7 +44,6 @@ class BumpFeeTest(BitcoinTestFramework):
self.extra_args = [[
"-walletrbf={}".format(i),
"-mintxfee=0.00002",
"-deprecatedrpc=totalFee",
"-addresstype=bech32",
] for i in range(self.num_nodes)]
@ -75,7 +81,6 @@ class BumpFeeTest(BitcoinTestFramework):
test_nonrbf_bumpfee_fails(self, peer_node, dest_address)
test_notmine_bumpfee_fails(self, rbf_node, peer_node, dest_address)
test_bumpfee_with_descendant_fails(self, rbf_node, rbf_node_address, dest_address)
test_small_output_fails(self, rbf_node, dest_address)
test_dust_to_fee(self, rbf_node, dest_address)
test_settxfee(self, rbf_node, dest_address)
test_watchonly_psbt(self, peer_node, rbf_node, dest_address)
@ -93,13 +98,13 @@ class BumpFeeTest(BitcoinTestFramework):
def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
self.log.info('Test simple bumpfee')
self.log.info('Test simple bumpfee: {}'.format(mode))
rbfid = spend_one_input(rbf_node, dest_address)
rbftx = rbf_node.gettransaction(rbfid)
self.sync_mempools((rbf_node, peer_node))
assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool()
if mode == "fee_rate":
bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate":0.0015})
bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate": NORMAL})
else:
bumped_tx = rbf_node.bumpfee(rbfid)
assert_equal(bumped_tx["errors"], [])
@ -120,21 +125,21 @@ def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
assert_equal(bumpedwtx["replaces_txid"], rbfid)
def test_feerate_args(self, rbf_node, peer_node, dest_address):
self.log.info('Test feerate args')
self.log.info('Test fee_rate args')
rbfid = spend_one_input(rbf_node, dest_address)
self.sync_mempools((rbf_node, peer_node))
assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool()
assert_raises_rpc_error(-8, "confTarget can't be set with totalFee or fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.", rbf_node.bumpfee, rbfid, {"fee_rate":0.00001, "confTarget":1})
assert_raises_rpc_error(-8, "confTarget can't be set with totalFee or fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.", rbf_node.bumpfee, rbfid, {"totalFee":0.00001, "confTarget":1})
assert_raises_rpc_error(-8, "fee_rate can't be set along with totalFee.", rbf_node.bumpfee, rbfid, {"fee_rate":0.00001, "totalFee":0.001})
assert_raises_rpc_error(-8, "confTarget can't be set with fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.", rbf_node.bumpfee, rbfid, {"fee_rate": NORMAL, "confTarget": 1})
assert_raises_rpc_error(-3, "Unexpected key totalFee", rbf_node.bumpfee, rbfid, {"totalFee": NORMAL})
# Bumping to just above minrelay should fail to increase total fee enough, at least
assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, rbfid, {"fee_rate":0.00001000})
assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, rbfid, {"fee_rate": INSUFFICIENT})
assert_raises_rpc_error(-3, "Amount out of range", rbf_node.bumpfee, rbfid, {"fee_rate":-1})
assert_raises_rpc_error(-4, "is too high (cannot be higher than", rbf_node.bumpfee, rbfid, {"fee_rate":1})
assert_raises_rpc_error(-4, "is too high (cannot be higher than", rbf_node.bumpfee, rbfid, {"fee_rate": TOO_HIGH})
def test_segwit_bumpfee_succeeds(self, rbf_node, dest_address):
@ -204,15 +209,6 @@ def test_bumpfee_with_descendant_fails(self, rbf_node, rbf_node_address, dest_ad
rbf_node.sendrawtransaction(tx["hex"])
assert_raises_rpc_error(-8, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id)
def test_small_output_fails(self, rbf_node, dest_address):
self.log.info('Test totalFee bump with small output fails')
# cannot bump fee with a too-small output
rbfid = spend_one_input(rbf_node, dest_address)
rbf_node.bumpfee(rbfid, {"totalFee": 50000})
rbfid = spend_one_input(rbf_node, dest_address)
assert_raises_rpc_error(-4, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 50001})
def test_small_output_with_feerate_succeeds(self, rbf_node, dest_address):
self.log.info('Testing small output with feerate bump succeeds')
@ -255,15 +251,19 @@ def test_small_output_with_feerate_succeeds(self, rbf_node, dest_address):
def test_dust_to_fee(self, rbf_node, dest_address):
self.log.info('Test that bumped output that is dust is dropped to fee')
# the bumped tx sets fee=49,900, but it converts to 50,000
rbfid = spend_one_input(rbf_node, dest_address)
fulltx = rbf_node.getrawtransaction(rbfid, 1)
# (31-vbyte p2wpkh output size + 67-vbyte p2wpkh spend estimate) * 10k(discard_rate) / 1000 = 980
bumped_tx = rbf_node.bumpfee(rbfid, {"totalFee": 50000 - 980})
# size of transaction (p2wpkh, 1 input, 2 outputs): 141 vbytes
assert_equal(fulltx["vsize"], 141)
# bump with fee_rate of 0.00350000 BTC per 1000 vbytes
# expected bump fee of 141 vbytes * fee_rate 0.00350000 BTC / 1000 vbytes = 0.00049350 BTC
# but dust is dropped, so actual bump fee is 0.00050000
bumped_tx = rbf_node.bumpfee(rbfid, {"fee_rate": 0.0035})
full_bumped_tx = rbf_node.getrawtransaction(bumped_tx["txid"], 1)
assert_equal(bumped_tx["fee"], Decimal("0.00050000"))
assert_equal(len(fulltx["vout"]), 2)
assert_equal(len(full_bumped_tx["vout"]), 1) # change output is eliminated
assert_equal(full_bumped_tx["vout"][0]['value'], Decimal("0.00050000"))
def test_settxfee(self, rbf_node, dest_address):
@ -285,7 +285,8 @@ def test_settxfee(self, rbf_node, dest_address):
def test_maxtxfee_fails(self, rbf_node, dest_address):
self.log.info('Test that bumpfee fails when it hits -matxfee')
# size of bumped transaction (p2wpkh, 1 input, 2 outputs): 141 vbytes
# expected bumping feerate of 20 sats/vbyte => 141*20 sats = 0.00002820 btc
# expected bump fee of 141 vbytes * 0.00200000 BTC / 1000 vbytes = 0.00002820 BTC
# which exceeds maxtxfee and is expected to raise
self.restart_node(1, ['-maxtxfee=0.000025'] + self.extra_args[1])
rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
rbfid = spend_one_input(rbf_node, dest_address)
@ -356,7 +357,7 @@ def test_watchonly_psbt(self, peer_node, rbf_node, dest_address):
assert_equal(len(watcher.decodepsbt(psbt)["tx"]["vin"]), 1)
# Bump fee, obnoxiously high to add additional watchonly input
bumped_psbt = watcher.bumpfee(original_txid, {"fee_rate":0.005})
bumped_psbt = watcher.bumpfee(original_txid, {"fee_rate": HIGH})
assert_greater_than(len(watcher.decodepsbt(bumped_psbt['psbt'])["tx"]["vin"]), 1)
assert "txid" not in bumped_psbt
assert_equal(bumped_psbt["origfee"], -watcher.gettransaction(original_txid)["fee"])
@ -378,17 +379,17 @@ def test_watchonly_psbt(self, peer_node, rbf_node, dest_address):
def test_rebumping(self, rbf_node, dest_address):
self.log.info('Test that re-bumping the original tx fails, but bumping successor works')
rbfid = spend_one_input(rbf_node, dest_address)
bumped = rbf_node.bumpfee(rbfid, {"totalFee": 2000})
assert_raises_rpc_error(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 3000})
rbf_node.bumpfee(bumped["txid"], {"totalFee": 3000})
bumped = rbf_node.bumpfee(rbfid, {"fee_rate": ECONOMICAL})
assert_raises_rpc_error(-4, "already bumped", rbf_node.bumpfee, rbfid, {"fee_rate": NORMAL})
rbf_node.bumpfee(bumped["txid"], {"fee_rate": NORMAL})
def test_rebumping_not_replaceable(self, rbf_node, dest_address):
self.log.info('Test that re-bumping non-replaceable fails')
rbfid = spend_one_input(rbf_node, dest_address)
bumped = rbf_node.bumpfee(rbfid, {"totalFee": 10000, "replaceable": False})
bumped = rbf_node.bumpfee(rbfid, {"fee_rate": ECONOMICAL, "replaceable": False})
assert_raises_rpc_error(-4, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"],
{"totalFee": 20000})
{"fee_rate": NORMAL})
def test_unconfirmed_not_spendable(self, rbf_node, rbf_node_address):
@ -450,7 +451,7 @@ def test_locked_wallet_fails(self, rbf_node, dest_address):
rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
def test_change_script_match(self, rbf_node, dest_address):
self.log.info('Test that the same change addresses is used for the replacement transaction when possible.')
self.log.info('Test that the same change addresses is used for the replacement transaction when possible')
def get_change_address(tx):
tx_details = rbf_node.getrawtransaction(tx, 1)
@ -463,7 +464,7 @@ def test_change_script_match(self, rbf_node, dest_address):
assert_equal(len(change_addresses), 1)
# Now find that address in each subsequent tx, and no other change
bumped_total_tx = rbf_node.bumpfee(rbfid, {"totalFee": 2000})
bumped_total_tx = rbf_node.bumpfee(rbfid, {"fee_rate": ECONOMICAL})
assert_equal(change_addresses, get_change_address(bumped_total_tx['txid']))
bumped_rate_tx = rbf_node.bumpfee(bumped_total_tx["txid"])
assert_equal(change_addresses, get_change_address(bumped_rate_tx['txid']))
@ -503,5 +504,6 @@ def test_no_more_inputs_fails(self, rbf_node, dest_address):
rbfid = rbf_node.sendtoaddress(rbf_node.getnewaddress(), rbf_node.getbalance(), "", "", True)
assert_raises_rpc_error(-4, "Unable to create transaction: Insufficient funds", rbf_node.bumpfee, rbfid)
if __name__ == "__main__":
BumpFeeTest().main()

View File

@ -1,54 +0,0 @@
#!/usr/bin/env python3
# Copyright (c) 2019 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test deprecation of passing `totalFee` to the bumpfee RPC."""
from decimal import Decimal
from test_framework.messages import BIP125_SEQUENCE_NUMBER
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import assert_raises_rpc_error
class BumpFeeWithTotalFeeArgumentDeprecationTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 2
self.extra_args = [[
"-walletrbf={}".format(i),
"-mintxfee=0.00002",
] for i in range(self.num_nodes)]
def skip_test_if_missing_module(self):
self.skip_if_no_wallet()
def run_test(self):
peer_node, rbf_node = self.nodes
peer_node.generate(110)
self.sync_all()
peer_node.sendtoaddress(rbf_node.getnewaddress(), 0.001)
self.sync_all()
peer_node.generate(1)
self.sync_all()
rbfid = spend_one_input(rbf_node, peer_node.getnewaddress())
self.log.info("Testing bumpfee with totalFee argument raises RPC error with deprecation message")
assert_raises_rpc_error(
-8,
"totalFee argument has been deprecated and will be removed in 0.20. " +
"Please use -deprecatedrpc=totalFee to continue using this argument until removal.",
rbf_node.bumpfee, rbfid, {"totalFee": 2000})
self.log.info("Testing bumpfee without totalFee argument does not raise")
rbf_node.bumpfee(rbfid)
def spend_one_input(node, dest_address, change_size=Decimal("0.00049000")):
tx_input = dict(sequence=BIP125_SEQUENCE_NUMBER,
**next(u for u in node.listunspent() if u["amount"] == Decimal("0.00100000")))
destinations = {dest_address: Decimal("0.00050000")}
destinations[node.getrawchangeaddress()] = change_size
rawtx = node.createrawtransaction([tx_input], destinations)
signedtx = node.signrawtransactionwithwallet(rawtx)
txid = node.sendrawtransaction(signedtx["hex"])
return txid
if __name__ == "__main__":
BumpFeeWithTotalFeeArgumentDeprecationTest().main()