I would just assert this one?

@TheBlueMatt I'm not sure under what conditions this might occur. Tip() explicitly returns NULL in some cases... Don't want running a miner to crash your Bitcoin node.

It should never return null unless you havent loaded even the genesis block, I think

Correct. chainActive.Tip should never be NULL after AppInit. Either LoadBlockIndex or InitBlockIndex fixed that.

@sipa Would you object to making Tip() return a reference rather than a pointer, then?

We use CBlockIndex* variables everywhere as identifiers for blocks, so yes, I would rather not change that. Also, it is effectively useful inside the initialization code to detect the we're-not-yet-initialized case.

Correct. chainActive.Tip should never be NULL after AppInit. Either LoadBlockIndex or InitBlockIndex fixed that.

This post #4995 (comment) in #4995 claims that in the case of -reindex it can be NULL (note: haven't checked this myself).

This is an assert, not a normal one. Should be impossible for this condition to occur, even with reindex.

reindex always hand-builds the genesis block.

Oh, no, indeed. If you reindex, InitBlockIndex bypassing rebuilding the
genesis block, as the one already on disk will be used.

@laanwj Hmm, would it not make more sense to fix that by making sure Tip() always points to at least genesis, instead of letting it be NULL?

When you're reindexing, there is no genesis CBlockIndex before you start
the reindex (as you don't know where it is on disk yet).

OK, so we should wait to return from AppInit before we at least have the genesis block reindexed?

Now that we've established that chainActive.Tip() can return NULL, I suppose the most straightforward way forward is document this behaviour and make the code robust against it where necessary.

Just delaying the return won't solve anything. @TheBlueMatt To be precise we'd have to wait after starting ThreadImport, before starting the node and RPC threads and internal miner. It would be another possible solution, although a bit hackier than just considering "no chain yet" a legit state, it may have less code impact overall...

@laanwj Yes, I was going for less impact. It certainly seems rather strange to start import in init, and then finish initializing everything else before we have even initialized the genesis block (which should be one of the first blocks imported, no?).

OK. Then cleanest would be to split up reindex into a 'locate genesis block' and 'reindex chain' part, the first part being executed from AppInit2. This avoids ugly constructions with waiting for ThreadImport.

Correct me if wrong: when reindexing, even with headers-first, the genesis block is always the first block in the first block file (blk00000.dat). If not we are reindexing the wrong chain, for example a testnet block chain on a mainnet node or dealing with a corrupted block file. In that case it acceptable to throw an initialization error.

I can't think of any exceptions to this.

The genesis block should be the first one yes, as it's not received from network but put there in InitBlockIndex, If it is not, there's a problem.

utACK

Ive always considered the chainActive.Tip() never returning NULL to be a rule, so I'd prefer to wait in init for it to be the case (ie #5243)

Closing in favor of #5243.