From 0aae968e6c7b8bac12c2a0ae9584073d742235ec Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Tue, 12 Mar 2024 16:51:15 -0400
Subject: [PATCH 01/11] Add support for new version of fork_db persistent file.

---
 libraries/chain/fork_database.cpp             | 252 +++++++++---------
 .../include/eosio/chain/fork_database.hpp     |  40 +--
 2 files changed, 154 insertions(+), 138 deletions(-)

diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp
index f5d29587b1..0100e3252e 100644
--- a/libraries/chain/fork_database.cpp
+++ b/libraries/chain/fork_database.cpp
@@ -122,12 +122,11 @@ namespace eosio::chain {
       fork_multi_index_type  index;
       bsp_t                  root;
       bsp_t                  head;
-      const uint32_t         magic_number;
 
-      explicit fork_database_impl(uint32_t magic_number) : magic_number(magic_number) {}
+      explicit fork_database_impl() = default;
 
-      void             open_impl( const std::filesystem::path& fork_db_file, validator_t& validator );
-      void             close_impl( const std::filesystem::path& fork_db_file );
+      void             open_impl( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator );
+      void             close_impl( std::ofstream& out );
       void             add_impl( const bsp_t& n, mark_valid_t mark_valid, ignore_duplicate_t ignore_duplicate, bool validate, validator_t& validator );
 
       bsp_t            get_block_impl( const block_id_type& id, include_root_t include_root = include_root_t::no ) const;
@@ -147,109 +146,67 @@ namespace eosio::chain {
    };
 
    template<class BSP>
-   fork_database_t<BSP>::fork_database_t(uint32_t magic_number)
-      :my( new fork_database_impl<BSP>(magic_number) )
+   fork_database_t<BSP>::fork_database_t()
+      :my( new fork_database_impl<BSP>() )
    {}
 
    template<class BSP>
    fork_database_t<BSP>::~fork_database_t() = default; // close is performed in fork_database::~fork_database()
 
    template<class BSP>
-   void fork_database_t<BSP>::open( const std::filesystem::path& fork_db_file, validator_t& validator ) {
+   void fork_database_t<BSP>::open( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ) {
       std::lock_guard g( my->mtx );
-      my->open_impl( fork_db_file, validator );
+      my->open_impl( fork_db_file, ds, validator );
    }
 
    template<class BSP>
-   void fork_database_impl<BSP>::open_impl( const std::filesystem::path& fork_db_file, validator_t& validator ) {
-      if( std::filesystem::exists( fork_db_file ) ) {
-         try {
-            string content;
-            fc::read_file_contents( fork_db_file, content );
-
-            fc::datastream<const char*> ds( content.data(), content.size() );
-
-            // validate totem
-            uint32_t totem = 0;
-            fc::raw::unpack( ds, totem );
-            EOS_ASSERT( totem == magic_number, fork_database_exception,
-                        "Fork database file '${filename}' has unexpected magic number: ${actual_totem}. Expected ${expected_totem}",
-                        ("filename", fork_db_file)("actual_totem", totem)("expected_totem", magic_number)
-            );
-
-            // validate version
-            uint32_t version = 0;
-            fc::raw::unpack( ds, version );
-            EOS_ASSERT( version >= fork_database::min_supported_version && version <= fork_database::max_supported_version,
-                        fork_database_exception,
-                       "Unsupported version of fork database file '${filename}'. "
-                       "Fork database version is ${version} while code supports version(s) [${min},${max}]",
-                       ("filename", fork_db_file)
-                       ("version", version)
-                       ("min", fork_database::min_supported_version)
-                       ("max", fork_database::max_supported_version)
-            );
-
-            bsp_t state = std::make_shared<bs_t>();
-            fc::raw::unpack( ds, *state );
-            reset_root_impl( state );
-
-            unsigned_int size; fc::raw::unpack( ds, size );
-            for( uint32_t i = 0, n = size.value; i < n; ++i ) {
-               bs_t s;
-               fc::raw::unpack( ds, s );
-               // do not populate transaction_metadatas, they will be created as needed in apply_block with appropriate key recovery
-               s.header_exts = s.block->validate_and_extract_header_extensions();
-               add_impl( std::make_shared<bs_t>( std::move( s ) ), mark_valid_t::no, ignore_duplicate_t::no, true, validator );
-            }
-            block_id_type head_id;
-            fc::raw::unpack( ds, head_id );
-
-            if( root->id() == head_id ) {
-               head = root;
-            } else {
-               head = get_block_impl( head_id );
-               EOS_ASSERT( head, fork_database_exception,
-                           "could not find head while reconstructing fork database from file; '${filename}' is likely corrupted",
-                           ("filename", fork_db_file) );
-            }
+   void fork_database_impl<BSP>::open_impl( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ) {
+      bsp_t _root = std::make_shared<bs_t>();
+      fc::raw::unpack( ds, *_root );
+      reset_root_impl( _root );
+
+      unsigned_int size; fc::raw::unpack( ds, size );
+      for( uint32_t i = 0, n = size.value; i < n; ++i ) {
+         bs_t s;
+         fc::raw::unpack( ds, s );
+         // do not populate transaction_metadatas, they will be created as needed in apply_block with appropriate key recovery
+         s.header_exts = s.block->validate_and_extract_header_extensions();
+         add_impl( std::make_shared<bs_t>( std::move( s ) ), mark_valid_t::no, ignore_duplicate_t::no, true, validator );
+      }
+      block_id_type head_id;
+      fc::raw::unpack( ds, head_id );
 
-            auto candidate = index.template get<by_best_branch>().begin();
-            if( candidate == index.template get<by_best_branch>().end() || !bs_accessor_t::is_valid(**candidate) ) {
-               EOS_ASSERT( head->id() == root->id(), fork_database_exception,
-                           "head not set to root despite no better option available; '${filename}' is likely corrupted",
-                           ("filename", fork_db_file) );
-            } else {
-               EOS_ASSERT( !first_preferred( **candidate, *head ), fork_database_exception,
-                           "head not set to best available option available; '${filename}' is likely corrupted",
-                           ("filename", fork_db_file) );
-            }
-         } FC_CAPTURE_AND_RETHROW( (fork_db_file) )
+      if( root->id() == head_id ) {
+         head = root;
+      } else {
+         head = get_block_impl( head_id );
+         EOS_ASSERT( head, fork_database_exception,
+                     "could not find head while reconstructing fork database from file; '${filename}' is likely corrupted",
+                     ("filename", fork_db_file) );
+      }
 
-         std::filesystem::remove( fork_db_file );
+      auto candidate = index.template get<by_best_branch>().begin();
+      if( candidate == index.template get<by_best_branch>().end() || !bs_accessor_t::is_valid(**candidate) ) {
+         EOS_ASSERT( head->id() == root->id(), fork_database_exception,
+                     "head not set to root despite no better option available; '${filename}' is likely corrupted",
+                     ("filename", fork_db_file) );
+      } else {
+         EOS_ASSERT( !first_preferred( **candidate, *head ), fork_database_exception,
+                     "head not set to best available option available; '${filename}' is likely corrupted",
+                     ("filename", fork_db_file) );
       }
    }
 
    template<class BSP>
-   void fork_database_t<BSP>::close(const std::filesystem::path& fork_db_file) {
+   void fork_database_t<BSP>::close(std::ofstream& out) {
       std::lock_guard g( my->mtx );
-      my->close_impl(fork_db_file);
+      my->close_impl(out);
    }
 
    template<class BSP>
-   void fork_database_impl<BSP>::close_impl(const std::filesystem::path& fork_db_file) {
-      if( !root ) {
-         if( index.size() > 0 ) {
-            elog( "fork_database is in a bad state when closing; not writing out '${filename}'",
-                  ("filename", fork_db_file) );
-         }
-         return;
-      }
-
-      std::ofstream out( fork_db_file.generic_string().c_str(), std::ios::out | std::ios::binary | std::ofstream::trunc );
-      fc::raw::pack( out, magic_number );
-      fc::raw::pack( out, fork_database::max_supported_version ); // write out current version which is always max_supported_version
+   void fork_database_impl<BSP>::close_impl(std::ofstream& out) {
       fc::raw::pack( out, *root );
+
       uint32_t num_blocks_in_fork_db = index.size();
       fc::raw::pack( out, unsigned_int{num_blocks_in_fork_db} );
 
@@ -288,12 +245,7 @@ namespace eosio::chain {
          fc::raw::pack( out, *(*itr) );
       }
 
-      if( head ) {
-         fc::raw::pack( out, head->id() );
-      } else {
-         elog( "head not set in fork database; '${filename}' will be corrupted",
-               ("filename", fork_db_file) );
-      }
+      fc::raw::pack( out, head ? head->id() : digest_type());
 
       index.clear();
    }
@@ -420,6 +372,11 @@ namespace eosio::chain {
       );
    }
 
+   template<class BSP>
+   bool fork_database_t<BSP>::in_valid_state() const {
+      return !!my->root;
+   }
+
    template<class BSP>
    bool fork_database_t<BSP>::has_root() const {
       return !!my->root;
@@ -703,7 +660,8 @@ namespace eosio::chain {
    fork_database::fork_database(const std::filesystem::path& data_dir)
       : data_dir(data_dir)
         // genesis starts with legacy
-      , fork_db_l{std::make_unique<fork_database_legacy_t>(fork_database_legacy_t::legacy_magic_number)}
+      , fork_db_l{std::make_unique<fork_database_legacy_t>()}
+      , fork_db_s{std::make_unique<fork_database_if_t>()}
    {
    }
 
@@ -712,7 +670,33 @@ namespace eosio::chain {
    }
 
    void fork_database::close() {
-      apply<void>([&](auto& forkdb) { forkdb.close(data_dir / config::forkdb_filename); });
+      auto fork_db_file {data_dir / config::forkdb_filename};
+      bool legacy_valid  = fork_db_l && fork_db_l->in_valid_state();
+      bool savanna_valid = fork_db_s && fork_db_s->in_valid_state();
+
+      // check that fork_dbs are in a consistent state
+      if (!legacy_valid && !savanna_valid) {
+         elog( "fork_database is in a bad state when closing; not writing out '${filename}'", ("filename", fork_db_file) );
+         return;
+      }
+
+      std::ofstream out( fork_db_file.generic_string().c_str(), std::ios::out | std::ios::binary | std::ofstream::trunc );
+
+      fc::raw::pack( out, magic_number );
+      fc::raw::pack( out, max_supported_version ); // write out current version which is always max_supported_version
+                                                   // version == 1 -> legacy
+                                                   // version == 1 -> savanna (two possible fork_db, one containing `block_state_legacy`,
+                                                   //                          one containing `block_state`)
+
+      fc::raw::pack(out, static_cast<uint32_t>(in_use.load()));
+
+      fc::raw::pack(out, legacy_valid);
+      if (legacy_valid)
+         fork_db_l->close(out);
+
+      fc::raw::pack(out, savanna_valid);
+      if (savanna_valid)
+         fork_db_s->close(out);
    }
 
    void fork_database::open( validator_t& validator ) {
@@ -731,41 +715,69 @@ namespace eosio::chain {
             // determine file type, validate totem
             uint32_t totem = 0;
             fc::raw::unpack( ds, totem );
-            EOS_ASSERT( totem == fork_database_legacy_t::legacy_magic_number ||
-                        totem == fork_database_if_t::magic_number, fork_database_exception,
-                        "Fork database file '${filename}' has unexpected magic number: ${actual_totem}. Expected ${t1} or ${t2}",
-                        ("filename", fork_db_file)
-                        ("actual_totem", totem)("t1", fork_database_legacy_t::legacy_magic_number)("t2", fork_database_if_t::magic_number)
-            );
-
-            if (totem == fork_database_legacy_t::legacy_magic_number) {
-               // fork_db_legacy created in constructor
-               apply_l<void>([&](auto& forkdb) {
-                  forkdb.open(fork_db_file, validator);
-               });
-            } else {
-               // file is instant-finality data, so switch to fork_database_if_t
-               fork_db_s = std::make_unique<fork_database_if_t>(fork_database_if_t::magic_number);
-               legacy = false;
-               apply_s<void>([&](auto& forkdb) {
-                  forkdb.open(fork_db_file, validator);
-               });
+            EOS_ASSERT( totem == magic_number, fork_database_exception,
+                        "Fork database file '${filename}' has unexpected magic number: ${actual_totem}. Expected ${t}",
+                        ("filename", fork_db_file)("actual_totem", totem)("t", magic_number));
+
+            uint32_t version = 0;
+            fc::raw::unpack( ds, version );
+            EOS_ASSERT( version >= fork_database::min_supported_version && version <= fork_database::max_supported_version,
+                        fork_database_exception,
+                       "Unsupported version of fork database file '${filename}'. "
+                       "Fork database version is ${version} while code supports version(s) [${min},${max}]",
+                       ("filename", fork_db_file)("version", version)("min", min_supported_version)("max", max_supported_version));
+
+            switch(version) {
+            case 1:
+            {
+               // ---------- pre-Savanna format. Just a single fork_database_l ----------------
+               in_use = in_use_t::legacy;
+               fork_db_l = std::make_unique<fork_database_legacy_t>();
+               fork_db_l->open(fork_db_file, ds, validator);
+               break;
+            }
+
+            case 2:
+            {
+               // ---------- Savanna format ----------------------------
+               uint32_t in_use_raw;
+               fc::raw::unpack( ds, in_use_raw );
+               in_use = static_cast<in_use_t>(in_use_raw);
+
+               bool legacy_valid { false };
+               fc::raw::unpack( ds, legacy_valid );
+               if (legacy_valid) {
+                  fork_db_l = std::make_unique<fork_database_legacy_t>();
+                  fork_db_l->open(fork_db_file, ds, validator);
+               }
+
+               bool savanna_valid { false };
+               fc::raw::unpack( ds, savanna_valid );
+               if (savanna_valid) {
+                  fork_db_s = std::make_unique<fork_database_if_t>();
+                  fork_db_s->open(fork_db_file, ds, validator);
+               }
+               break;
             }
-         } FC_CAPTURE_AND_RETHROW( (fork_db_file) )
+
+            default:
+               assert(0);
+               break;
+            }
+         } FC_CAPTURE_AND_RETHROW( (fork_db_file) );
+         std::filesystem::remove( fork_db_file );
       }
    }
 
    void fork_database::switch_from_legacy(const block_handle& bh) {
       // no need to close fork_db because we don't want to write anything out, file is removed on open
       // threads may be accessing (or locked on mutex about to access legacy forkdb) so don't delete it until program exit
-      assert(legacy);
+      assert(in_use == in_use_t::legacy);
       assert(std::holds_alternative<block_state_ptr>(bh.internal()));
       block_state_ptr new_head = std::get<block_state_ptr>(bh.internal());
-      fork_db_s = std::make_unique<fork_database_if_t>(fork_database_if_t::magic_number);
-      legacy = false;
-      apply_s<void>([&](auto& forkdb) {
-         forkdb.reset_root(new_head);
-      });
+      fork_db_s = std::make_unique<fork_database_if_t>();
+      in_use = in_use_t::savanna;
+      fork_db_s->reset_root(new_head);
    }
 
    block_branch_t fork_database::fetch_branch_from_head() const {
diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp
index 59486473d1..f25d6e0473 100644
--- a/libraries/chain/include/eosio/chain/fork_database.hpp
+++ b/libraries/chain/include/eosio/chain/fork_database.hpp
@@ -1,4 +1,5 @@
 #pragma once
+#include <fc/io/cfile.hpp>
 #include <eosio/chain/block_state_legacy.hpp>
 #include <eosio/chain/block_state.hpp>
 #include <eosio/chain/block_handle.hpp>
@@ -35,9 +36,6 @@ namespace eosio::chain {
    template<class BSP>  // either block_state_legacy_ptr or block_state_ptr
    class fork_database_t {
    public:
-      static constexpr uint32_t legacy_magic_number = 0x30510FDB;
-      static constexpr uint32_t magic_number = 0x4242FDB;
-
       using bsp_t            = BSP;
       using bs_t             = bsp_t::element_type;
       using bhsp_t           = bs_t::bhsp_t;
@@ -46,11 +44,11 @@ namespace eosio::chain {
       using full_branch_t    = std::vector<bhsp_t>;
       using branch_pair_t    = pair<branch_t, branch_t>;
 
-      explicit fork_database_t(uint32_t magic_number = legacy_magic_number);
+      explicit fork_database_t();
       ~fork_database_t();
 
-      void open( const std::filesystem::path& fork_db_file, validator_t& validator );
-      void close( const std::filesystem::path& fork_db_file );
+      void open( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator );
+      void close( std::ofstream& out );
 
       bsp_t get_block( const block_id_type& id, include_root_t include_root = include_root_t::no ) const;
       bool block_exists( const block_id_type& id ) const;
@@ -80,6 +78,8 @@ namespace eosio::chain {
 
       void remove( const block_id_type& id );
 
+      bool in_valid_state() const; // not thread safe
+
       bool has_root() const;
       bsp_t  root() const; // undefined if !has_root()
       bsp_t  head() const;
@@ -136,8 +136,12 @@ namespace eosio::chain {
     * All methods assert until open() is closed.
     */
    class fork_database {
+      static constexpr uint32_t magic_number = 0x30510FDB;
+
+      enum class in_use_t : uint32_t { legacy, savanna, both };
+
       const std::filesystem::path data_dir;
-      std::atomic<bool> legacy = true;
+      std::atomic<in_use_t> in_use = in_use_t::legacy;
       std::unique_ptr<fork_database_legacy_t> fork_db_l; // legacy
       std::unique_ptr<fork_database_if_t>     fork_db_s; // savanna
    public:
@@ -160,13 +164,13 @@ namespace eosio::chain {
       template <class R, class F>
       R apply(const F& f) {
          if constexpr (std::is_same_v<void, R>) {
-            if (legacy) {
+            if (in_use == in_use_t::legacy) {
                f(*fork_db_l);
             } else {
                f(*fork_db_s);
             }
          } else {
-            if (legacy) {
+            if (in_use == in_use_t::legacy) {
                return f(*fork_db_l);
             } else {
                return f(*fork_db_s);
@@ -177,13 +181,13 @@ namespace eosio::chain {
       template <class R, class F>
       R apply(const F& f) const {
          if constexpr (std::is_same_v<void, R>) {
-            if (legacy) {
+            if (in_use == in_use_t::legacy) {
                f(*fork_db_l);
             } else {
                f(*fork_db_s);
             }
          } else {
-            if (legacy) {
+            if (in_use == in_use_t::legacy) {
                return f(*fork_db_l);
             } else {
                return f(*fork_db_s);
@@ -195,11 +199,11 @@ namespace eosio::chain {
       template <class R, class F>
       R apply_s(const F& f) {
          if constexpr (std::is_same_v<void, R>) {
-            if (!legacy) {
+            if (in_use == in_use_t::savanna || in_use == in_use_t::both) {
                f(*fork_db_s);
             }
          } else {
-            if (!legacy) {
+            if (in_use == in_use_t::savanna || in_use == in_use_t::both) {
                return f(*fork_db_s);
             }
             return {};
@@ -210,11 +214,11 @@ namespace eosio::chain {
       template <class R, class F>
       R apply_l(const F& f) {
          if constexpr (std::is_same_v<void, R>) {
-            if (legacy) {
+            if (in_use == in_use_t::legacy || in_use == in_use_t::both) {
                f(*fork_db_l);
             }
          } else {
-            if (legacy) {
+            if (in_use == in_use_t::legacy || in_use == in_use_t::both) {
                return f(*fork_db_l);
             }
             return {};
@@ -226,13 +230,13 @@ namespace eosio::chain {
       template <class R, class LegacyF, class SavannaF>
       R apply(const LegacyF& legacy_f, const SavannaF& savanna_f) {
          if constexpr (std::is_same_v<void, R>) {
-            if (legacy) {
+            if (in_use == in_use_t::legacy) {
                legacy_f(*fork_db_l);
             } else {
                savanna_f(*fork_db_s);
             }
          } else {
-            if (legacy) {
+            if (in_use == in_use_t::legacy) {
                return legacy_f(*fork_db_l);
             } else {
                return savanna_f(*fork_db_s);
@@ -242,6 +246,6 @@ namespace eosio::chain {
 
       // if we ever support more than one version then need to save min/max in fork_database_t
       static constexpr uint32_t min_supported_version = 1;
-      static constexpr uint32_t max_supported_version = 1;
+      static constexpr uint32_t max_supported_version = 2;
    };
 } /// eosio::chain

From 009ff1486796f58be889f4fe02c643ea51f1a7d9 Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Wed, 13 Mar 2024 14:41:36 -0400
Subject: [PATCH 02/11] Fix issue when loading from snapshot post IF-transition

Now `ship_streamer_if_test` test passes.
---
 libraries/chain/controller.cpp                | 20 +++++++++++++------
 libraries/chain/fork_database.cpp             | 10 ++++++++++
 .../include/eosio/chain/block_handle.hpp      |  4 +++-
 .../include/eosio/chain/fork_database.hpp     |  2 ++
 tests/CMakeLists.txt                          |  4 ++--
 5 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp
index 140534ec72..aa5d66eb5d 100644
--- a/libraries/chain/controller.cpp
+++ b/libraries/chain/controller.cpp
@@ -139,6 +139,19 @@ R apply(const block_handle& bh, F&& f) {
                            }, bh.internal());
 }
 
+// apply methods of block_handle defined here as access to internal block_handle restricted to controller
+template <class R, class F_L, class F_S>
+   R apply(const block_handle& bh, F_L&& f_l, F_S&& f_s) {
+   if constexpr (std::is_same_v<void, R>)
+      std::visit<void>(overloaded{[&](const block_state_legacy_ptr& head) { std::forward<F_L>(f_l)(head); },
+                                  [&](const block_state_ptr& head)        { std::forward<F_S>(f_s)(head); }
+                       }, bh.internal());
+   else
+      return std::visit<R>(overloaded{[&](const block_state_legacy_ptr& head) -> R { return std::forward<F_L>(f_l)(head); },
+                                      [&](const block_state_ptr& head) -> R        { return std::forward<F_S>(f_s)(head); }
+                           }, bh.internal());
+}
+
 // apply savanna block_state
 template <class R, class F>
 R apply_s(const block_handle& bh, F&& f) {
@@ -1024,12 +1037,7 @@ struct controller_impl {
    }
 
    void fork_db_reset_root_to_chain_head() {
-      fork_db.apply<void>([&](auto& forkdb) {
-         apply<void>(chain_head, [&](const auto& head) {
-            if constexpr (std::is_same_v<std::decay_t<decltype(head)>, std::decay_t<decltype(forkdb.head())>>)
-               forkdb.reset_root(head);
-         });
-      });
+      fork_db.reset_root(chain_head.internal());
    }
 
    signed_block_ptr fork_db_fetch_block_by_id( const block_id_type& id ) const {
diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp
index a3769bce80..360381ef25 100644
--- a/libraries/chain/fork_database.cpp
+++ b/libraries/chain/fork_database.cpp
@@ -786,6 +786,16 @@ namespace eosio::chain {
       });
    }
 
+   void fork_database::reset_root(const block_handle::block_handle_variant_t& v) {
+       std::visit(overloaded{ [&](const block_state_legacy_ptr& bsp) { fork_db_l->reset_root(bsp); },
+                              [&](const block_state_ptr& bsp) {
+                                  if (in_use == in_use_t::legacy)
+                                     in_use = in_use_t::savanna;
+                                  fork_db_s->reset_root(bsp);
+                              } },
+                  v);
+   }
+
    // do class instantiations
    template class fork_database_t<block_state_legacy_ptr>;
    template class fork_database_t<block_state_ptr>;
diff --git a/libraries/chain/include/eosio/chain/block_handle.hpp b/libraries/chain/include/eosio/chain/block_handle.hpp
index 2e4ae5cbe8..961fc9010d 100644
--- a/libraries/chain/include/eosio/chain/block_handle.hpp
+++ b/libraries/chain/include/eosio/chain/block_handle.hpp
@@ -8,8 +8,10 @@ namespace eosio::chain {
 // Created via controller::create_block_handle(const block_id_type& id, const signed_block_ptr& b)
 // Valid to request id and signed_block_ptr it was created from.
 struct block_handle {
+   using block_handle_variant_t = std::variant<block_state_legacy_ptr, block_state_ptr>;
+
 private:
-   std::variant<block_state_legacy_ptr, block_state_ptr> _bsp;
+   block_handle_variant_t _bsp;
 
 public:
    block_handle() = default;
diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp
index 40ca9e1c33..0431c605e5 100644
--- a/libraries/chain/include/eosio/chain/fork_database.hpp
+++ b/libraries/chain/include/eosio/chain/fork_database.hpp
@@ -161,6 +161,8 @@ namespace eosio::chain {
       // see fork_database_t::fetch_branch(forkdb->head()->id())
       block_branch_t fetch_branch_from_head() const;
 
+      void reset_root(const block_handle::block_handle_variant_t& v);
+
       template <class R, class F>
       R apply(const F& f) const {
          if constexpr (std::is_same_v<void, R>) {
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index e82169eeee..b4945fa5af 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -151,8 +151,8 @@ set_property(TEST ship_if_test PROPERTY LABELS nonparallelizable_tests)
 add_test(NAME ship_streamer_test COMMAND tests/ship_streamer_test.py -v --num-clients 10 ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
 set_property(TEST ship_streamer_test PROPERTY LABELS long_running_tests)
 # uncomment after https://github.com/AntelopeIO/leap/issues/2285 implemented
-# add_test(NAME ship_streamer_if_test COMMAND tests/ship_streamer_test.py -v --num-clients 10 --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
-# set_property(TEST ship_streamer_if_test PROPERTY LABELS long_running_tests)
+add_test(NAME ship_streamer_if_test COMMAND tests/ship_streamer_test.py -v --num-clients 10 --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
+set_property(TEST ship_streamer_if_test PROPERTY LABELS long_running_tests)
 
 add_test(NAME p2p_dawn515_test COMMAND tests/p2p_tests/dawn_515/test.sh WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
 set_property(TEST p2p_dawn515_test PROPERTY LABELS nonparallelizable_tests)

From 8d4d5a081fa82a92f27ce6eaf422cab05e66d821 Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Wed, 13 Mar 2024 14:46:08 -0400
Subject: [PATCH 03/11] Update comment

---
 libraries/chain/controller.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp
index aa5d66eb5d..439c0d1cff 100644
--- a/libraries/chain/controller.cpp
+++ b/libraries/chain/controller.cpp
@@ -139,7 +139,7 @@ R apply(const block_handle& bh, F&& f) {
                            }, bh.internal());
 }
 
-// apply methods of block_handle defined here as access to internal block_handle restricted to controller
+// applies different lambdas, depending on whether `block_handle` holds a `block_state_legacy` or a `block_state`
 template <class R, class F_L, class F_S>
    R apply(const block_handle& bh, F_L&& f_l, F_S&& f_s) {
    if constexpr (std::is_same_v<void, R>)

From c03d87c277003586838808cd60a390169b3e134d Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Wed, 13 Mar 2024 15:59:03 -0400
Subject: [PATCH 04/11] Address PR comments.

---
 libraries/chain/fork_database.cpp | 3 ++-
 tests/CMakeLists.txt              | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp
index 360381ef25..148db0264b 100644
--- a/libraries/chain/fork_database.cpp
+++ b/libraries/chain/fork_database.cpp
@@ -676,7 +676,8 @@ namespace eosio::chain {
 
       // check that fork_dbs are in a consistent state
       if (!legacy_valid && !savanna_valid) {
-         elog( "fork_database is in a bad state when closing; not writing out '${filename}'", ("filename", fork_db_file) );
+         elog( "fork_database is in a bad state when closing; not writing out '${filename}', legacy_valid=${l}, savanna_valid=${s}",
+               ("filename", fork_db_file)("l", legacy_valid)("s", savanna_valid) );
          return;
       }
 
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index b4945fa5af..4a12e8a90b 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -150,7 +150,6 @@ set_property(TEST ship_if_test PROPERTY LABELS nonparallelizable_tests)
 
 add_test(NAME ship_streamer_test COMMAND tests/ship_streamer_test.py -v --num-clients 10 ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
 set_property(TEST ship_streamer_test PROPERTY LABELS long_running_tests)
-# uncomment after https://github.com/AntelopeIO/leap/issues/2285 implemented
 add_test(NAME ship_streamer_if_test COMMAND tests/ship_streamer_test.py -v --num-clients 10 --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
 set_property(TEST ship_streamer_if_test PROPERTY LABELS long_running_tests)
 

From c095bae2803c32703fcf4baf193b664007394f26 Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Thu, 14 Mar 2024 14:03:19 -0400
Subject: [PATCH 05/11] Address PR comments.

---
 libraries/chain/controller.cpp                | 19 ++---------
 libraries/chain/fork_database.cpp             | 23 +++++--------
 .../include/eosio/chain/block_handle.hpp      |  2 --
 .../chain/include/eosio/chain/block_state.hpp |  3 ++
 .../include/eosio/chain/fork_database.hpp     | 34 ++++++++++---------
 5 files changed, 33 insertions(+), 48 deletions(-)

diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp
index 439c0d1cff..583d46c2a3 100644
--- a/libraries/chain/controller.cpp
+++ b/libraries/chain/controller.cpp
@@ -139,19 +139,6 @@ R apply(const block_handle& bh, F&& f) {
                            }, bh.internal());
 }
 
-// applies different lambdas, depending on whether `block_handle` holds a `block_state_legacy` or a `block_state`
-template <class R, class F_L, class F_S>
-   R apply(const block_handle& bh, F_L&& f_l, F_S&& f_s) {
-   if constexpr (std::is_same_v<void, R>)
-      std::visit<void>(overloaded{[&](const block_state_legacy_ptr& head) { std::forward<F_L>(f_l)(head); },
-                                  [&](const block_state_ptr& head)        { std::forward<F_S>(f_s)(head); }
-                       }, bh.internal());
-   else
-      return std::visit<R>(overloaded{[&](const block_state_legacy_ptr& head) -> R { return std::forward<F_L>(f_l)(head); },
-                                      [&](const block_state_ptr& head) -> R        { return std::forward<F_S>(f_s)(head); }
-                           }, bh.internal());
-}
-
 // apply savanna block_state
 template <class R, class F>
 R apply_s(const block_handle& bh, F&& f) {
@@ -1414,7 +1401,7 @@ struct controller_impl {
          if (!fork_db.fork_db_if_present()) {
             // switch to savanna if needed
             apply_s<void>(chain_head, [&](const auto& head) {
-               fork_db.switch_from_legacy(chain_head);
+               fork_db.switch_from_legacy(chain_head.internal());
             });
          }
          auto do_startup = [&](auto& forkdb) {
@@ -2981,7 +2968,7 @@ struct controller_impl {
             auto new_head = std::make_shared<block_state>(*head);
             chain_head = block_handle{std::move(new_head)};
             if (s != controller::block_status::irreversible) {
-               fork_db.switch_from_legacy(chain_head);
+               fork_db.switch_from_legacy(chain_head.internal());
             }
          }
 
@@ -3692,7 +3679,7 @@ struct controller_impl {
          } else if( new_head->id() != chain_head.id() ) {
             ilog("switching forks from ${current_head_id} (block number ${current_head_num}) ${c} to ${new_head_id} (block number ${new_head_num}) ${n}",
                  ("current_head_id", chain_head.id())("current_head_num", chain_head.block_num())("new_head_id", new_head->id())("new_head_num", new_head->block_num())
-                 ("c", log_fork_comparison(chain_head))("n", log_fork_comparison(*new_head)));
+                 ("c", log_fork_comparison(chain_head.internal()))("n", log_fork_comparison(*new_head)));
 
             // not possible to log transaction specific infor when switching forks
             if (auto dm_logger = get_deep_mind_logger(false)) {
diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp
index 148db0264b..92cc424fc0 100644
--- a/libraries/chain/fork_database.cpp
+++ b/libraries/chain/fork_database.cpp
@@ -50,8 +50,8 @@ namespace eosio::chain {
       return r;
    }
 
-   std::string log_fork_comparison(const block_handle& bh) {
-      return std::visit([](const auto& bsp) { return log_fork_comparison(*bsp); }, bh.internal());
+   std::string log_fork_comparison(const block_handle_variant_t& bhv) {
+      return std::visit([](const auto& bsp) { return log_fork_comparison(*bsp); }, bhv);
    }
 
    struct by_block_id;
@@ -372,11 +372,6 @@ namespace eosio::chain {
       );
    }
 
-   template<class BSP>
-   bool fork_database_t<BSP>::in_valid_state() const {
-      return !!my->root;
-   }
-
    template<class BSP>
    bool fork_database_t<BSP>::has_root() const {
       return !!my->root;
@@ -671,8 +666,8 @@ namespace eosio::chain {
 
    void fork_database::close() {
       auto fork_db_file {data_dir / config::forkdb_filename};
-      bool legacy_valid  = fork_db_l && fork_db_l->in_valid_state();
-      bool savanna_valid = fork_db_s && fork_db_s->in_valid_state();
+      bool legacy_valid  = fork_db_l && fork_db_l->has_root();
+      bool savanna_valid = fork_db_s && fork_db_s->has_root();
 
       // check that fork_dbs are in a consistent state
       if (!legacy_valid && !savanna_valid) {
@@ -686,7 +681,7 @@ namespace eosio::chain {
       fc::raw::pack( out, magic_number );
       fc::raw::pack( out, max_supported_version ); // write out current version which is always max_supported_version
                                                    // version == 1 -> legacy
-                                                   // version == 1 -> savanna (two possible fork_db, one containing `block_state_legacy`,
+                                                   // version == 2 -> savanna (two possible fork_db, one containing `block_state_legacy`,
                                                    //                          one containing `block_state`)
 
       fc::raw::pack(out, static_cast<uint32_t>(in_use.load()));
@@ -770,12 +765,12 @@ namespace eosio::chain {
       }
    }
 
-   void fork_database::switch_from_legacy(const block_handle& bh) {
+   void fork_database::switch_from_legacy(const block_handle_variant_t& bhv) {
       // no need to close fork_db because we don't want to write anything out, file is removed on open
       // threads may be accessing (or locked on mutex about to access legacy forkdb) so don't delete it until program exit
       assert(in_use == in_use_t::legacy);
-      assert(std::holds_alternative<block_state_ptr>(bh.internal()));
-      block_state_ptr new_head = std::get<block_state_ptr>(bh.internal());
+      assert(std::holds_alternative<block_state_ptr>(bhv));
+      block_state_ptr new_head = std::get<block_state_ptr>(bhv);
       fork_db_s = std::make_unique<fork_database_if_t>();
       in_use = in_use_t::savanna;
       fork_db_s->reset_root(new_head);
@@ -787,7 +782,7 @@ namespace eosio::chain {
       });
    }
 
-   void fork_database::reset_root(const block_handle::block_handle_variant_t& v) {
+   void fork_database::reset_root(const block_handle_variant_t& v) {
        std::visit(overloaded{ [&](const block_state_legacy_ptr& bsp) { fork_db_l->reset_root(bsp); },
                               [&](const block_state_ptr& bsp) {
                                   if (in_use == in_use_t::legacy)
diff --git a/libraries/chain/include/eosio/chain/block_handle.hpp b/libraries/chain/include/eosio/chain/block_handle.hpp
index 961fc9010d..cb8f3646a4 100644
--- a/libraries/chain/include/eosio/chain/block_handle.hpp
+++ b/libraries/chain/include/eosio/chain/block_handle.hpp
@@ -8,8 +8,6 @@ namespace eosio::chain {
 // Created via controller::create_block_handle(const block_id_type& id, const signed_block_ptr& b)
 // Valid to request id and signed_block_ptr it was created from.
 struct block_handle {
-   using block_handle_variant_t = std::variant<block_state_legacy_ptr, block_state_ptr>;
-
 private:
    block_handle_variant_t _bsp;
 
diff --git a/libraries/chain/include/eosio/chain/block_state.hpp b/libraries/chain/include/eosio/chain/block_state.hpp
index 03433eb9b5..7585e6a365 100644
--- a/libraries/chain/include/eosio/chain/block_state.hpp
+++ b/libraries/chain/include/eosio/chain/block_state.hpp
@@ -92,6 +92,9 @@ struct block_state : public block_header_state {     // block_header_state provi
 };
 
 using block_state_ptr = std::shared_ptr<block_state>;
+
+using block_handle_variant_t = std::variant<std::shared_ptr<block_state_legacy>, block_state_ptr>;
+
    
 } // namespace eosio::chain
 
diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp
index 0431c605e5..8fc360e51b 100644
--- a/libraries/chain/include/eosio/chain/fork_database.hpp
+++ b/libraries/chain/include/eosio/chain/fork_database.hpp
@@ -1,8 +1,8 @@
 #pragma once
-#include <fc/io/cfile.hpp>
 #include <eosio/chain/block_state_legacy.hpp>
 #include <eosio/chain/block_state.hpp>
-#include <eosio/chain/block_handle.hpp>
+
+namespace fc { class cfile_datastream; } // forward decl
 
 namespace eosio::chain {
 
@@ -17,7 +17,7 @@ namespace eosio::chain {
    // Used for logging of comparison values used for best fork determination
    std::string log_fork_comparison(const block_state& bs);
    std::string log_fork_comparison(const block_state_legacy& bs);
-   std::string log_fork_comparison(const block_handle& bh);
+   std::string log_fork_comparison(const block_handle_variant_t& bh);
 
    /**
     * @class fork_database_t
@@ -78,8 +78,6 @@ namespace eosio::chain {
 
       void remove( const block_id_type& id );
 
-      bool in_valid_state() const; // not thread safe
-
       bool has_root() const;
       bsp_t  root() const; // undefined if !has_root()
       bsp_t  head() const;
@@ -153,7 +151,7 @@ namespace eosio::chain {
       void close();
 
       // expected to be called from main thread
-      void switch_from_legacy(const block_handle& bh);
+      void switch_from_legacy(const block_handle_variant_t& bhv);
 
       bool fork_db_if_present() const { return !!fork_db_s; }
       bool fork_db_legacy_present() const { return !!fork_db_l; }
@@ -161,18 +159,18 @@ namespace eosio::chain {
       // see fork_database_t::fetch_branch(forkdb->head()->id())
       block_branch_t fetch_branch_from_head() const;
 
-      void reset_root(const block_handle::block_handle_variant_t& v);
+      void reset_root(const block_handle_variant_t& v);
 
       template <class R, class F>
       R apply(const F& f) const {
          if constexpr (std::is_same_v<void, R>) {
-            if (in_use == in_use_t::legacy) {
+            if (in_use.load(std::memory_order_relaxed) == in_use_t::legacy) {
                f(*fork_db_l);
             } else {
                f(*fork_db_s);
             }
          } else {
-            if (in_use == in_use_t::legacy) {
+            if (in_use.load(std::memory_order_relaxed) == in_use_t::legacy) {
                return f(*fork_db_l);
             } else {
                return f(*fork_db_s);
@@ -184,11 +182,13 @@ namespace eosio::chain {
       template <class R, class F>
       R apply_s(const F& f) {
          if constexpr (std::is_same_v<void, R>) {
-            if (in_use == in_use_t::savanna || in_use == in_use_t::both) {
+            if (auto in_use_value = in_use.load(std::memory_order_relaxed);
+                in_use_value == in_use_t::savanna || in_use_value == in_use_t::both) {
                f(*fork_db_s);
             }
          } else {
-            if (in_use == in_use_t::savanna || in_use == in_use_t::both) {
+            if (auto in_use_value = in_use.load(std::memory_order_relaxed);
+                in_use_value == in_use_t::savanna || in_use_value == in_use_t::both) {
                return f(*fork_db_s);
             }
             return {};
@@ -199,11 +199,13 @@ namespace eosio::chain {
       template <class R, class F>
       R apply_l(const F& f) {
          if constexpr (std::is_same_v<void, R>) {
-            if (in_use == in_use_t::legacy || in_use == in_use_t::both) {
+            if (auto in_use_value = in_use.load(std::memory_order_relaxed);
+                in_use_value == in_use_t::legacy || in_use_value == in_use_t::both) {
                f(*fork_db_l);
             }
          } else {
-            if (in_use == in_use_t::legacy || in_use == in_use_t::both) {
+            if (auto in_use_value = in_use.load(std::memory_order_relaxed);
+                in_use_value == in_use_t::legacy || in_use_value == in_use_t::both) {
                return f(*fork_db_l);
             }
             return {};
@@ -215,13 +217,13 @@ namespace eosio::chain {
       template <class R, class LegacyF, class SavannaF>
       R apply(const LegacyF& legacy_f, const SavannaF& savanna_f) {
          if constexpr (std::is_same_v<void, R>) {
-            if (in_use == in_use_t::legacy) {
+            if (in_use.load(std::memory_order_relaxed) == in_use_t::legacy) {
                legacy_f(*fork_db_l);
             } else {
                savanna_f(*fork_db_s);
             }
          } else {
-            if (in_use == in_use_t::legacy) {
+            if (in_use.load(std::memory_order_relaxed) == in_use_t::legacy) {
                return legacy_f(*fork_db_l);
             } else {
                return savanna_f(*fork_db_s);
@@ -229,7 +231,7 @@ namespace eosio::chain {
          }
       }
 
-      // if we ever support more than one version then need to save min/max in fork_database_t
+      // Update max_supported_version if the persistent file format changes.
       static constexpr uint32_t min_supported_version = 1;
       static constexpr uint32_t max_supported_version = 2;
    };

From 647e35d62459b2c43fd53514101ba2f2ee70a2e9 Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Thu, 14 Mar 2024 14:13:01 -0400
Subject: [PATCH 06/11] Rename `block_handle_variant_t&` to
 `block_state_variant_t&`

---
 libraries/chain/fork_database.cpp                     | 6 +++---
 libraries/chain/include/eosio/chain/block_handle.hpp  | 2 +-
 libraries/chain/include/eosio/chain/block_state.hpp   | 2 +-
 libraries/chain/include/eosio/chain/fork_database.hpp | 6 +++---
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp
index 92cc424fc0..c8f09d09b0 100644
--- a/libraries/chain/fork_database.cpp
+++ b/libraries/chain/fork_database.cpp
@@ -50,7 +50,7 @@ namespace eosio::chain {
       return r;
    }
 
-   std::string log_fork_comparison(const block_handle_variant_t& bhv) {
+   std::string log_fork_comparison(const block_state_variant_t& bhv) {
       return std::visit([](const auto& bsp) { return log_fork_comparison(*bsp); }, bhv);
    }
 
@@ -765,7 +765,7 @@ namespace eosio::chain {
       }
    }
 
-   void fork_database::switch_from_legacy(const block_handle_variant_t& bhv) {
+   void fork_database::switch_from_legacy(const block_state_variant_t& bhv) {
       // no need to close fork_db because we don't want to write anything out, file is removed on open
       // threads may be accessing (or locked on mutex about to access legacy forkdb) so don't delete it until program exit
       assert(in_use == in_use_t::legacy);
@@ -782,7 +782,7 @@ namespace eosio::chain {
       });
    }
 
-   void fork_database::reset_root(const block_handle_variant_t& v) {
+   void fork_database::reset_root(const block_state_variant_t& v) {
        std::visit(overloaded{ [&](const block_state_legacy_ptr& bsp) { fork_db_l->reset_root(bsp); },
                               [&](const block_state_ptr& bsp) {
                                   if (in_use == in_use_t::legacy)
diff --git a/libraries/chain/include/eosio/chain/block_handle.hpp b/libraries/chain/include/eosio/chain/block_handle.hpp
index cb8f3646a4..8cd7af9cd9 100644
--- a/libraries/chain/include/eosio/chain/block_handle.hpp
+++ b/libraries/chain/include/eosio/chain/block_handle.hpp
@@ -9,7 +9,7 @@ namespace eosio::chain {
 // Valid to request id and signed_block_ptr it was created from.
 struct block_handle {
 private:
-   block_handle_variant_t _bsp;
+   block_state_variant_t _bsp;
 
 public:
    block_handle() = default;
diff --git a/libraries/chain/include/eosio/chain/block_state.hpp b/libraries/chain/include/eosio/chain/block_state.hpp
index 092d2eb2fa..27821fca42 100644
--- a/libraries/chain/include/eosio/chain/block_state.hpp
+++ b/libraries/chain/include/eosio/chain/block_state.hpp
@@ -142,7 +142,7 @@ struct block_state : public block_header_state {     // block_header_state provi
 
 using block_state_ptr = std::shared_ptr<block_state>;
 
-using block_handle_variant_t = std::variant<std::shared_ptr<block_state_legacy>, block_state_ptr>;
+using block_state_variant_t = std::variant<std::shared_ptr<block_state_legacy>, block_state_ptr>;
 
    
 } // namespace eosio::chain
diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp
index 8fc360e51b..189410d8cb 100644
--- a/libraries/chain/include/eosio/chain/fork_database.hpp
+++ b/libraries/chain/include/eosio/chain/fork_database.hpp
@@ -17,7 +17,7 @@ namespace eosio::chain {
    // Used for logging of comparison values used for best fork determination
    std::string log_fork_comparison(const block_state& bs);
    std::string log_fork_comparison(const block_state_legacy& bs);
-   std::string log_fork_comparison(const block_handle_variant_t& bh);
+   std::string log_fork_comparison(const block_state_variant_t& bh);
 
    /**
     * @class fork_database_t
@@ -151,7 +151,7 @@ namespace eosio::chain {
       void close();
 
       // expected to be called from main thread
-      void switch_from_legacy(const block_handle_variant_t& bhv);
+      void switch_from_legacy(const block_state_variant_t& bhv);
 
       bool fork_db_if_present() const { return !!fork_db_s; }
       bool fork_db_legacy_present() const { return !!fork_db_l; }
@@ -159,7 +159,7 @@ namespace eosio::chain {
       // see fork_database_t::fetch_branch(forkdb->head()->id())
       block_branch_t fetch_branch_from_head() const;
 
-      void reset_root(const block_handle_variant_t& v);
+      void reset_root(const block_state_variant_t& v);
 
       template <class R, class F>
       R apply(const F& f) const {

From 06329b41541ada95f2bd6b7855eb5e88828f9e0d Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Thu, 14 Mar 2024 21:23:23 -0400
Subject: [PATCH 07/11] remove `std::memory_order_relaxed`.

---
 .../chain/include/eosio/chain/fork_database.hpp  | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp
index 189410d8cb..bb4a9b9c33 100644
--- a/libraries/chain/include/eosio/chain/fork_database.hpp
+++ b/libraries/chain/include/eosio/chain/fork_database.hpp
@@ -164,13 +164,13 @@ namespace eosio::chain {
       template <class R, class F>
       R apply(const F& f) const {
          if constexpr (std::is_same_v<void, R>) {
-            if (in_use.load(std::memory_order_relaxed) == in_use_t::legacy) {
+            if (in_use.load() == in_use_t::legacy) {
                f(*fork_db_l);
             } else {
                f(*fork_db_s);
             }
          } else {
-            if (in_use.load(std::memory_order_relaxed) == in_use_t::legacy) {
+            if (in_use.load() == in_use_t::legacy) {
                return f(*fork_db_l);
             } else {
                return f(*fork_db_s);
@@ -182,12 +182,12 @@ namespace eosio::chain {
       template <class R, class F>
       R apply_s(const F& f) {
          if constexpr (std::is_same_v<void, R>) {
-            if (auto in_use_value = in_use.load(std::memory_order_relaxed);
+            if (auto in_use_value = in_use.load();
                 in_use_value == in_use_t::savanna || in_use_value == in_use_t::both) {
                f(*fork_db_s);
             }
          } else {
-            if (auto in_use_value = in_use.load(std::memory_order_relaxed);
+            if (auto in_use_value = in_use.load();
                 in_use_value == in_use_t::savanna || in_use_value == in_use_t::both) {
                return f(*fork_db_s);
             }
@@ -199,12 +199,12 @@ namespace eosio::chain {
       template <class R, class F>
       R apply_l(const F& f) {
          if constexpr (std::is_same_v<void, R>) {
-            if (auto in_use_value = in_use.load(std::memory_order_relaxed);
+            if (auto in_use_value = in_use.load();
                 in_use_value == in_use_t::legacy || in_use_value == in_use_t::both) {
                f(*fork_db_l);
             }
          } else {
-            if (auto in_use_value = in_use.load(std::memory_order_relaxed);
+            if (auto in_use_value = in_use.load();
                 in_use_value == in_use_t::legacy || in_use_value == in_use_t::both) {
                return f(*fork_db_l);
             }
@@ -217,13 +217,13 @@ namespace eosio::chain {
       template <class R, class LegacyF, class SavannaF>
       R apply(const LegacyF& legacy_f, const SavannaF& savanna_f) {
          if constexpr (std::is_same_v<void, R>) {
-            if (in_use.load(std::memory_order_relaxed) == in_use_t::legacy) {
+            if (in_use.load() == in_use_t::legacy) {
                legacy_f(*fork_db_l);
             } else {
                savanna_f(*fork_db_s);
             }
          } else {
-            if (in_use.load(std::memory_order_relaxed) == in_use_t::legacy) {
+            if (in_use.load() == in_use_t::legacy) {
                return legacy_f(*fork_db_l);
             } else {
                return savanna_f(*fork_db_s);

From d57644d1ef2ad1ba194245e215ca7093b02dcef9 Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Fri, 15 Mar 2024 09:50:03 -0400
Subject: [PATCH 08/11] Don't save a fork_db with an undefined head.

---
 libraries/chain/fork_database.cpp | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp
index c8f09d09b0..a8ece050a5 100644
--- a/libraries/chain/fork_database.cpp
+++ b/libraries/chain/fork_database.cpp
@@ -180,8 +180,11 @@ namespace eosio::chain {
          head = root;
       } else {
          head = get_block_impl( head_id );
+         if (!head)
+            std::filesystem::remove( fork_db_file );
          EOS_ASSERT( head, fork_database_exception,
-                     "could not find head while reconstructing fork database from file; '${filename}' is likely corrupted",
+                     "could not find head while reconstructing fork database from file; "
+                     "'${filename}' is likely corrupted and has been removed",
                      ("filename", fork_db_file) );
       }
 
@@ -205,6 +208,8 @@ namespace eosio::chain {
 
    template<class BSP>
    void fork_database_impl<BSP>::close_impl(std::ofstream& out) {
+      assert(!!head && !!root); // if head or root are null, we don't save and shouldn't get here
+
       fc::raw::pack( out, *root );
 
       uint32_t num_blocks_in_fork_db = index.size();
@@ -245,7 +250,7 @@ namespace eosio::chain {
          fc::raw::pack( out, *(*itr) );
       }
 
-      fc::raw::pack( out, head ? head->id() : digest_type());
+      fc::raw::pack( out, head->id() );
 
       index.clear();
    }
@@ -666,8 +671,8 @@ namespace eosio::chain {
 
    void fork_database::close() {
       auto fork_db_file {data_dir / config::forkdb_filename};
-      bool legacy_valid  = fork_db_l && fork_db_l->has_root();
-      bool savanna_valid = fork_db_s && fork_db_s->has_root();
+      bool legacy_valid  = fork_db_l && fork_db_l->has_root() && !!fork_db_l->head();
+      bool savanna_valid = fork_db_s && fork_db_s->has_root() && !!fork_db_s->head();
 
       // check that fork_dbs are in a consistent state
       if (!legacy_valid && !savanna_valid) {

From 3385dca9d546738b28abcd27877c65b555689991 Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Fri, 15 Mar 2024 10:01:33 -0400
Subject: [PATCH 09/11] Make sure `head` is in the index before saving (since
 we serialize only its `id`)

---
 libraries/chain/fork_database.cpp                    | 12 ++++++++++++
 .../chain/include/eosio/chain/fork_database.hpp      |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp
index a8ece050a5..8715235937 100644
--- a/libraries/chain/fork_database.cpp
+++ b/libraries/chain/fork_database.cpp
@@ -128,6 +128,7 @@ namespace eosio::chain {
       void             open_impl( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator );
       void             close_impl( std::ofstream& out );
       void             add_impl( const bsp_t& n, mark_valid_t mark_valid, ignore_duplicate_t ignore_duplicate, bool validate, validator_t& validator );
+      bool             is_valid() const;
 
       bsp_t            get_block_impl( const block_id_type& id, include_root_t include_root = include_root_t::no ) const;
       bool             block_exists_impl( const block_id_type& id ) const;
@@ -377,6 +378,17 @@ namespace eosio::chain {
       );
    }
 
+   template<class BSP>
+   bool fork_database_t<BSP>::is_valid() const {
+      std::lock_guard g( my->mtx );
+      return my->is_valid();
+   }
+
+   template<class BSP>
+   bool fork_database_impl<BSP>::is_valid() const {
+      return !!root && !!head && get_block_impl(head->id());
+   }
+
    template<class BSP>
    bool fork_database_t<BSP>::has_root() const {
       return !!my->root;
diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp
index bb4a9b9c33..1f22f3886d 100644
--- a/libraries/chain/include/eosio/chain/fork_database.hpp
+++ b/libraries/chain/include/eosio/chain/fork_database.hpp
@@ -78,6 +78,8 @@ namespace eosio::chain {
 
       void remove( const block_id_type& id );
 
+      bool is_valid() const; // sanity checks on this fork_db
+
       bool has_root() const;
       bsp_t  root() const; // undefined if !has_root()
       bsp_t  head() const;

From 5d57dcb1f3ba9a718d050a9c47b3e8cb16c7cfb2 Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Fri, 15 Mar 2024 10:37:52 -0400
Subject: [PATCH 10/11] Make `fork_db_l` and `fork_db_s` direct members and not
 pointers.

---
 libraries/chain/controller.cpp                |  6 +-
 libraries/chain/fork_database.cpp             | 32 +++++------
 .../include/eosio/chain/fork_database.hpp     | 57 ++++++++++++-------
 3 files changed, 55 insertions(+), 40 deletions(-)

diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp
index 9758769d77..6c52682d63 100644
--- a/libraries/chain/controller.cpp
+++ b/libraries/chain/controller.cpp
@@ -1433,7 +1433,7 @@ struct controller_impl {
       }
 
       if (startup == startup_t::genesis) {
-         if (!fork_db.fork_db_if_present()) {
+         if (fork_db.version_in_use() == fork_database::in_use_t::legacy) {
             // switch to savanna if needed
             apply_s<void>(chain_head, [&](const auto& head) {
                fork_db.switch_from_legacy(chain_head.internal());
@@ -1689,12 +1689,12 @@ struct controller_impl {
       // If we start at a block during or after the IF transition, we need to provide this information
       // at startup.
       // ---------------------------------------------------------------------------------------------
-      if (fork_db.fork_db_if_present()) {
+      if (auto in_use = fork_db.version_in_use(); in_use  == fork_database::in_use_t::both || in_use  == fork_database::in_use_t::savanna) {
          // we are already past the IF transition point where we create the updated fork_db.
          // so we can't rely on the finalizer safety information update happening during the transition.
          // see https://github.com/AntelopeIO/leap/issues/2070#issuecomment-1941901836
          // -------------------------------------------------------------------------------------------
-         if (fork_db.fork_db_legacy_present()) {
+         if (in_use  == fork_database::in_use_t::both) {
             // fork_db_legacy is present as well, which means that we have not completed the transition
             auto set_finalizer_defaults = [&](auto& forkdb) -> void {
                auto lib = forkdb.root();
diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp
index 8715235937..1e9055c8c2 100644
--- a/libraries/chain/fork_database.cpp
+++ b/libraries/chain/fork_database.cpp
@@ -386,7 +386,7 @@ namespace eosio::chain {
 
    template<class BSP>
    bool fork_database_impl<BSP>::is_valid() const {
-      return !!root && !!head && get_block_impl(head->id());
+      return !!root && !!head; // && get_block_impl(head->id());
    }
 
    template<class BSP>
@@ -671,9 +671,6 @@ namespace eosio::chain {
 
    fork_database::fork_database(const std::filesystem::path& data_dir)
       : data_dir(data_dir)
-        // genesis starts with legacy
-      , fork_db_l{std::make_unique<fork_database_legacy_t>()}
-      , fork_db_s{std::make_unique<fork_database_if_t>()}
    {
    }
 
@@ -683,8 +680,8 @@ namespace eosio::chain {
 
    void fork_database::close() {
       auto fork_db_file {data_dir / config::forkdb_filename};
-      bool legacy_valid  = fork_db_l && fork_db_l->has_root() && !!fork_db_l->head();
-      bool savanna_valid = fork_db_s && fork_db_s->has_root() && !!fork_db_s->head();
+      bool legacy_valid  = fork_db_l.is_valid();
+      bool savanna_valid = fork_db_s.is_valid();
 
       // check that fork_dbs are in a consistent state
       if (!legacy_valid && !savanna_valid) {
@@ -705,17 +702,19 @@ namespace eosio::chain {
 
       fc::raw::pack(out, legacy_valid);
       if (legacy_valid)
-         fork_db_l->close(out);
+         fork_db_l.close(out);
 
       fc::raw::pack(out, savanna_valid);
       if (savanna_valid)
-         fork_db_s->close(out);
+         fork_db_s.close(out);
    }
 
    void fork_database::open( validator_t& validator ) {
       if (!std::filesystem::is_directory(data_dir))
          std::filesystem::create_directories(data_dir);
 
+      assert(!fork_db_l.is_valid() && !fork_db_s.is_valid());
+
       auto fork_db_file = data_dir / config::forkdb_filename;
       if( std::filesystem::exists( fork_db_file ) ) {
          try {
@@ -745,8 +744,7 @@ namespace eosio::chain {
             {
                // ---------- pre-Savanna format. Just a single fork_database_l ----------------
                in_use = in_use_t::legacy;
-               fork_db_l = std::make_unique<fork_database_legacy_t>();
-               fork_db_l->open(fork_db_file, ds, validator);
+               fork_db_l.open(fork_db_file, ds, validator);
                break;
             }
 
@@ -760,15 +758,13 @@ namespace eosio::chain {
                bool legacy_valid { false };
                fc::raw::unpack( ds, legacy_valid );
                if (legacy_valid) {
-                  fork_db_l = std::make_unique<fork_database_legacy_t>();
-                  fork_db_l->open(fork_db_file, ds, validator);
+                  fork_db_l.open(fork_db_file, ds, validator);
                }
 
                bool savanna_valid { false };
                fc::raw::unpack( ds, savanna_valid );
                if (savanna_valid) {
-                  fork_db_s = std::make_unique<fork_database_if_t>();
-                  fork_db_s->open(fork_db_file, ds, validator);
+                  fork_db_s.open(fork_db_file, ds, validator);
                }
                break;
             }
@@ -788,9 +784,9 @@ namespace eosio::chain {
       assert(in_use == in_use_t::legacy);
       assert(std::holds_alternative<block_state_ptr>(bhv));
       block_state_ptr new_head = std::get<block_state_ptr>(bhv);
-      fork_db_s = std::make_unique<fork_database_if_t>();
+      assert(!fork_db_s.is_valid());
       in_use = in_use_t::savanna;
-      fork_db_s->reset_root(new_head);
+      fork_db_s.reset_root(new_head);
    }
 
    block_branch_t fork_database::fetch_branch_from_head() const {
@@ -800,11 +796,11 @@ namespace eosio::chain {
    }
 
    void fork_database::reset_root(const block_state_variant_t& v) {
-       std::visit(overloaded{ [&](const block_state_legacy_ptr& bsp) { fork_db_l->reset_root(bsp); },
+       std::visit(overloaded{ [&](const block_state_legacy_ptr& bsp) { fork_db_l.reset_root(bsp); },
                               [&](const block_state_ptr& bsp) {
                                   if (in_use == in_use_t::legacy)
                                      in_use = in_use_t::savanna;
-                                  fork_db_s->reset_root(bsp);
+                                  fork_db_s.reset_root(bsp);
                               } },
                   v);
    }
diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp
index 1f22f3886d..07118e06c4 100644
--- a/libraries/chain/include/eosio/chain/fork_database.hpp
+++ b/libraries/chain/include/eosio/chain/fork_database.hpp
@@ -136,14 +136,17 @@ namespace eosio::chain {
     * All methods assert until open() is closed.
     */
    class fork_database {
-      static constexpr uint32_t magic_number = 0x30510FDB;
-
+   public:
       enum class in_use_t : uint32_t { legacy, savanna, both };
 
+   private:
+      static constexpr uint32_t magic_number = 0x30510FDB;
+
       const std::filesystem::path data_dir;
-      std::atomic<in_use_t> in_use = in_use_t::legacy;
-      std::unique_ptr<fork_database_legacy_t> fork_db_l; // legacy
-      std::unique_ptr<fork_database_if_t>     fork_db_s; // savanna
+      std::atomic<in_use_t>  in_use = in_use_t::legacy;
+      fork_database_legacy_t fork_db_l; // legacy
+      fork_database_if_t     fork_db_s; // savanna
+
    public:
       explicit fork_database(const std::filesystem::path& data_dir);
       ~fork_database(); // close on destruction
@@ -155,8 +158,7 @@ namespace eosio::chain {
       // expected to be called from main thread
       void switch_from_legacy(const block_state_variant_t& bhv);
 
-      bool fork_db_if_present() const { return !!fork_db_s; }
-      bool fork_db_legacy_present() const { return !!fork_db_l; }
+      in_use_t version_in_use() const { return in_use.load(); }
 
       // see fork_database_t::fetch_branch(forkdb->head()->id())
       block_branch_t fetch_branch_from_head() const;
@@ -167,15 +169,32 @@ namespace eosio::chain {
       R apply(const F& f) const {
          if constexpr (std::is_same_v<void, R>) {
             if (in_use.load() == in_use_t::legacy) {
-               f(*fork_db_l);
+               f(fork_db_l);
+            } else {
+               f(fork_db_s);
+            }
+         } else {
+            if (in_use.load() == in_use_t::legacy) {
+               return f(fork_db_l);
+            } else {
+               return f(fork_db_s);
+            }
+         }
+      }
+
+      template <class R, class F>
+      R apply(const F& f) {
+         if constexpr (std::is_same_v<void, R>) {
+            if (in_use.load() == in_use_t::legacy) {
+               f(fork_db_l);
             } else {
-               f(*fork_db_s);
+               f(fork_db_s);
             }
          } else {
             if (in_use.load() == in_use_t::legacy) {
-               return f(*fork_db_l);
+               return f(fork_db_l);
             } else {
-               return f(*fork_db_s);
+               return f(fork_db_s);
             }
          }
       }
@@ -186,12 +205,12 @@ namespace eosio::chain {
          if constexpr (std::is_same_v<void, R>) {
             if (auto in_use_value = in_use.load();
                 in_use_value == in_use_t::savanna || in_use_value == in_use_t::both) {
-               f(*fork_db_s);
+               f(fork_db_s);
             }
          } else {
             if (auto in_use_value = in_use.load();
                 in_use_value == in_use_t::savanna || in_use_value == in_use_t::both) {
-               return f(*fork_db_s);
+               return f(fork_db_s);
             }
             return {};
          }
@@ -203,12 +222,12 @@ namespace eosio::chain {
          if constexpr (std::is_same_v<void, R>) {
             if (auto in_use_value = in_use.load();
                 in_use_value == in_use_t::legacy || in_use_value == in_use_t::both) {
-               f(*fork_db_l);
+               f(fork_db_l);
             }
          } else {
             if (auto in_use_value = in_use.load();
                 in_use_value == in_use_t::legacy || in_use_value == in_use_t::both) {
-               return f(*fork_db_l);
+               return f(fork_db_l);
             }
             return {};
          }
@@ -220,15 +239,15 @@ namespace eosio::chain {
       R apply(const LegacyF& legacy_f, const SavannaF& savanna_f) {
          if constexpr (std::is_same_v<void, R>) {
             if (in_use.load() == in_use_t::legacy) {
-               legacy_f(*fork_db_l);
+               legacy_f(fork_db_l);
             } else {
-               savanna_f(*fork_db_s);
+               savanna_f(fork_db_s);
             }
          } else {
             if (in_use.load() == in_use_t::legacy) {
-               return legacy_f(*fork_db_l);
+               return legacy_f(fork_db_l);
             } else {
-               return savanna_f(*fork_db_s);
+               return savanna_f(fork_db_s);
             }
          }
       }

From af8d8e6c3343388f40aedf73a014dc9a4668fb2b Mon Sep 17 00:00:00 2001
From: greg7mdp <greg7mdp@gmail.com>
Date: Fri, 15 Mar 2024 10:59:51 -0400
Subject: [PATCH 11/11] Fix `fork_database::is_valid`

---
 libraries/chain/fork_database.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp
index 1e9055c8c2..4fa621f144 100644
--- a/libraries/chain/fork_database.cpp
+++ b/libraries/chain/fork_database.cpp
@@ -386,7 +386,7 @@ namespace eosio::chain {
 
    template<class BSP>
    bool fork_database_impl<BSP>::is_valid() const {
-      return !!root && !!head; // && get_block_impl(head->id());
+      return !!root && !!head && (root->id() == head->id() || get_block_impl(head->id()));
    }
 
    template<class BSP>