IF: Unification: Changes in controller to complete unification - Part 2 #2077
Conversation
…related changes.
| @@ -930,7 +939,7 @@ struct controller_impl { | |||
| cfg.state_size, false, cfg.db_map_mode ), | |||
| blog( cfg.blocks_dir, cfg.blog ), | |||
| block_data(block_data_t::block_data_variant{ | |||
| std::in_place_type<block_data_legacy_t>, // [greg todo] create correct type depending on whether IF activated | |||
| std::in_place_type<block_data_legacy_t>, // initial state is always dpos | |||
There was a problem hiding this comment.
Not sure that will be true after IF is enabled on startup, but fine for now.
There was a problem hiding this comment.
OK. I thought I asked and we said we would always go through the transition, but I was thinking it might not be true when we start from a snapshot.
There was a problem hiding this comment.
We will always go through the transition when starting a new chain. We have no immediate plans to add the feature of enabling protocol feature activation in genesis.
Startup of nodeos after transition will start with state (either from chainbase or snapshot) that is in IF mode.
| @@ -2917,7 +2931,8 @@ struct controller_impl { | |||
| } | |||
|
|
|||
| auto do_push = [&](auto& fork_db, auto& head) { | |||
| fork_db.add( bsp ); | |||
| if constexpr (std::is_same_v<BSP, typename std::decay_t<decltype(head)>>) | |||
There was a problem hiding this comment.
That is really ugly, would be nice to find a way not to have these if constexpr.
| } | ||
|
|
||
| void controller::push_block( controller::block_report& br, | ||
| const block_state_ptr& bsp, |
There was a problem hiding this comment.
Don't think we will want to public push_block like this. Users of controller shouldn't need to know which one to call.
There was a problem hiding this comment.
Sure, I agree and ideally the block_state struct should not be visible outside libraries/chain. But I thought that if create_block_state and create_block_state_future were needed maybe push_block as well? We can certainly remove these if not needed.
There was a problem hiding this comment.
Worse case we have some type that controller provides via something like create_block_start_state() that controller::push_block breaks apart and calls the appropriate controller_impl::push_block.
There was a problem hiding this comment.
This is fine for now until we get the create_block_state() and create_block_state_future() fixed.
IF: Implement proposer policy change
| } | ||
|
|
||
| #warning Add last_proposed_finalizer_policy_generation to snapshot_block_header_state_v3, see header file TODO | ||
|
|
||
| block_header_state_core block_header_state_core::next(const uint32_t last_qc_block_num, bool is_last_qc_strong) const { | ||
| block_header_state_core block_header_state_core::next(qc_info_t incoming) const { |
There was a problem hiding this comment.
passing by reference (indirection with a pointer) would actually be slower since the whole struct is 8 bytes.
| @@ -201,6 +207,29 @@ struct block_data_t { | |||
| return std::visit([](const auto& bd) -> const signed_block_ptr& { return bd.head->block; }, v); | |||
| } | |||
|
|
|||
| void replace_producer_keys( const public_key_type& key ) { | |||
| ilog("Replace producer keys with ${k}", ("k", key)); | |||
There was a problem hiding this comment.
I think @heifner added this, but I don't see the harm in logging it.
There was a problem hiding this comment.
It is public keys so no harm. Useful to see it working.
| if( s != controller::block_status::irreversible ) { | ||
| fork_db.add( bsp, true ); | ||
| } | ||
| if (s != controller::block_status::irreversible) { |
There was a problem hiding this comment.
No need to change those existing styles. Lots of places using this if( .
| @@ -21,11 +21,13 @@ namespace eosio::chain { | |||
| * | |||
| * An internal mutex is used to provide thread-safety. | |||
| */ | |||
| template<class bsp, class bhsp> // either [block_state_legacy_ptr, block_state_ptr], same with block_header_state_ptr | |||
| template<class bsp> // either block_state_legacy_ptr or block_state_ptr | |||
There was a problem hiding this comment.
Conventionally template type should be capitalized as BSP.
|
Note:start |
Resolves #2034.
Resolves #2047.