[1.0] Update FSI serialization to support dynamically sized data and verify a checksum

libraries/chain/finality/finalizer.cpp

@@ -175,47 +175,80 @@ void my_finalizers_t::maybe_update_fsi(const block_state_ptr& bsp, const qc_t& r
 
 void my_finalizers_t::save_finalizer_safety_info() const {
 
-   if (!persist_file.is_open()) {
+   if (!cfile_ds.is_open()) {
       EOS_ASSERT(!persist_file_path.empty(), finalizer_safety_exception,
                  "path for storing finalizer safety information file not specified");
       if (!std::filesystem::exists(persist_file_path.parent_path()))
           std::filesystem::create_directories(persist_file_path.parent_path());
-      persist_file.set_file_path(persist_file_path);
-      persist_file.open(fc::cfile::truncate_rw_mode);
+      cfile_ds.set_file_path(persist_file_path);
+      cfile_ds.open(fc::cfile::truncate_rw_mode);
    }
    try {
-      static_assert(sizeof(finalizer_safety_information) == 152, "If size changes then need to handle loading old files");
-      static_assert(sizeof(decltype(bls_public_key{}.affine_non_montgomery_le())) == 96,
-                    "If size changes then need to handle loading old files, fc::pack uses affine_non_montgomery_le()");
-
-      persist_file.seek(0);
-      fc::raw::pack(persist_file, fsi_t::magic);
-      fc::raw::pack(persist_file, current_safety_file_version);
-      fc::raw::pack(persist_file, (uint64_t)(finalizers.size() + inactive_safety_info.size()));
-      for (const auto& [pub_key, f] : finalizers) {
-         fc::raw::pack(persist_file, pub_key);
-         fc::raw::pack(persist_file, f.fsi);
-      }
-      if (!inactive_safety_info_written) {
+      // optimize by only calculating crc for inactive once
+      if (inactive_safety_info_written_pos == 0) {
+         persist_file.seekp(0);
+         fc::raw::pack(persist_file, fsi_t::magic);
+         fc::raw::pack(persist_file, current_safety_file_version);
+         uint64_t size = finalizers.size() + inactive_safety_info.size();
+         fc::raw::pack(persist_file, size);
+
          // save also the fsi that was originally present in the file, but which applied to
          // finalizers not configured anymore.
          for (const auto& [pub_key, fsi] : inactive_safety_info) {
             fc::raw::pack(persist_file, pub_key);
             fc::raw::pack(persist_file, fsi);
          }
-         inactive_safety_info_written = true;
+         inactive_safety_info_written_pos = persist_file.tellp();
+         inactive_crc32 = persist_file.crc();
+      } else {
+         persist_file.seekp(inactive_safety_info_written_pos, inactive_crc32);
+      }
+
+      // active finalizers
+      for (const auto& [pub_key, f] : finalizers) {
+         fc::raw::pack(persist_file, pub_key);
+         fc::raw::pack(persist_file, f.fsi);
       }
-      persist_file.flush();
+
+      uint32_t cs = persist_file.checksum();
+      fc::raw::pack(persist_file, cs);
+
+      cfile_ds.flush();
    } FC_LOG_AND_RETHROW()
 }
 
 // ----------------------------------------------------------------------------------------
+
+void my_finalizers_t::load_finalizer_safety_info_v0(fsi_map& res) {
+   uint64_t num_finalizers {0};
+   fc::raw::unpack(persist_file, num_finalizers);
+   for (size_t i=0; i<num_finalizers; ++i) {
+      bls_public_key pubkey;
+      my_finalizers_t::fsi_t fsi;
+      fc::raw::unpack(persist_file, pubkey);
+      fc::raw::unpack(persist_file, fsi);
+      res.emplace(pubkey, fsi);
+   }
+}
+
+void my_finalizers_t::load_finalizer_safety_info_v1(fsi_map& res) {
+   uint64_t num_finalizers {0};
+   fc::raw::unpack(persist_file, num_finalizers);
+   for (size_t i=0; i<num_finalizers; ++i) {
+      bls_public_key pubkey;
+      my_finalizers_t::fsi_t fsi;
+      fc::raw::unpack(persist_file, pubkey);
+      fc::raw::unpack(persist_file, fsi);
+      res.emplace(pubkey, fsi);
+   }
+}
+
 my_finalizers_t::fsi_map my_finalizers_t::load_finalizer_safety_info() {
    fsi_map res;
 
    EOS_ASSERT(!persist_file_path.empty(), finalizer_safety_exception,
               "path for storing finalizer safety persistence file not specified");
-   EOS_ASSERT(!persist_file.is_open(), finalizer_safety_exception,
+   EOS_ASSERT(!cfile_ds.is_open(), finalizer_safety_exception,
               "Trying to read an already open finalizer safety persistence file: ${p}",
               ("p", persist_file_path));
 
@@ -225,11 +258,11 @@ my_finalizers_t::fsi_map my_finalizers_t::load_finalizer_safety_info() {
       return res;
    }
 
-   persist_file.set_file_path(persist_file_path);
+   cfile_ds.set_file_path(persist_file_path);
 
    try {
       // if we can't open the finalizer safety file, we return an empty map.
-      persist_file.open(fc::cfile::update_rw_mode);
+      cfile_ds.open(fc::cfile::update_rw_mode);
    } catch(std::exception& e) {
       fc_elog(vote_logger, "unable to open finalizer safety persistence file ${p}, using defaults. Exception: ${e}",
               ("p", persist_file_path)("e", e.what()));
@@ -239,7 +272,7 @@ my_finalizers_t::fsi_map my_finalizers_t::load_finalizer_safety_info() {
       return res;
    }
    try {
-      persist_file.seek(0);
+      persist_file.seekp(0);
 
       // read magic number. must be `fsi_t::magic`
       // -----------------------------------------
@@ -257,17 +290,30 @@ my_finalizers_t::fsi_map my_finalizers_t::load_finalizer_safety_info() {
 
       // finally read the `finalizer_safety_information` info
       // ----------------------------------------------------
-      uint64_t num_finalizers {0};
-      fc::raw::unpack(persist_file, num_finalizers);
-      for (size_t i=0; i<num_finalizers; ++i) {
-         bls_public_key pubkey;
-         fsi_t fsi;
-         fc::raw::unpack(persist_file, pubkey);
-         fc::raw::unpack(persist_file, fsi);
-         res.emplace(pubkey, fsi);
+      bool verify_checksum = true;
+      switch (file_version) {
+      case safety_file_version_0:
+         load_finalizer_safety_info_v0(res);
+         verify_checksum = false;
+         break;
+      case safety_file_version_1:
+         load_finalizer_safety_info_v1(res);
+         break;
+      default:
+         assert(0);
+      }
+
+      if (verify_checksum) {
+         // verify checksum
+         uint32_t calculated_checksum = persist_file.checksum();
+         uint32_t cs = 0;
+         fc::raw::unpack(persist_file, cs);
+         EOS_ASSERT(cs == calculated_checksum, finalizer_safety_exception,
+                    "bad checksum reading finalizer safety persistence file: ${p}", ("p", persist_file_path));
       }
 
-      persist_file.close();
+      // close file after write
+      cfile_ds.close();
    } FC_LOG_AND_RETHROW()
    // don't remove file we can't load
    return res;

libraries/chain/include/eosio/chain/finality/finalizer.hpp

@@ -3,6 +3,7 @@
 #include <eosio/chain/finality/vote_message.hpp>
 #include <fc/crypto/bls_utils.hpp>
 #include <fc/io/cfile.hpp>
+#include <fc/io/datastream_crc.hpp>
 #include <compare>
 #include <mutex>
 #include <ranges>
@@ -70,21 +71,34 @@ namespace eosio::chain {
    // ----------------------------------------------------------------------------------------
    struct my_finalizers_t {
    public:
-      static constexpr uint64_t current_safety_file_version = 0;
+      ///
+      /// Version 0: Spring 1.0.0 RC2 - File has fixed packed sizes with inactive safety info written to the end
+      ///                               of the file. Consists of [finalizer public_key, FSI]..
+      /// Version 1: Spring 1.0.0 RC3 - Variable length FSI with inactive safety info written at the beginning of
+      ///                               the file. Uses crc32 checksum to verify data on read.
+      ///
+      static constexpr uint64_t safety_file_version_0       = 0;
+      static constexpr uint64_t safety_file_version_1       = 1;
+      static constexpr uint64_t current_safety_file_version = safety_file_version_1;
 
       using fsi_t   = finalizer_safety_information;
       using fsi_map = std::map<bls_public_key, fsi_t>;
       using vote_t  = std::tuple<vote_message_ptr, finalizer_authority_ptr, finalizer_authority_ptr>;
 
    private:
+      using persist_file_t = fc::datastream_crc<fc::datastream<fc::cfile>>;
+      using finalizer_map_t = std::map<bls_public_key, finalizer>;
+
       const std::filesystem::path       persist_file_path;     // where we save the safety data
       std::atomic<bool>                 has_voted{false};      // true if this node has voted and updated safety info
       mutable std::mutex                mtx;
-      mutable fc::datastream<fc::cfile> persist_file;          // we want to keep the file open for speed
-      std::map<bls_public_key, finalizer>  finalizers;         // the active finalizers for this node, loaded at startup, not mutated afterwards
+      mutable fc::datastream<fc::cfile> cfile_ds;              // we want to keep the file open for speed
+      mutable persist_file_t            persist_file{cfile_ds};// we want to calculate checksum
+      finalizer_map_t                   finalizers;            // the active finalizers for this node, loaded at startup, not mutated afterwards
       fsi_map                           inactive_safety_info;  // loaded at startup, not mutated afterwards
       fsi_t                             default_fsi = fsi_t::unset_fsi(); // default provided at spring startup
-      mutable bool                      inactive_safety_info_written{false};
+      mutable long                      inactive_safety_info_written_pos{0};
+      mutable boost::crc_32_type        inactive_crc32; // cached value
 
    public:
       explicit my_finalizers_t(const std::filesystem::path& persist_file_path)
@@ -167,6 +181,11 @@ namespace eosio::chain {
       // for testing purposes only, not thread safe
       const fsi_t& get_fsi(const bls_public_key& k) { return finalizers[k].fsi; }
       void         set_fsi(const bls_public_key& k, const fsi_t& fsi) { finalizers[k].fsi = fsi; }
+
+   private:
+      void load_finalizer_safety_info_v0(fsi_map& res);
+      void load_finalizer_safety_info_v1(fsi_map& res);
+
    };
 
 }

libraries/libfc/CMakeLists.txt

@@ -103,7 +103,7 @@ if(APPLE)
   find_library(corefoundation_framework CoreFoundation)
 endif()
 target_link_libraries( fc PUBLIC Boost::date_time Boost::chrono Boost::iostreams Boost::interprocess Boost::multi_index Boost::dll
-                                 Boost::multiprecision Boost::beast Boost::asio Boost::thread Threads::Threads
+                                 Boost::multiprecision Boost::beast Boost::asio Boost::thread Boost::crc Threads::Threads
                                  boringssl ZLIB::ZLIB ${PLATFORM_SPECIFIC_LIBS} ${CMAKE_DL_LIBS} secp256k1 bls12-381 ${security_framework} ${corefoundation_framework})
 
 add_subdirectory( test )

libraries/libfc/include/fc/io/cfile.hpp

@@ -329,6 +329,16 @@ class datastream<fc::cfile, void> : public fc::cfile {
       return true;
    }
 
+   bool read( char* d, size_t s ) {
+      cfile::read( d, s );
+      return true;
+   }
+
+   bool write(const char* d, size_t s) {
+      cfile::write(d, s);
+      return true;
+   }
+
    fc::cfile&       storage() { return *this; }
    const fc::cfile& storage() const { return *this; }
 

libraries/libfc/include/fc/io/datastream_crc.hpp

@@ -0,0 +1,109 @@
+#pragma once
+#include <fc/io/datastream.hpp>
+#include <boost/crc.hpp>
+
+namespace fc {
+
+/**
+* Provides a datasteam wrapper around another datastream for calculation of checksum.
+* Example use:
+*      fc::datastream<fc::cfile>                      persist_cfile;
+*      fc::datastream_crc<fc::datastream<fc::cfile>>  persist_stream{persist_cfile};
+*
+*      persist_stream.seekp(0);
+*      fc::raw::pack(persist_stream, 'a');
+*      uint32_t cs = persist_stream.check_sum();
+*      fc::raw::pack(persist_stream, cs); // write checksum to file
+*      // ...
+*      persist_stream.seekp(0);
+*      char c;
+*      fc::raw::unpack(persist_stream, c);
+*      uint32_t calc_cs = persist_stream.check_sum();
+*      uint32_t cs;
+*      fc::raw::unpack(persist_stream, cs);
+*      FC_ASSERT(calc_cs == cs, "checksum not equal");
+*/
+template <typename DS, typename CRC = boost::crc_32_type>
+class datastream_crc {
+public:
+   // ds must outlive datasteam_crc
+   explicit datastream_crc( DS& ds ) : ds_(ds) {}
+
+   void skip(size_t s) {
+      ds_.skip(s);
+   }
+
+   bool read(char* d, size_t s) {
+      bool r = ds_.read(d, s);
+      crc_.process_bytes(d, s);
+      return r;
+   }
+
+   bool write(const char* d, size_t s) {
+      crc_.process_bytes(d, s);
+      return ds_.write(d, s);
+   }
+
+   bool put(char c) {
+      crc_.process_byte(c);
+      return ds_.put(c);
+   }
+
+   bool get(unsigned char& c) {
+      bool r = ds_.get(c);
+      crc_.process_byte(c);
+      return r;
+   }
+
+   bool get(char& c) {
+      bool r = ds_.get(c);
+      crc_.process_byte(c);
+      return r;
+   }
+
+   auto pos() const {
+      return ds_.pos();
+   }
+
+   bool valid() const {
+      return ds_.valid();
+   }
+
+   // only use with p==0, otherwise use seekp() below
+   bool seekp(size_t p) {
+      if (p == 0) {
+         crc_.reset();
+         return ds_.seekp(0);
+      }
+      return false;
+   }
+
+   size_t tellp() const {
+      return ds_.tellp();
+   }
+   size_t remaining() const {
+      return ds_.remaining();
+   }
+
+   // extension to datastream
+
+   bool seekp(size_t p, const CRC& crc) {
+      crc_ = crc;
+      return ds_.seekp(p);
+   }
+
+   uint32_t checksum() const {
+      return crc_.checksum();
+   }
+
+   CRC crc() const {
+      return crc_;
+   }
+
+private:
+   DS& ds_;
+   CRC crc_;
+};
+
+
+} // namespace fc