← Watch feed
BlockchainDB: rm `skip_coinbase` in `get_blocks_from()`
Public commit record
73/100 · Adequate
What the developer wrote
Authored by jeffro256
BlockchainDB: rm `skip_coinbase` in `get_blocks_from()`
Parameter `skip_coinbase` was always set to `true`, and didn't actually skip the coinbase transactions. The method broke when set to `false`.
Noticed by @akildemir
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This is a small code cleanup, not a security fix. A database function had a 'skip coinbase' option that was always turned on and didn't actually work when turned off. The patch removes the broken, unused option entirely. The only caller kept using the same behavior, so nothing changes for users.
Recommended action
No security action required. Treat as normal code-quality/maintenance commit. If reviewing for release notes, classify as cleanup/refactoring.
Security signals we found
01
No security-relevant behavioral change: the only caller previously passed `skip_coinbase=true`, and the new signature preserves that same code path.
02
Dead/broken parameter removal reduces API surface and future misuse risk.
03
No input validation, memory safety, cryptographic, or consensus logic changes visible in the diff.
04
No incident, advisory, CVE, or vendor security statement supplied or referenced.
Evidence from the diff
The commit removes the skip_coinbase boolean parameter from BlockchainDB::get_blocks_from() and its LMDB/test implementations. The commit message states the parameter was always passed as true, never actually skipped coinbase transactions, and the implementation broke if set to false. The sole caller in blockchain.cpp is updated to drop the argument while keeping all other arguments identical. The change is a refactoring/cleanup of dead/broken code rather than a behavioral or security fix.
Changed components
src/blockchain_db/blockchain_db.hsrc/blockchain_db/lmdb/db_lmdb.cppsrc/blockchain_db/lmdb/db_lmdb.hsrc/blockchain_db/testdb.hsrc/cryptonote_core/blockchain.cppInspect captured patch +6 / −8
diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h
index 5bcc63d..759200c 100644
--- a/src/blockchain_db/blockchain_db.h
+++ b/src/blockchain_db/blockchain_db.h
@@ -1300,14 +1300,13 @@ public:
* @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
diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp
index d5422e9..60b6bf7 100644
--- a/src/blockchain_db/lmdb/db_lmdb.cpp
+++ b/src/blockchain_db/lmdb/db_lmdb.cpp
@@ -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();
@@ -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)
@@ -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;
for (const auto &tx_hash: b.tx_hashes)
{
// get pruned data
diff --git a/src/blockchain_db/lmdb/db_lmdb.h b/src/blockchain_db/lmdb/db_lmdb.h
index 45b3f37..e6c2c50 100644
--- a/src/blockchain_db/lmdb/db_lmdb.h
+++ b/src/blockchain_db/lmdb/db_lmdb.h
@@ -254,7 +254,7 @@ public:
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;
diff --git a/src/blockchain_db/testdb.h b/src/blockchain_db/testdb.h
index c525dda..788c4bf 100644
--- a/src/blockchain_db/testdb.h
+++ b/src/blockchain_db/testdb.h
@@ -70,7 +70,7 @@ public:
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; }
diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp
index 552dbb6..b69bb00 100644
--- a/src/cryptonote_core/blockchain.cpp
+++ b/src/cryptonote_core/blockchain.cpp
@@ -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;
Risk score
Our methodology →Why this scored 18/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.