From ee43549507a5e2fa5f4469fea3468ac3706d563e Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Mon, 25 Mar 2024 22:29:00 +0000
Subject: [PATCH 01/13] QA: Add forbid_msgs param to
 TestNode.wait_for_debug_log

---
 test/functional/test_framework/test_node.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index c77cfbdd91..ff2d9e1fe1 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -476,7 +476,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 wait_for_debug_log(self, expected_msgs, timeout=60):
+    def 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:
@@ -493,6 +493,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

From 9b32a6f5d5bc10a17b214c5a2fecd9097cc30950 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Sat, 29 Jul 2023 23:40:09 +0000
Subject: [PATCH 02/13] Restore warning for individual unknown version bits, as
 well as unknown version schemas

---
 src/validation.cpp                            | 34 +++++++++++++++++++
 test/functional/feature_notifications.py      | 17 ++++++++++
 .../functional/feature_versionbits_warning.py | 26 ++++++++++++++
 3 files changed, 77 insertions(+)

diff --git a/src/validation.cpp b/src/validation.cpp
index a6cab6b095..322e015571 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2753,6 +2753,40 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
                 }
             }
         }
+
+        // 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;
+        static constexpr int WARNING_THRESHOLD = 100/2;
+        bool warning_threshold_hit = false;
+        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 && (pindex->nVersion & ~nExpectedVersion) != 0)
+            {
+                if ((pindex->nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) {
+                    for (int bit = 0; bit < VERSIONBITS_NUM_BITS; ++bit) {
+                        const int32_t mask = 1 << bit;
+                        if ((nExpectedVersion & mask) != (pindex->nVersion & mask)) {
+                            if (++unexpected_bit_count[bit] > WARNING_THRESHOLD) {
+                                warning_threshold_hit = true;
+                            }
+                        }
+                    }
+                } else {
+                    // Non-versionbits upgrade
+                    if (++nonversionbit_count > WARNING_THRESHOLD) {
+                        warning_threshold_hit = true;
+                    }
+                }
+            }
+            pindex = pindex->pprev;
+        }
+        if (warning_threshold_hit) {
+            auto strWarning = _("Warning: Unrecognised block version being mined! Unknown rules may or may not be in effect");
+            // notify GetWarnings(), called by Qt and the JSON-RPC code to warn the user:
+            m_chainman.GetNotifications().warning(strWarning);
+        }
     }
     UpdateTipLog(coins_tip, pindexNew, params, __func__, "", warning_messages.original);
 }
diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py
index adf6c13973..384a3a0019 100755
--- a/test/functional/feature_notifications.py
+++ b/test/functional/feature_notifications.py
@@ -49,6 +49,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]
@@ -163,6 +164,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)
diff --git a/test/functional/feature_versionbits_warning.py b/test/functional/feature_versionbits_warning.py
index 073d3de812..31292f3790 100755
--- a/test/functional/feature_versionbits_warning.py
+++ b/test/functional/feature_versionbits_warning.py
@@ -20,7 +20,10 @@ 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_VERSION = VB_TOP_BITS | (1 << VB_UNKNOWN_BIT)
+UNKNOWN_VERSION_SCHEMA = 0x60000000
+UNKNOWN_VERSION_SCHEMA_THRESHOLD = 51
 
+WARN_UNKNOWN_RULES_MINED = "Warning: Unrecognised block version being mined! Unknown rules may or may not be in effect"
 WARN_UNKNOWN_RULES_ACTIVE = f"Unknown new rules activated (versionbit {VB_UNKNOWN_BIT})"
 VB_PATTERN = re.compile("Unknown new rules activated.*versionbit")
 
@@ -76,10 +79,33 @@ class VersionBitsWarningTest(BitcoinTestFramework):
         assert not VB_PATTERN.match(node.getmininginfo()["warnings"])
         assert not VB_PATTERN.match(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 error.
+        assert(WARN_UNKNOWN_RULES_MINED in node.getmininginfo()["warnings"])
+        assert(WARN_UNKNOWN_RULES_MINED in 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 node.getmininginfo()["warnings"])
+        assert(WARN_UNKNOWN_RULES_MINED in 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 node.getmininginfo()["warnings"])
+        assert(WARN_UNKNOWN_RULES_MINED not in 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 error.
+        assert(WARN_UNKNOWN_RULES_MINED in node.getmininginfo()["warnings"])
+        assert(WARN_UNKNOWN_RULES_MINED in node.getnetworkinfo()["warnings"])
+
         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.

From 5282a401974a2f6731e774844edacb5e38aa0cbb Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Sat, 6 Jan 2024 00:06:33 +0000
Subject: [PATCH 03/13] Warn users earlier (at LOCKED_IN) if a protocol change
 is detected

---
 src/validation.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 322e015571..54c93f152b 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2746,9 +2746,8 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
             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().warning(warning);
-                } else {
                     AppendWarning(warning_messages, warning);
                 }
             }

From 4f5f6c755ae44137aa8a5e69d96332a460623994 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Fri, 5 Jan 2024 20:25:43 +0000
Subject: [PATCH 04/13] Bugfix: Include unknown-signal warnings in debug log

---
 src/validation.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/validation.cpp b/src/validation.cpp
index 54c93f152b..07af57ba4d 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2785,6 +2785,7 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
             auto strWarning = _("Warning: Unrecognised block version being mined! Unknown rules may or may not be in effect");
             // notify GetWarnings(), called by Qt and the JSON-RPC code to warn the user:
             m_chainman.GetNotifications().warning(strWarning);
+            AppendWarning(warning_messages, strWarning);
         }
     }
     UpdateTipLog(coins_tip, pindexNew, params, __func__, "", warning_messages.original);

From 50944ca9a7a703ea31a12b4370e472eb27047878 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Fri, 5 Jan 2024 20:30:17 +0000
Subject: [PATCH 05/13] Refactor block warning messages to display multiple

---
 src/validation.cpp | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 07af57ba4d..bc808f99ae 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2746,10 +2746,7 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
             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);
-                {
-                    m_chainman.GetNotifications().warning(warning);
-                    AppendWarning(warning_messages, warning);
-                }
+                AppendWarning(warning_messages, warning);
             }
         }
 
@@ -2782,13 +2779,14 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
             pindex = pindex->pprev;
         }
         if (warning_threshold_hit) {
-            auto strWarning = _("Warning: Unrecognised block version being mined! Unknown rules may or may not be in effect");
-            // notify GetWarnings(), called by Qt and the JSON-RPC code to warn the user:
-            m_chainman.GetNotifications().warning(strWarning);
-            AppendWarning(warning_messages, strWarning);
+            const auto warning = _("Warning: Unrecognised block version being mined! Unknown rules may or may not be in effect");
+            AppendWarning(warning_messages, warning);
         }
     }
     UpdateTipLog(coins_tip, pindexNew, params, __func__, "", warning_messages.original);
+    if (!warning_messages.empty()) {
+        m_chainman.GetNotifications().warning(warning_messages);
+    }
 }
 
 /** Disconnect m_chain's tip.

From 95dbbb86a299bc1bd6bcfcb0775babb987ae553a Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Fri, 5 Jan 2024 20:49:00 +0000
Subject: [PATCH 06/13] Make protocol change warning clearer

---
 src/validation.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index bc808f99ae..60364d602a 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2745,7 +2745,7 @@ 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);
+                const bilingual_str warning = strprintf(_("WARNING: Unknown new rules activated (versionbit %i) - this software is not secure"), bit);
                 AppendWarning(warning_messages, warning);
             }
         }

From 105c907fa409d5c4d92033198bf46431d59656f0 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Fri, 5 Jan 2024 23:56:52 +0000
Subject: [PATCH 07/13] Bugfix: Only warn about an unknown versionbit signal if
 it's positive

---
 src/validation.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 60364d602a..5c40d42391 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2763,7 +2763,7 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
                 if ((pindex->nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) {
                     for (int bit = 0; bit < VERSIONBITS_NUM_BITS; ++bit) {
                         const int32_t mask = 1 << bit;
-                        if ((nExpectedVersion & mask) != (pindex->nVersion & mask)) {
+                        if ((pindex->nVersion & mask) && !(nExpectedVersion & mask)) {
                             if (++unexpected_bit_count[bit] > WARNING_THRESHOLD) {
                                 warning_threshold_hit = true;
                             }

From aa6e5154dbd88a962f9d7d1ee4f6c6c936c62f0d Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Fri, 5 Jan 2024 21:23:38 +0000
Subject: [PATCH 08/13] In unexpected version signalling warnings, list the
 specific bits involved

---
 src/validation.cpp                            | 24 +++++++++++++++----
 .../functional/feature_versionbits_warning.py |  7 +++---
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 5c40d42391..dbb046d8ca 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2754,7 +2754,8 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
         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;
         static constexpr int WARNING_THRESHOLD = 100/2;
-        bool warning_threshold_hit = false;
+        std::set<uint8_t> warning_threshold_hit_bits;
+        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());
@@ -2765,21 +2766,34 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
                         const int32_t mask = 1 << bit;
                         if ((pindex->nVersion & mask) && !(nExpectedVersion & mask)) {
                             if (++unexpected_bit_count[bit] > WARNING_THRESHOLD) {
-                                warning_threshold_hit = true;
+                                warning_threshold_hit_bits.insert(bit);
                             }
                         }
                     }
                 } else {
                     // Non-versionbits upgrade
                     if (++nonversionbit_count > WARNING_THRESHOLD) {
-                        warning_threshold_hit = true;
+                        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;
+                        }
                     }
                 }
             }
             pindex = pindex->pprev;
         }
-        if (warning_threshold_hit) {
-            const auto warning = _("Warning: Unrecognised block version being mined! Unknown rules may or may not be in effect");
+        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"), Join(warning_threshold_hit_bits, ", ", [](const uint8_t bit){ return ::ToString(int(bit)); }));
+            AppendWarning(warning_messages, 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);
+            }
             AppendWarning(warning_messages, warning);
         }
     }
diff --git a/test/functional/feature_versionbits_warning.py b/test/functional/feature_versionbits_warning.py
index 31292f3790..b8b43ef5bc 100755
--- a/test/functional/feature_versionbits_warning.py
+++ b/test/functional/feature_versionbits_warning.py
@@ -23,7 +23,8 @@ VB_UNKNOWN_VERSION = VB_TOP_BITS | (1 << VB_UNKNOWN_BIT)
 UNKNOWN_VERSION_SCHEMA = 0x60000000
 UNKNOWN_VERSION_SCHEMA_THRESHOLD = 51
 
-WARN_UNKNOWN_RULES_MINED = "Warning: Unrecognised block version being mined! Unknown rules may or may not be in effect"
+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})"
 WARN_UNKNOWN_RULES_ACTIVE = f"Unknown new rules activated (versionbit {VB_UNKNOWN_BIT})"
 VB_PATTERN = re.compile("Unknown new rules activated.*versionbit")
 
@@ -103,8 +104,8 @@ class VersionBitsWarningTest(BitcoinTestFramework):
         self.generatetoaddress(node, VB_PERIOD - VB_THRESHOLD, node_deterministic_address)
 
         # Check that get*info() shows the 51/100 unknown block version error.
-        assert(WARN_UNKNOWN_RULES_MINED in node.getmininginfo()["warnings"])
-        assert(WARN_UNKNOWN_RULES_MINED in node.getnetworkinfo()["warnings"])
+        assert(WARN_UNKNOWN_BIT_MINED in node.getmininginfo()["warnings"])
+        assert(WARN_UNKNOWN_BIT_MINED in node.getnetworkinfo()["warnings"])
 
         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

From ff929ecfecd3b5186d0db77441d5c3929f5d6f99 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Sat, 6 Jan 2024 02:22:19 +0000
Subject: [PATCH 09/13] Docfix: QA: feature_versionbits_warning: Warnings are
 not errors

---
 test/functional/feature_versionbits_warning.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/functional/feature_versionbits_warning.py b/test/functional/feature_versionbits_warning.py
index b8b43ef5bc..316596ed41 100755
--- a/test/functional/feature_versionbits_warning.py
+++ b/test/functional/feature_versionbits_warning.py
@@ -83,7 +83,7 @@ class VersionBitsWarningTest(BitcoinTestFramework):
         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 error.
+        # Check that get*info() shows the 51/100 unknown block version warning
         assert(WARN_UNKNOWN_RULES_MINED in node.getmininginfo()["warnings"])
         assert(WARN_UNKNOWN_RULES_MINED in node.getnetworkinfo()["warnings"])
         # Close the period normally
@@ -103,7 +103,7 @@ class VersionBitsWarningTest(BitcoinTestFramework):
         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 error.
+        # Check that get*info() shows the 51/100 unknown block version warning
         assert(WARN_UNKNOWN_BIT_MINED in node.getmininginfo()["warnings"])
         assert(WARN_UNKNOWN_BIT_MINED in node.getnetworkinfo()["warnings"])
 

From c32468f51d5ce8b5a89de0ef055945ed59be1aa6 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Sat, 6 Jan 2024 03:11:34 +0000
Subject: [PATCH 10/13] Defer N/100 blocks signalling warning to 76/100 for
 BIP320 bits

---
 src/validation.cpp                            |  5 +++--
 .../functional/feature_versionbits_warning.py | 20 ++++++++++++++++++-
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index dbb046d8ca..384f16a7e4 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2753,7 +2753,6 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
         // 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;
-        static constexpr int WARNING_THRESHOLD = 100/2;
         std::set<uint8_t> warning_threshold_hit_bits;
         int32_t warning_threshold_hit_int{-1};
         for (int i = 0; i < 100 && pindex != nullptr; i++)
@@ -2765,13 +2764,15 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
                     for (int bit = 0; bit < VERSIONBITS_NUM_BITS; ++bit) {
                         const int32_t mask = 1 << bit;
                         if ((pindex->nVersion & mask) && !(nExpectedVersion & mask)) {
-                            if (++unexpected_bit_count[bit] > WARNING_THRESHOLD) {
+                            const int warning_threshold = (bit > 12 ? 75 : 50);
+                            if (++unexpected_bit_count[bit] > warning_threshold) {
                                 warning_threshold_hit_bits.insert(bit);
                             }
                         }
                     }
                 } else {
                     // 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;
diff --git a/test/functional/feature_versionbits_warning.py b/test/functional/feature_versionbits_warning.py
index 316596ed41..e2205453b2 100755
--- a/test/functional/feature_versionbits_warning.py
+++ b/test/functional/feature_versionbits_warning.py
@@ -18,13 +18,17 @@ 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})"
+WARN_BIP320_BIT_MINED = f"Warning: Miners are attempting to activate unknown new rules (bit {VB_BIP320_BIT})"
 WARN_UNKNOWN_RULES_ACTIVE = f"Unknown new rules activated (versionbit {VB_UNKNOWN_BIT})"
 VB_PATTERN = re.compile("Unknown new rules activated.*versionbit")
 
@@ -107,6 +111,20 @@ class VersionBitsWarningTest(BitcoinTestFramework):
         assert(WARN_UNKNOWN_BIT_MINED in node.getmininginfo()["warnings"])
         assert(WARN_UNKNOWN_BIT_MINED in node.getnetworkinfo()["warnings"])
 
+        self.log.info("Check that there is a warning if >75 blocks in the last 100 were a BIP320 version")
+        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 node.getmininginfo()["warnings"])
+        assert(WARN_BIP320_BIT_MINED not in node.getnetworkinfo()["warnings"])
+        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 node.getmininginfo()["warnings"])
+        assert(WARN_BIP320_BIT_MINED in node.getnetworkinfo()["warnings"])
+        self.generatetoaddress(node, 1, node_deterministic_address)
+        assert(WARN_BIP320_BIT_MINED in node.getmininginfo()["warnings"])
+        assert(WARN_BIP320_BIT_MINED 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.

From ce89dc95669a4ce681c1b09de9a6c7a2bb7fb170 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Sat, 6 Jan 2024 03:54:22 +0000
Subject: [PATCH 11/13] Once an unexpected-signal warning is triggered, persist
 it until node restart

---
 src/validation.cpp                             | 5 +++--
 test/functional/feature_versionbits_warning.py | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 384f16a7e4..fc33223367 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2753,8 +2753,9 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
         // 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;
-        std::set<uint8_t> warning_threshold_hit_bits;
-        int32_t warning_threshold_hit_int{-1};
+        // 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());
diff --git a/test/functional/feature_versionbits_warning.py b/test/functional/feature_versionbits_warning.py
index e2205453b2..17c3331f9a 100755
--- a/test/functional/feature_versionbits_warning.py
+++ b/test/functional/feature_versionbits_warning.py
@@ -28,7 +28,8 @@ 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})"
-WARN_BIP320_BIT_MINED = f"Warning: Miners are attempting to activate unknown new rules (bit {VB_BIP320_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})"
 VB_PATTERN = re.compile("Unknown new rules activated.*versionbit")
 

From 31eaa83c5526dabc0508315938b8293328ef1b21 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Sat, 6 Jan 2024 04:26:10 +0000
Subject: [PATCH 12/13] Warn in the debug log (only) for blocks where the block
 version is being abused

---
 src/validation.cpp                            | 10 +++++++++-
 .../functional/feature_versionbits_warning.py | 20 +++++++++++++++----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index fc33223367..72646cddee 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2799,10 +2799,18 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
             AppendWarning(warning_messages, warning);
         }
     }
-    UpdateTipLog(coins_tip, pindexNew, params, __func__, "", warning_messages.original);
+
     if (!warning_messages.empty()) {
         m_chainman.GetNotifications().warning(warning_messages);
     }
+
+    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");
+        AppendWarning(warning_messages, warning);
+    }
+
+    UpdateTipLog(coins_tip, pindexNew, params, __func__, "", warning_messages.original);
 }
 
 /** Disconnect m_chain's tip.
diff --git a/test/functional/feature_versionbits_warning.py b/test/functional/feature_versionbits_warning.py
index 17c3331f9a..ad7882d75f 100755
--- a/test/functional/feature_versionbits_warning.py
+++ b/test/functional/feature_versionbits_warning.py
@@ -31,6 +31,7 @@ WARN_UNKNOWN_BIT_MINED = f"Warning: Miners are attempting to activate unknown ne
 # 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):
@@ -112,18 +113,29 @@ class VersionBitsWarningTest(BitcoinTestFramework):
         assert(WARN_UNKNOWN_BIT_MINED in node.getmininginfo()["warnings"])
         assert(WARN_UNKNOWN_BIT_MINED in node.getnetworkinfo()["warnings"])
 
-        self.log.info("Check that there is a warning if >75 blocks in the last 100 were a BIP320 version")
-        self.send_blocks_with_version(peer, VB_BIP320_THRESHOLD - 1, VB_BIP320_VERSION)
+        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.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 node.getmininginfo()["warnings"])
         assert(WARN_BIP320_BIT_MINED not in node.getnetworkinfo()["warnings"])
-        self.send_blocks_with_version(peer, 1, VB_BIP320_VERSION)
+        # ...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.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 node.getmininginfo()["warnings"])
         assert(WARN_BIP320_BIT_MINED in node.getnetworkinfo()["warnings"])
-        self.generatetoaddress(node, 1, node_deterministic_address)
+        assert(WARN_BIP320_BLOCK not in node.getmininginfo()["warnings"])
+        assert(WARN_BIP320_BLOCK not in node.getnetworkinfo()["warnings"])
+        with node.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 node.getmininginfo()["warnings"])
         assert(WARN_BIP320_BIT_MINED in 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.")

From 0da086b42b6003bbb10e42e3304415fe295d57e3 Mon Sep 17 00:00:00 2001
From: Luke Dashjr <luke-jr+git@utopios.org>
Date: Tue, 26 Mar 2024 02:25:31 +0000
Subject: [PATCH 13/13] Refactor N/100 blocks signalling warning to detect edge
 case

Previously, it would have skipped the non-vb check if the set top bits were cleared
---
 src/validation.cpp | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index 72646cddee..f23b702493 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2759,26 +2759,25 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew)
         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 && (pindex->nVersion & ~nExpectedVersion) != 0)
-            {
-                if ((pindex->nVersion & VERSIONBITS_TOP_MASK) == VERSIONBITS_TOP_BITS) {
-                    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);
-                            }
-                        }
+            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 {
-                    // 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);
                         }
                     }
                 }