Skip to content

Commit

Permalink
adjust pillar votes period
Browse files Browse the repository at this point in the history
  • Loading branch information
JakubFornadel committed Jul 17, 2024
1 parent 35a7140 commit 818bdc0
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ class PbftManager {
bool already_next_voted_null_block_hash_ = false;
bool go_finish_state_ = false;
bool loop_back_finish_state_ = false;
bool already_placed_pillar_vote_ = false;
bool last_placed_pillar_vote_period_ = 0;

// Used to avoid cyclic logging in voting steps that are called repeatedly
bool printSecondFinishStepInfo_ = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ class PillarChainManager {
* @param pillar_block_hash
* @param node_sk
* @param broadcast_vote
* @return true if vote placed, otherwise false
* @return vote if it was created, otherwise nullptr
*/
bool genAndPlacePillarVote(PbftPeriod period, const blk_hash_t& pillar_block_hash, const secret_t& node_sk,
bool broadcast_vote);
std::shared_ptr<PillarVote> genAndPlacePillarVote(PbftPeriod period, const blk_hash_t& pillar_block_hash,
const secret_t& node_sk, bool broadcast_vote);

/**
* @brief Set network as a weak pointer
Expand Down
43 changes: 36 additions & 7 deletions libraries/core_libs/consensus/src/pbft/pbft_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ bool PbftManager::advancePeriod() {
resetPbftConsensus(1 /* round */);
broadcast_reward_votes_counter_ = 1;
rebroadcast_reward_votes_counter_ = 1;
already_placed_pillar_vote_ = false;
current_period_start_datetime_ = std::chrono::system_clock::now();

const auto new_period = getPbftPeriod();
Expand Down Expand Up @@ -738,6 +737,30 @@ bool PbftManager::placeVote(const std::shared_ptr<PbftVote> &vote, std::string_v
<< ", vote weight " << *vote->getWeight() << ", period " << period << ", round " << vote->getRound()
<< ", step " << vote->getStep();

// In case it is pbft with pillar block period and we have not voted yet, place a pillar vote (can be placed during
// any pbft step)
if (kGenesisConfig.state.hardforks.ficus_hf.isPbftWithPillarBlockPeriod(period) &&
last_placed_pillar_vote_period_ < period) {
std::optional<blk_hash_t> pillar_block_hash;
if (voted_block) {
// No need to check presence of extra data and pillar block hash - this was already validated in validatePbftBlock
pillar_block_hash = voted_block->getExtraData()->getPillarBlockHash();
} else {
const auto current_pillar_block = pillar_chain_mgr_->getCurrentPillarBlock();
// Check if the latest pillar block was created
if (current_pillar_block && current_pillar_block->getPeriod() == period - 1) {
pillar_block_hash = current_pillar_block->getHash();
}
}

if (pillar_block_hash.has_value()) {
const auto pillar_vote = pillar_chain_mgr_->genAndPlacePillarVote(period, *pillar_block_hash, node_sk_, true);
if (pillar_vote) {
last_placed_pillar_vote_period_ = pillar_vote->getPeriod();
}
}
}

return true;
}

Expand Down Expand Up @@ -1180,7 +1203,7 @@ PbftManager::proposePbftBlock() {
last_period_dag_anchor_block_hash = dag_genesis_block_hash_;
}

// Creates pillar block's extra data
// Creates pbft block's extra data
const auto extra_data = createPbftBlockExtraData(current_pbft_period);
if (!extra_data.has_value()) {
LOG(log_er_) << "Unable to propose pbft block due to wrong pillar block, pbft period: " << current_pbft_period;
Expand Down Expand Up @@ -1682,7 +1705,13 @@ void PbftManager::finalize_(PeriodData &&period_data, std::vector<h256> &&finali
// already finalized
if (final_chain_->dpos_is_eligible(period, node_addr_)) {
if (pillar_block) {
pillar_chain_mgr_->genAndPlacePillarVote(period, pillar_block->getHash(), node_sk_, periodDataQueueEmpty());
// Pillar votes are created in the next period (thus period + 1), this is optimization to create & broadcast it
// a bit faster
const auto pillar_vote = pillar_chain_mgr_->genAndPlacePillarVote(period + 1, pillar_block->getHash(), node_sk_,
periodDataQueueEmpty());
if (pillar_vote) {
last_placed_pillar_vote_period_ = pillar_vote->getPeriod();
}
} else {
LOG(log_er_) << "Unable to vote for pillar block with period " << period << ". Block was not created";
}
Expand Down Expand Up @@ -1713,7 +1742,7 @@ bool PbftManager::pushPbftBlock_(PeriodData &&period_data, std::vector<std::shar
LOG(log_er_) << "Cannot push PBFT block " << period_data.pbft_blk->getBlockHash() << ", period " << pbft_period
<< ". Not enough pillar votes for pillar block " << *pillar_block_hash << ". Request it";
if (auto net = network_.lock()) {
net->requestPillarBlockVotesBundle(pbft_period - 1, *pillar_block_hash);
net->requestPillarBlockVotesBundle(pbft_period, *pillar_block_hash);
}

return false;
Expand Down Expand Up @@ -2015,12 +2044,12 @@ bool PbftManager::validatePbftBlockPillarVotes(const PeriodData &period_data) co
}

const auto &pbft_block_hash = period_data.pbft_blk->getBlockHash();
const auto required_votes_period = period_data.pbft_blk->getPeriod() - 1;
const auto required_votes_period = period_data.pbft_blk->getPeriod();

const auto current_pillar_block = pillar_chain_mgr_->getCurrentPillarBlock();
if (current_pillar_block->getPeriod() != required_votes_period) {
if (current_pillar_block->getPeriod() + 1 != required_votes_period) {
LOG(log_er_) << "Sync pillar votes required period " << required_votes_period << " != "
<< " current pillar block period " << current_pillar_block->getPeriod();
<< " current pillar block period " << current_pillar_block->getPeriod() << " + 1";
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,17 @@ void PillarChainManager::saveNewPillarBlock(std::shared_ptr<PillarBlock> pillar_
current_pillar_block_vote_counts_ = std::move(data.vote_counts);
}

bool PillarChainManager::genAndPlacePillarVote(PbftPeriod period, const blk_hash_t& pillar_block_hash,
const secret_t& node_sk, bool broadcast_vote) {
std::shared_ptr<PillarVote> PillarChainManager::genAndPlacePillarVote(PbftPeriod period,
const blk_hash_t& pillar_block_hash,
const secret_t& node_sk, bool broadcast_vote) {
const auto vote = std::make_shared<PillarVote>(node_sk, period, pillar_block_hash);

// Broadcasts pillar vote
const auto vote_weight = addVerifiedPillarVote(vote);
if (!vote_weight) {
LOG(log_er_) << "Unable to gen pillar vote. Vote was not added to the verified votes. Vote hash "
<< vote->getHash();
return false;
return nullptr;
}
db_->saveOwnPillarBlockVote(vote);

Expand All @@ -139,7 +140,7 @@ bool PillarChainManager::genAndPlacePillarVote(PbftPeriod period, const blk_hash
<< vote->getPeriod() << ", weight " << vote_weight;
}

return true;
return vote;
}

bool PillarChainManager::finalizePillarBlock(const std::shared_ptr<PillarBlock>& pillar_block) {
Expand All @@ -153,7 +154,8 @@ bool PillarChainManager::finalizePillarBlock(const std::shared_ptr<PillarBlock>&
return false;
}

auto above_threshold_votes = pillar_votes_.getVerifiedVotes(pillar_block->getPeriod(), pillar_block->getHash(), true);
auto above_threshold_votes =
pillar_votes_.getVerifiedVotes(pillar_block->getPeriod() + 1, pillar_block->getHash(), true);
if (above_threshold_votes.empty()) {
LOG(log_tr_) << "Unable to finalize pillar block " << pillar_block->getHash()
<< ", period: " << pillar_block->getPeriod() << ". Not enough votes";
Expand All @@ -171,7 +173,7 @@ bool PillarChainManager::finalizePillarBlock(const std::shared_ptr<PillarBlock>&
last_finalized_pillar_block_ = pillar_block;

// Erase votes that are no longer needed
pillar_votes_.eraseVotes(last_finalized_pillar_block_->getPeriod());
pillar_votes_.eraseVotes(last_finalized_pillar_block_->getPeriod() + 1);
}

pillar_block_finalized_emitter_.emit(pillar_block_data);
Expand Down Expand Up @@ -207,19 +209,19 @@ bool PillarChainManager::isRelevantPillarVote(const std::shared_ptr<PillarVote>
// Empty current_pillar_block_ means there was no pillar block created yet at all
if (!current_pillar_block) [[unlikely]] {
// It could happen that other nodes created pillar block and voted for it before this node
if (vote->getPeriod() != kFicusHfConfig.firstPillarBlockPeriod()) {
if (vote->getPeriod() != kFicusHfConfig.firstPillarBlockPeriod() + 1) {
LOG(log_nf_) << "Received vote's period " << vote_period << ", no pillar block created yet. Accepting votes with "
<< kFicusHfConfig.firstPillarBlockPeriod() << " period";
<< kFicusHfConfig.firstPillarBlockPeriod() + 1 << " period";
return false;
}
} else if (vote_period == current_pillar_block->getPeriod()) [[likely]] {
} else if (vote_period == current_pillar_block->getPeriod() + 1) [[likely]] {
if (vote->getBlockHash() != current_pillar_block->getHash()) {
LOG(log_nf_) << "Received vote's block hash " << vote->getBlockHash() << " != current pillar block hash "
<< current_pillar_block->getHash();
return false;
}
} else if (vote_period !=
current_pillar_block->getPeriod() + kFicusHfConfig.pillar_blocks_interval /* +1 future pillar block */) {
} else if (vote_period != current_pillar_block->getPeriod() + kFicusHfConfig.pillar_blocks_interval +
1 /* future pillar block votes */) {
LOG(log_nf_) << "Received vote's period " << vote_period << ", current pillar block period "
<< current_pillar_block->getPeriod();
return false;
Expand All @@ -245,9 +247,7 @@ bool PillarChainManager::validatePillarVote(const std::shared_ptr<PillarVote> vo

// Check if signer is eligible validator
try {
// Note: period is used instead of period - 1 because pillar votes are created only after pbft block with <period>
// is finalized
if (!final_chain_->dpos_is_eligible(period, validator)) {
if (!final_chain_->dpos_is_eligible(period - 1, validator)) {
LOG(log_er_) << "Validator is not eligible. Pillar vote " << vote->getHash();
return false;
}
Expand All @@ -268,9 +268,7 @@ bool PillarChainManager::validatePillarVote(const std::shared_ptr<PillarVote> vo
uint64_t PillarChainManager::addVerifiedPillarVote(const std::shared_ptr<PillarVote>& vote) {
uint64_t validator_vote_count = 0;
try {
// Note: period is used instead of period - 1 because pillar votes are created only after pbft block with <period>
// is finalized
validator_vote_count = final_chain_->dpos_eligible_vote_count(vote->getPeriod(), vote->getVoterAddr());
validator_vote_count = final_chain_->dpos_eligible_vote_count(vote->getPeriod() - 1, vote->getVoterAddr());
} catch (state_api::ErrFutureBlock& e) {
LOG(log_er_) << "Pillar vote " << vote->getHash() << " with period " << vote->getPeriod()
<< " is too far ahead of DPOS. " << e.what();
Expand All @@ -284,9 +282,7 @@ uint64_t PillarChainManager::addVerifiedPillarVote(const std::shared_ptr<PillarV
}

if (!pillar_votes_.periodDataInitialized(vote->getPeriod())) {
// Note: period is used instead of period - 1 because pillar votes are created only after pbft block with <period>
// is finalized
const auto threshold = getPillarConsensusThreshold(vote->getPeriod());
const auto threshold = getPillarConsensusThreshold(vote->getPeriod() - 1);
if (!threshold) {
LOG(log_er_) << "Unable to get pillar consensus threshold for period " << vote->getPeriod();
return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ void GetPillarVotesBundlePacketHandler::process(const threadpool::PacketData &pa
peer->markPillarVoteAsKnown(vote->getHash());
}

LOG(log_nf_) << "Pillar votes bundle sent to " << peer->getId();
LOG(log_nf_) << "Pillar votes bundle for period " << period << " ans hash " << pillar_block_hash << " sent to "
<< peer->getId();
}
}

Expand Down

0 comments on commit 818bdc0

Please sign in to comment.