Merge 15861 via restore_vbits_warning

src/validation.cpp

@@ -2780,15 +2780,70 @@ 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);
-                if (state == ThresholdState::ACTIVE) {
+                AppendWarning(warning_messages, warning);
-                    m_chainman.GetNotifications().warning(warning);
-                } else {
-                    AppendWarning(warning_messages, 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"), 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);
+        }
     }
+
+    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);
 }
 

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(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 warning
+        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 warning
+        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 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"])
+        # ...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"])
+        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.")
         # 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

@@ -492,7 +492,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:
@@ -509,6 +509,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