[1.0.2] Don't assume that claimed block is in fork_db.

libraries/chain/controller.cpp

@@ -1131,8 +1131,19 @@ struct controller_impl {
    }
 
    bool fork_db_validated_block_exists( const block_id_type& id ) const {
+       return fork_db.apply<bool>([&](const auto& forkdb) {
+         auto bsp = forkdb.get_block(id);
+         return bsp ? bsp->is_valid() : false;
+      });
+   }
+
+   // prerequisite: claimed_id is either id, or an ancestor of id
+   // returns true if block `id`, or one of its ancestors not older than claimed_id, is found in fork_db
+   // and `is_valid()`
+   // ------------------------------------------------------------------------------------------------------
+   bool fork_db_validated_block_exists( const block_id_type& id, const block_id_type& claimed_id ) const {
       return fork_db.apply<bool>([&](const auto& forkdb) {
-         return forkdb.validated_block_exists(id);
+         return forkdb.validated_block_exists(id, claimed_id);
       });
    }
 
@@ -4241,14 +4252,14 @@ struct controller_impl {
             if (use_thread_pool == use_thread_pool_t::yes && async_voting == async_t::yes) {
                boost::asio::post(thread_pool.get_executor(), [this, bsp=bsp]() {
                   const auto& latest_qc_claim__block_ref = bsp->core.get_block_reference(bsp->core.latest_qc_claim().block_num);
-                  if (fork_db_validated_block_exists(latest_qc_claim__block_ref.block_id)) {
+                  if (fork_db_validated_block_exists(bsp->previous(), latest_qc_claim__block_ref.block_id)) {
                      create_and_send_vote_msg(bsp);
                   }
                });
             } else {
                // bsp can be used directly instead of copy needed for post
                const auto& latest_qc_claim__block_ref = bsp->core.get_block_reference(bsp->core.latest_qc_claim().block_num);
-               if (fork_db_validated_block_exists(latest_qc_claim__block_ref.block_id)) {
+               if (fork_db_validated_block_exists(bsp->previous(), latest_qc_claim__block_ref.block_id)) {
                   create_and_send_vote_msg(bsp);
                }
             }

libraries/chain/fork_database.cpp

@@ -99,7 +99,7 @@ namespace eosio::chain {
 
       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;
-      bool             validated_block_exists_impl( const block_id_type& id ) const;
+      bool             validated_block_exists_impl( const block_id_type& id, const block_id_type& claimed_id ) const;
       void             reset_root_impl( const bsp_t& root_bs );
       void             advance_root_impl( const block_id_type& id );
       void             remove_impl( const block_id_type& id );
@@ -603,15 +603,27 @@ namespace eosio::chain {
    }
 
    template<class BSP>
-   bool fork_database_t<BSP>::validated_block_exists(const block_id_type& id) const {
+   bool fork_database_t<BSP>::validated_block_exists(const block_id_type& id, const block_id_type& claimed_id) const {
       std::lock_guard g( my->mtx );
-      return my->validated_block_exists_impl(id);
+      return my->validated_block_exists_impl(id, claimed_id);
    }
 
+   // prerequisite: claimed_id is either id, or an ancestor of id
+   // returns true if block `id`, or one of its ancestors not older than claimed_id, is found in fork_db
+   // and `is_valid()`.
+   // ------------------------------------------------------------------------------------------------------
    template<class BSP>
-   bool fork_database_impl<BSP>::validated_block_exists_impl(const block_id_type& id) const {
-      auto itr = index.find( id );
-      return itr != index.end() && (*itr)->is_valid();
+   bool fork_database_impl<BSP>::validated_block_exists_impl(const block_id_type& id, const block_id_type& claimed_id) const {
+      bool id_present = false;
+
+      for (auto i = index.find(id); i != index.end(); i = index.find((*i)->previous())) {
+         id_present = true;
+         if ((*i)->is_valid())
+            return true;
+         if ((*i)->id() == claimed_id)
+            return false;
+      }
+      return id_present;
    }
 
 // ------------------ fork_database -------------------------

libraries/chain/include/eosio/chain/block_state.hpp

@@ -87,8 +87,6 @@ struct block_state : public block_header_state {     // block_header_state provi
    std::optional<digest_type> base_digest;  // For finality_data sent to SHiP, computed on demand in get_finality_data()
 
    // ------ private methods -----------------------------------------------------------
-   void                                set_valid(bool v) { validated.store(v); }
-   bool                                is_valid() const { return validated.load(); }
    bool                                is_pub_keys_recovered() const { return pub_keys_recovered; }
    deque<transaction_metadata_ptr>     extract_trxs_metas();
    void                                set_trxs_metas(deque<transaction_metadata_ptr>&& trxs_metas, bool keys_recovered);
@@ -97,9 +95,9 @@ struct block_state : public block_header_state {     // block_header_state provi
    friend struct test_block_state_accessor;
    friend struct fc::reflector<block_state>;
    friend struct controller_impl;
-   template <typename BS> friend struct fork_database_impl;
    friend struct completed_block;
    friend struct building_block;
+
 public:
    // ------ functions -----------------------------------------------------------------
    const block_id_type&   id()                const { return block_header_state::id(); }
@@ -115,6 +113,9 @@ struct block_state : public block_header_state {     // block_header_state provi
    uint32_t               latest_qc_block_num() const          { return core.latest_qc_claim().block_num; }
    block_timestamp_type   latest_qc_block_timestamp() const    { return core.latest_qc_block_timestamp(); }
 
+   void                   set_valid(bool v) { validated.store(v); }
+   bool                   is_valid() const  { return validated.load(); }
+
    std::optional<qc_t> get_best_qc() const { return aggregating_qc.get_best_qc(block_num()); } // thread safe
    bool received_qc_is_strong() const { return aggregating_qc.received_qc_is_strong(); } // thread safe
    // return true if better qc, thread safe

libraries/chain/include/eosio/chain/block_state_legacy.hpp

@@ -51,20 +51,17 @@ namespace eosio::chain {
       const producer_authority_schedule*     pending_schedule_auth() const { return &block_header_state_legacy::pending_schedule.schedule; }
       const deque<transaction_metadata_ptr>& trxs_metas()            const { return _cached_trxs; }
 
-
+      void set_valid(bool v) { validated.store(v); }
+      bool is_valid() const  { return validated.load(); }
+
       using fork_db_block_state_accessor_t = block_state_legacy_accessor;
+
    private: // internal use only, not thread safe
-      friend struct block_state_legacy_accessor;
       friend struct fc::reflector<block_state_legacy>;
       friend struct controller_impl;
-      template <typename BS> friend struct fork_database_impl;
       friend struct completed_block;
       friend struct block_state;
 
-      // not thread safe, expected to only be called from main thread
-      void set_valid(bool v) { validated = v; }
-      bool is_valid() const { return validated; }
-
       bool is_pub_keys_recovered()const { return _pub_keys_recovered; }
 
       deque<transaction_metadata_ptr> extract_trxs_metas() {
@@ -78,7 +75,7 @@ namespace eosio::chain {
          _cached_trxs = std::move( trxs_metas );
       }
 
-      bool                                                validated = false;
+      copyable_atomic<bool>                               validated{false};
 
       bool                                                _pub_keys_recovered = false;
       /// this data is redundant with the data stored in block, but facilitates

libraries/chain/include/eosio/chain/fork_database.hpp

@@ -51,7 +51,7 @@ namespace eosio::chain {
 
       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;
-      bool validated_block_exists( const block_id_type& id ) const;
+      bool validated_block_exists( const block_id_type& id, const block_id_type& claimed_id ) const;
 
       /**
        *  Purges any existing blocks from the fork database and resets the root block_header_state to the provided value.

unittests/fork_db_tests.cpp

@@ -54,33 +54,8 @@ struct test_block_state_accessor {
 
 using namespace eosio::chain;
 
-BOOST_AUTO_TEST_SUITE(fork_database_tests)
-
-BOOST_AUTO_TEST_CASE(add_remove_test) try {
-   fork_database_if_t forkdb;
-
-   // Setup fork database with blocks based on a root of block 10
-   // Add a number of forks in the fork database
-   auto root = test_block_state_accessor::make_genesis_block_state();
-   auto   bsp11a = test_block_state_accessor::make_unique_block_state(11, root);
-   auto     bsp12a = test_block_state_accessor::make_unique_block_state(12, bsp11a);
-   auto       bsp13a = test_block_state_accessor::make_unique_block_state(13, bsp12a);
-   auto   bsp11b = test_block_state_accessor::make_unique_block_state(11, root);
-   auto     bsp12b = test_block_state_accessor::make_unique_block_state(12, bsp11b);
-   auto       bsp13b = test_block_state_accessor::make_unique_block_state(13, bsp12b);
-   auto         bsp14b = test_block_state_accessor::make_unique_block_state(14, bsp13b);
-   auto     bsp12bb = test_block_state_accessor::make_unique_block_state(12, bsp11b);
-   auto       bsp13bb = test_block_state_accessor::make_unique_block_state(13, bsp12bb);
-   auto       bsp13bbb = test_block_state_accessor::make_unique_block_state(13, bsp12bb);
-   auto     bsp12bbb = test_block_state_accessor::make_unique_block_state(12, bsp11b);
-   auto   bsp11c = test_block_state_accessor::make_unique_block_state(11, root);
-   auto     bsp12c = test_block_state_accessor::make_unique_block_state(12, bsp11c);
-   auto       bsp13c = test_block_state_accessor::make_unique_block_state(13, bsp12c);
-
-   // keep track of all those added for easy verification
-   std::vector<block_state_ptr> all { bsp11a, bsp12a, bsp13a, bsp11b, bsp12b, bsp12bb, bsp12bbb, bsp13b, bsp13bb, bsp13bbb, bsp14b, bsp11c, bsp12c, bsp13c };
-
-   auto reset = [&]() {
+struct generate_forkdb_state {
+   generate_forkdb_state() {
       forkdb.reset_root(root);
       forkdb.add(bsp11a, ignore_duplicate_t::no);
       forkdb.add(bsp11b, ignore_duplicate_t::no);
@@ -96,9 +71,37 @@ BOOST_AUTO_TEST_CASE(add_remove_test) try {
       forkdb.add(bsp13bbb, ignore_duplicate_t::no);
       forkdb.add(bsp14b, ignore_duplicate_t::no);
       forkdb.add(bsp13c, ignore_duplicate_t::no);
-   };
-   reset();
+   }
 
+   fork_database_if_t forkdb;
+
+   // Setup fork database with blocks based on a root of block 10
+   // Add a number of forks in the fork database
+   block_state_ptr root = test_block_state_accessor::make_genesis_block_state();
+   block_state_ptr   bsp11a = test_block_state_accessor::make_unique_block_state(11, root);
+   block_state_ptr     bsp12a = test_block_state_accessor::make_unique_block_state(12, bsp11a);
+   block_state_ptr       bsp13a = test_block_state_accessor::make_unique_block_state(13, bsp12a);
+   block_state_ptr   bsp11b = test_block_state_accessor::make_unique_block_state(11, root);
+   block_state_ptr     bsp12b = test_block_state_accessor::make_unique_block_state(12, bsp11b);
+   block_state_ptr       bsp13b = test_block_state_accessor::make_unique_block_state(13, bsp12b);
+   block_state_ptr         bsp14b = test_block_state_accessor::make_unique_block_state(14, bsp13b);
+   block_state_ptr     bsp12bb = test_block_state_accessor::make_unique_block_state(12, bsp11b);
+   block_state_ptr       bsp13bb = test_block_state_accessor::make_unique_block_state(13, bsp12bb);
+   block_state_ptr       bsp13bbb = test_block_state_accessor::make_unique_block_state(13, bsp12bb);
+   block_state_ptr     bsp12bbb = test_block_state_accessor::make_unique_block_state(12, bsp11b);
+   block_state_ptr   bsp11c = test_block_state_accessor::make_unique_block_state(11, root);
+   block_state_ptr     bsp12c = test_block_state_accessor::make_unique_block_state(12, bsp11c);
+   block_state_ptr       bsp13c = test_block_state_accessor::make_unique_block_state(13, bsp12c);
+
+   // keep track of all those added for easy verification
+   std::vector<block_state_ptr> all{bsp11a, bsp12a,  bsp13a,   bsp11b, bsp12b, bsp12bb, bsp12bbb,
+                                    bsp13b, bsp13bb, bsp13bbb, bsp14b, bsp11c, bsp12c,  bsp13c};
+};
+
+
+BOOST_AUTO_TEST_SUITE(fork_database_tests)
+
+BOOST_FIXTURE_TEST_CASE(add_remove_test, generate_forkdb_state) try {
    // test get_block
    for (auto& i : all) {
       BOOST_TEST(forkdb.get_block(i->id()) == i);
@@ -137,4 +140,51 @@ BOOST_AUTO_TEST_CASE(add_remove_test) try {
    BOOST_TEST(branch[1] == bsp11a);
 } FC_LOG_AND_RETHROW();
 
+
+// test `fork_database_t::validated_block_exists() const` member
+// -------------------------------------------------------------
+BOOST_FIXTURE_TEST_CASE(validated_block_exists, generate_forkdb_state) try {
+   bsp12b->set_valid(true);
+   bsp13b->set_valid(true);
+   bsp14b->set_valid(true);
+   bsp11a->set_valid(true);
+
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp14b->id()));
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp13b->id()));
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp12b->id()));
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp11b->id()));
+
+   // incorrect call as bsp13a is not an ancestor of bsp14b. As a result `claimed_id` is
+   // not found, so we assume it is either the root or an older block which is `valid`.
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp13a->id()));
+
+   bsp14b->set_valid(false);
+   BOOST_REQUIRE_EQUAL(false, forkdb.validated_block_exists(bsp14b->id(), bsp14b->id()));
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp13b->id()));
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp12b->id()));
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp11b->id()));
+
+   bsp12b->set_valid(false);
+   BOOST_REQUIRE_EQUAL(false, forkdb.validated_block_exists(bsp14b->id(), bsp14b->id()));
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp13b->id()));
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp12b->id()));
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), bsp11b->id()));
+
+   bsp13b->set_valid(false);
+   BOOST_REQUIRE_EQUAL(false, forkdb.validated_block_exists(bsp14b->id(), bsp14b->id()));
+   BOOST_REQUIRE_EQUAL(false, forkdb.validated_block_exists(bsp14b->id(), bsp13b->id()));
+   BOOST_REQUIRE_EQUAL(false, forkdb.validated_block_exists(bsp14b->id(), bsp12b->id()));
+   BOOST_REQUIRE_EQUAL(false, forkdb.validated_block_exists(bsp14b->id(), bsp11b->id()));
+
+   bsp11b->set_valid(false);
+   BOOST_REQUIRE_EQUAL(false, forkdb.validated_block_exists(bsp14b->id(), bsp14b->id()));
+   BOOST_REQUIRE_EQUAL(false, forkdb.validated_block_exists(bsp14b->id(), bsp13b->id()));
+   BOOST_REQUIRE_EQUAL(false, forkdb.validated_block_exists(bsp14b->id(), bsp12b->id()));
+   BOOST_REQUIRE_EQUAL(false, forkdb.validated_block_exists(bsp14b->id(), bsp11b->id()));
+
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), root->id()));
+   BOOST_REQUIRE_EQUAL(true,  forkdb.validated_block_exists(bsp14b->id(), block_id_type{}));
+
+} FC_LOG_AND_RETHROW();
+
 BOOST_AUTO_TEST_SUITE_END()