Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IF: Fix double confirming test failure #2054

Merged
merged 5 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions libraries/chain/block_header_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@

namespace eosio::chain {

#warning Add last_proposed_finalizer_policy_generation to snapshot_block_header_state_v3, see header file TODO

namespace detail {
uint32_t get_next_next_round_block_num( block_timestamp_type t, uint32_t block_num ) {
auto index = t.slot % config::producer_repetitions; // current index in current round
// (increment to the end of this round ) + next round
return block_num + (config::producer_repetitions - index) + config::producer_repetitions;
}
}

Comment on lines +7 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this in a new file in my PR so as not to duplicate, however we can checkin as-is and I'll remove in my PR.

block_header_state_core block_header_state_core::next(uint32_t last_qc_block_height, bool is_last_qc_strong) const {
// no state change if last_qc_block_height is the same
if (last_qc_block_height == this->last_qc_block_height) {
Expand Down
73 changes: 30 additions & 43 deletions libraries/chain/block_header_state_legacy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

namespace eosio { namespace chain {

#warning Add last_proposed_finalizer_policy_generation to snapshot_block_header_state_v3, see header file TODO

namespace detail {
bool is_builtin_activated( const protocol_feature_activation_set_ptr& pfa,
const protocol_feature_set& pfs,
Expand All @@ -15,12 +13,6 @@ namespace eosio { namespace chain {
const auto& protocol_features = pfa->protocol_features;
return digest && protocol_features.find(*digest) != protocol_features.end();
}

uint32_t get_next_next_round_block_num( block_timestamp_type t, uint32_t block_num ) {
auto index = t.slot % config::producer_repetitions; // current index in current round
// (increment to the end of this round ) + next round
return block_num + (config::producer_repetitions - index) + config::producer_repetitions;
}
}

producer_authority block_header_state_legacy::get_scheduled_producer( block_timestamp_type t )const {
Expand All @@ -43,9 +35,6 @@ namespace eosio { namespace chain {
return blocknums[ index ];
}

// create pending_block_header_state from this for `when`
// If hotstuff_activated then use new consensus values and simpler active schedule update.
// If notstuff is not activated then use previous pre-hotstuff consensus logic.
pending_block_header_state_legacy block_header_state_legacy::next( block_timestamp_type when,
uint16_t num_prev_blocks_to_confirm )const
{
Expand All @@ -57,40 +46,36 @@ namespace eosio { namespace chain {
(when = header.timestamp).slot++;
}

auto proauth = get_scheduled_producer(when);

auto itr = producer_to_last_produced.find( proauth.producer_name );
if( itr != producer_to_last_produced.end() ) {
EOS_ASSERT( itr->second < (block_num+1) - num_prev_blocks_to_confirm, producer_double_confirm,
"producer ${prod} double-confirming known range",
("prod", proauth.producer_name)("num", block_num+1)
("confirmed", num_prev_blocks_to_confirm)("last_produced", itr->second) );
}

result.block_num = block_num + 1;
result.previous = id;
result.timestamp = when;
result.confirmed = num_prev_blocks_to_confirm;
result.active_schedule_version = active_schedule.version;
result.prev_activated_protocol_features = activated_protocol_features;

auto proauth = get_scheduled_producer(when);

result.valid_block_signing_authority = proauth.authority;
result.producer = proauth.producer_name;
result.last_proposed_finalizer_policy_generation = last_proposed_finalizer_policy_generation;

result.blockroot_merkle = blockroot_merkle;
result.blockroot_merkle.append( id );

result.prev_pending_schedule = pending_schedule;

auto itr = producer_to_last_produced.find( proauth.producer_name );
if( itr != producer_to_last_produced.end() ) {
EOS_ASSERT( itr->second < (block_num+1) - num_prev_blocks_to_confirm, producer_double_confirm,
"producer ${prod} double-confirming known range",
("prod", proauth.producer_name)("num", block_num+1)
("confirmed", num_prev_blocks_to_confirm)("last_produced", itr->second) );
}

result.confirmed = num_prev_blocks_to_confirm;

/// grow the confirmed count
static_assert(std::numeric_limits<uint8_t>::max() >= (config::max_producers * 2 / 3) + 1, "8bit confirmations may not be able to hold all of the needed confirmations");

// This uses the previous block active_schedule because thats the "schedule" that signs and therefore confirms _this_ block
auto num_active_producers = active_schedule.producers.size();
uint32_t required_confs = (uint32_t)(num_active_producers * 2 / 3) + 1;

if( confirm_count.size() < config::maximum_tracked_dpos_confirmations ) {
result.confirm_count.reserve( confirm_count.size() + 1 );
result.confirm_count = confirm_count;
Expand All @@ -107,37 +92,39 @@ namespace eosio { namespace chain {
int32_t i = (int32_t)(result.confirm_count.size() - 1);
uint32_t blocks_to_confirm = num_prev_blocks_to_confirm + 1; /// confirm the head block too
while( i >= 0 && blocks_to_confirm ) {
--result.confirm_count[i];
//idump((confirm_count[i]));
if( result.confirm_count[i] == 0 )
{
--result.confirm_count[i];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the space removed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just copied over the 5.0 impl, including spaces

//idump((confirm_count[i]));
if( result.confirm_count[i] == 0 )
{
uint32_t block_num_for_i = result.block_num - (uint32_t)(result.confirm_count.size() - 1 - i);
new_dpos_proposed_irreversible_blocknum = block_num_for_i;
//idump((dpos2_lib)(block_num)(dpos_irreversible_blocknum));

if (i == static_cast<int32_t>(result.confirm_count.size() - 1)) {
result.confirm_count.resize(0);
} else {
memmove( &result.confirm_count[0], &result.confirm_count[i + 1], result.confirm_count.size() - i - 1);
result.confirm_count.resize( result.confirm_count.size() - i - 1 );
}

break;
}
--i;
--blocks_to_confirm;
}
--i;
--blocks_to_confirm;
}

result.dpos_proposed_irreversible_blocknum = new_dpos_proposed_irreversible_blocknum;
result.dpos_irreversible_blocknum = calc_dpos_last_irreversible( proauth.producer_name );


result.prev_pending_schedule = pending_schedule;

if( pending_schedule.schedule.producers.size() &&
result.dpos_irreversible_blocknum >= pending_schedule.schedule_lib_num )
{
result.active_schedule = pending_schedule.schedule;

flat_map<account_name,uint32_t> new_producer_to_last_produced;

for( const auto& pro : result.active_schedule.producers ) {
if( pro.producer_name == proauth.producer_name ) {
new_producer_to_last_produced[pro.producer_name] = result.block_num;
Expand All @@ -151,7 +138,7 @@ namespace eosio { namespace chain {
}
}
new_producer_to_last_produced[proauth.producer_name] = result.block_num;

result.producer_to_last_produced = std::move( new_producer_to_last_produced );

flat_map<account_name,uint32_t> new_producer_to_last_implied_irb;
Expand All @@ -168,9 +155,9 @@ namespace eosio { namespace chain {
}
}
}

result.producer_to_last_implied_irb = std::move( new_producer_to_last_implied_irb );

result.was_pending_promoted = true;
} else {
result.active_schedule = active_schedule;
Expand Down
4 changes: 4 additions & 0 deletions libraries/chain/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3986,6 +3986,10 @@ const producer_authority_schedule& controller::active_producers()const {
return my->pending->active_producers();
}

const producer_authority_schedule& controller::head_active_producers()const {
return my->block_data.head_active_schedule_auth();
}

const producer_authority_schedule& controller::pending_producers()const {
if( !(my->pending) )
return my->block_data.head_pending_schedule_auth(); // [greg todo] implement pending_producers correctly for IF mode
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#include <eosio/chain/block_header.hpp>
#include <eosio/chain/incremental_merkle.hpp>
#include <eosio/chain/protocol_feature_manager.hpp>
#include <eosio/chain/hotstuff/finalizer_policy.hpp>
#include <eosio/chain/chain_snapshot.hpp>
#include <future>

Expand Down Expand Up @@ -58,7 +57,6 @@ namespace detail {
uint32_t dpos_proposed_irreversible_blocknum = 0;
uint32_t dpos_irreversible_blocknum = 0;
producer_authority_schedule active_schedule;
uint32_t last_proposed_finalizer_policy_generation = 0; // TODO: Add to snapshot_block_header_state_v3
incremental_canonical_merkle_tree blockroot_merkle;
flat_map<account_name,uint32_t> producer_to_last_produced;
flat_map<account_name,uint32_t> producer_to_last_implied_irb;
Expand Down Expand Up @@ -195,7 +193,6 @@ FC_REFLECT( eosio::chain::detail::block_header_state_legacy_common,
(dpos_proposed_irreversible_blocknum)
(dpos_irreversible_blocknum)
(active_schedule)
(last_proposed_finalizer_policy_generation)
(blockroot_merkle)
(producer_to_last_produced)
(producer_to_last_implied_irb)
Expand Down
1 change: 1 addition & 0 deletions libraries/chain/include/eosio/chain/controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ namespace eosio::chain {
uint32_t pending_block_num()const;

const producer_authority_schedule& active_producers()const;
const producer_authority_schedule& head_active_producers()const;
const producer_authority_schedule& pending_producers()const;
std::optional<producer_authority_schedule> proposed_producers()const;

Expand Down
2 changes: 1 addition & 1 deletion libraries/testing/tester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ namespace eosio { namespace testing {

void base_tester::_start_block(fc::time_point block_time) {
auto head_block_number = control->head_block_num();
auto producer = control->active_producers().get_scheduled_producer(block_time);
auto producer = control->head_active_producers().get_scheduled_producer(block_time);

auto last_produced_block_num = control->last_irreversible_block_num();
auto itr = last_produced_block.find(producer.producer_name);
Expand Down
8 changes: 4 additions & 4 deletions unittests/deep-mind/deep-mind.log

Large diffs are not rendered by default.

Loading