Hotstuff integration #1536

heifner · 2023-08-19T01:57:58Z

Merge of hotstuff consensus integration branch to main.

Suggested process for this PR:

Add resolvable comments (on lines of code); not just plain comments that can't be marked resolved. This will allow us to easily mark comments resolves once a PR is merged into this (hotstuff_integration) branch.
Do not push directly to hotstuff_integration branch; create PRs to hotstuff_integration branch.
For large tasks create a github issue and add it to IF Prototype Stage 1A Implementation in Leap #1508 meta issue. This can be done easily by adding it directly to IF Prototype Stage 1A Implementation in Leap #1508.
When all associated issues of IF Prototype Stage 1A Implementation in Leap #1508 are complete this branch can be merged to main to complete IF Prototype Stage 1A Implementation in Leap #1508.

Resolves #1508

heifner

First quick pass complete. Detailed review next.

libraries/chain/controller.cpp

libraries/chain/include/eosio/chain/hotstuff.hpp

libraries/chain/controller.cpp

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

plugins/producer_plugin/producer_plugin.cpp

plugins/producer_plugin/qc_chain.cpp.bkp

plugins/producer_plugin/qc_chain.old.cpp

plugins/producer_plugin/qc_chain.old2.cpp

plugins/net_plugin/net_plugin.cpp

libraries/chain/include/eosio/chain/hotstuff.hpp

linh2931 · 2023-08-22T15:27:59Z

libraries/chain/protocol_feature_manager.cpp

+            fc::variant("bc726a24928ea2d71ba294b70c5c9efc515c1542139bcf9e42f8bc174f2e72ff").as<digest_type>(),
+            // SHA256 hash of the raw message below within the comment delimiters (do not modify message below).
+/*
+Builtin protocol feature: INSTANT_FINALITY


Add some explanation about instant_finality like other protocol features do.

linh2931 · 2023-08-22T15:30:46Z

libraries/hotstuff/CMakeLists.txt

+
+add_library( hotstuff
+             chain_pacemaker.cpp
+             qc_chain.cpp


I am a little puzzled by chain being a part of those two file names, and one having chain in the front and the other in the end: chain_pacemake.cpp and qc_chain.cpp. Can the names be changed?

linh2931 · 2023-08-22T15:37:51Z

libraries/hotstuff/chain_pacemaker.cpp

+#include <iostream>
+
+// comment this out to remove the core profiler
+#define HS_CORE_PROFILER


Move HS_CORE_PROFILER to CmakeList.txt to let cmake to control whether to profile or not.

linh2931 · 2023-08-22T15:41:51Z

libraries/hotstuff/chain_pacemaker.cpp

+      if (! enabled())
+         return;
+
+      csc prof("beat");


For all those profiling statements, maybe each of them should be enclosed with ifdef. Even though csc is a dummy struct when HS_CORE_PROFILER is false, compiler will still generate some code. When read first, it appears profiling is always on. The downside of ifdefs is the cluttering.

Created #1542

linh2931 · 2023-08-22T17:26:54Z

libraries/libfc/src/crypto/bls_public_key.cpp

+
+namespace fc { namespace crypto { namespace blslib {
+
+  /* struct recovery_visitor : fc::visitor<bls_public_key::storage_type> {


should be removed?

linh2931 · 2023-08-22T17:27:24Z

libraries/libfc/src/crypto/bls_signature.cpp

+namespace fc { namespace crypto { namespace blslib {
+
+   struct hash_visitor : public fc::visitor<size_t> {
+/*      template<typename SigType>


linh2931 · 2023-08-22T17:32:26Z

plugins/chain_plugin/chain_plugin.cpp

+      results.current_qc             = fs.current_qc;
+      results.schedule               = fs.schedule;
+      for (auto proposal: fs.proposals) {
+         chain::hs_proposal_message & p = proposal.second;


remove the space before &.

linh2931 · 2023-08-22T17:41:26Z

programs/nodeos/main.cpp

@@ -147,6 +147,9 @@ enum return_codes {

 int main(int argc, char** argv)
 {
+
+   ilog("nodeos started");


why this log? if we do, probably should be "nodeos starting".

Can be safely removed

linh2931 · 2023-08-22T17:41:53Z

programs/nodeos/main.cpp

@@ -200,6 +203,7 @@ int main(int argc, char** argv)
   } catch( const node_management_success& e ) {
      return NODE_MANAGEMENT_SUCCESS;
   } catch( const fc::exception& e ) {
+


why new empty line?

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

libraries/hotstuff/qc_chain.cpp

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

libraries/hotstuff/qc_chain.cpp

libraries/hotstuff/chain_pacemaker.cpp

libraries/hotstuff/qc_chain.cpp

greg7mdp · 2023-08-22T22:22:07Z

libraries/chain/include/eosio/chain/hotstuff.hpp

+
+   struct extended_schedule {
+      producer_authority_schedule                          producer_schedule;
+      std::map<name, fc::crypto::blslib::bls_public_key>   bls_pub_keys;


Should the bls_pub_keys be added in the producer_authority_schedule instead of on the side like here?

Extended schedule was a placeholder until we reached a decision on the final format of the finalizers set structure. I think we agreed to only keep the bls public key, as well as the threshold for finalizers (as far as the hotstuff algorithm is concerned). The name would be present in the structure as well, but serves only to identify the finalizer on-chain, and is not required by Hotstuff.

greg7mdp · 2023-08-22T22:24:41Z

libraries/chain/include/eosio/chain/hotstuff.hpp

+      block_id_type                       block_id = NULL_BLOCK_ID;
+      fc::sha256                          parent_id = NULL_PROPOSAL_ID; //new proposal
+      fc::sha256                          final_on_qc = NULL_PROPOSAL_ID;
+      quorum_certificate                  justify; //justification


All these = NULL_PROPOSAL_ID are unneeded since the default construction of sha256 already initializes to all zeroes.

libraries/hotstuff/chain_pacemaker.cpp

greg7mdp · 2023-08-22T22:46:52Z

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

+      //
+      mutable std::mutex     _state_mutex;
+
+#ifdef QC_CHAIN_SIMPLE_PROPOSAL_STORE


Have we decided on a proposal store? Right now QC_CHAIN_SIMPLE_PROPOSAL_STORE is not defined.

The QC_CHAIN_SIMPLE_PROPOSAL_STORE and associated code will be removed. Maybe can be done during #1541 or #1520

libraries/hotstuff/qc_chain.cpp

libraries/hotstuff/chain_pacemaker.cpp

libraries/hotstuff/qc_chain.cpp

greg7mdp · 2023-08-26T16:16:43Z

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

+      uint32_t _v_height = 0;
+      eosio::chain::extended_schedule _schedule;
+      base_pacemaker* _pacemaker = nullptr;
+      std::set<name> _my_producers;


Why is this a std::set and not a std::vector. As far as I can tell we never use any of the std::set capabilities.

Agree; there's no reason to sort or uniquify producer account names.

ericpassmore · 2024-01-05T23:27:00Z

Note:start
group: IF
category: CHORE
summary: Integration of new faster finality into main branch, continued support for legacy DPOS.
Note:end

ericpassmore · 2024-01-17T04:34:15Z

Note:start
group: IF
category: CHORE
summary: Merge main branch into Faster Finality working branch.
Note: end

…d block_state_variant_t.

…finality_info_support

…ting temporary savanna block

…. Improve switch logging.

…ion_block IF: SHiP -- Provide finality_data for Transition blocks

… to terminate at a block don't put any blocks above that in the fork database.

IF: switch variant index of new `get_blocks_request_v1`

…lock

This reverts commit 914f218.

…gs all processed blocks

…-vote-processing

Increment `finalizer_policy`'s `generation` when `set_finalizers` is executed.

IF: Populated fork db ASAP; Vote processing off the main thread

heifner added the consensus label Aug 19, 2023

heifner commented Aug 21, 2023

View reviewed changes

heifner marked this pull request as draft August 21, 2023 17:30

greg7mdp self-requested a review August 21, 2023 21:13

linh2931 requested changes Aug 22, 2023

View reviewed changes

heifner commented Aug 22, 2023

View reviewed changes

heifner mentioned this pull request Aug 22, 2023

IF: Calculate quorum threshold from schedule #1544

Closed

greg7mdp reviewed Aug 22, 2023

View reviewed changes

heifner commented Aug 23, 2023

View reviewed changes

libraries/hotstuff/qc_chain.cpp Outdated Show resolved Hide resolved

libraries/hotstuff/qc_chain.cpp Outdated Show resolved Hide resolved

heifner commented Aug 23, 2023

View reviewed changes

heifner mentioned this pull request Aug 23, 2023

IF: hs_*_message types - review #1547

Closed

arhag mentioned this pull request Aug 23, 2023

IF: Host function to set active finalizer set #1511

Closed

heifner commented Aug 24, 2023

View reviewed changes

libraries/hotstuff/qc_chain.cpp Outdated Show resolved Hide resolved

greg7mdp reviewed Aug 26, 2023

View reviewed changes

heifner mentioned this pull request Jan 25, 2024

IF: Instant finality updates AntelopeIO/reference-contracts#43

Merged

greg7mdp and others added 13 commits March 16, 2024 10:05

Fix plugin_test (was missing action_return_value dependent feature)

28d7cbf

Remove leftover comment.

a9058bd

GH-2057 Remove unneeded reset_root that takes a variant. Remove unuse…

75b5627

…d block_state_variant_t.

update ABI definition and types for the new finality_data log

d08e321

GH-2057 WIP: Transition during replay

bac10e1

GH-2057 Handle startup from snapshot post-if transition without a forkdb

b432deb

GH-2057 Revert to v5.0 impl

71ea20c

Renamed if_ibc to svnn_ibc, fixed issues

4f8a5c4

Support finality_data in state_history_plugin

900fa22

add new finality_data tests to state_history_plugin test

0bda766

update ship_streamer tests for finality_data

54797c0

rename get_chain_head_finality_data() to head_finality_data()

31e4100

Merge remote-tracking branch 'origin/hotstuff_integration' into ship_…

84c80f9

…finality_info_support

heifner and others added 28 commits April 4, 2024 12:22

GH-2102 If forked out again then could still be in local state

274e44a

store action_mroot only for the last block in the process of construc…

93163ec

…ting temporary savanna block

GH-2102 Allow ctrl-c shutdown during sync of a large number of blocks…

2e49794

…. Improve switch logging.

GH-2102 Fix sync issue with receiving a current block while syncing

4c3535f

Merge pull request #2362 from AntelopeIO/add_finality_data_in_transit…

08d0c96

…ion_block IF: SHiP -- Provide finality_data for Transition blocks

GH-2102 On startup pending_head forkdb blocks are processed. If asked…

112bb69

… to terminate at a block don't put any blocks above that in the fork database.

switch variant index of new get_blocks_request_v1

84098e5

Merge pull request #2381 from AntelopeIO/ship_gbrv1_variant

66c7c62

IF: switch variant index of new `get_blocks_request_v1`

GH-2102 Fix comparison in waitForBlock

914f218

GH-2102 Better error reporting and a bit more tolerance for trxs in b…

5e9af0c

…lock

GH-2102 Add additional logging for applied blocks

2449830

Revert "GH-2102 Fix comparison in waitForBlock"

e464aee

This reverts commit 914f218.

GH-2102 Improve logging during sync

a74450c

GH-2102 Add a large_atomic wrapper around mutex

cafd0b6

GH-2102 Use large_atomic for if_irreversible_block_id

302b957

GH-2102 No need to recalculate finality digest

be77ca6

GH-2102 Move valid_qc into pending_qc and make thread safe

23945ff

GH-2102 Improve test conditions

ba5006e

GH-2102 Move produced/received block logging into controller so it lo…

9de11ff

…gs all processed blocks

GH-2102 Fix log_applied to not be called during replay

8e98c18

GH-2102 Add a copyable atomic type

a9058a1

GH-2102 Integrate qc and vote if possible off the main thread.

6d64c27

Merge remote-tracking branch 'origin/hotstuff_integration' into GH-2102…

6bd8128

…-vote-processing

Increment finalizer_policy's generation.

c54fe32

Merge pull request #2388 from AntelopeIO/gh_2345

232e95b

Increment `finalizer_policy`'s `generation` when `set_finalizers` is executed.

GH-2102 Small cleanup from PR review

33210c0

GH-2102 Move Produced block log into controller_impl commit_block

5cc1b87

Merge pull request #2385 from AntelopeIO/GH-2102-vote-processing

f9786e1

IF: Populated fork db ASAP; Vote processing off the main thread

arhag closed this Aug 20, 2024

arhag deleted the hotstuff_integration branch August 20, 2024 17:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hotstuff integration #1536

Hotstuff integration #1536

heifner commented Aug 19, 2023 •

edited

Loading

heifner left a comment

linh2931 Aug 22, 2023

linh2931 Aug 22, 2023

linh2931 Aug 22, 2023

linh2931 Aug 22, 2023

linh2931 Aug 22, 2023

linh2931 Aug 22, 2023

linh2931 Aug 22, 2023

linh2931 Aug 22, 2023

linh2931 Aug 22, 2023

systemzax Aug 24, 2023

linh2931 Aug 22, 2023

greg7mdp Aug 22, 2023

systemzax Aug 24, 2023

greg7mdp Aug 22, 2023

greg7mdp Aug 22, 2023

heifner Aug 23, 2023

greg7mdp Aug 26, 2023

fcecin Aug 26, 2023

ericpassmore commented Jan 5, 2024

ericpassmore commented Jan 17, 2024


		namespace fc { namespace crypto { namespace blslib {

		/* struct recovery_visitor : fc::visitor<bls_public_key::storage_type> {

Hotstuff integration #1536

Hotstuff integration #1536

Conversation

heifner commented Aug 19, 2023 • edited Loading

heifner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericpassmore commented Jan 5, 2024

ericpassmore commented Jan 17, 2024

heifner commented Aug 19, 2023 •

edited

Loading