Remove redundant run time assertions for locks already checked at compile time #14443

Remove redundant run time assertions (AssertLockHeld(foo)) for locks already checked at compile time (EXCLUSIVE_LOCKS_REQUIRED(foo), -Wthread-safety).

The redundancy can be verified by checking that the function that contains the removed run time assertion has the corresponding compile time check.

The relevant compile time checks can be listed using:

git grep -E -A3 '(ImplicitlyLearnRelatedKeyScripts|MaybeSetPeerAsAnnouncingHeaderAndIDs|TipMayBeStale|BlockRequestAllowed|SendRejectsAndCheckIfBanned|ConsiderEviction|IsRBFOptIn|entryToJSON|removeForReorg|GetSortedDepthAndScore|RemoveStaged|trackPackageRemoved|CheckFinalTx|TestLockPointValidity|IsCurrentForFeeEstimation|UpdateMempoolForReorg|CheckInputsFromMempoolAndCache|AcceptToMemoryPoolWorker|CheckForkWarningConditions|CheckForkWarningConditionsOnNewFork|CheckInputs|GetBlockScriptFlags|ConnectBlock|ActivateBestChainStep|InvalidateBlock|ResetBlockFailureFlags|AddToBlockIndex|AcceptBlockHeader|AcceptBlock|TestBlockValidity|InsertBlockIndex|GenerateNewKey|AddKeyPubKeyWithDB|LoadKeyMetadata|LoadScriptMetadata|UpdateTimeFirstKey|RemoveWatchOnly|HasWalletSpend|IncOrderPosNext|AvailableCoins|ListCoins|SignTransaction|ZapSelectTx|KeypoolCountExternalKeys|LoadKeyPool|GetAddressGroupings|MarkReserveKeysAsUsed|LockCoin|UnlockCoin|UnlockAllCoins|IsLockedCoin|ListLockedCoins)' | grep -B3 EXCLUSIVE_LOCKS_REQUIRED

<!--e57a25ab6845829454e8d69fc972939a-->Reviewers, this pull request conflicts with the following ones:

• #14437 (Refactor: Start to separate wallet from node by ryanofsky)
• #14193 (validation: Add missing mempool locks by MarcoFalke)
• #13868 (Remove unused fScriptChecks parameter from CheckInputs by Empact)
• #13804 (Transaction Pool Layer by MarcoFalke)
• #13204 (Faster sigcache nonce by JeremyRubin)
• #13189 (Remove 2nd mapTx lookup in CTxMemPool::removeForBlock by promag)
• #10973 (Refactor: separate wallet from node by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@practicalswift I'm going to close this. @MarcoFalke bought this up here:

Good point, but I think we want to keep these for now, since not all compilers support these compile-time annotations and sometimes they have to be disabled due to shortcomings.

I know @TheBlueMatt also holds the opinion that leaving these in is still a good idea, and there is essentially no overhead to doing that.

@fanquake Thanks for communicating the rationale clearly. That brings transparency to your maintainer work.

Since the maintainer role is a commission of trust I think it is crucial that the decision process is clear and that the maintainer is ready to bring clarity if needed by answering questions about his/her decisions.

FWIW: I think you're doing a great job with labelling the PR:s. I'm continually impressed by how quickly you do the labelling. Talking about that, did you see my curious question in [#14239](/bitcoin-bitcoin/14239/) on how to label PR:s fixing undefined behaviour ("bugs" vs "refactoring")? Not a big issues obviously but it would be nice to have it clarified. More communication is better than less communication when it comes to maintainership :-)

Thanks!