Merge bitcoin/bitcoin#32520: Remove legacy Parse(U)Int*

faf55fc80b doc: Remove ParseInt mentions in documentation (MarcoFalke)
3333282933 refactor: Remove unused Parse(U)Int* (MarcoFalke)
fa84e6c36c bitcoin-tx: Reject + sign in MutateTxDel* (MarcoFalke)
face2519fa bitcoin-tx: Reject + sign in vout parsing (MarcoFalke)
fa8acaf0b9 bitcoin-tx: Reject + sign in replaceable parsing (MarcoFalke)
faff25a558 bitcoin-tx: Reject + sign in locktime (MarcoFalke)
dddd9e5fe3 bitcoin-tx: Reject + sign in nversion parsing (MarcoFalke)
fab06ac037 rest: Use SAFE_CHARS_URI in SanitizeString error msg (MarcoFalke)
8888bb499d rest: Reject + sign in /blockhashbyheight/ (MarcoFalke)
fafd43c691 test: Reject + sign when parsing regtest deployment params (MarcoFalke)
fa123afa0e Reject + sign when checking -ipcfd (MarcoFalke)
fa479857ed Reject + sign in SplitHostPort (MarcoFalke)
fab4c2967d net: Reject + sign when parsing subnet mask (MarcoFalke)
fa89652e68 init: Reject + sign in -*port parsing (MarcoFalke)
fa9c45577d cli: Reject + sign in -netinfo level parsing (MarcoFalke)
fa98041325 refactor: Use ToIntegral in CreateFromDump (MarcoFalke)
fa23ed7fc2 refactor: Use ToIntegral in ParseHDKeypath (MarcoFalke)

Pull request description:

  The legacy int parsing is problematic, because it accepts the `+` sign for unsigned integers. In all cases this is either:

  * Useless, because the `+` sign was already rejected.
  * Erroneous and inconsistent, when third party parsers reject it. (C.f. https://github.com/bitcoin/bitcoin/pull/32365)
  * Confusing, because the `+` sign is  neither documented, nor can it be assumed to be present.

  Fix all issues by removing the legacy int parsing.

ACKs for top commit:
  stickies-v:
    re-ACK faf55fc80b
  brunoerg:
    code review ACK faf55fc80b

Tree-SHA512: a311ab6a58fe02a37741c1800feb3dcfad92377b4bfb61b433b2393f52ba89ef45d00940972b2767b213a3dd7b59e5e35d5b659c586eacdfe4e565a77b12b19f

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<uint32_t>& keypa
         }
 
         // Ensure this is only numbers
-        if (item.find_first_not_of( "0123456789" ) != std::string::npos) {
+        const auto number{ToIntegral<uint32_t>(item)};
+        if (!number) {
             return false;
         }
-        uint32_t number;
-        if (!ParseUInt32(item, &number)) {
-            return false;
-        }
-        path |= number;
+        path |= *number;
 
         keypath.push_back(path);
         first = false;

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<uint16_t>(in.substr(colon + 1))}) {
             in = in.substr(0, colon);
-            portOut = n;
+            portOut = *n;
             valid = (portOut != 0);
         }
     } else {
@@ -200,57 +199,6 @@ std::optional<std::vector<unsigned char>> DecodeBase32(std::string_view str)
     return ret;
 }
 
-namespace {
-template <typename T>
-bool ParseIntegral(std::string_view str, T* out)
-{
-    static_assert(std::is_integral_v<T>);
-    // 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<T> opt_int = ToIntegral<T>((!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<int32_t>(str, out);
-}
-
-bool ParseInt64(std::string_view str, int64_t* out)
-{
-    return ParseIntegral<int64_t>(str, out);
-}
-
-bool ParseUInt8(std::string_view str, uint8_t* out)
-{
-    return ParseIntegral<uint8_t>(str, out);
-}
-
-bool ParseUInt16(std::string_view str, uint16_t* out)
-{
-    return ParseIntegral<uint16_t>(str, out);
-}
-
-bool ParseUInt32(std::string_view str, uint32_t* out)
-{
-    return ParseIntegral<uint32_t>(str, out);
-}
-
-bool ParseUInt64(std::string_view str, uint64_t* out)
-{
-    return ParseIntegral<uint64_t>(str, out);
-}
-
 std::string FormatParagraph(std::string_view in, size_t width, size_t indent)
 {
     assert(width >= indent);

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
@@ -187,48 +186,6 @@ std::optional<T> 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.