Merge bitcoin/bitcoin#31325: Make m_tip_block std::optional

81cea5d Ensure m_tip_block is never ZERO (Sjors Provoost)
e058544 Make m_tip_block an std::optional (Sjors Provoost)

Pull request description:

  Suggested in bitcoin/bitcoin#31297 (comment)

ACKs for top commit:
  fjahr:
    re-ACK 81cea5d
  tdb3:
    code review re ACK 81cea5d
  l0rinc:
    ACK 81cea5d

Tree-SHA512: 31a75ba29e3d567bab32e4e7925a419d9d7a4d2d85ed1c1012116d8d22adc14d31d5b4ce5f6c499c994188dcd26a01cced05be74f94c892fc90ae17a6783a472

Author: ryanofsky

src/init.cpp

@@ -1807,7 +1807,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     {
         WAIT_LOCK(kernel_notifications.m_tip_block_mutex, lock);
         kernel_notifications.m_tip_block_cv.wait(lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(kernel_notifications.m_tip_block_mutex) {
-            return !kernel_notifications.m_tip_block.IsNull() || ShutdownRequested(node);
+            return kernel_notifications.TipBlock() || ShutdownRequested(node);
         });
     }
 

src/node/interfaces.cpp

@@ -971,7 +971,9 @@ class MinerImpl : public Mining
         {
             WAIT_LOCK(notifications().m_tip_block_mutex, lock);
             notifications().m_tip_block_cv.wait_for(lock, timeout, [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) {
-                return (notifications().m_tip_block != current_tip && notifications().m_tip_block != uint256::ZERO) || chainman().m_interrupt;
+                // We need to wait for m_tip_block to be set AND for the value
+                // to differ from the current_tip value.
+                return (notifications().TipBlock() && notifications().TipBlock() != current_tip) || chainman().m_interrupt;
             });
         }
         // Must release m_tip_block_mutex before locking cs_main, to avoid deadlocks.

src/node/kernel_notifications.cpp

@@ -52,6 +52,7 @@ kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state
 {
     {
         LOCK(m_tip_block_mutex);
+        Assume(index.GetBlockHash() != uint256::ZERO);
         m_tip_block = index.GetBlockHash();
         m_tip_block_cv.notify_all();
     }
@@ -99,6 +100,13 @@ void KernelNotifications::fatalError(const bilingual_str& message)
                     m_exit_status, message, &m_warnings);
 }
 
+std::optional<uint256> KernelNotifications::TipBlock()
+{
+    AssertLockHeld(m_tip_block_mutex);
+    return m_tip_block;
+};
+
+
 void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications)
 {
     if (auto value{args.GetIntArg("-stopatheight")}) notifications.m_stop_at_height = *value;

src/node/kernel_notifications.h

@@ -59,12 +59,14 @@ class KernelNotifications : public kernel::Notifications
     //! The block for which the last blockTip notification was received.
     //! It's first set when the tip is connected during node initialization.
     //! Might be unset during an early shutdown.
-    uint256 m_tip_block GUARDED_BY(m_tip_block_mutex){uint256::ZERO};
+    std::optional<uint256> TipBlock() EXCLUSIVE_LOCKS_REQUIRED(m_tip_block_mutex);
 
 private:
     const std::function<bool()>& m_shutdown_request;
     std::atomic<int>& m_exit_status;
     node::Warnings& m_warnings;
+
+    std::optional<uint256> m_tip_block GUARDED_BY(m_tip_block_mutex);
 };
 
 void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications);

src/test/validation_chainstate_tests.cpp

@@ -72,7 +72,8 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup)
     ChainstateManager& chainman = *Assert(m_node.chainman);
     const auto get_notify_tip{[&]() {
         LOCK(m_node.notifications->m_tip_block_mutex);
-        return m_node.notifications->m_tip_block;
+        BOOST_REQUIRE(m_node.notifications->TipBlock());
+        return *m_node.notifications->TipBlock();
     }};
     uint256 curr_tip = get_notify_tip();