Skip to content

BlockchainDB: rm skip_coinbase in get_blocks_from() #10166

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.

Sign up for GitHub

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

Open
jeffro256 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

BlockchainDB: rm skip_coinbase in get_blocks_from() #10166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions src/blockchain_db/blockchain_db.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1300,14 +1300,13 @@ class BlockchainDB
* @param max_size the maximum size of block/transaction data to return (will be exceeded by one blocks's worth at most, if min_count is met)
* @param blocks the returned block/transaction data
* @param pruned whether to return full or pruned tx data
* @param skip_coinbase whether to return or skip coinbase transactions (they're in blocks regardless)
* @param get_miner_tx_hash whether to calculate and return the miner (coinbase) tx hash
*
* The call will return at least min_block_count if possible, even if this contravenes max_tx_count
*
* @return true iff the blocks and transactions were found
*/
virtual bool get_blocks_from(uint64_t start_height, size_t min_block_count, size_t max_block_count, size_t max_tx_count, size_t max_size, std::vector<std::pair<std::pair<cryptonote::blobdata, crypto::hash>, std::vector<std::pair<crypto::hash, cryptonote::blobdata>>>>& blocks, bool pruned, bool skip_coinbase, bool get_miner_tx_hash) const = 0;
virtual bool get_blocks_from(uint64_t start_height, size_t min_block_count, size_t max_block_count, size_t max_tx_count, size_t max_size, std::vector<std::pair<std::pair<cryptonote::blobdata, crypto::hash>, std::vector<std::pair<crypto::hash, cryptonote::blobdata>>>>& blocks, bool pruned, bool get_miner_tx_hash) const = 0;

/**
* @brief fetches the prunable transaction blob with the given hash
Expand Down
5 changes: 2 additions & 3 deletions src/blockchain_db/lmdb/db_lmdb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3242,7 +3242,7 @@ std::vector<crypto::hash> BlockchainLMDB::get_txids_loose(const crypto::hash& tx
return matching_hashes;
}

bool BlockchainLMDB::get_blocks_from(uint64_t start_height, size_t min_block_count, size_t max_block_count, size_t max_tx_count, size_t max_size, std::vector<std::pair<std::pair<cryptonote::blobdata, crypto::hash>, std::vector<std::pair<crypto::hash, cryptonote::blobdata>>>>& blocks, bool pruned, bool skip_coinbase, bool get_miner_tx_hash) const
bool BlockchainLMDB::get_blocks_from(uint64_t start_height, size_t min_block_count, size_t max_block_count, size_t max_tx_count, size_t max_size, std::vector<std::pair<std::pair<cryptonote::blobdata, crypto::hash>, std::vector<std::pair<crypto::hash, cryptonote::blobdata>>>>& blocks, bool pruned, bool get_miner_tx_hash) const
{
LOG_PRINT_L3("BlockchainLMDB::" << __func__);
check_open();
Expand Down Expand Up @@ -3298,7 +3298,6 @@ bool BlockchainLMDB::get_blocks_from(uint64_t start_height, size_t min_block_cou
val_tx_id.mv_size = sizeof(tx_id);
}

if (skip_coinbase)
{
result = mdb_cursor_get(m_cur_txs_pruned, &val_tx_id, &v, op);
if (result)
Expand All @@ -3314,7 +3313,7 @@ bool BlockchainLMDB::get_blocks_from(uint64_t start_height, size_t min_block_cou
op = MDB_NEXT;

current_block.second.reserve(b.tx_hashes.size());
num_txes += b.tx_hashes.size() + (skip_coinbase ? 0 : 1);
num_txes += b.tx_hashes.size() + 1;
Copy link

@iamamyth iamamyth Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If skip_coinbase was previously true, adding 1 here would change the behavior.

Copy link
Contributor Author

@jeffro256 jeffro256 Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This simply makes the num_txes count more accurate, which makes the limiting check against max_tx_count more accurate as well.

Copy link
Contributor Author

@jeffro256 jeffro256 Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamamyth Can you please revisit this review?

for (const auto &tx_hash: b.tx_hashes)
{
// get pruned data
Expand Down
2 changes: 1 addition & 1 deletion src/blockchain_db/lmdb/db_lmdb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ class BlockchainLMDB final: public BlockchainDB
bool get_tx_blob(const crypto::hash& h, cryptonote::blobdata &tx) const override;
bool get_pruned_tx_blob(const crypto::hash& h, cryptonote::blobdata &tx) const override;
bool get_pruned_tx_blobs_from(const crypto::hash& h, size_t count, std::vector<cryptonote::blobdata> &bd) const override;
bool get_blocks_from(uint64_t start_height, size_t min_block_count, size_t max_block_count, size_t max_tx_count, size_t max_size, std::vector<std::pair<std::pair<cryptonote::blobdata, crypto::hash>, std::vector<std::pair<crypto::hash, cryptonote::blobdata>>>>& blocks, bool pruned, bool skip_coinbase, bool get_miner_tx_hash) const override;
bool get_blocks_from(uint64_t start_height, size_t min_block_count, size_t max_block_count, size_t max_tx_count, size_t max_size, std::vector<std::pair<std::pair<cryptonote::blobdata, crypto::hash>, std::vector<std::pair<crypto::hash, cryptonote::blobdata>>>>& blocks, bool pruned, bool get_miner_tx_hash) const override;
bool get_prunable_tx_blob(const crypto::hash& h, cryptonote::blobdata &tx) const override;
bool get_prunable_tx_hash(const crypto::hash& tx_hash, crypto::hash &prunable_hash) const override;

Expand Down
2 changes: 1 addition & 1 deletion src/blockchain_db/testdb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class BaseTestDB: public cryptonote::BlockchainDB {
virtual bool get_tx_blob(const crypto::hash& h, cryptonote::blobdata &tx) const override { return false; }
virtual bool get_pruned_tx_blob(const crypto::hash& h, cryptonote::blobdata &tx) const override { return false; }
virtual bool get_pruned_tx_blobs_from(const crypto::hash& h, size_t count, std::vector<cryptonote::blobdata> &bd) const override { return false; }
virtual bool get_blocks_from(uint64_t start_height, size_t min_block_count, size_t max_block_count, size_t max_tx_count, size_t max_size, std::vector<std::pair<std::pair<cryptonote::blobdata, crypto::hash>, std::vector<std::pair<crypto::hash, cryptonote::blobdata>>>>& blocks, bool pruned, bool skip_coinbase, bool get_miner_tx_hash) const override { return false; }
virtual bool get_blocks_from(uint64_t start_height, size_t min_block_count, size_t max_block_count, size_t max_tx_count, size_t max_size, std::vector<std::pair<std::pair<cryptonote::blobdata, crypto::hash>, std::vector<std::pair<crypto::hash, cryptonote::blobdata>>>>& blocks, bool pruned, bool get_miner_tx_hash) const override { return false; }
virtual std::vector<crypto::hash> get_txids_loose(const crypto::hash& h, std::uint32_t bits, uint64_t max_num_txs = 0) override { return {}; }
virtual bool get_prunable_tx_blob(const crypto::hash& h, cryptonote::blobdata &tx) const override { return false; }
virtual bool get_prunable_tx_hash(const crypto::hash& tx_hash, crypto::hash &prunable_hash) const override { return false; }
Expand Down
2 changes: 1 addition & 1 deletion src/cryptonote_core/blockchain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2765,7 +2765,7 @@ bool Blockchain::find_blockchain_supplement(const uint64_t req_start_block, cons
top_hash = m_db->top_block_hash(&total_height);
++total_height;
blocks.reserve(std::min(std::min(max_block_count, (size_t)10000), (size_t)(total_height - start_height)));
CHECK_AND_ASSERT_MES(m_db->get_blocks_from(start_height, 3, max_block_count, max_tx_count, FIND_BLOCKCHAIN_SUPPLEMENT_MAX_SIZE, blocks, pruned, true, get_miner_tx_hash),
CHECK_AND_ASSERT_MES(m_db->get_blocks_from(start_height, 3, max_block_count, max_tx_count, FIND_BLOCKCHAIN_SUPPLEMENT_MAX_SIZE, blocks, pruned, get_miner_tx_hash),
false, "Error getting blocks");

return true;
Expand Down
Loading