From 1675a2f14959b4bbef0a324f1fb8d21b6cba4349 Mon Sep 17 00:00:00 2001 From: Matus Kysel Date: Fri, 6 Oct 2023 12:02:55 +0200 Subject: [PATCH] chore: fix rewards stats saving --- .../include/rewards/rewards_stats.hpp | 5 +-- .../consensus/src/final_chain/final_chain.cpp | 2 +- .../consensus/src/rewards/rewards_stats.cpp | 9 ++--- tests/rewards_stats_test.cpp | 34 ++++++++++++------- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/libraries/core_libs/consensus/include/rewards/rewards_stats.hpp b/libraries/core_libs/consensus/include/rewards/rewards_stats.hpp index c84a94a4a6..1ca0c1e485 100644 --- a/libraries/core_libs/consensus/include/rewards/rewards_stats.hpp +++ b/libraries/core_libs/consensus/include/rewards/rewards_stats.hpp @@ -18,7 +18,8 @@ class Stats { * @param current_blk block to process * @return vector that should be processed at current block */ - std::vector processStats(const PeriodData& current_blk, const std::vector& trxs_gas_used); + std::vector processStats(const PeriodData& current_blk, const std::vector& trxs_gas_used, + DbStorage::Batch& write_batch); protected: /** @@ -38,7 +39,7 @@ class Stats { /** * @brief saves stats to database to not lose this data in case of node restart */ - void saveBlockStats(uint64_t number, const BlockStats& stats); + void saveBlockStats(uint64_t number, const BlockStats& stats, DbStorage::Batch& write_batch); /** * @brief called on start of new rewards interval. clears blocks_stats_ collection * and removes all data saved in db column 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 2d3da2b1c3..6ac4e68b80 100644 --- a/libraries/core_libs/consensus/src/final_chain/final_chain.cpp +++ b/libraries/core_libs/consensus/src/final_chain/final_chain.cpp @@ -197,7 +197,7 @@ class FinalChainImpl final : public FinalChain { }); } - auto rewards_stats = rewards_.processStats(new_blk, transactions_gas_used); + 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(), diff --git a/libraries/core_libs/consensus/src/rewards/rewards_stats.cpp b/libraries/core_libs/consensus/src/rewards/rewards_stats.cpp index af27d8c7d4..ebf3a6a6f3 100644 --- a/libraries/core_libs/consensus/src/rewards/rewards_stats.cpp +++ b/libraries/core_libs/consensus/src/rewards/rewards_stats.cpp @@ -19,11 +19,11 @@ void Stats::loadFromDb() { } } -void Stats::saveBlockStats(uint64_t period, const BlockStats& stats) { +void Stats::saveBlockStats(uint64_t period, const BlockStats& stats, DbStorage::Batch& write_batch) { dev::RLPStream encoding; stats.rlp(encoding); - db_->insert(DB::Columns::block_rewards_stats, period, encoding.out()); + db_->insert(write_batch, DB::Columns::block_rewards_stats, period, encoding.out()); } uint32_t Stats::getCurrentDistributionFrequency(uint64_t current_block) const { @@ -55,7 +55,8 @@ BlockStats Stats::getBlockStats(const PeriodData& blk, const std::vector& return BlockStats{blk, trxs_fees, dpos_vote_count, kCommitteeSize}; } -std::vector Stats::processStats(const PeriodData& current_blk, const std::vector& trxs_gas_used) { +std::vector Stats::processStats(const PeriodData& current_blk, const std::vector& trxs_gas_used, + DbStorage::Batch& write_batch) { const auto current_period = current_blk.pbft_blk->getPeriod(); const auto frequency = getCurrentDistributionFrequency(current_period); auto block_stats = getBlockStats(current_blk, trxs_gas_used); @@ -68,7 +69,7 @@ std::vector Stats::processStats(const PeriodData& current_blk, const // Blocks between distribution. Process and save for future processing if (current_period % frequency != 0) { // Save to db, so in case of restart data could be just loaded for the period - saveBlockStats(current_period, *blocks_stats_.rbegin()); + saveBlockStats(current_period, *blocks_stats_.rbegin(), write_batch); return {}; } diff --git a/tests/rewards_stats_test.cpp b/tests/rewards_stats_test.cpp index 0c478e4e99..194232d872 100644 --- a/tests/rewards_stats_test.cpp +++ b/tests/rewards_stats_test.cpp @@ -5,6 +5,7 @@ #include #include +#include "storage/storage.hpp" #include "test_util/gtest.hpp" #include "test_util/samples.hpp" @@ -31,16 +32,18 @@ class TestableBlockStats : public rewards::BlockStats { TEST_F(RewardsStatsTest, defaultDistribution) { auto db = std::make_shared(data_dir / "db"); + auto batch = db->createWriteBatch(); std::vector> empty_votes; auto rewards_stats = TestableRewardsStats({}, db); for (auto i = 1; i < 5; ++i) { PeriodData block(make_simple_pbft_block(blk_hash_t(i), i), empty_votes); - auto stats = rewards_stats.processStats(block, {}); + auto stats = rewards_stats.processStats(block, {}, batch); ASSERT_EQ(stats.size(), 1); ASSERT_TRUE(rewards_stats.getStats().empty()); } + db->commitWriteBatch(batch); } TEST_F(RewardsStatsTest, statsSaving) { @@ -53,16 +56,18 @@ TEST_F(RewardsStatsTest, statsSaving) { std::vector block_authors; { auto rewards_stats = TestableRewardsStats(distribution, db); + auto batch = db->createWriteBatch(); for (auto i = 1; i < 5; ++i) { auto kp = dev::KeyPair::create(); block_authors.push_back(kp.address()); PeriodData block(make_simple_pbft_block(blk_hash_t(i), i, kp.secret()), empty_votes); - auto stats = rewards_stats.processStats(block, {}); + auto stats = rewards_stats.processStats(block, {}, batch); ASSERT_EQ(rewards_stats.getStats().size(), block_authors.size()); ASSERT_TRUE(stats.empty()); } + db->commitWriteBatch(batch); } { // Load from db @@ -79,6 +84,7 @@ TEST_F(RewardsStatsTest, statsSaving) { TEST_F(RewardsStatsTest, statsCleaning) { auto db = std::make_shared(data_dir / "db"); + auto batch = db->createWriteBatch(); // distribute every 5 blocks HardforksConfig::RewardsDistributionMap distribution{{0, 5}}; @@ -93,14 +99,15 @@ TEST_F(RewardsStatsTest, statsCleaning) { block_authors.push_back(kp.address()); PeriodData block(make_simple_pbft_block(blk_hash_t(i), i, kp.secret()), empty_votes); - auto stats = rewards_stats.processStats(block, {}); + auto stats = rewards_stats.processStats(block, {}, batch); ASSERT_EQ(rewards_stats.getStats().size(), block_authors.size()); ASSERT_TRUE(stats.empty()); } - + db->commitWriteBatch(batch); // Process block 5 after which we should have no stats elements in db PeriodData block(make_simple_pbft_block(blk_hash_t(5), 5), empty_votes); - rewards_stats.processStats(block, {}); + rewards_stats.processStats(block, {}, batch); + db->commitWriteBatch(batch); } // Load from db @@ -112,6 +119,7 @@ TEST_F(RewardsStatsTest, statsProcessing) { auto db = std::make_shared(data_dir / "db"); // distribute every 10 blocks auto rewards_stats = TestableRewardsStats({{0, 10}}, db); + auto batch = db->createWriteBatch(); std::vector> empty_votes; std::vector block_authors; @@ -122,7 +130,7 @@ TEST_F(RewardsStatsTest, statsProcessing) { block_authors.push_back(kp.address()); PeriodData block(make_simple_pbft_block(blk_hash_t(i), i, kp.secret()), empty_votes); - auto stats = rewards_stats.processStats(block, {}); + auto stats = rewards_stats.processStats(block, {}, batch); ASSERT_TRUE(stats.empty()); ASSERT_EQ(rewards_stats.getStats().size(), block_authors.size()); } @@ -131,7 +139,7 @@ TEST_F(RewardsStatsTest, statsProcessing) { block_authors.push_back(kp.address()); PeriodData block(make_simple_pbft_block(blk_hash_t(10), 10, kp.secret()), empty_votes); - auto stats = rewards_stats.processStats(block, {}); + auto stats = rewards_stats.processStats(block, {}, batch); ASSERT_EQ(stats.size(), block_authors.size()); for (size_t i = 0; i < stats.size(); ++i) { @@ -143,6 +151,7 @@ TEST_F(RewardsStatsTest, statsProcessing) { TEST_F(RewardsStatsTest, distributionChange) { auto db = std::make_shared(data_dir / "db"); + auto batch = db->createWriteBatch(); HardforksConfig::RewardsDistributionMap distribution{{6, 5}, {11, 2}}; @@ -152,26 +161,27 @@ TEST_F(RewardsStatsTest, distributionChange) { uint64_t period = 1; for (; period <= 5; ++period) { PeriodData block(make_simple_pbft_block(blk_hash_t(period), period), empty_votes); - auto stats = rewards_stats.processStats(block, {}); + auto stats = rewards_stats.processStats(block, {}, batch); ASSERT_FALSE(stats.empty()); } { // make blocks [1,9] and process them. output of processStats should be empty for (; period < 10; ++period) { PeriodData block(make_simple_pbft_block(blk_hash_t(period), period), empty_votes); - auto stats = rewards_stats.processStats(block, {}); + auto stats = rewards_stats.processStats(block, {}, batch); ASSERT_TRUE(stats.empty()); } PeriodData block(make_simple_pbft_block(blk_hash_t(period), period), empty_votes); - auto stats = rewards_stats.processStats(block, {}); + auto stats = rewards_stats.processStats(block, {}, batch); } PeriodData block(make_simple_pbft_block(blk_hash_t(period), period), empty_votes); - auto stats = rewards_stats.processStats(block, {}); + auto stats = rewards_stats.processStats(block, {}, batch); } TEST_F(RewardsStatsTest, feeRewards) { auto db = std::make_shared(data_dir / "db"); + auto batch = db->createWriteBatch(); auto pbft_proposer = dev::KeyPair::create(); auto dag_proposer = dev::KeyPair::create(); @@ -198,7 +208,7 @@ TEST_F(RewardsStatsTest, feeRewards) { period_data.dag_blocks.push_back(dag_blk); period_data.transactions = {trx}; - auto stats = rewards_stats.processStats(period_data, {trx_gas_fee}).front(); + auto stats = rewards_stats.processStats(period_data, {trx_gas_fee}, batch).front(); auto testable_stats = reinterpret_cast(&stats); auto validators_stats = testable_stats->getValidatorStats();