diff options
-rwxr-xr-x | ci/test/00_setup_env_i686_multiprocess.sh | 1 | ||||
-rw-r--r-- | depends/builders/default.mk | 3 | ||||
-rw-r--r-- | depends/builders/openbsd.mk | 2 | ||||
-rw-r--r-- | depends/funcs.mk | 2 | ||||
-rw-r--r-- | src/Makefile.bench.include | 1 | ||||
-rw-r--r-- | src/addrdb.cpp | 8 | ||||
-rw-r--r-- | src/bench/parse_hex.cpp | 36 | ||||
-rw-r--r-- | src/common/args.cpp | 12 | ||||
-rw-r--r-- | src/common/args.h | 5 | ||||
-rw-r--r-- | src/init.cpp | 18 | ||||
-rw-r--r-- | src/net.cpp | 7 | ||||
-rw-r--r-- | src/util/strencodings.cpp | 2 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 2 | ||||
-rwxr-xr-x | test/functional/feature_addrman.py | 7 | ||||
-rwxr-xr-x | test/functional/feature_asmap.py | 12 | ||||
-rwxr-xr-x | test/functional/p2p_node_network_limited.py | 13 | ||||
-rwxr-xr-x | test/functional/rpc_net.py | 179 | ||||
-rwxr-xr-x | test/functional/test_framework/test_framework.py | 14 | ||||
-rwxr-xr-x | test/functional/test_framework/test_node.py | 4 | ||||
-rwxr-xr-x | test/functional/test_runner.py | 28 | ||||
-rwxr-xr-x | test/functional/wallet_migration.py | 11 |
21 files changed, 231 insertions, 136 deletions
diff --git a/ci/test/00_setup_env_i686_multiprocess.sh b/ci/test/00_setup_env_i686_multiprocess.sh index 0d18e2f029..a5f3682c36 100755 --- a/ci/test/00_setup_env_i686_multiprocess.sh +++ b/ci/test/00_setup_env_i686_multiprocess.sh @@ -12,6 +12,7 @@ export CI_IMAGE_NAME_TAG="docker.io/amd64/ubuntu:22.04" export PACKAGES="llvm clang g++-multilib" export DEP_OPTS="DEBUG=1 MULTIPROCESS=1" export GOAL="install" +export TEST_RUNNER_EXTRA="--v2transport" export BITCOIN_CONFIG="--enable-debug CC='clang -m32' CXX='clang++ -m32' \ CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'" export BITCOIND=bitcoin-node # Used in functional tests diff --git a/depends/builders/default.mk b/depends/builders/default.mk index cc6dec66c2..806d6e7c50 100644 --- a/depends/builders/default.mk +++ b/depends/builders/default.mk @@ -5,13 +5,14 @@ default_build_TAR = tar default_build_RANLIB = ranlib default_build_STRIP = strip default_build_NM = nm +default_build_TOUCH = touch -h -m -t 200001011200 define add_build_tool_func build_$(build_os)_$1 ?= $$(default_build_$1) build_$(build_arch)_$(build_os)_$1 ?= $$(build_$(build_os)_$1) build_$1=$$(build_$(build_arch)_$(build_os)_$1) endef -$(foreach var,CC CXX AR TAR RANLIB NM STRIP SHA256SUM DOWNLOAD OTOOL INSTALL_NAME_TOOL DSYMUTIL,$(eval $(call add_build_tool_func,$(var)))) +$(foreach var,CC CXX AR TAR RANLIB NM STRIP SHA256SUM DOWNLOAD OTOOL INSTALL_NAME_TOOL DSYMUTIL TOUCH,$(eval $(call add_build_tool_func,$(var)))) define add_build_flags_func build_$(build_arch)_$(build_os)_$1 += $(build_$(build_os)_$1) build_$1=$$(build_$(build_arch)_$(build_os)_$1) diff --git a/depends/builders/openbsd.mk b/depends/builders/openbsd.mk index 44825d106a..9c94c4baae 100644 --- a/depends/builders/openbsd.mk +++ b/depends/builders/openbsd.mk @@ -5,3 +5,5 @@ build_openbsd_SHA256SUM = sha256 build_openbsd_DOWNLOAD = curl --location --fail --connect-timeout $(DOWNLOAD_CONNECT_TIMEOUT) --retry $(DOWNLOAD_RETRIES) -o build_openbsd_TAR = gtar +# openBSD touch doesn't understand -h +build_openbsd_TOUCH = touch -m -t 200001011200 diff --git a/depends/funcs.mk b/depends/funcs.mk index 24c911b0f7..7b5c3d0c59 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -234,7 +234,7 @@ $($(1)_postprocessed): | $($(1)_staged) $($(1)_cached): | $($(1)_dependencies) $($(1)_postprocessed) echo Caching $(1)... cd $$($(1)_staging_dir)/$(host_prefix); \ - find . ! -name '.stamp_postprocessed' -print0 | TZ=UTC xargs -0r touch -h -m -t 200001011200; \ + find . ! -name '.stamp_postprocessed' -print0 | TZ=UTC xargs -0r $(build_TOUCH); \ find . ! -name '.stamp_postprocessed' | LC_ALL=C sort | $(build_TAR) --numeric-owner --no-recursion -czf $$($(1)_staging_dir)/$$(@F) -T - mkdir -p $$(@D) rm -rf $$(@D) && mkdir -p $$(@D) diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index b24405ce19..4d814bc5dc 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -42,6 +42,7 @@ bench_bench_bitcoin_SOURCES = \ bench/merkle_root.cpp \ bench/nanobench.cpp \ bench/nanobench.h \ + bench/parse_hex.cpp \ bench/peer_eviction.cpp \ bench/poly1305.cpp \ bench/pool.cpp \ diff --git a/src/addrdb.cpp b/src/addrdb.cpp index c41c9c9086..f8d4240f3f 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -193,7 +193,9 @@ void ReadFromStream(AddrMan& addr, DataStream& ssPeers) util::Result<std::unique_ptr<AddrMan>> LoadAddrman(const NetGroupManager& netgroupman, const ArgsManager& args) { auto check_addrman = std::clamp<int32_t>(args.GetIntArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000); - auto addrman{std::make_unique<AddrMan>(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman)}; + bool deterministic = HasTestOption(args, "addrman"); // use a deterministic addrman only for tests + + auto addrman{std::make_unique<AddrMan>(netgroupman, /*deterministic=*/deterministic, /*consistency_check_ratio=*/check_addrman)}; const auto start{SteadyClock::now()}; const auto path_addr{args.GetDataDirNet() / "peers.dat"}; @@ -202,7 +204,7 @@ util::Result<std::unique_ptr<AddrMan>> LoadAddrman(const NetGroupManager& netgro LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman->Size(), Ticks<std::chrono::milliseconds>(SteadyClock::now() - start)); } catch (const DbNotFoundError&) { // Addrman can be in an inconsistent state after failure, reset it - addrman = std::make_unique<AddrMan>(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); + addrman = std::make_unique<AddrMan>(netgroupman, /*deterministic=*/deterministic, /*consistency_check_ratio=*/check_addrman); LogPrintf("Creating peers.dat because the file was not found (%s)\n", fs::quoted(fs::PathToString(path_addr))); DumpPeerAddresses(args, *addrman); } catch (const InvalidAddrManVersionError&) { @@ -210,7 +212,7 @@ util::Result<std::unique_ptr<AddrMan>> LoadAddrman(const NetGroupManager& netgro return util::Error{strprintf(_("Failed to rename invalid peers.dat file. Please move or delete it and try again."))}; } // Addrman can be in an inconsistent state after failure, reset it - addrman = std::make_unique<AddrMan>(netgroupman, /*deterministic=*/false, /*consistency_check_ratio=*/check_addrman); + addrman = std::make_unique<AddrMan>(netgroupman, /*deterministic=*/deterministic, /*consistency_check_ratio=*/check_addrman); LogPrintf("Creating new peers.dat because the file version was not compatible (%s). Original backed up to peers.dat.bak\n", fs::quoted(fs::PathToString(path_addr))); DumpPeerAddresses(args, *addrman); } catch (const std::exception& e) { diff --git a/src/bench/parse_hex.cpp b/src/bench/parse_hex.cpp new file mode 100644 index 0000000000..db3ead043c --- /dev/null +++ b/src/bench/parse_hex.cpp @@ -0,0 +1,36 @@ +// Copyright (c) 2024- The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <bench/bench.h> +#include <random.h> +#include <stddef.h> +#include <util/strencodings.h> +#include <cassert> +#include <optional> +#include <vector> + +std::string generateHexString(size_t length) { + const auto hex_digits = "0123456789ABCDEF"; + FastRandomContext rng(/*fDeterministic=*/true); + + std::string data; + while (data.size() < length) { + auto digit = hex_digits[rng.randbits(4)]; + data.push_back(digit); + } + return data; +} + +static void HexParse(benchmark::Bench& bench) +{ + auto data = generateHexString(130); // Generates 678B0EDA0A1FD30904D5A65E3568DB82DB2D918B0AD8DEA18A63FECCB877D07CAD1495C7157584D877420EF38B8DA473A6348B4F51811AC13C786B962BEE5668F9 by default + + bench.batch(data.size()).unit("base16").run([&] { + auto result = TryParseHex(data); + assert(result != std::nullopt); // make sure we're measuring the successful case + ankerl::nanobench::doNotOptimizeAway(result); + }); +} + +BENCHMARK(HexParse, benchmark::PriorityLevel::HIGH); diff --git a/src/common/args.cpp b/src/common/args.cpp index a9108e5916..c90eb0c685 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -682,6 +682,18 @@ std::string HelpMessageOpt(const std::string &option, const std::string &message std::string("\n\n"); } +const std::vector<std::string> TEST_OPTIONS_DOC{ + "addrman (use deterministic addrman)", +}; + +bool HasTestOption(const ArgsManager& args, const std::string& test_option) +{ + const auto options = args.GetArgs("-test"); + return std::any_of(options.begin(), options.end(), [test_option](const auto& option) { + return option == test_option; + }); +} + fs::path GetDefaultDataDir() { // Windows: C:\Users\Username\AppData\Roaming\Bitcoin diff --git a/src/common/args.h b/src/common/args.h index 6451b194d1..78a61313b9 100644 --- a/src/common/args.h +++ b/src/common/args.h @@ -447,6 +447,11 @@ bool HelpRequested(const ArgsManager& args); /** Add help options to the args manager */ void SetupHelpOptions(ArgsManager& args); +extern const std::vector<std::string> TEST_OPTIONS_DOC; + +/** Checks if a particular test option is present in -test command-line arg options */ +bool HasTestOption(const ArgsManager& args, const std::string& test_option); + /** * Format a string to be used as group of options in help messages * diff --git a/src/init.cpp b/src/init.cpp index 9ea7b881cb..b9a0bb732a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -614,7 +614,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-limitancestorsize=<n>", strprintf("Do not accept transactions whose size with all in-mempool ancestors exceeds <n> kilobytes (default: %u)", DEFAULT_ANCESTOR_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-limitdescendantcount=<n>", strprintf("Do not accept transactions if any ancestor would have <n> or more in-mempool descendants (default: %u)", DEFAULT_DESCENDANT_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-limitdescendantsize=<n>", strprintf("Do not accept transactions if any ancestor would have more than <n> kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); - argsman.AddArg("-addrmantest", "Allows to test address relay on localhost", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); + argsman.AddArg("-test=<option>", "Pass a test-only option. Options include : " + Join(TEST_OPTIONS_DOC, ", ") + ".", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-capturemessages", "Capture all P2P messages to disk", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-mocktime=<n>", "Replace actual time with " + UNIX_EPOCH_TIME + " (default: 0)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); argsman.AddArg("-maxsigcachesize=<n>", strprintf("Limit sum of signature cache and script execution cache sizes to <n> MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_BYTES >> 20), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST); @@ -1028,6 +1028,22 @@ bool AppInitParameterInteraction(const ArgsManager& args) if (args.GetBoolArg("-peerbloomfilters", DEFAULT_PEERBLOOMFILTERS)) nLocalServices = ServiceFlags(nLocalServices | NODE_BLOOM); + if (args.IsArgSet("-test")) { + if (chainparams.GetChainType() != ChainType::REGTEST) { + return InitError(Untranslated("-test=<option> should only be used in functional tests")); + } + const std::vector<std::string> options = args.GetArgs("-test"); + for (const std::string& option : options) { + auto it = std::find_if(TEST_OPTIONS_DOC.begin(), TEST_OPTIONS_DOC.end(), [&option](const std::string& doc_option) { + size_t pos = doc_option.find(" ("); + return (pos != std::string::npos) && (doc_option.substr(0, pos) == option); + }); + if (it == TEST_OPTIONS_DOC.end()) { + InitWarning(strprintf(_("Unrecognised option \"%s\" provided in -test=<option>."), option)); + } + } + } + // Also report errors from parsing before daemonization { kernel::Notifications notifications{}; diff --git a/src/net.cpp b/src/net.cpp index 6701e5cd4e..e8d8f370dd 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -238,10 +238,6 @@ static int GetnScore(const CService& addr) std::optional<CService> GetLocalAddrForPeer(CNode& node) { CService addrLocal{GetLocalAddress(node)}; - if (gArgs.GetBoolArg("-addrmantest", false)) { - // use IPv4 loopback during addrmantest - addrLocal = CService(LookupNumeric("127.0.0.1", GetListenPort())); - } // If discovery is enabled, sometimes give our peer the address it // tells us that it sees us as in case it has a better idea of our // address than we do. @@ -261,8 +257,7 @@ std::optional<CService> GetLocalAddrForPeer(CNode& node) addrLocal.SetIP(node.GetAddrLocal()); } } - if (addrLocal.IsRoutable() || gArgs.GetBoolArg("-addrmantest", false)) - { + if (addrLocal.IsRoutable()) { LogPrint(BCLog::NET, "Advertising address %s to peer=%d\n", addrLocal.ToStringAddrPort(), node.GetId()); return addrLocal; } diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index a54f408496..b51b283a69 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -81,6 +81,8 @@ template <typename Byte> std::optional<std::vector<Byte>> TryParseHex(std::string_view str) { std::vector<Byte> vch; + vch.reserve(str.size() / 2); // two hex characters form a single byte + auto it = str.begin(); while (it != str.end()) { if (IsSpace(*it)) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3ac09430d8..9c15c2a827 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4336,7 +4336,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle // Make a backup of the DB fs::path this_wallet_dir = fs::absolute(fs::PathFromString(local_wallet->GetDatabase().Filename())).parent_path(); - fs::path backup_filename = fs::PathFromString(strprintf("%s-%d.legacy.bak", wallet_name, GetTime())); + fs::path backup_filename = fs::PathFromString(strprintf("%s_%d.legacy.bak", (wallet_name.empty() ? "default_wallet" : wallet_name), GetTime())); fs::path backup_path = this_wallet_dir / backup_filename; if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) { if (was_loaded) { diff --git a/test/functional/feature_addrman.py b/test/functional/feature_addrman.py index a7ce864fde..95d33d62ea 100755 --- a/test/functional/feature_addrman.py +++ b/test/functional/feature_addrman.py @@ -156,12 +156,7 @@ class AddrmanTest(BitcoinTestFramework): ) self.log.info("Check that missing addrman is recreated") - self.stop_node(0) - os.remove(peers_dat) - with self.nodes[0].assert_debug_log([ - f'Creating peers.dat because the file was not found ("{peers_dat}")', - ]): - self.start_node(0) + self.restart_node(0, clear_addrman=True) assert_equal(self.nodes[0].getnodeaddresses(), []) diff --git a/test/functional/feature_asmap.py b/test/functional/feature_asmap.py index ae483fe449..7e32f8d515 100755 --- a/test/functional/feature_asmap.py +++ b/test/functional/feature_asmap.py @@ -42,8 +42,8 @@ class AsmapTest(BitcoinTestFramework): self.extra_args = [["-checkaddrman=1"]] # Do addrman checks on all operations. def fill_addrman(self, node_id): - """Add 1 tried address to the addrman, followed by 1 new address.""" - for addr, tried in [[0, True], [1, False]]: + """Add 2 tried addresses to the addrman, followed by 2 new addresses.""" + for addr, tried in [[0, True], [1, True], [2, False], [3, False]]: self.nodes[node_id].addpeeraddress(address=f"101.{addr}.0.0", tried=tried, port=8333) def test_without_asmap_arg(self): @@ -84,12 +84,12 @@ class AsmapTest(BitcoinTestFramework): self.log.info("Test bitcoind -asmap restart with addrman containing new and tried entries") self.stop_node(0) shutil.copyfile(self.asmap_raw, self.default_asmap) - self.start_node(0, ["-asmap", "-checkaddrman=1"]) + self.start_node(0, ["-asmap", "-checkaddrman=1", "-test=addrman"]) self.fill_addrman(node_id=0) - self.restart_node(0, ["-asmap", "-checkaddrman=1"]) + self.restart_node(0, ["-asmap", "-checkaddrman=1", "-test=addrman"]) with self.node.assert_debug_log( expected_msgs=[ - "CheckAddrman: new 1, tried 1, total 2 started", + "CheckAddrman: new 2, tried 2, total 4 started", "CheckAddrman: completed", ] ): @@ -114,7 +114,7 @@ class AsmapTest(BitcoinTestFramework): def test_asmap_health_check(self): self.log.info('Test bitcoind -asmap logs ASMap Health Check with basic stats') shutil.copyfile(self.asmap_raw, self.default_asmap) - msg = "ASMap Health Check: 2 clearnet peers are mapped to 1 ASNs with 0 peers being unmapped" + msg = "ASMap Health Check: 4 clearnet peers are mapped to 3 ASNs with 0 peers being unmapped" with self.node.assert_debug_log(expected_msgs=[msg]): self.start_node(0, extra_args=['-asmap']) os.remove(self.default_asmap) diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py index 05d227990e..467bbad09c 100755 --- a/test/functional/p2p_node_network_limited.py +++ b/test/functional/p2p_node_network_limited.py @@ -15,7 +15,6 @@ from test_framework.messages import ( NODE_P2P_V2, NODE_WITNESS, msg_getdata, - msg_verack, ) from test_framework.p2p import P2PInterface from test_framework.test_framework import BitcoinTestFramework @@ -47,7 +46,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 - self.extra_args = [['-prune=550', '-addrmantest'], [], []] + self.extra_args = [['-prune=550'], [], []] def disconnect_all(self): self.disconnect_nodes(0, 1) @@ -139,16 +138,6 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): self.log.info("Requesting block at height 2 (tip-289) must fail (ignored).") node.send_getdata_for_block(blocks[0]) # first block outside of the 288+2 limit node.wait_for_disconnect(5) - - self.log.info("Check local address relay, do a fresh connection.") - self.nodes[0].disconnect_p2ps() - node1 = self.nodes[0].add_p2p_connection(P2PIgnoreInv()) - node1.send_message(msg_verack()) - - node1.wait_for_addr() - #must relay address with NODE_NETWORK_LIMITED - assert_equal(node1.firstAddrnServices, expected_services) - self.nodes[0].disconnect_p2ps() # connect unsynced node 2 with pruned NODE_NETWORK_LIMITED peer diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index accb2439f2..265d9d959a 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -13,7 +13,6 @@ import platform import time import test_framework.messages -from test_framework.netutil import ADDRMAN_NEW_BUCKET_COUNT, ADDRMAN_TRIED_BUCKET_COUNT, ADDRMAN_BUCKET_SIZE from test_framework.p2p import ( P2PInterface, P2P_SERVICES, @@ -42,6 +41,20 @@ def assert_net_servicesnames(servicesflag, servicenames): assert servicesflag_generated == servicesflag +def seed_addrman(node): + """ Populate the addrman with addresses from different networks. + Here 2 ipv4, 2 ipv6, 1 cjdns, 2 onion and 1 i2p addresses are added. + """ + node.addpeeraddress(address="1.2.3.4", tried=True, port=8333) + node.addpeeraddress(address="2.0.0.0", port=8333) + node.addpeeraddress(address="1233:3432:2434:2343:3234:2345:6546:4534", tried=True, port=8333) + node.addpeeraddress(address="2803:0:1234:abcd::1", port=45324) + node.addpeeraddress(address="fc00:1:2:3:4:5:6:7", port=8333) + node.addpeeraddress(address="pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion", tried=True, port=8333) + node.addpeeraddress(address="nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion", port=45324, tried=True) + node.addpeeraddress(address="c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p", port=8333) + + class NetTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 @@ -305,16 +318,8 @@ class NetTest(BitcoinTestFramework): assert_raises_rpc_error(-8, "Network not recognized: Foo", self.nodes[0].getnodeaddresses, 1, "Foo") def test_addpeeraddress(self): - """RPC addpeeraddress sets the source address equal to the destination address. - If an address with the same /16 as an existing new entry is passed, it will be - placed in the same new bucket and have a 1/64 chance of the bucket positions - colliding (depending on the value of nKey in the addrman), in which case the - new address won't be added. The probability of collision can be reduced to - 1/2^16 = 1/65536 by using an address from a different /16. We avoid this here - by first testing adding a tried table entry before testing adding a new table one. - """ self.log.info("Test addpeeraddress") - self.restart_node(1, ["-checkaddrman=1"]) + self.restart_node(1, ["-checkaddrman=1", "-test=addrman"]) node = self.nodes[1] self.log.debug("Test that addpeerinfo is a hidden RPC") @@ -390,25 +395,33 @@ class NetTest(BitcoinTestFramework): def test_getaddrmaninfo(self): self.log.info("Test getaddrmaninfo") + self.restart_node(1, extra_args=["-cjdnsreachable", "-test=addrman"], clear_addrman=True) node = self.nodes[1] + seed_addrman(node) + + expected_network_count = { + 'all_networks': {'new': 4, 'tried': 4, 'total': 8}, + 'ipv4': {'new': 1, 'tried': 1, 'total': 2}, + 'ipv6': {'new': 1, 'tried': 1, 'total': 2}, + 'onion': {'new': 0, 'tried': 2, 'total': 2}, + 'i2p': {'new': 1, 'tried': 0, 'total': 1}, + 'cjdns': {'new': 1, 'tried': 0, 'total': 1}, + } - # current count of ipv4 addresses in addrman is {'new':1, 'tried':1} - self.log.info("Test that count of addresses in addrman match expected values") + self.log.debug("Test that count of addresses in addrman match expected values") res = node.getaddrmaninfo() - assert_equal(res["ipv4"]["new"], 1) - assert_equal(res["ipv4"]["tried"], 1) - assert_equal(res["ipv4"]["total"], 2) - assert_equal(res["all_networks"]["new"], 1) - assert_equal(res["all_networks"]["tried"], 1) - assert_equal(res["all_networks"]["total"], 2) - for net in ["ipv6", "onion", "i2p", "cjdns"]: - assert_equal(res[net]["new"], 0) - assert_equal(res[net]["tried"], 0) - assert_equal(res[net]["total"], 0) + for network, count in expected_network_count.items(): + assert_equal(res[network]['new'], count['new']) + assert_equal(res[network]['tried'], count['tried']) + assert_equal(res[network]['total'], count['total']) def test_getrawaddrman(self): self.log.info("Test getrawaddrman") + self.restart_node(1, extra_args=["-cjdnsreachable", "-test=addrman"], clear_addrman=True) node = self.nodes[1] + self.addr_time = int(time.time()) + node.setmocktime(self.addr_time) + seed_addrman(node) self.log.debug("Test that getrawaddrman is a hidden RPC") # It is hidden from general help, but its detailed help may be called directly. @@ -430,88 +443,96 @@ class NetTest(BitcoinTestFramework): getrawaddrman = node.getrawaddrman() getaddrmaninfo = node.getaddrmaninfo() for (table_name, table_info) in expected.items(): - assert_equal(len(getrawaddrman[table_name]), len(table_info["entries"])) + assert_equal(len(getrawaddrman[table_name]), len(table_info)) assert_equal(len(getrawaddrman[table_name]), getaddrmaninfo["all_networks"][table_name]) for bucket_position in getrawaddrman[table_name].keys(): - bucket = int(bucket_position.split("/")[0]) - position = int(bucket_position.split("/")[1]) - - # bucket and position only be sanity checked here as the - # test-addrman isn't deterministic - assert 0 <= int(bucket) < table_info["bucket_count"] - assert 0 <= int(position) < ADDRMAN_BUCKET_SIZE - entry = getrawaddrman[table_name][bucket_position] - expected_entry = list(filter(lambda e: e["address"] == entry["address"], table_info["entries"]))[0] + expected_entry = list(filter(lambda e: e["address"] == entry["address"], table_info))[0] + assert bucket_position == expected_entry["bucket_position"] check_addr_information(entry, expected_entry) - # we expect one addrman new and tried table entry, which were added in a previous test + # we expect 4 new and 4 tried table entries in the addrman which were added using seed_addrman() expected = { - "new": { - "bucket_count": ADDRMAN_NEW_BUCKET_COUNT, - "entries": [ + "new": [ { + "bucket_position": "82/8", "address": "2.0.0.0", "port": 8333, "services": 9, "network": "ipv4", "source": "2.0.0.0", "source_network": "ipv4", + }, + { + "bucket_position": "336/24", + "address": "fc00:1:2:3:4:5:6:7", + "port": 8333, + "services": 9, + "network": "cjdns", + "source": "fc00:1:2:3:4:5:6:7", + "source_network": "cjdns", + }, + { + "bucket_position": "963/46", + "address": "c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p", + "port": 8333, + "services": 9, + "network": "i2p", + "source": "c4gfnttsuwqomiygupdqqqyy5y5emnk5c73hrfvatri67prd7vyq.b32.i2p", + "source_network": "i2p", + }, + { + "bucket_position": "613/6", + "address": "2803:0:1234:abcd::1", + "services": 9, + "network": "ipv6", + "source": "2803:0:1234:abcd::1", + "source_network": "ipv6", + "port": 45324, } - ] - }, - "tried": { - "bucket_count": ADDRMAN_TRIED_BUCKET_COUNT, - "entries": [ + ], + "tried": [ { + "bucket_position": "6/33", "address": "1.2.3.4", "port": 8333, "services": 9, "network": "ipv4", "source": "1.2.3.4", "source_network": "ipv4", + }, + { + "bucket_position": "197/34", + "address": "1233:3432:2434:2343:3234:2345:6546:4534", + "port": 8333, + "services": 9, + "network": "ipv6", + "source": "1233:3432:2434:2343:3234:2345:6546:4534", + "source_network": "ipv6", + }, + { + "bucket_position": "72/61", + "address": "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion", + "port": 8333, + "services": 9, + "network": "onion", + "source": "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion", + "source_network": "onion" + }, + { + "bucket_position": "139/46", + "address": "nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion", + "services": 9, + "network": "onion", + "source": "nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion", + "source_network": "onion", + "port": 45324, } - ] - } + ] } - self.log.debug("Test that the getrawaddrman contains information about the addresses added in a previous test") - check_getrawaddrman_entries(expected) - - self.log.debug("Add one new address to each addrman table") - expected["new"]["entries"].append({ - "address": "2803:0:1234:abcd::1", - "services": 9, - "network": "ipv6", - "source": "2803:0:1234:abcd::1", - "source_network": "ipv6", - "port": -1, # set once addpeeraddress is successful - }) - expected["tried"]["entries"].append({ - "address": "nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion", - "services": 9, - "network": "onion", - "source": "nrfj6inpyf73gpkyool35hcmne5zwfmse3jl3aw23vk7chdemalyaqad.onion", - "source_network": "onion", - "port": -1, # set once addpeeraddress is successful - }) - - port = 0 - for (table_name, table_info) in expected.items(): - # There's a slight chance that the to-be-added address collides with an already - # present table entry. To avoid this, we increment the port until an address has been - # added. Incrementing the port changes the position in the new table bucket (bucket - # stays the same) and changes both the bucket and the position in the tried table. - while True: - if node.addpeeraddress(address=table_info["entries"][1]["address"], port=port, tried=table_name == "tried")["success"]: - table_info["entries"][1]["port"] = port - self.log.debug(f"Added {table_info['entries'][1]['address']} to {table_name} table") - break - else: - port += 1 - - self.log.debug("Test that the newly added addresses appear in getrawaddrman") + self.log.debug("Test that getrawaddrman contains information about newly added addresses in each addrman table") check_getrawaddrman_entries(expected) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index d8ae20981d..3072f000d0 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -191,6 +191,8 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): parser.add_argument("--timeout-factor", dest="timeout_factor", type=float, help="adjust test timeouts by a factor. Setting it to 0 disables all timeouts") parser.add_argument("--v2transport", dest="v2transport", default=False, action="store_true", help="use BIP324 v2 connections between all nodes by default") + parser.add_argument("--v1transport", dest="v1transport", default=False, action="store_true", + help="Explicitly use v1 transport (can be used to overwrite global --v2transport option)") self.add_options(parser) # Running TestShell in a Jupyter notebook causes an additional -f argument @@ -206,6 +208,8 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): config = configparser.ConfigParser() config.read_file(open(self.options.configfile)) self.config = config + if self.options.v1transport: + self.options.v2transport=False if "descriptors" not in self.options: # Wallet is not required by the test at all and the value of self.options.descriptors won't matter. @@ -577,10 +581,16 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # Wait for nodes to stop node.wait_until_stopped() - def restart_node(self, i, extra_args=None): + def restart_node(self, i, extra_args=None, clear_addrman=False): """Stop and start a test node""" self.stop_node(i) - self.start_node(i, extra_args) + if clear_addrman: + peers_dat = self.nodes[i].chain_path / "peers.dat" + os.remove(peers_dat) + with self.nodes[i].assert_debug_log(expected_msgs=[f'Creating peers.dat because the file was not found ("{peers_dat}")']): + self.start_node(i, extra_args) + else: + self.start_node(i, extra_args) def wait_for_node_exit(self, i, timeout): self.nodes[i].process.wait(timeout) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 3baa78fd79..ef6b9dd5ea 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -136,9 +136,7 @@ class TestNode(): self.args.append("-v2transport=1") else: self.args.append("-v2transport=0") - else: - # v2transport requested but not supported for node - assert not v2transport + # if v2transport is requested via global flag but not supported for node version, ignore it self.cli = TestNodeCLI(bitcoin_cli, self.datadir_path) self.use_cli = use_cli diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 4d66ea97c8..59931bc208 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -120,7 +120,7 @@ BASE_SCRIPTS = [ 'wallet_backup.py --legacy-wallet', 'wallet_backup.py --descriptors', 'feature_segwit.py --legacy-wallet', - 'feature_segwit.py --descriptors', + 'feature_segwit.py --descriptors --v1transport', 'feature_segwit.py --descriptors --v2transport', 'p2p_tx_download.py', 'wallet_avoidreuse.py --legacy-wallet', @@ -156,7 +156,7 @@ BASE_SCRIPTS = [ # vv Tests less than 30s vv 'p2p_invalid_messages.py', 'rpc_createmultisig.py', - 'p2p_timeouts.py', + 'p2p_timeouts.py --v1transport', 'p2p_timeouts.py --v2transport', 'wallet_dump.py --legacy-wallet', 'rpc_signer.py', @@ -201,7 +201,7 @@ BASE_SCRIPTS = [ 'mempool_spend_coinbase.py', 'wallet_avoid_mixing_output_types.py --descriptors', 'mempool_reorg.py', - 'p2p_block_sync.py', + 'p2p_block_sync.py --v1transport', 'p2p_block_sync.py --v2transport', 'wallet_createwallet.py --legacy-wallet', 'wallet_createwallet.py --usecli', @@ -230,13 +230,13 @@ BASE_SCRIPTS = [ 'wallet_transactiontime_rescan.py --descriptors', 'wallet_transactiontime_rescan.py --legacy-wallet', 'p2p_addrv2_relay.py', - 'p2p_compactblocks_hb.py', + 'p2p_compactblocks_hb.py --v1transport', 'p2p_compactblocks_hb.py --v2transport', - 'p2p_disconnect_ban.py', + 'p2p_disconnect_ban.py --v1transport', 'p2p_disconnect_ban.py --v2transport', 'feature_posix_fs_permissions.py', 'rpc_decodescript.py', - 'rpc_blockchain.py', + 'rpc_blockchain.py --v1transport', 'rpc_blockchain.py --v2transport', 'rpc_deprecated.py', 'wallet_disable.py', @@ -246,21 +246,21 @@ BASE_SCRIPTS = [ 'p2p_getaddr_caching.py', 'p2p_getdata.py', 'p2p_addrfetch.py', - 'rpc_net.py', + 'rpc_net.py --v1transport', 'rpc_net.py --v2transport', 'wallet_keypool.py --legacy-wallet', 'wallet_keypool.py --descriptors', 'wallet_descriptor.py --descriptors', 'p2p_nobloomfilter_messages.py', 'p2p_filter.py', - 'rpc_setban.py', + 'rpc_setban.py --v1transport', 'rpc_setban.py --v2transport', 'p2p_blocksonly.py', 'mining_prioritisetransaction.py', 'p2p_invalid_locator.py', - 'p2p_invalid_block.py', + 'p2p_invalid_block.py --v1transport', 'p2p_invalid_block.py --v2transport', - 'p2p_invalid_tx.py', + 'p2p_invalid_tx.py --v1transport', 'p2p_invalid_tx.py --v2transport', 'p2p_v2_transport.py', 'p2p_v2_encrypted.py', @@ -286,12 +286,12 @@ BASE_SCRIPTS = [ 'rpc_preciousblock.py', 'wallet_importprunedfunds.py --legacy-wallet', 'wallet_importprunedfunds.py --descriptors', - 'p2p_leak_tx.py', + 'p2p_leak_tx.py --v1transport', 'p2p_leak_tx.py --v2transport', 'p2p_eviction.py', - 'p2p_ibd_stalling.py', + 'p2p_ibd_stalling.py --v1transport', 'p2p_ibd_stalling.py --v2transport', - 'p2p_net_deadlock.py', + 'p2p_net_deadlock.py --v1transport', 'p2p_net_deadlock.py --v2transport', 'wallet_signmessagewithaddress.py', 'rpc_signmessagewithprivkey.py', @@ -381,7 +381,7 @@ BASE_SCRIPTS = [ 'feature_coinstatsindex.py', 'wallet_orphanedreward.py', 'wallet_timelock.py', - 'p2p_node_network_limited.py', + 'p2p_node_network_limited.py --v1transport', 'p2p_node_network_limited.py --v2transport', 'p2p_permissions.py', 'feature_blocksdir.py', diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index f9919716be..890b6a5c1b 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -529,11 +529,20 @@ class WalletMigrationTest(BitcoinTestFramework): self.log.info("Test migration of the wallet named as the empty string") wallet = self.create_legacy_wallet("") - self.migrate_wallet(wallet) + # Set time to verify backup existence later + curr_time = int(time.time()) + wallet.setmocktime(curr_time) + + res = self.migrate_wallet(wallet) info = wallet.getwalletinfo() assert_equal(info["descriptors"], True) assert_equal(info["format"], "sqlite") + # Check backup existence and its non-empty wallet filename + backup_path = self.nodes[0].wallets_path / f'default_wallet_{curr_time}.legacy.bak' + assert backup_path.exists() + assert_equal(str(backup_path), res['backup_path']) + def test_direct_file(self): self.log.info("Test migration of a wallet that is not in a wallet directory") wallet = self.create_legacy_wallet("plainfile") |