wallet: update fee rate units, use sat/vB for fee_rate error messages

jonatack · jonatack · commit 173b5b5fe07d · 2020-11-12T11:43:03.000+01:00
and BTC/kvB for feeRate error messages.
diff --git a/src/policy/feerate.cpp b/src/policy/feerate.cpp
@@ -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);
     }
 }
diff --git a/src/policy/feerate.h b/src/policy/feerate.h
@@ -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
 };
 
 /**
@@ -65,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); }
 };
diff --git a/src/test/amount_tests.cpp b/src/test/amount_tests.cpp
@@ -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);
 }
@@ -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()
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -3136,7 +3136,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f
             }
             CFeeRate fee_rate(AmountFromValue(options["feeRate"]));
             if (fee_rate <= CFeeRate(0)) {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid feeRate %s (must be greater than 0)", fee_rate.ToString()));
+                throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid feeRate %s (must be greater than 0)", fee_rate.ToString(FeeEstimateMode::BTC_KVB)));
             }
             coinControl.m_feerate = fee_rate;
             coinControl.fOverrideFeeRate = true;
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -2834,7 +2834,7 @@ bool CWallet::CreateTransactionInternal(
             // Do not, ever, assume that it's fine to change the fee rate if the user has explicitly
             // provided one
             if (coin_control.m_feerate && nFeeRateNeeded > *coin_control.m_feerate) {
-                error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(), nFeeRateNeeded.ToString());
+                error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), nFeeRateNeeded.ToString(FeeEstimateMode::SAT_VB));
                 return false;
             }
 
diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py
@@ -752,9 +752,9 @@ def test_option_feerate(self):
                     node.fundrawtransaction, rawtx, {"estimate_mode": mode, "conf_target": n, "add_inputs": True})
 
         self.log.info("Test invalid fee rate settings")
-        assert_raises_rpc_error(-4, "Fee rate (0.00000000 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)",
+        assert_raises_rpc_error(-4, "Fee rate (0.000 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)",
             node.fundrawtransaction, rawtx, {"fee_rate": 0, "add_inputs": True})
-        assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 BTC/kB (must be greater than 0)",
+        assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 BTC/kvB (must be greater than 0)",
             node.fundrawtransaction, rawtx, {"feeRate": 0, "add_inputs": True})
         for param, value in {("fee_rate", 100000), ("feeRate", 1.000)}:
             assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)",
@@ -768,7 +768,7 @@ def test_option_feerate(self):
 
         # Test setting explicit fee rate just below the minimum.
         self.log.info("- raises RPC error 'fee rate too low' if fee_rate of 0.99999999 sat/vB is passed")
-        msg = "Fee rate (0.00000999 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)"
+        msg = "Fee rate (0.999 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)"
         assert_raises_rpc_error(-4, msg, node.fundrawtransaction, rawtx, {"fee_rate": 0.99999999, "add_inputs": True})
         # This feeRate test only passes if `coinControl.fOverrideFeeRate = true` in wallet/rpcwallet.cpp::FundTransaction is removed.
         # assert_raises_rpc_error(-4, msg, node.fundrawtransaction, rawtx, {"feeRate": 0.00000999, "add_inputs": True})
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
@@ -194,9 +194,9 @@ def run_test(self):
         assert_approx(res2["fee"], 0.055, 0.005)
 
         self.log.info("Test invalid fee rate settings")
-        assert_raises_rpc_error(-4, "Fee rate (0.00000000 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)",
+        assert_raises_rpc_error(-4, "Fee rate (0.000 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)",
             self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": 0, "add_inputs": True})
-        assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 BTC/kB (must be greater than 0)",
+        assert_raises_rpc_error(-8, "Invalid feeRate 0.00000000 BTC/kvB (must be greater than 0)",
             self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"feeRate": 0, "add_inputs": True})
         for param, value in {("fee_rate", 100000), ("feeRate", 1)}:
             assert_raises_rpc_error(-4, "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)",
@@ -246,7 +246,7 @@ def run_test(self):
 
         # Test setting explicit fee rate just below the minimum.
         self.log.info("- raises RPC error 'fee rate too low' if feerate_sat_vb of 0.99999999 is passed")
-        assert_raises_rpc_error(-4, "Fee rate (0.00000999 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)",
+        assert_raises_rpc_error(-4, "Fee rate (0.999 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)",
             self.nodes[1].walletcreatefundedpsbt, inputs, outputs, 0, {"fee_rate": 0.99999999, "add_inputs": True})
 
         self.log.info("Test walletcreatefundedpsbt with too-high fee rate produces total fee well above -maxtxfee and raises RPC error")
diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py
@@ -250,11 +250,11 @@ def run_test(self):
 
         # Test setting explicit fee rate just below the minimum.
         self.log.info("Test sendmany raises 'fee rate too low' if fee_rate of 0.99999999 is passed")
-        assert_raises_rpc_error(-6, "Fee rate (0.00000999 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)",
+        assert_raises_rpc_error(-6, "Fee rate (0.999 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)",
             self.nodes[2].sendmany, amounts={address: 10}, fee_rate=0.99999999)
 
         self.log.info("Test sendmany raises if fee_rate of 0 or -1 is passed")
-        assert_raises_rpc_error(-6, "Fee rate (0.00000000 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)",
+        assert_raises_rpc_error(-6, "Fee rate (0.000 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)",
             self.nodes[2].sendmany, amounts={address: 10}, fee_rate=0)
         assert_raises_rpc_error(-3, OUT_OF_RANGE, self.nodes[2].sendmany, amounts={address: 10}, fee_rate=-1)
 
@@ -421,11 +421,11 @@ def run_test(self):
 
             # Test setting explicit fee rate just below the minimum.
             self.log.info("Test sendtoaddress raises 'fee rate too low' if fee_rate of 0.99999999 is passed")
-            assert_raises_rpc_error(-6, "Fee rate (0.00000999 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)",
+            assert_raises_rpc_error(-6, "Fee rate (0.999 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)",
                 self.nodes[2].sendtoaddress, address=address, amount=1, fee_rate=0.99999999)
 
             self.log.info("Test sendtoaddress raises if fee_rate of 0 or -1 is passed")
-            assert_raises_rpc_error(-6, "Fee rate (0.00000000 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)",
+            assert_raises_rpc_error(-6, "Fee rate (0.000 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)",
                 self.nodes[2].sendtoaddress, address=address, amount=10, fee_rate=0)
             assert_raises_rpc_error(-3, OUT_OF_RANGE, self.nodes[2].sendtoaddress, address=address, amount=1.0, fee_rate=-1)
 
diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py
@@ -306,11 +306,10 @@ def run_test(self):
                 self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=v, estimate_mode=mode,
                     expect_error=(-3, "Expected type number for conf_target, got {}".format(k)))
 
-        # TODO: The error message should use sat/vB units instead of BTC/kB.
         # Test setting explicit fee rate just below the minimum.
         self.log.info("Explicit fee rate raises RPC error 'fee rate too low' if fee_rate of 0.99999999 is passed")
         self.test_send(from_wallet=w0, to_wallet=w1, amount=1, fee_rate=0.99999999,
-            expect_error=(-4, "Fee rate (0.00000999 BTC/kB) is lower than the minimum fee rate setting (0.00001000 BTC/kB)"))
+            expect_error=(-4, "Fee rate (0.999 sat/vB) is lower than the minimum fee rate setting (1.000 sat/vB)"))
 
         # TODO: Return hex if fee rate is below -maxmempool
         # res = self.test_send(from_wallet=w0, to_wallet=w1, amount=1, conf_target=0.1, estimate_mode="sat/b", add_to_wallet=False)

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ CAmount CFeeRate::GetFee(size_t nBytes_) const`
`38`	`38`	`std::string CFeeRate::ToString(const FeeEstimateMode& fee_estimate_mode) const`
`39`	`39`	`{`
`40`	`40`	`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);`
`43`	`43`	`}`
`44`	`44`	`}`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ BOOST_AUTO_TEST_CASE(BinaryOperatorTest)`
`98`	`98`	`BOOST_CHECK(a <= a);`
`99`	`99`	`BOOST_CHECK(b >= a);`
`100`	`100`	`BOOST_CHECK(b >= b);`
`101`		`- // a should be 0.00000002 BTC/kB now`
	`101`	`+ // a should be 0.00000002 BTC/kvB now`
`102`	`102`	`a += a;`
`103`	`103`	`BOOST_CHECK(a == b);`
`104`	`104`	`}`
`@@ -107,7 +107,9 @@ BOOST_AUTO_TEST_CASE(ToStringTest)`
`107`	`107`	`{`
`108`	`108`	`CFeeRate feeRate;`
`109`	`109`	`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");`
`111`	`113`	`}`
`112`	`114`
`113`	`115`	`BOOST_AUTO_TEST_SUITE_END()`
Original file line number	Diff line number	Diff line change
`@@ -3136,7 +3136,7 @@ void FundTransaction(CWallet* const pwallet, CMutableTransaction& tx, CAmount& f`
`3136`	`3136`	`}`
`3137`	`3137`	`CFeeRate fee_rate(AmountFromValue(options["feeRate"]));`
`3138`	`3138`	`if (fee_rate <= CFeeRate(0)) {`
`3139`		`- throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid feeRate %s (must be greater than 0)", fee_rate.ToString()));`
	`3139`	`+ throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid feeRate %s (must be greater than 0)", fee_rate.ToString(FeeEstimateMode::BTC_KVB)));`
`3140`	`3140`	`}`
`3141`	`3141`	`coinControl.m_feerate = fee_rate;`
`3142`	`3142`	`coinControl.fOverrideFeeRate = true;`
Original file line number	Diff line number	Diff line change
`@@ -2834,7 +2834,7 @@ bool CWallet::CreateTransactionInternal(`
`2834`	`2834`	`// Do not, ever, assume that it's fine to change the fee rate if the user has explicitly`
`2835`	`2835`	`// provided one`
`2836`	`2836`	`if (coin_control.m_feerate && nFeeRateNeeded > *coin_control.m_feerate) {`
`2837`		`- error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(), nFeeRateNeeded.ToString());`
	`2837`	`+ error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), nFeeRateNeeded.ToString(FeeEstimateMode::SAT_VB));`
`2838`	`2838`	`return false;`
`2839`	`2839`	`}`
`2840`	`2840`