From 599e4eba736b2996b5455a22fae87d9abfa11a29 Mon Sep 17 00:00:00 2001 From: Jakub Fornadel Date: Thu, 11 Jul 2024 14:59:41 -0700 Subject: [PATCH] small fixes --- for_devs/local-net | 2 +- .../config_jsons/default/default_genesis.json | 2 +- .../config_jsons/testnet/testnet_genesis.json | 2 -- .../consensus/include/pbft/pbft_manager.hpp | 2 +- .../consensus/src/pbft/pbft_manager.cpp | 17 ++++++++--------- .../src/pillar_chain/pillar_chain_manager.cpp | 10 ++++++---- .../common/ext_bls_sig_packet_handler.cpp | 3 ++- .../latest/pillar_vote_packet_handler.cpp | 6 ++++-- tests/pillar_chain_test.cpp | 6 ++++-- 9 files changed, 27 insertions(+), 23 deletions(-) diff --git a/for_devs/local-net b/for_devs/local-net index 8add8bec84..9292441ff0 100755 --- a/for_devs/local-net +++ b/for_devs/local-net @@ -1,4 +1,4 @@ -#!/usr/bin/env python3 +#!/usr/bin/env python3.11 import click import subprocess diff --git a/libraries/cli/include/cli/config_jsons/default/default_genesis.json b/libraries/cli/include/cli/config_jsons/default/default_genesis.json index c35ad3c10f..90ce1d6eb7 100644 --- a/libraries/cli/include/cli/config_jsons/default/default_genesis.json +++ b/libraries/cli/include/cli/config_jsons/default/default_genesis.json @@ -116,7 +116,7 @@ "generated_rewards": "0x0" }, "ficus_hf": { - "block_num": 50, + "block_num": 48, "pillar_blocks_interval": 16, "bridge_contract_address": "0xcAF2b453FE8382a4B8110356DF0508f6d71F22BF" } diff --git a/libraries/cli/include/cli/config_jsons/testnet/testnet_genesis.json b/libraries/cli/include/cli/config_jsons/testnet/testnet_genesis.json index 7cf900839c..8e8bf4af8f 100644 --- a/libraries/cli/include/cli/config_jsons/testnet/testnet_genesis.json +++ b/libraries/cli/include/cli/config_jsons/testnet/testnet_genesis.json @@ -152,8 +152,6 @@ "ficus_hf": { "block_num": 1000, "pillar_blocks_interval": 100, - "pillar_chain_sync_interval": 25, - "pbft_inclusion_delay": 6, "bridge_contract_address": "0xcAF2b453FE8382a4B8110356DF0508f6d71F22BF" } } diff --git a/libraries/core_libs/consensus/include/pbft/pbft_manager.hpp b/libraries/core_libs/consensus/include/pbft/pbft_manager.hpp index 7b607a009c..ba157cb2de 100644 --- a/libraries/core_libs/consensus/include/pbft/pbft_manager.hpp +++ b/libraries/core_libs/consensus/include/pbft/pbft_manager.hpp @@ -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 last_placed_pillar_vote_period_ = 0; + PbftPeriod last_placed_pillar_vote_period_ = 0; // Used to avoid cyclic logging in voting steps that are called repeatedly bool printSecondFinishStepInfo_ = true; diff --git a/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp b/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp index 0123a19d81..abfce2f9c8 100644 --- a/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp +++ b/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp @@ -1204,10 +1204,14 @@ PbftManager::proposePbftBlock() { } // 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; - return {}; + std::optional extra_data; + if (kGenesisConfig.state.hardforks.ficus_hf.isFicusHardfork(current_pbft_period)) { + extra_data = createPbftBlockExtraData(current_pbft_period); + if (!extra_data.has_value()) { + LOG(log_er_) << "Unable to propose block for period " << current_pbft_period << ", round " << current_pbft_round + << ". Empty extra data"; + return {}; + } } auto ghost = dag_mgr_->getGhostPath(last_period_dag_anchor_block_hash); @@ -1301,17 +1305,12 @@ PbftManager::proposePbftBlock() { } std::optional PbftManager::createPbftBlockExtraData(PbftPeriod pbft_period) const { - if (!kGenesisConfig.state.hardforks.ficus_hf.isFicusHardfork(pbft_period)) { - return {}; - } - std::optional pillar_block_hash; if (kGenesisConfig.state.hardforks.ficus_hf.isPbftWithPillarBlockPeriod(pbft_period)) { // Anchor pillar block hash into the pbft block const auto pillar_block = pillar_chain_mgr_->getCurrentPillarBlock(); if (!pillar_block) { LOG(log_er_) << "Missing pillar block, pbft period " << pbft_period; - assert(false); return {}; } diff --git a/libraries/core_libs/consensus/src/pillar_chain/pillar_chain_manager.cpp b/libraries/core_libs/consensus/src/pillar_chain/pillar_chain_manager.cpp index 0e224971f2..005adc460f 100644 --- a/libraries/core_libs/consensus/src/pillar_chain/pillar_chain_manager.cpp +++ b/libraries/core_libs/consensus/src/pillar_chain/pillar_chain_manager.cpp @@ -133,8 +133,8 @@ std::shared_ptr PillarChainManager::genAndPlacePillarVote(PbftPeriod if (auto net = network_.lock(); net && broadcast_vote) { net->gossipPillarBlockVote(vote); - LOG(log_nf_) << "Created & broadcast pillar vote " << vote->getHash() << " for block " << vote->getBlockHash() - << ", period " << vote->getPeriod() << ", weight " << vote_weight; + LOG(log_nf_) << "Placed pillar vote " << vote->getHash() << " for block " << vote->getBlockHash() << ", period " + << vote->getPeriod() << ", weight " << vote_weight; } else { LOG(log_nf_) << "Created pillar vote " << vote->getHash() << " for block " << vote->getBlockHash() << ", period " << vote->getPeriod() << ", weight " << vote_weight; @@ -291,11 +291,13 @@ uint64_t PillarChainManager::addVerifiedPillarVote(const std::shared_ptrgetHash() << ", validator " << vote->getVoterAddr(); + LOG(log_er_) << "Non-unique pillar vote " << vote->getHash() << ", period " << vote->getPeriod() << ", validator " + << vote->getVoterAddr(); return 0; } - LOG(log_nf_) << "Added pillar vote " << vote->getHash(); + LOG(log_nf_) << "Added pillar vote " << vote->getHash() << ", period " << vote->getPeriod() << ", pillar block hash " + << vote->getBlockHash(); if (const auto current_pillar_block = getCurrentPillarBlock(); current_pillar_block && vote->getBlockHash() == current_pillar_block->getHash()) { diff --git a/libraries/core_libs/network/src/tarcap/packets_handlers/latest/common/ext_bls_sig_packet_handler.cpp b/libraries/core_libs/network/src/tarcap/packets_handlers/latest/common/ext_bls_sig_packet_handler.cpp index 20e17e1ace..90b3ab940e 100644 --- a/libraries/core_libs/network/src/tarcap/packets_handlers/latest/common/ext_bls_sig_packet_handler.cpp +++ b/libraries/core_libs/network/src/tarcap/packets_handlers/latest/common/ext_bls_sig_packet_handler.cpp @@ -14,7 +14,8 @@ ExtPillarVotePacketHandler::ExtPillarVotePacketHandler( bool ExtPillarVotePacketHandler::processPillarVote(const std::shared_ptr &vote, const std::shared_ptr &peer) { if (!pillar_chain_manager_->isRelevantPillarVote(vote)) { - LOG(log_dg_) << "Drop irrelevant pillar vote " << vote->getHash() << " from peer " << peer->getId(); + LOG(log_dg_) << "Drop irrelevant pillar vote " << vote->getHash() << ", period " << vote->getPeriod() + << " from peer " << peer->getId(); return false; } diff --git a/libraries/core_libs/network/src/tarcap/packets_handlers/latest/pillar_vote_packet_handler.cpp b/libraries/core_libs/network/src/tarcap/packets_handlers/latest/pillar_vote_packet_handler.cpp index 8def36ac36..d0ef86ee62 100644 --- a/libraries/core_libs/network/src/tarcap/packets_handlers/latest/pillar_vote_packet_handler.cpp +++ b/libraries/core_libs/network/src/tarcap/packets_handlers/latest/pillar_vote_packet_handler.cpp @@ -36,7 +36,8 @@ void PillarVotePacketHandler::process(const threadpool::PacketData &packet_data, void PillarVotePacketHandler::onNewPillarVote(const std::shared_ptr &vote, bool rebroadcast) { for (const auto &peer : peers_state_->getAllPeers()) { if (peer.second->syncing_) { - LOG(log_dg_) << "Pillar vote " << vote->getHash() << " not sent to " << peer.first << " peer syncing"; + LOG(log_dg_) << "Pillar vote " << vote->getHash() << ", period " << vote->getPeriod() << " not sent to " + << peer.first << ". Peer syncing"; continue; } @@ -55,7 +56,8 @@ void PillarVotePacketHandler::sendPillarVote(const std::shared_ptr & if (sealAndSend(peer->getId(), SubprotocolPacketType::PillarVotePacket, std::move(s))) { peer->markPillarVoteAsKnown(vote->getHash()); - LOG(log_nf_) << "Pillar vote " << vote->getHash() << " sent to " << peer->getId(); + LOG(log_dg_) << "Pillar vote " << vote->getHash() << ", period " << vote->getPeriod() << " sent to " + << peer->getId(); } } diff --git a/tests/pillar_chain_test.cpp b/tests/pillar_chain_test.cpp index b7d20fe3aa..b0240e008c 100644 --- a/tests/pillar_chain_test.cpp +++ b/tests/pillar_chain_test.cpp @@ -241,8 +241,10 @@ TEST_F(PillarChainTest, pillar_chain_syncing) { pillar_blocks_count * node_cfgs[0].genesis.state.hardforks.ficus_hf.pillar_blocks_interval); // Trigger pillar votes syncing for the latest unfinalized pillar block - node2->getNetwork()->requestPillarBlockVotesBundle(node2_current_pillar_block_data->getPeriod() + 1, node2_current_pillar_block_data->getHash()); - // After pillar votes syncing, node 2 should not have already finalized pillar block with period pillar_blocks_count * pillar_blocks_interval + node2->getNetwork()->requestPillarBlockVotesBundle(node2_current_pillar_block_data->getPeriod() + 1, + node2_current_pillar_block_data->getHash()); + // After pillar votes syncing, node 2 should not have already finalized pillar block with period pillar_blocks_count * + // pillar_blocks_interval ASSERT_HAPPENS({20s, 250ms}, [&](auto& ctx) { WAIT_EXPECT_EQ(ctx, node2->getDB()->getLatestPillarBlockData()->block_->getPeriod(), pillar_blocks_count * node_cfgs[0].genesis.state.hardforks.ficus_hf.pillar_blocks_interval)