Merge 15861 via restore_vbits_warning

src/bitcoin-chainstate.cpp

@@ -87,7 +87,7 @@ int main(int argc, char* argv[])
         {
             std::cout << "Progress: " << title.original << ", " << progress_percent << ", " << resume_possible << std::endl;
         }
-        void warningSet(kernel::Warning id, const bilingual_str& message) override
+        void warningSet(kernel::Warning id, const bilingual_str& message, bool update) override
         {
             std::cout << "Warning " << static_cast<int>(id) << " set: " << message.original << std::endl;
         }

src/kernel/notifications_interface.h

@@ -40,7 +40,7 @@ public:
     [[nodiscard]] virtual InterruptResult blockTip(SynchronizationState state, CBlockIndex& index) { return {}; }
     virtual void headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) {}
     virtual void progress(const bilingual_str& title, int progress_percent, bool resume_possible) {}
-    virtual void warningSet(Warning id, const bilingual_str& message) {}
+    virtual void warningSet(Warning id, const bilingual_str& message, bool update=false) {}
     virtual void warningUnset(Warning id) {}
 
     //! The flush error notification is sent to notify the user that an error

src/kernel/warning.h

@@ -9,6 +9,8 @@ namespace kernel {
 enum class Warning {
     UNKNOWN_NEW_RULES_ACTIVATED,
     LARGE_WORK_INVALID_CHAIN,
+    UNKNOWN_NEW_RULES_SIGNAL_VBITS,
+    UNKNOWN_NEW_RULES_SIGNAL_INTVER,
 };
 } // namespace kernel
 #endif // BITCOIN_KERNEL_WARNING_H

src/node/kernel_notifications.cpp

@@ -72,9 +72,9 @@ void KernelNotifications::progress(const bilingual_str& title, int progress_perc
     uiInterface.ShowProgress(title.translated, progress_percent, resume_possible);
 }
 
-void KernelNotifications::warningSet(kernel::Warning id, const bilingual_str& message)
+void KernelNotifications::warningSet(kernel::Warning id, const bilingual_str& message, const bool update)
 {
-    if (m_warnings.Set(id, message)) {
+    if (m_warnings.Set(id, message, update)) {
         AlertNotify(message.original);
     }
 }

src/node/kernel_notifications.h

@@ -40,7 +40,7 @@ public:
 
     void progress(const bilingual_str& title, int progress_percent, bool resume_possible) override;
 
-    void warningSet(kernel::Warning id, const bilingual_str& message) override;
+    void warningSet(kernel::Warning id, const bilingual_str& message, bool update) override;
 
     void warningUnset(kernel::Warning id) override;
 

src/node/warnings.cpp

@@ -27,9 +27,20 @@ Warnings::Warnings()
              _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications")});
     }
 }
-bool Warnings::Set(warning_type id, bilingual_str message)
+bool Warnings::Set(warning_type id, bilingual_str message, const bool update)
 {
-    const auto& [_, inserted]{WITH_LOCK(m_mutex, return m_warnings.insert({id, std::move(message)}))};
+    bool inserted{false};
+    if (update) {
+        LOCK(m_mutex);
+        auto& warning_msg = m_warnings[id];
+        if (warning_msg.original != message.original) {
+            warning_msg = message;
+            inserted = true;
+        }
+    } else {
+        const auto& [_, inserted_res]{WITH_LOCK(m_mutex, return m_warnings.insert({id, std::move(message)}))};
+        inserted = inserted_res;
+    }
     if (inserted) uiInterface.NotifyAlertChanged();
     return inserted;
 }

src/node/warnings.h

@@ -53,14 +53,20 @@ public:
      *        `id` is already active, false is returned and the new
      *        warning is ignored. If `id` does not yet exist, the
      *        warning is set, the UI is updated, and true is returned.
+     *        If `update` is true, already active warnings will be
+     *        updated with the new `message`, and this method will
+     *        return true unless there has been no change (only the
+     *        untranslated/original string is compared).
      *
      * @param[in]   id  Unique identifier of the warning.
      * @param[in]   message Warning message to be shown.
+     * @param[in]   update  Whether an existing warning should be
+     *              updated.
      *
      * @returns true if the warning was indeed set (i.e. there is no
      *          active warning with this `id`), otherwise false.
      */
-    bool Set(warning_type id, bilingual_str message) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+    bool Set(warning_type id, bilingual_str message, bool update=false) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
     /**
      * @brief Unset a warning message. If a warning with the specified
      *        `id` is active, it is unset, the UI is updated, and true

src/validation.cpp

@@ -3010,15 +3010,69 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
             WarningBitsConditionChecker checker(m_chainman, bit);
             ThresholdState state = checker.GetStateFor(pindex, params.GetConsensus(), m_chainman.m_warningcache.at(bit));
             if (state == ThresholdState::ACTIVE || state == ThresholdState::LOCKED_IN) {
-                const bilingual_str warning = strprintf(_("Unknown new rules activated (versionbit %i)"), bit);
-                if (state == ThresholdState::ACTIVE) {
-                    m_chainman.GetNotifications().warningSet(kernel::Warning::UNKNOWN_NEW_RULES_ACTIVATED, warning);
-                } else {
-                    warning_messages.push_back(warning);
-                }
+                const bilingual_str warning = strprintf(_("WARNING: Unknown new rules activated (versionbit %i) - this software is not secure"), bit);
+                m_chainman.GetNotifications().warningSet(kernel::Warning::UNKNOWN_NEW_RULES_ACTIVATED, warning);
+                warning_messages.push_back(warning);
             }
         }
+
+        // Check the version of the last 100 blocks to see if we need to upgrade:
+        int unexpected_bit_count[VERSIONBITS_NUM_BITS], nonversionbit_count = 0;
+        for (size_t i = 0; i < VERSIONBITS_NUM_BITS; ++i) unexpected_bit_count[i] = 0;
+        // NOTE: The warning_threshold_hit* variables are static to ensure the warnings persist even after the condition changes, until the node is restarted
+        static std::set<uint8_t> warning_threshold_hit_bits;
+        static int32_t warning_threshold_hit_int{-1};
+        for (int i = 0; i < 100 && pindex != nullptr; i++)
+        {
+            int32_t nExpectedVersion = m_chainman.m_versionbitscache.ComputeBlockVersion(pindex->pprev, params.GetConsensus());
+            if (pindex->nVersion <= VERSIONBITS_LAST_OLD_BLOCK_VERSION) {
+                // We don't care
+            } else if ((pindex->nVersion & VERSIONBITS_TOP_MASK) != VERSIONBITS_TOP_BITS) {
+                // Non-versionbits upgrade
+                static constexpr int WARNING_THRESHOLD = 100/2;
+                if (++nonversionbit_count > WARNING_THRESHOLD) {
+                    if (warning_threshold_hit_int == -1) {
+                        warning_threshold_hit_int = pindex->nVersion;
+                    } else if (warning_threshold_hit_int != pindex->nVersion) {
+                        warning_threshold_hit_int = -2;
+                    }
+                }
+            } else if ((pindex->nVersion & ~nExpectedVersion) != 0) {
+                for (int bit = 0; bit < VERSIONBITS_NUM_BITS; ++bit) {
+                    const int32_t mask = 1 << bit;
+                    if ((pindex->nVersion & mask) && !(nExpectedVersion & mask)) {
+                        const int warning_threshold = (bit > 12 ? 75 : 50);
+                        if (++unexpected_bit_count[bit] > warning_threshold) {
+                            warning_threshold_hit_bits.insert(bit);
+                        }
+                    }
+                }
+            }
+            pindex = pindex->pprev;
+        }
+        if (!warning_threshold_hit_bits.empty()) {
+            const auto warning = strprintf(_("Warning: Miners are attempting to activate unknown new rules (bit %s)! You may or may not need to act to remain secure"), util::Join(warning_threshold_hit_bits, ", ", [](const uint8_t bit){ return util::ToString(int(bit)); }));
+            m_chainman.GetNotifications().warningSet(kernel::Warning::UNKNOWN_NEW_RULES_SIGNAL_VBITS, warning, /*update=*/true);
+            warning_messages.push_back(warning);
+        }
+        if (warning_threshold_hit_int != -1) {
+            bilingual_str warning;
+            if (warning_threshold_hit_int == -2) {
+                warning = _("Warning: Unrecognised block versions are being mined! Unknown rules may or may not be in effect");
+            } else {
+                warning = strprintf(_("Warning: Unrecognised block version (0x%08x) is being mined! Unknown rules may or may not be in effect"), warning_threshold_hit_int);
+            }
+            m_chainman.GetNotifications().warningSet(kernel::Warning::UNKNOWN_NEW_RULES_SIGNAL_INTVER, warning, /*update=*/true);
+            warning_messages.push_back(warning);
+        }
     }
+
+    static constexpr int32_t BIP320_MASK = 0x1fffe000UL;
+    if ((pindexNew->nVersion & BIP320_MASK) && pindexNew->nVersion != m_chainman.m_versionbitscache.ComputeBlockVersion(pindexNew->pprev, params.GetConsensus())) {
+        const auto warning = _("Miner violated version bit protocol");
+        warning_messages.push_back(warning);
+    }
+
     UpdateTipLog(coins_tip, pindexNew, params, __func__, "",
                  util::Join(warning_messages, Untranslated(", ")).original);
 }

test/functional/feature_notifications.py

@@ -50,6 +50,7 @@ class NotificationsTest(BitcoinTestFramework):
             f"-blocknotify=echo > {os.path.join(self.blocknotify_dir, '%s')}",
             f"-shutdownnotify=echo > {self.shutdownnotify_file}",
         ], [
+            "-blockversion=211",
             f"-walletnotify=echo %h_%b > {os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s'))}",
         ]]
         self.wallet_names = [self.default_wallet_name, self.wallet]
@@ -164,6 +165,22 @@ class NotificationsTest(BitcoinTestFramework):
 
         # TODO: add test for `-alertnotify` large fork notifications
 
+        # Mine 51 unknown-version blocks. -alertnotify should trigger on the 51st.
+        self.log.info("test -alertnotify")
+        self.generatetoaddress(self.nodes[1], 51, ADDRESS_BCRT1_UNSPENDABLE)
+
+        # Give bitcoind 10 seconds to write the alert notification
+        self.wait_until(lambda: len(os.listdir(self.alertnotify_dir)), timeout=10)
+
+        for notify_file in os.listdir(self.alertnotify_dir):
+            os.remove(os.path.join(self.alertnotify_dir, notify_file))
+
+        # Mine more up-version blocks, should not get more alerts:
+        self.generatetoaddress(self.nodes[1], 2, ADDRESS_BCRT1_UNSPENDABLE)
+
+        self.log.info("-alertnotify should not continue notifying for more unknown version blocks")
+        assert_equal(len(os.listdir(self.alertnotify_dir)), 0)
+
         self.log.info("test -shutdownnotify")
         self.stop_nodes()
         self.wait_until(lambda: os.path.isfile(self.shutdownnotify_file), timeout=10)

test/functional/feature_versionbits_warning.py

@@ -18,10 +18,20 @@ from test_framework.test_framework import BitcoinTestFramework
 VB_PERIOD = 144           # versionbits period length for regtest
 VB_THRESHOLD = 108        # versionbits activation threshold for regtest
 VB_TOP_BITS = 0x20000000
-VB_UNKNOWN_BIT = 27       # Choose a bit unassigned to any deployment
+VB_UNKNOWN_BIT = 12       # Choose a bit unassigned to any deployment
 VB_UNKNOWN_VERSION = VB_TOP_BITS | (1 << VB_UNKNOWN_BIT)
+VB_BIP320_BIT = 13
+VB_BIP320_VERSION = VB_TOP_BITS | (1 << VB_BIP320_BIT)
+VB_BIP320_THRESHOLD = 76
+UNKNOWN_VERSION_SCHEMA = 0x60000000
+UNKNOWN_VERSION_SCHEMA_THRESHOLD = 51
 
+WARN_UNKNOWN_RULES_MINED = "Warning: Unrecognised block version (0x%08x) is being mined! Unknown rules may or may not be in effect" % (UNKNOWN_VERSION_SCHEMA,)
+WARN_UNKNOWN_BIT_MINED = f"Warning: Miners are attempting to activate unknown new rules (bit {VB_UNKNOWN_BIT})"
+# NOTE: WARN_BIP320_BIT_MINED includes VB_UNKNOWN_BIT because it persists from the earlier check
+WARN_BIP320_BIT_MINED = f"Warning: Miners are attempting to activate unknown new rules (bit {VB_UNKNOWN_BIT}, {VB_BIP320_BIT})"
 WARN_UNKNOWN_RULES_ACTIVE = f"Unknown new rules activated (versionbit {VB_UNKNOWN_BIT})"
+WARN_BIP320_BLOCK = "Miner violated version bit protocol"
 VB_PATTERN = re.compile("Unknown new rules activated.*versionbit")
 
 class VersionBitsWarningTest(BitcoinTestFramework):
@@ -76,10 +86,58 @@ class VersionBitsWarningTest(BitcoinTestFramework):
         assert not VB_PATTERN.match(",".join(node.getmininginfo()["warnings"]))
         assert not VB_PATTERN.match(",".join(node.getnetworkinfo()["warnings"]))
 
+        self.log.info("Check that there is a warning if >50 blocks in the last 100 were an unknown version schema")
+        # Build UNKNOWN_VERSION_SCHEMA_THRESHOLD blocks signaling some unknown schema
+        self.send_blocks_with_version(peer, UNKNOWN_VERSION_SCHEMA_THRESHOLD, UNKNOWN_VERSION_SCHEMA)
+        # Check that get*info() shows the 51/100 unknown block version warning
+        assert(WARN_UNKNOWN_RULES_MINED in ",".join(node.getmininginfo()["warnings"]))
+        assert(WARN_UNKNOWN_RULES_MINED in ",".join(node.getnetworkinfo()["warnings"]))
+        # Close the period normally
+        self.generatetoaddress(node, VB_PERIOD - UNKNOWN_VERSION_SCHEMA_THRESHOLD, node_deterministic_address)
+        # Make sure the warning remains
+        assert(WARN_UNKNOWN_RULES_MINED in ",".join(node.getmininginfo()["warnings"]))
+        assert(WARN_UNKNOWN_RULES_MINED in ",".join(node.getnetworkinfo()["warnings"]))
+
+        # Stop-start the node, and make sure the warning is gone
+        self.restart_node(0)
+        assert(WARN_UNKNOWN_RULES_MINED not in ",".join(node.getmininginfo()["warnings"]))
+        assert(WARN_UNKNOWN_RULES_MINED not in ",".join(node.getnetworkinfo()["warnings"]))
+        peer = node.add_p2p_connection(P2PInterface())
+
+        self.log.info("Check that there is a warning if >50 blocks in the last 100 were an unknown version")
         # Build one period of blocks with VB_THRESHOLD blocks signaling some unknown bit
         self.send_blocks_with_version(peer, VB_THRESHOLD, VB_UNKNOWN_VERSION)
         self.generatetoaddress(node, VB_PERIOD - VB_THRESHOLD, node_deterministic_address)
 
+        # Check that get*info() shows the 51/100 unknown block version warning
+        assert(WARN_UNKNOWN_BIT_MINED in ",".join(node.getmininginfo()["warnings"]))
+        assert(WARN_UNKNOWN_BIT_MINED in ",".join(node.getnetworkinfo()["warnings"]))
+
+        self.log.info("Check that there is a warning if BIP320 is used, and a second persistent warning if >75 blocks in the last 100 were a BIP320 version")
+        with node.busy_wait_for_debug_log([WARN_BIP320_BLOCK.encode('ascii')]):
+            self.send_blocks_with_version(peer, VB_BIP320_THRESHOLD - 1, VB_BIP320_VERSION)
+        # Check that get*info() doesn't shows the 76/100 unknown block version warning yet.
+        assert(WARN_BIP320_BIT_MINED not in ",".join(node.getmininginfo()["warnings"]))
+        assert(WARN_BIP320_BIT_MINED not in ",".join(node.getnetworkinfo()["warnings"]))
+        # ...and it shouldn't show the BIP320-specific warning
+        assert(WARN_BIP320_BLOCK not in node.getmininginfo()["warnings"])
+        assert(WARN_BIP320_BLOCK not in node.getnetworkinfo()["warnings"])
+        with node.busy_wait_for_debug_log([WARN_BIP320_BLOCK.encode('ascii'), b'Enqueuing UpdatedBlockTip']):
+            self.send_blocks_with_version(peer, 1, VB_BIP320_VERSION)
+        # Check that get*info() shows the 76/100 unknown block version warning.
+        assert(WARN_BIP320_BIT_MINED in ",".join(node.getmininginfo()["warnings"]))
+        assert(WARN_BIP320_BIT_MINED in ",".join(node.getnetworkinfo()["warnings"]))
+        assert(WARN_BIP320_BLOCK not in node.getmininginfo()["warnings"])
+        assert(WARN_BIP320_BLOCK not in node.getnetworkinfo()["warnings"])
+        with node.busy_wait_for_debug_log([b'Enqueuing UpdatedBlockTip'], forbid_msgs=[WARN_BIP320_BLOCK.encode('ascii')]):
+            self.generatetoaddress(node, 1, node_deterministic_address)
+        # Only the 76/100 should persist
+        assert(WARN_BIP320_BIT_MINED in ",".join(node.getmininginfo()["warnings"]))
+        assert(WARN_BIP320_BIT_MINED in ",".join(node.getnetworkinfo()["warnings"]))
+        assert(WARN_BIP320_BLOCK not in node.getmininginfo()["warnings"])
+        assert(WARN_BIP320_BLOCK not in node.getnetworkinfo()["warnings"])
+        self.generatetoaddress(node, VB_PERIOD - VB_BIP320_THRESHOLD - 1, node_deterministic_address)
+
         self.log.info("Check that there is a warning if previous VB_BLOCKS have >=VB_THRESHOLD blocks with unknown versionbits version.")
         # Mine a period worth of expected blocks so the generic block-version warning
         # is cleared. This will move the versionbit state to ACTIVE.

test/functional/test_framework/test_node.py

@@ -524,7 +524,7 @@ class TestNode():
         self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log))
 
     @contextlib.contextmanager
-    def busy_wait_for_debug_log(self, expected_msgs, timeout=60):
+    def busy_wait_for_debug_log(self, expected_msgs, timeout=60, *, forbid_msgs=()):
         """
         Block until we see a particular debug log message fragment or until we exceed the timeout.
         Return:
@@ -541,6 +541,13 @@ class TestNode():
                 dl.seek(prev_size)
                 log = dl.read()
 
+            for msg in forbid_msgs:
+                if msg in log:
+                    print_log = " - " + "\n - ".join(log.decode("utf8", errors="replace").splitlines())
+                    self._raise_assertion_error(
+                        'Forbidden message "{}" partially matched log:\n\n{}\n\n'.format(
+                            str(msg), print_log))
+
             for expected_msg in expected_msgs:
                 if expected_msg not in log:
                     found = False