Giving the user a hint about what action to take when they encounter an error related to a feerate that is below the minimum. I encountered this myself not knowing about -mintxfee and the manpage was slightly less than clear,
0 -mintxfee=<amt>
1
2 Fee rates (in BTC/kvB) smaller than this are considered zero fee for transaction creation
3 (default: 0.00001)
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | ismaelsadeeq |
| Concept ACK | rkrux, musaHaruna |
If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.
1434@@ -1435,7 +1435,18 @@ RPCHelpMan sendall()
1435 // Do not, ever, assume that it's fine to change the fee rate if the user has explicitly
1436 // provided one
1437 if (coin_control.m_feerate && fee_rate > *coin_control.m_feerate) {
1438- throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Fee rate (%s) is lower than the minimum fee rate setting (%s)", coin_control.m_feerate->ToString(FeeRateFormat::SAT_VB), fee_rate.ToString(FeeRateFormat::SAT_VB)));
1439+ const auto f = FeeRateFormat::SAT_VB;
f doesn’t help the reader much.
feerate_format
1443+ if (fee_calc_out.reason == FeeReason::REQUIRED) {
1444+ msg += strprintf("\nConsider modifying %s (%s) or %s (%s).",
1445+ "-mintxfee",
1446+ pwallet->m_min_fee.ToString(f),
1447+ "-minrelaytxfee",
1448+ pwallet->chain().relayMinFee().ToString(f));
I don’t see a strong reason to add placeholders for hardcoded values. The following diff for succinctness?
0--- a/src/wallet/rpc/spend.cpp
1+++ b/src/wallet/rpc/spend.cpp
2@@ -1440,10 +1440,8 @@ RPCHelpMan sendall()
3 coin_control.m_feerate->ToString(f),
4 fee_rate.ToString(f))};
5 if (fee_calc_out.reason == FeeReason::REQUIRED) {
6- msg += strprintf("\nConsider modifying %s (%s) or %s (%s).",
7- "-mintxfee",
8+ msg += strprintf("\nConsider modifying -mintxfee (%s) or -minrelaytxfee (%s).",
9 pwallet->m_min_fee.ToString(f),
10- "-minrelaytxfee",
11 pwallet->chain().relayMinFee().ToString(f));
12 }
This was copied and pasted from https://github.com/bitcoin/bitcoin/blob/b77555c8fba1e8c1cc25124e62302684fabdb32f/src/wallet/spend.cpp#L1160-L1166
where using variables is necessary to avoid parameter names from being translated, hopefully in the future this logic will be consolidated and live exclusively in src/wallet/spend.cpp and so I tried to keep the two as similar as possible, but I’ll include it in the string since it’s not that important.
Concept ACK on improving error message for low feerates
Sure its usage is limited and bounded but calling it just
fdoesn’t help the reader much.
Also agree with this comment. I think naming the variable format or fee_rate_format makes it more readable.
Pushed to address @rkrux’s feedback:
f -> feerate_format (https://github.com/bitcoin/bitcoin/pull/34640#discussion_r2852726961)
1160+ coin_selection_params.m_effective_feerate.ToString(feerate_format))};
1161+ if (feeCalc.reason == FeeReason::REQUIRED) {
1162+ msg += strprintf(_("\nConsider modifying %s (%s) or %s (%s)."),
1163+ "-mintxfee",
1164+ wallet.m_min_fee.ToString(feerate_format),
1165+ "-minrelaytxfee",
src/wallet/rpc/spend.cpp where you just include mintxfee and minrelaytxfee in the msg string, it is less verbose than this.
1442+ fee_rate.ToString(feerate_format))};
1443+ if (fee_calc_out.reason == FeeReason::REQUIRED) {
1444+ msg += strprintf("\nConsider modifying -mintxfee (%s) or -minrelaytxfee (%s).",
1445+ pwallet->m_min_fee.ToString(feerate_format),
1446+ pwallet->chain().relayMinFee().ToString(feerate_format));
1447+ }
-minrelaytxfee is potentially not a good idea for the user. They may modify and have the transaction created, but not propagated.
So maybe clarify that?
ACK 98fcd7af23f3628514a499936720fd0885886b30
Should also test this e,g here and other places?
0diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py
1index ab8f74f20e..1674341b99 100755
2--- a/test/functional/wallet_send.py
3+++ b/test/functional/wallet_send.py
4@@ -328,7 +328,7 @@ class WalletSendTest(BitcoinTestFramework):
5
6 # Test setting explicit fee rate just below the minimum of 1 sat/vB.
7 self.log.info("Explicit fee rate raises RPC error 'fee rate too low' if fee_rate of 0.99999999 is passed")
8- msg = "Fee rate (0.999 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)"
9+ msg = "Fee rate (0.999 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB).\nConsider modifying -mintxfee (1.000 sat/vB) or -minrelaytxfee (0.100 sat/vB)."
10 self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=0.999, expect_error=(-4, msg))
11 self.test_send(from_wallet=w0, to_wallet=w1, amount=1, arg_fee_rate=0.999, expect_error=(-4, msg))