Skip to content

Commit

Permalink
minor feerate improvements
Browse files Browse the repository at this point in the history
Summary:
This is a partial backport of [[bitcoin/bitcoin#20220 | core#20220]]

Most of the PR is not applicable because we do not use `estimate_mode` nor `conf_target` for fees.

List of changes and source commits:
- Use a const ref in `FundTransaction` for `options`: bitcoin/bitcoin@1697a40
- Minor rpc_psbt.py refactoring: bitcoin/bitcoin@6e1ea42
- Add a `log.info` in wallet_basic.py: bitcoin/bitcoin@dd341e6
- Improve RPC doc for `subtract_fee_from_outputs`: bitcoin/bitcoin@778b9be

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10716
  • Loading branch information
PiRK committed Dec 22, 2021
1 parent aec24e9 commit dbd80d6
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 35 deletions.
7 changes: 4 additions & 3 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3799,8 +3799,8 @@ static RPCHelpMan listunspent() {
}

void FundTransaction(CWallet *const pwallet, CMutableTransaction &tx,
Amount &fee_out, int &change_position, UniValue options,
CCoinControl &coinControl) {
Amount &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
pwallet->BlockUntilSyncedToCurrentChain();
Expand Down Expand Up @@ -4825,7 +4825,8 @@ static RPCHelpMan send() {
"subtract_fee_from_outputs",
RPCArg::Type::ARR,
/* default */ "empty array",
"A JSON array of integers.\n"
"Outputs to subtract the fee from, specified as integer "
"indices.\n"
"The fee will be equally deducted from the amount of each "
"specified output.\n"
"Those recipients will receive less bitcoins than you "
Expand Down
54 changes: 22 additions & 32 deletions test/functional/rpc_psbt.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,42 +118,32 @@ def run_test(self):
self.nodes[1].sendrawtransaction(
self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex'])

# feeRate of 100,000 XEC / KB produces a total fee slightly below
# -maxtxfee
inputs = [{"txid": txid, "vout": p2sh_pos},
{"txid": txid, "vout": p2pkh_pos}]
output = {self.nodes[1].getnewaddress(): 29_990_000}

self.log.info(
"Test walletcreatefundedpsbt feeRate of 100,000 XEC/kB produces a "
"total fee at or slightly below -maxtxfee")
res = self.nodes[1].walletcreatefundedpsbt(
[
{
"txid": txid, "vout": p2sh_pos}, {
"txid": txid, "vout": p2pkh_pos}], {
self.nodes[1].getnewaddress(): 29990000}, 0, {
"feeRate": 100000, "add_inputs": True})
inputs, output, 0, {"feeRate": 100_000, "add_inputs": True})
assert_approx(res["fee"], 65000, 5000)

# feeRate of 10,000,000 XEC / KB produces a total fee well above -maxtxfee
self.log.info(
"Test walletcreatefundedpsbt feeRate of 10,000,000 XEC/kB produces "
"a total fee well above -maxtxfee and raises RPC error")
# previously this was silently capped at -maxtxfee
assert_raises_rpc_error(-4,
"Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)",
self.nodes[1].walletcreatefundedpsbt,
[{"txid": txid,
"vout": p2sh_pos},
{"txid": txid,
"vout": p2pkh_pos}],
{self.nodes[1].getnewaddress(): 29990000},
0,
{"feeRate": 10000000,
"add_inputs": True})
assert_raises_rpc_error(-4,
"Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)",
self.nodes[1].walletcreatefundedpsbt,
[{"txid": txid,
"vout": p2sh_pos},
{"txid": txid,
"vout": p2pkh_pos}],
{self.nodes[1].getnewaddress(): 1000000},
0,
{"feeRate": 10000000,
"add_inputs": False})

for bool_add, output_ in (
(True, output),
(False, {self.nodes[1].getnewaddress(): 1_000_000})):
assert_raises_rpc_error(
-4,
"Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)",
self.nodes[1].walletcreatefundedpsbt,
inputs, output_, 0,
{"feeRate": 10_000_000, "add_inputs": bool_add})

self.log.info("Test various PSBT operations")
# partially sign multisig things with node 1
psbtx = self.nodes[1].walletcreatefundedpsbt([{"txid": txid, "vout": p2sh_pos}], {
self.nodes[1].getnewaddress(): 9990000})['psbt']
Expand Down
2 changes: 2 additions & 0 deletions test/functional/wallet_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ def run_test(self):
node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), Decimal(
'20000000'), fee_per_byte, count_bytes(self.nodes[2].gettransaction(txid)['hex']))

self.log.info("Test sendmany")

# Sendmany 10,000,000 XEC
txid = self.nodes[2].sendmany('', {address: 10000000}, 0, "", [])
self.nodes[2].generate(1)
Expand Down

0 comments on commit dbd80d6

Please sign in to comment.