Skip to content

Commit

Permalink
wallet: improve bumpfee error/help, add explicit fee rate coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
@jonatack
jonatack committed Oct 27, 2020
1 parent fc57217 commit 1697a40
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 12 deletions.
8 changes: 4 additions & 4 deletions src/wallet/rpcwallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3039,7 +3039,7 @@ static RPCHelpMan listunspent()
};
}

void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, UniValue options, CCoinControl& coinControl)
void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& fee_out, int& change_position, const UniValue& options, CCoinControl& coinControl)
{
// Make sure the results are valid at least up to the most recent block
// the user could have gotten from another RPC command prior to now
Expand Down Expand Up @@ -3373,7 +3373,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
"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"
"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"
"By default, the new fee will be calculated automatically using the estimatesmartfee RPC.\n"
"The user can specify a confirmation target for estimatesmartfee.\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"
Expand Down Expand Up @@ -3459,7 +3459,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)

if (!conf_target.isNull()) {
if (options.exists("fee_rate")) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "conf_target can't be set with fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate.");
throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both conf_target and 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")) {
CFeeRate fee_rate(AmountFromValue(options["fee_rate"]));
Expand Down Expand Up @@ -4115,7 +4115,7 @@ static RPCHelpMan send()
CMutableTransaction rawTx = ConstructTransaction(options["inputs"], request.params[0], options["locktime"], rbf);
CCoinControl coin_control;
// Automatically select coins, unless at least one is manually selected. Can
// be overriden by options.add_inputs.
// be overridden by options.add_inputs.
coin_control.m_add_inputs = rawTx.vin.size() == 0;
FundTransaction(pwallet, rawTx, fee, change_position, options, coin_control);

Expand Down
47 changes: 39 additions & 8 deletions test/functional/wallet_bumpfee.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,19 +173,50 @@ def test_feerate_args(self, rbf_node, peer_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, "conf_target 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})
assert_raises_rpc_error(-8, "conf_target 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":0.00001, "confTarget": 1})

# 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": INSUFFICIENT})

# For each fee mode, bumping to just above minrelay should fail to increase the total fee enough.
for options in [{"fee_rate": INSUFFICIENT}, {"conf_target": INSUFFICIENT, "estimate_mode": BTC_MODE}, {"conf_target": 1, "estimate_mode": SAT_MODE}]:
assert_raises_rpc_error(-8, "Insufficient total fee", rbf_node.bumpfee, rbfid, options)
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": TOO_HIGH})
self.clear_mempool()

self.log.info("Test explicit fee rate raises RPC error if estimate_mode is passed without a conf_target")
for unit, fee_rate in {"SAT/B": 10, "BTC/KB": NORMAL}.items():
assert_raises_rpc_error(-8, "Selected estimate_mode {} requires a fee rate to be specified in conf_target".format(unit),
rbf_node.bumpfee, rbfid, {"fee_rate": fee_rate, "estimate_mode": unit})

self.log.info("Test explicit fee rate raises RPC error if both fee_rate and conf_target are passed")
error_both = "Cannot specify both conf_target and fee_rate. Please provide either a confirmation " \
"target in blocks for automatic fee estimation, or an explicit fee rate."
assert_raises_rpc_error(-8, error_both, rbf_node.bumpfee, rbfid, {"conf_target": NORMAL, "fee_rate": NORMAL})

self.log.info("Test invalid conf_target settings")
for field in ["confTarget", "conf_target"]:
assert_raises_rpc_error(-8, error_both, rbf_node.bumpfee, rbfid, {field: 1, "fee_rate": NORMAL})
too_high = "is too high (cannot be higher than -maxtxfee"
assert_raises_rpc_error(-4, too_high, lambda: rbf_node.bumpfee(rbfid, {"estimate_mode": BTC_MODE, "conf_target": 2009}))
assert_raises_rpc_error(-4, too_high, lambda: rbf_node.bumpfee(rbfid, {"estimate_mode": SAT_MODE, "conf_target": 2009 * 10000}))

self.log.info("Test invalid estimate_mode settings")
for k, v in {"number": 42, "object": {"foo": "bar"}}.items():
assert_raises_rpc_error(-3, "Expected type string for estimate_mode, got {}".format(k),
lambda: rbf_node.bumpfee(rbfid, {"estimate_mode": v, "fee_rate": NORMAL}))
for mode in ["foo", Decimal("3.141592")]:
assert_raises_rpc_error(-8, "Invalid estimate_mode parameter",
lambda: rbf_node.bumpfee(rbfid, {"estimate_mode": mode, "fee_rate": NORMAL}))

self.log.info("Test invalid fee_rate settings")
for mode in ["unset", "economical", "conservative", BTC_MODE, SAT_MODE]:
self.log.debug("{}".format(mode))
for k, v in {"string": "", "object": {"foo": "bar"}}.items():
assert_raises_rpc_error(-3, "Expected type number for fee_rate, got {}".format(k),
lambda: rbf_node.bumpfee(rbfid, {"estimate_mode": mode, "fee_rate": v}))
assert_raises_rpc_error(-3, "Amount out of range",
lambda: rbf_node.bumpfee(rbfid, {"estimate_mode": mode, "fee_rate": -1}))
assert_raises_rpc_error(-8, "Invalid fee_rate 0.00000000 BTC/kB (must be greater than 0)",
lambda: rbf_node.bumpfee(rbfid, {"estimate_mode": mode, "fee_rate": 0}))
self.clear_mempool()

def test_segwit_bumpfee_succeeds(self, rbf_node, dest_address):
self.log.info('Test that segwit-sourcing bumpfee works')
Expand Down

0 comments on commit 1697a40

Please sign in to comment.