From 396f4b048c3d5adbc318bcc46b60f836e051e443 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 31 Jan 2024 15:48:51 -0500 Subject: [PATCH 1/2] Merge bitcoin/bitcoin#28956: Nuke adjusted time from validation (attempt 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ff9039f6ea876bab2c40a06a93e0dd087f445fa2 Remove GetAdjustedTime (dergoegge) Pull request description: This picks up parts of #25908. The use of adjusted time is removed from validation code while the warning to users if their clock is out of sync with the rest of the network remains. ACKs for top commit: naumenkogs: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 achow101: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 maflcko: lgtm ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 🤽 stickies-v: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 Tree-SHA512: d1f6b9445c236915503fd2ea828f0d3b92285a5dbc677b168453276115e349972edbad37194d8becd9136d8e7219b576af64ec51c72bdb1923e57e405c0483fc (cherry picked from commit 3c13f5d6124d887ebeb7f473d73722b570945130) --- src/bitcoin-chainstate.cpp | 1 - src/init.cpp | 1 - src/kernel/chainstatemanager_opts.h | 2 -- src/net_processing.cpp | 8 ++++---- src/node/miner.cpp | 6 +++--- src/rpc/mining.cpp | 4 ++-- src/test/bls_tests.cpp | 2 +- src/test/util/setup_common.cpp | 1 - src/validation.cpp | 14 ++++++-------- src/validation.h | 4 ---- 10 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 271a6ed7ed5c..a5f7f91c9ff6 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -88,7 +88,6 @@ int main(int argc, char* argv[]) // SETUP: Chainstate const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, - .adjusted_time_callback = static_cast(GetTime), }; ChainstateManager chainman{chainman_opts}; diff --git a/src/init.cpp b/src/init.cpp index 3884ed3fd7f4..cbd4254f5a71 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2021,7 +2021,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, - .adjusted_time_callback = GetAdjustedTime, }; node.chainman = std::make_unique(chainman_opts); ChainstateManager& chainman = *node.chainman; diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index 510a1f9edcfa..5c7b9cce63f2 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -6,7 +6,6 @@ #define BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H #include -#include class CChainParams; @@ -19,7 +18,6 @@ namespace kernel { */ struct ChainstateManagerOpts { const CChainParams& chainparams; - const std::function adjusted_time_callback{nullptr}; }; } // namespace kernel diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 2aabdc199172..91336d1e2225 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1406,7 +1406,7 @@ bool PeerManagerImpl::TipMayBeStale() bool PeerManagerImpl::CanDirectFetch() { - return m_chainman.ActiveChain().Tip()->GetBlockTime() > GetAdjustedTime() - m_chainparams.GetConsensus().nPowTargetSpacing * 20; + return m_chainman.ActiveChain().Tip()->GetBlockTime() > GetTime() - m_chainparams.GetConsensus().nPowTargetSpacing * 20; } static bool PeerHasHeader(CNodeState *state, const CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main) @@ -6180,7 +6180,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (!state.fSyncStarted && CanServeBlocks(*peer) && !fImporting && !fReindex && pto->CanRelay()) { // Only actively request headers from a single peer, unless we're close to end of initial download. - if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->GetBlockTime() > GetAdjustedTime() - nMaxTipAge) { + if ((nSyncStarted == 0 && sync_blocks_and_headers_from_peer) || m_chainman.m_best_header->GetBlockTime() > GetTime() - nMaxTipAge) { const CBlockIndex* pindexStart = m_chainman.m_best_header; /* If possible, start at the block preceding the currently best known header. This ensures that we always get a @@ -6201,7 +6201,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Convert HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER to microseconds before scaling // to maintain precision std::chrono::microseconds{HEADERS_DOWNLOAD_TIMEOUT_PER_HEADER} * - (GetAdjustedTime() - m_chainman.m_best_header->GetBlockTime()) / consensusParams.nPowTargetSpacing + (GetTime() - m_chainman.m_best_header->GetBlockTime()) / consensusParams.nPowTargetSpacing ); nSyncStarted++; } @@ -6582,7 +6582,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Check for headers sync timeouts if (state.fSyncStarted && state.m_headers_sync_timeout < std::chrono::microseconds::max()) { // Detect whether this is a stalling initial-headers-sync peer - if (m_chainman.m_best_header->GetBlockTime() <= GetAdjustedTime() - nMaxTipAge) { + if (m_chainman.m_best_header->GetBlockTime() <= GetTime() - nMaxTipAge) { if (current_time > state.m_headers_sync_timeout && nSyncStarted == 1 && (m_num_preferred_download_peers - state.fPreferredDownload >= 1)) { // Disconnect a peer (without NetPermissionFlags::NoBan permission) if it is our only sync peer, // and we have others we could be using instead. diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 86b563b2567f..b8efd851f760 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -49,7 +49,7 @@ namespace node { int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev) { int64_t nOldTime = pblock->nTime; - int64_t nNewTime = std::max(pindexPrev->GetMedianTimePast() + 1, GetAdjustedTime()); + int64_t nNewTime = std::max(pindexPrev->GetMedianTimePast() + 1, GetTime()); if (nOldTime < nNewTime) { pblock->nTime = nNewTime; @@ -218,7 +218,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc pblock->nVersion = gArgs.GetIntArg("-blockversion", pblock->nVersion); } - pblock->nTime = GetAdjustedTime(); + pblock->nTime = GetTime(); m_lock_time_cutoff = pindexPrev->GetMedianTimePast(); if (fDIP0003Active_context) { @@ -334,7 +334,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc pblocktemplate->vTxSigOps[0] = GetLegacySigOpCount(*pblock->vtx[0]); BlockValidationState state; - if (!TestBlockValidity(state, m_chainlocks, m_evoDb, chainparams, m_chainstate, *pblock, pindexPrev, GetAdjustedTime, false, false)) { + if (!TestBlockValidity(state, m_chainlocks, m_evoDb, chainparams, m_chainstate, *pblock, pindexPrev, false, false)) { throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, state.ToString())); } int64_t nTime2 = GetTimeMicros(); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index f419d141ba87..d46e24e337b3 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -377,7 +377,7 @@ static RPCHelpMan generateblock() BlockValidationState state; if (!TestBlockValidity(state, *CHECK_NONFATAL(node.chainlocks), *CHECK_NONFATAL(node.evodb), chainman.GetParams(), active_chainstate, - block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), GetAdjustedTime, false, false)) { + block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), false, false)) { throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.GetRejectReason())); } } @@ -698,7 +698,7 @@ static RPCHelpMan getblocktemplate() return "inconclusive-not-best-prevblk"; BlockValidationState state; TestBlockValidity(state, *CHECK_NONFATAL(node.chainlocks), *CHECK_NONFATAL(node.evodb), chainman.GetParams(), active_chainstate, - block, pindexPrev, GetAdjustedTime, false, true); + block, pindexPrev, false, true); return BIP22ValidationResult(state); } diff --git a/src/test/bls_tests.cpp b/src/test/bls_tests.cpp index ffc6c05082b1..b236584ed933 100644 --- a/src/test/bls_tests.cpp +++ b/src/test/bls_tests.cpp @@ -680,7 +680,7 @@ BOOST_AUTO_TEST_CASE(v19_boundary_validation_failure_restores_bls_scheme) BlockValidationState state; BOOST_CHECK(!TestBlockValidity(state, *Assert(setup.m_node.chainlocks), *Assert(setup.m_node.evodb), Params(), chainman.ActiveChainstate(), proposal_block, chainman.ActiveChain().Tip(), - GetAdjustedTime, /*fCheckPOW=*/true, /*fCheckMerkleRoot=*/true)); + /*fCheckPOW=*/true, /*fCheckMerkleRoot=*/true)); BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-txns-inputs-missingorspent"); } BOOST_CHECK(bls::bls_legacy_scheme.load()); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 895358f6507d..96777fed90c1 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -291,7 +291,6 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, - .adjusted_time_callback = GetAdjustedTime, }; m_node.chainman = std::make_unique(chainman_opts); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique(m_cache_sizes.block_tree_db, true); diff --git a/src/validation.cpp b/src/validation.cpp index 012783385685..182a57fa5df7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2169,8 +2169,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, // for one approach that was used for BIP 141 deployment). // Also, currently the rule against blocks more than 2 hours in the future // is enforced in ContextualCheckBlockHeader(); we wouldn't want to - // re-enforce that rule here (at least until we make it impossible for - // m_adjusted_time_callback() to go backward). + // re-enforce that rule here. if (!CheckBlock(block, state, m_params.GetConsensus(), !fJustCheck, !fJustCheck)) { if (state.GetResult() == BlockValidationResult::BLOCK_MUTATED) { // We don't write down blocks to disk if they may have been @@ -3915,7 +3914,7 @@ bool IsBlockMutated(const CBlock& block) * in ConnectBlock(). * Note that -reindex-chainstate skips the validation that happens here! */ -static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const ChainstateManager& chainman, const CBlockIndex* pindexPrev, int64_t nAdjustedTime) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) +static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidationState& state, BlockManager& blockman, const ChainstateManager& chainman, const CBlockIndex* pindexPrev) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); assert(pindexPrev != nullptr); @@ -3955,8 +3954,8 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "time-too-old", strprintf("block's timestamp is too early %d %d", block.GetBlockTime(), pindexPrev->GetMedianTimePast())); // Check timestamp - if (block.GetBlockTime() > nAdjustedTime + MAX_FUTURE_BLOCK_TIME) - return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, "time-too-new", strprintf("block timestamp too far in the future %d %d", block.GetBlockTime(), nAdjustedTime + 2 * 60 * 60)); + if (block.GetBlockTime() > GetTime() + MAX_FUTURE_BLOCK_TIME) + return state.Invalid(BlockValidationResult::BLOCK_TIME_FUTURE, "time-too-new", strprintf("block timestamp too far in the future %d %d", block.GetBlockTime(), GetTime() + MAX_FUTURE_BLOCK_TIME)); // Reject blocks with outdated version if ((block.nVersion < 2 && DeploymentActiveAfter(pindexPrev, chainman.GetConsensus(), Consensus::DEPLOYMENT_HEIGHTINCB)) || @@ -4094,7 +4093,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida return state.Invalid(BlockValidationResult::BLOCK_CHAINLOCK, "bad-prevblk-chainlock"); } - if (!ContextualCheckBlockHeader(block, state, m_blockman, *this, pindexPrev, m_adjusted_time_callback())) { + if (!ContextualCheckBlockHeader(block, state, m_blockman, *this, pindexPrev)) { LogPrint(BCLog::VALIDATION, "%s: Consensus::ContextualCheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString()); return false; } @@ -4360,7 +4359,6 @@ bool TestBlockValidity(BlockValidationState& state, CChainState& chainstate, const CBlock& block, CBlockIndex* pindexPrev, - const std::function& adjusted_time_callback, bool fCheckPOW, bool fCheckMerkleRoot) { @@ -4384,7 +4382,7 @@ bool TestBlockValidity(BlockValidationState& state, auto dbTx = evoDb.BeginTransaction(); // NOTE: CheckBlockHeader is called by CheckBlock - if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev, adjusted_time_callback())) + if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev)) return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, state.ToString()); if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot)) return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString()); diff --git a/src/validation.h b/src/validation.h index 307865f5f558..60a776dd56ee 100644 --- a/src/validation.h +++ b/src/validation.h @@ -382,7 +382,6 @@ bool TestBlockValidity(BlockValidationState& state, CChainState& chainstate, const CBlock& block, CBlockIndex* pindexPrev, - const std::function& adjusted_time_callback, bool fCheckPOW = true, bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -904,8 +903,6 @@ class ChainstateManager const CChainParams m_chainparams; - const std::function m_adjusted_time_callback; - //! Internal helper for ActivateSnapshot(). [[nodiscard]] bool PopulateAndValidateSnapshot( CChainState& snapshot_chainstate, @@ -930,7 +927,6 @@ class ChainstateManager explicit ChainstateManager(const Options& opts) : m_chainparams{opts.chainparams}, - m_adjusted_time_callback{Assert(opts.adjusted_time_callback)}, m_blockman{{opts.chainparams}} {}; const CChainParams& GetParams() const { return m_chainparams; } From c32a42a92de5a2dbf46c581c8f625b5df74e2910 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 11 Jan 2024 12:08:55 -0500 Subject: [PATCH 2/2] Merge bitcoin/bitcoin#29034: test: detect OS in functional tests consistently using `platform.system()` 878d914777a03a04ecb84217152e8b7fd73a5062 doc: test: mention OS detection preferences in style guideline (Sebastian Falbesoner) 4c65ac96f8b021c107783adce3e8afe4f8edee6e test: detect OS consistently using `platform.system()` (Sebastian Falbesoner) 37324ae3dfb0e50daaf752dc863a880559fa4637 test: use `skip_if_platform_not_linux` helper where possible (Sebastian Falbesoner) Pull request description: There are at least three ways to detect the operating system in Python3: - `os.name` (https://docs.python.org/3.9/library/os.html#os.name) - `sys.platform` (https://docs.python.org/3.9/library/sys.html#sys.platform) - `platform.system()` (https://docs.python.org/3.9/library/platform.html#platform.system) We are currently using all of them in functional tests (both in individual tests and shared test framework code), which seems a bit messy. This PR consolidates into using `platform.system()`, as it appears to be one most consistent and easy to read (see also [IRC discussion](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-12-08#989301;) and table below). `sys.platform` is inconsistent as it has the major version number encoded for BSD systems, which doesn't make much sense for e.g. OpenBSD, where there is no concept of major versions, but instead the version is simply increased by 0.1 on each release. Note that `os.name` is still useful to detect whether we are running a POSIX system (see `BitcoinTestFramework.skip_if_platform_not_posix`), so for this use-case it is kept as only exception. The following table shows values for common operating systems, found via ``` $ python3 -c "import os; import sys; import platform; print(os.name, sys.platform, platform.system())" ``` | OS | os.name | sys.platform | platform.system() | |--------------|---------|--------------|--------------------| | Linux 6.2.0 | posix | linux | Linux | | MacOS* | posix | darwin | Darwin | | OpenBSD 7.4 | posix | openbsd7 | OpenBSD | | Windows* | nt | win32 | Windows | \* = I neither have a MacOS nor a Windows machine available, so I extracted the values from documentation and our current code. Also I'm relying on CI for testing the relevant code-paths. Having reviewers to this this locally would be very appreciated, if this gets Concept ACKed. ACKs for top commit: kevkevinpal: ACK [878d914](https://github.com/bitcoin/bitcoin/pull/29034/commits/878d914777a03a04ecb84217152e8b7fd73a5062) achow101: ACK 878d914777a03a04ecb84217152e8b7fd73a5062 hebasto: ACK 878d914777a03a04ecb84217152e8b7fd73a5062, I have reviewed the code and it looks OK. pablomartin4btc: tACK 878d914777a03a04ecb84217152e8b7fd73a5062 Tree-SHA512: 24513d493e47f572028c843260b81c47c2c29bfb701991050255c9f9529cd19065ecbc7b3b6e15619da7f3f608b4825c345ce6fee30d8fd1eaadbd08cff400fc (cherry picked from commit bb6de1befb9f687337ac615e0628b9b40c77eb3d) --- test/functional/README.md | 4 +++ test/functional/feature_bind_extra.py | 10 ++---- test/functional/feature_init.py | 4 +-- test/functional/feature_notifications.py | 9 +++--- test/functional/rpc_bind.py | 11 +++---- test/functional/test_framework/p2p.py | 3 +- test/functional/test_framework/test_node.py | 4 +-- test/functional/test_runner.py | 35 +++++++++++---------- test/functional/wallet_multiwallet.py | 4 +-- 9 files changed, 42 insertions(+), 42 deletions(-) diff --git a/test/functional/README.md b/test/functional/README.md index 2586c66c8368..76e5d16cceb9 100644 --- a/test/functional/README.md +++ b/test/functional/README.md @@ -37,6 +37,10 @@ don't have test cases for. `set_test_params()`, `add_options()` and `setup_xxxx()` methods at the top of the subclass, then locally-defined helper methods, then the `run_test()` method. - Use `f'{x}'` for string formatting in preference to `'{}'.format(x)` or `'%s' % x`. +- Use `platform.system()` for detecting the running operating system and `os.name` to + check whether it's a POSIX system (see also the `skip_if_platform_not_{linux,posix}` + methods in the `BitcoinTestFramework` class, which can be used to skip a whole test + depending on the platform). #### Naming guidelines diff --git a/test/functional/feature_bind_extra.py b/test/functional/feature_bind_extra.py index 5de9ff203cc2..a6d76854a6d6 100755 --- a/test/functional/feature_bind_extra.py +++ b/test/functional/feature_bind_extra.py @@ -7,15 +7,12 @@ that bind happens on the expected ports. """ -import sys - from test_framework.netutil import ( addr_to_hex, get_bind_addrs, ) from test_framework.test_framework import ( BitcoinTestFramework, - SkipTest, ) from test_framework.util import ( assert_equal, @@ -32,12 +29,11 @@ def set_test_params(self): self.bind_to_localhost_only = False self.num_nodes = 2 - def setup_network(self): + def skip_test_if_missing_module(self): # Due to OS-specific network stats queries, we only run on Linux. - self.log.info("Checking for Linux") - if not sys.platform.startswith('linux'): - raise SkipTest("This test can only be run on Linux.") + self.skip_if_platform_not_linux() + def setup_network(self): loopback_ipv4 = addr_to_hex("127.0.0.1") # Start custom ports by reusing unused p2p ports diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index ce12319933de..4622783e81e4 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -3,7 +3,7 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Stress tests related to node initialization.""" -import os +import platform from pathlib import Path from random import randint import shutil @@ -37,7 +37,7 @@ def run_test(self): # and other approaches (like below) don't work: # # os.kill(node.process.pid, signal.CTRL_C_EVENT) - if os.name == 'nt': + if platform.system() == 'Windows': raise SkipTest("can't SIGTERM on Windows") self.stop_node(0) diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index a77ae3dbb0ff..de482e57b2e5 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -5,6 +5,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the -alertnotify, -blocknotify, -chainlocknotify, -instantsendnotify and -walletnotify options.""" import os +import platform from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE @@ -15,13 +16,13 @@ # Linux allow all characters other than \x00 # Windows disallow control characters (0-31) and /\?%:|"<> -FILE_CHAR_START = 32 if os.name == 'nt' else 1 +FILE_CHAR_START = 32 if platform.system() == 'Windows' else 1 FILE_CHAR_END = 128 -FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if os.name == 'nt' else '/' +FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if platform.system() == 'Windows' else '/' UNCONFIRMED_HASH_STRING = 'unconfirmed' def notify_outputname(walletname, txid): - return txid if os.name == 'nt' else f'{walletname}_{txid}' + return txid if platform.system() == 'Windows' else f'{walletname}_{txid}' class NotificationsTest(DashTestFramework): @@ -165,7 +166,7 @@ def expect_wallet_notify(self, tx_details): # Universal newline ensures '\n' on 'nt' assert_equal(text[-1], '\n') text = text[:-1] - if os.name == 'nt': + if platform.system() == 'Windows': # On Windows, echo as above will append a whitespace assert_equal(text[-1], ' ') text = text[:-1] diff --git a/test/functional/rpc_bind.py b/test/functional/rpc_bind.py index 91c624222dc7..9a8990161568 100755 --- a/test/functional/rpc_bind.py +++ b/test/functional/rpc_bind.py @@ -4,8 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test running dashd with the -rpcbind and -rpcallowip options.""" -import sys - from test_framework.netutil import all_interfaces, addr_to_hex, get_bind_addrs, test_ipv6_local from test_framework.test_framework import BitcoinTestFramework, SkipTest from test_framework.util import assert_equal, assert_raises_rpc_error, get_rpc_proxy, rpc_port, rpc_url @@ -17,6 +15,10 @@ def set_test_params(self): self.num_nodes = 1 self.supports_cli = False + def skip_test_if_missing_module(self): + # due to OS-specific network stats queries, this test works only on Linux + self.skip_if_platform_not_linux() + def setup_network(self): self.add_nodes(self.num_nodes, None) @@ -61,14 +63,9 @@ def run_allowip_test(self, allow_ips, rpchost, rpcport): self.stop_nodes() def run_test(self): - # due to OS-specific network stats queries, this test works only on Linux if sum([self.options.run_ipv4, self.options.run_ipv6, self.options.run_nonloopback]) > 1: raise AssertionError("Only one of --ipv4, --ipv6 and --nonloopback can be set") - self.log.info("Check for linux") - if not sys.platform.startswith('linux'): - raise SkipTest("This test can only be run on linux.") - self.log.info("Check for ipv6") have_ipv6 = test_ipv6_local() if not have_ipv6 and not (self.options.run_ipv4 or self.options.run_nonloopback): diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index a890b59c7186..2cb93b37c780 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -24,6 +24,7 @@ from collections import defaultdict from io import BytesIO import logging +import platform import struct import sys import threading @@ -786,7 +787,7 @@ def __init__(self): NetworkThread.listeners = {} NetworkThread.protos = {} - if sys.platform == 'win32': + if platform.system() == 'Windows': asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy()) NetworkThread.network_event_loop = asyncio.new_event_loop() diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index cbe42cd1de88..dad1a802a2b9 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -12,13 +12,13 @@ import json import logging import os.path +import platform import re import subprocess import tempfile import time import urllib.parse import shlex -import sys import collections from pathlib import Path @@ -573,7 +573,7 @@ def test_success(cmd): cmd, shell=True, stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL) == 0 - if not sys.platform.startswith('linux'): + if platform.system() != 'Linux': self.log.warning("Can't profile with perf; only available on Linux platforms") return None diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index e303013a6b75..4588cfa3b758 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -17,6 +17,7 @@ import configparser import datetime import os +import platform import time import shutil import signal @@ -42,23 +43,23 @@ CROSS = "x " CIRCLE = "o " -if os.name == 'nt': #type:ignore - import ctypes - kernel32 = ctypes.windll.kernel32 # type: ignore - ENABLE_VIRTUAL_TERMINAL_PROCESSING = 4 - STD_OUTPUT_HANDLE = -11 - STD_ERROR_HANDLE = -12 - # Enable ascii color control to stdout - stdout = kernel32.GetStdHandle(STD_OUTPUT_HANDLE) - stdout_mode = ctypes.c_int32() - kernel32.GetConsoleMode(stdout, ctypes.byref(stdout_mode)) - kernel32.SetConsoleMode(stdout, stdout_mode.value | ENABLE_VIRTUAL_TERMINAL_PROCESSING) - # Enable ascii color control to stderr - stderr = kernel32.GetStdHandle(STD_ERROR_HANDLE) - stderr_mode = ctypes.c_int32() - kernel32.GetConsoleMode(stderr, ctypes.byref(stderr_mode)) - kernel32.SetConsoleMode(stderr, stderr_mode.value | ENABLE_VIRTUAL_TERMINAL_PROCESSING) -else: +if platform.system() != 'Windows' or sys.getwindowsversion() >= (10, 0, 14393): #type:ignore + if platform.system() == 'Windows': + import ctypes + kernel32 = ctypes.windll.kernel32 # type: ignore + ENABLE_VIRTUAL_TERMINAL_PROCESSING = 4 + STD_OUTPUT_HANDLE = -11 + STD_ERROR_HANDLE = -12 + # Enable ascii color control to stdout + stdout = kernel32.GetStdHandle(STD_OUTPUT_HANDLE) + stdout_mode = ctypes.c_int32() + kernel32.GetConsoleMode(stdout, ctypes.byref(stdout_mode)) + kernel32.SetConsoleMode(stdout, stdout_mode.value | ENABLE_VIRTUAL_TERMINAL_PROCESSING) + # Enable ascii color control to stderr + stderr = kernel32.GetStdHandle(STD_ERROR_HANDLE) + stderr_mode = ctypes.c_int32() + kernel32.GetConsoleMode(stderr, ctypes.byref(stderr_mode)) + kernel32.SetConsoleMode(stderr, stderr_mode.value | ENABLE_VIRTUAL_TERMINAL_PROCESSING) # primitive formatting on supported # terminal via ANSI escape sequences: DEFAULT = ('\033[0m', '\033[0m') diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index b610347496e2..423cd9efb22c 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -9,9 +9,9 @@ from threading import Thread from decimal import Decimal import os +import platform import shutil import stat -import sys import time from test_framework.authproxy import JSONRPCException @@ -143,7 +143,7 @@ def wallet_file(name): # should raise rpc error if wallet path can't be created err_code = -4 if self.options.descriptors else -1 - assert_raises_rpc_error(err_code, "filesystem error:" if sys.platform != 'win32' else "create_directories:", self.nodes[0].createwallet, "w8/bad") + assert_raises_rpc_error(err_code, "filesystem error:" if platform.system() != 'Windows' else "create_directories:", self.nodes[0].createwallet, "w8/bad") # check that all requested wallets were created self.stop_node(0)