From 6d7fcaa4555843f364062f3bc8580671f3611c9e Mon Sep 17 00:00:00 2001 From: Matus Kysel Date: Thu, 29 Feb 2024 21:51:24 +0100 Subject: [PATCH] Revert "chore: adding version to pbft block" This reverts commit f31077e7f5b4a119a0cb946e4175fe3cbe077578. --- .../consensus/include/pbft/pbft_manager.hpp | 12 ++--- .../consensus/src/final_chain/final_chain.cpp | 8 ++- .../consensus/src/pbft/pbft_manager.cpp | 39 ++++++--------- libraries/core_libs/node/src/node.cpp | 5 +- libraries/types/pbft_block/CMakeLists.txt | 2 - .../pbft_block/include/pbft/pbft_block.hpp | 16 +----- .../include/pbft/pbft_block_extra_data.hpp | 48 ------------------ libraries/types/pbft_block/src/pbft_block.cpp | 27 ++-------- .../pbft_block/src/pbft_block_extra_data.cpp | 49 ------------------- tests/final_chain_test.cpp | 2 +- tests/pbft_chain_test.cpp | 2 +- 11 files changed, 35 insertions(+), 175 deletions(-) delete mode 100644 libraries/types/pbft_block/include/pbft/pbft_block_extra_data.hpp delete mode 100644 libraries/types/pbft_block/src/pbft_block_extra_data.cpp diff --git a/libraries/core_libs/consensus/include/pbft/pbft_manager.hpp b/libraries/core_libs/consensus/include/pbft/pbft_manager.hpp index 2877ac1f8b..4ba7185add 100644 --- a/libraries/core_libs/consensus/include/pbft/pbft_manager.hpp +++ b/libraries/core_libs/consensus/include/pbft/pbft_manager.hpp @@ -55,10 +55,10 @@ class PbftManager { public: using time_point = std::chrono::system_clock::time_point; - PbftManager(const GenesisConfig &conf, addr_t node_addr, std::shared_ptr db, - std::shared_ptr pbft_chain, std::shared_ptr vote_mgr, - std::shared_ptr dag_mgr, std::shared_ptr trx_mgr, - std::shared_ptr final_chain, secret_t node_sk); + PbftManager(const PbftConfig &conf, const blk_hash_t &dag_genesis_block_hash, addr_t node_addr, + std::shared_ptr db, std::shared_ptr pbft_chain, + std::shared_ptr vote_mgr, std::shared_ptr dag_mgr, + std::shared_ptr trx_mgr, std::shared_ptr final_chain, secret_t node_sk); ~PbftManager(); PbftManager(const PbftManager &) = delete; PbftManager(PbftManager &&) = delete; @@ -252,7 +252,7 @@ class PbftManager { * @brief Get PBFT committee size * @return PBFT committee size */ - size_t getPbftCommitteeSize() const { return config_.pbft.committee_size; } + size_t getPbftCommitteeSize() const { return config_.committee_size; } /** * @brief Test/enforce broadcastVotes() to actually send votes @@ -567,7 +567,7 @@ class PbftManager { const blk_hash_t dag_genesis_block_hash_; - const GenesisConfig &config_; + const PbftConfig &config_; std::condition_variable stop_cv_; std::mutex stop_mtx_; diff --git a/libraries/core_libs/consensus/src/final_chain/final_chain.cpp b/libraries/core_libs/consensus/src/final_chain/final_chain.cpp index c2b0b57893..b9ae9a2fe2 100644 --- a/libraries/core_libs/consensus/src/final_chain/final_chain.cpp +++ b/libraries/core_libs/consensus/src/final_chain/final_chain.cpp @@ -197,9 +197,8 @@ class FinalChainImpl final : public FinalChain { auto rewards_stats = rewards_.processStats(new_blk, transactions_gas_used, batch); const auto& [state_root, total_reward] = state_api_.distribute_rewards(rewards_stats); - auto blk_header = - append_block(batch, new_blk.pbft_blk->getBeneficiary(), new_blk.pbft_blk->getTimestamp(), kBlockGasLimit, - state_root, total_reward, new_blk.transactions, receipts, new_blk.pbft_blk->getExtraDataRlp()); + auto blk_header = append_block(batch, new_blk.pbft_blk->getBeneficiary(), new_blk.pbft_blk->getTimestamp(), + kBlockGasLimit, state_root, total_reward, new_blk.transactions, receipts); // Update number of executed DAG blocks and transactions auto num_executed_dag_blk = num_executed_dag_blk_ + finalized_dag_blk_hashes.size(); auto num_executed_trx = num_executed_trx_ + new_blk.transactions.size(); @@ -275,7 +274,7 @@ class FinalChainImpl final : public FinalChain { std::shared_ptr append_block(DB::Batch& batch, const addr_t& author, uint64_t timestamp, uint64_t gas_limit, const h256& state_root, u256 total_reward, const SharedTransactions& transactions = {}, - const TransactionReceipts& receipts = {}, const bytes& extra_data = {}) { + const TransactionReceipts& receipts = {}) { auto blk_header_ptr = std::make_shared(); auto& blk_header = *blk_header_ptr; auto last_block = block_header(); @@ -287,7 +286,6 @@ class FinalChainImpl final : public FinalChain { blk_header.gas_used = receipts.empty() ? 0 : receipts.back().cumulative_gas_used; blk_header.gas_limit = gas_limit; blk_header.total_reward = total_reward; - blk_header.extra_data = extra_data; dev::BytesMap trxs_trie, receipts_trie; dev::RLPStream rlp_strm; for (size_t i(0); i < transactions.size(); ++i) { diff --git a/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp b/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp index f510714bb8..00cf742dbe 100644 --- a/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp +++ b/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp @@ -14,7 +14,6 @@ #include #include -#include "config/version.hpp" #include "dag/dag.hpp" #include "final_chain/final_chain.hpp" #include "network/tarcap/packets_handlers/latest/pbft_sync_packet_handler.hpp" @@ -29,10 +28,11 @@ using vrf_output_t = vrf_wrapper::vrf_output_t; constexpr std::chrono::milliseconds kPollingIntervalMs{100}; constexpr PbftStep kMaxSteps{13}; // Need to be a odd number -PbftManager::PbftManager(const GenesisConfig &conf, addr_t node_addr, std::shared_ptr db, - std::shared_ptr pbft_chain, std::shared_ptr vote_mgr, - std::shared_ptr dag_mgr, std::shared_ptr trx_mgr, - std::shared_ptr final_chain, secret_t node_sk) +PbftManager::PbftManager(const PbftConfig &conf, const blk_hash_t &dag_genesis_block_hash, addr_t node_addr, + std::shared_ptr db, std::shared_ptr pbft_chain, + std::shared_ptr vote_mgr, std::shared_ptr dag_mgr, + std::shared_ptr trx_mgr, std::shared_ptr final_chain, + secret_t node_sk) : db_(std::move(db)), pbft_chain_(std::move(pbft_chain)), vote_mgr_(std::move(vote_mgr)), @@ -41,8 +41,8 @@ PbftManager::PbftManager(const GenesisConfig &conf, addr_t node_addr, std::share final_chain_(std::move(final_chain)), node_addr_(std::move(node_addr)), node_sk_(std::move(node_sk)), - kMinLambda(conf.pbft.lambda_ms), - dag_genesis_block_hash_(conf.dag_genesis_block.getHash()), + kMinLambda(conf.lambda_ms), + dag_genesis_block_hash_(dag_genesis_block_hash), config_(conf), proposed_blocks_(db_) { LOG_OBJECTS_CREATE("PBFT_MGR"); @@ -1085,16 +1085,8 @@ std::optional, std::vector block; - // TODO: Activate in next hardfork - /*if (HARDFORK) { - block = std::make_shared(prev_blk_hash, anchor_hash, order_hash, last_state_root, propose_period, - node_addr_, node_sk_, std::move(reward_votes_hashes), TARAXA_MAJOR_VERSION, - TARAXA_MINOR_VERSION, TARAXA_PATCH_VERSION, "T"); - } else */ - - block = std::make_shared(prev_blk_hash, anchor_hash, order_hash, last_state_root, propose_period, - node_addr_, node_sk_, std::move(reward_votes_hashes)); + auto block = std::make_shared(prev_blk_hash, anchor_hash, order_hash, last_state_root, propose_period, + node_addr_, node_sk_, std::move(reward_votes_hashes)); return {std::make_pair(std::move(block), std::move(reward_votes))}; } catch (const std::exception &e) { @@ -1170,11 +1162,10 @@ PbftManager::proposePbftBlock() { } blk_hash_t dag_block_hash; - if (ghost.size() <= config_.pbft.dag_blocks_size) { + if (ghost.size() <= config_.dag_blocks_size) { // Move back config_.ghost_path_move_back DAG blocks for DAG sycning - auto ghost_index = (ghost.size() < config_.pbft.ghost_path_move_back + 1) - ? 0 - : (ghost.size() - 1 - config_.pbft.ghost_path_move_back); + auto ghost_index = + (ghost.size() < config_.ghost_path_move_back + 1) ? 0 : (ghost.size() - 1 - config_.ghost_path_move_back); while (ghost_index < ghost.size() - 1) { if (ghost[ghost_index] != last_period_dag_anchor_block_hash) { break; @@ -1183,7 +1174,7 @@ PbftManager::proposePbftBlock() { } dag_block_hash = ghost[ghost_index]; } else { - dag_block_hash = ghost[config_.pbft.dag_blocks_size - 1]; + dag_block_hash = ghost[config_.dag_blocks_size - 1]; } if (dag_block_hash == dag_genesis_block_hash_) { @@ -1220,7 +1211,7 @@ PbftManager::proposePbftBlock() { } const auto &dag_block_weight = dag_blk->getGasEstimation(); - if (total_weight + dag_block_weight > config_.pbft.gas_limit) { + if (total_weight + dag_block_weight > config_.gas_limit) { break; } total_weight += dag_block_weight; @@ -1854,7 +1845,7 @@ bool PbftManager::checkBlockWeight(const std::vector &dag_blocks) cons const u256 total_weight = std::accumulate(dag_blocks.begin(), dag_blocks.end(), u256(0), [](u256 value, const auto &dag_block) { return value + dag_block.getGasEstimation(); }); - if (total_weight > config_.pbft.gas_limit) { + if (total_weight > config_.gas_limit) { return false; } return true; diff --git a/libraries/core_libs/node/src/node.cpp b/libraries/core_libs/node/src/node.cpp index f3efe09cb0..e482c12f8e 100644 --- a/libraries/core_libs/node/src/node.cpp +++ b/libraries/core_libs/node/src/node.cpp @@ -135,8 +135,9 @@ void FullNode::init() { auto slashing_manager = std::make_shared(final_chain_, trx_mgr_, gas_pricer_, conf_, kp_.secret()); vote_mgr_ = std::make_shared(node_addr, conf_.genesis.pbft, kp_.secret(), conf_.vrf_secret, db_, pbft_chain_, final_chain_, key_manager_, slashing_manager); - pbft_mgr_ = std::make_shared(conf_.genesis, node_addr, db_, pbft_chain_, vote_mgr_, dag_mgr_, trx_mgr_, - final_chain_, kp_.secret()); + pbft_mgr_ = + std::make_shared(conf_.genesis.pbft, conf_.genesis.dag_genesis_block.getHash(), node_addr, db_, + pbft_chain_, vote_mgr_, dag_mgr_, trx_mgr_, final_chain_, kp_.secret()); dag_block_proposer_ = std::make_shared( conf_.genesis.dag.block_proposer, dag_mgr_, trx_mgr_, final_chain_, db_, key_manager_, node_addr, getSecretKey(), getVrfSecretKey(), conf_.genesis.pbft.gas_limit, conf_.genesis.dag.gas_limit, conf_.genesis.state); diff --git a/libraries/types/pbft_block/CMakeLists.txt b/libraries/types/pbft_block/CMakeLists.txt index 8021357650..4e9a940ef8 100644 --- a/libraries/types/pbft_block/CMakeLists.txt +++ b/libraries/types/pbft_block/CMakeLists.txt @@ -1,11 +1,9 @@ set(HEADERS include/pbft/pbft_block.hpp - include/pbft/pbft_block_extra_data.hpp include/pbft/period_data.hpp ) set(SOURCES src/pbft_block.cpp - src/pbft_block_extra_data.cpp src/period_data.cpp ) diff --git a/libraries/types/pbft_block/include/pbft/pbft_block.hpp b/libraries/types/pbft_block/include/pbft/pbft_block.hpp index bab952cc0c..9aa3a35cbf 100644 --- a/libraries/types/pbft_block/include/pbft/pbft_block.hpp +++ b/libraries/types/pbft_block/include/pbft/pbft_block.hpp @@ -7,7 +7,6 @@ #include "common/types.hpp" #include "dag/dag_block.hpp" -#include "pbft_block_extra_data.hpp" #include "vote/vote.hpp" namespace taraxa { @@ -31,12 +30,11 @@ class PbftBlock { addr_t beneficiary_; sig_t signature_; std::vector reward_votes_; // Cert votes in previous period - std::optional extra_data_; public: PbftBlock(const blk_hash_t& prev_blk_hash, const blk_hash_t& dag_blk_hash_as_pivot, const blk_hash_t& order_hash, const blk_hash_t& prev_state_root, PbftPeriod period, const addr_t& beneficiary, const secret_t& sk, - std::vector&& reward_votes, const PbftBlockExtraData& extra_data = {}); + std::vector&& reward_votes); explicit PbftBlock(const dev::RLP& rlp); explicit PbftBlock(const bytes& RLP); @@ -119,18 +117,6 @@ class PbftBlock { */ auto getTimestamp() const { return timestamp_; } - /** - * @brief Get extra data - * @return extra data - */ - auto getExtraData() const { return extra_data_; } - - /** - * @brief Get extra data rlp - * @return extra data rlp - */ - auto getExtraDataRlp() const { return extra_data_.has_value() ? extra_data_->rlp() : bytes(); } - /** * @brief Get PBFT block proposer address * @return PBFT block proposer address diff --git a/libraries/types/pbft_block/include/pbft/pbft_block_extra_data.hpp b/libraries/types/pbft_block/include/pbft/pbft_block_extra_data.hpp deleted file mode 100644 index 19918c9904..0000000000 --- a/libraries/types/pbft_block/include/pbft/pbft_block_extra_data.hpp +++ /dev/null @@ -1,48 +0,0 @@ -#pragma once - -#include -#include -#include -#include - -#include "common/types.hpp" -#include "dag/dag_block.hpp" -#include "vote/vote.hpp" - -namespace taraxa { - -/** @addtogroup PBFT - * @{ - */ - -class PbftBlockExtraData { - public: - PbftBlockExtraData() {} - PbftBlockExtraData(const uint16_t major_version, const uint16_t minor_version, const uint16_t patch_version, - const uint16_t net_version, const std::string node_implementation); - PbftBlockExtraData(const bytes& data); - - /** - * @brief Get rlp - * @return rlp - */ - bytes rlp() const; - - /** - * @brief Get JSON - * @return Json - */ - Json::Value getJson() const; - - protected: - uint16_t major_version_; - uint16_t minor_version_; - uint16_t patch_version_; - uint16_t net_version_; - std::string node_implementation_; - static constexpr uint32_t kExtraDataMaxSize = 1024; -}; - -/** @}*/ - -} // namespace taraxa \ No newline at end of file diff --git a/libraries/types/pbft_block/src/pbft_block.cpp b/libraries/types/pbft_block/src/pbft_block.cpp index 0fc166a7c0..317ef637c5 100644 --- a/libraries/types/pbft_block/src/pbft_block.cpp +++ b/libraries/types/pbft_block/src/pbft_block.cpp @@ -11,32 +11,22 @@ namespace taraxa { PbftBlock::PbftBlock(bytes const& b) : PbftBlock(dev::RLP(b)) {} PbftBlock::PbftBlock(dev::RLP const& rlp) { - if (rlp.itemCount() == 9) { - dev::bytes extra_data_bytes; - util::rlp_tuple(util::RLPDecoderRef(rlp, true), prev_block_hash_, dag_block_hash_as_pivot_, order_hash_, - prev_state_root_hash_, period_, timestamp_, reward_votes_, extra_data_bytes, signature_); - extra_data_ = PbftBlockExtraData(extra_data_bytes); - } else { - util::rlp_tuple(util::RLPDecoderRef(rlp, true), prev_block_hash_, dag_block_hash_as_pivot_, order_hash_, - prev_state_root_hash_, period_, timestamp_, reward_votes_, signature_); - } - + util::rlp_tuple(util::RLPDecoderRef(rlp, true), prev_block_hash_, dag_block_hash_as_pivot_, order_hash_, + prev_state_root_hash_, period_, timestamp_, reward_votes_, signature_); calculateHash_(); checkUniqueRewardVotes(); } PbftBlock::PbftBlock(const blk_hash_t& prev_blk_hash, const blk_hash_t& dag_blk_hash_as_pivot, const blk_hash_t& order_hash, const blk_hash_t& prev_state_root, PbftPeriod period, - const addr_t& beneficiary, const secret_t& sk, std::vector&& reward_votes, - const PbftBlockExtraData& extra_data) + const addr_t& beneficiary, const secret_t& sk, std::vector&& reward_votes) : prev_block_hash_(prev_blk_hash), dag_block_hash_as_pivot_(dag_blk_hash_as_pivot), order_hash_(order_hash), prev_state_root_hash_(prev_state_root), period_(period), beneficiary_(beneficiary), - reward_votes_(reward_votes), - extra_data_(extra_data) { + reward_votes_(reward_votes) { timestamp_ = dev::utcTime(); signature_ = dev::sign(sk, sha3(false)); calculateHash_(); @@ -96,16 +86,13 @@ Json::Value PbftBlock::getJson() const { for (const auto& v : reward_votes_) { json["reward_votes"].append(v.toString()); } - json["extra_data"] = extra_data_->getJson(); return json; } // Using to setup PBFT block hash void PbftBlock::streamRLP(dev::RLPStream& strm, bool include_sig) const { - uint32_t rlp_count = (include_sig ? 8 : 7); - if (extra_data_.has_value()) rlp_count++; - strm.appendList(rlp_count); + strm.appendList(include_sig ? 8 : 7); strm << prev_block_hash_; strm << dag_block_hash_as_pivot_; strm << order_hash_; @@ -113,10 +100,6 @@ void PbftBlock::streamRLP(dev::RLPStream& strm, bool include_sig) const { strm << period_; strm << timestamp_; strm.appendVector(reward_votes_); - - if (extra_data_.has_value()) { - strm << extra_data_->rlp(); - } if (include_sig) { strm << signature_; } diff --git a/libraries/types/pbft_block/src/pbft_block_extra_data.cpp b/libraries/types/pbft_block/src/pbft_block_extra_data.cpp deleted file mode 100644 index c75d6fa3c2..0000000000 --- a/libraries/types/pbft_block/src/pbft_block_extra_data.cpp +++ /dev/null @@ -1,49 +0,0 @@ -#include "pbft/pbft_block_extra_data.hpp" - -#include - -#include "common/jsoncpp.hpp" - -namespace taraxa { - -PbftBlockExtraData::PbftBlockExtraData(const uint16_t major_version, const uint16_t minor_version, - const uint16_t patch_version, const uint16_t net_version, - const std::string node_implementation) - : major_version_(major_version), - minor_version_(minor_version), - patch_version_(patch_version), - net_version_(net_version), - node_implementation_(node_implementation) {} - -PbftBlockExtraData::PbftBlockExtraData(const bytes& data) { - if (data.size() > kExtraDataMaxSize) { - throw std::runtime_error("Pbft block invalid, extra data size over the limit"); - } - dev::RLP rlp(data); - util::rlp_tuple(util::RLPDecoderRef(rlp, true), major_version_, minor_version_, patch_version_, net_version_, - node_implementation_); -} - -bytes PbftBlockExtraData::rlp() const { - dev::RLPStream s; - s.appendList(5); - s << major_version_; - s << minor_version_; - s << patch_version_; - s << net_version_; - s << node_implementation_; - return s.invalidate(); -} - -Json::Value PbftBlockExtraData::getJson() const { - Json::Value json; - json["major_version"] = major_version_; - json["minor_version"] = minor_version_; - json["patch_version"] = patch_version_; - json["net_version"] = net_version_; - json["node_implementation"] = node_implementation_; - - return json; -} - -} // namespace taraxa \ No newline at end of file diff --git a/tests/final_chain_test.cpp b/tests/final_chain_test.cpp index 1ef9444b5e..c4f5d86d1f 100644 --- a/tests/final_chain_test.cpp +++ b/tests/final_chain_test.cpp @@ -106,7 +106,7 @@ struct FinalChainTest : WithDataDir { trxs.size(), [&](auto i) { return dev::rlp(i); }, [&](auto i) { return util::rlp_enc(receipts[i]); })); EXPECT_EQ(blk_h.gas_limit, cfg.genesis.pbft.gas_limit); - EXPECT_EQ(blk_h.extra_data, pbft_block->getExtraData()->rlp()); + EXPECT_EQ(blk_h.extra_data, bytes()); EXPECT_EQ(blk_h.nonce(), Nonce()); EXPECT_EQ(blk_h.difficulty(), 0); EXPECT_EQ(blk_h.mix_hash(), h256()); diff --git a/tests/pbft_chain_test.cpp b/tests/pbft_chain_test.cpp index b82d390031..018a6b0f59 100644 --- a/tests/pbft_chain_test.cpp +++ b/tests/pbft_chain_test.cpp @@ -17,7 +17,7 @@ namespace taraxa::core_tests { struct PbftChainTest : NodesTest {}; -TEST_F(PbftChainTest, serialize_deserialize_pbft_block) { +TEST_F(PbftChainTest, serialize_desiriablize_pbft_block) { auto node_cfgs = make_node_cfgs(1); dev::Secret sk(node_cfgs[0].node_secret); // Generate PBFT block sample