Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use smartpointers for DagBlock and avoid unnecessary copies #2883

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ class DagBlockProposer {
* @param estimations transactions gas estimation
* @param vdf vdf with correct difficulty calculation
*/
DagBlock createDagBlock(DagFrontier&& frontier, level_t level, const SharedTransactions& trxs,
std::vector<uint64_t>&& estimations, VdfSortition&& vdf) const;
std::shared_ptr<DagBlock> createDagBlock(DagFrontier&& frontier, level_t level, const SharedTransactions& trxs,
std::vector<uint64_t>&& estimations, VdfSortition&& vdf) const;

/**
* @brief Gets transactions to include in the block - sharding not supported yet
Expand Down
13 changes: 7 additions & 6 deletions libraries/core_libs/consensus/include/dag/dag_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,15 @@ class DagManager : public std::enable_shared_from_this<DagManager> {
* @return verification result and all the transactions which are part of the block
*/
std::pair<VerifyBlockReturnType, SharedTransactions> verifyBlock(
const DagBlock &blk, const std::unordered_map<trx_hash_t, std::shared_ptr<Transaction>> &trxs = {});
const std::shared_ptr<DagBlock> &blk,
const std::unordered_map<trx_hash_t, std::shared_ptr<Transaction>> &trxs = {});

/**
* @brief Checks if block pivot and tips are in DAG
* @param blk Block to check
* @return true if all pivot and tips are in the DAG, false if some is missing with the hash of missing tips/pivot
*/
std::pair<bool, std::vector<blk_hash_t>> pivotAndTipsAvailable(DagBlock const &blk);
std::pair<bool, std::vector<blk_hash_t>> pivotAndTipsAvailable(const std::shared_ptr<DagBlock> &blk);

/**
* @brief adds verified DAG block in the DAG
Expand All @@ -95,8 +96,8 @@ class DagManager : public std::enable_shared_from_this<DagManager> {
* @param save if true save block and transactions to database
* @return true if block added successfully, false with the hash of missing tips/pivot
*/
std::pair<bool, std::vector<blk_hash_t>> addDagBlock(DagBlock &&blk, SharedTransactions &&trxs = {},
bool proposed = false,
std::pair<bool, std::vector<blk_hash_t>> addDagBlock(const std::shared_ptr<DagBlock> &blk,
SharedTransactions &&trxs = {}, bool proposed = false,
bool save = true); // insert to buffer if fail

/**
Expand Down Expand Up @@ -184,7 +185,7 @@ class DagManager : public std::enable_shared_from_this<DagManager> {
*/
std::pair<size_t, size_t> getNonFinalizedBlocksSize() const;

util::Event<DagManager, DagBlock> const block_verified_{};
util::Event<DagManager, std::shared_ptr<DagBlock>> const block_verified_{};

/**
* @brief Retrieves Dag Manager mutex, only to be used when finalizing pbft block
Expand Down Expand Up @@ -273,7 +274,7 @@ class DagManager : public std::enable_shared_from_this<DagManager> {

const uint32_t cache_max_size_ = 10000;
const uint32_t cache_delete_step_ = 100;
ExpirationCacheMap<blk_hash_t, DagBlock> seen_blocks_;
ExpirationCacheMap<blk_hash_t, std::shared_ptr<DagBlock>> seen_blocks_;
std::shared_ptr<final_chain::FinalChain> final_chain_;
const uint64_t kPbftGasLimit;
const HardforksConfig kHardforks;
Expand Down
6 changes: 3 additions & 3 deletions libraries/core_libs/consensus/include/pbft/pbft_manager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class PbftManager {
* @param dag_blocks DAG blocks
* @return DAG blocks ordering hash
*/
static blk_hash_t calculateOrderHash(const std::vector<DagBlock> &dag_blocks);
static blk_hash_t calculateOrderHash(const std::vector<std::shared_ptr<DagBlock>> &dag_blocks);

/**
* @brief Reorder transactions data if DAG reordering caused transactions with same sender to have nonce in incorrect
Expand All @@ -226,7 +226,7 @@ class PbftManager {
* @param dag_blocks dag blocks
* @return true if total weight of gas estimation is less or equal to gas limit. Otherwise return false
*/
bool checkBlockWeight(const std::vector<DagBlock> &dag_blocks) const;
bool checkBlockWeight(const std::vector<std::shared_ptr<DagBlock>> &dag_blocks) const;

blk_hash_t getLastPbftBlockHash();

Expand Down Expand Up @@ -564,7 +564,7 @@ class PbftManager {

// Multiple proposed pbft blocks could have same dag block anchor at same period so this cache improves retrieval of
// dag block order for specific anchor
mutable std::unordered_map<blk_hash_t, std::vector<DagBlock>> anchor_dag_block_order_cache_;
mutable std::unordered_map<blk_hash_t, std::vector<std::shared_ptr<DagBlock>>> anchor_dag_block_order_cache_;

std::unique_ptr<std::thread> daemon_;
std::shared_ptr<DbStorage> db_;
Expand Down
20 changes: 10 additions & 10 deletions libraries/core_libs/consensus/src/dag/dag_block_proposer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ bool DagBlockProposer::proposeDagBlock() {
auto dag_block =
createDagBlock(std::move(frontier), propose_level, transactions, std::move(estimations), std::move(vdf));

if (dag_mgr_->addDagBlock(std::move(dag_block), std::move(transactions), true).first) {
LOG(log_nf_) << "Proposed new DAG block " << dag_block.getHash() << ", pivot " << dag_block.getPivot()
<< " , txs num " << dag_block.getTrxs().size();
if (dag_mgr_->addDagBlock(dag_block, std::move(transactions), true).first) {
LOG(log_nf_) << "Proposed new DAG block " << dag_block->getHash() << ", pivot " << dag_block->getPivot()
<< " , txs num " << dag_block->getTrxs().size();
proposed_blocks_count_ += 1;
} else {
LOG(log_er_) << "Failed to add newly proposed dag block " << dag_block.getHash() << " into dag";
LOG(log_er_) << "Failed to add newly proposed dag block " << dag_block->getHash() << " into dag";
}

last_propose_level_ = propose_level;
Expand Down Expand Up @@ -320,8 +320,10 @@ vec_blk_t DagBlockProposer::selectDagBlockTips(const vec_blk_t& frontier_tips, u
return tips;
}

DagBlock DagBlockProposer::createDagBlock(DagFrontier&& frontier, level_t level, const SharedTransactions& trxs,
std::vector<uint64_t>&& estimations, VdfSortition&& vdf) const {
std::shared_ptr<DagBlock> DagBlockProposer::createDagBlock(DagFrontier&& frontier, level_t level,
const SharedTransactions& trxs,
std::vector<uint64_t>&& estimations,
VdfSortition&& vdf) const {
// When we propose block we know it is valid, no need for block verification with queue,
// simply add the block to the DAG
vec_trx_t trx_hashes;
Expand All @@ -336,10 +338,8 @@ DagBlock DagBlockProposer::createDagBlock(DagFrontier&& frontier, level_t level,
frontier.tips = selectDagBlockTips(frontier.tips, kPbftGasLimit - block_estimation);
}

DagBlock block(frontier.pivot, std::move(level), std::move(frontier.tips), std::move(trx_hashes), block_estimation,
std::move(vdf), node_sk_);

return block;
return std::make_shared<DagBlock>(frontier.pivot, std::move(level), std::move(frontier.tips), std::move(trx_hashes),
block_estimation, std::move(vdf), node_sk_);
}

bool DagBlockProposer::isValidDposProposer(PbftPeriod propose_period) const {
Expand Down
Loading
Loading