From e01a7939d3a3b231f68ae2f36cbc4de0cf4d4999 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Tue, 7 Oct 2014 13:11:48 -0400 Subject: [PATCH 1/2] Refactor -alertnotify code Refactor common -alertnotify code into static CAlert::Notify method. --- src/alert.cpp | 37 +++++++++++++++++++++---------------- src/alert.h | 3 ++- src/main.cpp | 11 +++-------- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/alert.cpp b/src/alert.cpp index 3271ecfbf..d49584920 100644 --- a/src/alert.cpp +++ b/src/alert.cpp @@ -233,25 +233,30 @@ bool CAlert::ProcessAlert(bool fThread) if(AppliesToMe()) { uiInterface.NotifyAlertChanged(GetHash(), CT_NEW); - std::string strCmd = GetArg("-alertnotify", ""); - if (!strCmd.empty()) - { - // Alert text should be plain ascii coming from a trusted source, but to - // be safe we first strip anything not in safeChars, then add single quotes around - // the whole string before passing it to the shell: - std::string singleQuote("'"); - std::string safeStatus = SanitizeString(strStatusBar); - safeStatus = singleQuote+safeStatus+singleQuote; - boost::replace_all(strCmd, "%s", safeStatus); - - if (fThread) - boost::thread t(runCommand, strCmd); // thread runs free - else - runCommand(strCmd); - } + Notify(strStatusBar, fThread); } } LogPrint("alert", "accepted alert %d, AppliesToMe()=%d\n", nID, AppliesToMe()); return true; } + +void +CAlert::Notify(const std::string& strMessage, bool fThread) +{ + std::string strCmd = GetArg("-alertnotify", ""); + if (strCmd.empty()) return; + + // Alert text should be plain ascii coming from a trusted source, but to + // be safe we first strip anything not in safeChars, then add single quotes around + // the whole string before passing it to the shell: + std::string singleQuote("'"); + std::string safeStatus = SanitizeString(strMessage); + safeStatus = singleQuote+safeStatus+singleQuote; + boost::replace_all(strCmd, "%s", safeStatus); + + if (fThread) + boost::thread t(runCommand, strCmd); // thread runs free + else + runCommand(strCmd); +} diff --git a/src/alert.h b/src/alert.h index 5ecf94cea..ba3235858 100644 --- a/src/alert.h +++ b/src/alert.h @@ -101,7 +101,8 @@ public: bool AppliesToMe() const; bool RelayTo(CNode* pnode) const; bool CheckSignature() const; - bool ProcessAlert(bool fThread = true); + bool ProcessAlert(bool fThread = true); // fThread means run -alertnotify in a free-running thread + static void Notify(const std::string& strMessage, bool fThread); /* * Get copy of (active) alert object by hash. Returns a null alert if it is not found. diff --git a/src/main.cpp b/src/main.cpp index 5d46c30a9..106c336a8 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1178,14 +1178,9 @@ void CheckForkWarningConditions() { if (!fLargeWorkForkFound) { - std::string strCmd = GetArg("-alertnotify", ""); - if (!strCmd.empty()) - { - std::string warning = std::string("'Warning: Large-work fork detected, forking after block ") + - pindexBestForkBase->phashBlock->ToString() + std::string("'"); - boost::replace_all(strCmd, "%s", warning); - boost::thread t(runCommand, strCmd); // thread runs free - } + std::string warning = std::string("'Warning: Large-work fork detected, forking after block ") + + pindexBestForkBase->phashBlock->ToString() + std::string("'"); + CAlert::Notify(warning, true); } if (pindexBestForkTip) { From dbca89b74b76610331d21656cd6747f5bf8375d6 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Tue, 7 Oct 2014 14:22:58 -0400 Subject: [PATCH 2/2] Trigger -alertnotify if network is upgrading without you This adds a -regetest-only undocumented (for regression testing only) command-line option -blockversion=N to set block.nVersion. Adds to the "has the rest of the network upgraded to a block.nVersion we don't understand" code so it calls -alertnotify when 51 of the last 100 blocks are up-version. But it only alerts once, not with every subsequent new, upversion block. And adds a forknotify.py regression test to make sure it works. Tested using forknotify.py: Before adding CAlert::Notify, get: Assertion failed: -alertnotify did not warn of up-version blocks Before adding code to only alert once: Assertion failed: -alertnotify excessive warning of up-version blocks After final code in this pull: Tests successful --- qa/pull-tester/build-tests.sh.in | 1 + qa/rpc-tests/forknotify.py | 65 ++++++++++++++++++++++++++++++++ src/main.cpp | 7 +++- src/miner.cpp | 5 +++ 4 files changed, 77 insertions(+), 1 deletion(-) create mode 100755 qa/rpc-tests/forknotify.py diff --git a/qa/pull-tester/build-tests.sh.in b/qa/pull-tester/build-tests.sh.in index ebf377a48..1ef47d77f 100755 --- a/qa/pull-tester/build-tests.sh.in +++ b/qa/pull-tester/build-tests.sh.in @@ -75,6 +75,7 @@ make check # Run RPC integration test on Linux: @abs_top_srcdir@/qa/rpc-tests/wallet.sh @abs_top_srcdir@/linux-build/src @abs_top_srcdir@/qa/rpc-tests/listtransactions.py --srcdir @abs_top_srcdir@/linux-build/src +@abs_top_srcdir@/qa/rpc-tests/forknotify.py --srcdir @abs_top_srcdir@/linux-build/src # Clean up cache/ directory that the python regression tests create rm -rf cache diff --git a/qa/rpc-tests/forknotify.py b/qa/rpc-tests/forknotify.py new file mode 100755 index 000000000..a482f7cc5 --- /dev/null +++ b/qa/rpc-tests/forknotify.py @@ -0,0 +1,65 @@ +#!/usr/bin/env python +# Copyright (c) 2014 The Bitcoin Core developers +# Distributed under the MIT/X11 software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +# +# Test -alertnotify +# + +from test_framework import BitcoinTestFramework +from bitcoinrpc.authproxy import AuthServiceProxy, JSONRPCException +from util import * +import os +import shutil + +class ForkNotifyTest(BitcoinTestFramework): + + alert_filename = None # Set by setup_network + + def setup_network(self, test_dir): + nodes = [] + self.alert_filename = os.path.join(test_dir, "alert.txt") + with open(self.alert_filename, 'w') as f: + pass # Just open then close to create zero-length file + nodes.append(start_node(0, test_dir, + ["-blockversion=2", "-alertnotify=echo %s >> '" + self.alert_filename + "'"])) + # Node1 mines block.version=211 blocks + nodes.append(start_node(1, test_dir, + ["-blockversion=211"])) + connect_nodes(nodes[1], 0) + + sync_blocks(nodes) + return nodes + + + def run_test(self, nodes): + # Mine 51 up-version blocks + nodes[1].setgenerate(True, 51) + sync_blocks(nodes) + # -alertnotify should trigger on the 51'st, + # but mine and sync another to give + # -alertnotify time to write + nodes[1].setgenerate(True, 1) + sync_blocks(nodes) + + with open(self.alert_filename, 'r') as f: + alert_text = f.read() + + if len(alert_text) == 0: + raise AssertionError("-alertnotify did not warn of up-version blocks") + + # Mine more up-version blocks, should not get more alerts: + nodes[1].setgenerate(True, 1) + sync_blocks(nodes) + nodes[1].setgenerate(True, 1) + sync_blocks(nodes) + + with open(self.alert_filename, 'r') as f: + alert_text2 = f.read() + + if alert_text != alert_text2: + raise AssertionError("-alertnotify excessive warning of up-version blocks") + +if __name__ == '__main__': + ForkNotifyTest().main() diff --git a/src/main.cpp b/src/main.cpp index 106c336a8..43b81aaef 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1755,7 +1755,8 @@ void static UpdateTip(CBlockIndex *pindexNew) { cvBlockChange.notify_all(); // Check the version of the last 100 blocks to see if we need to upgrade: - if (!IsInitialBlockDownload()) + static bool fWarned = false; + if (!IsInitialBlockDownload() && !fWarned) { int nUpgraded = 0; const CBlockIndex* pindex = chainActive.Tip(); @@ -1768,8 +1769,12 @@ void static UpdateTip(CBlockIndex *pindexNew) { if (nUpgraded > 0) LogPrintf("SetBestChain: %d of last 100 blocks above version %d\n", nUpgraded, (int)CBlock::CURRENT_VERSION); if (nUpgraded > 100/2) + { // strMiscWarning is read by GetWarnings(), called by Qt and the JSON-RPC code to warn the user: strMiscWarning = _("Warning: This version is obsolete, upgrade required!"); + CAlert::Notify(strMiscWarning, true); + fWarned = true; + } } } diff --git a/src/miner.cpp b/src/miner.cpp index 280349e8c..2eb028b1d 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -83,6 +83,11 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn) return NULL; CBlock *pblock = &pblocktemplate->block; // pointer for convenience + // -regtest only: allow overriding block.nVersion with + // -blockversion=N to test forking scenarios + if (Params().MineBlocksOnDemand()) + pblock->nVersion = GetArg("-blockversion", pblock->nVersion); + // Create coinbase tx CMutableTransaction txNew; txNew.vin.resize(1);