From 29e399c7e102fe3078da266e30db2d1943fd0a25 Mon Sep 17 00:00:00 2001 From: Dmytro Kostenko Date: Fri, 28 Apr 2023 11:09:48 +0200 Subject: [PATCH] refactor: rewards stats passing --- .../common/include/common/range_view.hpp | 7 +-- .../include/final_chain/rewards_stats.hpp | 34 +++++++------ .../include/final_chain/state_api.hpp | 4 +- .../consensus/src/final_chain/final_chain.cpp | 25 ++++++---- .../src/final_chain/rewards_stats.cpp | 50 +++++++++---------- .../consensus/src/final_chain/state_api.cpp | 7 +-- .../pbft_block/include/pbft/pbft_block.hpp | 23 +++++---- submodules/taraxa-evm | 2 +- tests/final_chain_test.cpp | 31 ++++++++++-- tests/state_api_test.cpp | 6 +-- 10 files changed, 105 insertions(+), 84 deletions(-) diff --git a/libraries/common/include/common/range_view.hpp b/libraries/common/include/common/range_view.hpp index 5c93b77870..8f6cbf29d4 100644 --- a/libraries/common/include/common/range_view.hpp +++ b/libraries/common/include/common/range_view.hpp @@ -3,7 +3,7 @@ #include #include -namespace taraxa::util::range_view { +namespace taraxa::util { template struct RangeView { @@ -72,9 +72,4 @@ auto make_range_view(Seq const &seq) { return RangeView(seq); } -} // namespace taraxa::util::range_view - -namespace taraxa::util { -using range_view::make_range_view; -using range_view::RangeView; } // namespace taraxa::util diff --git a/libraries/core_libs/consensus/include/final_chain/rewards_stats.hpp b/libraries/core_libs/consensus/include/final_chain/rewards_stats.hpp index ee925e16db..4e0b6eb04f 100644 --- a/libraries/core_libs/consensus/include/final_chain/rewards_stats.hpp +++ b/libraries/core_libs/consensus/include/final_chain/rewards_stats.hpp @@ -16,20 +16,25 @@ namespace taraxa { class RewardsStats { public: /** - * @brief Process PeriodData and returns vector of validators, who included provided block.transactions as first in - * dag block, e.g. returned validator on position 2 included transaction block.transactions[2] as first in his dag - * block + * @brief setting block_author_, max_votes_weight_ and calls processStats function * - * @param block * @param dpos_vote_count - votes count for previous block * @param committee_size - * @return vector of validators */ - std::vector processStats(const PeriodData& block, uint64_t dpos_vote_count, uint32_t committee_size); + RewardsStats(const PeriodData& block, uint64_t dpos_vote_count, uint32_t committee_size); HAS_RLP_FIELDS private: + /** + * @brief Process PeriodData and returns vector of validators, who included provided block.transactions as first in + * dag block, e.g. returned validator on position 2 included transaction block.transactions[2] as first in his dag + * block + * + * @param block + * @return vector of validators + */ + void processStats(const PeriodData& block); /** * @brief In case unique tx_hash is provided, it is mapped to it's validator's address + validator's unique txs count * is incremented. If provided tx_hash was already processed, nothing happens @@ -56,15 +61,6 @@ class RewardsStats { */ bool addVote(const std::shared_ptr& vote); - /** - * @brief Prepares reward statistics bases on period data data - * - * @param sync_blk - * @param dpos_vote_count - votes count for previous block - * @param committee_size - */ - void initStats(const PeriodData& sync_blk, uint64_t dpos_vote_count, uint32_t committee_size); - private: struct ValidatorStats { // count of rewardable(with 1 or more unique transactions) DAG blocks produced by this validator @@ -76,8 +72,14 @@ class RewardsStats { HAS_RLP_FIELDS }; + // Pbft block author + addr_t block_author_; + // Transactions validators: tx hash -> validator that included it as first in his block - std::unordered_map txs_validators_; + std::unordered_map validator_by_tx_hash_; + + // Vector with all transactions validators + std::vector txs_validators_; // Txs stats: validator -> ValidatorStats std::unordered_map validators_stats_; diff --git a/libraries/core_libs/consensus/include/final_chain/state_api.hpp b/libraries/core_libs/consensus/include/final_chain/state_api.hpp index 5096eec020..1f9045dfac 100644 --- a/libraries/core_libs/consensus/include/final_chain/state_api.hpp +++ b/libraries/core_libs/consensus/include/final_chain/state_api.hpp @@ -43,9 +43,7 @@ class StateAPI { StateDescriptor get_last_committed_state_descriptor() const; const StateTransitionResult& transition_state(const EVMBlock& block, const util::RangeView& transactions, - const util::RangeView& transactions_validators = {}, - const util::RangeView& uncles = {}, - const RewardsStats& rewards_stats = {}); + const std::vector& rewards_stats = {}); void transition_state_commit(); void create_snapshot(PbftPeriod period); void prune(const dev::h256& state_root_to_keep, const std::vector& state_root_to_prune, 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 212a2a2127..9110de2b05 100644 --- a/libraries/core_libs/consensus/src/final_chain/final_chain.cpp +++ b/libraries/core_libs/consensus/src/final_chain/final_chain.cpp @@ -142,19 +142,26 @@ class FinalChainImpl final : public FinalChain { EthBlockNumber delegation_delay() const override { return delegation_delay_; } + std::vector prepare_rewards_stats_(const PeriodData& blk) { + std::vector rewards_stats; + uint64_t dpos_vote_count = kCommitteeSize; + + // Block zero + if (!blk.previous_block_cert_votes.empty()) [[likely]] { + dpos_vote_count = dpos_eligible_total_vote_count(blk.previous_block_cert_votes[0]->getPeriod() - 1); + } + + rewards_stats.emplace_back(blk, dpos_vote_count, kCommitteeSize); + + return rewards_stats; + } + std::shared_ptr finalize_(PeriodData&& new_blk, std::vector&& finalized_dag_blk_hashes, std::shared_ptr&& anchor) { auto batch = db_->createWriteBatch(); - RewardsStats rewards_stats; - uint64_t dpos_vote_count = kCommitteeSize; - // Block zero - if (!new_blk.previous_block_cert_votes.empty()) [[unlikely]] { - dpos_vote_count = dpos_eligible_total_vote_count(new_blk.previous_block_cert_votes[0]->getPeriod() - 1); - } - // returns list of validators for new_blk.transactions - const std::vector txs_validators = rewards_stats.processStats(new_blk, dpos_vote_count, kCommitteeSize); + auto rewards_stats = prepare_rewards_stats_(new_blk); block_applying_emitter_.emit(block_header()->number + 1); @@ -178,7 +185,7 @@ class FinalChainImpl final : public FinalChain { auto const& [exec_results, state_root, total_reward] = state_api_.transition_state({new_blk.pbft_blk->getBeneficiary(), kBlockGasLimit, new_blk.pbft_blk->getTimestamp(), BlockHeader::difficulty()}, - to_state_api_transactions(new_blk.transactions), txs_validators, {}, rewards_stats); + to_state_api_transactions(new_blk.transactions), rewards_stats); TransactionReceipts receipts; receipts.reserve(exec_results.size()); diff --git a/libraries/core_libs/consensus/src/final_chain/rewards_stats.cpp b/libraries/core_libs/consensus/src/final_chain/rewards_stats.cpp index 809a48d227..45f20a82ea 100644 --- a/libraries/core_libs/consensus/src/final_chain/rewards_stats.cpp +++ b/libraries/core_libs/consensus/src/final_chain/rewards_stats.cpp @@ -2,25 +2,33 @@ #include +#include "pbft/pbft_block.hpp" + namespace taraxa { +RewardsStats::RewardsStats(const PeriodData& block, uint64_t dpos_vote_count, uint32_t committee_size) + : block_author_(block.pbft_blk->getBeneficiary()), + max_votes_weight_(std::min(committee_size, dpos_vote_count)) { + processStats(block); +} + bool RewardsStats::addTransaction(const trx_hash_t& tx_hash, const addr_t& validator) { - auto found_tx = txs_validators_.find(tx_hash); + auto found_tx = validator_by_tx_hash_.find(tx_hash); // Already processed tx - if (found_tx != txs_validators_.end()) { + if (found_tx != validator_by_tx_hash_.end()) { return false; } // New tx - txs_validators_[tx_hash] = validator; + validator_by_tx_hash_[tx_hash] = validator; return true; } std::optional RewardsStats::getTransactionValidator(const trx_hash_t& tx_hash) { - auto found_tx = txs_validators_.find(tx_hash); - if (found_tx == txs_validators_.end()) { + auto found_tx = validator_by_tx_hash_.find(tx_hash); + if (found_tx == validator_by_tx_hash_.end()) { return {}; } @@ -52,12 +60,12 @@ std::set toTrxHashesSet(const SharedTransactions& transactions) { return block_transactions_hashes_; } -void RewardsStats::initStats(const PeriodData& sync_blk, uint64_t dpos_vote_count, uint32_t committee_size) { - txs_validators_.reserve(sync_blk.transactions.size()); - validators_stats_.reserve(std::max(sync_blk.dag_blocks.size(), sync_blk.previous_block_cert_votes.size())); - auto block_transactions_hashes_ = toTrxHashesSet(sync_blk.transactions); +void RewardsStats::processStats(const PeriodData& block) { + validator_by_tx_hash_.reserve(block.transactions.size()); + validators_stats_.reserve(std::max(block.dag_blocks.size(), block.previous_block_cert_votes.size())); + auto block_transactions_hashes_ = toTrxHashesSet(block.transactions); - for (const auto& dag_block : sync_blk.dag_blocks) { + for (const auto& dag_block : block.dag_blocks) { const addr_t& dag_block_author = dag_block.getSender(); bool has_unique_transactions = false; for (const auto& tx_hash : dag_block.getTrxs()) { @@ -78,34 +86,24 @@ void RewardsStats::initStats(const PeriodData& sync_blk, uint64_t dpos_vote_coun } } // total_unique_txs_count_ should be always equal to transactions count in block - assert(txs_validators_.size() == sync_blk.transactions.size()); + assert(validator_by_tx_hash_.size() == block.transactions.size()); - max_votes_weight_ = std::min(committee_size, dpos_vote_count); - for (const auto& vote : sync_blk.previous_block_cert_votes) { + for (const auto& vote : block.previous_block_cert_votes) { addVote(vote); } -} - -std::vector RewardsStats::processStats(const PeriodData& block, uint64_t dpos_vote_count, - uint32_t committee_size) { - initStats(block, dpos_vote_count, committee_size); - - // Dag blocks validators that included transactions to be executed as first in their blocks - std::vector txs_validators; - txs_validators.reserve(block.transactions.size()); + txs_validators_.reserve(block.transactions.size()); for (const auto& tx : block.transactions) { // Non-executed trxs auto tx_validator = getTransactionValidator(tx->getHash()); assert(tx_validator.has_value()); - txs_validators.push_back(*tx_validator); + txs_validators_.push_back(*tx_validator); } - - return txs_validators; } RLP_FIELDS_DEFINE(RewardsStats::ValidatorStats, dag_blocks_count_, vote_weight_) -RLP_FIELDS_DEFINE(RewardsStats, validators_stats_, total_dag_blocks_count_, total_votes_weight_, max_votes_weight_) +RLP_FIELDS_DEFINE(RewardsStats, block_author_, validators_stats_, txs_validators_, total_dag_blocks_count_, + total_votes_weight_, max_votes_weight_) } // namespace taraxa \ No newline at end of file diff --git a/libraries/core_libs/consensus/src/final_chain/state_api.cpp b/libraries/core_libs/consensus/src/final_chain/state_api.cpp index 7e18f6c408..8b0d369516 100644 --- a/libraries/core_libs/consensus/src/final_chain/state_api.cpp +++ b/libraries/core_libs/consensus/src/final_chain/state_api.cpp @@ -181,14 +181,11 @@ StateDescriptor StateAPI::get_last_committed_state_descriptor() const { const StateTransitionResult& StateAPI::transition_state(const EVMBlock& block, const util::RangeView& transactions, - const util::RangeView& transactions_validators, - const util::RangeView& uncles, - const RewardsStats& rewards_stats) { + const std::vector& rewards_stats) { result_buf_transition_state_.execution_results.clear(); rlp_enc_transition_state_.clear(); c_method_args_rlp( - this_c_, rlp_enc_transition_state_, result_buf_transition_state_, block, transactions, transactions_validators, - uncles, rewards_stats); + this_c_, rlp_enc_transition_state_, result_buf_transition_state_, block, transactions, rewards_stats); return result_buf_transition_state_; } diff --git a/libraries/types/pbft_block/include/pbft/pbft_block.hpp b/libraries/types/pbft_block/include/pbft/pbft_block.hpp index a830a3afa5..9aa3a35cbf 100644 --- a/libraries/types/pbft_block/include/pbft/pbft_block.hpp +++ b/libraries/types/pbft_block/include/pbft/pbft_block.hpp @@ -16,7 +16,7 @@ namespace taraxa { */ /** - * @brief The PbftBlockk class is a PBFT block class that includes PBFT block hash, previous PBFT block hash, DAG anchor + * @brief The PbftBlock class is a PBFT block class that includes PBFT block hash, previous PBFT block hash, DAG anchor * hash, DAG blocks ordering hash, period number, timestamp, proposer address, and proposer signature. */ class PbftBlock { @@ -35,8 +35,8 @@ class 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); - explicit PbftBlock(dev::RLP const& rlp); - explicit PbftBlock(bytes const& RLP); + explicit PbftBlock(const dev::RLP& rlp); + explicit PbftBlock(const bytes& RLP); /** * @brief Secure Hash Algorithm 3 @@ -77,33 +77,33 @@ class PbftBlock { * @param dag_blks DAG blocks hashes * @return PBFT block with DAG blocks in JSON */ - static Json::Value toJson(PbftBlock const& b, std::vector const& dag_blks); + static Json::Value toJson(const PbftBlock& b, const std::vector& dag_blks); /** * @brief Get PBFT block hash * @return PBFT block hash */ - auto const& getBlockHash() const { return block_hash_; } + const auto& getBlockHash() const { return block_hash_; } /** * @brief Get previous PBFT block hash * @return previous PBFT block hash */ - auto const& getPrevBlockHash() const { return prev_block_hash_; } + const auto& getPrevBlockHash() const { return prev_block_hash_; } /** * @brief Get DAG anchor hash for the finalized PBFT block * @return DAG anchor hash */ - auto const& getPivotDagBlockHash() const { return dag_block_hash_as_pivot_; } + const auto& getPivotDagBlockHash() const { return dag_block_hash_as_pivot_; } /** * @brief Get DAG blocks ordering hash * @return DAG blocks ordering hash */ - auto const& getOrderHash() const { return order_hash_; } + const auto& getOrderHash() const { return order_hash_; } - auto const& getPrevStateRoot() const { return prev_state_root_hash_; } + const auto& getPrevStateRoot() const { return prev_state_root_hash_; } /** * @brief Get period number @@ -121,7 +121,8 @@ class PbftBlock { * @brief Get PBFT block proposer address * @return PBFT block proposer address */ - auto const& getBeneficiary() const { return beneficiary_; } + const auto& getBeneficiary() const { return beneficiary_; } + const auto& getRewardVotes() const { return reward_votes_; } private: @@ -136,7 +137,7 @@ class PbftBlock { */ void checkUniqueRewardVotes(); }; -std::ostream& operator<<(std::ostream& strm, PbftBlock const& pbft_blk); +std::ostream& operator<<(std::ostream& strm, const PbftBlock& pbft_blk); /** @}*/ diff --git a/submodules/taraxa-evm b/submodules/taraxa-evm index 9eaa5f51ce..4415110195 160000 --- a/submodules/taraxa-evm +++ b/submodules/taraxa-evm @@ -1 +1 @@ -Subproject commit 9eaa5f51ce4dba40ad2c123bca058b2eab0cc948 +Subproject commit 4415110195038a0f20dd57c75e37a77c5745882f diff --git a/tests/final_chain_test.cpp b/tests/final_chain_test.cpp index a15be2ae36..781223a14b 100644 --- a/tests/final_chain_test.cpp +++ b/tests/final_chain_test.cpp @@ -49,12 +49,15 @@ struct FinalChainTest : WithDataDir { for (const auto& trx : trxs) { trx_hashes.emplace_back(trx->getHash()); } - DagBlock dag_blk({}, {}, {}, trx_hashes, {}, {}, secret_t::random()); + + auto proposer_keys = dev::KeyPair::create(); + DagBlock dag_blk({}, {}, {}, trx_hashes, {}, {}, proposer_keys.secret()); db->saveDagBlock(dag_blk); std::vector reward_votes_hashes; auto pbft_block = std::make_shared(kNullBlockHash, kNullBlockHash, kNullBlockHash, kNullBlockHash, expected_blk_num, - addr_t::random(), dev::KeyPair::create().secret(), std::move(reward_votes_hashes)); + addr_t::random(), proposer_keys.secret(), std::move(reward_votes_hashes)); + std::vector> votes; PeriodData period_data(pbft_block, votes); period_data.dag_blocks.push_back(dag_blk); @@ -62,7 +65,6 @@ struct FinalChainTest : WithDataDir { auto batch = db->createWriteBatch(); db->savePeriodData(period_data, batch); - db->commitWriteBatch(batch); auto result = SUT->finalize(std::move(period_data), {dag_blk.getHash()}).get(); @@ -447,6 +449,9 @@ TEST_F(FinalChainTest, failed_transaction_fee) { auto trx2_1 = std::make_shared(2, 101, 1, gas, dev::bytes(), sk, receiver); advance({trx1}); + auto blk = SUT->block_header(expected_blk_num); + auto proposer_balance = SUT->getBalance(blk->author); + EXPECT_EQ(proposer_balance.first, 21000); advance({trx2}); advance({trx3}); @@ -600,6 +605,26 @@ TEST_F(FinalChainTest, incorrect_estimation_regress) { } } +TEST_F(FinalChainTest, fee_rewards_distribution) { + auto sender_keys = dev::KeyPair::create(); + auto gas = 30000; + + const auto& receiver = dev::KeyPair::create().address(); + const auto& addr = sender_keys.address(); + const auto& sk = sender_keys.secret(); + cfg.genesis.state.initial_balances = {}; + cfg.genesis.state.initial_balances[addr] = 100000; + init(); + const auto gas_price = 1; + auto trx1 = std::make_shared(1, 100, gas_price, gas, dev::bytes(), sk, receiver); + + auto res = advance({trx1}); + auto gas_used = res->trx_receipts.front().gas_used; + auto blk = SUT->block_header(expected_blk_num); + auto proposer_balance = SUT->getBalance(blk->author); + EXPECT_EQ(proposer_balance.first, gas_used * gas_price); +} + // This test should be last as state_api isn't destructed correctly because of exception TEST_F(FinalChainTest, initial_validator_exceed_maximum_stake) { const dev::KeyPair key = dev::KeyPair::create(); diff --git a/tests/state_api_test.cpp b/tests/state_api_test.cpp index 49ce8fbe72..9d27f86220 100644 --- a/tests/state_api_test.cpp +++ b/tests/state_api_test.cpp @@ -25,9 +25,8 @@ struct TestBlock { h256 state_root; EVMBlock evm_block; vector transactions; - vector uncle_blocks; - RLP_FIELDS_DEFINE_INPLACE(hash, state_root, evm_block, transactions, uncle_blocks) + RLP_FIELDS_DEFINE_INPLACE(hash, state_root, evm_block, transactions) }; template @@ -202,8 +201,7 @@ TEST_F(StateAPITest, DISABLED_eth_mainnet_smoke) { progress_pct_log_threshold += 10; } auto const& test_block = test_blocks[blk_num]; - auto const& result = - SUT.transition_state(test_block.evm_block, test_block.transactions, {}, test_block.uncle_blocks); + auto const& result = SUT.transition_state(test_block.evm_block, test_block.transactions); ASSERT_EQ(result.state_root, test_block.state_root); SUT.transition_state_commit(); }