IF: Transition from dpos to instant-finality #2085

heifner · 2024-01-14T02:49:41Z

Transition from dpos to instant-finality on the block after set_finalizer host function call. This is a temporary method of transition until we work out the algorithm to use in production.
- fork_database switched over after block with set_finalizer call.
Add block_token as a wrapper of block_state_ptr and block_state_legacy_ptr.
- block_state no longer used outside of chain library except for leap-util blocklog_actions::read_log which will be fixed by IF: fork database transition not thread safe #2083
Enable instant-finality nodeos_run_test.py integration test which tests transition.
Performance improvement via controller::block_exists instead of always pulling the complete block out of the block log.
Fixes producer schedule switch bug after instant-finality is enabled.
LIB does not advance after switching to instant-finality. This is expected as we are not generating vote messages yet.

Resolves #2045

… finalizing a block in instant finality

…ransision

…ck_state. Add missing set of transactions in block.

… not at beginning.

…rk database. Remove fetch_block_state_* methods.

greg7mdp · 2024-01-14T03:55:31Z

libraries/chain/controller.cpp

@@ -3757,10 +3772,10 @@ void controller::start_block( block_timestamp_type when,
                    bs, std::optional<block_id_type>(), deadline );
 }

-void controller::finalize_block( block_report& br, const signer_callback_type& signer_callback ) {
+void controller::finish_block( block_report& br, const signer_callback_type& signer_callback ) {


I think I would have called it complete_block as it switches to the completed_block stage.

@arhag mentioned calling it finish_block so just went with that. I'm ok with either finish_block or complete_block.

No strong feeling about this but I like the symmetry of:

assemble_block(building_block) -> assembled_block complete_block(assembled_block) -> completed_block

I think finish_block highlights the last step of those block building stages. Sounds more natural.

libraries/chain/controller.cpp

…egacy. Also add block_exists as a performance improvement.

…roducer schedule test.

greg7mdp

Nice, thanks for all the fixes Kevin.

Naming personal perference again, instead of block_token, I would prefer block_handle, I think it is somewhat more accurate,

greg7mdp · 2024-01-15T18:29:21Z

libraries/chain/block_state.cpp

+   block->transactions = std::move(trx_receipts);
+}
+
+// Used for transition from dbpos to instant-finality


Suggested change

// Used for transition from dbpos to instant-finality

// Used for transition from dpos to instant-finality

libraries/chain/block_state.cpp

heifner · 2024-01-15T18:43:50Z

Nice, thanks for all the fixes Kevin.

Naming personal perference again, instead of block_token, I would prefer block_handle, I think it is somewhat more accurate,

Hmm, either nomenclature works I think. Not sure which I would consider more accurate. Open to changing the name. Thoughts @arhag @linh2931 ?

linh2931

I like your combining block_state_legacy and block_state into block_token. The fewer the _legacy, the better.

libraries/chain/controller.cpp

linh2931 · 2024-01-15T21:15:03Z

libraries/chain/controller.cpp

+      }, v);
+   }
+
+   std::optional<block_id_type> fork_db_fetch_block_id(uint32_t block_num) const {


Can be named as fork_db_fetch_block_id_by_num to match fork_db_fetch_block_by_num.

linh2931 · 2024-01-15T21:20:27Z

libraries/chain/controller.cpp

@@ -3757,10 +3772,10 @@ void controller::start_block( block_timestamp_type when,
                    bs, std::optional<block_id_type>(), deadline );
 }

-void controller::finalize_block( block_report& br, const signer_callback_type& signer_callback ) {
+void controller::finish_block( block_report& br, const signer_callback_type& signer_callback ) {


I think finish_block highlights the last step of those block building stages. Sounds more natural.

libraries/chain/controller.cpp

linh2931 · 2024-01-15T21:35:32Z

libraries/chain/controller.cpp

+                  ilog("Transition to instant finality happening after block ${b}", ("b", bsp->block_num()));
+                  hs_irreversible_block_num = bsp->block_num();
+
+                  log_irreversible();


Will log_irreversible be executed twice as there was one above?

It will be executed twice. But if_irreversible_block_num is set before the second call. log_irreversible determines internally if it should do anything. Always safe to call it multiple times.

libraries/chain/controller.cpp

linh2931 · 2024-01-15T21:53:12Z

Nice, thanks for all the fixes Kevin.
Naming personal perference again, instead of block_token, I would prefer block_handle, I think it is somewhat more accurate,

Hmm, either nomenclature works I think. Not sure which I would consider more accurate. Open to changing the name. Thoughts @arhag @linh2931 ?

Either is fine. Or block_ptr or block_ctx to be shorter?

libraries/chain/controller.cpp

ericpassmore · 2024-01-17T03:50:36Z

Note:start
group: IF
category: INTERNALS
summary: Initial early version of transition from legacy finality to new Faster Finality. Additional work required on transition to address all scenarios.
Note: end

heifner added 8 commits January 12, 2024 13:32

GH-2045 Rename finalize_block to finish_block to avoid confusion with…

3b15160

… finalizing a block in instant finality

Merge remote-tracking branch 'origin/gh_2034_part2' into GH-2045-if-t…

8408763

…ransision

Merge remote-tracking branch 'origin/gh_2034_part2' into GH-2045-if-t…

1c3315a

…ransision

Merge remote-tracking branch 'origin/gh_2034_part2' into GH-2045-if-t…

fe1d3bb

…ransision

GH-2045 Add get_data_dir()

a482982

GH-2045 Add constructor for transition from block_state_legacy to blo…

4e631ae

…ck_state. Add missing set of transactions in block.

GH-2045 Fix when active_producer_policy updated, done at end of block…

abdfa68

… not at beginning.

GH-2045 Add transition from dpos fork database to instant finality fo…

61a595f

…rk database. Remove fetch_block_state_* methods.

heifner added the OCI Work exclusive to OCI team label Jan 14, 2024

greg7mdp approved these changes Jan 14, 2024

View reviewed changes

greg7mdp reviewed Jan 14, 2024

View reviewed changes

libraries/chain/controller.cpp Outdated Show resolved Hide resolved

libraries/chain/controller.cpp Outdated Show resolved Hide resolved

heifner added 4 commits January 15, 2024 09:31

GH-2045 Fix assert and use block exception type

c3e8423

GH-2045 Add block_token as a wrapper of block_state and block_state_l…

07f1036

…egacy. Also add block_exists as a performance improvement.

GH-2045 cleanup implementation

9b39982

GH-2045 Fix proposer_policy switching time. Enable instant-finality p…

03f2322

…roducer schedule test.

heifner marked this pull request as ready for review January 15, 2024 17:51

heifner requested review from linh2931 and greg7mdp January 15, 2024 17:52

greg7mdp approved these changes Jan 15, 2024

View reviewed changes

GH-2045 Serialize all block_header_state and block_state data

7962634

heifner added 4 commits January 15, 2024 12:50

GH-2045 Fix spelling

7c59cf3

GH-2045 Don't export qcs

d0a1e07

GH-2045 Fix for next_producers when no pending

b5dfdd4

GH-2045 Removed a warning and renamed hs_irreversible_block_num

35705b3

linh2931 reviewed Jan 15, 2024

View reviewed changes

libraries/chain/controller.cpp Outdated Show resolved Hide resolved

linh2931 reviewed Jan 15, 2024

View reviewed changes

libraries/chain/controller.cpp Show resolved Hide resolved

heifner added 2 commits January 15, 2024 16:05

GH-2045 Misc cleanup

6b62496

GH-2045 No need to close or open fork_db

852f69e

linh2931 approved these changes Jan 15, 2024

View reviewed changes

Base automatically changed from gh_2034_part2 to hotstuff_integration January 16, 2024 02:44

heifner merged commit cbb6b28 into hotstuff_integration Jan 16, 2024
26 checks passed

heifner deleted the GH-2045-if-transision branch January 16, 2024 02:45

heifner mentioned this pull request Jan 16, 2024

IF: Unification: IF transition of fork database usage #2045

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IF: Transition from dpos to instant-finality #2085

IF: Transition from dpos to instant-finality #2085

heifner commented Jan 14, 2024 •

edited

Loading

greg7mdp Jan 14, 2024

heifner Jan 15, 2024

greg7mdp Jan 15, 2024

linh2931 Jan 15, 2024

greg7mdp left a comment

greg7mdp Jan 15, 2024

heifner Jan 15, 2024

heifner commented Jan 15, 2024 •

edited

Loading

linh2931 left a comment

linh2931 Jan 15, 2024

linh2931 Jan 15, 2024

linh2931 Jan 15, 2024

heifner Jan 15, 2024

linh2931 commented Jan 15, 2024

ericpassmore commented Jan 17, 2024

	// Used for transition from dbpos to instant-finality
	// Used for transition from dpos to instant-finality

IF: Transition from dpos to instant-finality #2085

IF: Transition from dpos to instant-finality #2085

Conversation

heifner commented Jan 14, 2024 • edited Loading

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

greg7mdp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heifner commented Jan 15, 2024 • edited Loading

linh2931 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

linh2931 commented Jan 15, 2024

ericpassmore commented Jan 17, 2024

heifner commented Jan 14, 2024 •

edited

Loading

heifner commented Jan 15, 2024 •

edited

Loading