refactor: GetFirstStoredBlock() and getblockchaininfo follow-ups #25016

pull jonatack wants to merge 3 commits into bitcoin:master from jonatack:GetFirstStoredBlock-improvements changing 4 files +13 −12

jonatack commented at 5:18 PM on April 28, 2022: contributor
Picks up the remaining review feedback in #21726 and #24956.
- make the global function GetFirstStoredBlock() a member of the BlockManager class
- pass the start_block param of GetFirstStoredBlock() by reference instead of a pointer
- use GetBlockTime() for RPC getblockchaininfo#time
jonatack cross-referenced this on Apr 28, 2022 from issue Improve Indices on pruned nodes via prune blockers by fjahr
DrahtBot added the label Block storage on Apr 28, 2022
DrahtBot added the label RPC/REST/ZMQ on Apr 28, 2022
DrahtBot added the label UTXO Db and Indexes on Apr 28, 2022
jonatack cross-referenced this on Apr 28, 2022 from issue Call CHECK_NONFATAL only once where needed by MarcoFalke
blockstorage, refactor: make GetFirstStoredBlock() a member of BlockManager
instead of a global
ed12c0a49d
blockstorage, refactor: pass GetFirstStoredBlock() start_block by reference
instead of by pointer, so as to not accept a nullptr.
86ce844d3b
MarcoFalke removed the label UTXO Db and Indexes on Apr 28, 2022
MarcoFalke removed the label RPC/REST/ZMQ on Apr 28, 2022
MarcoFalke removed the label Block storage on Apr 28, 2022
MarcoFalke added the label Refactoring on Apr 28, 2022
jonatack renamed this:
~~blockstorage, refactor: improve GetFirstStoredBlock()~~
blockstorage, refactor: GetFirstStoredBlock() follow-ups
on Apr 28, 2022
rpc: use GetBlockTime() for getblockchaininfo#time e2b954e87f
jonatack force-pushed on Apr 28, 2022
jonatack commented at 6:54 PM on April 28, 2022: contributor

Updated now that #24956 is merged, and ready for review.
jonatack renamed this:
~~blockstorage, refactor: GetFirstStoredBlock() follow-ups~~
refactor: GetFirstStoredBlock() and getblockchaininfo follow-ups
on Apr 28, 2022
MarcoFalke approved
MarcoFalke commented at 6:55 PM on April 28, 2022: member

ACK e2b954e87f0c4cd5c8ac6e4d9c6b4d784844b4d2
fanquake merged this on Apr 29, 2022
fanquake closed this on Apr 29, 2022
jonatack deleted the branch on Apr 29, 2022
sidhujag referenced this in commit cd0f3ec4a9 on Apr 29, 2022

in src/node/blockstorage.cpp:393 in e2b954e87f

 389 | @@ -390,10 +390,10 @@ bool BlockManager::IsBlockPruned(const CBlockIndex* pblockindex)
 390 |      return (m_have_pruned && !(pblockindex->nStatus & BLOCK_HAVE_DATA) && pblockindex->nTx > 0);
 391 |  }
 392 |  
 393 | -const CBlockIndex* GetFirstStoredBlock(const CBlockIndex* start_block) {
 394 | +const CBlockIndex* BlockManager::GetFirstStoredBlock(const CBlockIndex& start_block)

MarcoFalke commented at 8:30 AM on April 30, 2022:

In theory start_block could be marked LIFETIMEBOUND.

jonatack commented at 8:54 PM on May 3, 2022:

In theory start_block could be marked LIFETIMEBOUND.

Ah, yes; done in #25060.

jonatack cross-referenced this on May 3, 2022 from issue blockstorage: add LIFETIMEBOUND to GetFirstStoredBlock()::start_time by jonatack
MarcoFalke referenced this in commit d17bbc3c48 on May 4, 2022
sidhujag referenced this in commit cedc09f1e9 on May 4, 2022
bitcoin locked this on May 3, 2023

Contributors

jonatack

MarcoFalke

Labels

Refactoring

Linked (view graph)

#21726 Improve Indices on pruned nodes via prune blockers #24956 Call CHECK_NONFATAL only once where needed #25060 blockstorage: add LIFETIMEBOUND to GetFirstStoredBlock()::start_time