Skip to content

Commit a0d4957

Browse files
jonatackmurchandamus
jonatack
and
murchandamus
committed
wallet: introduce fee_rate (sat/vB) param/option
Create a fee_rate (sat/vB) RPC param and replace overloading the conf_target and estimate_mode params in the following 6 RPCs with it: - sendtoaddress - sendmany - send - fundrawtransaction - walletcreatefundedpsbt - bumpfee In RPC bumpfee, the previously existing fee_rate remains but the unit is changed from BTC/kvB to sat/vB. This is a breaking change, but it should not be an overly risky one, as the units change by a factor of 1e5 and any fees specified in BTC/kvB after this commit will either be too low and raise an error or be 1 sat/vB and can be RBFed. Update the test coverage for each RPC. Co-authored-by: Murch <[email protected]>
1 parent e21212f commit a0d4957

File tree

8 files changed

+359
-349
lines changed

8 files changed

+359
-349
lines changed

‎src/rpc/client.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
4141
{ "sendtoaddress", 5 , "replaceable" },
4242
{ "sendtoaddress", 6 , "conf_target" },
4343
{ "sendtoaddress", 8, "avoid_reuse" },
44-
{ "sendtoaddress", 9, "verbose"},
44+
{ "sendtoaddress", 9, "fee_rate"},
45+
{ "sendtoaddress", 10, "verbose"},
4546
{ "settxfee", 0, "amount" },
4647
{ "sethdseed", 0, "newkeypool" },
4748
{ "getreceivedbyaddress", 1, "minconf" },
@@ -73,7 +74,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
7374
{ "sendmany", 4, "subtractfeefrom" },
7475
{ "sendmany", 5 , "replaceable" },
7576
{ "sendmany", 6 , "conf_target" },
76-
{ "sendmany", 8, "verbose" },
77+
{ "sendmany", 8, "fee_rate"},
78+
{ "sendmany", 9, "verbose" },
7779
{ "deriveaddresses", 1, "range" },
7880
{ "scantxoutset", 1, "scanobjects" },
7981
{ "addmultisigaddress", 0, "nrequired" },
@@ -129,7 +131,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
129131
{ "lockunspent", 1, "transactions" },
130132
{ "send", 0, "outputs" },
131133
{ "send", 1, "conf_target" },
132-
{ "send", 3, "options" },
134+
{ "send", 3, "fee_rate"},
135+
{ "send", 4, "options" },
133136
{ "importprivkey", 2, "rescan" },
134137
{ "importaddress", 2, "rescan" },
135138
{ "importaddress", 3, "p2sh" },

‎src/util/fees.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@ const std::vector<std::pair<std::string, FeeEstimateMode>>& FeeModeMap()
4040
{"unset", FeeEstimateMode::UNSET},
4141
{"economical", FeeEstimateMode::ECONOMICAL},
4242
{"conservative", FeeEstimateMode::CONSERVATIVE},
43-
{(CURRENCY_UNIT + "/kB"), FeeEstimateMode::BTC_KB},
44-
{(CURRENCY_ATOM + "/B"), FeeEstimateMode::SAT_B},
4543
};
4644
return FEE_MODES;
4745
}

‎src/wallet/rpcwallet.cpp‎

Lines changed: 77 additions & 49 deletions
Large diffs are not rendered by default.

‎test/functional/rpc_fundrawtransaction.py‎

Lines changed: 86 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ def run_test(self):
9090
self.test_op_return()
9191
self.test_watchonly()
9292
self.test_all_watched_funds()
93-
self.test_feerate_with_conf_target_and_estimate_mode()
9493
self.test_option_feerate()
9594
self.test_address_reuse()
9695
self.test_option_subtract_fee_from_outputs()
@@ -708,74 +707,89 @@ def test_all_watched_funds(self):
708707
wwatch.unloadwallet()
709708

710709
def test_option_feerate(self):
711-
self.log.info("Test fundrawtxn feeRate option")
712-
713-
# Make sure there is exactly one input so coin selection can't skew the result.
714-
assert_equal(len(self.nodes[3].listunspent(1)), 1)
715-
716-
inputs = []
717-
outputs = {self.nodes[3].getnewaddress() : 1}
718-
rawtx = self.nodes[3].createrawtransaction(inputs, outputs)
719-
result = self.nodes[3].fundrawtransaction(rawtx) # uses self.min_relay_tx_fee (set by settxfee)
720-
result2 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee})
721-
result3 = self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 10 * self.min_relay_tx_fee})
722-
assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)", self.nodes[3].fundrawtransaction, rawtx, {"feeRate": 1})
723-
result_fee_rate = result['fee'] * 1000 / count_bytes(result['hex'])
724-
assert_fee_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate)
725-
assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate)
726-
727-
def test_feerate_with_conf_target_and_estimate_mode(self):
728-
self.log.info("Test fundrawtxn passing an explicit fee rate using conf_target and estimate_mode")
710+
self.log.info("Test fundrawtxn with explicit fee rates (fee_rate sat/vB and feeRate BTC/kvB)")
729711
node = self.nodes[3]
730712
# Make sure there is exactly one input so coin selection can't skew the result.
731-
assert_equal(len(node.listunspent(1)), 1)
713+
assert_equal(len(self.nodes[3].listunspent(1)), 1)
732714
inputs = []
733715
outputs = {node.getnewaddress() : 1}
734716
rawtx = node.createrawtransaction(inputs, outputs)
735717

736-
for unit, fee_rate in {"btc/kb": 0.1, "sat/b": 10000}.items():
737-
self.log.info("Test fundrawtxn with conf_target {} estimate_mode {} produces expected fee".format(fee_rate, unit))
738-
# With no arguments passed, expect fee of 141 sats/b.
739-
assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000141, vspan=0.00000001)
740-
# Expect fee to be 10,000x higher when explicit fee 10,000x greater is specified.
741-
result = node.fundrawtransaction(rawtx, {"conf_target": fee_rate, "estimate_mode": unit})
742-
assert_approx(result["fee"], vexp=0.0141, vspan=0.0001)
718+
result = node.fundrawtransaction(rawtx) # uses self.min_relay_tx_fee (set by settxfee)
719+
btc_kvb_to_sat_vb = 100000 # (1e5)
720+
result1 = node.fundrawtransaction(rawtx, {"fee_rate": 2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee})
721+
result2 = node.fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee})
722+
result3 = node.fundrawtransaction(rawtx, {"fee_rate": 10 * btc_kvb_to_sat_vb * self.min_relay_tx_fee})
723+
result4 = node.fundrawtransaction(rawtx, {"feeRate": 10 * self.min_relay_tx_fee})
724+
result_fee_rate = result['fee'] * 1000 / count_bytes(result['hex'])
725+
assert_fee_amount(result1['fee'], count_bytes(result2['hex']), 2 * result_fee_rate)
726+
assert_fee_amount(result2['fee'], count_bytes(result2['hex']), 2 * result_fee_rate)
727+
assert_fee_amount(result3['fee'], count_bytes(result3['hex']), 10 * result_fee_rate)
728+
assert_fee_amount(result4['fee'], count_bytes(result3['hex']), 10 * result_fee_rate)
743729

744-
for field, fee_rate in {"conf_target": 0.1, "estimate_mode": "sat/b"}.items():
745-
self.log.info("Test fundrawtxn raises RPC error if both feeRate and {} are passed".format(field))
746-
assert_raises_rpc_error(
747-
-8, "Cannot specify both {} and feeRate".format(field),
748-
lambda: node.fundrawtransaction(rawtx, {"feeRate": 0.1, field: fee_rate}))
730+
# With no arguments passed, expect fee of 141 satoshis.
731+
assert_approx(node.fundrawtransaction(rawtx)["fee"], vexp=0.00000141, vspan=0.00000001)
732+
# Expect fee to be 10,000x higher when an explicit fee rate 10,000x greater is specified.
733+
result = node.fundrawtransaction(rawtx, {"fee_rate": 10000})
734+
assert_approx(result["fee"], vexp=0.0141, vspan=0.0001)
749735

750736
self.log.info("Test fundrawtxn with invalid estimate_mode settings")
751737
for k, v in {"number": 42, "object": {"foo": "bar"}}.items():
752738
assert_raises_rpc_error(-3, "Expected type string for estimate_mode, got {}".format(k),
753-
lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": v, "conf_target": 0.1}))
754-
for mode in ["foo", Decimal("3.141592")]:
739+
node.fundrawtransaction, rawtx, {"estimate_mode": v, "conf_target": 0.1, "add_inputs": True})
740+
for mode in ["", "foo", Decimal("3.141592")]:
755741
assert_raises_rpc_error(-8, "Invalid estimate_mode parameter",
756-
lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": 0.1}))
742+
node.fundrawtransaction, rawtx, {"estimate_mode": mode, "conf_target": 0.1, "add_inputs": True})
757743

758744
self.log.info("Test fundrawtxn with invalid conf_target settings")
759-
for mode in ["unset", "economical", "conservative", "btc/kb", "sat/b"]:
745+
for mode in ["unset", "economical", "conservative"]:
760746
self.log.debug("{}".format(mode))
761747
for k, v in {"string": "", "object": {"foo": "bar"}}.items():
762748
assert_raises_rpc_error(-3, "Expected type number for conf_target, got {}".format(k),
763-
lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": v}))
764-
if mode in ["btc/kb", "sat/b"]:
765-
assert_raises_rpc_error(-3, "Amount out of range",
766-
lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": -1}))
767-
assert_raises_rpc_error(-4, "Fee rate (0.00000000 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)",
768-
lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": 0}))
769-
else:
770-
for n in [-1, 0, 1009]:
771-
assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008",
772-
lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": mode, "conf_target": n}))
773-
774-
for unit, fee_rate in {"sat/B": 0.99999999, "BTC/kB": 0.00000999}.items():
775-
self.log.info("- raises RPC error 'fee rate too low' if conf_target {} and estimate_mode {} are passed".format(fee_rate, unit))
776-
assert_raises_rpc_error(-4, "Fee rate (0.00000999 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)",
777-
lambda: self.nodes[1].fundrawtransaction(rawtx, {"estimate_mode": unit, "conf_target": fee_rate, "add_inputs": True}))
778-
749+
node.fundrawtransaction, rawtx, {"estimate_mode": mode, "conf_target": v, "add_inputs": True})
750+
for n in [-1, 0, 1009]:
751+
assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008", # max value of 1008 per src/policy/fees.h
752+
node.fundrawtransaction, rawtx, {"estimate_mode": mode, "conf_target": n, "add_inputs": True})
753+
754+
self.log.info("Test invalid fee rate settings")
755+
assert_raises_rpc_error(-4, "Fee rate (0.00000000 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)",
756+
node.fundrawtransaction, rawtx, {"fee_rate": 0, "add_inputs": True})
757+
assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 BTC/kB (must be greater than 0)",
758+
node.fundrawtransaction, rawtx, {"feeRate": 0, "add_inputs": True})
759+
for param, value in {("fee_rate", 100000), ("feeRate", 1.000)}:
760+
assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)",
761+
node.fundrawtransaction, rawtx, {param: value, "add_inputs": True})
762+
assert_raises_rpc_error(-3, "Amount out of range",
763+
node.fundrawtransaction, rawtx, {"fee_rate": -1, "add_inputs": True})
764+
assert_raises_rpc_error(-3, "Amount is not a number or string",
765+
node.fundrawtransaction, rawtx, {"fee_rate": {"foo": "bar"}, "add_inputs": True})
766+
assert_raises_rpc_error(-3, "Invalid amount",
767+
node.fundrawtransaction, rawtx, {"fee_rate": "", "add_inputs": True})
768+
769+
# Test setting explicit fee rate just below the minimum.
770+
self.log.info("- raises RPC error 'fee rate too low' if fee_rate of 0.99999999 sat/vB is passed")
771+
msg = "Fee rate (0.00000999 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)"
772+
assert_raises_rpc_error(-4, msg, node.fundrawtransaction, rawtx, {"fee_rate": 0.99999999, "add_inputs": True})
773+
# This feeRate test only passes if `coinControl.fOverrideFeeRate = true` in wallet/rpcwallet.cpp::FundTransaction is removed.
774+
# assert_raises_rpc_error(-4, msg, node.fundrawtransaction, rawtx, {"feeRate": 0.00000999, "add_inputs": True})
775+
776+
self.log.info("- raises RPC error if both feeRate and fee_rate are passed")
777+
assert_raises_rpc_error(-8, "Cannot specify both fee_rate (sat/vB) and feeRate (BTC/kvB)",
778+
node.fundrawtransaction, rawtx, {"fee_rate": 0.1, "feeRate": 0.1, "add_inputs": True})
779+
780+
self.log.info("- raises RPC error if both feeRate and estimate_mode passed")
781+
assert_raises_rpc_error(-8, "Cannot specify both estimate_mode and feeRate",
782+
node.fundrawtransaction, rawtx, {"estimate_mode": "economical", "feeRate": 0.1, "add_inputs": True})
783+
784+
for param in ["feeRate", "fee_rate"]:
785+
self.log.info("- raises RPC error if both {} and conf_target are passed".format(param))
786+
assert_raises_rpc_error(-8, "Cannot specify both conf_target and {}. Please provide either a confirmation "
787+
"target in blocks for automatic fee estimation, or an explicit fee rate.".format(param),
788+
node.fundrawtransaction, rawtx, {param: 1, "conf_target": 1, "add_inputs": True})
789+
790+
self.log.info("- raises RPC error if both fee_rate and estimate_mode are passed")
791+
assert_raises_rpc_error(-8, "Cannot specify both estimate_mode and fee_rate",
792+
node.fundrawtransaction, rawtx, {"fee_rate": 1, "estimate_mode": "economical", "add_inputs": True})
779793

780794
def test_address_reuse(self):
781795
"""Test no address reuse occurs."""
@@ -803,12 +817,32 @@ def test_option_subtract_fee_from_outputs(self):
803817
outputs = {self.nodes[2].getnewaddress(): 1}
804818
rawtx = self.nodes[3].createrawtransaction(inputs, outputs)
805819

820+
# Test subtract fee from outputs with feeRate (BTC/kvB)
806821
result = [self.nodes[3].fundrawtransaction(rawtx), # uses self.min_relay_tx_fee (set by settxfee)
807822
self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": []}), # empty subtraction list
808823
self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0]}), # uses self.min_relay_tx_fee (set by settxfee)
809824
self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee}),
810825
self.nodes[3].fundrawtransaction(rawtx, {"feeRate": 2 * self.min_relay_tx_fee, "subtractFeeFromOutputs": [0]}),]
826+
dec_tx = [self.nodes[3].decoderawtransaction(tx_['hex']) for tx_ in result]
827+
output = [d['vout'][1 - r['changepos']]['value'] for d, r in zip(dec_tx, result)]
828+
change = [d['vout'][r['changepos']]['value'] for d, r in zip(dec_tx, result)]
829+
830+
assert_equal(result[0]['fee'], result[1]['fee'], result[2]['fee'])
831+
assert_equal(result[3]['fee'], result[4]['fee'])
832+
assert_equal(change[0], change[1])
833+
assert_equal(output[0], output[1])
834+
assert_equal(output[0], output[2] + result[2]['fee'])
835+
assert_equal(change[0] + result[0]['fee'], change[2])
836+
assert_equal(output[3], output[4] + result[4]['fee'])
837+
assert_equal(change[3] + result[3]['fee'], change[4])
811838

839+
# Test subtract fee from outputs with fee_rate (sat/vB)
840+
btc_kvb_to_sat_vb = 100000 # (1e5)
841+
result = [self.nodes[3].fundrawtransaction(rawtx), # uses self.min_relay_tx_fee (set by settxfee)
842+
self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": []}), # empty subtraction list
843+
self.nodes[3].fundrawtransaction(rawtx, {"subtractFeeFromOutputs": [0]}), # uses self.min_relay_tx_fee (set by settxfee)
844+
self.nodes[3].fundrawtransaction(rawtx, {"fee_rate": 2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee}),
845+
self.nodes[3].fundrawtransaction(rawtx, {"fee_rate": 2 * btc_kvb_to_sat_vb * self.min_relay_tx_fee, "subtractFeeFromOutputs": [0]}),]
812846
dec_tx = [self.nodes[3].decoderawtransaction(tx_['hex']) for tx_ in result]
813847
output = [d['vout'][1 - r['changepos']]['value'] for d, r in zip(dec_tx, result)]
814848
change = [d['vout'][r['changepos']]['value'] for d, r in zip(dec_tx, result)]

0 commit comments

Comments
 (0)