Make m_tip_block std::optional

[Sjors]

Suggested in https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1844244309

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31325.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	tdb3, l0rinc, fjahr
Concept ACK	BrandonOdiwuor
Stale ACK	ryanofsky

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

[Sjors]

I dropped the use of has_value() https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1851743969 as well as value() https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1850680750. Except in the test, where throwing std::bad_optional_access is probably useful.

Added a comment: https://github.com/bitcoin/bitcoin/pull/31325/files#r1851747612

Dropped the word "kernel" and added a motivation to the commit message.

[l0rinc]

ACK 2fff10a656edb8bec7d45433bf431fd233d0fd81

[Sjors]

Rebased (just in case) and shortened the comment: https://github.com/bitcoin/bitcoin/pull/31325#discussion_r1870138808

[l0rinc]

ACK 5c48d098dc300ae47f2ffeb9305f58bd6352152d The only change was to the comment which I find less technical and more descriptive 👍

[l0rinc]

ACK fe6487ca6ab005faab9f1d565740f2259c04cd16

[luke-jr]

Not sure this is an improvement. Seems like a case of "two ways to be omitted" which has caused issues in the past.

[Sjors]

"two ways to be omitted"

If this change is merged, we should discourage the use of uint256::ZERO / uint256{}. But there are other places in the codebase that still use this pattern, which I didn't refactor here. I even found myself using it yesterday for an upcoming PR.

I pushed a commit that makes m_tip_block private, accessible through TipBlock(). I then added an Assert at the only place that sets it to ensure it's not ZERO.

Also modified the first commit to drop a redundant "for" from the m_tip_block comment.

[Sjors]

I switched from Assert to Assume.

[l0rinc]

ACK fc0b5f3b77166aad9fb288b8eb9185bb2dc7496c

[Sjors]

Rebased after #31346.

[l0rinc]

ACK 43f23cd29de13ea600bbe1172cf1ac0d535f66ee

[Sjors]

@maflcko thoughts, since you suggested it?

[Sjors]

I replaced the use of uint256{} with uint256::ZERO, since it makes the intention more clear. I also limited the not-ZERO check to just the setter and dropped the comment.

Also dropped the whitespace change in kernel_notifications.h in the first commit.

[fjahr]

re-ACK 81cea5d4ee0e226f7c7145072de85829f4166ec9