Merge #6531: backport: finish merging 25001

e714607 Merge bitcoin#25001: Modernize util/strencodings and util/string (pasta)
f6bd506 rename: ValidAsCString -> ContainsNoNUL (pasta)
0ce624f Use std::string_view throughout util strencodings/string (Pieter Wuille)
5271cfa fix: DecodeBase64 changes in dash code (pasta)
b6122ea Make DecodeBase{32,64} take string_view arguments (Pieter Wuille)
7e5c0b5 Generalize ConvertBits to permit transforming the input (Pieter Wuille)
9519ace Make DecodeBase{32,64} return optional instead of taking bool* (Pieter Wuille)
765ec3f Make DecodeBase{32,64} always return vector, not string (Pieter Wuille)

Pull request description:

  ## Issue being fixed or feature implemented
  This was partially done earlier; do the remaining commits

  ## What was done?
  see commits

  ## How Has This Been Tested?
  built locally

  ## Breaking Changes
  none

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK e714607

Tree-SHA512: e65c347a551c30b04f398ecd2d45388cbc04374a035726be35138dbd39d4921162ecee440056acd13d83aeec1b15ba6db6744c0de62a725859de0177f7c5f3ef

src/base58.cpp

@@ -126,7 +126,7 @@ std::string EncodeBase58(Span<const unsigned char> input)
 
 bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet, int max_ret_len)
 {
-    if (!ValidAsCString(str)) {
+    if (!ContainsNoNUL(str)) {
         return false;
     }
     return DecodeBase58(str.c_str(), vchRet, max_ret_len);
@@ -160,7 +160,7 @@ std::string EncodeBase58Check(Span<const unsigned char> input)
 
 bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet, int max_ret)
 {
-    if (!ValidAsCString(str)) {
+    if (!ContainsNoNUL(str)) {
         return false;
     }
     return DecodeBase58Check(str.c_str(), vchRet, max_ret);

src/bitcoin-tx.cpp

@@ -223,7 +223,7 @@ static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal)
 template <typename T>
 static T TrimAndParse(const std::string& int_str, const std::string& err)
 {
-    const auto parsed{ToIntegral<T>(TrimString(int_str))};
+    const auto parsed{ToIntegral<T>(TrimStringView(int_str))};
     if (!parsed.has_value()) {
         throw std::runtime_error(err + " '" + int_str + "'");
     }

src/httprpc.cpp

@@ -171,10 +171,11 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna
         return false;
     if (strAuth.substr(0, 6) != "Basic ")
         return false;
-    std::string strUserPass64 = TrimString(strAuth.substr(6));
-    bool invalid;
-    std::string strUserPass = DecodeBase64(strUserPass64, &invalid);
-    if (invalid) return false;
+    std::string_view strUserPass64 = TrimStringView(std::string_view{strAuth}.substr(6));
+    auto userpass_data = DecodeBase64(strUserPass64);
+    std::string strUserPass;
+    if (!userpass_data) return false;
+    strUserPass.assign(userpass_data->begin(), userpass_data->end());
 
     if (strUserPass.find(':') != std::string::npos)
         strAuthUsernameOut = strUserPass.substr(0, strUserPass.find(':'));

src/httpserver.cpp

@@ -272,12 +272,11 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
             return false;
 
         std::string strUserPass64 = TrimString(strAuth.substr(6));
-        bool invalid;
-        std::string strUserPass = DecodeBase64(strUserPass64, &invalid);
-        if (invalid) return false;
-
-        if (strUserPass.find(':') == std::string::npos) return false;
-        const std::string username{strUserPass.substr(0, strUserPass.find(':'))};
+        auto opt_strUserPass = DecodeBase64(strUserPass64);
+        if (!opt_strUserPass.has_value()) return false;
+        auto it = std::find(opt_strUserPass->begin(), opt_strUserPass->end(), ':');
+        if (it == opt_strUserPass->end()) return false;
+        const std::string username = std::string(opt_strUserPass->begin(), it);
         return find(g_external_usernames.begin(), g_external_usernames.end(), username) != g_external_usernames.end();
     }();
 

src/i2p.cpp

@@ -69,12 +69,11 @@ static std::string SwapBase64(const std::string& from)
 static Binary DecodeI2PBase64(const std::string& i2p_b64)
 {
     const std::string& std_b64 = SwapBase64(i2p_b64);
-    bool invalid;
-    Binary decoded = DecodeBase64(std_b64.c_str(), &invalid);
-    if (invalid) {
+    auto decoded = DecodeBase64(std_b64);
+    if (!decoded) {
         throw std::runtime_error(strprintf("Cannot decode Base64: \"%s\"", i2p_b64));
     }
-    return decoded;
+    return std::move(*decoded);
 }
 
 /**

src/netaddress.cpp

@@ -211,7 +211,7 @@ static void Checksum(Span<const uint8_t> addr_pubkey, uint8_t (&checksum)[CHECKS
 
 bool CNetAddr::SetSpecial(const std::string& addr)
 {
-    if (!ValidAsCString(addr)) {
+    if (!ContainsNoNUL(addr)) {
         return false;
     }
 
@@ -235,17 +235,16 @@ bool CNetAddr::SetTor(const std::string& addr)
         return false;
     }
 
-    bool invalid;
-    const auto& input = DecodeBase32(addr.substr(0, addr.size() - suffix_len).c_str(), &invalid);
+    auto input = DecodeBase32(std::string_view{addr}.substr(0, addr.size() - suffix_len));
 
-    if (invalid) {
+    if (!input) {
         return false;
     }
 
-    if (input.size() == torv3::TOTAL_LEN) {
-        Span<const uint8_t> input_pubkey{input.data(), ADDR_TORV3_SIZE};
-        Span<const uint8_t> input_checksum{input.data() + ADDR_TORV3_SIZE, torv3::CHECKSUM_LEN};
-        Span<const uint8_t> input_version{input.data() + ADDR_TORV3_SIZE + torv3::CHECKSUM_LEN, sizeof(torv3::VERSION)};
+    if (input->size() == torv3::TOTAL_LEN) {
+        Span<const uint8_t> input_pubkey{input->data(), ADDR_TORV3_SIZE};
+        Span<const uint8_t> input_checksum{input->data() + ADDR_TORV3_SIZE, torv3::CHECKSUM_LEN};
+        Span<const uint8_t> input_version{input->data() + ADDR_TORV3_SIZE + torv3::CHECKSUM_LEN, sizeof(torv3::VERSION)};
 
         if (input_version != torv3::VERSION) {
             return false;
@@ -281,15 +280,14 @@ bool CNetAddr::SetI2P(const std::string& addr)
     // can decode it.
     const std::string b32_padded = addr.substr(0, b32_len) + "====";
 
-    bool invalid;
-    const auto& address_bytes = DecodeBase32(b32_padded.c_str(), &invalid);
+    auto address_bytes = DecodeBase32(b32_padded);
 
-    if (invalid || address_bytes.size() != ADDR_I2P_SIZE) {
+    if (!address_bytes || address_bytes->size() != ADDR_I2P_SIZE) {
         return false;
     }
 
     m_net = NET_I2P;
-    m_addr.assign(address_bytes.begin(), address_bytes.end());
+    m_addr.assign(address_bytes->begin(), address_bytes->end());
 
     return true;
 }

src/netbase.cpp

@@ -144,7 +144,7 @@ std::vector<std::string> GetNetworkNames(bool append_unroutable)
 
 static std::vector<CNetAddr> LookupIntern(const std::string& name, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function)
 {
-    if (!ValidAsCString(name)) return {};
+    if (!ContainsNoNUL(name)) return {};
     {
         CNetAddr addr;
         // From our perspective, onion addresses are not hostnames but rather
@@ -173,7 +173,7 @@ static std::vector<CNetAddr> LookupIntern(const std::string& name, unsigned int
 
 std::vector<CNetAddr> LookupHost(const std::string& name, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function)
 {
-    if (!ValidAsCString(name)) return {};
+    if (!ContainsNoNUL(name)) return {};
     std::string strHost = name;
     if (strHost.empty()) return {};
     if (strHost.front() == '[' && strHost.back() == ']') {
@@ -191,7 +191,7 @@ std::optional<CNetAddr> LookupHost(const std::string& name, bool fAllowLookup, D
 
 std::vector<CService> Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function)
 {
-    if (name.empty() || !ValidAsCString(name)) {
+    if (name.empty() || !ContainsNoNUL(name)) {
         return {};
     }
     uint16_t port{portDefault};
@@ -216,7 +216,7 @@ std::optional<CService> Lookup(const std::string& name, uint16_t portDefault, bo
 
 CService LookupNumeric(const std::string& name, uint16_t portDefault, DNSLookupFn dns_lookup_function)
 {
-    if (!ValidAsCString(name)) {
+    if (!ContainsNoNUL(name)) {
         return {};
     }
     // "1.2:345" will fail to resolve the ip, but will still set the port.
@@ -667,7 +667,7 @@ bool ConnectThroughProxy(const Proxy& proxy, const std::string& strDest, uint16_
 
 bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out)
 {
-    if (!ValidAsCString(subnet_str)) {
+    if (!ContainsNoNUL(subnet_str)) {
         return false;
     }
 

src/psbt.cpp

@@ -343,18 +343,17 @@ std::string PSBTRoleName(PSBTRole role) {
 
 bool DecodeBase64PSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error)
 {
-    bool invalid;
-    std::string tx_data = DecodeBase64(base64_tx, &invalid);
-    if (invalid) {
+    auto tx_data = DecodeBase64(base64_tx);
+    if (!tx_data) {
         error = "invalid base64";
         return false;
     }
-    return DecodeRawPSBT(psbt, tx_data, error);
+    return DecodeRawPSBT(psbt, MakeByteSpan(*tx_data), error);
 }
 
-bool DecodeRawPSBT(PartiallySignedTransaction& psbt, const std::string& tx_data, std::string& error)
+bool DecodeRawPSBT(PartiallySignedTransaction& psbt, Span<const std::byte> tx_data, std::string& error)
 {
-    CDataStream ss_data(MakeByteSpan(tx_data), SER_NETWORK, PROTOCOL_VERSION);
+    CDataStream ss_data(tx_data, SER_NETWORK, PROTOCOL_VERSION);
     try {
         ss_data >> psbt;
         if (!ss_data.empty()) {

src/psbt.h

@@ -914,6 +914,6 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti
 //! Decode a base64ed PSBT into a PartiallySignedTransaction
 [[nodiscard]] bool DecodeBase64PSBT(PartiallySignedTransaction& decoded_psbt, const std::string& base64_psbt, std::string& error);
 //! Decode a raw (binary blob) PSBT into a PartiallySignedTransaction
-[[nodiscard]] bool DecodeRawPSBT(PartiallySignedTransaction& decoded_psbt, const std::string& raw_psbt, std::string& error);
+[[nodiscard]] bool DecodeRawPSBT(PartiallySignedTransaction& decoded_psbt, Span<const std::byte> raw_psbt, std::string& error);
 
 #endif // BITCOIN_PSBT_H

src/qt/walletframe.cpp

@@ -249,16 +249,16 @@ void WalletFrame::gotoVerifyMessageTab(QString addr)
 
 void WalletFrame::gotoLoadPSBT(bool from_clipboard)
 {
-    std::string data;
+    std::vector<unsigned char> data;
 
     if (from_clipboard) {
         std::string raw = QApplication::clipboard()->text().toStdString();
-        bool invalid;
-        data = DecodeBase64(raw, &invalid);
-        if (invalid) {
+        auto result = DecodeBase64(raw);
+        if (!result) {
             Q_EMIT message(tr("Error"), tr("Unable to decode PSBT from clipboard (invalid base64)"), CClientUIInterface::MSG_ERROR);
             return;
         }
+        data = std::move(*result);
     } else {
         QString filename = GUIUtil::getOpenFileName(this,
             tr("Load Transaction Data"), QString(),
@@ -269,12 +269,12 @@ void WalletFrame::gotoLoadPSBT(bool from_clipboard)
             return;
         }
         std::ifstream in{filename.toLocal8Bit().data(), std::ios::binary};
-        data = std::string(std::istreambuf_iterator<char>{in}, {});
+        data.assign(std::istream_iterator<unsigned char>{in}, {});
     }
 
     std::string error;
     PartiallySignedTransaction psbtx;
-    if (!DecodeRawPSBT(psbtx, data, error)) {
+    if (!DecodeRawPSBT(psbtx, MakeByteSpan(data), error)) {
         Q_EMIT message(tr("Error"), tr("Unable to decode PSBT") + "\n" + QString::fromStdString(error), CClientUIInterface::MSG_ERROR);
         return;
     }

src/rpc/evo.cpp

@@ -815,9 +815,9 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request,
                     } else if (err != SigningResult::OK){
                         throw JSONRPCError(RPC_WALLET_ERROR, SigningResultString(err));
                     }
-                    bool invalid = false;
-                    ptx.vchSig = DecodeBase64(signed_payload.c_str(), &invalid);
-                    if (invalid) throw JSONRPCError(RPC_INTERNAL_ERROR, "failed to decode base64 ready signature for protx");
+                    auto opt_vchSig = DecodeBase64(signed_payload);
+                    if (!opt_vchSig.has_value()) throw JSONRPCError(RPC_INTERNAL_ERROR, "failed to decode base64 ready signature for protx");
+                    ptx.vchSig = opt_vchSig.value();
                 } // cs_wallet
                 SetTxPayload(tx, ptx);
                 return SignAndSendSpecialTx(request, chain_helper, chainman, tx, fSubmit);
@@ -877,11 +877,11 @@ static RPCHelpMan protx_register_submit()
         throw JSONRPCError(RPC_INVALID_PARAMETER, "payload signature not empty");
     }
 
-    bool decode_fail{false};
-    ptx.vchSig = DecodeBase64(request.params[1].get_str().c_str(), &decode_fail);
-    if (decode_fail) {
+    auto opt_vchSig= DecodeBase64(request.params[1].get_str());
+    if (!opt_vchSig.has_value()) {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "malformed base64 encoding");
     }
+    ptx.vchSig = opt_vchSig.value();
 
     SetTxPayload(tx, ptx);
     return SignAndSendSpecialTx(request, chain_helper, chainman, tx);

src/rpc/governance.cpp

@@ -421,11 +421,10 @@ static bool SignVote(const CWallet& wallet, const CKeyID& keyID, CGovernanceVote
         LogPrintf("SignVote failed due to: %s\n", SigningResultString(err));
         return false;
     }
-    bool failed = true;
-    const auto decoded = DecodeBase64(signature, &failed);
-    CHECK_NONFATAL(!failed); // DecodeBase64 should not fail
+    const auto opt_decoded = DecodeBase64(signature);
+    CHECK_NONFATAL(opt_decoded.has_value()); // DecodeBase64 should not fail
 
-    vote.SetSignature(std::vector<unsigned char>(decoded.data(), decoded.data() + decoded.size()));
+    vote.SetSignature(std::vector<unsigned char>(opt_decoded->data(), opt_decoded->data() + opt_decoded->size()));
     return true;
 }
 
@@ -959,10 +958,9 @@ static RPCHelpMan voteraw()
 
     int64_t nTime = request.params[5].get_int64();
     std::string strSig = request.params[6].get_str();
-    bool fInvalid = false;
-    std::vector<unsigned char> vchSig = DecodeBase64(strSig.c_str(), &fInvalid);
+    auto opt_vchSig = DecodeBase64(strSig);
 
-    if (fInvalid) {
+    if (!opt_vchSig.has_value()) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Malformed base64 encoding");
     }
 
@@ -975,7 +973,7 @@ static RPCHelpMan voteraw()
 
     CGovernanceVote vote(outpoint, hashGovObj, eVoteSignal, eVoteOutcome);
     vote.SetTime(nTime);
-    vote.SetSignature(vchSig);
+    vote.SetSignature(opt_vchSig.value());
 
     bool onlyVotingKeyAllowed = govObjType == GovernanceObject::PROPOSAL && vote.GetSignal() == VOTE_SIGNAL_FUNDING;
 

src/stacktraces.cpp

@@ -426,13 +426,12 @@ std::string GetCrashInfoStrFromSerializedStr(const std::string& ciStr)
 {
     static uint64_t basePtr = GetBaseAddress();
 
-    bool dataInvalid = false;
-    auto buf = DecodeBase32(ciStr.c_str(), &dataInvalid);
-    if (buf.empty() || dataInvalid) {
+    auto opt_buf = DecodeBase32(ciStr.c_str());
+    if (!opt_buf.has_value() || opt_buf->empty()) {
         return "Error while deserializing crash info";
     }
 
-    CDataStream ds(buf, SER_DISK, 0);
+    CDataStream ds(*opt_buf, SER_DISK, 0);
 
     crash_info_header hdr;
     try {

src/test/base32_tests.cpp

@@ -22,22 +22,16 @@ BOOST_AUTO_TEST_CASE(base32_testvectors)
         BOOST_CHECK_EQUAL(strEnc, vstrOut[i]);
         strEnc = EncodeBase32(vstrIn[i], false);
         BOOST_CHECK_EQUAL(strEnc, vstrOutNoPadding[i]);
-        bool invalid;
-        std::string strDec = DecodeBase32(vstrOut[i], &invalid);
-        BOOST_CHECK(!invalid);
-        BOOST_CHECK_EQUAL(strDec, vstrIn[i]);
+        auto dec = DecodeBase32(vstrOut[i]);
+        BOOST_REQUIRE(dec);
+        BOOST_CHECK_MESSAGE(MakeByteSpan(*dec) == MakeByteSpan(vstrIn[i]), vstrOut[i]);
     }
 
     // Decoding strings with embedded NUL characters should fail
-    bool failure;
-    (void)DecodeBase32("invalid\0"s, &failure); // correct size, invalid due to \0
-    BOOST_CHECK(failure);
-    (void)DecodeBase32("AWSX3VPP"s, &failure); // valid
-    BOOST_CHECK(!failure);
-    (void)DecodeBase32("AWSX3VPP\0invalid"s, &failure); // correct size, invalid due to \0
-    BOOST_CHECK(failure);
-    (void)DecodeBase32("AWSX3VPPinvalid"s, &failure); // invalid size
-    BOOST_CHECK(failure);
+    BOOST_CHECK(!DecodeBase32("invalid\0"s)); // correct size, invalid due to \0
+    BOOST_CHECK(DecodeBase32("AWSX3VPP"s)); // valid
+    BOOST_CHECK(!DecodeBase32("AWSX3VPP\0invalid"s)); // correct size, invalid due to \0
+    BOOST_CHECK(!DecodeBase32("AWSX3VPPinvalid"s)); // invalid size
 }
 
 BOOST_AUTO_TEST_SUITE_END()

src/test/base64_tests.cpp

@@ -19,10 +19,9 @@ BOOST_AUTO_TEST_CASE(base64_testvectors)
     {
         std::string strEnc = EncodeBase64(vstrIn[i]);
         BOOST_CHECK_EQUAL(strEnc, vstrOut[i]);
-        bool invalid;
-        std::string strDec = DecodeBase64(strEnc, &invalid);
-        BOOST_CHECK(!invalid);
-        BOOST_CHECK_EQUAL(strDec, vstrIn[i]);
+        auto dec = DecodeBase64(strEnc);
+        BOOST_REQUIRE(dec);
+        BOOST_CHECK_MESSAGE(MakeByteSpan(*dec) == MakeByteSpan(vstrIn[i]), vstrOut[i]);
     }
 
     {
@@ -36,15 +35,10 @@ BOOST_AUTO_TEST_CASE(base64_testvectors)
     }
 
     // Decoding strings with embedded NUL characters should fail
-    bool failure;
-    (void)DecodeBase64("invalid\0"s, &failure);
-    BOOST_CHECK(failure);
-    (void)DecodeBase64("nQB/pZw="s, &failure);
-    BOOST_CHECK(!failure);
-    (void)DecodeBase64("nQB/pZw=\0invalid"s, &failure);
-    BOOST_CHECK(failure);
-    (void)DecodeBase64("nQB/pZw=invalid\0"s, &failure);
-    BOOST_CHECK(failure);
+    BOOST_CHECK(!DecodeBase64("invalid\0"s));
+    BOOST_CHECK(DecodeBase64("nQB/pZw="s));
+    BOOST_CHECK(!DecodeBase64("nQB/pZw=\0invalid"s));
+    BOOST_CHECK(!DecodeBase64("nQB/pZw=invalid\0"s));
 }
 
 BOOST_AUTO_TEST_SUITE_END()