Skip to content

Commit

Permalink
chore: fix rewards stats saving
Browse files Browse the repository at this point in the history
  • Loading branch information
@MatusKysel
MatusKysel committed Oct 6, 2023
1 parent bdba3bc commit 1675a2f
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 19 deletions.
5 changes: 3 additions & 2 deletions libraries/core_libs/consensus/include/rewards/rewards_stats.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ class Stats {
* @param current_blk block to process
* @return vector<BlockStats> that should be processed at current block
*/
std::vector<BlockStats> processStats(const PeriodData& current_blk, const std::vector<gas_t>& trxs_gas_used);
std::vector<BlockStats> processStats(const PeriodData& current_blk, const std::vector<gas_t>& trxs_gas_used,
DbStorage::Batch& write_batch);

protected:
/**
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion libraries/core_libs/consensus/src/final_chain/final_chain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
9 changes: 5 additions & 4 deletions libraries/core_libs/consensus/src/rewards/rewards_stats.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -55,7 +55,8 @@ BlockStats Stats::getBlockStats(const PeriodData& blk, const std::vector<gas_t>&
return BlockStats{blk, trxs_fees, dpos_vote_count, kCommitteeSize};
}

std::vector<BlockStats> Stats::processStats(const PeriodData& current_blk, const std::vector<gas_t>& trxs_gas_used) {
std::vector<BlockStats> Stats::processStats(const PeriodData& current_blk, const std::vector<gas_t>& 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);
Expand All @@ -68,7 +69,7 @@ std::vector<BlockStats> 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 {};
}

Expand Down
34 changes: 22 additions & 12 deletions tests/rewards_stats_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <libdevcore/Common.h>
#include <libdevcore/CommonJS.h>

#include "storage/storage.hpp"
#include "test_util/gtest.hpp"
#include "test_util/samples.hpp"

Expand All @@ -31,16 +32,18 @@ class TestableBlockStats : public rewards::BlockStats {

TEST_F(RewardsStatsTest, defaultDistribution) {
auto db = std::make_shared<DbStorage>(data_dir / "db");
auto batch = db->createWriteBatch();

std::vector<std::shared_ptr<Vote>> 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) {
Expand All @@ -53,16 +56,18 @@ TEST_F(RewardsStatsTest, statsSaving) {
std::vector<addr_t> 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
Expand All @@ -79,6 +84,7 @@ TEST_F(RewardsStatsTest, statsSaving) {

TEST_F(RewardsStatsTest, statsCleaning) {
auto db = std::make_shared<DbStorage>(data_dir / "db");
auto batch = db->createWriteBatch();

// distribute every 5 blocks
HardforksConfig::RewardsDistributionMap distribution{{0, 5}};
Expand All @@ -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
Expand All @@ -112,6 +119,7 @@ TEST_F(RewardsStatsTest, statsProcessing) {
auto db = std::make_shared<DbStorage>(data_dir / "db");
// distribute every 10 blocks
auto rewards_stats = TestableRewardsStats({{0, 10}}, db);
auto batch = db->createWriteBatch();

std::vector<std::shared_ptr<Vote>> empty_votes;
std::vector<addr_t> block_authors;
Expand All @@ -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());
}
Expand All @@ -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) {
Expand All @@ -143,6 +151,7 @@ TEST_F(RewardsStatsTest, statsProcessing) {

TEST_F(RewardsStatsTest, distributionChange) {
auto db = std::make_shared<DbStorage>(data_dir / "db");
auto batch = db->createWriteBatch();

HardforksConfig::RewardsDistributionMap distribution{{6, 5}, {11, 2}};

Expand All @@ -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<DbStorage>(data_dir / "db");
auto batch = db->createWriteBatch();
auto pbft_proposer = dev::KeyPair::create();
auto dag_proposer = dev::KeyPair::create();

Expand All @@ -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<TestableBlockStats*>(&stats);
auto validators_stats = testable_stats->getValidatorStats();
Expand Down

0 comments on commit 1675a2f

Please sign in to comment.