Skip to content

Commit

Permalink
Merge #20305: wallet: introduce fee_rate sat/vB param/option
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
MarcoFalke committed Nov 17, 2020
2 parents e7986c5 + 05e82d8 commit 80e32e1
Show file tree
Hide file tree
Showing 15 changed files with 447 additions and 426 deletions.
4 changes: 2 additions & 2 deletions src/policy/feerate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ CAmount CFeeRate::GetFee(size_t nBytes_) const
std::string CFeeRate::ToString(const FeeEstimateMode& fee_estimate_mode) const
{
switch (fee_estimate_mode) {
case FeeEstimateMode::SAT_B: return strprintf("%d.%03d %s/B", nSatoshisPerK / 1000, nSatoshisPerK % 1000, CURRENCY_ATOM);
default: return strprintf("%d.%08d %s/kB", nSatoshisPerK / COIN, nSatoshisPerK % COIN, CURRENCY_UNIT);
case FeeEstimateMode::SAT_VB: return strprintf("%d.%03d %s/vB", nSatoshisPerK / 1000, nSatoshisPerK % 1000, CURRENCY_ATOM);
default: return strprintf("%d.%08d %s/kvB", nSatoshisPerK / COIN, nSatoshisPerK % COIN, CURRENCY_UNIT);
}
}
17 changes: 13 additions & 4 deletions src/policy/feerate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ enum class FeeEstimateMode {
UNSET, //!< Use default settings based on other criteria
ECONOMICAL, //!< Force estimateSmartFee to use non-conservative estimates
CONSERVATIVE, //!< Force estimateSmartFee to use conservative estimates
BTC_KB, //!< Use explicit BTC/kB fee given in coin control
SAT_B, //!< Use explicit sat/B fee given in coin control
BTC_KVB, //!< Use BTC/kvB fee rate unit
SAT_VB, //!< Use sat/vB fee rate unit
};

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

SERIALIZE_METHODS(CFeeRate, obj) { READWRITE(obj.nSatoshisPerK); }
};
Expand Down
9 changes: 6 additions & 3 deletions src/rpc/client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "sendtoaddress", 5 , "replaceable" },
{ "sendtoaddress", 6 , "conf_target" },
{ "sendtoaddress", 8, "avoid_reuse" },
{ "sendtoaddress", 9, "verbose"},
{ "sendtoaddress", 9, "fee_rate"},
{ "sendtoaddress", 10, "verbose"},
{ "settxfee", 0, "amount" },
{ "sethdseed", 0, "newkeypool" },
{ "getreceivedbyaddress", 1, "minconf" },
Expand Down Expand Up @@ -73,7 +74,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "sendmany", 4, "subtractfeefrom" },
{ "sendmany", 5 , "replaceable" },
{ "sendmany", 6 , "conf_target" },
{ "sendmany", 8, "verbose" },
{ "sendmany", 8, "fee_rate"},
{ "sendmany", 9, "verbose" },
{ "deriveaddresses", 1, "range" },
{ "scantxoutset", 1, "scanobjects" },
{ "addmultisigaddress", 0, "nrequired" },
Expand Down Expand Up @@ -129,7 +131,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "lockunspent", 1, "transactions" },
{ "send", 0, "outputs" },
{ "send", 1, "conf_target" },
{ "send", 3, "options" },
{ "send", 3, "fee_rate"},
{ "send", 4, "options" },
{ "importprivkey", 2, "rescan" },
{ "importaddress", 2, "rescan" },
{ "importaddress", 3, "p2sh" },
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/mining.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,7 @@ static RPCHelpMan estimatesmartfee()
if (!request.params[1].isNull()) {
FeeEstimateMode fee_mode;
if (!FeeModeFromString(request.params[1].get_str(), fee_mode)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter");
throw JSONRPCError(RPC_INVALID_PARAMETER, InvalidEstimateModeErrorMessage());
}
if (fee_mode == FeeEstimateMode::ECONOMICAL) conservative = false;
}
Expand Down
6 changes: 4 additions & 2 deletions src/test/amount_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ BOOST_AUTO_TEST_CASE(BinaryOperatorTest)
BOOST_CHECK(a <= a);
BOOST_CHECK(b >= a);
BOOST_CHECK(b >= b);
// a should be 0.00000002 BTC/kB now
// a should be 0.00000002 BTC/kvB now
a += a;
BOOST_CHECK(a == b);
}
Expand All @@ -107,7 +107,9 @@ BOOST_AUTO_TEST_CASE(ToStringTest)
{
CFeeRate feeRate;
feeRate = CFeeRate(1);
BOOST_CHECK_EQUAL(feeRate.ToString(), "0.00000001 BTC/kB");
BOOST_CHECK_EQUAL(feeRate.ToString(), "0.00000001 BTC/kvB");
BOOST_CHECK_EQUAL(feeRate.ToString(FeeEstimateMode::BTC_KVB), "0.00000001 BTC/kvB");
BOOST_CHECK_EQUAL(feeRate.ToString(FeeEstimateMode::SAT_VB), "0.001 sat/vB");
}

BOOST_AUTO_TEST_SUITE_END()
7 changes: 5 additions & 2 deletions src/util/fees.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ const std::vector<std::pair<std::string, FeeEstimateMode>>& FeeModeMap()
{"unset", FeeEstimateMode::UNSET},
{"economical", FeeEstimateMode::ECONOMICAL},
{"conservative", FeeEstimateMode::CONSERVATIVE},
{(CURRENCY_UNIT + "/kB"), FeeEstimateMode::BTC_KB},
{(CURRENCY_ATOM + "/B"), FeeEstimateMode::SAT_B},
};
return FEE_MODES;
}
Expand All @@ -51,6 +49,11 @@ std::string FeeModes(const std::string& delimiter)
return Join(FeeModeMap(), delimiter, [&](const std::pair<std::string, FeeEstimateMode>& i) { return i.first; });
}

const std::string InvalidEstimateModeErrorMessage()
{
return "Invalid estimate_mode parameter, must be one of: \"" + FeeModes("\", \"") + "\"";
}

bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode)
{
auto searchkey = ToUpper(mode_string);
Expand Down
1 change: 1 addition & 0 deletions src/util/fees.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ enum class FeeReason;
bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode);
std::string StringForFeeReason(FeeReason reason);
std::string FeeModes(const std::string& delimiter);
const std::string InvalidEstimateModeErrorMessage();

#endif // BITCOIN_UTIL_FEES_H
Loading

0 comments on commit 80e32e1

Please sign in to comment.