Skip to content

Commit

Permalink
Merge pull request #128 from eosiosg/fix-cert-gen
Browse files Browse the repository at this point in the history 
further fix on pbft certs
  • Loading branch information
@Thaipanda
Thaipanda authored Oct 3, 2019
2 parents 6c8470d + 91ba180 commit cad626a
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 16 deletions.
4 changes: 2 additions & 2 deletions libraries/chain/include/eosio/chain/pbft_database.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,8 @@ namespace eosio {
block_info_type cal_pending_stable_checkpoint() const;
bool is_less_than_high_watermark(block_num_type bnum);
bool is_valid_prepared_certificate(const pbft_prepared_certificate& certificate, bool add_to_pbft_db = false);
bool is_valid_committed_certificate(const pbft_committed_certificate& certificate, bool add_to_pbft_db = false);
bool is_valid_longest_fork(const vector<producer_and_block_info>& producers, const block_info_type& cert_info);
bool is_valid_committed_certificate(const pbft_committed_certificate& certificate, bool add_to_pbft_db = false, bool at_the_top = false);
bool is_valid_longest_fork(const vector<producer_and_block_info>& producers, const block_info_type& cert_info, bool at_the_top = false);

producer_schedule_type lscb_active_producers() const;
vector<block_num_type>& get_updated_watermarks();
Expand Down
44 changes: 30 additions & 14 deletions libraries/chain/pbft_database.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -796,8 +796,8 @@ namespace eosio {
bool pbft_database::is_valid_prepared_certificate(const pbft_prepared_certificate& certificate, bool add_to_pbft_db) {
// an empty certificate is valid since it acts as a null digest in pbft.
if (certificate.empty()) return true;
// a certificate under lscb (no longer in fork_db) is also treated as null.
if (certificate.block_info.block_num() <= ctrl.last_stable_checkpoint_block_num()) return true;
// a certificate under lib is also treated as null.
if (certificate.block_info.block_num() <= ctrl.last_irreversible_block_num()) return true;

auto prepares = certificate.prepares;
auto prepares_metadata = vector<pbft_message_metadata<pbft_prepare>>{};
Expand Down Expand Up @@ -846,10 +846,10 @@ namespace eosio {
}
}

return should_prepared && is_valid_longest_fork(valid_prepares, certificate.block_info);
return should_prepared && is_valid_longest_fork(valid_prepares, certificate.block_info, true);
}

bool pbft_database::is_valid_committed_certificate(const pbft_committed_certificate& certificate, bool add_to_pbft_db) {
bool pbft_database::is_valid_committed_certificate(const pbft_committed_certificate& certificate, bool add_to_pbft_db, bool at_the_top) {
// an empty certificate is valid since it acts as a null digest in pbft.
if (certificate.empty()) return true;
// a certificate under lscb (no longer in fork_db) is also treated as null.
Expand Down Expand Up @@ -902,21 +902,24 @@ namespace eosio {
}
}

return should_committed && is_valid_longest_fork(valid_commits, certificate.block_info);
return should_committed && is_valid_longest_fork(valid_commits, certificate.block_info, at_the_top);
}

bool pbft_database::is_valid_longest_fork(const vector<producer_and_block_info>& block_infos, const block_info_type& cert_info) {
bool pbft_database::is_valid_longest_fork(const vector<producer_and_block_info>& block_infos, const block_info_type& cert_info, bool at_the_top ) {

//add all valid block_infos in to a temp multi_index, this implementation might contains heavier computation
auto local_index = local_state_multi_index_type();
auto& by_block_id_index = local_index.get<by_block_id>();
auto watermark = get_current_pbft_watermark();

auto next_watermark = get_current_pbft_watermark();

for (auto &e: block_infos) {

auto current = ctrl.fetch_block_state_by_id(e.second.block_id);

while ( current ) {
if (watermark == 0 || current->block_num <= watermark) {
if (next_watermark == 0 || current->block_num <= next_watermark) {

auto curr_itr = by_block_id_index.find(current->id);

if (curr_itr == by_block_id_index.end()) {
Expand Down Expand Up @@ -959,21 +962,28 @@ namespace eosio {

if (count >= threshold) {
by_block_id_index.modify(curr_itr,
[&](const validation_state_ptr &p) { p->enough = true; });
[&](const validation_state_ptr &p) { p->enough = true; });
}
}
}
}
current = ctrl.fetch_block_state_by_id(current->prev());
}
}
const auto& by_status_and_num_index = local_index.get<by_status_and_num>();
auto itr = by_status_and_num_index.begin();
if (itr == by_status_and_num_index.end()) return false;

validation_state_ptr psp = *itr;
if (!at_the_top) {
auto itr = by_block_id_index.find(cert_info.block_id);
if (itr == by_block_id_index.end()) return false;
return (*itr)->enough;

return psp->enough && psp->block_id == cert_info.block_id;
} else {
const auto &by_status_and_num_index = local_index.get<by_status_and_num>();
auto itr = by_status_and_num_index.begin();
if (itr == by_status_and_num_index.end()) return false;

validation_state_ptr psp = *itr;
return psp->enough && psp->block_id == cert_info.block_id;
}
}

bool pbft_database::is_valid_view_change(const pbft_view_change& vc, const public_key_type& pk) {
Expand Down Expand Up @@ -1076,6 +1086,12 @@ namespace eosio {
("hpcc", highest_pcc[i])("cc", committed_certs[i]));
}

if (!committed_certs.empty()) {
EOS_ASSERT(is_valid_committed_certificate(committed_certs.back(), false, true), pbft_exception,
"highest committed certificate is invalid, ${cc}",
("cc", committed_certs.back()));
}

EOS_ASSERT(highest_scp.block_info == nv.stable_checkpoint.block_info, pbft_exception,
"stable checkpoint does not match, should be ${hscp} but ${scp} given",
("hpcc", highest_scp)("pc", nv.stable_checkpoint));
Expand Down

0 comments on commit cad626a

Please sign in to comment.