refactor: Avoid locking cs_main in ProcessNewBlockHeaders #16793

Builds on #16774, this change avoids locking cs_main in ProcessNewBlockHeaders when the tip has changed - in this case the removed lock was necessary to just log a message.

Tend to NACK, as this seems like a layer violation

@MarcoFalke I don't understand why, this change makes NotifyHeaderTip return whether it's IBD. Note that NotifyHeaderTip already sends it in the notification.

NACK as-is, but IsInitialBlockDownload() doesn't need cs_main anyway (it takes it itself if it needs it, and has an atomic cache first), so it looks like you should be able to just drop the cs_mains here wholesale with no other changes.

the lock is needed for g_chainstate

Huh? g_chainstate is set during init (pre-threading) and never edited after. No reason it would need cs_main, either?

I figured that would change with assumeutxo? Maybe @jamesob knows

Later on g_chainstate (or ChainstateActive()) will be covered by some lock (since it may be swapped out during runtime), but probably not cs_main. I think for now we should be able to do as @TheBlueMatt and just drop the lock acquisition.

...though don't we need cs_main to safely use ppindex?

Not for the members that are read (nHeight et al)

#15615 (review)

@MarcoFalke why are you linking that comment?

cs_main might be needed for pindexBestHeader, but that is no longer in the current code. So, indeed, you can just remove cs_main here.

Done.