From fa23ed7fc24212d85cdc7f52b317906b37a1a120 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 28 Apr 2025 15:47:16 +0200 Subject: [PATCH 01/17] refactor: Use ToIntegral in ParseHDKeypath ToIntegral only accepts numbers, so just use that to replace the equivalent but more verbose way with find_first_not_of+ParseUInt32. --- src/util/bip32.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/util/bip32.cpp b/src/util/bip32.cpp index 4ab14bd7041..db40bfb5b9e 100644 --- a/src/util/bip32.cpp +++ b/src/util/bip32.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2022 The Bitcoin Core developers +// Copyright (c) 2019-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -36,14 +36,11 @@ bool ParseHDKeypath(const std::string& keypath_str, std::vector& keypa } // Ensure this is only numbers - if (item.find_first_not_of( "0123456789" ) != std::string::npos) { + const auto number{ToIntegral(item)}; + if (!number) { return false; } - uint32_t number; - if (!ParseUInt32(item, &number)) { - return false; - } - path |= number; + path |= *number; keypath.push_back(path); first = false; From fa980413257e2004a8d48a8be66c6d67f90b76ad Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 28 Apr 2025 15:49:42 +0200 Subject: [PATCH 02/17] refactor: Use ToIntegral in CreateFromDump The unsigned version is never serialized with a `+` prefix, so the stricter ToIntegral can be used. --- src/wallet/dump.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index ceed5b6b52b..fc904de973b 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -151,13 +151,13 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: return false; } // Check the version number (value of first record) - uint32_t ver; - if (!ParseUInt32(version_value, &ver)) { - error =strprintf(_("Error: Unable to parse version %u as a uint32_t"), version_value); + const auto ver{ToIntegral(version_value)}; + if (!ver) { + error = strprintf(_("Error: Unable to parse version %u as a uint32_t"), version_value); dump_file.close(); return false; } - if (ver != DUMP_VERSION) { + if (*ver != DUMP_VERSION) { error = strprintf(_("Error: Dumpfile version is not supported. This version of bitcoin-wallet only supports version 1 dumpfiles. Got dumpfile with version %s"), version_value); dump_file.close(); return false; From fa9c45577dfbae67535e4965b5660288557d3631 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 19:52:38 +0200 Subject: [PATCH 03/17] cli: Reject + sign in -netinfo level parsing It would be confusing to specify the sign for an unsigned value here, so reject it. --- src/bitcoin-cli.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 03104b81361..14a0cb245fa 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2022 The Bitcoin Core developers +// Copyright (c) 2009-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -472,7 +472,8 @@ public: { if (!args.empty()) { uint8_t n{0}; - if (ParseUInt8(args.at(0), &n)) { + if (const auto res{ToIntegral(args.at(0))}) { + n = *res; m_details_level = std::min(n, NETINFO_MAX_LEVEL); } else { throw std::runtime_error(strprintf("invalid -netinfo level argument: %s\nFor more information, run: bitcoin-cli -netinfo help", args.at(0))); From fa89652e68fc07fb6c9f3d9e34dc11d35f0cc1e1 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 20:06:32 +0200 Subject: [PATCH 04/17] init: Reject + sign in -*port parsing It would be confusing to specify the sign for an unsigned value here, so reject it. --- src/init.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index dfc0724adf1..388a21bc88b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1179,8 +1179,8 @@ bool CheckHostPortOptions(const ArgsManager& args) { "-rpcport", }) { if (const auto port{args.GetArg(port_option)}) { - uint16_t n; - if (!ParseUInt16(*port, &n) || n == 0) { + const auto n{ToIntegral(*port)}; + if (!n || *n == 0) { return InitError(InvalidPortErrMsg(port_option, *port)); } } From fab4c2967d554ddbc15f732cea6cd190c547d89f Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 20:42:21 +0200 Subject: [PATCH 05/17] net: Reject + sign when parsing subnet mask It does not make sense and it is rejected by other parsers as well: >>> ipaddress.ip_network("1.2.3.0/+24") ValueError: '1.2.3.0/+24' does not appear to be an IPv4 or IPv6 network --- src/netbase.cpp | 5 ++--- src/test/netbase_tests.cpp | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/netbase.cpp b/src/netbase.cpp index a5da1c9dcef..1f3faa18643 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -829,10 +829,9 @@ CSubNet LookupSubNet(const std::string& subnet_str) addr = static_cast(MaybeFlipIPv6toCJDNS(CService{addr.value(), /*port=*/0})); if (slash_pos != subnet_str.npos) { const std::string netmask_str{subnet_str.substr(slash_pos + 1)}; - uint8_t netmask; - if (ParseUInt8(netmask_str, &netmask)) { + if (const auto netmask{ToIntegral(netmask_str)}) { // Valid number; assume CIDR variable-length subnet masking. - subnet = CSubNet{addr.value(), netmask}; + subnet = CSubNet{addr.value(), *netmask}; } else { // Invalid number; try full netmask syntax. Never allow lookup for netmask. const std::optional full_netmask{LookupHost(netmask_str, /*fAllowLookup=*/false)}; diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index 3422cb80233..7da87bd17fa 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -150,7 +150,6 @@ BOOST_AUTO_TEST_CASE(embedded_test) BOOST_AUTO_TEST_CASE(subnet_test) { - BOOST_CHECK(LookupSubNet("1.2.3.0/24") == LookupSubNet("1.2.3.0/255.255.255.0")); BOOST_CHECK(LookupSubNet("1.2.3.0/24") != LookupSubNet("1.2.4.0/255.255.255.0")); BOOST_CHECK(LookupSubNet("1.2.3.0/24").Match(ResolveIP("1.2.3.4"))); @@ -185,6 +184,7 @@ BOOST_AUTO_TEST_CASE(subnet_test) // Check valid/invalid BOOST_CHECK(LookupSubNet("1.2.3.0/0").IsValid()); BOOST_CHECK(!LookupSubNet("1.2.3.0/-1").IsValid()); + BOOST_CHECK(!LookupSubNet("1.2.3.0/+24").IsValid()); BOOST_CHECK(LookupSubNet("1.2.3.0/32").IsValid()); BOOST_CHECK(!LookupSubNet("1.2.3.0/33").IsValid()); BOOST_CHECK(!LookupSubNet("1.2.3.0/300").IsValid()); From fa479857ed234d54df31d33b60de14c6ffab3d6f Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 21:06:48 +0200 Subject: [PATCH 06/17] Reject + sign in SplitHostPort It is better to reject it with an error. For example, $ bitcoin-cli -rpcconnect=127.0.0.1:+23501 -getinfo error: Invalid port provided in -rpcconnect: 127.0.0.1:+23501 --- src/util/strencodings.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index fbccebad942..0c09915c8b4 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -78,10 +78,9 @@ bool SplitHostPort(std::string_view in, uint16_t& portOut, std::string& hostOut) bool fBracketed = fHaveColon && (in[0] == '[' && in[colon - 1] == ']'); // if there is a colon, and in[0]=='[', colon is not 0, so in[colon-1] is safe bool fMultiColon{fHaveColon && colon != 0 && (in.find_last_of(':', colon - 1) != in.npos)}; if (fHaveColon && (colon == 0 || fBracketed || !fMultiColon)) { - uint16_t n; - if (ParseUInt16(in.substr(colon + 1), &n)) { + if (const auto n{ToIntegral(in.substr(colon + 1))}) { in = in.substr(0, colon); - portOut = n; + portOut = *n; valid = (portOut != 0); } } else { From fa123afa0ef752e8645bf695d121da66d8f1382b Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 21:10:31 +0200 Subject: [PATCH 07/17] Reject + sign when checking -ipcfd --- src/ipc/process.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ipc/process.cpp b/src/ipc/process.cpp index bdc541b6544..33667883b76 100644 --- a/src/ipc/process.cpp +++ b/src/ipc/process.cpp @@ -55,9 +55,11 @@ public: // in combination with other arguments because the parent process // should be able to control the child process through the IPC protocol // without passing information out of band. - if (!ParseInt32(argv[2], &fd)) { + const auto maybe_fd{ToIntegral(argv[2])}; + if (!maybe_fd) { throw std::runtime_error(strprintf("Invalid -ipcfd number '%s'", argv[2])); } + fd = *maybe_fd; return true; } int connect(const fs::path& data_dir, From fafd43c69192fcb48a9e04d52eeb07fff15655d0 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 21:25:10 +0200 Subject: [PATCH 08/17] test: Reject + sign when parsing regtest deployment params The + sign does not make sense for times or heights. --- src/chainparams.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 7cf28d69b42..6be2a63f4e5 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -53,14 +53,14 @@ void ReadRegTestArgs(const ArgsManager& args, CChainParams::RegTestOptions& opti } const auto value{arg.substr(found + 1)}; - int32_t height; - if (!ParseInt32(value, &height) || height < 0 || height >= std::numeric_limits::max()) { + const auto height{ToIntegral(value)}; + if (!height || *height < 0 || *height >= std::numeric_limits::max()) { throw std::runtime_error(strprintf("Invalid height value (%s) for -testactivationheight=name@height.", arg)); } const auto deployment_name{arg.substr(0, found)}; if (const auto buried_deployment = GetBuriedDeployment(deployment_name)) { - options.activation_heights[*buried_deployment] = height; + options.activation_heights[*buried_deployment] = *height; } else { throw std::runtime_error(strprintf("Invalid name (%s) for -testactivationheight=name@height.", arg)); } @@ -72,16 +72,22 @@ void ReadRegTestArgs(const ArgsManager& args, CChainParams::RegTestOptions& opti throw std::runtime_error("Version bits parameters malformed, expecting deployment:start:end[:min_activation_height]"); } CChainParams::VersionBitsParameters vbparams{}; - if (!ParseInt64(vDeploymentParams[1], &vbparams.start_time)) { + const auto start_time{ToIntegral(vDeploymentParams[1])}; + if (!start_time) { throw std::runtime_error(strprintf("Invalid nStartTime (%s)", vDeploymentParams[1])); } - if (!ParseInt64(vDeploymentParams[2], &vbparams.timeout)) { + vbparams.start_time = *start_time; + const auto timeout{ToIntegral(vDeploymentParams[2])}; + if (!timeout) { throw std::runtime_error(strprintf("Invalid nTimeout (%s)", vDeploymentParams[2])); } + vbparams.timeout = *timeout; if (vDeploymentParams.size() >= 4) { - if (!ParseInt32(vDeploymentParams[3], &vbparams.min_activation_height)) { + const auto min_activation_height{ToIntegral(vDeploymentParams[3])}; + if (!min_activation_height) { throw std::runtime_error(strprintf("Invalid min_activation_height (%s)", vDeploymentParams[3])); } + vbparams.min_activation_height = *min_activation_height; } else { vbparams.min_activation_height = 0; } From 8888bb499dec79258b1857b404d72f93650503f4 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 21:36:36 +0200 Subject: [PATCH 09/17] rest: Reject + sign in /blockhashbyheight/ --- src/rest.cpp | 8 ++++---- test/functional/interface_rest.py | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/rest.cpp b/src/rest.cpp index 44984b360ff..b340567bd1b 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -962,8 +962,8 @@ static bool rest_blockhash_by_height(const std::any& context, HTTPRequest* req, std::string height_str; const RESTResponseFormat rf = ParseDataFormat(height_str, str_uri_part); - int32_t blockheight = -1; // Initialization done only to prevent valgrind false positive, see https://github.com/bitcoin/bitcoin/pull/18785 - if (!ParseInt32(height_str, &blockheight) || blockheight < 0) { + const auto blockheight{ToIntegral(height_str)}; + if (!blockheight || *blockheight < 0) { return RESTERR(req, HTTP_BAD_REQUEST, "Invalid height: " + SanitizeString(height_str)); } @@ -974,10 +974,10 @@ static bool rest_blockhash_by_height(const std::any& context, HTTPRequest* req, ChainstateManager& chainman = *maybe_chainman; LOCK(cs_main); const CChain& active_chain = chainman.ActiveChain(); - if (blockheight > active_chain.Height()) { + if (*blockheight > active_chain.Height()) { return RESTERR(req, HTTP_NOT_FOUND, "Block height out of range"); } - pblockindex = active_chain[blockheight]; + pblockindex = active_chain[*blockheight]; } switch (rf) { case RESTResponseFormat::BINARY: { diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 0e294696b0e..ff766978840 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -271,6 +271,8 @@ class RESTTest (BitcoinTestFramework): # Check invalid blockhashbyheight requests resp = self.test_rest_request(f"/blockhashbyheight/{INVALID_PARAM}", ret_type=RetType.OBJ, status=400) assert_equal(resp.read().decode('utf-8').rstrip(), f"Invalid height: {INVALID_PARAM}") + resp = self.test_rest_request("/blockhashbyheight/+1", ret_type=RetType.OBJ, status=400) + assert_equal(resp.read().decode('utf-8').rstrip(), "Invalid height: 1") resp = self.test_rest_request("/blockhashbyheight/1000000", ret_type=RetType.OBJ, status=404) assert_equal(resp.read().decode('utf-8').rstrip(), "Block height out of range") resp = self.test_rest_request("/blockhashbyheight/-1", ret_type=RetType.OBJ, status=400) From fab06ac03788243847b799a3feaac56bc918fba9 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Sat, 17 May 2025 09:06:42 +0200 Subject: [PATCH 10/17] rest: Use SAFE_CHARS_URI in SanitizeString error msg When dealing with URI parts, it seems more consistent to use corresponding SAFE_CHARS_URI mode in error messages. Co-Authored-By: stickies-v --- src/rest.cpp | 2 +- test/functional/interface_rest.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rest.cpp b/src/rest.cpp index b340567bd1b..464880224c8 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -964,7 +964,7 @@ static bool rest_blockhash_by_height(const std::any& context, HTTPRequest* req, const auto blockheight{ToIntegral(height_str)}; if (!blockheight || *blockheight < 0) { - return RESTERR(req, HTTP_BAD_REQUEST, "Invalid height: " + SanitizeString(height_str)); + return RESTERR(req, HTTP_BAD_REQUEST, "Invalid height: " + SanitizeString(height_str, SAFE_CHARS_URI)); } CBlockIndex* pblockindex = nullptr; diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index ff766978840..a853b2ec0ec 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -272,7 +272,7 @@ class RESTTest (BitcoinTestFramework): resp = self.test_rest_request(f"/blockhashbyheight/{INVALID_PARAM}", ret_type=RetType.OBJ, status=400) assert_equal(resp.read().decode('utf-8').rstrip(), f"Invalid height: {INVALID_PARAM}") resp = self.test_rest_request("/blockhashbyheight/+1", ret_type=RetType.OBJ, status=400) - assert_equal(resp.read().decode('utf-8').rstrip(), "Invalid height: 1") + assert_equal(resp.read().decode('utf-8').rstrip(), "Invalid height: +1") resp = self.test_rest_request("/blockhashbyheight/1000000", ret_type=RetType.OBJ, status=404) assert_equal(resp.read().decode('utf-8').rstrip(), "Block height out of range") resp = self.test_rest_request("/blockhashbyheight/-1", ret_type=RetType.OBJ, status=400) From dddd9e5fe38b81f1af6b343661b65e16b0de7c60 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 20:03:25 +0200 Subject: [PATCH 11/17] bitcoin-tx: Reject + sign in nversion parsing It would be confusing to specify the sign for an unsigned value here, so reject it. --- src/bitcoin-tx.cpp | 7 +++---- test/util/data/bitcoin-util-test.json | 6 ++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index e2b3a3b5545..c319c374913 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -211,12 +211,11 @@ static CAmount ExtractAndValidateValue(const std::string& strValue) static void MutateTxVersion(CMutableTransaction& tx, const std::string& cmdVal) { - uint32_t newVersion; - if (!ParseUInt32(cmdVal, &newVersion) || newVersion < 1 || newVersion > TX_MAX_STANDARD_VERSION) { + const auto ver{ToIntegral(cmdVal)}; + if (!ver || *ver < 1 || *ver > TX_MAX_STANDARD_VERSION) { throw std::runtime_error("Invalid TX version requested: '" + cmdVal + "'"); } - - tx.version = newVersion; + tx.version = *ver; } static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal) diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json index 83b3c430d53..16e51124e2c 100644 --- a/test/util/data/bitcoin-util-test.json +++ b/test/util/data/bitcoin-util-test.json @@ -56,6 +56,12 @@ "output_cmp": "blanktxv2.json", "description": "Creates a blank transaction when nothing is piped into bitcoin-tx (output in json)" }, + { "exec": "./bitcoin-tx", + "args": ["-create", "nversion=+1"], + "return_code": 1, + "error_txt": "error: Invalid TX version requested", + "description": "Tests the check for invalid nversion value" + }, { "exec": "./bitcoin-tx", "args": ["-create", "nversion=1foo"], "return_code": 1, From faff25a558ab15b5d8eeea5dd4c9c0d76350051b Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 21:48:23 +0200 Subject: [PATCH 12/17] bitcoin-tx: Reject + sign in locktime --- src/bitcoin-tx.cpp | 8 ++++---- test/util/data/bitcoin-util-test.json | 6 ++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index c319c374913..84bfe6995ff 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -220,11 +220,11 @@ static void MutateTxVersion(CMutableTransaction& tx, const std::string& cmdVal) static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal) { - int64_t newLocktime; - if (!ParseInt64(cmdVal, &newLocktime) || newLocktime < 0LL || newLocktime > 0xffffffffLL) + const auto locktime{ToIntegral(cmdVal)}; + if (!locktime) { throw std::runtime_error("Invalid TX locktime requested: '" + cmdVal + "'"); - - tx.nLockTime = (unsigned int) newLocktime; + } + tx.nLockTime = *locktime; } static void MutateTxRBFOptIn(CMutableTransaction& tx, const std::string& strInIdx) diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json index 16e51124e2c..d599731ca6e 100644 --- a/test/util/data/bitcoin-util-test.json +++ b/test/util/data/bitcoin-util-test.json @@ -132,6 +132,12 @@ "output_cmp": "tt-locktime317000-out.json", "description": "Adds an nlocktime to a transaction (output in json)" }, + { "exec": "./bitcoin-tx", + "args": ["-create", "locktime=+317"], + "return_code": 1, + "error_txt": "error: Invalid TX locktime requested", + "description": "Tests the check for invalid locktime value" + }, { "exec": "./bitcoin-tx", "args": ["-create", "locktime=317000foo"], "return_code": 1, From fa8acaf0b993c879ee8c516baa36339ff7b72406 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 22:00:30 +0200 Subject: [PATCH 13/17] bitcoin-tx: Reject + sign in replaceable parsing --- src/bitcoin-tx.cpp | 9 ++++----- test/util/data/bitcoin-util-test.json | 9 +++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 84bfe6995ff..4d377a4823d 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -229,16 +229,15 @@ static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal) static void MutateTxRBFOptIn(CMutableTransaction& tx, const std::string& strInIdx) { - // parse requested index - int64_t inIdx = -1; - if (strInIdx != "" && (!ParseInt64(strInIdx, &inIdx) || inIdx < 0 || inIdx >= static_cast(tx.vin.size()))) { + const auto idx{ToIntegral(strInIdx)}; + if (strInIdx != "" && (!idx || *idx >= tx.vin.size())) { throw std::runtime_error("Invalid TX input index '" + strInIdx + "'"); } // set the nSequence to MAX_INT - 2 (= RBF opt in flag) - int cnt = 0; + uint32_t cnt{0}; for (CTxIn& txin : tx.vin) { - if (strInIdx == "" || cnt == inIdx) { + if (strInIdx == "" || cnt == *idx) { if (txin.nSequence > MAX_BIP125_RBF_SEQUENCE) { txin.nSequence = MAX_BIP125_RBF_SEQUENCE; } diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json index d599731ca6e..81423724075 100644 --- a/test/util/data/bitcoin-util-test.json +++ b/test/util/data/bitcoin-util-test.json @@ -177,6 +177,15 @@ "error_txt": "error: Invalid TX input index", "description": "Tests the check for an invalid string input index with replaceable" }, + { "exec": "./bitcoin-tx", + "args": + ["-create", + "in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:0", + "replaceable=+0"], + "return_code": 1, + "error_txt": "error: Invalid TX input index", + "description": "Tests the check for an invalid string input index with replaceable" + }, { "exec": "./bitcoin-tx", "args": From face2519fac9e840d52f0334d9079e664be7eb28 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 22:00:48 +0200 Subject: [PATCH 14/17] bitcoin-tx: Reject + sign in vout parsing --- src/bitcoin-tx.cpp | 7 ++++--- test/util/data/bitcoin-util-test.json | 8 ++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 4d377a4823d..257c1061e6b 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -275,9 +275,10 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu // extract and validate vout const std::string& strVout = vStrInputParts[1]; - int64_t vout; - if (!ParseInt64(strVout, &vout) || vout < 0 || vout > static_cast(maxVout)) + const auto vout{ToIntegral(strVout)}; + if (!vout || *vout > maxVout) { throw std::runtime_error("invalid TX input vout '" + strVout + "'"); + } // extract the optional sequence number uint32_t nSequenceIn = CTxIn::SEQUENCE_FINAL; @@ -286,7 +287,7 @@ static void MutateTxAddInput(CMutableTransaction& tx, const std::string& strInpu } // append to transaction input list - CTxIn txin(*txid, vout, CScript(), nSequenceIn); + CTxIn txin{*txid, *vout, CScript{}, nSequenceIn}; tx.vin.push_back(txin); } diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json index 81423724075..896decc4e3e 100644 --- a/test/util/data/bitcoin-util-test.json +++ b/test/util/data/bitcoin-util-test.json @@ -264,6 +264,14 @@ "error_txt": "error: invalid TX sequence id 'abcdef00'", "description": "Try to make invalid input replaceable" }, + { "exec": "./bitcoin-tx", + "args": + ["-create", + "in=5897de6bd6027a475eadd57019d4e6872c396d0716c4875a5f1a6fcfdf385c1f:+0"], + "return_code": 1, + "error_txt": "error: invalid TX input vout", + "description": "Tests the check for an invalid vout value when adding an input" + }, { "exec": "./bitcoin-tx", "args": ["-create", From fa84e6c36cb0accf87123ca4eb98f6219d55fb5e Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 22:00:29 +0200 Subject: [PATCH 15/17] bitcoin-tx: Reject + sign in MutateTxDel* --- src/bitcoin-tx.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 257c1061e6b..937db68e5af 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -507,26 +507,20 @@ static void MutateTxAddOutScript(CMutableTransaction& tx, const std::string& str static void MutateTxDelInput(CMutableTransaction& tx, const std::string& strInIdx) { - // parse requested deletion index - int64_t inIdx; - if (!ParseInt64(strInIdx, &inIdx) || inIdx < 0 || inIdx >= static_cast(tx.vin.size())) { + const auto idx{ToIntegral(strInIdx)}; + if (!idx || idx >= tx.vin.size()) { throw std::runtime_error("Invalid TX input index '" + strInIdx + "'"); } - - // delete input from transaction - tx.vin.erase(tx.vin.begin() + inIdx); + tx.vin.erase(tx.vin.begin() + *idx); } static void MutateTxDelOutput(CMutableTransaction& tx, const std::string& strOutIdx) { - // parse requested deletion index - int64_t outIdx; - if (!ParseInt64(strOutIdx, &outIdx) || outIdx < 0 || outIdx >= static_cast(tx.vout.size())) { + const auto idx{ToIntegral(strOutIdx)}; + if (!idx || idx >= tx.vout.size()) { throw std::runtime_error("Invalid TX output index '" + strOutIdx + "'"); } - - // delete output from transaction - tx.vout.erase(tx.vout.begin() + outIdx); + tx.vout.erase(tx.vout.begin() + *idx); } static const unsigned int N_SIGHASH_OPTS = 7; From 33332829333b589420f8038541d04ec6970f051d Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 15 May 2025 22:33:51 +0200 Subject: [PATCH 16/17] refactor: Remove unused Parse(U)Int* --- src/test/fuzz/locale.cpp | 20 ---- src/test/fuzz/parse_numbers.cpp | 56 ++++++--- src/test/util_tests.cpp | 199 -------------------------------- src/util/strencodings.cpp | 51 -------- src/util/strencodings.h | 42 ------- 5 files changed, 41 insertions(+), 327 deletions(-) diff --git a/src/test/fuzz/locale.cpp b/src/test/fuzz/locale.cpp index 68db8422477..c0289172696 100644 --- a/src/test/fuzz/locale.cpp +++ b/src/test/fuzz/locale.cpp @@ -46,16 +46,8 @@ FUZZ_TARGET(locale) assert(c_locale != nullptr); const std::string random_string = fuzzed_data_provider.ConsumeRandomLengthString(5); - int32_t parseint32_out_without_locale; - const bool parseint32_without_locale = ParseInt32(random_string, &parseint32_out_without_locale); - int64_t parseint64_out_without_locale; - const bool parseint64_without_locale = ParseInt64(random_string, &parseint64_out_without_locale); const int64_t random_int64 = fuzzed_data_provider.ConsumeIntegral(); const std::string tostring_without_locale = util::ToString(random_int64); - // The variable `random_int32` is no longer used, but the harness still needs to - // consume the same data that it did previously to not invalidate existing seeds. - const int32_t random_int32 = fuzzed_data_provider.ConsumeIntegral(); - (void)random_int32; const std::string strprintf_int_without_locale = strprintf("%d", random_int64); const double random_double = fuzzed_data_provider.ConsumeFloatingPoint(); const std::string strprintf_double_without_locale = strprintf("%f", random_double); @@ -63,18 +55,6 @@ FUZZ_TARGET(locale) const char* new_locale = std::setlocale(LC_ALL, locale_identifier.c_str()); assert(new_locale != nullptr); - int32_t parseint32_out_with_locale; - const bool parseint32_with_locale = ParseInt32(random_string, &parseint32_out_with_locale); - assert(parseint32_without_locale == parseint32_with_locale); - if (parseint32_without_locale) { - assert(parseint32_out_without_locale == parseint32_out_with_locale); - } - int64_t parseint64_out_with_locale; - const bool parseint64_with_locale = ParseInt64(random_string, &parseint64_out_with_locale); - assert(parseint64_without_locale == parseint64_with_locale); - if (parseint64_without_locale) { - assert(parseint64_out_without_locale == parseint64_out_with_locale); - } const std::string tostring_with_locale = util::ToString(random_int64); assert(tostring_without_locale == tostring_with_locale); const std::string strprintf_int_with_locale = strprintf("%d", random_int64); diff --git a/src/test/fuzz/parse_numbers.cpp b/src/test/fuzz/parse_numbers.cpp index 2cd31466790..6102647d695 100644 --- a/src/test/fuzz/parse_numbers.cpp +++ b/src/test/fuzz/parse_numbers.cpp @@ -5,33 +5,59 @@ #include #include #include +#include +#include +#include +#include #include FUZZ_TARGET(parse_numbers) { const std::string random_string(buffer.begin(), buffer.end()); + { + const auto i8{ToIntegral(random_string)}; + const auto u8{ToIntegral(random_string)}; + const auto i16{ToIntegral(random_string)}; + const auto u16{ToIntegral(random_string)}; + const auto i32{ToIntegral(random_string)}; + const auto u32{ToIntegral(random_string)}; + const auto i64{ToIntegral(random_string)}; + const auto u64{ToIntegral(random_string)}; + // Dont check any values, just that each success result must fit into + // the one with the largest bit-width. + if (i8) { + assert(i8 == i64); + } + if (u8) { + assert(u8 == u64); + } + if (i16) { + assert(i16 == i64); + } + if (u16) { + assert(u16 == u64); + } + if (i32) { + assert(i32 == i64); + } + if (u32) { + assert(u32 == u64); + } + constexpr auto digits{"0123456789"}; + if (i64) { + assert(util::RemovePrefixView(random_string, "-").find_first_not_of(digits) == std::string::npos); + } + if (u64) { + assert(random_string.find_first_not_of(digits) == std::string::npos); + } + } (void)ParseMoney(random_string); - uint8_t u8; - (void)ParseUInt8(random_string, &u8); - - uint16_t u16; - (void)ParseUInt16(random_string, &u16); - - int32_t i32; - (void)ParseInt32(random_string, &i32); (void)LocaleIndependentAtoi(random_string); - uint32_t u32; - (void)ParseUInt32(random_string, &u32); - int64_t i64; (void)LocaleIndependentAtoi(random_string); (void)ParseFixedPoint(random_string, 3, &i64); - (void)ParseInt64(random_string, &i64); - - uint64_t u64; - (void)ParseUInt64(random_string, &u64); } diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index dc0e630a225..4263dfa711b 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -647,40 +647,6 @@ BOOST_AUTO_TEST_CASE(util_overflow) TestAddMatrix(); } -BOOST_AUTO_TEST_CASE(test_ParseInt32) -{ - int32_t n; - // Valid values - BOOST_CHECK(ParseInt32("1234", nullptr)); - BOOST_CHECK(ParseInt32("0", &n) && n == 0); - BOOST_CHECK(ParseInt32("1234", &n) && n == 1234); - BOOST_CHECK(ParseInt32("01234", &n) && n == 1234); // no octal - BOOST_CHECK(ParseInt32("2147483647", &n) && n == 2147483647); - BOOST_CHECK(ParseInt32("-2147483648", &n) && n == (-2147483647 - 1)); // (-2147483647 - 1) equals INT_MIN - BOOST_CHECK(ParseInt32("-1234", &n) && n == -1234); - BOOST_CHECK(ParseInt32("00000000000000001234", &n) && n == 1234); - BOOST_CHECK(ParseInt32("-00000000000000001234", &n) && n == -1234); - BOOST_CHECK(ParseInt32("00000000000000000000", &n) && n == 0); - BOOST_CHECK(ParseInt32("-00000000000000000000", &n) && n == 0); - // Invalid values - BOOST_CHECK(!ParseInt32("", &n)); - BOOST_CHECK(!ParseInt32(" 1", &n)); // no padding inside - BOOST_CHECK(!ParseInt32("1 ", &n)); - BOOST_CHECK(!ParseInt32("++1", &n)); - BOOST_CHECK(!ParseInt32("+-1", &n)); - BOOST_CHECK(!ParseInt32("-+1", &n)); - BOOST_CHECK(!ParseInt32("--1", &n)); - BOOST_CHECK(!ParseInt32("1a", &n)); - BOOST_CHECK(!ParseInt32("aap", &n)); - BOOST_CHECK(!ParseInt32("0x1", &n)); // no hex - BOOST_CHECK(!ParseInt32(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); - // Overflow and underflow - BOOST_CHECK(!ParseInt32("-2147483649", nullptr)); - BOOST_CHECK(!ParseInt32("2147483648", nullptr)); - BOOST_CHECK(!ParseInt32("-32482348723847471234", nullptr)); - BOOST_CHECK(!ParseInt32("32482348723847471234", nullptr)); -} - template static void RunToIntegralTests() { @@ -868,171 +834,6 @@ BOOST_AUTO_TEST_CASE(test_LocaleIndependentAtoi) BOOST_CHECK_EQUAL(LocaleIndependentAtoi("256"), 255U); } -BOOST_AUTO_TEST_CASE(test_ParseInt64) -{ - int64_t n; - // Valid values - BOOST_CHECK(ParseInt64("1234", nullptr)); - BOOST_CHECK(ParseInt64("0", &n) && n == 0LL); - BOOST_CHECK(ParseInt64("1234", &n) && n == 1234LL); - BOOST_CHECK(ParseInt64("01234", &n) && n == 1234LL); // no octal - BOOST_CHECK(ParseInt64("2147483647", &n) && n == 2147483647LL); - BOOST_CHECK(ParseInt64("-2147483648", &n) && n == -2147483648LL); - BOOST_CHECK(ParseInt64("9223372036854775807", &n) && n == int64_t{9223372036854775807}); - BOOST_CHECK(ParseInt64("-9223372036854775808", &n) && n == int64_t{-9223372036854775807-1}); - BOOST_CHECK(ParseInt64("-1234", &n) && n == -1234LL); - // Invalid values - BOOST_CHECK(!ParseInt64("", &n)); - BOOST_CHECK(!ParseInt64(" 1", &n)); // no padding inside - BOOST_CHECK(!ParseInt64("1 ", &n)); - BOOST_CHECK(!ParseInt64("1a", &n)); - BOOST_CHECK(!ParseInt64("aap", &n)); - BOOST_CHECK(!ParseInt64("0x1", &n)); // no hex - BOOST_CHECK(!ParseInt64(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); - // Overflow and underflow - BOOST_CHECK(!ParseInt64("-9223372036854775809", nullptr)); - BOOST_CHECK(!ParseInt64("9223372036854775808", nullptr)); - BOOST_CHECK(!ParseInt64("-32482348723847471234", nullptr)); - BOOST_CHECK(!ParseInt64("32482348723847471234", nullptr)); -} - -BOOST_AUTO_TEST_CASE(test_ParseUInt8) -{ - uint8_t n; - // Valid values - BOOST_CHECK(ParseUInt8("255", nullptr)); - BOOST_CHECK(ParseUInt8("0", &n) && n == 0); - BOOST_CHECK(ParseUInt8("255", &n) && n == 255); - BOOST_CHECK(ParseUInt8("0255", &n) && n == 255); // no octal - BOOST_CHECK(ParseUInt8("255", &n) && n == static_cast(255)); - BOOST_CHECK(ParseUInt8("+255", &n) && n == 255); - BOOST_CHECK(ParseUInt8("00000000000000000012", &n) && n == 12); - BOOST_CHECK(ParseUInt8("00000000000000000000", &n) && n == 0); - // Invalid values - BOOST_CHECK(!ParseUInt8("-00000000000000000000", &n)); - BOOST_CHECK(!ParseUInt8("", &n)); - BOOST_CHECK(!ParseUInt8(" 1", &n)); // no padding inside - BOOST_CHECK(!ParseUInt8(" -1", &n)); - BOOST_CHECK(!ParseUInt8("++1", &n)); - BOOST_CHECK(!ParseUInt8("+-1", &n)); - BOOST_CHECK(!ParseUInt8("-+1", &n)); - BOOST_CHECK(!ParseUInt8("--1", &n)); - BOOST_CHECK(!ParseUInt8("-1", &n)); - BOOST_CHECK(!ParseUInt8("1 ", &n)); - BOOST_CHECK(!ParseUInt8("1a", &n)); - BOOST_CHECK(!ParseUInt8("aap", &n)); - BOOST_CHECK(!ParseUInt8("0x1", &n)); // no hex - BOOST_CHECK(!ParseUInt8(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); - // Overflow and underflow - BOOST_CHECK(!ParseUInt8("-255", &n)); - BOOST_CHECK(!ParseUInt8("256", &n)); - BOOST_CHECK(!ParseUInt8("-123", &n)); - BOOST_CHECK(!ParseUInt8("-123", nullptr)); - BOOST_CHECK(!ParseUInt8("256", nullptr)); -} - -BOOST_AUTO_TEST_CASE(test_ParseUInt16) -{ - uint16_t n; - // Valid values - BOOST_CHECK(ParseUInt16("1234", nullptr)); - BOOST_CHECK(ParseUInt16("0", &n) && n == 0); - BOOST_CHECK(ParseUInt16("1234", &n) && n == 1234); - BOOST_CHECK(ParseUInt16("01234", &n) && n == 1234); // no octal - BOOST_CHECK(ParseUInt16("65535", &n) && n == static_cast(65535)); - BOOST_CHECK(ParseUInt16("+65535", &n) && n == 65535); - BOOST_CHECK(ParseUInt16("00000000000000000012", &n) && n == 12); - BOOST_CHECK(ParseUInt16("00000000000000000000", &n) && n == 0); - // Invalid values - BOOST_CHECK(!ParseUInt16("-00000000000000000000", &n)); - BOOST_CHECK(!ParseUInt16("", &n)); - BOOST_CHECK(!ParseUInt16(" 1", &n)); // no padding inside - BOOST_CHECK(!ParseUInt16(" -1", &n)); - BOOST_CHECK(!ParseUInt16("++1", &n)); - BOOST_CHECK(!ParseUInt16("+-1", &n)); - BOOST_CHECK(!ParseUInt16("-+1", &n)); - BOOST_CHECK(!ParseUInt16("--1", &n)); - BOOST_CHECK(!ParseUInt16("-1", &n)); - BOOST_CHECK(!ParseUInt16("1 ", &n)); - BOOST_CHECK(!ParseUInt16("1a", &n)); - BOOST_CHECK(!ParseUInt16("aap", &n)); - BOOST_CHECK(!ParseUInt16("0x1", &n)); // no hex - BOOST_CHECK(!ParseUInt16(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); - // Overflow and underflow - BOOST_CHECK(!ParseUInt16("-65535", &n)); - BOOST_CHECK(!ParseUInt16("65536", &n)); - BOOST_CHECK(!ParseUInt16("-123", &n)); - BOOST_CHECK(!ParseUInt16("-123", nullptr)); - BOOST_CHECK(!ParseUInt16("65536", nullptr)); -} - -BOOST_AUTO_TEST_CASE(test_ParseUInt32) -{ - uint32_t n; - // Valid values - BOOST_CHECK(ParseUInt32("1234", nullptr)); - BOOST_CHECK(ParseUInt32("0", &n) && n == 0); - BOOST_CHECK(ParseUInt32("1234", &n) && n == 1234); - BOOST_CHECK(ParseUInt32("01234", &n) && n == 1234); // no octal - BOOST_CHECK(ParseUInt32("2147483647", &n) && n == 2147483647); - BOOST_CHECK(ParseUInt32("2147483648", &n) && n == uint32_t{2147483648}); - BOOST_CHECK(ParseUInt32("4294967295", &n) && n == uint32_t{4294967295}); - BOOST_CHECK(ParseUInt32("+1234", &n) && n == 1234); - BOOST_CHECK(ParseUInt32("00000000000000001234", &n) && n == 1234); - BOOST_CHECK(ParseUInt32("00000000000000000000", &n) && n == 0); - // Invalid values - BOOST_CHECK(!ParseUInt32("-00000000000000000000", &n)); - BOOST_CHECK(!ParseUInt32("", &n)); - BOOST_CHECK(!ParseUInt32(" 1", &n)); // no padding inside - BOOST_CHECK(!ParseUInt32(" -1", &n)); - BOOST_CHECK(!ParseUInt32("++1", &n)); - BOOST_CHECK(!ParseUInt32("+-1", &n)); - BOOST_CHECK(!ParseUInt32("-+1", &n)); - BOOST_CHECK(!ParseUInt32("--1", &n)); - BOOST_CHECK(!ParseUInt32("-1", &n)); - BOOST_CHECK(!ParseUInt32("1 ", &n)); - BOOST_CHECK(!ParseUInt32("1a", &n)); - BOOST_CHECK(!ParseUInt32("aap", &n)); - BOOST_CHECK(!ParseUInt32("0x1", &n)); // no hex - BOOST_CHECK(!ParseUInt32(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); - // Overflow and underflow - BOOST_CHECK(!ParseUInt32("-2147483648", &n)); - BOOST_CHECK(!ParseUInt32("4294967296", &n)); - BOOST_CHECK(!ParseUInt32("-1234", &n)); - BOOST_CHECK(!ParseUInt32("-32482348723847471234", nullptr)); - BOOST_CHECK(!ParseUInt32("32482348723847471234", nullptr)); -} - -BOOST_AUTO_TEST_CASE(test_ParseUInt64) -{ - uint64_t n; - // Valid values - BOOST_CHECK(ParseUInt64("1234", nullptr)); - BOOST_CHECK(ParseUInt64("0", &n) && n == 0LL); - BOOST_CHECK(ParseUInt64("1234", &n) && n == 1234LL); - BOOST_CHECK(ParseUInt64("01234", &n) && n == 1234LL); // no octal - BOOST_CHECK(ParseUInt64("2147483647", &n) && n == 2147483647LL); - BOOST_CHECK(ParseUInt64("9223372036854775807", &n) && n == 9223372036854775807ULL); - BOOST_CHECK(ParseUInt64("9223372036854775808", &n) && n == 9223372036854775808ULL); - BOOST_CHECK(ParseUInt64("18446744073709551615", &n) && n == 18446744073709551615ULL); - // Invalid values - BOOST_CHECK(!ParseUInt64("", &n)); - BOOST_CHECK(!ParseUInt64(" 1", &n)); // no padding inside - BOOST_CHECK(!ParseUInt64(" -1", &n)); - BOOST_CHECK(!ParseUInt64("1 ", &n)); - BOOST_CHECK(!ParseUInt64("1a", &n)); - BOOST_CHECK(!ParseUInt64("aap", &n)); - BOOST_CHECK(!ParseUInt64("0x1", &n)); // no hex - BOOST_CHECK(!ParseUInt64(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); - // Overflow and underflow - BOOST_CHECK(!ParseUInt64("-9223372036854775809", nullptr)); - BOOST_CHECK(!ParseUInt64("18446744073709551616", nullptr)); - BOOST_CHECK(!ParseUInt64("-32482348723847471234", nullptr)); - BOOST_CHECK(!ParseUInt64("-2147483648", &n)); - BOOST_CHECK(!ParseUInt64("-9223372036854775808", &n)); - BOOST_CHECK(!ParseUInt64("-1234", &n)); -} - BOOST_AUTO_TEST_CASE(test_FormatParagraph) { BOOST_CHECK_EQUAL(FormatParagraph("", 79, 0), ""); diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 0c09915c8b4..289f49d46a7 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -199,57 +199,6 @@ std::optional> DecodeBase32(std::string_view str) return ret; } -namespace { -template -bool ParseIntegral(std::string_view str, T* out) -{ - static_assert(std::is_integral_v); - // Replicate the exact behavior of strtol/strtoll/strtoul/strtoull when - // handling leading +/- for backwards compatibility. - if (str.length() >= 2 && str[0] == '+' && str[1] == '-') { - return false; - } - const std::optional opt_int = ToIntegral((!str.empty() && str[0] == '+') ? str.substr(1) : str); - if (!opt_int) { - return false; - } - if (out != nullptr) { - *out = *opt_int; - } - return true; -} -}; // namespace - -bool ParseInt32(std::string_view str, int32_t* out) -{ - return ParseIntegral(str, out); -} - -bool ParseInt64(std::string_view str, int64_t* out) -{ - return ParseIntegral(str, out); -} - -bool ParseUInt8(std::string_view str, uint8_t* out) -{ - return ParseIntegral(str, out); -} - -bool ParseUInt16(std::string_view str, uint16_t* out) -{ - return ParseIntegral(str, out); -} - -bool ParseUInt32(std::string_view str, uint32_t* out) -{ - return ParseIntegral(str, out); -} - -bool ParseUInt64(std::string_view str, uint64_t* out) -{ - return ParseIntegral(str, out); -} - std::string FormatParagraph(std::string_view in, size_t width, size_t indent) { assert(width >= indent); diff --git a/src/util/strencodings.h b/src/util/strencodings.h index fd713a555c7..154a610dfb4 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -187,48 +187,6 @@ std::optional ToIntegral(std::string_view str) return result; } -/** - * Convert string to signed 32-bit integer with strict parse error feedback. - * @returns true if the entire string could be parsed as valid integer, - * false if not the entire string could be parsed or when overflow or underflow occurred. - */ -[[nodiscard]] bool ParseInt32(std::string_view str, int32_t *out); - -/** - * Convert string to signed 64-bit integer with strict parse error feedback. - * @returns true if the entire string could be parsed as valid integer, - * false if not the entire string could be parsed or when overflow or underflow occurred. - */ -[[nodiscard]] bool ParseInt64(std::string_view str, int64_t *out); - -/** - * Convert decimal string to unsigned 8-bit integer with strict parse error feedback. - * @returns true if the entire string could be parsed as valid integer, - * false if not the entire string could be parsed or when overflow or underflow occurred. - */ -[[nodiscard]] bool ParseUInt8(std::string_view str, uint8_t *out); - -/** - * Convert decimal string to unsigned 16-bit integer with strict parse error feedback. - * @returns true if the entire string could be parsed as valid integer, - * false if the entire string could not be parsed or if overflow or underflow occurred. - */ -[[nodiscard]] bool ParseUInt16(std::string_view str, uint16_t* out); - -/** - * Convert decimal string to unsigned 32-bit integer with strict parse error feedback. - * @returns true if the entire string could be parsed as valid integer, - * false if not the entire string could be parsed or when overflow or underflow occurred. - */ -[[nodiscard]] bool ParseUInt32(std::string_view str, uint32_t *out); - -/** - * Convert decimal string to unsigned 64-bit integer with strict parse error feedback. - * @returns true if the entire string could be parsed as valid integer, - * false if not the entire string could be parsed or when overflow or underflow occurred. - */ -[[nodiscard]] bool ParseUInt64(std::string_view str, uint64_t *out); - /** * Format a paragraph of text to a fixed width, adding spaces for * indentation to any added line. From faf55fc80b11f3d9b0b12c1b26a9612ea9ce9b40 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 19 May 2025 23:58:33 +0200 Subject: [PATCH 17/17] doc: Remove ParseInt mentions in documentation In the dev notes, remove the whole section, because: * ParseDouble was removed in commit fa9d72a7947d2cff541794e21e0040c3c1d43b32 * The locale-dependent atoi is already checked by test/lint/lint-locale-dependence.py Co-authored-by: Fabian Jahr --- doc/developer-notes.md | 4 ---- src/util/strencodings.h | 3 +-- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index f9ad26651a8..4759146a8fc 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -1009,10 +1009,6 @@ Strings and formatting buffer overflows, and surprises with `\0` characters. Also, some C string manipulations tend to act differently depending on platform, or even the user locale. -- Use `ToIntegral` from [`strencodings.h`](/src/util/strencodings.h) for number parsing. In legacy code you might also find `ParseInt*` family of functions, `ParseDouble` or `LocaleIndependentAtoi`. - - - *Rationale*: These functions do overflow checking and avoid pesky locale issues. - - For `strprintf`, `LogInfo`, `LogDebug`, etc formatting characters don't need size specifiers. - *Rationale*: Bitcoin Core uses tinyformat, which is type safe. Leave them out to avoid confusion. diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 154a610dfb4..01063858047 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -105,8 +105,7 @@ bool SplitHostPort(std::string_view in, uint16_t& portOut, std::string& hostOut) // LocaleIndependentAtoi is provided for backwards compatibility reasons. // -// New code should use ToIntegral or the ParseInt* functions -// which provide parse error feedback. +// New code should use ToIntegral. // // The goal of LocaleIndependentAtoi is to replicate the defined behaviour of // std::atoi as it behaves under the "C" locale, and remove some undefined