diff --git a/.github/ci-test-each-commit-exec.py b/.github/ci-test-each-commit-exec.py index 3b2eaeeb596..a6796d7bc60 100755 --- a/.github/ci-test-each-commit-exec.py +++ b/.github/ci-test-each-commit-exec.py @@ -10,10 +10,11 @@ import shlex def run(cmd, **kwargs): print("+ " + shlex.join(cmd), flush=True) + kwargs.setdefault("check", True) try: - return subprocess.run(cmd, check=True, **kwargs) + return subprocess.run(cmd, **kwargs) except Exception as e: - sys.exit(e) + sys.exit(str(e)) def main(): @@ -44,7 +45,11 @@ def main(): "-DWITH_USDT=ON", "-DCMAKE_CXX_FLAGS=-Wno-error=unused-member-function", ]) - run(["cmake", "--build", "build", "-j", str(num_procs)]) + + if run(["cmake", "--build", "build", "-j", str(num_procs)], check=False).returncode != 0: + print("Build failure. Verbose build follows.") + run(["cmake", "--build", "build", "-j1", "--verbose"]) + run([ "ctest", "--output-on-failure", diff --git a/ci/test/02_run_container.py b/ci/test/02_run_container.py index 513ecacaca8..e7777b324af 100755 --- a/ci/test/02_run_container.py +++ b/ci/test/02_run_container.py @@ -14,7 +14,7 @@ def run(cmd, **kwargs): try: return subprocess.run(cmd, check=True, **kwargs) except Exception as e: - sys.exit(e) + sys.exit(str(e)) def main(): diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index 05e4d8fd54a..7a1e7f32133 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -146,11 +146,9 @@ cmake --build "${BASE_BUILD_DIR}" "$MAKEJOBS" --target all $GOAL || ( ) bash -c "${PRINT_CCACHE_STATISTICS}" -if [ "$CI" = "true" ]; then - hit_rate=$(ccache -s | grep "Hits:" | head -1 | sed 's/.*(\(.*\)%).*/\1/') - if [ "${hit_rate%.*}" -lt 75 ]; then - echo "::notice title=low ccache hitrate::Ccache hit-rate in $CONTAINER_NAME was $hit_rate%" - fi +hit_rate=$(ccache --show-stats | grep "Hits:" | head -1 | sed 's/.*(\(.*\)%).*/\1/') +if [ "${hit_rate%.*}" -lt 75 ]; then + echo "::notice title=low ccache hitrate::Ccache hit-rate in $CONTAINER_NAME was $hit_rate%" fi du -sh "${DEPENDS_DIR}"/*/ du -sh "${PREVIOUS_RELEASES_DIR}" diff --git a/cmake/secp256k1.cmake b/cmake/secp256k1.cmake index 5302f516d89..c82f361aba0 100644 --- a/cmake/secp256k1.cmake +++ b/cmake/secp256k1.cmake @@ -9,6 +9,11 @@ function(add_secp256k1 subdir) message("Configuring secp256k1 subtree...") set(BUILD_SHARED_LIBS OFF) set(CMAKE_EXPORT_COMPILE_COMMANDS OFF) + + # Unconditionally prevent secp's symbols from being exported by our libs + set(CMAKE_C_VISIBILITY_PRESET hidden) + set(SECP256K1_ENABLE_API_VISIBILITY_ATTRIBUTES OFF CACHE BOOL "" FORCE) + set(SECP256K1_ENABLE_MODULE_ECDH OFF CACHE BOOL "" FORCE) set(SECP256K1_ENABLE_MODULE_RECOVERY ON CACHE BOOL "" FORCE) set(SECP256K1_ENABLE_MODULE_MUSIG ON CACHE BOOL "" FORCE) diff --git a/contrib/guix/INSTALL.md b/contrib/guix/INSTALL.md index f9a79f66349..93fcdf485b4 100644 --- a/contrib/guix/INSTALL.md +++ b/contrib/guix/INSTALL.md @@ -71,13 +71,13 @@ https://repology.org/project/guix/versions ### Debian / Ubuntu -Guix is available as a distribution package in [Debian -](https://packages.debian.org/search?keywords=guix) and [Ubuntu -](https://packages.ubuntu.com/search?keywords=guix). +Currently, the `guix` package is no longer present in recent Debian or Ubuntu +repositories. Any other installation option mentioned in this document may be +used. -To install: +If you previously installed `guix` via `apt`, you can remove it with: ```sh -sudo apt install guix +sudo apt purge guix ``` ### Arch Linux diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index ebfafb75baa..cfaa0bf70ba 100755 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -393,12 +393,14 @@ mv --no-target-directory "$OUTDIR" "$ACTUAL_OUTDIR" \ || ( rm -rf "$ACTUAL_OUTDIR" && exit 1 ) ( + tmp="$(mktemp)" cd /outdir-base { echo "$GIT_ARCHIVE" find "$ACTUAL_OUTDIR" -type f } | xargs realpath --relative-base="$PWD" \ - | xargs sha256sum \ - | sort -k2 \ - | sponge "$ACTUAL_OUTDIR"/SHA256SUMS.part + | xargs sha256sum \ + | sort -k2 \ + > "$tmp"; + mv "$tmp" "$ACTUAL_OUTDIR"/SHA256SUMS.part ) diff --git a/contrib/guix/libexec/codesign.sh b/contrib/guix/libexec/codesign.sh index fe86065350e..43db9159946 100755 --- a/contrib/guix/libexec/codesign.sh +++ b/contrib/guix/libexec/codesign.sh @@ -141,6 +141,7 @@ mv --no-target-directory "$OUTDIR" "$ACTUAL_OUTDIR" \ || ( rm -rf "$ACTUAL_OUTDIR" && exit 1 ) ( + tmp="$(mktemp)" cd /outdir-base { echo "$CODESIGNING_TARBALL" @@ -149,5 +150,6 @@ mv --no-target-directory "$OUTDIR" "$ACTUAL_OUTDIR" \ } | xargs realpath --relative-base="$PWD" \ | xargs sha256sum \ | sort -k2 \ - | sponge "$ACTUAL_OUTDIR"/SHA256SUMS.part + > "$tmp"; + mv "$tmp" "$ACTUAL_OUTDIR"/SHA256SUMS.part ) diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm index 59a6366984a..9895b066f2f 100644 --- a/contrib/guix/manifest.scm +++ b/contrib/guix/manifest.scm @@ -504,36 +504,6 @@ inspecting signatures in Mach-O binaries.") (("^install-others =.*$") (string-append "install-others = " out "/etc/rpc\n"))))))))))))) -;; The sponge tool from moreutils. -(define-public sponge - (package - (name "sponge") - (version "0.69") - (source (origin - (method url-fetch) - (uri (string-append - "https://git.joeyh.name/index.cgi/moreutils.git/snapshot/ - moreutils-" version ".tar.gz")) - (file-name (string-append "moreutils-" version ".tar.gz")) - (sha256 - (base32 - "1l859qnzccslvxlh5ghn863bkq2vgmqgnik6jr21b9kc6ljmsy8g")))) - (build-system gnu-build-system) - (arguments - (list #:phases - #~(modify-phases %standard-phases - (delete 'configure) - (replace 'install - (lambda* (#:key outputs #:allow-other-keys) - (let ((bin (string-append (assoc-ref outputs "out") "/bin"))) - (install-file "sponge" bin))))) - #:make-flags - #~(list "sponge" (string-append "CC=" #$(cc-for-target))))) - (home-page "https://joeyh.name/code/moreutils/") - (synopsis "Miscellaneous general-purpose command-line tools") - (description "Just sponge") - (license license:gpl2+))) - (packages->manifest (append (list ;; The Basics @@ -548,7 +518,6 @@ inspecting signatures in Mach-O binaries.") patch gawk sed - sponge ;; Compression and archiving tar gzip diff --git a/contrib/tracing/README.md b/contrib/tracing/README.md index 252053e7b87..192182e17d6 100644 --- a/contrib/tracing/README.md +++ b/contrib/tracing/README.md @@ -22,7 +22,7 @@ corresponding packages. See [installing bpftrace] and [installing BCC] for more information. For development there exist a [bpftrace Reference Guide], a [BCC Reference Guide], and a [bcc Python Developer Tutorial]. -[installing bpftrace]: https://github.com/iovisor/bpftrace/blob/master/INSTALL.md +[installing bpftrace]: https://github.com/bpftrace/bpftrace/blob/master/README.md#quick-start [installing BCC]: https://github.com/iovisor/bcc/blob/master/INSTALL.md [bpftrace Reference Guide]: https://github.com/iovisor/bpftrace/blob/master/docs/reference_guide.md [BCC Reference Guide]: https://github.com/iovisor/bcc/blob/master/docs/reference_guide.md diff --git a/doc/release-notes.md b/doc/release-notes.md index e8ce2e59f29..f5d7d3976db 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -44,44 +44,70 @@ Notable changes - #34358 wallet: fix removeprunedfunds bug with conflicting transactions +### Net + +- #34549 net: reduce log level for PCP/NAT-PMP NOT_AUTHORIZED failures + ### PSBT - #34272 psbt: Fix PSBTInputSignedAndVerified bounds assert +### Miniscript + +- #34434 miniscript: correct and_v() properties + ### Build - #34281 build: Temporarily remove confusing and brittle -fdebug-prefix-map +- #34554 build: avoid exporting secp256k1 symbols +- #34627 guix: use a temporary file over sponge, drop moreutils ### Test - #34185 test: fix feature_pruning when built without wallet - #34282 qa: Fix Windows logging bug - #34390 test: allow overriding tar in get_previous_releases.py +- #34409 test: use ModuleNotFoundError in interface_ipc.py +- #34445 fuzz: Use AFL_SHM_ID for naming test directories +- #34608 test: Fix broken --valgrind handling after bitcoin wrapper ### Doc - #34252 doc: add 433 (Pay to Anchor) to bips.md - #34413 doc: Remove outdated -fdebug-prefix-map section in dev notes +- #34510 doc: fix broken bpftrace installation link +- #34561 wallet: rpc: manpage: fix example missing `fee_rate` argument +- #34671 doc: Update Guix install for Debian/Ubuntu ### CI - #32513 ci: remove 3rd party js from windows dll gha job - #34344 ci: update GitHub Actions versions +- #34453 ci: Always print low ccache hit rate notice +- #34461 ci: Print verbose build error message in test-each-commit Credits ======= Thanks to everyone who directly contributed to this release: +- ANAVHEOBA - brunoerg +- darosior - fanquake - Hennadii Stepanov +- jayvaliya - Lőrinc - m3dwards +- marcofleon - MarcoFalke - mzumsande - Padraic Slattery - Sebastian Falbesoner +- SomberNight +- theuni +- ToRyVand +- willcl-ark As well as to everyone that helped with translations on [Transifex](https://explore.transifex.com/bitcoin/bitcoin/). diff --git a/src/common/pcp.cpp b/src/common/pcp.cpp index 6112dbf54c9..7889d211fa2 100644 --- a/src/common/pcp.cpp +++ b/src/common/pcp.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -81,6 +82,8 @@ constexpr size_t NATPMP_MAP_RESPONSE_LIFETIME_OFS = 12; constexpr uint8_t NATPMP_RESULT_SUCCESS = 0; //! Result code representing unsupported version. constexpr uint8_t NATPMP_RESULT_UNSUPP_VERSION = 1; +//! Result code representing not authorized (router doesn't support port mapping). +constexpr uint8_t NATPMP_RESULT_NOT_AUTHORIZED = 2; //! Result code representing lack of resources. constexpr uint8_t NATPMP_RESULT_NO_RESOURCES = 4; @@ -144,6 +147,8 @@ constexpr size_t PCP_MAP_EXTERNAL_IP_OFS = 20; //! Result code representing success (RFC6887 7.4), shared with NAT-PMP. constexpr uint8_t PCP_RESULT_SUCCESS = NATPMP_RESULT_SUCCESS; +//! Result code representing not authorized (RFC6887 7.4), shared with NAT-PMP. +constexpr uint8_t PCP_RESULT_NOT_AUTHORIZED = NATPMP_RESULT_NOT_AUTHORIZED; //! Result code representing lack of resources (RFC6887 7.4). constexpr uint8_t PCP_RESULT_NO_RESOURCES = 8; @@ -374,7 +379,16 @@ std::variant NATPMPRequestPortMap(const CNetAddr &g Assume(response.size() >= NATPMP_MAP_RESPONSE_SIZE); uint16_t result_code = ReadBE16(response.data() + NATPMP_RESPONSE_HDR_RESULT_OFS); if (result_code != NATPMP_RESULT_SUCCESS) { - LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Port mapping failed with result %s\n", NATPMPResultString(result_code)); + if (result_code == NATPMP_RESULT_NOT_AUTHORIZED) { + static std::atomic warned{false}; + if (!warned.exchange(true)) { + LogWarning("natpmp: Port mapping failed with result %s\n", NATPMPResultString(result_code)); + } else { + LogDebug(BCLog::NET, "natpmp: Port mapping failed with result %s\n", NATPMPResultString(result_code)); + } + } else { + LogWarning("natpmp: Port mapping failed with result %s\n", NATPMPResultString(result_code)); + } if (result_code == NATPMP_RESULT_NO_RESOURCES) { return MappingError::NO_RESOURCES; } @@ -508,7 +522,16 @@ std::variant PCPRequestPortMap(const PCPMappingNonc uint16_t external_port = ReadBE16(response.data() + PCP_HDR_SIZE + PCP_MAP_EXTERNAL_PORT_OFS); CNetAddr external_addr{PCPUnwrapAddress(response.subspan(PCP_HDR_SIZE + PCP_MAP_EXTERNAL_IP_OFS, ADDR_IPV6_SIZE))}; if (result_code != PCP_RESULT_SUCCESS) { - LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "pcp: Mapping failed with result %s\n", PCPResultString(result_code)); + if (result_code == PCP_RESULT_NOT_AUTHORIZED) { + static std::atomic warned{false}; + if (!warned.exchange(true)) { + LogWarning("pcp: Mapping failed with result %s\n", PCPResultString(result_code)); + } else { + LogDebug(BCLog::NET, "pcp: Mapping failed with result %s\n", PCPResultString(result_code)); + } + } else { + LogWarning("pcp: Mapping failed with result %s\n", PCPResultString(result_code)); + } if (result_code == PCP_RESULT_NO_RESOURCES) { return MappingError::NO_RESOURCES; } diff --git a/src/script/miniscript.cpp b/src/script/miniscript.cpp index f04291af1ca..6dc154a8a26 100644 --- a/src/script/miniscript.cpp +++ b/src/script/miniscript.cpp @@ -144,7 +144,7 @@ Type ComputeType(Fragment fragment, Type x, Type y, Type z, const std::vector UniValue { diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py index 2c9a0022442..89a656f10c5 100755 --- a/test/functional/interface_ipc.py +++ b/test/functional/interface_ipc.py @@ -14,7 +14,7 @@ from test_framework.wallet import MiniWallet # Test may be skipped and not have capnp installed try: import capnp # type: ignore[import] # noqa: F401 -except ImportError: +except ModuleNotFoundError: pass diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 0b9295cef53..eb8baf97853 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -11,6 +11,7 @@ from datetime import datetime, timezone import json import logging import os +import pathlib import platform import pdb import random @@ -72,9 +73,19 @@ class Binaries: binaries, which takes precedence over the paths above, if specified. This is used by tests calling binaries from previous releases. """ - def __init__(self, paths, bin_dir): + def __init__(self, paths, bin_dir, *, use_valgrind=False): self.paths = paths self.bin_dir = bin_dir + suppressions_file = pathlib.Path(__file__).parents[3] / "contrib" / "valgrind.supp" + self.valgrind_cmd = [ + "valgrind", + f"--suppressions={suppressions_file}", + "--gen-suppressions=all", + "--trace-children=yes", # Needed for 'bitcoin' wrapper + "--exit-on-first-error=yes", + "--error-exitcode=1", + "--quiet", + ] if use_valgrind else [] def node_argv(self, **kwargs): "Return argv array that should be used to invoke bitcoind" @@ -102,20 +113,26 @@ class Binaries: return self._argv("chainstate", self.paths.bitcoinchainstate) def _argv(self, command, bin_path, need_ipc=False): - """Return argv array that should be used to invoke the command. It - either uses the bitcoin wrapper executable (if BITCOIN_CMD is set or + """Return argv array that should be used to invoke the command. + + It either uses the bitcoin wrapper executable (if BITCOIN_CMD is set or need_ipc is True), or the direct binary path (bitcoind, etc). When bin_dir is set (by tests calling binaries from previous releases) it - always uses the direct path.""" + always uses the direct path. + + The returned args include valgrind, except when bin_dir is set + (previous releases). Also, valgrind will only apply to the bitcoin + wrapper executable directly, not to the commands that `bitcoin` calls. + """ if self.bin_dir is not None: return [os.path.join(self.bin_dir, os.path.basename(bin_path))] elif self.paths.bitcoin_cmd is not None or need_ipc: # If the current test needs IPC functionality, use the bitcoin # wrapper binary and append -m so it calls multiprocess binaries. bitcoin_cmd = self.paths.bitcoin_cmd or [self.paths.bitcoin_bin] - return bitcoin_cmd + (["-m"] if need_ipc else []) + [command] + return self.valgrind_cmd + bitcoin_cmd + (["-m"] if need_ipc else []) + [command] else: - return [bin_path] + return self.valgrind_cmd + [bin_path] class BitcoinTestMetaClass(type): @@ -234,7 +251,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): help="The seed to use for assigning port numbers (default: current process id)") parser.add_argument("--previous-releases", dest="prev_releases", action="store_true", default=os.path.isdir(previous_releases_path) and bool(os.listdir(previous_releases_path)), - help="Force test of previous releases (default: %(default)s)") + help="Force test of previous releases (default: %(default)s). Previous releases binaries can be downloaded via `test/get_previous_releases.py`.") parser.add_argument("--coveragedir", dest="coveragedir", help="Write tested RPC commands into this directory") parser.add_argument("--configfile", dest="configfile", @@ -247,7 +264,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): parser.add_argument("--perf", dest="perf", default=False, action="store_true", help="profile running nodes with perf for the duration of the test") parser.add_argument("--valgrind", dest="valgrind", default=False, action="store_true", - help="run nodes under the valgrind memory error detector: expect at least a ~10x slowdown. valgrind 3.14 or later required. Does not apply to previous release binaries.") + help="Run binaries under the valgrind memory error detector: Expect at least a ~10x slowdown. Does not apply to previous release binaries.") parser.add_argument("--randomseed", type=int, help="set a random seed for deterministically reproducing a previous test run") parser.add_argument("--timeout-factor", dest="timeout_factor", type=float, help="adjust test timeouts by a factor. Setting it to 0 disables all timeouts") @@ -305,7 +322,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): return paths def get_binaries(self, bin_dir=None): - return Binaries(self.binary_paths, bin_dir) + return Binaries(self.binary_paths, bin_dir, use_valgrind=self.options.valgrind) def setup(self): """Call this method to start up the test framework object with options set.""" @@ -543,18 +560,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): bin_dirs = [] for v in versions: bin_dir = bin_dir_from_version(v) - - # Fail test if any of the needed release binaries is missing - for bin_path in (argv[0] for binaries in (self.get_binaries(bin_dir),) - for argv in (binaries.node_argv(), binaries.rpc_argv())): - - if shutil.which(bin_path) is None: - self.log.error(f"Binary not found: {bin_path}") - if v is None: - raise AssertionError("At least one binary is missing, did you compile?") - raise AssertionError("At least one release binary is missing. " - "Previous releases binaries can be downloaded via `test/get_previous_releases.py`.") - bin_dirs.append(bin_dir) extra_init = [{}] * num_nodes if self.extra_init is None else self.extra_init # type: ignore[var-annotated] @@ -578,7 +583,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): extra_args=args, use_cli=self.options.usecli, start_perf=self.options.perf, - use_valgrind=self.options.valgrind, v2transport=self.options.v2transport, uses_wallet=self.uses_wallet, ) @@ -1049,8 +1053,8 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): """Checks whether previous releases are present and enabled.""" if not os.path.isdir(self.options.previous_releases_path): if self.options.prev_releases: - raise AssertionError("Force test of previous releases but releases missing: {}".format( - self.options.previous_releases_path)) + raise AssertionError(f"Force test of previous releases but releases missing: {self.options.previous_releases_path}\n" + "Previous releases binaries can be downloaded via `test/get_previous_releases.py`.") return self.options.prev_releases def skip_if_no_external_signer(self): diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 3b55b915efd..74c95da4c12 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -91,7 +91,27 @@ 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_path, *, chain, rpchost, timewait, timeout_factor, binaries, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, v2transport=False, uses_wallet=False, ipcbind=False): + def __init__( + self, + i, + datadir_path, + *, + chain, + rpchost, + timewait, + timeout_factor, + binaries, + coverage_dir, + cwd, + extra_conf=None, + extra_args=None, + use_cli=False, + start_perf=False, + version=None, + v2transport=False, + uses_wallet=False, + ipcbind=False, + ): """ Kwargs: start_perf (bool): If True, begin profiling the node with `perf` as soon as @@ -146,15 +166,6 @@ class TestNode(): self.ipc_socket_path = self.ipc_tmp_dir / "node.sock" self.args.append(f"-ipcbind=unix:{self.ipc_socket_path}") - # Use valgrind, expect for previous release binaries - if use_valgrind and version is None: - 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), - "--gen-suppressions=all", "--exit-on-first-error=yes", - "--error-exitcode=1", "--quiet"] + self.args - if self.version_is_at_least(190000): self.args.append("-logthreadnames") if self.version_is_at_least(219900): diff --git a/test/functional/tool_bitcoin.py b/test/functional/tool_bitcoin.py index 1112e02e090..d59c75dee53 100755 --- a/test/functional/tool_bitcoin.py +++ b/test/functional/tool_bitcoin.py @@ -39,7 +39,9 @@ class ToolBitcoinTest(BitcoinTestFramework): def set_cmd_args(self, node, args): """Set up node so it will be started through bitcoin wrapper command with specified arguments.""" - node.args = [self.binary_paths.bitcoin_bin] + args + ["node"] + self.node_options[node.index] + # Manually construct the `bitcoin node` command, similar to Binaries::node_argv() + bitcoin_cmd = node.binaries.valgrind_cmd + [node.binaries.paths.bitcoin_bin] + node.args = bitcoin_cmd + args + ["node"] + self.node_options[node.index] def test_args(self, cmd_args, node_args, expect_exe=None, expect_error=None): node = self.nodes[0]