Skip to content

Commit 61a843e

Browse files
fanquakefanquake
committed
Merge bitcoin/bitcoin#22220: util: make ParseMoney return a std::optional<CAmount>
f7752ad util: check MoneyRange() inside ParseMoney() (fanquake) 5ef2738 util: make ParseMoney return a std::optional<CAmount> (fanquake) Pull request description: Related discussion in #22193. ACKs for top commit: MarcoFalke: review ACK f7752ad 📄 Tree-SHA512: 88453f9e28f668deff4290d4bc0b2468cbd54699a3be1bfeac63a512276d309354672e7ea7deefa01466c3d9d826e837cc1ea244d4d74b4fa9c11c56f074e098
2 parents d3203a9 + f7752ad commit 61a843e

File tree

9 files changed

+126
-143
lines changed

9 files changed

+126
-143
lines changed

‎src/bitcoin-tx.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,10 +188,11 @@ static void RegisterLoad(const std::string& strInput)
188188

189189
static CAmount ExtractAndValidateValue(const std::string& strValue)
190190
{
191-
CAmount value;
192-
if (!ParseMoney(strValue, value))
191+
if (std::optional<CAmount> parsed = ParseMoney(strValue)) {
192+
return parsed.value();
193+
} else {
193194
throw std::runtime_error("invalid TX output value");
194-
return value;
195+
}
195196
}
196197

197198
static void MutateTxVersion(CMutableTransaction& tx, const std::string& cmdVal)

‎src/init.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -921,10 +921,11 @@ bool AppInitParameterInteraction(const ArgsManager& args)
921921
// incremental relay fee sets the minimum feerate increase necessary for BIP 125 replacement in the mempool
922922
// and the amount the mempool min fee increases above the feerate of txs evicted due to mempool limiting.
923923
if (args.IsArgSet("-incrementalrelayfee")) {
924-
CAmount n = 0;
925-
if (!ParseMoney(args.GetArg("-incrementalrelayfee", ""), n))
924+
if (std::optional<CAmount> inc_relay_fee = ParseMoney(args.GetArg("-incrementalrelayfee", ""))) {
925+
::incrementalRelayFee = CFeeRate{inc_relay_fee.value()};
926+
} else {
926927
return InitError(AmountErrMsg("incrementalrelayfee", args.GetArg("-incrementalrelayfee", "")));
927-
incrementalRelayFee = CFeeRate(n);
928+
}
928929
}
929930

930931
// block pruning; get the amount of disk space (in MiB) to allot for block & undo files
@@ -956,12 +957,12 @@ bool AppInitParameterInteraction(const ArgsManager& args)
956957
}
957958

958959
if (args.IsArgSet("-minrelaytxfee")) {
959-
CAmount n = 0;
960-
if (!ParseMoney(args.GetArg("-minrelaytxfee", ""), n)) {
960+
if (std::optional<CAmount> min_relay_fee = ParseMoney(args.GetArg("-minrelaytxfee", ""))) {
961+
// High fee check is done afterward in CWallet::Create()
962+
::minRelayTxFee = CFeeRate{min_relay_fee.value()};
963+
} else {
961964
return InitError(AmountErrMsg("minrelaytxfee", args.GetArg("-minrelaytxfee", "")));
962965
}
963-
// High fee check is done afterward in CWallet::Create()
964-
::minRelayTxFee = CFeeRate(n);
965966
} else if (incrementalRelayFee > ::minRelayTxFee) {
966967
// Allow only setting incrementalRelayFee to control both
967968
::minRelayTxFee = incrementalRelayFee;
@@ -971,18 +972,19 @@ bool AppInitParameterInteraction(const ArgsManager& args)
971972
// Sanity check argument for min fee for including tx in block
972973
// TODO: Harmonize which arguments need sanity checking and where that happens
973974
if (args.IsArgSet("-blockmintxfee")) {
974-
CAmount n = 0;
975-
if (!ParseMoney(args.GetArg("-blockmintxfee", ""), n))
975+
if (!ParseMoney(args.GetArg("-blockmintxfee", ""))) {
976976
return InitError(AmountErrMsg("blockmintxfee", args.GetArg("-blockmintxfee", "")));
977+
}
977978
}
978979

979980
// Feerate used to define dust. Shouldn't be changed lightly as old
980981
// implementations may inadvertently create non-standard transactions
981982
if (args.IsArgSet("-dustrelayfee")) {
982-
CAmount n = 0;
983-
if (!ParseMoney(args.GetArg("-dustrelayfee", ""), n))
983+
if (std::optional<CAmount> parsed = ParseMoney(args.GetArg("-dustrelayfee", ""))) {
984+
dustRelayFee = CFeeRate{parsed.value()};
985+
} else {
984986
return InitError(AmountErrMsg("dustrelayfee", args.GetArg("-dustrelayfee", "")));
985-
dustRelayFee = CFeeRate(n);
987+
}
986988
}
987989

988990
fRequireStandard = !args.GetBoolArg("-acceptnonstdtxn", !chainparams.RequireStandard());

‎src/miner.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ static BlockAssembler::Options DefaultOptions()
7373
// If -blockmaxweight is not given, limit to DEFAULT_BLOCK_MAX_WEIGHT
7474
BlockAssembler::Options options;
7575
options.nBlockMaxWeight = gArgs.GetArg("-blockmaxweight", DEFAULT_BLOCK_MAX_WEIGHT);
76-
CAmount n = 0;
77-
if (gArgs.IsArgSet("-blockmintxfee") && ParseMoney(gArgs.GetArg("-blockmintxfee", ""), n)) {
78-
options.blockMinFeeRate = CFeeRate(n);
76+
if (gArgs.IsArgSet("-blockmintxfee")) {
77+
std::optional<CAmount> parsed = ParseMoney(gArgs.GetArg("-blockmintxfee", ""));
78+
options.blockMinFeeRate = CFeeRate{parsed.value_or(DEFAULT_BLOCK_MIN_TX_FEE)};
7979
} else {
80-
options.blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE);
80+
options.blockMinFeeRate = CFeeRate{DEFAULT_BLOCK_MIN_TX_FEE};
8181
}
8282
return options;
8383
}

‎src/test/fuzz/integer.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,8 @@ FUZZ_TARGET_INIT(integer, initialize_integer)
8383
(void)FormatISO8601Date(i64);
8484
(void)FormatISO8601DateTime(i64);
8585
{
86-
int64_t parsed_money;
87-
if (ParseMoney(FormatMoney(i64), parsed_money)) {
88-
assert(parsed_money == i64);
86+
if (std::optional<CAmount> parsed = ParseMoney(FormatMoney(i64))) {
87+
assert(parsed.value() == i64);
8988
}
9089
}
9190
(void)GetSizeOfCompactSize(u64);
@@ -126,9 +125,8 @@ FUZZ_TARGET_INIT(integer, initialize_integer)
126125
(void)ToLower(ch);
127126
(void)ToUpper(ch);
128127
{
129-
int64_t parsed_money;
130-
if (ParseMoney(ValueFromAmount(i64).getValStr(), parsed_money)) {
131-
assert(parsed_money == i64);
128+
if (std::optional<CAmount> parsed = ParseMoney(ValueFromAmount(i64).getValStr())) {
129+
assert(parsed.value() == i64);
132130
}
133131
}
134132
if (i32 >= 0 && i32 <= 16) {

‎src/test/fuzz/parse_numbers.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ FUZZ_TARGET(parse_numbers)
1212
{
1313
const std::string random_string(buffer.begin(), buffer.end());
1414

15-
CAmount amount;
16-
(void)ParseMoney(random_string, amount);
15+
(void)ParseMoney(random_string);
1716

1817
double d;
1918
(void)ParseDouble(random_string, &d);

‎src/test/util_tests.cpp

Lines changed: 43 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,86 +1222,59 @@ BOOST_AUTO_TEST_CASE(util_FormatMoney)
12221222

12231223
BOOST_AUTO_TEST_CASE(util_ParseMoney)
12241224
{
1225-
CAmount ret = 0;
1226-
BOOST_CHECK(ParseMoney("0.0", ret));
1227-
BOOST_CHECK_EQUAL(ret, 0);
1228-
1229-
BOOST_CHECK(ParseMoney("12345.6789", ret));
1230-
BOOST_CHECK_EQUAL(ret, (COIN/10000)*123456789);
1231-
1232-
BOOST_CHECK(ParseMoney("100000000.00", ret));
1233-
BOOST_CHECK_EQUAL(ret, COIN*100000000);
1234-
BOOST_CHECK(ParseMoney("10000000.00", ret));
1235-
BOOST_CHECK_EQUAL(ret, COIN*10000000);
1236-
BOOST_CHECK(ParseMoney("1000000.00", ret));
1237-
BOOST_CHECK_EQUAL(ret, COIN*1000000);
1238-
BOOST_CHECK(ParseMoney("100000.00", ret));
1239-
BOOST_CHECK_EQUAL(ret, COIN*100000);
1240-
BOOST_CHECK(ParseMoney("10000.00", ret));
1241-
BOOST_CHECK_EQUAL(ret, COIN*10000);
1242-
BOOST_CHECK(ParseMoney("1000.00", ret));
1243-
BOOST_CHECK_EQUAL(ret, COIN*1000);
1244-
BOOST_CHECK(ParseMoney("100.00", ret));
1245-
BOOST_CHECK_EQUAL(ret, COIN*100);
1246-
BOOST_CHECK(ParseMoney("10.00", ret));
1247-
BOOST_CHECK_EQUAL(ret, COIN*10);
1248-
BOOST_CHECK(ParseMoney("1.00", ret));
1249-
BOOST_CHECK_EQUAL(ret, COIN);
1250-
BOOST_CHECK(ParseMoney("1", ret));
1251-
BOOST_CHECK_EQUAL(ret, COIN);
1252-
BOOST_CHECK(ParseMoney(" 1", ret));
1253-
BOOST_CHECK_EQUAL(ret, COIN);
1254-
BOOST_CHECK(ParseMoney("1 ", ret));
1255-
BOOST_CHECK_EQUAL(ret, COIN);
1256-
BOOST_CHECK(ParseMoney(" 1 ", ret));
1257-
BOOST_CHECK_EQUAL(ret, COIN);
1258-
BOOST_CHECK(ParseMoney("0.1", ret));
1259-
BOOST_CHECK_EQUAL(ret, COIN/10);
1260-
BOOST_CHECK(ParseMoney("0.01", ret));
1261-
BOOST_CHECK_EQUAL(ret, COIN/100);
1262-
BOOST_CHECK(ParseMoney("0.001", ret));
1263-
BOOST_CHECK_EQUAL(ret, COIN/1000);
1264-
BOOST_CHECK(ParseMoney("0.0001", ret));
1265-
BOOST_CHECK_EQUAL(ret, COIN/10000);
1266-
BOOST_CHECK(ParseMoney("0.00001", ret));
1267-
BOOST_CHECK_EQUAL(ret, COIN/100000);
1268-
BOOST_CHECK(ParseMoney("0.000001", ret));
1269-
BOOST_CHECK_EQUAL(ret, COIN/1000000);
1270-
BOOST_CHECK(ParseMoney("0.0000001", ret));
1271-
BOOST_CHECK_EQUAL(ret, COIN/10000000);
1272-
BOOST_CHECK(ParseMoney("0.00000001", ret));
1273-
BOOST_CHECK_EQUAL(ret, COIN/100000000);
1274-
BOOST_CHECK(ParseMoney(" 0.00000001 ", ret));
1275-
BOOST_CHECK_EQUAL(ret, COIN/100000000);
1276-
BOOST_CHECK(ParseMoney("0.00000001 ", ret));
1277-
BOOST_CHECK_EQUAL(ret, COIN/100000000);
1278-
BOOST_CHECK(ParseMoney(" 0.00000001", ret));
1279-
BOOST_CHECK_EQUAL(ret, COIN/100000000);
1280-
1281-
// Parsing amount that can not be represented in ret should fail
1282-
BOOST_CHECK(!ParseMoney("0.000000001", ret));
1225+
BOOST_CHECK_EQUAL(ParseMoney("0.0").value(), 0);
1226+
1227+
BOOST_CHECK_EQUAL(ParseMoney("12345.6789").value(), (COIN/10000)*123456789);
1228+
1229+
BOOST_CHECK_EQUAL(ParseMoney("10000000.00").value(), COIN*10000000);
1230+
BOOST_CHECK_EQUAL(ParseMoney("1000000.00").value(), COIN*1000000);
1231+
BOOST_CHECK_EQUAL(ParseMoney("100000.00").value(), COIN*100000);
1232+
BOOST_CHECK_EQUAL(ParseMoney("10000.00").value(), COIN*10000);
1233+
BOOST_CHECK_EQUAL(ParseMoney("1000.00").value(), COIN*1000);
1234+
BOOST_CHECK_EQUAL(ParseMoney("100.00").value(), COIN*100);
1235+
BOOST_CHECK_EQUAL(ParseMoney("10.00").value(), COIN*10);
1236+
BOOST_CHECK_EQUAL(ParseMoney("1.00").value(), COIN);
1237+
BOOST_CHECK_EQUAL(ParseMoney("1").value(), COIN);
1238+
BOOST_CHECK_EQUAL(ParseMoney(" 1").value(), COIN);
1239+
BOOST_CHECK_EQUAL(ParseMoney("1 ").value(), COIN);
1240+
BOOST_CHECK_EQUAL(ParseMoney(" 1 ").value(), COIN);
1241+
BOOST_CHECK_EQUAL(ParseMoney("0.1").value(), COIN/10);
1242+
BOOST_CHECK_EQUAL(ParseMoney("0.01").value(), COIN/100);
1243+
BOOST_CHECK_EQUAL(ParseMoney("0.001").value(), COIN/1000);
1244+
BOOST_CHECK_EQUAL(ParseMoney("0.0001").value(), COIN/10000);
1245+
BOOST_CHECK_EQUAL(ParseMoney("0.00001").value(), COIN/100000);
1246+
BOOST_CHECK_EQUAL(ParseMoney("0.000001").value(), COIN/1000000);
1247+
BOOST_CHECK_EQUAL(ParseMoney("0.0000001").value(), COIN/10000000);
1248+
BOOST_CHECK_EQUAL(ParseMoney("0.00000001").value(), COIN/100000000);
1249+
BOOST_CHECK_EQUAL(ParseMoney(" 0.00000001 ").value(), COIN/100000000);
1250+
BOOST_CHECK_EQUAL(ParseMoney("0.00000001 ").value(), COIN/100000000);
1251+
BOOST_CHECK_EQUAL(ParseMoney(" 0.00000001").value(), COIN/100000000);
1252+
1253+
// Parsing amount that can not be represented should fail
1254+
BOOST_CHECK(!ParseMoney("100000000.00"));
1255+
BOOST_CHECK(!ParseMoney("0.000000001"));
12831256

12841257
// Parsing empty string should fail
1285-
BOOST_CHECK(!ParseMoney("", ret));
1286-
BOOST_CHECK(!ParseMoney(" ", ret));
1287-
BOOST_CHECK(!ParseMoney(" ", ret));
1258+
BOOST_CHECK(!ParseMoney(""));
1259+
BOOST_CHECK(!ParseMoney(" "));
1260+
BOOST_CHECK(!ParseMoney(" "));
12881261

12891262
// Parsing two numbers should fail
1290-
BOOST_CHECK(!ParseMoney("1 2", ret));
1291-
BOOST_CHECK(!ParseMoney(" 1 2 ", ret));
1292-
BOOST_CHECK(!ParseMoney(" 1.2 3 ", ret));
1293-
BOOST_CHECK(!ParseMoney(" 1 2.3 ", ret));
1263+
BOOST_CHECK(!ParseMoney("1 2"));
1264+
BOOST_CHECK(!ParseMoney(" 1 2 "));
1265+
BOOST_CHECK(!ParseMoney(" 1.2 3 "));
1266+
BOOST_CHECK(!ParseMoney(" 1 2.3 "));
12941267

12951268
// Attempted 63 bit overflow should fail
1296-
BOOST_CHECK(!ParseMoney("92233720368.54775808", ret));
1269+
BOOST_CHECK(!ParseMoney("92233720368.54775808"));
12971270

12981271
// Parsing negative amounts must fail
1299-
BOOST_CHECK(!ParseMoney("-1", ret));
1272+
BOOST_CHECK(!ParseMoney("-1"));
13001273

13011274
// Parsing strings with embedded NUL characters should fail
1302-
BOOST_CHECK(!ParseMoney("\0-1"s, ret));
1303-
BOOST_CHECK(!ParseMoney(STRING_WITH_EMBEDDED_NULL_CHAR, ret));
1304-
BOOST_CHECK(!ParseMoney("1\0"s, ret));
1275+
BOOST_CHECK(!ParseMoney("\0-1"s));
1276+
BOOST_CHECK(!ParseMoney(STRING_WITH_EMBEDDED_NULL_CHAR));
1277+
BOOST_CHECK(!ParseMoney("1\0"s));
13051278
}
13061279

13071280
BOOST_AUTO_TEST_CASE(util_IsHex)

‎src/util/moneystr.cpp

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@
55

66
#include <util/moneystr.h>
77

8+
#include <amount.h>
89
#include <tinyformat.h>
910
#include <util/strencodings.h>
1011
#include <util/string.h>
1112

13+
#include <optional>
14+
1215
std::string FormatMoney(const CAmount n)
1316
{
1417
// Note: not using straight sprintf here because we do NOT want
@@ -35,14 +38,14 @@ std::string FormatMoney(const CAmount n)
3538
}
3639

3740

38-
bool ParseMoney(const std::string& money_string, CAmount& nRet)
41+
std::optional<CAmount> ParseMoney(const std::string& money_string)
3942
{
4043
if (!ValidAsCString(money_string)) {
41-
return false;
44+
return std::nullopt;
4245
}
4346
const std::string str = TrimString(money_string);
4447
if (str.empty()) {
45-
return false;
48+
return std::nullopt;
4649
}
4750

4851
std::string strWhole;
@@ -62,21 +65,25 @@ bool ParseMoney(const std::string& money_string, CAmount& nRet)
6265
break;
6366
}
6467
if (IsSpace(*p))
65-
return false;
68+
return std::nullopt;
6669
if (!IsDigit(*p))
67-
return false;
70+
return std::nullopt;
6871
strWhole.insert(strWhole.end(), *p);
6972
}
7073
if (*p) {
71-
return false;
74+
return std::nullopt;
7275
}
7376
if (strWhole.size() > 10) // guard against 63 bit overflow
74-
return false;
77+
return std::nullopt;
7578
if (nUnits < 0 || nUnits > COIN)
76-
return false;
79+
return std::nullopt;
7780
int64_t nWhole = atoi64(strWhole);
78-
CAmount nValue = nWhole*COIN + nUnits;
7981

80-
nRet = nValue;
81-
return true;
82+
CAmount value = nWhole * COIN + nUnits;
83+
84+
if (!MoneyRange(value)) {
85+
return std::nullopt;
86+
}
87+
88+
return value;
8289
}

‎src/util/moneystr.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,14 @@
1212
#include <amount.h>
1313
#include <attributes.h>
1414

15+
#include <optional>
1516
#include <string>
1617

1718
/* Do not use these functions to represent or parse monetary amounts to or from
1819
* JSON but use AmountFromValue and ValueFromAmount for that.
1920
*/
2021
std::string FormatMoney(const CAmount n);
2122
/** Parse an amount denoted in full coins. E.g. "0.0034" supplied on the command line. **/
22-
[[nodiscard]] bool ParseMoney(const std::string& str, CAmount& nRet);
23+
std::optional<CAmount> ParseMoney(const std::string& str);
2324

2425
#endif // BITCOIN_UTIL_MONEYSTR_H

0 commit comments

Comments
 (0)