Use AutoFile

src/Makefile.test.include

@@ -126,7 +126,6 @@ BITCOIN_TESTS =\
   test/raii_event_tests.cpp \
   test/random_tests.cpp \
   test/rbf_tests.cpp \
-  test/readwritefile_tests.cpp \
   test/rest_tests.cpp \
   test/result_tests.cpp \
   test/reverselock_tests.cpp \

src/i2p.cpp

@@ -434,9 +434,9 @@ void Session::CreateIfNotCreatedAlready()
     } else {
         // Read our persistent destination (private key) from disk or generate
         // one and save it to disk. Then use it when creating the session.
-        std::optional<std::string> data{ReadBinaryFile<std::string>(m_private_key_file)};
-        if (data) {
-            m_private_key.assign(data->begin(), data->end());
+        const auto& [read_ok, data] = ReadBinaryFile(m_private_key_file);
+        if (read_ok) {
+            m_private_key.assign(data.begin(), data.end());
         } else {
             GenerateAndSavePrivateKey(*sock);
         }

src/key.h

@@ -213,6 +213,19 @@ class CKey
      * Returns whether the public key as an even Y coordinate.
      */
     bool HasEvenY() const;
+
+    template <typename Stream>
+    inline void Unserialize(Stream& s) {
+        s >> fCompressed;
+        MakeKeyData();
+        s >> MakeWritableByteSpan(*keydata);
+    }
+
+    template<typename Stream>
+    void Serialize(Stream &s) const {
+        s << fCompressed;
+        ::Serialize(s, MakeUCharSpan(*this));
+    }
 };
 
 CKey GenerateRandomKey(bool compressed = true) noexcept;

src/node/sv2_template_provider.cpp

@@ -26,23 +26,22 @@ using node::Sv2SubmitSolutionMsg;
 Sv2TemplateProvider::Sv2TemplateProvider(ChainstateManager& chainman, CTxMemPool& mempool) : m_chainman{chainman}, m_mempool{mempool}
 {
     // Read static key if cached
-    auto data{ReadBinaryFile<std::vector<unsigned char>>(GetStaticKeyFile())};
-    if (data) {
-        if (data->size() != 32) {
-            LogPrintLevel(BCLog::SV2, BCLog::Level::Error,
-                          "Failed to load static key from %s: size %d != 32\n",
-                          fs::PathToString(GetStaticKeyFile()), data->size());
-        }
+    try {
+        AutoFile{fsbridge::fopen(GetStaticKeyFile(), "wb")} >> m_static_key;
         LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Reading cached static key from %s\n", fs::PathToString(GetStaticKeyFile()));
-        m_static_key.Set(data->begin(), data->end(), /*fCompressedIn=*/true);
-    } else {
+    } catch (const std::ios_base::failure&) {
+        // File is not expected to exist the first time.
+        // In the unlikely event that loading an existing key fails, create a new one.
+    }
+    if (!m_static_key.IsValid()) {
         m_static_key = GenerateRandomKey();
-        std::vector<unsigned char> data(m_static_key.begin(), m_static_key.end());
-        if (WriteBinaryFile(GetStaticKeyFile(), data)) {
-            LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Generated static key, saved to %s\n", fs::PathToString(GetStaticKeyFile()));
-        } else {
+        try {
+            AutoFile{fsbridge::fopen(GetStaticKeyFile(), "wb")} << m_static_key;
+        } catch (const std::ios_base::failure&) {
             LogPrintLevel(BCLog::SV2, BCLog::Level::Error, "Error writing static key to %s\n", fs::PathToString(GetStaticKeyFile()));
+            // Continue, because this is not a critical failure.
         }
+        LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Generated static key, saved to %s\n", fs::PathToString(GetStaticKeyFile()));
     }
     LogPrintLevel(BCLog::SV2, BCLog::Level::Info, "Static key: %s\n", HexStr(m_static_key.GetPubKey()));
 
@@ -51,23 +50,21 @@ Sv2TemplateProvider::Sv2TemplateProvider(ChainstateManager& chainman, CTxMemPool
 
     // Load authority key if cached
     CKey authority_key;
-    auto auth_key_data{ReadBinaryFile<std::vector<unsigned char>>(GetAuthorityKeyFile())};
-    if (auth_key_data) {
-        if (auth_key_data->size() != 32) {
-            LogPrintLevel(BCLog::SV2, BCLog::Level::Error,
-                          "Failed to load authority key from %s: size %d != 32\n",
-                          fs::PathToString(GetAuthorityKeyFile()), auth_key_data->size());
-        }
-        LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Reading cached authority key from %s\n", fs::PathToString(GetAuthorityKeyFile()));
-        authority_key.Set(auth_key_data->begin(), auth_key_data->end(), /*fCompressedIn=*/true);
-    } else {
+    try {
+        AutoFile{fsbridge::fopen(GetAuthorityKeyFile(), "wb")} >> authority_key;
+    } catch (const std::ios_base::failure&) {
+        // File is not expected to exist the first time.
+        // In the unlikely event that loading an existing key fails, create a new one.
+    }
+    if (!authority_key.IsValid()) {
         authority_key = GenerateRandomKey();
-        std::vector<unsigned char> data(m_static_key.begin(), m_static_key.end());
-        if (WriteBinaryFile(GetStaticKeyFile(), data)) {
-            LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Generated authority key, saved to %s\n", fs::PathToString(GetAuthorityKeyFile()));
-        } else {
+        try {
+            AutoFile{fsbridge::fopen(GetAuthorityKeyFile(), "wb")} << authority_key;
+        } catch (const std::ios_base::failure&) {
             LogPrintLevel(BCLog::SV2, BCLog::Level::Error, "Error writing authority key to %s\n", fs::PathToString(GetAuthorityKeyFile()));
+            // Continue, because this is not a critical failure.
         }
+        LogPrintLevel(BCLog::SV2, BCLog::Level::Debug, "Generated authority key, saved to %s\n", fs::PathToString(GetAuthorityKeyFile()));
     }
     // SRI uses base58 encoded x-only pubkeys in its configuration files
     LogPrintLevel(BCLog::SV2, BCLog::Level::Info, "Authority key: %s\n", EncodeBase58(XOnlyPubKey(m_static_key.GetPubKey())));

src/test/key_tests.cpp

@@ -398,4 +398,26 @@ BOOST_AUTO_TEST_CASE(ecdh_test)
     BOOST_CHECK(std::memcmp(&ecdh_output_1[0], &ecdh_output_2[0], 32) == 0);
 }
 
+BOOST_AUTO_TEST_CASE(key_serialization)
+{
+    {
+        CKey key{GenerateRandomKey()};
+        DataStream s{};
+        s << key;
+        CKey key_copy;
+        s >> key_copy;
+        BOOST_CHECK(key == key_copy);
+    }
+
+    {
+        CKey key{GenerateRandomKey(/*compressed=*/false)};
+        DataStream s{};
+        s << key;
+        CKey key_copy;
+        s >> key_copy;
+        BOOST_CHECK(key == key_copy);
+    }
+
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/test/util_tests.cpp

@@ -15,6 +15,7 @@
 #include <util/message.h> // For MessageSign(), MessageVerify(), MESSAGE_MAGIC
 #include <util/moneystr.h>
 #include <util/overflow.h>
+#include <util/readwritefile.h>
 #include <util/spanparsing.h>
 #include <util/strencodings.h>
 #include <util/string.h>
@@ -1744,6 +1745,52 @@ BOOST_AUTO_TEST_CASE(util_ParseByteUnits)
     BOOST_CHECK(!ParseByteUnits("1x", noop));
 }
 
+BOOST_AUTO_TEST_CASE(util_ReadBinaryFile)
+{
+    fs::path tmpfolder = m_args.GetDataDirBase();
+    fs::path tmpfile = tmpfolder / "read_binary.dat";
+    std::string expected_text;
+    for (int i = 0; i < 30; i++) {
+        expected_text += "0123456789";
+    }
+    {
+        std::ofstream file{tmpfile};
+        file << expected_text;
+    }
+    {
+        // read all contents in file
+        auto [valid, text] = ReadBinaryFile(tmpfile);
+        BOOST_CHECK(valid);
+        BOOST_CHECK_EQUAL(text, expected_text);
+    }
+    {
+        // read half contents in file
+        auto [valid, text] = ReadBinaryFile(tmpfile, expected_text.size() / 2);
+        BOOST_CHECK(valid);
+        BOOST_CHECK_EQUAL(text, expected_text.substr(0, expected_text.size() / 2));
+    }
+    {
+        // read from non-existent file
+        fs::path invalid_file = tmpfolder / "invalid_binary.dat";
+        auto [valid, text] = ReadBinaryFile(invalid_file);
+        BOOST_CHECK(!valid);
+        BOOST_CHECK(text.empty());
+    }
+}
+
+BOOST_AUTO_TEST_CASE(util_WriteBinaryFile)
+{
+    fs::path tmpfolder = m_args.GetDataDirBase();
+    fs::path tmpfile = tmpfolder / "write_binary.dat";
+    std::string expected_text = "bitcoin";
+    auto valid = WriteBinaryFile(tmpfile, expected_text);
+    std::string actual_text;
+    std::ifstream file{tmpfile};
+    file >> actual_text;
+    BOOST_CHECK(valid);
+    BOOST_CHECK_EQUAL(actual_text, expected_text);
+}
+
 BOOST_AUTO_TEST_CASE(clearshrink_test)
 {
     {

src/torcontrol.cpp

@@ -328,10 +328,10 @@ TorController::TorController(struct event_base* _base, const std::string& tor_co
         LogPrintf("tor: Initiating connection to Tor control port %s failed\n", m_tor_control_center);
     }
     // Read service private key if cached
-    auto data{ReadBinaryFile<std::string>(GetPrivateKeyFile())};
-    if (data) {
+    std::pair<bool,std::string> pkf = ReadBinaryFile(GetPrivateKeyFile());
+    if (pkf.first) {
         LogPrint(BCLog::TOR, "Reading cached private key from %s\n", fs::PathToString(GetPrivateKeyFile()));
-        private_key = data.value();
+        private_key = pkf.second;
     }
 }
 
@@ -586,15 +586,15 @@ void TorController::protocolinfo_cb(TorControlConnection& _conn, const TorContro
         } else if (methods.count("SAFECOOKIE")) {
             // Cookie: hexdump -e '32/1 "%02x""\n"'  ~/.tor/control_auth_cookie
             LogPrint(BCLog::TOR, "Using SAFECOOKIE authentication, reading cookie authentication from %s\n", cookiefile);
-            auto status_cookie = ReadBinaryFile<std::string>(fs::PathFromString(cookiefile), TOR_COOKIE_SIZE);
-            if (status_cookie && status_cookie->size() == TOR_COOKIE_SIZE) {
-                // _conn.Command("AUTHENTICATE " + HexStr(status_cookie), std::bind(&TorController::auth_cb, this, std::placeholders::_1, std::placeholders::_2));
-                cookie = std::vector<uint8_t>(status_cookie->begin(), status_cookie->end());
+            std::pair<bool,std::string> status_cookie = ReadBinaryFile(fs::PathFromString(cookiefile), TOR_COOKIE_SIZE);
+            if (status_cookie.first && status_cookie.second.size() == TOR_COOKIE_SIZE) {
+                // _conn.Command("AUTHENTICATE " + HexStr(status_cookie.second), std::bind(&TorController::auth_cb, this, std::placeholders::_1, std::placeholders::_2));
+                cookie = std::vector<uint8_t>(status_cookie.second.begin(), status_cookie.second.end());
                 clientNonce = std::vector<uint8_t>(TOR_NONCE_SIZE, 0);
                 GetRandBytes(clientNonce);
                 _conn.Command("AUTHCHALLENGE SAFECOOKIE " + HexStr(clientNonce), std::bind(&TorController::authchallenge_cb, this, std::placeholders::_1, std::placeholders::_2));
             } else {
-                if (status_cookie) {
+                if (status_cookie.first) {
                     LogPrintf("tor: Authentication cookie %s is not exactly %i bytes, as is required by the spec\n", cookiefile, TOR_COOKIE_SIZE);
                 } else {
                     LogPrintf("tor: Authentication cookie %s could not be opened (check permissions)\n", cookiefile);

src/util/readwritefile.cpp

@@ -12,34 +12,29 @@
 #include <limits>
 #include <string>
 #include <utility>
-#include <vector>
 
-template <typename T>
-std::optional<T> ReadBinaryFile(const fs::path& filename, size_t maxsize)
+std::pair<bool,std::string> ReadBinaryFile(const fs::path &filename, size_t maxsize)
 {
     FILE *f = fsbridge::fopen(filename, "rb");
-    if (f == nullptr) return {};
-    T output{};
+    if (f == nullptr)
+        return std::make_pair(false,"");
+    std::string retval;
     char buffer[128];
     do {
-        const size_t n = fread(buffer, 1, std::min(sizeof(buffer), maxsize - output.size()), f);
+        const size_t n = fread(buffer, 1, std::min(sizeof(buffer), maxsize - retval.size()), f);
         // Check for reading errors so we don't return any data if we couldn't
         // read the entire file (or up to maxsize)
         if (ferror(f)) {
             fclose(f);
-            return {};
+            return std::make_pair(false,"");
         }
-        output.insert(output.end(), buffer, buffer + n);
-    } while (!feof(f) && output.size() < maxsize);
+        retval.append(buffer, buffer+n);
+    } while (!feof(f) && retval.size() < maxsize);
     fclose(f);
-    return output;
+    return std::make_pair(true,retval);
 }
 
-template std::optional<std::string> ReadBinaryFile(const fs::path &filename, size_t maxsize);
-template std::optional<std::vector<unsigned char>> ReadBinaryFile(const fs::path &filename, size_t maxsize);
-
-template <typename T>
-bool WriteBinaryFile(const fs::path& filename, const T& data)
+bool WriteBinaryFile(const fs::path &filename, const std::string &data)
 {
     FILE *f = fsbridge::fopen(filename, "wb");
     if (f == nullptr)
@@ -53,6 +48,3 @@ bool WriteBinaryFile(const fs::path& filename, const T& data)
     }
     return true;
 }
-
-template bool WriteBinaryFile(const fs::path& filename, const std::string& data);
-template bool WriteBinaryFile(const fs::path& filename, const std::vector<unsigned char>& data);