Skip to content

Commit 80e32e1

Browse files
author
MarcoFalke
committed
Merge #20305: wallet: introduce fee_rate sat/vB param/option
05e82d8 wallet: override minfee checks (fOverrideFeeRate) for fee_rate (Jon Atack) 9a670b4 wallet: update sendtoaddress, send RPC examples with fee_rate (Jon Atack) be481b7 wallet: use MIN_RELAY_TX_FEE in bumpfee help (Jon Atack) 449b730 wallet: provide valid values if invalid estimate mode passed (Jon Atack) 6da3afb wallet: update remaining rpcwallet fee rate units to BTC/kvB (Jon Atack) 173b5b5 wallet: update fee rate units, use sat/vB for fee_rate error messages (Jon Atack) 7f9835a wallet: remove fee rates from conf_target helps (Jon Atack) b7994c0 wallet: add fee_rate unit warnings to bumpfee (Jon Atack) 410e471 wallet: remove redundant bumpfee fee_rate checks (Jon Atack) a0d4957 wallet: introduce fee_rate (sat/vB) param/option (Jon Atack) e21212f wallet: remove unneeded WALLET_BTC_KB_TO_SAT_B constant (Jon Atack) 6112cf2 wallet: add CFeeRate ctor doxygen documentation (Jon Atack) 3f72791 wallet: fix bug in RPC send options (Jon Atack) Pull request description: This PR builds on #11413 and #20220 to address #19543. - replace overloading the conf_target and estimate_mode params with `fee_rate` in sat/vB in the sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, and bumpfee RPCs - allow non-actionable conf_target value of `0` and estimate_mode value of `""` to be passed to use `fee_rate` as a positional argument, in addition to as a named argument - fix a bug in the experimental send RPC described in bitcoin/bitcoin#20220 (comment) where args were not being passed correctly into the options values - update the feerate error message units for these RPCs from BTC/kB to sat/vB - update the test coverage, help docs, doxygen docs, and some of the RPC examples - other changes to address the excellent review feedback See this wallet meeting log for more context: http://www.erisian.com.au/bitcoin-core-dev/log-2020-11-06.html#l-309 ACKs for top commit: achow101: re-ACK 05e82d8 MarcoFalke: review ACK 05e82d8 did not test and found a few style nits, which can be fixed later 🍯 Xekyo: tACK 05e82d8 Sjors: utACK 05e82d8 Tree-SHA512: a4ee5f184ada53f1840b2923d25873bda88c5a2ae48e67eeea2417a0b35154798cfdb3c147b05dd56bd6608a784e1b91623bb985ee2ab9ef2baaec22206d0a9c
2 parents e7986c5 + 05e82d8 commit 80e32e1

File tree

15 files changed

+447
-426
lines changed

15 files changed

+447
-426
lines changed

‎src/policy/feerate.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ CAmount CFeeRate::GetFee(size_t nBytes_) const
3838
std::string CFeeRate::ToString(const FeeEstimateMode& fee_estimate_mode) const
3939
{
4040
switch (fee_estimate_mode) {
41-
case FeeEstimateMode::SAT_B: return strprintf("%d.%03d %s/B", nSatoshisPerK / 1000, nSatoshisPerK % 1000, CURRENCY_ATOM);
42-
default: return strprintf("%d.%08d %s/kB", nSatoshisPerK / COIN, nSatoshisPerK % COIN, CURRENCY_UNIT);
41+
case FeeEstimateMode::SAT_VB: return strprintf("%d.%03d %s/vB", nSatoshisPerK / 1000, nSatoshisPerK % 1000, CURRENCY_ATOM);
42+
default: return strprintf("%d.%08d %s/kvB", nSatoshisPerK / COIN, nSatoshisPerK % COIN, CURRENCY_UNIT);
4343
}
4444
}

‎src/policy/feerate.h

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ enum class FeeEstimateMode {
1919
UNSET, //!< Use default settings based on other criteria
2020
ECONOMICAL, //!< Force estimateSmartFee to use non-conservative estimates
2121
CONSERVATIVE, //!< Force estimateSmartFee to use conservative estimates
22-
BTC_KB, //!< Use explicit BTC/kB fee given in coin control
23-
SAT_B, //!< Use explicit sat/B fee given in coin control
22+
BTC_KVB, //!< Use BTC/kvB fee rate unit
23+
SAT_VB, //!< Use sat/vB fee rate unit
2424
};
2525

2626
/**
@@ -39,7 +39,16 @@ class CFeeRate
3939
// We've previously had bugs creep in from silent double->int conversion...
4040
static_assert(std::is_integral<I>::value, "CFeeRate should be used without floats");
4141
}
42-
/** Constructor for a fee rate in satoshis per kB. The size in bytes must not exceed (2^63 - 1)*/
42+
/** Constructor for a fee rate in satoshis per kvB (sat/kvB). The size in bytes must not exceed (2^63 - 1).
43+
*
44+
* Passing an nBytes value of COIN (1e8) returns a fee rate in satoshis per vB (sat/vB),
45+
* e.g. (nFeePaid * 1e8 / 1e3) == (nFeePaid / 1e5),
46+
* where 1e5 is the ratio to convert from BTC/kvB to sat/vB.
47+
*
48+
* @param[in] nFeePaid CAmount fee rate to construct with
49+
* @param[in] nBytes size_t bytes (units) to construct with
50+
* @returns fee rate
51+
*/
4352
CFeeRate(const CAmount& nFeePaid, size_t nBytes);
4453
/**
4554
* Return the fee in satoshis for the given size in bytes.
@@ -56,7 +65,7 @@ class CFeeRate
5665
friend bool operator>=(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK >= b.nSatoshisPerK; }
5766
friend bool operator!=(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK != b.nSatoshisPerK; }
5867
CFeeRate& operator+=(const CFeeRate& a) { nSatoshisPerK += a.nSatoshisPerK; return *this; }
59-
std::string ToString(const FeeEstimateMode& fee_estimate_mode = FeeEstimateMode::BTC_KB) const;
68+
std::string ToString(const FeeEstimateMode& fee_estimate_mode = FeeEstimateMode::BTC_KVB) const;
6069

6170
SERIALIZE_METHODS(CFeeRate, obj) { READWRITE(obj.nSatoshisPerK); }
6271
};

‎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/rpc/mining.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1070,7 +1070,7 @@ static RPCHelpMan estimatesmartfee()
10701070
if (!request.params[1].isNull()) {
10711071
FeeEstimateMode fee_mode;
10721072
if (!FeeModeFromString(request.params[1].get_str(), fee_mode)) {
1073-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter");
1073+
throw JSONRPCError(RPC_INVALID_PARAMETER, InvalidEstimateModeErrorMessage());
10741074
}
10751075
if (fee_mode == FeeEstimateMode::ECONOMICAL) conservative = false;
10761076
}

‎src/test/amount_tests.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ BOOST_AUTO_TEST_CASE(BinaryOperatorTest)
9898
BOOST_CHECK(a <= a);
9999
BOOST_CHECK(b >= a);
100100
BOOST_CHECK(b >= b);
101-
// a should be 0.00000002 BTC/kB now
101+
// a should be 0.00000002 BTC/kvB now
102102
a += a;
103103
BOOST_CHECK(a == b);
104104
}
@@ -107,7 +107,9 @@ BOOST_AUTO_TEST_CASE(ToStringTest)
107107
{
108108
CFeeRate feeRate;
109109
feeRate = CFeeRate(1);
110-
BOOST_CHECK_EQUAL(feeRate.ToString(), "0.00000001 BTC/kB");
110+
BOOST_CHECK_EQUAL(feeRate.ToString(), "0.00000001 BTC/kvB");
111+
BOOST_CHECK_EQUAL(feeRate.ToString(FeeEstimateMode::BTC_KVB), "0.00000001 BTC/kvB");
112+
BOOST_CHECK_EQUAL(feeRate.ToString(FeeEstimateMode::SAT_VB), "0.001 sat/vB");
111113
}
112114

113115
BOOST_AUTO_TEST_SUITE_END()

‎src/util/fees.cpp

Lines changed: 5 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
}
@@ -51,6 +49,11 @@ std::string FeeModes(const std::string& delimiter)
5149
return Join(FeeModeMap(), delimiter, [&](const std::pair<std::string, FeeEstimateMode>& i) { return i.first; });
5250
}
5351

52+
const std::string InvalidEstimateModeErrorMessage()
53+
{
54+
return "Invalid estimate_mode parameter, must be one of: \"" + FeeModes("\", \"") + "\"";
55+
}
56+
5457
bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode)
5558
{
5659
auto searchkey = ToUpper(mode_string);

‎src/util/fees.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,6 @@ enum class FeeReason;
1313
bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode);
1414
std::string StringForFeeReason(FeeReason reason);
1515
std::string FeeModes(const std::string& delimiter);
16+
const std::string InvalidEstimateModeErrorMessage();
1617

1718
#endif // BITCOIN_UTIL_FEES_H

0 commit comments

Comments
 (0)