diff options
31 files changed, 332 insertions, 193 deletions
diff --git a/ci/test/00_setup_env_native_tidy.sh b/ci/test/00_setup_env_native_tidy.sh index 456166d3be..6b0c708f19 100755 --- a/ci/test/00_setup_env_native_tidy.sh +++ b/ci/test/00_setup_env_native_tidy.sh @@ -8,12 +8,13 @@ export LC_ALL=C.UTF-8 export CI_IMAGE_NAME_TAG="docker.io/ubuntu:23.10" # This version will reach EOL in Jul 2024, and can be replaced by "ubuntu:24.04" (or anything else that ships the wanted clang version). export CONTAINER_NAME=ci_native_tidy -export PACKAGES="clang-16 libclang-16-dev llvm-16-dev libomp-16-dev clang-tidy-16 jq bear cmake libevent-dev libboost-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev systemtap-sdt-dev libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools libqrencode-dev libsqlite3-dev libdb++-dev" +export TIDY_LLVM_V="17" +export PACKAGES="clang-${TIDY_LLVM_V} libclang-${TIDY_LLVM_V}-dev llvm-${TIDY_LLVM_V}-dev libomp-${TIDY_LLVM_V}-dev clang-tidy-${TIDY_LLVM_V} jq bear cmake libevent-dev libboost-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev systemtap-sdt-dev libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools libqrencode-dev libsqlite3-dev libdb++-dev" export NO_DEPENDS=1 export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false export RUN_FUZZ_TESTS=false export RUN_TIDY=true export GOAL="install" -export BITCOIN_CONFIG="CC=clang-16 CXX=clang++-16 --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0 -I/usr/lib/llvm-16/lib/clang/16/include'" +export BITCOIN_CONFIG="CC=clang-${TIDY_LLVM_V} CXX=clang++-${TIDY_LLVM_V} --with-incompatible-bdb --disable-hardening CFLAGS='-O0 -g0' CXXFLAGS='-O0 -g0 -I/usr/lib/llvm-${TIDY_LLVM_V}/lib/clang/${TIDY_LLVM_V}/include'" export CCACHE_MAXSIZE=200M diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh index c4095e4c1a..68b701f3ca 100755 --- a/ci/test/01_base_install.sh +++ b/ci/test/01_base_install.sh @@ -42,7 +42,7 @@ if [ -n "$PIP_PACKAGES" ]; then fi if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then - git clone --depth=1 https://github.com/llvm/llvm-project -b "llvmorg-17.0.0-rc4" /msan/llvm-project + ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b llvmorg-17.0.2 /msan/llvm-project cmake -G Ninja -B /msan/clang_build/ \ -DLLVM_ENABLE_PROJECTS="clang" \ @@ -73,8 +73,9 @@ if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then fi if [[ "${RUN_TIDY}" == "true" ]]; then - git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_16 /include-what-you-use - cmake -B /iwyu-build/ -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-16 -S /include-what-you-use + ${CI_RETRY_EXE} git clone https://github.com/include-what-you-use/include-what-you-use -b master /include-what-you-use + git -C /include-what-you-use checkout a138eaac254e5a472464e31d5ec418fe6e6f1fc7 + cmake -B /iwyu-build/ -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-"${TIDY_LLVM_V}" -S /include-what-you-use make -C /iwyu-build/ install "-j$( nproc )" # Use nproc, because MAKEJOBS is the default in docker image builds fi diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index 89af61b87f..4d5f31b956 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -175,13 +175,13 @@ if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then fi if [ "${RUN_TIDY}" = "true" ]; then - cmake -B /tidy-build -DLLVM_DIR=/usr/lib/llvm-16/cmake -DCMAKE_BUILD_TYPE=Release -S "${BASE_ROOT_DIR}"/contrib/devtools/bitcoin-tidy + cmake -B /tidy-build -DLLVM_DIR=/usr/lib/llvm-"${TIDY_LLVM_V}"/cmake -DCMAKE_BUILD_TYPE=Release -S "${BASE_ROOT_DIR}"/contrib/devtools/bitcoin-tidy cmake --build /tidy-build "$MAKEJOBS" cmake --build /tidy-build --target bitcoin-tidy-tests "$MAKEJOBS" set -eo pipefail cd "${BASE_BUILD_DIR}/bitcoin-$HOST/src/" - ( run-clang-tidy-16 -quiet -load="/tidy-build/libbitcoin-tidy.so" "${MAKEJOBS}" ) | grep -C5 "error" + ( run-clang-tidy-"${TIDY_LLVM_V}" -quiet -load="/tidy-build/libbitcoin-tidy.so" "${MAKEJOBS}" ) | grep -C5 "error" # Filter out files by regex here, because regex may not be # accepted in src/.bear-tidy-config # Filter out: diff --git a/contrib/devtools/README.md b/contrib/devtools/README.md index 8bbf39b67f..56eaeef815 100644 --- a/contrib/devtools/README.md +++ b/contrib/devtools/README.md @@ -83,7 +83,7 @@ A small script to automatically create manpages in ../../doc/man by running the This requires help2man which can be found at: https://www.gnu.org/software/help2man/ With in-tree builds this tool can be run from any directory within the -repostitory. To use this tool with out-of-tree builds set `BUILDDIR`. For +repository. To use this tool with out-of-tree builds set `BUILDDIR`. For example: ```bash diff --git a/src/.clang-tidy b/src/.clang-tidy index 4deb5a85a5..9f16553706 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -8,6 +8,7 @@ modernize-use-default-member-init, modernize-use-noexcept, modernize-use-nullptr, performance-*, +-performance-avoid-endl, -performance-inefficient-string-concatenation, -performance-no-int-to-ptr, -performance-noexcept-move-constructor, diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index ba2334df49..7e62d75583 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1413,8 +1413,16 @@ std::unique_ptr<PubkeyProvider> ParsePubkey(uint32_t key_exp_index, const Span<c return std::make_unique<OriginPubkeyProvider>(key_exp_index, std::move(info), std::move(provider), apostrophe); } -std::unique_ptr<PubkeyProvider> InferPubkey(const CPubKey& pubkey, ParseScriptContext, const SigningProvider& provider) +std::unique_ptr<PubkeyProvider> InferPubkey(const CPubKey& pubkey, ParseScriptContext ctx, const SigningProvider& provider) { + // Key cannot be hybrid + if (!pubkey.IsValidNonHybrid()) { + return nullptr; + } + // Uncompressed is only allowed in TOP and P2SH contexts + if (ctx != ParseScriptContext::TOP && ctx != ParseScriptContext::P2SH && !pubkey.IsCompressed()) { + return nullptr; + } std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, false); KeyOriginInfo info; if (provider.GetKeyOrigin(pubkey.GetID(), info)) { @@ -1491,12 +1499,14 @@ struct KeyParser { if (miniscript::IsTapscript(m_script_ctx) && end - begin == 32) { XOnlyPubKey pubkey; std::copy(begin, end, pubkey.begin()); - m_keys.push_back(InferPubkey(pubkey.GetEvenCorrespondingCPubKey(), ParseContext(), *m_in)); - return key; + if (auto pubkey_provider = InferPubkey(pubkey.GetEvenCorrespondingCPubKey(), ParseContext(), *m_in)) { + m_keys.push_back(std::move(pubkey_provider)); + return key; + } } else if (!miniscript::IsTapscript(m_script_ctx)) { - CPubKey pubkey{begin, end}; - if (pubkey.IsValidNonHybrid()) { - m_keys.push_back(InferPubkey(pubkey, ParseContext(), *m_in)); + CPubKey pubkey(begin, end); + if (auto pubkey_provider = InferPubkey(pubkey, ParseContext(), *m_in)) { + m_keys.push_back(std::move(pubkey_provider)); return key; } } @@ -1512,9 +1522,11 @@ struct KeyParser { CKeyID keyid(hash); CPubKey pubkey; if (m_in->GetPubKey(keyid, pubkey)) { - Key key = m_keys.size(); - m_keys.push_back(InferPubkey(pubkey, ParseContext(), *m_in)); - return key; + if (auto pubkey_provider = InferPubkey(pubkey, ParseContext(), *m_in)) { + Key key = m_keys.size(); + m_keys.push_back(std::move(pubkey_provider)); + return key; + } } return {}; } @@ -1849,8 +1861,8 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo if (txntype == TxoutType::PUBKEY && (ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH || ctx == ParseScriptContext::P2WSH)) { CPubKey pubkey(data[0]); - if (pubkey.IsValidNonHybrid()) { - return std::make_unique<PKDescriptor>(InferPubkey(pubkey, ctx, provider)); + if (auto pubkey_provider = InferPubkey(pubkey, ctx, provider)) { + return std::make_unique<PKDescriptor>(std::move(pubkey_provider)); } } if (txntype == TxoutType::PUBKEYHASH && (ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH || ctx == ParseScriptContext::P2WSH)) { @@ -1858,7 +1870,9 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo CKeyID keyid(hash); CPubKey pubkey; if (provider.GetPubKey(keyid, pubkey)) { - return std::make_unique<PKHDescriptor>(InferPubkey(pubkey, ctx, provider)); + if (auto pubkey_provider = InferPubkey(pubkey, ctx, provider)) { + return std::make_unique<PKHDescriptor>(std::move(pubkey_provider)); + } } } if (txntype == TxoutType::WITNESS_V0_KEYHASH && (ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH)) { @@ -1866,16 +1880,24 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo CKeyID keyid(hash); CPubKey pubkey; if (provider.GetPubKey(keyid, pubkey)) { - return std::make_unique<WPKHDescriptor>(InferPubkey(pubkey, ctx, provider)); + if (auto pubkey_provider = InferPubkey(pubkey, ParseScriptContext::P2WPKH, provider)) { + return std::make_unique<WPKHDescriptor>(std::move(pubkey_provider)); + } } } if (txntype == TxoutType::MULTISIG && (ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH || ctx == ParseScriptContext::P2WSH)) { + bool ok = true; std::vector<std::unique_ptr<PubkeyProvider>> providers; for (size_t i = 1; i + 1 < data.size(); ++i) { CPubKey pubkey(data[i]); - providers.push_back(InferPubkey(pubkey, ctx, provider)); + if (auto pubkey_provider = InferPubkey(pubkey, ctx, provider)) { + providers.push_back(std::move(pubkey_provider)); + } else { + ok = false; + break; + } } - return std::make_unique<MultisigDescriptor>((int)data[0][0], std::move(providers)); + if (ok) return std::make_unique<MultisigDescriptor>((int)data[0][0], std::move(providers)); } if (txntype == TxoutType::SCRIPTHASH && ctx == ParseScriptContext::TOP) { uint160 hash(data[0]); diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 9b48c48ff8..f4f4e39f40 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -384,6 +384,54 @@ void Check(const std::string& prv, const std::string& pub, const std::string& no } } +void CheckInferDescriptor(const std::string& script_hex, const std::string& expected_desc, const std::vector<std::string>& hex_scripts, const std::vector<std::pair<std::string, std::string>>& origin_pubkeys) +{ + std::vector<unsigned char> script_bytes{ParseHex(script_hex)}; + const CScript& script{script_bytes.begin(), script_bytes.end()}; + + FlatSigningProvider provider; + for (const std::string& prov_script_hex : hex_scripts) { + std::vector<unsigned char> prov_script_bytes{ParseHex(prov_script_hex)}; + const CScript& prov_script{prov_script_bytes.begin(), prov_script_bytes.end()}; + provider.scripts.emplace(CScriptID(prov_script), prov_script); + } + for (const auto& [pubkey_hex, origin_str] : origin_pubkeys) { + CPubKey origin_pubkey{ParseHex(pubkey_hex)}; + provider.pubkeys.emplace(origin_pubkey.GetID(), origin_pubkey); + + if (!origin_str.empty()) { + using namespace spanparsing; + KeyOriginInfo info; + Span<const char> origin_sp{origin_str}; + std::vector<Span<const char>> origin_split = Split(origin_sp, "/"); + std::string fpr_str(origin_split[0].begin(), origin_split[0].end()); + auto fpr_bytes = ParseHex(fpr_str); + std::copy(fpr_bytes.begin(), fpr_bytes.end(), info.fingerprint); + for (size_t i = 1; i < origin_split.size(); ++i) { + Span<const char> elem = origin_split[i]; + bool hardened = false; + if (elem.size() > 0) { + const char last = elem[elem.size() - 1]; + if (last == '\'' || last == 'h') { + elem = elem.first(elem.size() - 1); + hardened = true; + } + } + uint32_t p; + assert(ParseUInt32(std::string(elem.begin(), elem.end()), &p)); + info.path.push_back(p | (((uint32_t)hardened) << 31)); + } + + provider.origins.emplace(origin_pubkey.GetID(), std::make_pair(origin_pubkey, info)); + } + } + + std::string checksum{GetDescriptorChecksum(expected_desc)}; + + std::unique_ptr<Descriptor> desc = InferDescriptor(script, provider); + BOOST_CHECK_EQUAL(desc->ToString(), expected_desc + "#" + checksum); +} + } BOOST_FIXTURE_TEST_SUITE(descriptor_tests, BasicTestingSetup) @@ -594,6 +642,19 @@ BOOST_AUTO_TEST_CASE(descriptor_test) // preimages and the sequence, passes with.) Check("tr(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,{and_v(and_v(v:hash256(ae253ca2a54debcac7ecf414f6734f48c56421a08bb59182ff9f39a6fffdb588),v:pk(KykUPmR5967F4URzMUeCv9kNMU9CNRWycrPmx3ZvfkWoQLabbimL)),older(42)),multi_a(2,adf586a32ad4b0674a86022b000348b681b4c97a811f67eefe4a6e066e55080c,KztMyyi1pXUtuZfJSB7JzVdmJMAz7wfGVFoSRUR5CVZxXxULXuGR)})", "tr(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,{and_v(and_v(v:hash256(ae253ca2a54debcac7ecf414f6734f48c56421a08bb59182ff9f39a6fffdb588),v:pk(1c9bc926084382e76da33b5a52d17b1fa153c072aae5fb5228ecc2ccf89d79d5)),older(42)),multi_a(2,adf586a32ad4b0674a86022b000348b681b4c97a811f67eefe4a6e066e55080c,14fa4ad085cdee1e2fc73d491b36a96c192382b1d9a21108eb3533f630364f9f)})", "tr(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,{and_v(and_v(v:hash256(ae253ca2a54debcac7ecf414f6734f48c56421a08bb59182ff9f39a6fffdb588),v:pk(1c9bc926084382e76da33b5a52d17b1fa153c072aae5fb5228ecc2ccf89d79d5)),older(42)),multi_a(2,adf586a32ad4b0674a86022b000348b681b4c97a811f67eefe4a6e066e55080c,14fa4ad085cdee1e2fc73d491b36a96c192382b1d9a21108eb3533f630364f9f)})", MISSING_PRIVKEYS | XONLY_KEYS | SIGNABLE | SIGNABLE_FAILS, {{"51209a3d79db56fbe3ba4d905d827b62e1ed31cd6df1198b8c759d589c0f4efc27bd"}}, OutputType::BECH32M); Check("tr(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,{and_v(and_v(v:hash256(ae253ca2a54debcac7ecf414f6734f48c56421a08bb59182ff9f39a6fffdb588),v:pk(KykUPmR5967F4URzMUeCv9kNMU9CNRWycrPmx3ZvfkWoQLabbimL)),older(42)),multi_a(2,adf586a32ad4b0674a86022b000348b681b4c97a811f67eefe4a6e066e55080c,KztMyyi1pXUtuZfJSB7JzVdmJMAz7wfGVFoSRUR5CVZxXxULXuGR)})", "tr(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,{and_v(and_v(v:hash256(ae253ca2a54debcac7ecf414f6734f48c56421a08bb59182ff9f39a6fffdb588),v:pk(1c9bc926084382e76da33b5a52d17b1fa153c072aae5fb5228ecc2ccf89d79d5)),older(42)),multi_a(2,adf586a32ad4b0674a86022b000348b681b4c97a811f67eefe4a6e066e55080c,14fa4ad085cdee1e2fc73d491b36a96c192382b1d9a21108eb3533f630364f9f)})", "tr(a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,{and_v(and_v(v:hash256(ae253ca2a54debcac7ecf414f6734f48c56421a08bb59182ff9f39a6fffdb588),v:pk(1c9bc926084382e76da33b5a52d17b1fa153c072aae5fb5228ecc2ccf89d79d5)),older(42)),multi_a(2,adf586a32ad4b0674a86022b000348b681b4c97a811f67eefe4a6e066e55080c,14fa4ad085cdee1e2fc73d491b36a96c192382b1d9a21108eb3533f630364f9f)})", MISSING_PRIVKEYS | XONLY_KEYS | SIGNABLE, {{"51209a3d79db56fbe3ba4d905d827b62e1ed31cd6df1198b8c759d589c0f4efc27bd"}}, OutputType::BECH32M, /*op_desc_id=*/{}, {{}}, /*spender_nlocktime=*/0, /*spender_nsequence=*/42, /*preimages=*/{{ParseHex("ae253ca2a54debcac7ecf414f6734f48c56421a08bb59182ff9f39a6fffdb588"), ParseHex("000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f")}}); + + // Basic sh(pkh()) with key origin + CheckInferDescriptor("a9141a31ad23bf49c247dd531a623c2ef57da3c400c587", "sh(pkh([deadbeef/0h/0h/0]03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", {"76a9149a1c78a507689f6f54b847ad1cef1e614ee23f1e88ac"}, {{"03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd", "deadbeef/0h/0h/0"}}); + // p2pk script with hybrid key must infer as raw() + CheckInferDescriptor("41069228de6902abb4f541791f6d7f925b10e2078ccb1298856e5ea5cc5fd667f930eac37a00cc07f9a91ef3c2d17bf7a17db04552ff90ac312a5b8b4caca6c97aa4ac", "raw(41069228de6902abb4f541791f6d7f925b10e2078ccb1298856e5ea5cc5fd667f930eac37a00cc07f9a91ef3c2d17bf7a17db04552ff90ac312a5b8b4caca6c97aa4ac)", {}, {{"069228de6902abb4f541791f6d7f925b10e2078ccb1298856e5ea5cc5fd667f930eac37a00cc07f9a91ef3c2d17bf7a17db04552ff90ac312a5b8b4caca6c97aa4", ""}}); + // p2pkh script with hybrid key must infer as addr() + CheckInferDescriptor("76a91445ff7c2327866472639d507334a9a00119dfd32688ac", "addr(17P7ge56F2QcdHxxRBa2NyzmejFggPwBJ9)", {}, {{"069228de6902abb4f541791f6d7f925b10e2078ccb1298856e5ea5cc5fd667f930eac37a00cc07f9a91ef3c2d17bf7a17db04552ff90ac312a5b8b4caca6c97aa4", ""}}); + // p2wpkh script with uncompressed key must infer as addr() + CheckInferDescriptor("001422e363a523947a110d9a9eb114820de183aca313", "addr(bc1qyt3k8ffrj3apzrv6n6c3fqsduxp6egcnk2r66j)", {}, {{"049228de6902abb4f541791f6d7f925b10e2078ccb1298856e5ea5cc5fd667f930eac37a00cc07f9a91ef3c2d17bf7a17db04552ff90ac312a5b8b4caca6c97aa4", ""}}); + // Infer pkh() from p2pkh with uncompressed key + CheckInferDescriptor("76a914a31725c74421fadc50d35520ab8751ed120af80588ac", "pkh(04c56fe4a92d401bcbf1b3dfbe4ac3dac5602ca155a3681497f02c1b9a733b92d704e2da6ec4162e4846af9236ef4171069ac8b7f8234a8405b6cadd96f34f5a31)", {}, {{"04c56fe4a92d401bcbf1b3dfbe4ac3dac5602ca155a3681497f02c1b9a733b92d704e2da6ec4162e4846af9236ef4171069ac8b7f8234a8405b6cadd96f34f5a31", ""}}); + // Infer pk() from p2pk with uncompressed key + CheckInferDescriptor("4104032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220ac", "pk(04032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220)", {}, {{"04032540df1d3c7070a8ab3a9cdd304dfc7fd1e6541369c53c4c3310b2537d91059afc8b8e7673eb812a32978dabb78c40f2e423f7757dca61d11838c7aeeb5220", ""}}); } BOOST_AUTO_TEST_SUITE_END() diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 6c09a6f5e7..ea93b7ab88 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -36,7 +36,11 @@ Interesting starting states could be loading a snapshot when the current chain t """ from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) + START_HEIGHT = 199 SNAPSHOT_BASE_HEIGHT = 299 @@ -62,6 +66,25 @@ class AssumeutxoTest(BitcoinTestFramework): self.add_nodes(3) self.start_nodes(extra_args=self.extra_args) + def test_invalid_snapshot_scenarios(self, valid_snapshot_path): + self.log.info("Test different scenarios of loading invalid snapshot files") + self.log.info(" - snapshot file refering to a block that is not in the assumeutxo parameters") + with open(valid_snapshot_path, 'rb') as f: + valid_snapshot_contents = f.read() + + # we can only test this with a block that is already known, as otherwise the `loadtxoutset` RPC + # would time out (waiting to see the hash in the headers chain), rather than error immediately + bad_snapshot_height = SNAPSHOT_BASE_HEIGHT - 1 + bad_snapshot_path = valid_snapshot_path + '.mod' + with open(bad_snapshot_path, 'wb') as f: + bad_snapshot_block_hash = self.nodes[0].getblockhash(bad_snapshot_height) + # block hash of the snapshot base is stored right at the start (first 32 bytes) + f.write(bytes.fromhex(bad_snapshot_block_hash)[::-1] + valid_snapshot_contents[32:]) + + expected_log = f"assumeutxo height in snapshot metadata not recognized ({bad_snapshot_height}) - refusing to load snapshot" + with self.nodes[1].assert_debug_log(expected_log): + assert_raises_rpc_error(-32603, "Unable to load UTXO snapshot", self.nodes[1].loadtxoutset, bad_snapshot_path) + def run_test(self): """ Bring up two (disconnected) nodes, mine some new blocks on the first, @@ -120,6 +143,8 @@ class AssumeutxoTest(BitcoinTestFramework): assert_equal(n0.getblockchaininfo()["blocks"], FINAL_HEIGHT) + self.test_invalid_snapshot_scenarios(dump_output['path']) + self.log.info(f"Loading snapshot into second node from {dump_output['path']}") loaded = n1.loadtxoutset(dump_output['path']) assert_equal(loaded['coins_loaded'], SNAPSHOT_BASE_HEIGHT) diff --git a/test/functional/feature_blocksdir.py b/test/functional/feature_blocksdir.py index 76b9277e2f..1a60c13c2c 100755 --- a/test/functional/feature_blocksdir.py +++ b/test/functional/feature_blocksdir.py @@ -5,8 +5,8 @@ """Test the blocksdir option. """ -import os import shutil +from pathlib import Path from test_framework.test_framework import BitcoinTestFramework, initialize_datadir @@ -18,20 +18,20 @@ class BlocksdirTest(BitcoinTestFramework): def run_test(self): self.stop_node(0) - assert os.path.isdir(os.path.join(self.nodes[0].blocks_path)) - assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "blocks")) - shutil.rmtree(self.nodes[0].datadir) + assert self.nodes[0].blocks_path.is_dir() + assert not (self.nodes[0].datadir_path / "blocks").is_dir() + shutil.rmtree(self.nodes[0].datadir_path) initialize_datadir(self.options.tmpdir, 0, self.chain) self.log.info("Starting with nonexistent blocksdir ...") - blocksdir_path = os.path.join(self.options.tmpdir, 'blocksdir') + blocksdir_path = Path(self.options.tmpdir) / 'blocksdir' self.nodes[0].assert_start_raises_init_error([f"-blocksdir={blocksdir_path}"], f'Error: Specified blocks directory "{blocksdir_path}" does not exist.') - os.mkdir(blocksdir_path) + blocksdir_path.mkdir() self.log.info("Starting with existing blocksdir ...") self.start_node(0, [f"-blocksdir={blocksdir_path}"]) self.log.info("mining blocks..") self.generatetoaddress(self.nodes[0], 10, self.nodes[0].get_deterministic_priv_key().address) - assert os.path.isfile(os.path.join(blocksdir_path, self.chain, "blocks", "blk00000.dat")) - assert os.path.isdir(os.path.join(self.nodes[0].blocks_path, "index")) + assert (blocksdir_path / self.chain / "blocks" / "blk00000.dat").is_file() + assert (self.nodes[0].blocks_path / "index").is_dir() if __name__ == '__main__': diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 97ee9538dc..dcea662089 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -5,7 +5,7 @@ """Test various command line arguments and configuration file parameters.""" import os -import pathlib +from pathlib import Path import re import sys import tempfile @@ -39,8 +39,8 @@ class ConfArgsTest(BitcoinTestFramework): extra_args=[f'-conf={bad_conf_file_path}'], expected_msg=conf_in_config_file_err, ) - inc_conf_file_path = os.path.join(self.nodes[0].datadir, 'include.conf') - with open(os.path.join(self.nodes[0].datadir, 'bitcoin.conf'), 'a', encoding='utf-8') as conf: + inc_conf_file_path = self.nodes[0].datadir_path / 'include.conf' + with open(self.nodes[0].datadir_path / 'bitcoin.conf', 'a', encoding='utf-8') as conf: conf.write(f'includeconf={inc_conf_file_path}\n') with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: conf.write('conf=some.conf\n') @@ -97,8 +97,8 @@ class ConfArgsTest(BitcoinTestFramework): conf.write('server=1\nrpcuser=someuser\n[main]\nrpcpassword=some#pass') self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Error reading configuration file: parse error on line 4, using # in rpcpassword can be ambiguous and should be avoided') - inc_conf_file2_path = os.path.join(self.nodes[0].datadir, 'include2.conf') - with open(os.path.join(self.nodes[0].datadir, 'bitcoin.conf'), 'a', encoding='utf-8') as conf: + inc_conf_file2_path = self.nodes[0].datadir_path / 'include2.conf' + with open(self.nodes[0].datadir_path / 'bitcoin.conf', 'a', encoding='utf-8') as conf: conf.write(f'includeconf={inc_conf_file2_path}\n') with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: @@ -123,15 +123,15 @@ class ConfArgsTest(BitcoinTestFramework): # Create a temporary directory that will be treated as the default data # directory by bitcoind. - env, default_datadir = util.get_temp_default_datadir(pathlib.Path(self.options.tmpdir, "test_config_file_log")) + env, default_datadir = util.get_temp_default_datadir(Path(self.options.tmpdir, "test_config_file_log")) default_datadir.mkdir(parents=True) # Write a bitcoin.conf file in the default data directory containing a # datadir= line pointing at the node datadir. node = self.nodes[0] - conf_text = pathlib.Path(node.bitcoinconf).read_text() + conf_text = node.bitcoinconf.read_text() conf_path = default_datadir / "bitcoin.conf" - conf_path.write_text(f"datadir={node.datadir}\n{conf_text}") + conf_path.write_text(f"datadir={node.datadir_path}\n{conf_text}") # Drop the node -datadir= argument during this test, because if it is # specified it would take precedence over the datadir setting in the @@ -218,7 +218,8 @@ class ConfArgsTest(BitcoinTestFramework): def test_seed_peers(self): self.log.info('Test seed peers') - default_data_dir = self.nodes[0].datadir + default_data_dir = self.nodes[0].datadir_path + peer_dat = default_data_dir / 'peers.dat' # Only regtest has no fixed seeds. To avoid connections to random # nodes, regtest is the only network where it is safe to enable # -fixedseeds in tests @@ -229,7 +230,7 @@ class ConfArgsTest(BitcoinTestFramework): # We expect the node will use DNS Seeds, but Regtest mode does not have # any valid DNS seeds. So after 60 seconds, the node should fallback to # fixed seeds - assert not os.path.exists(os.path.join(default_data_dir, "peers.dat")) + assert not peer_dat.exists() start = int(time.time()) with self.nodes[0].assert_debug_log( expected_msgs=[ @@ -248,7 +249,7 @@ class ConfArgsTest(BitcoinTestFramework): # No peers.dat exists and -dnsseed=0 # We expect the node will fallback immediately to fixed seeds - assert not os.path.exists(os.path.join(default_data_dir, "peers.dat")) + assert not peer_dat.exists() with self.nodes[0].assert_debug_log(expected_msgs=[ "Loaded 0 addresses from peers.dat", "DNS seeding disabled", @@ -260,7 +261,7 @@ class ConfArgsTest(BitcoinTestFramework): # No peers.dat exists and dns seeds are disabled. # We expect the node will not add fixed seeds when explicitly disabled. - assert not os.path.exists(os.path.join(default_data_dir, "peers.dat")) + assert not peer_dat.exists() with self.nodes[0].assert_debug_log(expected_msgs=[ "Loaded 0 addresses from peers.dat", "DNS seeding disabled", @@ -271,7 +272,7 @@ class ConfArgsTest(BitcoinTestFramework): # No peers.dat exists and -dnsseed=0, but a -addnode is provided # We expect the node will allow 60 seconds prior to using fixed seeds - assert not os.path.exists(os.path.join(default_data_dir, "peers.dat")) + assert not peer_dat.exists() start = int(time.time()) with self.nodes[0].assert_debug_log( expected_msgs=[ @@ -323,16 +324,16 @@ class ConfArgsTest(BitcoinTestFramework): 'because a conflicting -conf file argument is passed.') node = self.nodes[0] with tempfile.NamedTemporaryFile(dir=self.options.tmpdir, mode="wt", delete=False) as temp_conf: - temp_conf.write(f"datadir={node.datadir}\n") + temp_conf.write(f"datadir={node.datadir_path}\n") node.assert_start_raises_init_error([f"-conf={temp_conf.name}"], re.escape( - f'Error: Data directory "{node.datadir}" contains a "bitcoin.conf" file which is ignored, because a ' + f'Error: Data directory "{node.datadir_path}" contains a "bitcoin.conf" file which is ignored, because a ' f'different configuration file "{temp_conf.name}" from command line argument "-conf={temp_conf.name}" ' f'is being used instead.') + r"[\s\S]*", match=ErrorMatch.FULL_REGEX) # Test that passing a redundant -conf command line argument pointing to # the same bitcoin.conf that would be loaded anyway does not trigger an # error. - self.start_node(0, [f'-conf={node.datadir}/bitcoin.conf']) + self.start_node(0, [f'-conf={node.datadir_path}/bitcoin.conf']) self.stop_node(0) def test_ignored_default_conf(self): @@ -346,7 +347,7 @@ class ConfArgsTest(BitcoinTestFramework): # Create a temporary directory that will be treated as the default data # directory by bitcoind. - env, default_datadir = util.get_temp_default_datadir(pathlib.Path(self.options.tmpdir, "home")) + env, default_datadir = util.get_temp_default_datadir(Path(self.options.tmpdir, "home")) default_datadir.mkdir(parents=True) # Write a bitcoin.conf file in the default data directory containing a @@ -354,7 +355,7 @@ class ConfArgsTest(BitcoinTestFramework): # startup error because the node datadir contains a different # bitcoin.conf that would be ignored. node = self.nodes[0] - (default_datadir / "bitcoin.conf").write_text(f"datadir={node.datadir}\n") + (default_datadir / "bitcoin.conf").write_text(f"datadir={node.datadir_path}\n") # Drop the node -datadir= argument during this test, because if it is # specified it would take precedence over the datadir setting in the @@ -362,7 +363,7 @@ class ConfArgsTest(BitcoinTestFramework): node_args = node.args node.args = [arg for arg in node.args if not arg.startswith("-datadir=")] node.assert_start_raises_init_error([], re.escape( - f'Error: Data directory "{node.datadir}" contains a "bitcoin.conf" file which is ignored, because a ' + f'Error: Data directory "{node.datadir_path}" contains a "bitcoin.conf" file which is ignored, because a ' f'different configuration file "{default_datadir}/bitcoin.conf" from data directory "{default_datadir}" ' f'is being used instead.') + r"[\s\S]*", env=env, match=ErrorMatch.FULL_REGEX) node.args = node_args @@ -392,16 +393,16 @@ class ConfArgsTest(BitcoinTestFramework): # Remove the -datadir argument so it doesn't override the config file self.nodes[0].args = [arg for arg in self.nodes[0].args if not arg.startswith("-datadir")] - default_data_dir = self.nodes[0].datadir - new_data_dir = os.path.join(default_data_dir, 'newdatadir') - new_data_dir_2 = os.path.join(default_data_dir, 'newdatadir2') + default_data_dir = self.nodes[0].datadir_path + new_data_dir = default_data_dir / 'newdatadir' + new_data_dir_2 = default_data_dir / 'newdatadir2' # Check that using -datadir argument on non-existent directory fails - self.nodes[0].datadir = new_data_dir + self.nodes[0].datadir_path = new_data_dir self.nodes[0].assert_start_raises_init_error([f'-datadir={new_data_dir}'], f'Error: Specified data directory "{new_data_dir}" does not exist.') # Check that using non-existent datadir in conf file fails - conf_file = os.path.join(default_data_dir, "bitcoin.conf") + conf_file = default_data_dir / "bitcoin.conf" # datadir needs to be set before [chain] section with open(conf_file, encoding='utf8') as f: @@ -413,20 +414,20 @@ class ConfArgsTest(BitcoinTestFramework): self.nodes[0].assert_start_raises_init_error([f'-conf={conf_file}'], f'Error: Error reading configuration file: specified data directory "{new_data_dir}" does not exist.') # Check that an explicitly specified config file that cannot be opened fails - none_existent_conf_file = os.path.join(default_data_dir, "none_existent_bitcoin.conf") - self.nodes[0].assert_start_raises_init_error(['-conf=' + none_existent_conf_file], 'Error: Error reading configuration file: specified config file "' + none_existent_conf_file + '" could not be opened.') + none_existent_conf_file = default_data_dir / "none_existent_bitcoin.conf" + self.nodes[0].assert_start_raises_init_error(['-conf=' + f'{none_existent_conf_file}'], 'Error: Error reading configuration file: specified config file "' + f'{none_existent_conf_file}' + '" could not be opened.') # Create the directory and ensure the config file now works - os.mkdir(new_data_dir) + new_data_dir.mkdir() self.start_node(0, [f'-conf={conf_file}']) self.stop_node(0) - assert os.path.exists(os.path.join(new_data_dir, self.chain, 'blocks')) + assert (new_data_dir / self.chain / 'blocks').exists() # Ensure command line argument overrides datadir in conf - os.mkdir(new_data_dir_2) - self.nodes[0].datadir = new_data_dir_2 + new_data_dir_2.mkdir() + self.nodes[0].datadir_path = new_data_dir_2 self.start_node(0, [f'-datadir={new_data_dir_2}', f'-conf={conf_file}']) - assert os.path.exists(os.path.join(new_data_dir_2, self.chain, 'blocks')) + assert (new_data_dir_2 / self.chain / 'blocks').exists() if __name__ == '__main__': diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py index cf2f21d553..24a68a04bf 100755 --- a/test/functional/feature_filelock.py +++ b/test/functional/feature_filelock.py @@ -28,7 +28,7 @@ class FilelockTest(BitcoinTestFramework): self.log.info("Check that we can't start a second bitcoind instance using the same datadir") expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['PACKAGE_NAME']} is probably already running." - self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir}', '-noserver'], expected_msg=expected_msg) + self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir_path}', '-noserver'], expected_msg=expected_msg) if self.is_wallet_compiled(): def check_wallet_filelock(descriptors): diff --git a/test/functional/feature_loadblock.py b/test/functional/feature_loadblock.py index 12d65fde68..5129e0d328 100755 --- a/test/functional/feature_loadblock.py +++ b/test/functional/feature_loadblock.py @@ -10,7 +10,7 @@ To generate that file this test uses the helper scripts available in contrib/linearize. """ -import os +from pathlib import Path import subprocess import sys import tempfile @@ -32,10 +32,10 @@ class LoadblockTest(BitcoinTestFramework): self.generate(self.nodes[0], COINBASE_MATURITY, sync_fun=self.no_op) # Parsing the url of our node to get settings for config file - data_dir = self.nodes[0].datadir + data_dir = self.nodes[0].datadir_path node_url = urllib.parse.urlparse(self.nodes[0].url) - cfg_file = os.path.join(data_dir, "linearize.cfg") - bootstrap_file = os.path.join(self.options.tmpdir, "bootstrap.dat") + cfg_file = data_dir / "linearize.cfg" + bootstrap_file = Path(self.options.tmpdir) / "bootstrap.dat" genesis_block = self.nodes[0].getblockhash(0) blocks_dir = self.nodes[0].blocks_path hash_list = tempfile.NamedTemporaryFile(dir=data_dir, @@ -58,16 +58,16 @@ class LoadblockTest(BitcoinTestFramework): cfg.write(f"hashlist={hash_list.name}\n") base_dir = self.config["environment"]["SRCDIR"] - linearize_dir = os.path.join(base_dir, "contrib", "linearize") + linearize_dir = Path(base_dir) / "contrib" / "linearize" self.log.info("Run linearization of block hashes") - linearize_hashes_file = os.path.join(linearize_dir, "linearize-hashes.py") + linearize_hashes_file = linearize_dir / "linearize-hashes.py" subprocess.run([sys.executable, linearize_hashes_file, cfg_file], stdout=hash_list, check=True) self.log.info("Run linearization of block data") - linearize_data_file = os.path.join(linearize_dir, "linearize-data.py") + linearize_data_file = linearize_dir / "linearize-data.py" subprocess.run([sys.executable, linearize_data_file, cfg_file], check=True) diff --git a/test/functional/feature_settings.py b/test/functional/feature_settings.py index bcae963428..1bacdd447a 100755 --- a/test/functional/feature_settings.py +++ b/test/functional/feature_settings.py @@ -6,7 +6,6 @@ import json -from pathlib import Path from test_framework.test_framework import BitcoinTestFramework from test_framework.test_node import ErrorMatch @@ -21,8 +20,8 @@ class SettingsTest(BitcoinTestFramework): def run_test(self): node, = self.nodes - settings = Path(node.chain_path, "settings.json") - conf = Path(node.datadir, "bitcoin.conf") + settings = node.chain_path / "settings.json" + conf = node.datadir_path / "bitcoin.conf" # Assert empty settings file was created self.stop_node(0) @@ -79,7 +78,7 @@ class SettingsTest(BitcoinTestFramework): self.stop_node(0) # Test alternate settings path - altsettings = Path(node.datadir, "altsettings.json") + altsettings = node.datadir_path / "altsettings.json" with altsettings.open("w") as fp: fp.write('{"key": "value"}') with node.assert_debug_log(expected_msgs=['Setting file arg: key = "value"']): diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 25ea557217..83bb5121e5 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -94,7 +94,7 @@ class TestBitcoinCli(BitcoinTestFramework): assert_equal(self.nodes[0].cli("-named", "echo", "arg0=0", "arg1=1", "arg2=2", "arg1=3").send_cli(), ['0', '3', '2']) assert_raises_rpc_error(-8, "Parameter args specified multiple times", self.nodes[0].cli("-named", "echo", "args=[0,1,2,3]", "4", "5", "6", ).send_cli) - user, password = get_auth_cookie(self.nodes[0].datadir, self.chain) + user, password = get_auth_cookie(self.nodes[0].datadir_path, self.chain) self.log.info("Test -stdinrpcpass option") assert_equal(BLOCKS, self.nodes[0].cli(f'-rpcuser={user}', '-stdinrpcpass', input=password).getblockcount()) diff --git a/test/functional/rpc_bind.py b/test/functional/rpc_bind.py index 664a15a5ec..43cd23fc7a 100755 --- a/test/functional/rpc_bind.py +++ b/test/functional/rpc_bind.py @@ -56,7 +56,7 @@ class RPCBindTest(BitcoinTestFramework): self.nodes[0].rpchost = None self.start_nodes([node_args]) # connect to node through non-loopback interface - node = get_rpc_proxy(rpc_url(self.nodes[0].datadir, 0, self.chain, "%s:%d" % (rpchost, rpcport)), 0, coveragedir=self.options.coveragedir) + node = get_rpc_proxy(rpc_url(self.nodes[0].datadir_path, 0, self.chain, "%s:%d" % (rpchost, rpcport)), 0, coveragedir=self.options.coveragedir) node.getnetworkinfo() self.stop_nodes() diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py index 2cae602cc2..f378878181 100755 --- a/test/functional/rpc_dumptxoutset.py +++ b/test/functional/rpc_dumptxoutset.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the generation of UTXO snapshots using `dumptxoutset`. """ -from pathlib import Path from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework @@ -29,7 +28,7 @@ class DumptxoutsetTest(BitcoinTestFramework): FILENAME = 'txoutset.dat' out = node.dumptxoutset(FILENAME) - expected_path = Path(node.datadir) / self.chain / FILENAME + expected_path = node.datadir_path / self.chain / FILENAME assert expected_path.is_file() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index c46c04c0ec..50444b95bb 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1006,5 +1006,5 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): return self.config["components"].getboolean("USE_BDB") def has_blockfile(self, node, filenum: str): - blocksdir = os.path.join(node.datadir, self.chain, 'blocks', '') - return os.path.isfile(os.path.join(blocksdir, f"blk{filenum}.dat")) + blocksdir = node.datadir_path / self.chain / 'blocks' + return (blocksdir / f"blk{filenum}.dat").is_file() diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 33a7539641..fc92aa445a 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -67,7 +67,7 @@ class TestNode(): To make things easier for the test writer, any unrecognised messages will be dispatched to the RPC connection.""" - def __init__(self, i, datadir, *, chain, rpchost, timewait, timeout_factor, bitcoind, bitcoin_cli, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, descriptors=False): + def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor, bitcoind, bitcoin_cli, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, descriptors=False): """ Kwargs: start_perf (bool): If True, begin profiling the node with `perf` as soon as @@ -76,10 +76,10 @@ class TestNode(): self.index = i self.p2p_conn_index = 1 - self.datadir = datadir - self.bitcoinconf = os.path.join(self.datadir, "bitcoin.conf") - self.stdout_dir = os.path.join(self.datadir, "stdout") - self.stderr_dir = os.path.join(self.datadir, "stderr") + self.datadir_path = datadir_path + self.bitcoinconf = self.datadir_path / "bitcoin.conf" + self.stdout_dir = self.datadir_path / "stdout" + self.stderr_dir = self.datadir_path / "stderr" self.chain = chain self.rpchost = rpchost self.rpc_timeout = timewait @@ -88,7 +88,7 @@ class TestNode(): self.cwd = cwd self.descriptors = descriptors if extra_conf is not None: - append_config(datadir, extra_conf) + append_config(self.datadir_path, extra_conf) # Most callers will just need to add extra args to the standard list below. # For those callers that need more flexibility, they can just set the args property directly. # Note that common args are set in the config file (see initialize_datadir) @@ -99,7 +99,7 @@ class TestNode(): # spam debug.log. self.args = [ self.binary, - "-datadir=" + self.datadir, + f"-datadir={self.datadir_path}", "-logtimemicros", "-debug", "-debugexclude=libevent", @@ -111,9 +111,7 @@ class TestNode(): self.args.append("-disablewallet") if use_valgrind: - default_suppressions_file = os.path.join( - os.path.dirname(os.path.realpath(__file__)), - "..", "..", "..", "contrib", "valgrind.supp") + default_suppressions_file = Path(__file__).parents[3] / "contrib" / "valgrind.supp" suppressions_file = os.getenv("VALGRIND_SUPPRESSIONS_FILE", default_suppressions_file) self.args = ["valgrind", "--suppressions={}".format(suppressions_file), @@ -127,7 +125,7 @@ class TestNode(): if self.version_is_at_least(239000): self.args.append("-loglevel=trace") - self.cli = TestNodeCLI(bitcoin_cli, self.datadir) + self.cli = TestNodeCLI(bitcoin_cli, self.datadir_path) self.use_cli = use_cli self.start_perf = start_perf @@ -213,7 +211,7 @@ class TestNode(): # Delete any existing cookie file -- if such a file exists (eg due to # unclean shutdown), it will get overwritten anyway by bitcoind, and # potentially interfere with our attempt to authenticate - delete_cookie_file(self.datadir, self.chain) + delete_cookie_file(self.datadir_path, self.chain) # add environment variable LIBC_FATAL_STDERR_=1 so that libc errors are written to stderr and not the terminal subp_env = dict(os.environ, LIBC_FATAL_STDERR_="1") @@ -243,7 +241,7 @@ class TestNode(): f'bitcoind exited with status {self.process.returncode} during initialization. {str_error}')) try: rpc = get_rpc_proxy( - rpc_url(self.datadir, self.index, self.chain, self.rpchost), + rpc_url(self.datadir_path, self.index, self.chain, self.rpchost), self.index, timeout=self.rpc_timeout // 2, # Shorter timeout to allow for one retry in case of ETIMEDOUT coveragedir=self.coverage_dir, @@ -307,7 +305,7 @@ class TestNode(): poll_per_s = 4 for _ in range(poll_per_s * self.rpc_timeout): try: - get_auth_cookie(self.datadir, self.chain) + get_auth_cookie(self.datadir_path, self.chain) self.log.debug("Cookie credentials successfully retrieved") return except ValueError: # cookie file not found and no rpcuser or rpcpassword; bitcoind is still starting @@ -425,10 +423,6 @@ class TestNode(): conf.write(conf_data) @property - def datadir_path(self) -> Path: - return Path(self.datadir) - - @property def chain_path(self) -> Path: return self.datadir_path / self.chain @@ -556,7 +550,7 @@ class TestNode(): "perf output won't be very useful without debug symbols compiled into bitcoind") output_path = tempfile.NamedTemporaryFile( - dir=self.datadir, + dir=self.datadir_path, prefix="{}.perf.data.".format(profile_name or 'test'), delete=False, ).name @@ -783,7 +777,7 @@ class TestNodeCLI(): """Run bitcoin-cli command. Deserializes returned string as python object.""" pos_args = [arg_to_cli(arg) for arg in args] named_args = [str(key) + "=" + arg_to_cli(value) for (key, value) in kwargs.items()] - p_args = [self.binary, "-datadir=" + self.datadir] + self.options + p_args = [self.binary, f"-datadir={self.datadir}"] + self.options if named_args: p_args += ["-named"] if command is not None: diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 0c10d500af..61346e9d19 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -413,7 +413,7 @@ def write_config(config_path, *, n, chain, extra_config="", disable_autoconnect= def get_datadir_path(dirname, n): - return os.path.join(dirname, "node" + str(n)) + return pathlib.Path(dirname) / f"node{n}" def get_temp_default_datadir(temp_dir: pathlib.Path) -> Tuple[dict, pathlib.Path]: diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index 8b0c8ce405..fc042bca66 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -32,7 +32,7 @@ class ToolWalletTest(BitcoinTestFramework): self.skip_if_no_wallet_tool() def bitcoin_wallet_process(self, *args): - default_args = ['-datadir={}'.format(self.nodes[0].datadir), '-chain=%s' % self.chain] + default_args = ['-datadir={}'.format(self.nodes[0].datadir_path), '-chain=%s' % self.chain] if not self.options.descriptors and 'create' in args: default_args.append('-legacy') @@ -153,8 +153,8 @@ class ToolWalletTest(BitcoinTestFramework): assert_equal(v, r[k]) def do_tool_createfromdump(self, wallet_name, dumpfile, file_format=None): - dumppath = os.path.join(self.nodes[0].datadir, dumpfile) - rt_dumppath = os.path.join(self.nodes[0].datadir, "rt-{}.dump".format(wallet_name)) + dumppath = self.nodes[0].datadir_path / dumpfile + rt_dumppath = self.nodes[0].datadir_path / "rt-{}.dump".format(wallet_name) dump_data = self.read_dump(dumppath) @@ -324,7 +324,7 @@ class ToolWalletTest(BitcoinTestFramework): self.assert_raises_tool_error('No dump file provided. To use dump, -dumpfile=<filename> must be provided.', '-wallet=todump', 'dump') self.log.info('Checking basic dump') - wallet_dump = os.path.join(self.nodes[0].datadir, "wallet.dump") + wallet_dump = self.nodes[0].datadir_path / "wallet.dump" self.assert_tool_output('The dumpfile may contain private keys. To ensure the safety of your Bitcoin, do not share the dumpfile.\n', '-wallet=todump', '-dumpfile={}'.format(wallet_dump), 'dump') dump_data = self.read_dump(wallet_dump) @@ -339,7 +339,7 @@ class ToolWalletTest(BitcoinTestFramework): self.log.info('Checking createfromdump arguments') self.assert_raises_tool_error('No dump file provided. To use createfromdump, -dumpfile=<filename> must be provided.', '-wallet=todump', 'createfromdump') - non_exist_dump = os.path.join(self.nodes[0].datadir, "wallet.nodump") + non_exist_dump = self.nodes[0].datadir_path / "wallet.nodump" self.assert_raises_tool_error('Unknown wallet file format "notaformat" provided. Please provide one of "bdb" or "sqlite".', '-wallet=todump', '-format=notaformat', '-dumpfile={}'.format(wallet_dump), 'createfromdump') self.assert_raises_tool_error('Dump file {} does not exist.'.format(non_exist_dump), '-wallet=todump', '-dumpfile={}'.format(non_exist_dump), 'createfromdump') wallet_path = self.nodes[0].wallets_path / "todump2" @@ -354,17 +354,17 @@ class ToolWalletTest(BitcoinTestFramework): self.do_tool_createfromdump("load-sqlite", "wallet.dump", "sqlite") self.log.info('Checking createfromdump handling of magic and versions') - bad_ver_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_ver1.dump") + bad_ver_wallet_dump = self.nodes[0].datadir_path / "wallet-bad_ver1.dump" dump_data["BITCOIN_CORE_WALLET_DUMP"] = "0" self.write_dump(dump_data, bad_ver_wallet_dump) self.assert_raises_tool_error('Error: Dumpfile version is not supported. This version of bitcoin-wallet only supports version 1 dumpfiles. Got dumpfile with version 0', '-wallet=badload', '-dumpfile={}'.format(bad_ver_wallet_dump), 'createfromdump') assert not (self.nodes[0].wallets_path / "badload").is_dir() - bad_ver_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_ver2.dump") + bad_ver_wallet_dump = self.nodes[0].datadir_path / "wallet-bad_ver2.dump" dump_data["BITCOIN_CORE_WALLET_DUMP"] = "2" self.write_dump(dump_data, bad_ver_wallet_dump) self.assert_raises_tool_error('Error: Dumpfile version is not supported. This version of bitcoin-wallet only supports version 1 dumpfiles. Got dumpfile with version 2', '-wallet=badload', '-dumpfile={}'.format(bad_ver_wallet_dump), 'createfromdump') assert not (self.nodes[0].wallets_path / "badload").is_dir() - bad_magic_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_magic.dump") + bad_magic_wallet_dump = self.nodes[0].datadir_path / "wallet-bad_magic.dump" del dump_data["BITCOIN_CORE_WALLET_DUMP"] dump_data["not_the_right_magic"] = "1" self.write_dump(dump_data, bad_magic_wallet_dump, "not_the_right_magic") @@ -372,19 +372,19 @@ class ToolWalletTest(BitcoinTestFramework): assert not (self.nodes[0].wallets_path / "badload").is_dir() self.log.info('Checking createfromdump handling of checksums') - bad_sum_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_sum1.dump") + bad_sum_wallet_dump = self.nodes[0].datadir_path / "wallet-bad_sum1.dump" dump_data = orig_dump.copy() checksum = dump_data["checksum"] dump_data["checksum"] = "1" * 64 self.write_dump(dump_data, bad_sum_wallet_dump) self.assert_raises_tool_error('Error: Dumpfile checksum does not match. Computed {}, expected {}'.format(checksum, "1" * 64), '-wallet=bad', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump') assert not (self.nodes[0].wallets_path / "badload").is_dir() - bad_sum_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_sum2.dump") + bad_sum_wallet_dump = self.nodes[0].datadir_path / "wallet-bad_sum2.dump" del dump_data["checksum"] self.write_dump(dump_data, bad_sum_wallet_dump, skip_checksum=True) self.assert_raises_tool_error('Error: Missing checksum', '-wallet=badload', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump') assert not (self.nodes[0].wallets_path / "badload").is_dir() - bad_sum_wallet_dump = os.path.join(self.nodes[0].datadir, "wallet-bad_sum3.dump") + bad_sum_wallet_dump = self.nodes[0].datadir_path / "wallet-bad_sum3.dump" dump_data["checksum"] = "2" * 10 self.write_dump(dump_data, bad_sum_wallet_dump) self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=badload', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump') @@ -452,7 +452,7 @@ class ToolWalletTest(BitcoinTestFramework): self.assert_tool_output(expected_output, "-wallet=conflicts", "info") def run_test(self): - self.wallet_path = os.path.join(self.nodes[0].wallets_path, self.default_wallet_name, self.wallet_data_filename) + self.wallet_path = self.nodes[0].wallets_path / self.default_wallet_name / self.wallet_data_filename self.test_invalid_tool_commands_and_args() # Warning: The following tests are order-dependent. self.test_tool_wallet_info() diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index 9f6f54c7a6..eb3e0ae728 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -109,36 +109,35 @@ class WalletBackupTest(BitcoinTestFramework): self.stop_node(2) def erase_three(self): - os.remove(os.path.join(self.nodes[0].wallets_path, self.default_wallet_name, self.wallet_data_filename)) - os.remove(os.path.join(self.nodes[1].wallets_path, self.default_wallet_name, self.wallet_data_filename)) - os.remove(os.path.join(self.nodes[2].wallets_path, self.default_wallet_name, self.wallet_data_filename)) + for node_num in range(3): + (self.nodes[node_num].wallets_path / self.default_wallet_name / self.wallet_data_filename).unlink() def restore_invalid_wallet(self): node = self.nodes[3] - invalid_wallet_file = os.path.join(self.nodes[0].datadir, 'invalid_wallet_file.bak') + invalid_wallet_file = self.nodes[0].datadir_path / 'invalid_wallet_file.bak' open(invalid_wallet_file, 'a', encoding="utf8").write('invald wallet') wallet_name = "res0" - not_created_wallet_file = os.path.join(node.wallets_path, wallet_name) + not_created_wallet_file = node.wallets_path / wallet_name error_message = "Wallet file verification failed. Failed to load database path '{}'. Data is not in recognized format.".format(not_created_wallet_file) assert_raises_rpc_error(-18, error_message, node.restorewallet, wallet_name, invalid_wallet_file) - assert not os.path.exists(not_created_wallet_file) + assert not not_created_wallet_file.exists() def restore_nonexistent_wallet(self): node = self.nodes[3] - nonexistent_wallet_file = os.path.join(self.nodes[0].datadir, 'nonexistent_wallet.bak') + nonexistent_wallet_file = self.nodes[0].datadir_path / 'nonexistent_wallet.bak' wallet_name = "res0" assert_raises_rpc_error(-8, "Backup file does not exist", node.restorewallet, wallet_name, nonexistent_wallet_file) - not_created_wallet_file = os.path.join(node.wallets_path, wallet_name) - assert not os.path.exists(not_created_wallet_file) + not_created_wallet_file = node.wallets_path / wallet_name + assert not not_created_wallet_file.exists() def restore_wallet_existent_name(self): node = self.nodes[3] - backup_file = os.path.join(self.nodes[0].datadir, 'wallet.bak') + backup_file = self.nodes[0].datadir_path / 'wallet.bak' wallet_name = "res0" - wallet_file = os.path.join(node.wallets_path, wallet_name) + wallet_file = node.wallets_path / wallet_name error_message = "Failed to create database path '{}'. Database already exists.".format(wallet_file) assert_raises_rpc_error(-36, error_message, node.restorewallet, wallet_name, backup_file) - assert os.path.exists(wallet_file) + assert wallet_file.exists() def run_test(self): self.log.info("Generating initial blockchain") @@ -159,14 +158,12 @@ class WalletBackupTest(BitcoinTestFramework): self.log.info("Backing up") - self.nodes[0].backupwallet(os.path.join(self.nodes[0].datadir, 'wallet.bak')) - self.nodes[1].backupwallet(os.path.join(self.nodes[1].datadir, 'wallet.bak')) - self.nodes[2].backupwallet(os.path.join(self.nodes[2].datadir, 'wallet.bak')) + for node_num in range(3): + self.nodes[node_num].backupwallet(self.nodes[node_num].datadir_path / 'wallet.bak') if not self.options.descriptors: - self.nodes[0].dumpwallet(os.path.join(self.nodes[0].datadir, 'wallet.dump')) - self.nodes[1].dumpwallet(os.path.join(self.nodes[1].datadir, 'wallet.dump')) - self.nodes[2].dumpwallet(os.path.join(self.nodes[2].datadir, 'wallet.dump')) + for node_num in range(3): + self.nodes[node_num].dumpwallet(self.nodes[node_num].datadir_path / 'wallet.dump') self.log.info("More transactions") for _ in range(5): @@ -193,17 +190,13 @@ class WalletBackupTest(BitcoinTestFramework): self.restore_invalid_wallet() self.restore_nonexistent_wallet() - backup_file_0 = os.path.join(self.nodes[0].datadir, 'wallet.bak') - backup_file_1 = os.path.join(self.nodes[1].datadir, 'wallet.bak') - backup_file_2 = os.path.join(self.nodes[2].datadir, 'wallet.bak') - - self.nodes[3].restorewallet("res0", backup_file_0) - self.nodes[3].restorewallet("res1", backup_file_1) - self.nodes[3].restorewallet("res2", backup_file_2) + backup_files = [] + for node_num in range(3): + backup_files.append(self.nodes[node_num].datadir_path / 'wallet.bak') - assert os.path.exists(os.path.join(self.nodes[3].wallets_path, "res0")) - assert os.path.exists(os.path.join(self.nodes[3].wallets_path, "res1")) - assert os.path.exists(os.path.join(self.nodes[3].wallets_path, "res2")) + for idx, backup_file in enumerate(backup_files): + self.nodes[3].restorewallet(f'res{idx}', backup_file) + assert (self.nodes[3].wallets_path / f'res{idx}').exists() res0_rpc = self.nodes[3].get_wallet_rpc("res0") res1_rpc = self.nodes[3].get_wallet_rpc("res1") @@ -221,22 +214,16 @@ class WalletBackupTest(BitcoinTestFramework): self.erase_three() #start node2 with no chain - shutil.rmtree(os.path.join(self.nodes[2].blocks_path)) - shutil.rmtree(os.path.join(self.nodes[2].chain_path, 'chainstate')) + shutil.rmtree(self.nodes[2].blocks_path) + shutil.rmtree(self.nodes[2].chain_path / 'chainstate') self.start_three(["-nowallet"]) # Create new wallets for the three nodes. # We will use this empty wallets to test the 'importwallet()' RPC command below. for node_num in range(3): self.nodes[node_num].createwallet(wallet_name=self.default_wallet_name, descriptors=self.options.descriptors, load_on_startup=True) - - assert_equal(self.nodes[0].getbalance(), 0) - assert_equal(self.nodes[1].getbalance(), 0) - assert_equal(self.nodes[2].getbalance(), 0) - - self.nodes[0].importwallet(os.path.join(self.nodes[0].datadir, 'wallet.dump')) - self.nodes[1].importwallet(os.path.join(self.nodes[1].datadir, 'wallet.dump')) - self.nodes[2].importwallet(os.path.join(self.nodes[2].datadir, 'wallet.dump')) + assert_equal(self.nodes[node_num].getbalance(), 0) + self.nodes[node_num].importwallet(self.nodes[node_num].datadir_path / 'wallet.dump') self.sync_blocks() diff --git a/test/functional/wallet_blank.py b/test/functional/wallet_blank.py index 4836eba3b2..e646d27005 100755 --- a/test/functional/wallet_blank.py +++ b/test/functional/wallet_blank.py @@ -3,7 +3,6 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or https://www.opensource.org/licenses/mit-license.php. -import os from test_framework.test_framework import BitcoinTestFramework from test_framework.address import ( @@ -110,7 +109,7 @@ class WalletBlankTest(BitcoinTestFramework): assert_equal(info["descriptors"], False) assert_equal(info["blank"], True) - wallet_dump_path = os.path.join(self.nodes[0].datadir, "wallet.dump") + wallet_dump_path = self.nodes[0].datadir_path / "wallet.dump" def_wallet.dumpwallet(wallet_dump_path) wallet.importwallet(wallet_dump_path) diff --git a/test/functional/wallet_crosschain.py b/test/functional/wallet_crosschain.py index 7a1297e65f..4c5d7192ae 100755 --- a/test/functional/wallet_crosschain.py +++ b/test/functional/wallet_crosschain.py @@ -3,8 +3,6 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -import os - from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_raises_rpc_error @@ -31,13 +29,13 @@ class WalletCrossChain(BitcoinTestFramework): def run_test(self): self.log.info("Creating wallets") - node0_wallet = os.path.join(self.nodes[0].datadir, 'node0_wallet') - node0_wallet_backup = os.path.join(self.nodes[0].datadir, 'node0_wallet.bak') + node0_wallet = self.nodes[0].datadir_path / 'node0_wallet' + node0_wallet_backup = self.nodes[0].datadir_path / 'node0_wallet.bak' self.nodes[0].createwallet(node0_wallet) self.nodes[0].backupwallet(node0_wallet_backup) self.nodes[0].unloadwallet(node0_wallet) - node1_wallet = os.path.join(self.nodes[1].datadir, 'node1_wallet') - node1_wallet_backup = os.path.join(self.nodes[0].datadir, 'node1_wallet.bak') + node1_wallet = self.nodes[1].datadir_path / 'node1_wallet' + node1_wallet_backup = self.nodes[0].datadir_path / 'node1_wallet.bak' self.nodes[1].createwallet(node1_wallet) self.nodes[1].backupwallet(node1_wallet_backup) self.nodes[1].unloadwallet(node1_wallet) diff --git a/test/functional/wallet_descriptor.py b/test/functional/wallet_descriptor.py index 6af01f8cfd..c220675eb6 100755 --- a/test/functional/wallet_descriptor.py +++ b/test/functional/wallet_descriptor.py @@ -3,7 +3,6 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test descriptor wallet function.""" -import os try: import sqlite3 @@ -234,7 +233,7 @@ class WalletDescriptorTest(BitcoinTestFramework): self.log.info("Test that loading descriptor wallet containing legacy key types throws error") self.nodes[0].createwallet(wallet_name="crashme", descriptors=True) self.nodes[0].unloadwallet("crashme") - wallet_db = os.path.join(self.nodes[0].wallets_path, "crashme", self.wallet_data_filename) + wallet_db = self.nodes[0].wallets_path / "crashme" / self.wallet_data_filename conn = sqlite3.connect(wallet_db) with conn: # add "cscript" entry: key type is uint160 (20 bytes), value type is CScript (zero-length here) diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index 8c68d03f97..fdce9739eb 100755 --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the dumpwallet RPC.""" import datetime -import os import time from test_framework.test_framework import BitcoinTestFramework @@ -111,8 +110,8 @@ class WalletDumpTest(BitcoinTestFramework): def run_test(self): self.nodes[0].createwallet("dump") - wallet_unenc_dump = os.path.join(self.nodes[0].datadir, "wallet.unencrypted.dump") - wallet_enc_dump = os.path.join(self.nodes[0].datadir, "wallet.encrypted.dump") + wallet_unenc_dump = self.nodes[0].datadir_path / "wallet.unencrypted.dump" + wallet_enc_dump = self.nodes[0].datadir_path / "wallet.encrypted.dump" # generate 30 addresses to compare against the dump # - 10 legacy P2PKH @@ -156,7 +155,7 @@ class WalletDumpTest(BitcoinTestFramework): self.log.info('Dump unencrypted wallet') result = self.nodes[0].dumpwallet(wallet_unenc_dump) - assert_equal(result['filename'], wallet_unenc_dump) + assert_equal(result['filename'], str(wallet_unenc_dump)) found_comments, found_legacy_addr, found_p2sh_segwit_addr, found_bech32_addr, found_script_addr, found_addr_chg, found_addr_rsv, hd_master_addr_unenc = \ read_dump(wallet_unenc_dump, addrs, [multisig_addr], None) @@ -220,7 +219,7 @@ class WalletDumpTest(BitcoinTestFramework): w3.sendtoaddress(w3.getnewaddress(), 10) w3.unloadwallet() self.nodes[0].loadwallet("w3") - w3.dumpwallet(os.path.join(self.nodes[0].datadir, "w3.dump")) + w3.dumpwallet(self.nodes[0].datadir_path / "w3.dump") if __name__ == '__main__': WalletDumpTest().main() diff --git a/test/functional/wallet_fast_rescan.py b/test/functional/wallet_fast_rescan.py index 112aa25e86..2f9c924e71 100755 --- a/test/functional/wallet_fast_rescan.py +++ b/test/functional/wallet_fast_rescan.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test that fast rescan using block filters for descriptor wallets detects top-ups correctly and finds the same transactions than the slow variant.""" -import os from typing import List from test_framework.address import address_to_scriptpubkey @@ -43,7 +42,7 @@ class WalletFastRescanTest(BitcoinTestFramework): wallet = MiniWallet(node) self.log.info("Create descriptor wallet with backup") - WALLET_BACKUP_FILENAME = os.path.join(node.datadir, 'wallet.bak') + WALLET_BACKUP_FILENAME = node.datadir_path / 'wallet.bak' node.createwallet(wallet_name='topup_test', descriptors=True) w = node.get_wallet_rpc('topup_test') fixed_key = get_generate_key() diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index 62f8301c16..0f4b7cfcb1 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test Hierarchical Deterministic wallet function.""" -import os import shutil from test_framework.blocktools import COINBASE_MATURITY @@ -51,8 +50,8 @@ class WalletHDTest(BitcoinTestFramework): self.nodes[1].importprivkey(non_hd_key) # This should be enough to keep the master key and the non-HD key - self.nodes[1].backupwallet(os.path.join(self.nodes[1].datadir, "hd.bak")) - #self.nodes[1].dumpwallet(os.path.join(self.nodes[1].datadir, "hd.dump")) + self.nodes[1].backupwallet(self.nodes[1].datadir_path / "hd.bak") + #self.nodes[1].dumpwallet(self.nodes[1].datadir_path / "hd.dump") # Derive some HD addresses and remember the last # Also send funds to each add @@ -87,11 +86,11 @@ class WalletHDTest(BitcoinTestFramework): self.stop_node(1) # we need to delete the complete chain directory # otherwise node1 would auto-recover all funds in flag the keypool keys as used - shutil.rmtree(os.path.join(self.nodes[1].blocks_path)) - shutil.rmtree(os.path.join(self.nodes[1].chain_path, "chainstate")) + shutil.rmtree(self.nodes[1].blocks_path) + shutil.rmtree(self.nodes[1].chain_path / "chainstate") shutil.copyfile( - os.path.join(self.nodes[1].datadir, "hd.bak"), - os.path.join(self.nodes[1].wallets_path, self.default_wallet_name, self.wallet_data_filename), + self.nodes[1].datadir_path / "hd.bak", + self.nodes[1].wallets_path / self.default_wallet_name / self.wallet_data_filename ) self.start_node(1) @@ -115,11 +114,11 @@ class WalletHDTest(BitcoinTestFramework): # Try a RPC based rescan self.stop_node(1) - shutil.rmtree(os.path.join(self.nodes[1].blocks_path)) - shutil.rmtree(os.path.join(self.nodes[1].chain_path, "chainstate")) + shutil.rmtree(self.nodes[1].blocks_path) + shutil.rmtree(self.nodes[1].chain_path / "chainstate") shutil.copyfile( - os.path.join(self.nodes[1].datadir, "hd.bak"), - os.path.join(self.nodes[1].wallets_path, self.default_wallet_name, self.wallet_data_filename), + self.nodes[1].datadir_path / "hd.bak", + self.nodes[1].wallets_path / self.default_wallet_name / self.wallet_data_filename ) self.start_node(1, extra_args=self.extra_args[1]) self.connect_nodes(0, 1) diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py index 0f1c33a0c2..48180e8294 100755 --- a/test/functional/wallet_keypool_topup.py +++ b/test/functional/wallet_keypool_topup.py @@ -10,7 +10,6 @@ Two nodes. Node1 is under test. Node0 is providing transactions and generating b - Generate 110 keys (enough to drain the keypool). Store key 90 (in the initial keypool) and key 110 (beyond the initial keypool). Send funds to key 90 and key 110. - Stop node1, clear the datadir, move wallet file back into the datadir and restart node1. - connect node1 to node0. Verify that they sync and node1 receives its funds.""" -import os import shutil from test_framework.blocktools import COINBASE_MATURITY @@ -33,8 +32,8 @@ class KeypoolRestoreTest(BitcoinTestFramework): self.skip_if_no_wallet() def run_test(self): - wallet_path = os.path.join(self.nodes[1].wallets_path, self.default_wallet_name, self.wallet_data_filename) - wallet_backup_path = os.path.join(self.nodes[1].datadir, "wallet.bak") + wallet_path = self.nodes[1].wallets_path / self.default_wallet_name / self.wallet_data_filename + wallet_backup_path = self.nodes[1].datadir_path / "wallet.bak" self.generate(self.nodes[0], COINBASE_MATURITY + 1) self.log.info("Make backup of wallet") diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index bcd71197bf..286dcb5fda 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -6,8 +6,13 @@ import random import shutil -from test_framework.address import script_to_p2sh +from test_framework.address import ( + script_to_p2sh, + key_to_p2pkh, + key_to_p2wpkh, +) from test_framework.descriptors import descsum_create +from test_framework.key import ECPubKey from test_framework.test_framework import BitcoinTestFramework from test_framework.messages import COIN, CTransaction, CTxOut from test_framework.script_util import key_to_p2pkh_script, script_to_p2sh_script, script_to_p2wsh_script @@ -770,6 +775,58 @@ class WalletMigrationTest(BitcoinTestFramework): wallet.unloadwallet() + def test_hybrid_pubkey(self): + self.log.info("Test migration when wallet contains a hybrid pubkey") + + wallet = self.create_legacy_wallet("hybrid_keys") + + # Get the hybrid pubkey for one of the keys in the wallet + normal_pubkey = wallet.getaddressinfo(wallet.getnewaddress())["pubkey"] + first_byte = bytes.fromhex(normal_pubkey)[0] + 4 # Get the hybrid pubkey first byte + parsed_pubkey = ECPubKey() + parsed_pubkey.set(bytes.fromhex(normal_pubkey)) + parsed_pubkey.compressed = False + hybrid_pubkey_bytes = bytearray(parsed_pubkey.get_bytes()) + hybrid_pubkey_bytes[0] = first_byte # Make it hybrid + hybrid_pubkey = hybrid_pubkey_bytes.hex() + + # Import the hybrid pubkey + wallet.importpubkey(hybrid_pubkey) + p2pkh_addr = key_to_p2pkh(hybrid_pubkey) + p2pkh_addr_info = wallet.getaddressinfo(p2pkh_addr) + assert_equal(p2pkh_addr_info["iswatchonly"], True) + assert_equal(p2pkh_addr_info["ismine"], False) # Things involving hybrid pubkeys are not spendable + + # Also import the p2wpkh for the pubkey to make sure we don't migrate it + p2wpkh_addr = key_to_p2wpkh(hybrid_pubkey) + wallet.importaddress(p2wpkh_addr) + + migrate_info = wallet.migratewallet() + + # Both addresses should only appear in the watchonly wallet + p2pkh_addr_info = wallet.getaddressinfo(p2pkh_addr) + assert_equal(p2pkh_addr_info["iswatchonly"], False) + assert_equal(p2pkh_addr_info["ismine"], False) + p2wpkh_addr_info = wallet.getaddressinfo(p2wpkh_addr) + assert_equal(p2wpkh_addr_info["iswatchonly"], False) + assert_equal(p2wpkh_addr_info["ismine"], False) + + watchonly_wallet = self.nodes[0].get_wallet_rpc(migrate_info["watchonly_name"]) + watchonly_p2pkh_addr_info = watchonly_wallet.getaddressinfo(p2pkh_addr) + assert_equal(watchonly_p2pkh_addr_info["iswatchonly"], False) + assert_equal(watchonly_p2pkh_addr_info["ismine"], True) + watchonly_p2wpkh_addr_info = watchonly_wallet.getaddressinfo(p2wpkh_addr) + assert_equal(watchonly_p2wpkh_addr_info["iswatchonly"], False) + assert_equal(watchonly_p2wpkh_addr_info["ismine"], True) + + # There should only be raw or addr descriptors + for desc in watchonly_wallet.listdescriptors()["descriptors"]: + if desc["desc"].startswith("raw(") or desc["desc"].startswith("addr("): + continue + assert False, "Hybrid pubkey watchonly wallet has more than just raw() and addr()" + + wallet.unloadwallet() + def run_test(self): self.generate(self.nodes[0], 101) @@ -787,6 +844,7 @@ class WalletMigrationTest(BitcoinTestFramework): self.test_addressbook() self.test_migrate_raw_p2sh() self.test_conflict_txs() + self.test_hybrid_pubkey() if __name__ == '__main__': WalletMigrationTest().main() diff --git a/test/functional/wallet_pruning.py b/test/functional/wallet_pruning.py index 06bd992da7..6e252b5406 100755 --- a/test/functional/wallet_pruning.py +++ b/test/functional/wallet_pruning.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test wallet import on pruned node.""" -import os from test_framework.util import assert_equal, assert_raises_rpc_error from test_framework.blocktools import ( @@ -74,7 +73,7 @@ class WalletPruningTest(BitcoinTestFramework): # Import wallet into pruned node self.nodes[1].createwallet(wallet_name="wallet_pruned", descriptors=False, load_on_startup=True) - self.nodes[1].importwallet(os.path.join(self.nodes[0].datadir, wallet_file)) + self.nodes[1].importwallet(self.nodes[0].datadir_path / wallet_file) # Make sure that prune node's wallet correctly accounts for balances assert_equal(self.nodes[1].getbalance(), self.nodes[0].getbalance()) @@ -93,12 +92,12 @@ class WalletPruningTest(BitcoinTestFramework): # Make sure wallet cannot be imported because of missing blocks # This will try to rescan blocks `TIMESTAMP_WINDOW` (2h) before the wallet birthheight. # There are 6 blocks an hour, so 11 blocks (excluding birthheight). - assert_raises_rpc_error(-4, f"Pruned blocks from height {wallet_birthheight - 11} required to import keys. Use RPC call getblockchaininfo to determine your pruned height.", self.nodes[1].importwallet, os.path.join(self.nodes[0].datadir, wallet_file)) + assert_raises_rpc_error(-4, f"Pruned blocks from height {wallet_birthheight - 11} required to import keys. Use RPC call getblockchaininfo to determine your pruned height.", self.nodes[1].importwallet, self.nodes[0].datadir_path / wallet_file) self.log.info("- Done") def get_birthheight(self, wallet_file): """Gets birthheight of a wallet on node0""" - with open(os.path.join(self.nodes[0].datadir, wallet_file), 'r', encoding="utf8") as f: + with open(self.nodes[0].datadir_path / wallet_file, 'r', encoding="utf8") as f: for line in f: if line.startswith('# * Best block at time of backup'): wallet_birthheight = int(line.split(' ')[9]) @@ -106,12 +105,12 @@ class WalletPruningTest(BitcoinTestFramework): def has_block(self, block_index): """Checks if the pruned node has the specific blk0000*.dat file""" - return os.path.isfile(os.path.join(self.nodes[1].blocks_path, f"blk{block_index:05}.dat")) + return (self.nodes[1].blocks_path / f"blk{block_index:05}.dat").is_file() def create_wallet(self, wallet_name, *, unload=False): """Creates and dumps a wallet on the non-pruned node0 to be later import by the pruned node""" self.nodes[0].createwallet(wallet_name=wallet_name, descriptors=False, load_on_startup=True) - self.nodes[0].dumpwallet(os.path.join(self.nodes[0].datadir, wallet_name + ".dat")) + self.nodes[0].dumpwallet(self.nodes[0].datadir_path / f"{wallet_name}.dat") if (unload): self.nodes[0].unloadwallet(wallet_name) diff --git a/test/functional/wallet_reorgsrestore.py b/test/functional/wallet_reorgsrestore.py index af01b9439f..86a2905c72 100755 --- a/test/functional/wallet_reorgsrestore.py +++ b/test/functional/wallet_reorgsrestore.py @@ -14,7 +14,6 @@ disconnected. """ from decimal import Decimal -import os import shutil from test_framework.test_framework import BitcoinTestFramework @@ -88,8 +87,8 @@ class ReorgsRestoreTest(BitcoinTestFramework): # Node0 wallet file is loaded on longest sync'ed node1 self.stop_node(1) - self.nodes[0].backupwallet(os.path.join(self.nodes[0].datadir, 'wallet.bak')) - shutil.copyfile(os.path.join(self.nodes[0].datadir, 'wallet.bak'), os.path.join(self.nodes[1].chain_path, self.default_wallet_name, self.wallet_data_filename)) + self.nodes[0].backupwallet(self.nodes[0].datadir_path / 'wallet.bak') + shutil.copyfile(self.nodes[0].datadir_path / 'wallet.bak', self.nodes[1].chain_path / self.default_wallet_name / self.wallet_data_filename) self.start_node(1) tx_after_reorg = self.nodes[1].gettransaction(txid) # Check that normal confirmed tx is confirmed again but with different blockhash |