More code cleanup and prepare storing hs_commitment in block extension

libraries/chain/controller.cpp

@@ -2990,6 +2990,10 @@ void controller::commit_block() {
    my->commit_block(block_status::incomplete);
 }
 
+void controller::mark_irreversible(block_id_type b) { // called from HotStuff consensus
+}
+
+
 deque<transaction_metadata_ptr> controller::abort_block() {
    return my->abort_block();
 }

libraries/chain/fork_database.cpp

@@ -573,6 +573,8 @@ namespace eosio { namespace chain {
    }
 
    void fork_database_impl::mark_irreversible_impl( const block_id_type& b ) {
+      // called by HotStuff consensus to notify `chain` that block b is now irreversible
+
    }
 
    block_state_ptr fork_database::get_block(const block_id_type& id)const {

libraries/chain/include/eosio/chain/controller.hpp

@@ -168,6 +168,8 @@ namespace eosio { namespace chain {
          block_state_ptr finalize_block( block_report& br, const signer_callback_type& signer_callback );
          void sign_block( const signer_callback_type& signer_callback );
          void commit_block();
+
+         void mark_irreversible(block_id_type b); // called from HotStuff consensus
 
          // thread-safe
          std::future<block_state_ptr> create_block_state_future( const block_id_type& id, const signed_block_ptr& b );

libraries/hotstuff/chain_pacemaker.cpp

@@ -105,7 +105,8 @@ namespace eosio { namespace hotstuff {
                                     std::set<account_name> my_producers,
                                     bls_key_map_t finalizer_keys,
                                     fc::logger& logger)
-      : _qc_chain("default"_n, this, std::move(my_producers), std::move(finalizer_keys), logger),
+      : _chain(chain),
+        _qc_chain("default"_n, this, std::move(my_producers), std::move(finalizer_keys), logger),
         _logger(logger)
    {
       _accepted_block_connection = chain->accepted_block.connect( [this]( const block_state_ptr& blk ) {
@@ -213,6 +214,13 @@ namespace eosio { namespace hotstuff {
       return n;
    }
 
+   // called from main thread
+   void chain_pacemaker::on_pre_accepted_block( const signed_block_ptr& blk ) {
+      std::scoped_lock g( _chain_state_mutex );
+      // store _qc_chain.get_last_commitment() into block extension
+      // is it the right location? What happens if this block does not become final?
+   }
+
    // called from main thread
    void chain_pacemaker::on_accepted_block( const block_state_ptr& blk ) {
       std::scoped_lock g( _chain_state_mutex );
@@ -302,48 +310,15 @@ namespace eosio { namespace hotstuff {
 
    // called from net threads
    void chain_pacemaker::on_hs_msg(const eosio::chain::hs_message &msg) {
-      std::visit(overloaded{
-              [this](const hs_vote_message& m) { on_hs_vote_msg(m); },
-              [this](const hs_proposal_message& m) { on_hs_proposal_msg(m); },
-              [this](const hs_new_block_message& m) { on_hs_new_block_msg(m); },
-              [this](const hs_new_view_message& m) { on_hs_new_view_msg(m); },
-      }, msg);
-   }
-
-   // called from net threads
-   void chain_pacemaker::on_hs_proposal_msg(const hs_proposal_message& msg) {
-      csc prof("prop");
-      std::lock_guard g( _hotstuff_global_mutex );
-      prof.core_in();
-      _qc_chain.on_hs_proposal_msg(msg);
-      prof.core_out();
-   }
-
-   // called from net threads
-   void chain_pacemaker::on_hs_vote_msg(const hs_vote_message& msg) {
-      csc prof("vote");
-      std::lock_guard g( _hotstuff_global_mutex );
-      prof.core_in();
-      _qc_chain.on_hs_vote_msg(msg);
-      prof.core_out();
-   }
-
-   // called from net threads
-   void chain_pacemaker::on_hs_new_block_msg(const hs_new_block_message& msg) {
-      csc prof("nblk");
-      std::lock_guard g( _hotstuff_global_mutex );
-      prof.core_in();
-      _qc_chain.on_hs_new_block_msg(msg);
-      prof.core_out();
-   }
-
-   // called from net threads
-   void chain_pacemaker::on_hs_new_view_msg(const hs_new_view_message& msg) {
-      csc prof("view");
-      std::lock_guard g( _hotstuff_global_mutex );
-      prof.core_in();
-      _qc_chain.on_hs_new_view_msg(msg);
-      prof.core_out();
+      auto res = _qc_chain.on_hs_msg(msg); // returns block_id made irrersible (if any)
+      if (res) {
+         // todo: problem calling into the main thread from a net_plugin thread
+         // one solution would be for:
+         //   - `_chain->mark_irreversible` to update an `atomic_shared_ptr`
+         //   - or: Instead of calling into _chain here, we could store the lib block_id into a member variable of chain_pacemaker,
+         //         and call `_chain->mark_irreversible` from on_accepted_block.
+         //   - or: controller::log_irreversible to query `chain_pacemaker` for the lib instead of `fork_head->dpos_irreversible_blocknum`
+         _chain->mark_irreversible(*res);
+      }
    }
-
 }}

libraries/hotstuff/include/eosio/hotstuff/chain_pacemaker.hpp

@@ -48,11 +48,8 @@ namespace eosio::hotstuff {
    private:
       void on_accepted_block( const block_state_ptr& blk );
       void on_irreversible_block( const block_state_ptr& blk );
-
-      void on_hs_proposal_msg(const hs_proposal_message& msg); //consensus msg event handler
-      void on_hs_vote_msg(const hs_vote_message& msg); //confirmation msg event handler
-      void on_hs_new_view_msg(const hs_new_view_message& msg); //new view msg event handler
-      void on_hs_new_block_msg(const hs_new_block_message& msg); //new block msg event handler
+      void on_pre_accepted_block( const signed_block_ptr& blk );
+
    private:
 
       //FIXME/REMOVE: for testing/debugging only
@@ -78,6 +75,7 @@ namespace eosio::hotstuff {
       boost::signals2::scoped_connection _accepted_block_connection;
       boost::signals2::scoped_connection _irreversible_block_connection;
 
+      controller*             _chain;
       qc_chain                _qc_chain;
       std::function<void(const chain::hs_message&)> bcast_hs_message;
 

libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp

@@ -110,10 +110,7 @@ namespace eosio::hotstuff {
 
       void on_beat(); //handler for pacemaker beat()
 
-      void on_hs_vote_msg(const hs_vote_message& msg); //vote msg event handler
-      std::optional<block_id_type> on_hs_proposal_msg(const hs_proposal_message& msg); //proposal msg event handler
-      void on_hs_new_view_msg(const hs_new_view_message& msg); //new view msg event handler
-      void on_hs_new_block_msg(const hs_new_block_message& msg); //new block msg event handler
+      std::optional<block_id_type> on_hs_msg(const hs_message& msg);
 
    private:
        std::optional<block_id_type> process_hs_msg(const hs_message& msg)
@@ -122,8 +119,8 @@ namespace eosio::hotstuff {
           std::visit(overloaded{
                         [this](const hs_vote_message& m) { process_vote(m); },
                         [this, &res](const hs_proposal_message& m) { res = process_proposal(m); },
-                        [this](const hs_new_block_message& m) {},
-                        [this](const hs_new_view_message& m) {},
+                        [](const hs_new_block_message& m) {},
+                        [](const hs_new_view_message& m) {},
                     },
                     msg);
           return res;
@@ -178,6 +175,8 @@ namespace eosio::hotstuff {
 
       void gc_proposals(uint64_t cutoff); //garbage collection of old proposals
 
+      const std::optional<hs_commitment>& get_last_commitment() const { return _last_commitment; }
+
 #warning remove. bls12-381 key used for testing purposes
       //todo : remove. bls12-381 key used for testing purposes
       std::vector<uint8_t> _seed =
@@ -209,6 +208,7 @@ namespace eosio::hotstuff {
       std::set<name> _my_producers;
       chain::bls_key_map_t _my_finalizer_keys;
       name _id;
+      std::optional<hs_commitment> _last_commitment; // thread safety??
 
       mutable std::atomic<uint64_t> _state_version = 1;
 

libraries/hotstuff/qc_chain.cpp

@@ -823,25 +823,18 @@ namespace eosio::hotstuff {
       return final_on_qc_check && monotony_check && (liveness_check || safety_check);
    }
 
-   //on proposal received, called from network thread
+   //hs_message received, called from network thread
    //returns highest block_id that was made irreversible (if any)
-   std::optional<block_id_type> qc_chain::on_hs_proposal_msg(const hs_proposal_message& msg) {
-      return process_proposal(msg);
-   }
-
-   //on vote received, called from network thread
-   void qc_chain::on_hs_vote_msg(const hs_vote_message& msg) {
-      process_vote(msg);
-   }
-
-   //on new view received, called from network thread
-   void qc_chain::on_hs_new_view_msg(const hs_new_view_message& msg) {
-      process_new_view(msg);
-   }
-
-   //on new block received, called from network thread
-   void qc_chain::on_hs_new_block_msg(const hs_new_block_message& msg) {
-      process_new_block(msg);
+   std::optional<block_id_type> qc_chain::on_hs_msg(const hs_message& msg) {
+      std::optional<block_id_type> res;
+      std::visit(overloaded{
+                     [this](const hs_vote_message& m) { process_vote(m); },
+                     [this, &res](const hs_proposal_message& m) { res = process_proposal(m); },
+                     [this](const hs_new_block_message& m) { process_new_block(m); },
+                     [this](const hs_new_view_message& m) { process_new_view(m); },
+                 },
+                 msg);
+      return res;
    }
 
    std::optional<block_id_type> qc_chain::update(const hs_proposal_message& proposal) {
@@ -1038,15 +1031,15 @@ void qc_chain::commit(const hs_commitment& commitment) {
    }
 
    if (!proposal_chain.empty()) {
-
-      // update latest known commitment in controller
-      // controller->update_hs_commitment(commitment);
+      _last_commitment = commitment; // will be provided to controller by pacemaker in `on_irreversible_block`
 
       // commit all ancestor blocks sequentially first (hence the reverse)
       for (auto p : boost::adaptors::reverse(proposal_chain)) {
-         // Execute commands [...]
          // issue #1522:  HotStuff finality should drive LIB in controller
-         // ??? controller->commit_block(controller::block_status::irreversible, p.block_id);
+         // no need to do anything here. `qc_chain::update()` will return the `block_id_type` made final
+         // (which is b.block_id) to `chain_pacemaker`, which will notify the controller. The controller
+         // will notify `fork_database` so that this block and its ancestors are marked irreversible and
+         // moved out of `fork_database`.
          ;
       }
 

libraries/hotstuff/test/test_pacemaker.cpp

@@ -168,7 +168,7 @@ namespace eosio::hotstuff {
          const name                & qcc_name = qc_itr->first;
          std::shared_ptr<qc_chain> & qcc_ptr  = qc_itr->second;
          if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) )
-            qcc_ptr->on_hs_proposal_msg(msg);
+            qcc_ptr->on_hs_msg(msg);
          qc_itr++;
       }
    }
@@ -179,7 +179,7 @@ namespace eosio::hotstuff {
          const name                & qcc_name = qc_itr->first;
          std::shared_ptr<qc_chain> & qcc_ptr  = qc_itr->second;
          if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) )
-            qcc_ptr->on_hs_vote_msg(msg);
+            qcc_ptr->on_hs_msg(msg);
          qc_itr++;
       }
    }
@@ -190,7 +190,7 @@ namespace eosio::hotstuff {
          const name                & qcc_name = qc_itr->first;
          std::shared_ptr<qc_chain> & qcc_ptr  = qc_itr->second;
          if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) )
-            qcc_ptr->on_hs_new_block_msg(msg);
+            qcc_ptr->on_hs_msg(msg);
          qc_itr++;
       }
    }
@@ -201,7 +201,7 @@ namespace eosio::hotstuff {
          const name                & qcc_name = qc_itr->first;
          std::shared_ptr<qc_chain> & qcc_ptr  = qc_itr->second;
          if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) )
-            qcc_ptr->on_hs_new_view_msg(msg);
+            qcc_ptr->on_hs_msg(msg);
          qc_itr++;
       }
    }