From 2721d64989c2b2114890586b7efd01ab4b062ca6 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 30 Apr 2024 13:19:05 -0400 Subject: [PATCH 01/39] chainparams: Add achow101 DNS seeder --- src/kernel/chainparams.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 26c261eba2..1e67aaf00b 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -140,6 +140,7 @@ class CMainParams : public CChainParams { vSeeds.emplace_back("seed.bitcoin.sprovoost.nl."); // Sjors Provoost vSeeds.emplace_back("dnsseed.emzy.de."); // Stephan Oeste vSeeds.emplace_back("seed.bitcoin.wiz.biz."); // Jason Maurice + vSeeds.emplace_back("seed.mainnet.achownodes.xyz."); // Ava Chow, only supports x1, x5, x9, x49, x809, x849, xd, x400, x404, x408, x448, xc08, xc48, x40c base58Prefixes[PUBKEY_ADDRESS] = std::vector(1,0); base58Prefixes[SCRIPT_ADDRESS] = std::vector(1,5); @@ -246,6 +247,7 @@ class CTestNetParams : public CChainParams { vSeeds.emplace_back("seed.tbtc.petertodd.net."); vSeeds.emplace_back("seed.testnet.bitcoin.sprovoost.nl."); vSeeds.emplace_back("testnet-seed.bluematt.me."); // Just a static list of stable node(s), only supports x9 + vSeeds.emplace_back("seed.testnet.achownodes.xyz."); // Ava Chow, only supports x1, x5, x9, x49, x809, x849, xd, x400, x404, x408, x448, xc08, xc48, x40c base58Prefixes[PUBKEY_ADDRESS] = std::vector(1,111); base58Prefixes[SCRIPT_ADDRESS] = std::vector(1,196); @@ -297,6 +299,7 @@ class SigNetParams : public CChainParams { if (!options.challenge) { bin = ParseHex("512103ad5e0edad18cb1f0fc0d28a3d4f1f3e445640337489abb10404f2d1e086be430210359ef5021964fe22d6f8e05b2463c9540ce96883fe3b278760f048f5189f2e6c452ae"); vSeeds.emplace_back("seed.signet.bitcoin.sprovoost.nl."); + vSeeds.emplace_back("seed.signet.achownodes.xyz."); // Ava Chow, only supports x1, x5, x9, x49, x809, x849, xd, x400, x404, x408, x448, xc08, xc48, x40c // Hardcoded nodes can be removed once there are more DNS seeds vSeeds.emplace_back("178.128.221.177"); From b5fc6d46a3854c18f6e8dfc89540d24ef778caa2 Mon Sep 17 00:00:00 2001 From: fanquake Date: Sat, 27 Apr 2024 15:36:28 +0800 Subject: [PATCH 02/39] guix: use glibc 2.31 Set minimum required glibc to 2.31. The glibc 2.31 branch is still maintained: https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/release/2.31/master. Remove the stack-protector check from test-security-check, as the test no-longer fails, and given the control we have of the end, the actual security-check test seems sufficient (this might also be applied to some of the other checks). Drops runtime support for Ubuntu Bionic 18.04 and RHEL-8 from the release binaries. --- contrib/devtools/symbol-check.py | 28 +- contrib/devtools/test-security-check.py | 34 +-- contrib/guix/manifest.scm | 24 +- contrib/guix/patches/glibc-2.27-fcommon.patch | 34 --- .../guix/patches/glibc-2.27-no-librt.patch | 53 ---- .../patches/glibc-2.27-powerpc-ldbrx.patch | 245 ------------------ ...as_include-to-include-asm-syscalls.h.patch | 78 ------ ...x-prefix.patch => glibc-guix-prefix.patch} | 8 +- doc/dependencies.md | 2 +- doc/release-notes-29987.md | 6 + 10 files changed, 50 insertions(+), 462 deletions(-) delete mode 100644 contrib/guix/patches/glibc-2.27-fcommon.patch delete mode 100644 contrib/guix/patches/glibc-2.27-no-librt.patch delete mode 100644 contrib/guix/patches/glibc-2.27-powerpc-ldbrx.patch delete mode 100644 contrib/guix/patches/glibc-2.27-riscv64-Use-__has_include-to-include-asm-syscalls.h.patch rename contrib/guix/patches/{glibc-2.27-guix-prefix.patch => glibc-guix-prefix.patch} (78%) create mode 100644 doc/release-notes-29987.md diff --git a/contrib/devtools/symbol-check.py b/contrib/devtools/symbol-check.py index 6613874ce3..b4e1ce64ae 100755 --- a/contrib/devtools/symbol-check.py +++ b/contrib/devtools/symbol-check.py @@ -14,31 +14,31 @@ import lief -# Debian 10 (Buster) EOL: 2024. https://wiki.debian.org/LTS +# Debian 11 (Bullseye) EOL: 2026. https://wiki.debian.org/LTS # -# - libgcc version 8.3.0 (https://packages.debian.org/search?suite=buster&arch=any&searchon=names&keywords=libgcc1) -# - libc version 2.28 (https://packages.debian.org/search?suite=buster&arch=any&searchon=names&keywords=libc6) +# - libgcc version 10.2.1 (https://packages.debian.org/bullseye/libgcc-s1) +# - libc version 2.31 (https://packages.debian.org/source/bullseye/glibc) # -# Ubuntu 18.04 (Bionic) EOL: 2028. https://wiki.ubuntu.com/ReleaseTeam +# Ubuntu 20.04 (Focal) EOL: 2030. https://wiki.ubuntu.com/ReleaseTeam # -# - libgcc version 8.4.0 (https://packages.ubuntu.com/bionic/libgcc1) -# - libc version 2.27 (https://packages.ubuntu.com/bionic/libc6) +# - libgcc version 10.5.0 (https://packages.ubuntu.com/focal/libgcc1) +# - libc version 2.31 (https://packages.ubuntu.com/focal/libc6) # -# CentOS Stream 8 EOL: 2024. https://wiki.centos.org/About/Product +# CentOS Stream 9 EOL: 2027. https://www.centos.org/cl-vs-cs/#end-of-life # -# - libgcc version 8.5.0 (http://mirror.centos.org/centos/8-stream/AppStream/x86_64/os/Packages/) -# - libc version 2.28 (http://mirror.centos.org/centos/8-stream/AppStream/x86_64/os/Packages/) +# - libgcc version 12.2.1 (https://mirror.stream.centos.org/9-stream/AppStream/x86_64/os/Packages/) +# - libc version 2.34 (https://mirror.stream.centos.org/9-stream/AppStream/x86_64/os/Packages/) # # See https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html for more info. MAX_VERSIONS = { 'GCC': (4,3,0), 'GLIBC': { - lief.ELF.ARCH.x86_64: (2,27), - lief.ELF.ARCH.ARM: (2,27), - lief.ELF.ARCH.AARCH64:(2,27), - lief.ELF.ARCH.PPC64: (2,27), - lief.ELF.ARCH.RISCV: (2,27), + lief.ELF.ARCH.x86_64: (2,31), + lief.ELF.ARCH.ARM: (2,31), + lief.ELF.ARCH.AARCH64:(2,31), + lief.ELF.ARCH.PPC64: (2,31), + lief.ELF.ARCH.RISCV: (2,31), }, 'LIBATOMIC': (1,0), 'V': (0,5,0), # xkb (bitcoin-qt only) diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index dd0cf7030a..7bfd4d98da 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -59,32 +59,32 @@ def test_ELF(self): arch = get_arch(cc, source, executable) if arch == lief.ARCHITECTURES.X86: - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), - (1, executable+': failed PIE NX RELRO Canary CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), - (1, executable+': failed PIE RELRO Canary CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + (1, executable+': failed PIE NX RELRO CONTROL_FLOW')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE RELRO CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + (1, executable+': failed PIE RELRO CONTROL_FLOW')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']), (1, executable+': failed RELRO CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']), (1, executable+': failed separate_code CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']), (1, executable+': failed CONTROL_FLOW')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code', '-fcf-protection=full']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code', '-fcf-protection=full']), (0, '')) else: - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), - (1, executable+': failed PIE NX RELRO Canary')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), - (1, executable+': failed PIE RELRO Canary')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + (1, executable+': failed PIE NX RELRO')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), + (1, executable+': failed PIE RELRO')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE RELRO')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']), (1, executable+': failed RELRO')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']), (1, executable+': failed separate_code')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']), (0, '')) clean_files(source, executable) diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm index 53569d7f7d..44fbfa1c0b 100644 --- a/contrib/guix/manifest.scm +++ b/contrib/guix/manifest.scm @@ -98,7 +98,7 @@ chain for " target " development.")) #:key (base-gcc-for-libc linux-base-gcc) (base-kernel-headers base-linux-kernel-headers) - (base-libc glibc-2.27) + (base-libc glibc-2.31) (base-gcc linux-base-gcc)) "Convenience wrapper around MAKE-CROSS-TOOLCHAIN with default values desirable for building Bitcoin Core release binaries." @@ -440,24 +440,21 @@ inspecting signatures in Mach-O binaries.") (("-rpath=") "-rpath-link=")) #t)))))))) -(define-public glibc-2.27 +(define-public glibc-2.31 + (let ((commit "8e30f03744837a85e33d84ccd34ed3abe30d37c3")) (package - (inherit glibc-2.31) - (version "2.27") + (inherit glibc) ;; 2.35 + (version "2.31") (source (origin (method git-fetch) (uri (git-reference (url "https://sourceware.org/git/glibc.git") - (commit "73886db6218e613bd6d4edf529f11e008a6c2fa6"))) - (file-name (git-file-name "glibc" "73886db6218e613bd6d4edf529f11e008a6c2fa6")) + (commit commit))) + (file-name (git-file-name "glibc" commit)) (sha256 (base32 - "0azpb9cvnbv25zg8019rqz48h8i2257ngyjg566dlnp74ivrs9vq")) - (patches (search-our-patches "glibc-2.27-riscv64-Use-__has_include-to-include-asm-syscalls.h.patch" - "glibc-2.27-fcommon.patch" - "glibc-2.27-guix-prefix.patch" - "glibc-2.27-no-librt.patch" - "glibc-2.27-powerpc-ldbrx.patch")))) + "1zi0s9yy5zkisw823vivn7zlj8w6g9p3mm7lmlqiixcxdkz4dbn6")) + (patches (search-our-patches "glibc-guix-prefix.patch")))) (arguments (substitute-keyword-arguments (package-arguments glibc) ((#:configure-flags flags) @@ -473,12 +470,13 @@ inspecting signatures in Mach-O binaries.") (lambda* (#:key outputs #:allow-other-keys) ;; Install the rpc data base file under `$out/etc/rpc'. ;; Otherwise build will fail with "Permission denied." + ;; Can be removed when we are building 2.32 or later. (let ((out (assoc-ref outputs "out"))) (substitute* "sunrpc/Makefile" (("^\\$\\(inst_sysconfdir\\)/rpc(.*)$" _ suffix) (string-append out "/etc/rpc" suffix "\n")) (("^install-others =.*$") - (string-append "install-others = " out "/etc/rpc\n")))))))))))) + (string-append "install-others = " out "/etc/rpc\n"))))))))))))) (packages->manifest (append diff --git a/contrib/guix/patches/glibc-2.27-fcommon.patch b/contrib/guix/patches/glibc-2.27-fcommon.patch deleted file mode 100644 index f8d14837fc..0000000000 --- a/contrib/guix/patches/glibc-2.27-fcommon.patch +++ /dev/null @@ -1,34 +0,0 @@ -commit 264a4a0dbe1f4369db315080034b500bed66016c -Author: fanquake -Date: Fri May 6 11:03:04 2022 +0100 - - build: use -fcommon to retain legacy behaviour with GCC 10 - - GCC 10 started using -fno-common by default, which causes issues with - the powerpc builds using gibc 2.27. A patch was committed to glibc to fix - the issue, 18363b4f010da9ba459b13310b113ac0647c2fcc but is non-trvial - to backport, and was broken in at least one way, see the followup in - commit 7650321ce037302bfc2f026aa19e0213b8d02fe6. - - For now, retain the legacy GCC behaviour by passing -fcommon when - building glibc. - - https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html. - https://sourceware.org/git/?p=glibc.git;a=commit;h=18363b4f010da9ba459b13310b113ac0647c2fcc - https://sourceware.org/git/?p=glibc.git;a=commit;h=7650321ce037302bfc2f026aa19e0213b8d02fe6 - - This patch can be dropped when we are building with glibc 2.31+. - -diff --git a/Makeconfig b/Makeconfig -index 86a71e5802..aa2166be60 100644 ---- a/Makeconfig -+++ b/Makeconfig -@@ -896,7 +896,7 @@ ifeq "$(strip $(+cflags))" "" - endif # $(+cflags) == "" - - +cflags += $(cflags-cpu) $(+gccwarn) $(+merge-constants) $(+math-flags) \ -- $(+stack-protector) -+ $(+stack-protector) -fcommon - +gcc-nowarn := -w - - # Don't duplicate options if we inherited variables from the parent. diff --git a/contrib/guix/patches/glibc-2.27-no-librt.patch b/contrib/guix/patches/glibc-2.27-no-librt.patch deleted file mode 100644 index 4f2092ba7e..0000000000 --- a/contrib/guix/patches/glibc-2.27-no-librt.patch +++ /dev/null @@ -1,53 +0,0 @@ -This patch can be dropped when we are building with glibc 2.30+. - -commit 6e41ef56c9baab719a02f1377b1e7ce7bff61e73 -Author: Florian Weimer -Date: Fri Feb 8 10:21:56 2019 +0100 - - rt: Turn forwards from librt to libc into compat symbols [BZ #24194] - - As the result of commit 6e6249d0b461b952d0f544792372663feb6d792a - ("BZ#14743: Move clock_* symbols from librt to libc."), in glibc 2.17, - clock_gettime, clock_getres, clock_settime, clock_getcpuclockid, - clock_nanosleep were added to libc, and the file rt/clock-compat.c - was added with forwarders to the actual implementations in libc. - These forwarders were wrapped in - - #if SHLIB_COMPAT (librt, GLIBC_2_2, GLIBC_2_17) - - so that they are not present for newer architectures (such as - powerpc64le) with a 2.17 or later ABI baseline. But the forwarders - were not marked as compatibility symbols. As a result, on older - architectures, historic configure checks such as - - AC_CHECK_LIB(rt, clock_gettime) - - still cause linking against librt, even though this is completely - unnecessary. It also creates a needless porting hazard because - architectures behave differently when it comes to symbol availability. - - Reviewed-by: Carlos O'Donell - -diff --git a/rt/clock-compat.c b/rt/clock-compat.c -index f816973c05..11e71aa890 100644 ---- a/rt/clock-compat.c -+++ b/rt/clock-compat.c -@@ -30,14 +30,16 @@ - #if HAVE_IFUNC - # undef INIT_ARCH - # define INIT_ARCH() --# define COMPAT_REDIRECT(name, proto, arglist) libc_ifunc (name, &__##name) -+# define COMPAT_REDIRECT(name, proto, arglist) libc_ifunc (name, &__##name) \ -+ compat_symbol (librt, name, name, GLIBC_2_2); - #else - # define COMPAT_REDIRECT(name, proto, arglist) \ - int \ - name proto \ - { \ - return __##name arglist; \ -- } -+ } \ -+ compat_symbol (librt, name, name, GLIBC_2_2); - #endif - - COMPAT_REDIRECT (clock_getres, diff --git a/contrib/guix/patches/glibc-2.27-powerpc-ldbrx.patch b/contrib/guix/patches/glibc-2.27-powerpc-ldbrx.patch deleted file mode 100644 index 26716054c8..0000000000 --- a/contrib/guix/patches/glibc-2.27-powerpc-ldbrx.patch +++ /dev/null @@ -1,245 +0,0 @@ -From 50b0b3c9ff71ffd7ebbd74ae46844c3566478123 Mon Sep 17 00:00:00 2001 -From: "Gabriel F. T. Gomes" -Date: Mon, 27 May 2019 15:21:22 -0300 -Subject: [PATCH] powerpc: Fix build failures with current GCC - -Since GCC commit 271500 (svn), also known as the following commit on the -git mirror: - -commit e154242724b084380e3221df7c08fcdbd8460674 -Author: amodra -Date: Wed May 22 04:34:26 2019 +0000 - - [RS6000] Don't pass -many to the assembler - -glibc builds are failing when an assembly implementation does not -declare the correct '.machine' directive, or when no such directive is -declared at all. For example, when a POWER6 instruction is used, but -'.machine power6' is not declared, the assembler will fail with an error -similar to the following: - - ../sysdeps/powerpc/powerpc64/power8/strcmp.S: Assembler messages: - 24 ../sysdeps/powerpc/powerpc64/power8/strcmp.S:55: Error: unrecognized opcode: `cmpb' - -This patch adds '.machine powerN' directives where none existed, as well -as it updates '.machine power7' directives on POWER8 files, because the -minimum binutils version required to build glibc (binutils 2.25) now -provides this machine version. It also adds '-many' to the assembler -command used to build tst-set_ppr.c. - -Tested for powerpc, powerpc64, and powerpc64le, as well as with -build-many-glibcs.py for powerpc targets. - -Reviewed-by: Tulio Magno Quites Machado Filho ---- - sysdeps/powerpc/Makefile | 5 +++ - sysdeps/powerpc/powerpc64/power4/memcmp.S | 7 ++++ - sysdeps/powerpc/powerpc64/power7/strncmp.S | 1 + - .../powerpc/powerpc64/power8/fpu/s_llround.S | 1 + - sysdeps/powerpc/powerpc64/power8/strcasecmp.S | 36 ++++++------------- - sysdeps/powerpc/powerpc64/power8/strcasestr.S | 14 ++------ - sysdeps/powerpc/powerpc64/power8/strcmp.S | 1 + - 7 files changed, 28 insertions(+), 37 deletions(-) - -diff --git a/sysdeps/powerpc/Makefile b/sysdeps/powerpc/Makefile -index 6aa683b03f..23126147df 100644 ---- a/sysdeps/powerpc/Makefile -+++ b/sysdeps/powerpc/Makefile -@@ -45,6 +45,11 @@ ifeq ($(subdir),misc) - sysdep_headers += sys/platform/ppc.h - tests += test-gettimebase - tests += tst-set_ppr -+ -+# This test is expected to run and exit with EXIT_UNSUPPORTED on -+# processors that do not implement the Power ISA 2.06 or greater. -+# But the test makes use of instructions from Power ISA 2.06 and 2.07. -+CFLAGS-tst-set_ppr.c += -Wa,-many - endif - - ifneq (,$(filter %le,$(config-machine))) -diff --git a/sysdeps/powerpc/powerpc64/power4/memcmp.S b/sysdeps/powerpc/powerpc64/power4/memcmp.S -index e5319f101f..38dcf4c9a1 100644 ---- a/sysdeps/powerpc/powerpc64/power4/memcmp.S -+++ b/sysdeps/powerpc/powerpc64/power4/memcmp.S -@@ -26,7 +26,14 @@ - # define MEMCMP memcmp - #endif - -+#ifndef __LITTLE_ENDIAN__ - .machine power4 -+#else -+/* Little endian is only available since POWER8, so it's safe to -+ specify .machine as power8 (or older), even though this is a POWER4 -+ file. Since the little-endian code uses 'ldbrx', power7 is enough. */ -+ .machine power7 -+#endif - ENTRY_TOCLESS (MEMCMP, 4) - CALL_MCOUNT 3 - -diff --git a/sysdeps/powerpc/powerpc64/power7/strncmp.S b/sysdeps/powerpc/powerpc64/power7/strncmp.S -index 0c7429d19f..10f898c5a3 100644 ---- a/sysdeps/powerpc/powerpc64/power7/strncmp.S -+++ b/sysdeps/powerpc/powerpc64/power7/strncmp.S -@@ -28,6 +28,7 @@ - const char *s2 [r4], - size_t size [r5]) */ - -+ .machine power7 - ENTRY_TOCLESS (STRNCMP, 5) - CALL_MCOUNT 3 - -diff --git a/sysdeps/powerpc/powerpc64/power8/fpu/s_llround.S b/sysdeps/powerpc/powerpc64/power8/fpu/s_llround.S -index a22fc63bb3..84c76ba0f9 100644 ---- a/sysdeps/powerpc/powerpc64/power8/fpu/s_llround.S -+++ b/sysdeps/powerpc/powerpc64/power8/fpu/s_llround.S -@@ -26,6 +26,7 @@ - - /* long long [r3] llround (float x [fp1]) */ - -+ .machine power8 - ENTRY_TOCLESS (__llround) - CALL_MCOUNT 0 - frin fp1,fp1 /* Round to nearest +-0.5. */ -diff --git a/sysdeps/powerpc/powerpc64/power8/strcasecmp.S b/sysdeps/powerpc/powerpc64/power8/strcasecmp.S -index 3a2efe2a64..eeacd40c7f 100644 ---- a/sysdeps/powerpc/powerpc64/power8/strcasecmp.S -+++ b/sysdeps/powerpc/powerpc64/power8/strcasecmp.S -@@ -91,21 +91,7 @@ - 3: \ - TOLOWER() - --#ifdef _ARCH_PWR8 --# define VCLZD_V8_v7 vclzd v8, v7; --# define MFVRD_R3_V1 mfvrd r3, v1; --# define VSUBUDM_V9_V8 vsubudm v9, v9, v8; --# define VPOPCNTD_V8_V8 vpopcntd v8, v8; --# define VADDUQM_V7_V8 vadduqm v9, v7, v8; --#else --# define VCLZD_V8_v7 .long 0x11003fc2 --# define MFVRD_R3_V1 .long 0x7c230067 --# define VSUBUDM_V9_V8 .long 0x112944c0 --# define VPOPCNTD_V8_V8 .long 0x110047c3 --# define VADDUQM_V7_V8 .long 0x11274100 --#endif -- -- .machine power7 -+ .machine power8 - - ENTRY (__STRCASECMP) - #ifdef USE_AS_STRNCASECMP -@@ -265,15 +251,15 @@ L(different): - #ifdef __LITTLE_ENDIAN__ - /* Count trailing zero. */ - vspltisb v8, -1 -- VADDUQM_V7_V8 -+ vadduqm v9, v7, v8 - vandc v8, v9, v7 -- VPOPCNTD_V8_V8 -+ vpopcntd v8, v8 - vspltb v6, v8, 15 - vcmpequb. v6, v6, v1 - blt cr6, L(shift8) - #else - /* Count leading zero. */ -- VCLZD_V8_v7 -+ vclzd v8, v7 - vspltb v6, v8, 7 - vcmpequb. v6, v6, v1 - blt cr6, L(shift8) -@@ -291,7 +277,7 @@ L(skipsum): - /* Merge and move to GPR. */ - vmrglb v6, v6, v7 - vslo v1, v6, v1 -- MFVRD_R3_V1 -+ mfvrd r3, v1 - /* Place the characters that are different in first position. */ - sldi rSTR2, rRTN, 56 - srdi rSTR2, rSTR2, 56 -@@ -301,7 +287,7 @@ L(skipsum): - vslo v6, v5, v8 - vslo v7, v4, v8 - vmrghb v1, v6, v7 -- MFVRD_R3_V1 -+ mfvrd r3, v1 - srdi rSTR2, rRTN, 48 - sldi rSTR2, rSTR2, 56 - srdi rSTR2, rSTR2, 56 -@@ -320,15 +306,15 @@ L(null_found): - #ifdef __LITTLE_ENDIAN__ - /* Count trailing zero. */ - vspltisb v8, -1 -- VADDUQM_V7_V8 -+ vadduqm v9, v7, v8 - vandc v8, v9, v7 -- VPOPCNTD_V8_V8 -+ vpopcntd v8, v8 - vspltb v6, v8, 15 - vcmpequb. v6, v6, v10 - blt cr6, L(shift_8) - #else - /* Count leading zero. */ -- VCLZD_V8_v7 -+ vclzd v8, v7 - vspltb v6, v8, 7 - vcmpequb. v6, v6, v10 - blt cr6, L(shift_8) -@@ -343,10 +329,10 @@ L(skipsum1): - vspltisb v10, 7 - vslb v10, v10, v10 - vsldoi v9, v0, v10, 1 -- VSUBUDM_V9_V8 -+ vsubudm v9, v9, v8 - vspltisb v8, 8 - vsldoi v8, v0, v8, 1 -- VSUBUDM_V9_V8 -+ vsubudm v9, v9, v8 - /* Shift and remove junk after null character. */ - #ifdef __LITTLE_ENDIAN__ - vslo v5, v5, v9 -diff --git a/sysdeps/powerpc/powerpc64/power8/strcasestr.S b/sysdeps/powerpc/powerpc64/power8/strcasestr.S -index 9fc24c29f9..e10f06fd86 100644 ---- a/sysdeps/powerpc/powerpc64/power8/strcasestr.S -+++ b/sysdeps/powerpc/powerpc64/power8/strcasestr.S -@@ -73,18 +73,8 @@ - vor reg, v8, reg; \ - vcmpequb. v6, reg, v4; - --/* TODO: change these to the actual instructions when the minimum required -- binutils allows it. */ --#ifdef _ARCH_PWR8 --#define VCLZD_V8_v7 vclzd v8, v7; --#else --#define VCLZD_V8_v7 .long 0x11003fc2 --#endif -- - #define FRAMESIZE (FRAME_MIN_SIZE+48) --/* TODO: change this to .machine power8 when the minimum required binutils -- allows it. */ -- .machine power7 -+ .machine power8 - ENTRY (STRCASESTR, 4) - CALL_MCOUNT 2 - mflr r0 /* Load link register LR to r0. */ -@@ -291,7 +281,7 @@ L(nullchk1): - vcmpequb. v6, v0, v7 - /* Shift r3 by 16 bytes and proceed. */ - blt cr6, L(shift16) -- VCLZD_V8_v7 -+ vclzd v8, v7 - #ifdef __LITTLE_ENDIAN__ - vspltb v6, v8, 15 - #else -diff --git a/sysdeps/powerpc/powerpc64/power8/strcmp.S b/sysdeps/powerpc/powerpc64/power8/strcmp.S -index 15e7351d1b..d592266d1d 100644 ---- a/sysdeps/powerpc/powerpc64/power8/strcmp.S -+++ b/sysdeps/powerpc/powerpc64/power8/strcmp.S -@@ -31,6 +31,7 @@ - 64K as default, the page cross handling assumes minimum page size of - 4k. */ - -+ .machine power8 - ENTRY_TOCLESS (STRCMP, 4) - li r0,0 - --- -2.41.0 diff --git a/contrib/guix/patches/glibc-2.27-riscv64-Use-__has_include-to-include-asm-syscalls.h.patch b/contrib/guix/patches/glibc-2.27-riscv64-Use-__has_include-to-include-asm-syscalls.h.patch deleted file mode 100644 index ab8ae9c023..0000000000 --- a/contrib/guix/patches/glibc-2.27-riscv64-Use-__has_include-to-include-asm-syscalls.h.patch +++ /dev/null @@ -1,78 +0,0 @@ -Note that this has been modified from the original commit, to use __has_include -instead of __has_include__, as the later was causing build failures with GCC 10. -See also: http://lists.busybox.net/pipermail/buildroot/2020-July/590376.html. - -https://sourceware.org/git/?p=glibc.git;a=commit;h=0b9c84906f653978fb8768c7ebd0ee14a47e662e - -This patch can be dropped when we are building with glibc 2.28+. - -From 562c52cc81a4e456a62e6455feb32732049e9070 Mon Sep 17 00:00:00 2001 -From: "H.J. Lu" -Date: Mon, 31 Dec 2018 09:26:42 -0800 -Subject: [PATCH] riscv: Use __has_include__ to include [BZ - #24022] - - has been removed by - -commit 27f8899d6002e11a6e2d995e29b8deab5aa9cc25 -Author: David Abdurachmanov -Date: Thu Nov 8 20:02:39 2018 +0100 - - riscv: add asm/unistd.h UAPI header - - Marcin Juszkiewicz reported issues while generating syscall table for riscv - using 4.20-rc1. The patch refactors our unistd.h files to match some other - architectures. - - - Add asm/unistd.h UAPI header, which has __ARCH_WANT_NEW_STAT only for 64-bit - - Remove asm/syscalls.h UAPI header and merge to asm/unistd.h - - Adjust kernel asm/unistd.h - - So now asm/unistd.h UAPI header should show all syscalls for riscv. - - may be restored by - -Subject: [PATCH] riscv: restore asm/syscalls.h UAPI header -Date: Tue, 11 Dec 2018 09:09:35 +0100 - -UAPI header asm/syscalls.h was merged into UAPI asm/unistd.h header, -which did resolve issue with missing syscalls macros resulting in -glibc (2.28) build failure. It also broke glibc in a different way: -asm/syscalls.h is being used by glibc. I noticed this while doing -Fedora 30/Rawhide mass rebuild. - -The patch returns asm/syscalls.h header and incl. it into asm/unistd.h. -I plan to send a patch to glibc to use asm/unistd.h instead of -asm/syscalls.h - -In the meantime, we use __has_include__, which was added to GCC 5, to -check if exists before including it. Tested with -build-many-glibcs.py for riscv against kernel 4.19.12 and 4.20-rc7. - - [BZ #24022] - * sysdeps/unix/sysv/linux/riscv/flush-icache.c: Check if - exists with __has_include__ before including it. ---- - sysdeps/unix/sysv/linux/riscv/flush-icache.c | 6 +++++- - 1 file changed, 5 insertions(+), 1 deletion(-) - -diff --git a/sysdeps/unix/sysv/linux/riscv/flush-icache.c b/sysdeps/unix/sysv/linux/riscv/flush-icache.c -index d612ef4c6c..0b2042620b 100644 ---- a/sysdeps/unix/sysv/linux/riscv/flush-icache.c -+++ b/sysdeps/unix/sysv/linux/riscv/flush-icache.c -@@ -21,7 +21,11 @@ - #include - #include - #include --#include -+#if __has_include () -+# include -+#else -+# include -+#endif - - typedef int (*func_type) (void *, void *, unsigned long int); - --- -2.31.1 - diff --git a/contrib/guix/patches/glibc-2.27-guix-prefix.patch b/contrib/guix/patches/glibc-guix-prefix.patch similarity index 78% rename from contrib/guix/patches/glibc-2.27-guix-prefix.patch rename to contrib/guix/patches/glibc-guix-prefix.patch index dc515907ff..60e12ca525 100644 --- a/contrib/guix/patches/glibc-2.27-guix-prefix.patch +++ b/contrib/guix/patches/glibc-guix-prefix.patch @@ -4,19 +4,13 @@ hash for the same package will differ when on different architectures. In order to be reproducible regardless of the architecture used to build the package, map all guix store prefixes to something fixed, e.g. /usr. -We might be able to drop this in favour of using --with-nonshared-cflags -when we begin using newer versions of glibc. - --- a/Makeconfig +++ b/Makeconfig -@@ -992,6 +992,10 @@ object-suffixes := +@@ -1007,6 +1007,7 @@ object-suffixes := CPPFLAGS-.o = $(pic-default) # libc.a must be compiled with -fPIE/-fpie for static PIE. CFLAGS-.o = $(filter %frame-pointer,$(+cflags)) $(pie-default) -+ -+# Map Guix store paths to /usr +CFLAGS-.o += `find /gnu/store -maxdepth 1 -mindepth 1 -type d -exec echo -n " -ffile-prefix-map={}=/usr" \;` -+ libtype.o := lib%.a object-suffixes += .o ifeq (yes,$(build-shared)) diff --git a/doc/dependencies.md b/doc/dependencies.md index fc574b6164..63c505c9cb 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -19,7 +19,7 @@ You can find installation instructions in the `build-*.md` file for your platfor | --- | --- | --- | --- | --- | | [Boost](../depends/packages/boost.mk) | [link](https://www.boost.org/users/download/) | [1.81.0](https://github.com/bitcoin/bitcoin/pull/26557) | [1.73.0](https://github.com/bitcoin/bitcoin/pull/29066) | No | | [libevent](../depends/packages/libevent.mk) | [link](https://github.com/libevent/libevent/releases) | [2.1.12-stable](https://github.com/bitcoin/bitcoin/pull/21991) | [2.1.8](https://github.com/bitcoin/bitcoin/pull/24681) | No | -| glibc | [link](https://www.gnu.org/software/libc/) | N/A | [2.27](https://github.com/bitcoin/bitcoin/pull/27029) | Yes | +| glibc | [link](https://www.gnu.org/software/libc/) | N/A | [2.31](https://github.com/bitcoin/bitcoin/pull/29987) | Yes | | Linux Kernel | [link](https://www.kernel.org/) | N/A | [3.17.0](https://github.com/bitcoin/bitcoin/pull/27699) | Yes | ## Optional diff --git a/doc/release-notes-29987.md b/doc/release-notes-29987.md new file mode 100644 index 0000000000..4d4c2358d1 --- /dev/null +++ b/doc/release-notes-29987.md @@ -0,0 +1,6 @@ +Compatibility +============= + +The minimum required glibc to run Bitcoin Core is now +2.31. This means that RHEL 8 and Ubuntu 18.04 (Bionic) +are no-longer supported. (#29987) \ No newline at end of file From 3d3a83fab2bcc5750e5c5854d121e943922fefd8 Mon Sep 17 00:00:00 2001 From: brunoerg Date: Mon, 8 Apr 2024 10:48:11 -0300 Subject: [PATCH 03/39] i2p: log errors properly according to their severity Co-authored-by: Vasil Dimov --- src/i2p.cpp | 24 +++++++++--------------- src/i2p.h | 8 -------- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/src/i2p.cpp b/src/i2p.cpp index 962adb124d..d30fa7258e 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -146,7 +146,7 @@ bool Session::Listen(Connection& conn) conn.sock = StreamAccept(); return true; } catch (const std::runtime_error& e) { - Log("Error listening: %s", e.what()); + LogPrintLevel(BCLog::I2P, BCLog::Level::Error, "Couldn't listen: %s\n", e.what()); CheckControlSock(); } return false; @@ -202,7 +202,7 @@ bool Session::Accept(Connection& conn) return true; } - Log("Error accepting%s: %s", disconnect ? " (will close the session)" : "", errmsg); + LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error accepting%s: %s\n", disconnect ? " (will close the session)" : "", errmsg); if (disconnect) { LOCK(m_mutex); Disconnect(); @@ -217,7 +217,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error) // Refuse connecting to arbitrary ports. We don't specify any destination port to the SAM proxy // when connecting (SAM 3.1 does not use ports) and it forces/defaults it to I2P_SAM31_PORT. if (to.GetPort() != I2P_SAM31_PORT) { - Log("Error connecting to %s, connection refused due to arbitrary port %s", to.ToStringAddrPort(), to.GetPort()); + LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error connecting to %s, connection refused due to arbitrary port %s\n", to.ToStringAddrPort(), to.GetPort()); proxy_error = false; return false; } @@ -265,7 +265,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error) throw std::runtime_error(strprintf("\"%s\"", connect_reply.full)); } catch (const std::runtime_error& e) { - Log("Error connecting to %s: %s", to.ToStringAddrPort(), e.what()); + LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error connecting to %s: %s\n", to.ToStringAddrPort(), e.what()); CheckControlSock(); return false; } @@ -283,12 +283,6 @@ std::string Session::Reply::Get(const std::string& key) const return pos->second.value(); } -template -void Session::Log(const std::string& fmt, const Args&... args) const -{ - LogPrint(BCLog::I2P, "%s\n", tfm::format(fmt, args...)); -} - Session::Reply Session::SendRequestAndGetReply(const Sock& sock, const std::string& request, bool check_result_ok) const @@ -344,7 +338,7 @@ void Session::CheckControlSock() std::string errmsg; if (m_control_sock && !m_control_sock->IsConnected(errmsg)) { - Log("Control socket error: %s", errmsg); + LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Control socket error: %s\n", errmsg); Disconnect(); } } @@ -414,7 +408,7 @@ void Session::CreateIfNotCreatedAlready() const auto session_type = m_transient ? "transient" : "persistent"; const auto session_id = GetRandHash().GetHex().substr(0, 10); // full is overkill, too verbose in the logs - Log("Creating %s SAM session %s with %s", session_type, session_id, m_control_host.ToString()); + LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Creating %s SAM session %s with %s\n", session_type, session_id, m_control_host.ToString()); auto sock = Hello(); @@ -451,7 +445,7 @@ void Session::CreateIfNotCreatedAlready() m_session_id = session_id; m_control_sock = std::move(sock); - Log("%s SAM session %s created, my address=%s", + LogPrintLevel(BCLog::I2P, BCLog::Level::Info, "%s SAM session %s created, my address=%s\n", Capitalize(session_type), m_session_id, m_my_addr.ToStringAddrPort()); @@ -482,9 +476,9 @@ void Session::Disconnect() { if (m_control_sock) { if (m_session_id.empty()) { - Log("Destroying incomplete SAM session"); + LogPrintLevel(BCLog::I2P, BCLog::Level::Info, "Destroying incomplete SAM session\n"); } else { - Log("Destroying SAM session %s", m_session_id); + LogPrintLevel(BCLog::I2P, BCLog::Level::Info, "Destroying SAM session %s\n", m_session_id); } m_control_sock.reset(); } diff --git a/src/i2p.h b/src/i2p.h index 8b0f1e1182..153263399d 100644 --- a/src/i2p.h +++ b/src/i2p.h @@ -156,14 +156,6 @@ class Session std::string Get(const std::string& key) const; }; - /** - * Log a message in the `BCLog::I2P` category. - * @param[in] fmt printf(3)-like format string. - * @param[in] args printf(3)-like arguments that correspond to `fmt`. - */ - template - void Log(const std::string& fmt, const Args&... args) const; - /** * Send request and get a reply from the SAM proxy. * @param[in] sock A socket that is connected to the SAM proxy. From 7d3662fbe35032178c5a5e27e73c592268f6e41b Mon Sep 17 00:00:00 2001 From: brunoerg Date: Mon, 8 Apr 2024 10:50:55 -0300 Subject: [PATCH 04/39] i2p: fix log when an interruption happens during `Accept` Before, interruption was printed as an error. Also, it did not log the reason when an interruption happened, e.g. "Error accepting:". Co-authored-by: Vasil Dimov --- src/i2p.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/i2p.cpp b/src/i2p.cpp index d30fa7258e..d85dc30003 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -202,7 +202,11 @@ bool Session::Accept(Connection& conn) return true; } - LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error accepting%s: %s\n", disconnect ? " (will close the session)" : "", errmsg); + if (*m_interrupt) { + LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Accept was interrupted\n"); + } else { + LogPrintLevel(BCLog::I2P, BCLog::Level::Debug, "Error accepting%s: %s\n", disconnect ? " (will close the session)" : "", errmsg); + } if (disconnect) { LOCK(m_mutex); Disconnect(); From adc6ab25bba42ce9e7ed6bd7599aabb6dead6987 Mon Sep 17 00:00:00 2001 From: josibake Date: Sun, 5 May 2024 14:16:08 +0200 Subject: [PATCH 05/39] wallet: use CRecipient instead of CTxOut Now that a CRecipient holds a CTxDestination, we can get the serialized size and determine if the output is dust using the CRecipient directly. This does not change any current behavior, but provides a nice generalization that can be used to apply special logic to a CTxDestination serialization and dust calculations in the future. Specifically, in a later PR when support for `V0SilentPayment` destinations is added, we need to use `WitnessV1Taproot` as the scriptPubKey for serialized size calcuations whenever the `CRecipient` destination is a `V0SilentPayment` destination. --- src/wallet/spend.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 4cbcfdb60f..39744f714a 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -976,6 +976,16 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng } } +size_t GetSerializeSizeForRecipient(const CRecipient& recipient) +{ + return ::GetSerializeSize(CTxOut(recipient.nAmount, GetScriptForDestination(recipient.dest))); +} + +bool IsDust(const CRecipient& recipient, const CFeeRate& dustRelayFee) +{ + return ::IsDust(CTxOut(recipient.nAmount, GetScriptForDestination(recipient.dest)), dustRelayFee); +} + static util::Result CreateTransactionInternal( CWallet& wallet, const std::vector& vecSend, @@ -1097,9 +1107,9 @@ static util::Result CreateTransactionInternal( CTxOut txout(recipient.nAmount, GetScriptForDestination(recipient.dest)); // Include the fee cost for outputs. - coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout); + coin_selection_params.tx_noinputs_size += GetSerializeSizeForRecipient(recipient); - if (IsDust(txout, wallet.chain().relayDustFee())) { + if (IsDust(recipient, wallet.chain().relayDustFee())) { return util::Error{_("Transaction amount too small")}; } txNew.vout.push_back(txout); From a9c7300135f0188daa5bce5491e2daf2dd8da8ae Mon Sep 17 00:00:00 2001 From: josibake Date: Sun, 5 May 2024 14:17:17 +0200 Subject: [PATCH 06/39] move-only: refactor CreateTransactionInternal Move the output serialization size and dust calculation into the loop where the outputs are iterated over to calculate the total sum. Move the code for adding the the txoutputs to the transaction to after coin selection. While this code structure generally follows a more logical flow, the primary motivation for moving the code for adding outputs to the transaction sets us up nicely for silent payments (in a future PR): we need to know the input set before generating the final output scriptPubKeys. --- src/wallet/spend.cpp | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 39744f714a..2e4d2d4334 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1008,12 +1008,20 @@ static util::Result CreateTransactionInternal( // Set the long term feerate estimate to the wallet's consolidate feerate coin_selection_params.m_long_term_feerate = wallet.m_consolidate_feerate; + // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 witness overhead (dummy, flag, stack size) + coin_selection_params.tx_noinputs_size = 10 + GetSizeOfCompactSize(vecSend.size()); // bytes for output count CAmount recipients_sum = 0; const OutputType change_type = wallet.TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : wallet.m_default_change_type, vecSend); ReserveDestination reservedest(&wallet, change_type); unsigned int outputs_to_subtract_fee_from = 0; // The number of outputs which we are subtracting the fee from for (const auto& recipient : vecSend) { + if (IsDust(recipient, wallet.chain().relayDustFee())) { + return util::Error{_("Transaction amount too small")}; + } + + // Include the fee cost for outputs. + coin_selection_params.tx_noinputs_size += GetSerializeSizeForRecipient(recipient); recipients_sum += recipient.nAmount; if (recipient.fSubtractFeeFromAmount) { @@ -1098,23 +1106,6 @@ static util::Result CreateTransactionInternal( const auto change_spend_fee = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size); coin_selection_params.min_viable_change = std::max(change_spend_fee + 1, dust); - // Static vsize overhead + outputs vsize. 4 version, 4 nLocktime, 1 input count, 1 witness overhead (dummy, flag, stack size) - coin_selection_params.tx_noinputs_size = 10 + GetSizeOfCompactSize(vecSend.size()); // bytes for output count - - // vouts to the payees - for (const auto& recipient : vecSend) - { - CTxOut txout(recipient.nAmount, GetScriptForDestination(recipient.dest)); - - // Include the fee cost for outputs. - coin_selection_params.tx_noinputs_size += GetSerializeSizeForRecipient(recipient); - - if (IsDust(recipient, wallet.chain().relayDustFee())) { - return util::Error{_("Transaction amount too small")}; - } - txNew.vout.push_back(txout); - } - // Include the fees for things that aren't inputs, excluding the change output const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.m_subtract_fee_outputs ? 0 : coin_selection_params.tx_noinputs_size); CAmount selection_target = recipients_sum + not_input_fees; @@ -1155,6 +1146,11 @@ static util::Result CreateTransactionInternal( result.GetWaste(), result.GetSelectedValue()); + // vouts to the payees + for (const auto& recipient : vecSend) + { + txNew.vout.emplace_back(recipient.nAmount, GetScriptForDestination(recipient.dest)); + } const CAmount change_amount = result.GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee); if (change_amount > 0) { CTxOut newTxOut(change_amount, scriptChange); From 8ecb6816781c7c7f423b501cbb2de3abd7250119 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 7 Jun 2024 11:22:44 +0200 Subject: [PATCH 07/39] Introduce Mining interface Start out with a single method isTestChain() that's used by the getblocktemplate RPC. --- doc/developer-notes.md | 5 +++-- src/Makefile.am | 1 + src/init.cpp | 2 ++ src/init/bitcoin-node.cpp | 1 + src/init/bitcoin-qt.cpp | 2 ++ src/init/bitcoind.cpp | 2 ++ src/interfaces/init.h | 2 ++ src/interfaces/mining.h | 35 +++++++++++++++++++++++++++++++++++ src/node/context.cpp | 1 + src/node/context.h | 2 ++ src/node/interfaces.cpp | 18 ++++++++++++++++++ src/rpc/mining.cpp | 5 ++++- src/rpc/server_util.cpp | 8 ++++++++ src/rpc/server_util.h | 4 ++++ 14 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 src/interfaces/mining.h diff --git a/doc/developer-notes.md b/doc/developer-notes.md index eb2bb41aa4..d9d5b392c5 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -1457,8 +1457,9 @@ independent (node, wallet, GUI), are defined in there are [`interfaces::Chain`](../src/interfaces/chain.h), used by wallet to access the node's latest chain state, [`interfaces::Node`](../src/interfaces/node.h), used by the GUI to control the -node, and [`interfaces::Wallet`](../src/interfaces/wallet.h), used by the GUI -to control an individual wallet. There are also more specialized interface +node, [`interfaces::Wallet`](../src/interfaces/wallet.h), used by the GUI +to control an individual wallet and [`interfaces::Mining`](../src/interfaces/mining.h), +used by RPC to generate block templates. There are also more specialized interface types like [`interfaces::Handler`](../src/interfaces/handler.h) [`interfaces::ChainClient`](../src/interfaces/chain.h) passed to and from various interface methods. diff --git a/src/Makefile.am b/src/Makefile.am index 4a1973aa87..f95a6fa123 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -177,6 +177,7 @@ BITCOIN_CORE_H = \ interfaces/handler.h \ interfaces/init.h \ interfaces/ipc.h \ + interfaces/mining.h \ interfaces/node.h \ interfaces/wallet.h \ kernel/blockmanager_opts.h \ diff --git a/src/init.cpp b/src/init.cpp index 5bb82dc320..0c57dfea7f 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -1117,6 +1118,7 @@ bool AppInitLockDataDirectory() bool AppInitInterfaces(NodeContext& node) { node.chain = node.init->makeChain(); + node.mining = node.init->makeMining(); return true; } diff --git a/src/init/bitcoin-node.cpp b/src/init/bitcoin-node.cpp index 97b8dc1161..00a3822791 100644 --- a/src/init/bitcoin-node.cpp +++ b/src/init/bitcoin-node.cpp @@ -30,6 +30,7 @@ class BitcoinNodeInit : public interfaces::Init } std::unique_ptr makeNode() override { return interfaces::MakeNode(m_node); } std::unique_ptr makeChain() override { return interfaces::MakeChain(m_node); } + std::unique_ptr makeMining() override { return interfaces::MakeMining(m_node); } std::unique_ptr makeWalletLoader(interfaces::Chain& chain) override { return MakeWalletLoader(chain, *Assert(m_node.args)); diff --git a/src/init/bitcoin-qt.cpp b/src/init/bitcoin-qt.cpp index 3003a8fde1..5209c72973 100644 --- a/src/init/bitcoin-qt.cpp +++ b/src/init/bitcoin-qt.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ class BitcoinQtInit : public interfaces::Init } std::unique_ptr makeNode() override { return interfaces::MakeNode(m_node); } std::unique_ptr makeChain() override { return interfaces::MakeChain(m_node); } + std::unique_ptr makeMining() override { return interfaces::MakeMining(m_node); } std::unique_ptr makeWalletLoader(interfaces::Chain& chain) override { return MakeWalletLoader(chain, *Assert(m_node.args)); diff --git a/src/init/bitcoind.cpp b/src/init/bitcoind.cpp index b5df764017..48be8831d2 100644 --- a/src/init/bitcoind.cpp +++ b/src/init/bitcoind.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -27,6 +28,7 @@ class BitcoindInit : public interfaces::Init } std::unique_ptr makeNode() override { return interfaces::MakeNode(m_node); } std::unique_ptr makeChain() override { return interfaces::MakeChain(m_node); } + std::unique_ptr makeMining() override { return interfaces::MakeMining(m_node); } std::unique_ptr makeWalletLoader(interfaces::Chain& chain) override { return MakeWalletLoader(chain, *Assert(m_node.args)); diff --git a/src/interfaces/init.h b/src/interfaces/init.h index addc45aa26..094ead399d 100644 --- a/src/interfaces/init.h +++ b/src/interfaces/init.h @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -32,6 +33,7 @@ class Init virtual ~Init() = default; virtual std::unique_ptr makeNode() { return nullptr; } virtual std::unique_ptr makeChain() { return nullptr; } + virtual std::unique_ptr makeMining() { return nullptr; } virtual std::unique_ptr makeWalletLoader(Chain& chain) { return nullptr; } virtual std::unique_ptr makeEcho() { return nullptr; } virtual Ipc* ipc() { return nullptr; } diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h new file mode 100644 index 0000000000..afcd8d1cda --- /dev/null +++ b/src/interfaces/mining.h @@ -0,0 +1,35 @@ +// Copyright (c) 2024 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_INTERFACES_MINING_H +#define BITCOIN_INTERFACES_MINING_H + +namespace node { +struct NodeContext; +} // namespace node + +namespace interfaces { + +//! Interface giving clients (RPC, Stratum v2 Template Provider in the future) +//! ability to create block templates. + +class Mining +{ +public: + virtual ~Mining() {} + + //! If this chain is exclusively used for testing + virtual bool isTestChain() = 0; + + //! Get internal node context. Useful for RPC and testing, + //! but not accessible across processes. + virtual node::NodeContext* context() { return nullptr; } +}; + +//! Return implementation of Mining interface. +std::unique_ptr MakeMining(node::NodeContext& node); + +} // namespace interfaces + +#endif // BITCOIN_INTERFACES_MINING_H diff --git a/src/node/context.cpp b/src/node/context.cpp index da05fde6ee..75dfaee866 100644 --- a/src/node/context.cpp +++ b/src/node/context.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include diff --git a/src/node/context.h b/src/node/context.h index 77838ea99b..a664fad80b 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -27,6 +27,7 @@ class PeerManager; namespace interfaces { class Chain; class ChainClient; +class Mining; class Init; class WalletLoader; } // namespace interfaces @@ -74,6 +75,7 @@ struct NodeContext { std::vector> chain_clients; //! Reference to chain client that should used to load or create wallets //! opened by the gui. + std::unique_ptr mining; interfaces::WalletLoader* wallet_loader{nullptr}; std::unique_ptr scheduler; std::function rpc_interruption_point = [] {}; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 2b36f4ceae..e44cb51873 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -69,6 +70,7 @@ using interfaces::Chain; using interfaces::FoundBlock; using interfaces::Handler; using interfaces::MakeSignalHandler; +using interfaces::Mining; using interfaces::Node; using interfaces::WalletLoader; using util::Join; @@ -831,10 +833,26 @@ class ChainImpl : public Chain ValidationSignals& validation_signals() { return *Assert(m_node.validation_signals); } NodeContext& m_node; }; + +class MinerImpl : public Mining +{ +public: + explicit MinerImpl(NodeContext& node) : m_node(node) {} + + bool isTestChain() override + { + return chainman().GetParams().IsTestChain(); + } + + NodeContext* context() override { return &m_node; } + ChainstateManager& chainman() { return *Assert(m_node.chainman); } + NodeContext& m_node; +}; } // namespace } // namespace node namespace interfaces { std::unique_ptr MakeNode(node::NodeContext& context) { return std::make_unique(context); } std::unique_ptr MakeChain(node::NodeContext& context) { return std::make_unique(context); } +std::unique_ptr MakeMining(node::NodeContext& context) { return std::make_unique(context); } } // namespace interfaces diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 0f6853ef37..0e9d4829d2 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -45,6 +46,7 @@ using node::BlockAssembler; using node::CBlockTemplate; +using interfaces::Mining; using node::NodeContext; using node::RegenerateCommitments; using node::UpdateTime; @@ -724,7 +726,8 @@ static RPCHelpMan getblocktemplate() if (strMode != "template") throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid mode"); - if (!chainman.GetParams().IsTestChain()) { + Mining& miner = EnsureMining(node); + if (!miner.isTestChain()) { const CConnman& connman = EnsureConnman(node); if (connman.GetNodeCount(ConnectionDirection::Both) == 0) { throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, PACKAGE_NAME " is not connected!"); diff --git a/src/rpc/server_util.cpp b/src/rpc/server_util.cpp index efd4a43c28..0387cbb8e2 100644 --- a/src/rpc/server_util.cpp +++ b/src/rpc/server_util.cpp @@ -101,6 +101,14 @@ CConnman& EnsureConnman(const NodeContext& node) return *node.connman; } +interfaces::Mining& EnsureMining(const NodeContext& node) +{ + if (!node.mining) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Node miner not found"); + } + return *node.mining; +} + PeerManager& EnsurePeerman(const NodeContext& node) { if (!node.peerman) { diff --git a/src/rpc/server_util.h b/src/rpc/server_util.h index a4a53166b4..1e6fb7e6a6 100644 --- a/src/rpc/server_util.h +++ b/src/rpc/server_util.h @@ -18,6 +18,9 @@ class BanMan; namespace node { struct NodeContext; } // namespace node +namespace interfaces { +class Mining; +} // namespace interfaces node::NodeContext& EnsureAnyNodeContext(const std::any& context); CTxMemPool& EnsureMemPool(const node::NodeContext& node); @@ -31,6 +34,7 @@ ChainstateManager& EnsureAnyChainman(const std::any& context); CBlockPolicyEstimator& EnsureFeeEstimator(const node::NodeContext& node); CBlockPolicyEstimator& EnsureAnyFeeEstimator(const std::any& context); CConnman& EnsureConnman(const node::NodeContext& node); +interfaces::Mining& EnsureMining(const node::NodeContext& node); PeerManager& EnsurePeerman(const node::NodeContext& node); AddrMan& EnsureAddrman(const node::NodeContext& node); AddrMan& EnsureAnyAddrman(const std::any& context); From d8a3496b5ad27bea4c79ea0344f595cc1b95f0d3 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 10 Jun 2024 17:58:13 +0200 Subject: [PATCH 08/39] rpc: call TestBlockValidity via miner interface --- src/interfaces/mining.h | 15 +++++++++++++++ src/node/interfaces.cpp | 7 +++++++ src/rpc/mining.cpp | 14 ++++++++------ test/functional/rpc_generate.py | 2 +- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index afcd8d1cda..603d8475d3 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -9,6 +9,9 @@ namespace node { struct NodeContext; } // namespace node +class BlockValidationState; +class CBlock; + namespace interfaces { //! Interface giving clients (RPC, Stratum v2 Template Provider in the future) @@ -22,6 +25,18 @@ class Mining //! If this chain is exclusively used for testing virtual bool isTestChain() = 0; + /** + * Check a block is completely valid from start to finish. + * Only works on top of our current best block. + * Does not check proof-of-work. + * + * @param[out] state details of why a block failed to validate + * @param[in] block the block to validate + * @param[in] check_merkle_root call CheckMerkleRoot() + * @returns false if any of the checks fail + */ + virtual bool testBlockValidity(BlockValidationState& state, const CBlock& block, bool check_merkle_root = true) = 0; + //! Get internal node context. Useful for RPC and testing, //! but not accessible across processes. virtual node::NodeContext* context() { return nullptr; } diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index e44cb51873..a528afcff0 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -844,6 +845,12 @@ class MinerImpl : public Mining return chainman().GetParams().IsTestChain(); } + bool testBlockValidity(BlockValidationState& state, const CBlock& block, bool check_merkle_root) override + { + LOCK(::cs_main); + return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, chainman().ActiveChain().Tip(), /*fCheckPOW=*/false, check_merkle_root); + } + NodeContext* context() override { return &m_node; } ChainstateManager& chainman() { return *Assert(m_node.chainman); } NodeContext& m_node; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 0e9d4829d2..a9968b6e5f 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -339,6 +339,7 @@ static RPCHelpMan generateblock() } NodeContext& node = EnsureAnyNodeContext(request.context); + Mining& miner = EnsureMining(node); const CTxMemPool& mempool = EnsureMemPool(node); std::vector txs; @@ -389,8 +390,8 @@ static RPCHelpMan generateblock() LOCK(cs_main); BlockValidationState state; - if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), false, false)) { - throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.ToString())); + if (!miner.testBlockValidity(state, block, /*check_merkle_root=*/false)) { + throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("testBlockValidity failed: %s", state.ToString())); } } @@ -664,6 +665,7 @@ static RPCHelpMan getblocktemplate() { NodeContext& node = EnsureAnyNodeContext(request.context); ChainstateManager& chainman = EnsureChainman(node); + Mining& miner = EnsureMining(node); LOCK(cs_main); std::string strMode = "template"; @@ -706,11 +708,12 @@ static RPCHelpMan getblocktemplate() } CBlockIndex* const pindexPrev = active_chain.Tip(); - // TestBlockValidity only supports blocks built on the current Tip - if (block.hashPrevBlock != pindexPrev->GetBlockHash()) + // testBlockValidity only supports blocks built on the current Tip + if (block.hashPrevBlock != pindexPrev->GetBlockHash()) { return "inconclusive-not-best-prevblk"; + } BlockValidationState state; - TestBlockValidity(state, chainman.GetParams(), active_chainstate, block, pindexPrev, false, true); + miner.testBlockValidity(state, block); return BIP22ValidationResult(state); } @@ -726,7 +729,6 @@ static RPCHelpMan getblocktemplate() if (strMode != "template") throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid mode"); - Mining& miner = EnsureMining(node); if (!miner.isTestChain()) { const CConnman& connman = EnsureConnman(node); if (connman.GetNodeCount(ConnectionDirection::Both) == 0) { diff --git a/test/functional/rpc_generate.py b/test/functional/rpc_generate.py index 20f62079fd..3e250925e7 100755 --- a/test/functional/rpc_generate.py +++ b/test/functional/rpc_generate.py @@ -87,7 +87,7 @@ def test_generateblock(self): txid1 = miniwallet.send_self_transfer(from_node=node)['txid'] utxo1 = miniwallet.get_utxo(txid=txid1) rawtx2 = miniwallet.create_self_transfer(utxo_to_spend=utxo1)['hex'] - assert_raises_rpc_error(-25, 'TestBlockValidity failed: bad-txns-inputs-missingorspent', self.generateblock, node, address, [rawtx2, txid1]) + assert_raises_rpc_error(-25, 'testBlockValidity failed: bad-txns-inputs-missingorspent', self.generateblock, node, address, [rawtx2, txid1]) self.log.info('Fail to generate block with txid not in mempool') missing_txid = '0000000000000000000000000000000000000000000000000000000000000000' From 404b01c436122b951e9e06ed26d79dba4651685e Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 30 May 2024 15:55:02 +0200 Subject: [PATCH 09/39] rpc: getblocktemplate getTipHash() via Miner interface --- src/interfaces/mining.h | 5 +++++ src/node/interfaces.cpp | 8 ++++++++ src/rpc/mining.cpp | 12 +++++------- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 603d8475d3..8ed273252b 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_INTERFACES_MINING_H #define BITCOIN_INTERFACES_MINING_H +#include + namespace node { struct NodeContext; } // namespace node @@ -25,6 +27,9 @@ class Mining //! If this chain is exclusively used for testing virtual bool isTestChain() = 0; + //! Returns the hash for the tip of this chain, 0 if none + virtual uint256 getTipHash() = 0; + /** * Check a block is completely valid from start to finish. * Only works on top of our current best block. diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index a528afcff0..bd200e8d29 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -845,6 +845,14 @@ class MinerImpl : public Mining return chainman().GetParams().IsTestChain(); } + uint256 getTipHash() override + { + LOCK(::cs_main); + CBlockIndex* tip{chainman().ActiveChain().Tip()}; + if (!tip) return uint256{0}; + return tip->GetBlockHash(); + } + bool testBlockValidity(BlockValidationState& state, const CBlock& block, bool check_merkle_root) override { LOCK(::cs_main); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index a9968b6e5f..9762d7648d 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -672,7 +672,6 @@ static RPCHelpMan getblocktemplate() UniValue lpval = NullUniValue; std::set setClientRules; Chainstate& active_chainstate = chainman.ActiveChainstate(); - CChain& active_chain = active_chainstate.m_chain; if (!request.params[0].isNull()) { const UniValue& oparam = request.params[0].get_obj(); @@ -707,9 +706,8 @@ static RPCHelpMan getblocktemplate() return "duplicate-inconclusive"; } - CBlockIndex* const pindexPrev = active_chain.Tip(); // testBlockValidity only supports blocks built on the current Tip - if (block.hashPrevBlock != pindexPrev->GetBlockHash()) { + if (block.hashPrevBlock != miner.getTipHash()) { return "inconclusive-not-best-prevblk"; } BlockValidationState state; @@ -761,7 +759,7 @@ static RPCHelpMan getblocktemplate() else { // NOTE: Spec does not specify behaviour for non-string longpollid, but this makes testing easier - hashWatchedChain = active_chain.Tip()->GetBlockHash(); + hashWatchedChain = miner.getTipHash(); nTransactionsUpdatedLastLP = nTransactionsUpdatedLast; } @@ -806,7 +804,7 @@ static RPCHelpMan getblocktemplate() static CBlockIndex* pindexPrev; static int64_t time_start; static std::unique_ptr pblocktemplate; - if (pindexPrev != active_chain.Tip() || + if (!pindexPrev || pindexPrev->GetBlockHash() != miner.getTipHash() || (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - time_start > 5)) { // Clear pindexPrev so future calls make a new block, despite any failures from here on @@ -814,7 +812,7 @@ static RPCHelpMan getblocktemplate() // Store the pindexBest used before CreateNewBlock, to avoid races nTransactionsUpdatedLast = mempool.GetTransactionsUpdated(); - CBlockIndex* pindexPrevNew = active_chain.Tip(); + CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(miner.getTipHash()); time_start = GetTime(); // Create new block @@ -946,7 +944,7 @@ static RPCHelpMan getblocktemplate() result.pushKV("transactions", std::move(transactions)); result.pushKV("coinbaseaux", std::move(aux)); result.pushKV("coinbasevalue", (int64_t)pblock->vtx[0]->vout[0].nValue); - result.pushKV("longpollid", active_chain.Tip()->GetBlockHash().GetHex() + ToString(nTransactionsUpdatedLast)); + result.pushKV("longpollid", miner.getTipHash().GetHex() + ToString(nTransactionsUpdatedLast)); result.pushKV("target", hashTarget.GetHex()); result.pushKV("mintime", (int64_t)pindexPrev->GetMedianTimePast()+1); result.pushKV("mutable", std::move(aMutable)); From 4bf2e361da1964f7c278b4939967a0e5afde20b0 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 10 Jun 2024 17:03:33 +0200 Subject: [PATCH 10/39] rpc: call CreateNewBlock via miner interface --- src/interfaces/mining.h | 11 +++++++++++ src/node/interfaces.cpp | 7 +++++++ src/rpc/mining.cpp | 24 ++++++++++++------------ 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 8ed273252b..9494fd75bf 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -8,11 +8,13 @@ #include namespace node { +struct CBlockTemplate; struct NodeContext; } // namespace node class BlockValidationState; class CBlock; +class CScript; namespace interfaces { @@ -30,6 +32,15 @@ class Mining //! Returns the hash for the tip of this chain, 0 if none virtual uint256 getTipHash() = 0; + /** + * Construct a new block template + * + * @param[in] script_pub_key the coinbase output + * @param[in] use_mempool set false to omit mempool transactions + * @returns a block template + */ + virtual std::unique_ptr createNewBlock(const CScript& script_pub_key, bool use_mempool = true) = 0; + /** * Check a block is completely valid from start to finish. * Only works on top of our current best block. diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index bd200e8d29..215dbf5e17 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -859,6 +860,12 @@ class MinerImpl : public Mining return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, chainman().ActiveChain().Tip(), /*fCheckPOW=*/false, check_merkle_root); } + std::unique_ptr createNewBlock(const CScript& script_pub_key, bool use_mempool) override + { + LOCK(::cs_main); + return BlockAssembler{chainman().ActiveChainstate(), use_mempool ? context()->mempool.get() : nullptr}.CreateNewBlock(script_pub_key); + } + NodeContext* context() override { return &m_node; } ChainstateManager& chainman() { return *Assert(m_node.chainman); } NodeContext& m_node; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 9762d7648d..a65413de1e 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -156,11 +156,11 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& return true; } -static UniValue generateBlocks(ChainstateManager& chainman, const CTxMemPool& mempool, const CScript& coinbase_script, int nGenerate, uint64_t nMaxTries) +static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const CScript& coinbase_script, int nGenerate, uint64_t nMaxTries) { UniValue blockHashes(UniValue::VARR); while (nGenerate > 0 && !chainman.m_interrupt) { - std::unique_ptr pblocktemplate(BlockAssembler{chainman.ActiveChainstate(), &mempool}.CreateNewBlock(coinbase_script)); + std::unique_ptr pblocktemplate(miner.createNewBlock(coinbase_script)); if (!pblocktemplate.get()) throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block"); @@ -241,10 +241,10 @@ static RPCHelpMan generatetodescriptor() } NodeContext& node = EnsureAnyNodeContext(request.context); - const CTxMemPool& mempool = EnsureMemPool(node); + Mining& miner = EnsureMining(node); ChainstateManager& chainman = EnsureChainman(node); - return generateBlocks(chainman, mempool, coinbase_script, num_blocks, max_tries); + return generateBlocks(chainman, miner, coinbase_script, num_blocks, max_tries); }, }; } @@ -287,12 +287,12 @@ static RPCHelpMan generatetoaddress() } NodeContext& node = EnsureAnyNodeContext(request.context); - const CTxMemPool& mempool = EnsureMemPool(node); + Mining& miner = EnsureMining(node); ChainstateManager& chainman = EnsureChainman(node); CScript coinbase_script = GetScriptForDestination(destination); - return generateBlocks(chainman, mempool, coinbase_script, num_blocks, max_tries); + return generateBlocks(chainman, miner, coinbase_script, num_blocks, max_tries); }, }; } @@ -373,7 +373,7 @@ static RPCHelpMan generateblock() { LOCK(cs_main); - std::unique_ptr blocktemplate(BlockAssembler{chainman.ActiveChainstate(), nullptr}.CreateNewBlock(coinbase_script)); + std::unique_ptr blocktemplate{miner.createNewBlock(coinbase_script, /*use_mempool=*/false)}; if (!blocktemplate) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block"); } @@ -671,7 +671,6 @@ static RPCHelpMan getblocktemplate() std::string strMode = "template"; UniValue lpval = NullUniValue; std::set setClientRules; - Chainstate& active_chainstate = chainman.ActiveChainstate(); if (!request.params[0].isNull()) { const UniValue& oparam = request.params[0].get_obj(); @@ -810,18 +809,19 @@ static RPCHelpMan getblocktemplate() // Clear pindexPrev so future calls make a new block, despite any failures from here on pindexPrev = nullptr; - // Store the pindexBest used before CreateNewBlock, to avoid races + // Store the pindexBest used before createNewBlock, to avoid races nTransactionsUpdatedLast = mempool.GetTransactionsUpdated(); CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(miner.getTipHash()); time_start = GetTime(); // Create new block CScript scriptDummy = CScript() << OP_TRUE; - pblocktemplate = BlockAssembler{active_chainstate, &mempool}.CreateNewBlock(scriptDummy); - if (!pblocktemplate) + pblocktemplate = miner.createNewBlock(scriptDummy); + if (!pblocktemplate) { throw JSONRPCError(RPC_OUT_OF_MEMORY, "Out of memory"); + } - // Need to update only after we know CreateNewBlock succeeded + // Need to update only after we know createNewBlock succeeded pindexPrev = pindexPrevNew; } CHECK_NONFATAL(pindexPrev); From 64ebb0f97178687517c2060bf6b9931064607888 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 10 Jun 2024 17:27:12 +0200 Subject: [PATCH 11/39] Always pass options to BlockAssembler constructor This makes the options argument for BlockAssembler constructor mandatory, dropping implicit use of ArgsManager. The caller i.e. the Mining interface implementation now handles this. In a future Stratum v2 change the Options object needs to be mofified after arguments have been processed. Specifically the pool communicates how many extra bytes it needs for its own outputs (payouts, extra commitments, etc). This will need to be substracted from what the user set as -blockmaxweight. Such a change can be implemented in createNewBlock, after ApplyArgsManOptions. --- src/node/interfaces.cpp | 6 +++++- src/node/miner.cpp | 9 --------- src/node/miner.h | 1 - src/test/blockfilter_index_tests.cpp | 3 ++- src/test/peerman_tests.cpp | 3 ++- src/test/util/setup_common.cpp | 3 ++- src/test/validation_block_tests.cpp | 6 ++++-- 7 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 215dbf5e17..356a01b286 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -75,6 +75,7 @@ using interfaces::MakeSignalHandler; using interfaces::Mining; using interfaces::Node; using interfaces::WalletLoader; +using node::BlockAssembler; using util::Join; namespace node { @@ -862,8 +863,11 @@ class MinerImpl : public Mining std::unique_ptr createNewBlock(const CScript& script_pub_key, bool use_mempool) override { + BlockAssembler::Options options; + ApplyArgsManOptions(gArgs, options); + LOCK(::cs_main); - return BlockAssembler{chainman().ActiveChainstate(), use_mempool ? context()->mempool.get() : nullptr}.CreateNewBlock(script_pub_key); + return BlockAssembler{chainman().ActiveChainstate(), use_mempool ? context()->mempool.get() : nullptr, options}.CreateNewBlock(script_pub_key); } NodeContext* context() override { return &m_node; } diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 87f40e993f..03c6d74deb 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -80,15 +80,6 @@ void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& optio if (const auto parsed{ParseMoney(*blockmintxfee)}) options.blockMinFeeRate = CFeeRate{*parsed}; } } -static BlockAssembler::Options ConfiguredOptions() -{ - BlockAssembler::Options options; - ApplyArgsManOptions(gArgs, options); - return options; -} - -BlockAssembler::BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool) - : BlockAssembler(chainstate, mempool, ConfiguredOptions()) {} void BlockAssembler::resetBlock() { diff --git a/src/node/miner.h b/src/node/miner.h index 06a917228d..c3178a7532 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -161,7 +161,6 @@ class BlockAssembler bool test_block_validity{true}; }; - explicit BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool); explicit BlockAssembler(Chainstate& chainstate, const CTxMemPool* mempool, const Options& options); /** Construct a new block template with coinbase to scriptPubKeyIn */ diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index d44d84af93..067a32d6a4 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -67,7 +67,8 @@ CBlock BuildChainTestingSetup::CreateBlock(const CBlockIndex* prev, const std::vector& txns, const CScript& scriptPubKey) { - std::unique_ptr pblocktemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get()}.CreateNewBlock(scriptPubKey); + BlockAssembler::Options options; + std::unique_ptr pblocktemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(scriptPubKey); CBlock& block = pblocktemplate->block; block.hashPrevBlock = prev->GetBlockHash(); block.nTime = prev->nTime + 1; diff --git a/src/test/peerman_tests.cpp b/src/test/peerman_tests.cpp index 397b1d8c2d..6de373eef2 100644 --- a/src/test/peerman_tests.cpp +++ b/src/test/peerman_tests.cpp @@ -20,7 +20,8 @@ static void mineBlock(const node::NodeContext& node, std::chrono::seconds block_ { auto curr_time = GetTime(); SetMockTime(block_time); // update time so the block is created with it - CBlock block = node::BlockAssembler{node.chainman->ActiveChainstate(), nullptr}.CreateNewBlock(CScript() << OP_TRUE)->block; + node::BlockAssembler::Options options; + CBlock block = node::BlockAssembler{node.chainman->ActiveChainstate(), nullptr, options}.CreateNewBlock(CScript() << OP_TRUE)->block; while (!CheckProofOfWork(block.GetHash(), block.nBits, node.chainman->GetConsensus())) ++block.nNonce; block.fChecked = true; // little speedup SetMockTime(curr_time); // process block at current time diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 79e33eacec..cc7b2d6546 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -374,7 +374,8 @@ CBlock TestChain100Setup::CreateBlock( const CScript& scriptPubKey, Chainstate& chainstate) { - CBlock block = BlockAssembler{chainstate, nullptr}.CreateNewBlock(scriptPubKey)->block; + BlockAssembler::Options options; + CBlock block = BlockAssembler{chainstate, nullptr, options}.CreateNewBlock(scriptPubKey)->block; Assert(block.vtx.size() == 1); for (const CMutableTransaction& tx : txns) { diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 69f4e305ab..588ac60498 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -65,7 +65,8 @@ std::shared_ptr MinerTestingSetup::Block(const uint256& prev_hash) static int i = 0; static uint64_t time = Params().GenesisBlock().nTime; - auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get()}.CreateNewBlock(CScript{} << i++ << OP_TRUE); + BlockAssembler::Options options; + auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(CScript{} << i++ << OP_TRUE); auto pblock = std::make_shared(ptemplate->block); pblock->hashPrevBlock = prev_hash; pblock->nTime = ++time; @@ -329,7 +330,8 @@ BOOST_AUTO_TEST_CASE(witness_commitment_index) LOCK(Assert(m_node.chainman)->GetMutex()); CScript pubKey; pubKey << 1 << OP_TRUE; - auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get()}.CreateNewBlock(pubKey); + BlockAssembler::Options options; + auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(pubKey); CBlock pblock = ptemplate->block; CTxOut witness; From 9e228351e761d8d24413bbc4ac1610b4f3dec2bf Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 30 May 2024 16:46:29 +0200 Subject: [PATCH 12/39] rpc: getTransactionsUpdated via miner interface --- src/interfaces/mining.h | 4 ++++ src/node/interfaces.cpp | 5 +++++ src/rpc/mining.cpp | 7 +++---- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 9494fd75bf..3ebc48dffa 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -41,6 +41,10 @@ class Mining */ virtual std::unique_ptr createNewBlock(const CScript& script_pub_key, bool use_mempool = true) = 0; + //! Return the number of transaction updates in the mempool, + //! used to decide whether to make a new block template. + virtual unsigned int getTransactionsUpdated() = 0; + /** * Check a block is completely valid from start to finish. * Only works on top of our current best block. diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 356a01b286..91ee858597 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -855,6 +855,11 @@ class MinerImpl : public Mining return tip->GetBlockHash(); } + unsigned int getTransactionsUpdated() override + { + return context()->mempool->GetTransactionsUpdated(); + } + bool testBlockValidity(BlockValidationState& state, const CBlock& block, bool check_merkle_root) override { LOCK(::cs_main); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index a65413de1e..3cca6a53fa 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -738,7 +738,6 @@ static RPCHelpMan getblocktemplate() } static unsigned int nTransactionsUpdatedLast; - const CTxMemPool& mempool = EnsureMemPool(node); if (!lpval.isNull()) { @@ -774,7 +773,7 @@ static RPCHelpMan getblocktemplate() { // Timeout: Check transactions for update // without holding the mempool lock to avoid deadlocks - if (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLastLP) + if (miner.getTransactionsUpdated() != nTransactionsUpdatedLastLP) break; checktxtime += std::chrono::seconds(10); } @@ -804,13 +803,13 @@ static RPCHelpMan getblocktemplate() static int64_t time_start; static std::unique_ptr pblocktemplate; if (!pindexPrev || pindexPrev->GetBlockHash() != miner.getTipHash() || - (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - time_start > 5)) + (miner.getTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - time_start > 5)) { // Clear pindexPrev so future calls make a new block, despite any failures from here on pindexPrev = nullptr; // Store the pindexBest used before createNewBlock, to avoid races - nTransactionsUpdatedLast = mempool.GetTransactionsUpdated(); + nTransactionsUpdatedLast = miner.getTransactionsUpdated(); CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(miner.getTipHash()); time_start = GetTime(); From 7b4d3249ced93ec5986500e43b324005ed89502f Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 30 May 2024 17:06:59 +0200 Subject: [PATCH 13/39] rpc: call processNewBlock via miner interface --- src/interfaces/mining.h | 8 ++++++++ src/node/interfaces.cpp | 5 +++++ src/rpc/mining.cpp | 13 ++++++++----- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 3ebc48dffa..cd092397f3 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -40,6 +40,14 @@ class Mining * @returns a block template */ virtual std::unique_ptr createNewBlock(const CScript& script_pub_key, bool use_mempool = true) = 0; + /** + * Processes new block. A valid new block is automatically relayed to peers. + * + * @param[in] block The block we want to process. + * @param[out] new_block A boolean which is set to indicate if the block was first received via this call + * @returns If the block was processed, independently of block validity + */ + virtual bool processNewBlock(const std::shared_ptr& block, bool* new_block) = 0; //! Return the number of transaction updates in the mempool, //! used to decide whether to make a new block template. diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 91ee858597..2633ff1b98 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -855,6 +855,11 @@ class MinerImpl : public Mining return tip->GetBlockHash(); } + bool processNewBlock(const std::shared_ptr& block, bool* new_block) override + { + return chainman().ProcessNewBlock(block, /*force_processing=*/true, /*min_pow_checked=*/true, /*new_block=*/new_block); + } + unsigned int getTransactionsUpdated() override { return context()->mempool->GetTransactionsUpdated(); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 3cca6a53fa..e404cea90f 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -129,7 +129,7 @@ static RPCHelpMan getnetworkhashps() }; } -static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& max_tries, std::shared_ptr& block_out, bool process_new_block) +static bool GenerateBlock(ChainstateManager& chainman, Mining& miner, CBlock& block, uint64_t& max_tries, std::shared_ptr& block_out, bool process_new_block) { block_out.reset(); block.hashMerkleRoot = BlockMerkleRoot(block); @@ -149,7 +149,7 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& if (!process_new_block) return true; - if (!chainman.ProcessNewBlock(block_out, /*force_processing=*/true, /*min_pow_checked=*/true, nullptr)) { + if (!miner.processNewBlock(block_out, nullptr)) { throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted"); } @@ -165,7 +165,7 @@ static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block"); std::shared_ptr block_out; - if (!GenerateBlock(chainman, pblocktemplate->block, nMaxTries, block_out, /*process_new_block=*/true)) { + if (!GenerateBlock(chainman, miner, pblocktemplate->block, nMaxTries, block_out, /*process_new_block=*/true)) { break; } @@ -398,7 +398,7 @@ static RPCHelpMan generateblock() std::shared_ptr block_out; uint64_t max_tries{DEFAULT_MAX_TRIES}; - if (!GenerateBlock(chainman, block, max_tries, block_out, process_new_block) || !block_out) { + if (!GenerateBlock(chainman, miner, block, max_tries, block_out, process_new_block) || !block_out) { throw JSONRPCError(RPC_MISC_ERROR, "Failed to make block."); } @@ -1049,10 +1049,13 @@ static RPCHelpMan submitblock() } } + NodeContext& node = EnsureAnyNodeContext(request.context); + Mining& miner = EnsureMining(node); + bool new_block; auto sc = std::make_shared(block.GetHash()); CHECK_NONFATAL(chainman.m_options.signals)->RegisterSharedValidationInterface(sc); - bool accepted = chainman.ProcessNewBlock(blockptr, /*force_processing=*/true, /*min_pow_checked=*/true, /*new_block=*/&new_block); + bool accepted = miner.processNewBlock(blockptr, /*new_block=*/&new_block); CHECK_NONFATAL(chainman.m_options.signals)->UnregisterSharedValidationInterface(sc); if (!new_block && accepted) { return "duplicate"; From dda0b0834faf7be7e8938bf63e7bb01cd54a416a Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 7 Jun 2024 11:51:18 +0200 Subject: [PATCH 14/39] rpc: minize getTipHash() calls in gbt Set tip at the start of the function and only update it for a long poll. Additionally have getTipHash return an optional, so the caller can explicitly check that a tip exists. --- src/interfaces/mining.h | 5 +++-- src/node/interfaces.cpp | 4 ++-- src/rpc/mining.cpp | 17 ++++++++++++----- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index cd092397f3..6e47333fd5 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_INTERFACES_MINING_H #define BITCOIN_INTERFACES_MINING_H +#include #include namespace node { @@ -29,8 +30,8 @@ class Mining //! If this chain is exclusively used for testing virtual bool isTestChain() = 0; - //! Returns the hash for the tip of this chain, 0 if none - virtual uint256 getTipHash() = 0; + //! Returns the hash for the tip of this chain + virtual std::optional getTipHash() = 0; /** * Construct a new block template diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 2633ff1b98..68c1c598cd 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -847,11 +847,11 @@ class MinerImpl : public Mining return chainman().GetParams().IsTestChain(); } - uint256 getTipHash() override + std::optional getTipHash() override { LOCK(::cs_main); CBlockIndex* tip{chainman().ActiveChain().Tip()}; - if (!tip) return uint256{0}; + if (!tip) return {}; return tip->GetBlockHash(); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index e404cea90f..9324ba4a1c 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -667,6 +667,9 @@ static RPCHelpMan getblocktemplate() ChainstateManager& chainman = EnsureChainman(node); Mining& miner = EnsureMining(node); LOCK(cs_main); + std::optional maybe_tip{miner.getTipHash()}; + CHECK_NONFATAL(maybe_tip); + uint256 tip{maybe_tip.value()}; std::string strMode = "template"; UniValue lpval = NullUniValue; @@ -706,7 +709,7 @@ static RPCHelpMan getblocktemplate() } // testBlockValidity only supports blocks built on the current Tip - if (block.hashPrevBlock != miner.getTipHash()) { + if (block.hashPrevBlock != tip) { return "inconclusive-not-best-prevblk"; } BlockValidationState state; @@ -757,7 +760,7 @@ static RPCHelpMan getblocktemplate() else { // NOTE: Spec does not specify behaviour for non-string longpollid, but this makes testing easier - hashWatchedChain = miner.getTipHash(); + hashWatchedChain = tip; nTransactionsUpdatedLastLP = nTransactionsUpdatedLast; } @@ -781,6 +784,10 @@ static RPCHelpMan getblocktemplate() } ENTER_CRITICAL_SECTION(cs_main); + std::optional maybe_tip{miner.getTipHash()}; + CHECK_NONFATAL(maybe_tip); + tip = maybe_tip.value(); + if (!IsRPCRunning()) throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, "Shutting down"); // TODO: Maybe recheck connections/IBD and (if something wrong) send an expires-immediately template to stop miners? @@ -802,7 +809,7 @@ static RPCHelpMan getblocktemplate() static CBlockIndex* pindexPrev; static int64_t time_start; static std::unique_ptr pblocktemplate; - if (!pindexPrev || pindexPrev->GetBlockHash() != miner.getTipHash() || + if (!pindexPrev || pindexPrev->GetBlockHash() != tip || (miner.getTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - time_start > 5)) { // Clear pindexPrev so future calls make a new block, despite any failures from here on @@ -810,7 +817,7 @@ static RPCHelpMan getblocktemplate() // Store the pindexBest used before createNewBlock, to avoid races nTransactionsUpdatedLast = miner.getTransactionsUpdated(); - CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(miner.getTipHash()); + CBlockIndex* pindexPrevNew = chainman.m_blockman.LookupBlockIndex(tip); time_start = GetTime(); // Create new block @@ -943,7 +950,7 @@ static RPCHelpMan getblocktemplate() result.pushKV("transactions", std::move(transactions)); result.pushKV("coinbaseaux", std::move(aux)); result.pushKV("coinbasevalue", (int64_t)pblock->vtx[0]->vout[0].nValue); - result.pushKV("longpollid", miner.getTipHash().GetHex() + ToString(nTransactionsUpdatedLast)); + result.pushKV("longpollid", tip.GetHex() + ToString(nTransactionsUpdatedLast)); result.pushKV("target", hashTarget.GetHex()); result.pushKV("mintime", (int64_t)pindexPrev->GetMedianTimePast()+1); result.pushKV("mutable", std::move(aMutable)); From a9716c53f05082d6d89ebea51a46d4404efb12d7 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 18 Jun 2024 21:07:51 +0200 Subject: [PATCH 15/39] rpc: call IsInitialBlockDownload via miner interface --- src/interfaces/mining.h | 3 +++ src/node/interfaces.cpp | 5 +++++ src/rpc/mining.cpp | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 6e47333fd5..b96881f67c 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -30,6 +30,9 @@ class Mining //! If this chain is exclusively used for testing virtual bool isTestChain() = 0; + //! Returns whether IBD is still in progress. + virtual bool isInitialBlockDownload() = 0; + //! Returns the hash for the tip of this chain virtual std::optional getTipHash() = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 68c1c598cd..e0bab6e22e 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -847,6 +847,11 @@ class MinerImpl : public Mining return chainman().GetParams().IsTestChain(); } + bool isInitialBlockDownload() override + { + return chainman().IsInitialBlockDownload(); + } + std::optional getTipHash() override { LOCK(::cs_main); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 9324ba4a1c..2b93c18965 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -735,7 +735,7 @@ static RPCHelpMan getblocktemplate() throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, PACKAGE_NAME " is not connected!"); } - if (chainman.IsInitialBlockDownload()) { + if (miner.isInitialBlockDownload()) { throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, PACKAGE_NAME " is in initial sync and waiting for blocks..."); } } From 197b5404b0f4a1d6e989000845b45c8bd24e0bc6 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 17 Jun 2024 14:10:53 +0000 Subject: [PATCH 16/39] QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core" --- test/functional/feature_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/feature_settings.py b/test/functional/feature_settings.py index 0214e781de..1cd0aeabd3 100755 --- a/test/functional/feature_settings.py +++ b/test/functional/feature_settings.py @@ -25,7 +25,7 @@ def run_test(self): # Assert default settings file was created self.stop_node(0) - default_settings = {"_warning_": "This file is automatically generated and updated by Bitcoin Core. Please do not edit this file while the node is running, as any changes might be ignored or overwritten."} + default_settings = {"_warning_": f"This file is automatically generated and updated by {self.config['environment']['PACKAGE_NAME']}. Please do not edit this file while the node is running, as any changes might be ignored or overwritten."} with settings.open() as fp: assert_equal(json.load(fp), default_settings) From 4289dd02cce688a69c596f7cd5e47f831b00aa1b Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 4 Jun 2024 13:06:25 +0100 Subject: [PATCH 17/39] contrib: add R(UN)PATH check to ELF symbol-check Our binaries shouldn't contains any rpaths, or runpaths, so check that at release time. --- contrib/devtools/symbol-check.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contrib/devtools/symbol-check.py b/contrib/devtools/symbol-check.py index 6613874ce3..1d487d4f6b 100755 --- a/contrib/devtools/symbol-check.py +++ b/contrib/devtools/symbol-check.py @@ -212,6 +212,11 @@ def check_exported_symbols(binary) -> bool: ok = False return ok +def check_RUNPATH(binary) -> bool: + assert binary.get(lief.ELF.DYNAMIC_TAGS.RUNPATH) is None + assert binary.get(lief.ELF.DYNAMIC_TAGS.RPATH) is None + return True + def check_ELF_libraries(binary) -> bool: ok: bool = True for library in binary.libraries: @@ -277,6 +282,7 @@ def check_ELF_ABI(binary) -> bool: ('LIBRARY_DEPENDENCIES', check_ELF_libraries), ('INTERPRETER_NAME', check_ELF_interpreter), ('ABI', check_ELF_ABI), + ('RUNPATH', check_RUNPATH), ], lief.EXE_FORMATS.MACHO: [ ('DYNAMIC_LIBRARIES', check_MACHO_libraries), From 4ecbbd9b7fa6f30e1d297cd26b112d3148744f58 Mon Sep 17 00:00:00 2001 From: Max Edwards Date: Wed, 19 Jun 2024 17:40:10 +0100 Subject: [PATCH 18/39] ci: add option for running tests without volume DANGER_CI_ON_HOST_CACHE_FOLDERS if set will mount caches in directories on the host rather than in docker volumes. Supports saving and restoring caches on Github Actions. --- .github/workflows/ci.yml | 4 ++++ ci/test/02_run_container.sh | 26 ++++++++++++++++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab314c47b6..54795332e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -313,6 +313,7 @@ jobs: timeout-minutes: 120 env: FILE_ENV: "./ci/test/00_setup_env_native_asan.sh" + DANGER_CI_ON_HOST_CACHE_FOLDERS: 1 steps: - name: Checkout uses: actions/checkout@v4 @@ -320,6 +321,9 @@ jobs: - name: Set Ccache directory run: echo "CCACHE_DIR=${RUNNER_TEMP}/ccache_dir" >> "$GITHUB_ENV" + - name: Set base root directory + run: echo "BASE_ROOT_DIR=${RUNNER_TEMP}" >> "$GITHUB_ENV" + - name: Restore Ccache cache id: ccache-cache uses: actions/cache/restore@v4 diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh index 17a940de07..afd447c347 100755 --- a/ci/test/02_run_container.sh +++ b/ci/test/02_run_container.sh @@ -30,6 +30,24 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then docker volume create "${CONTAINER_NAME}_depends_sources" || true docker volume create "${CONTAINER_NAME}_previous_releases" || true + CI_CCACHE_MOUNT="type=volume,src=${CONTAINER_NAME}_ccache,dst=$CCACHE_DIR" + CI_DEPENDS_MOUNT="type=volume,src=${CONTAINER_NAME}_depends,dst=$DEPENDS_DIR/built" + CI_DEPENDS_SOURCES_MOUNT="type=volume,src=${CONTAINER_NAME}_depends_sources,dst=$DEPENDS_DIR/sources" + CI_PREVIOUS_RELEASES_MOUNT="type=volume,src=${CONTAINER_NAME}_previous_releases,dst=$PREVIOUS_RELEASES_DIR" + + if [ "$DANGER_CI_ON_HOST_CACHE_FOLDERS" ]; then + # ensure the directories exist + mkdir -p "${CCACHE_DIR}" + mkdir -p "${DEPENDS_DIR}/built" + mkdir -p "${DEPENDS_DIR}/sources" + mkdir -p "${PREVIOUS_RELEASES_DIR}" + + CI_CCACHE_MOUNT="type=bind,src=${CCACHE_DIR},dst=$CCACHE_DIR" + CI_DEPENDS_MOUNT="type=bind,src=${DEPENDS_DIR}/built,dst=$DEPENDS_DIR/built" + CI_DEPENDS_SOURCES_MOUNT="type=bind,src=${DEPENDS_DIR}/sources,dst=$DEPENDS_DIR/sources" + CI_PREVIOUS_RELEASES_MOUNT="type=bind,src=${PREVIOUS_RELEASES_DIR},dst=$PREVIOUS_RELEASES_DIR" + fi + docker network create --ipv6 --subnet 1111:1111::/112 ci-ip6net || true if [ -n "${RESTART_CI_DOCKER_BEFORE_RUN}" ] ; then @@ -52,10 +70,10 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then # shellcheck disable=SC2086 CI_CONTAINER_ID=$(docker run --cap-add LINUX_IMMUTABLE $CI_CONTAINER_CAP --rm --interactive --detach --tty \ --mount "type=bind,src=$BASE_READ_ONLY_DIR,dst=$BASE_READ_ONLY_DIR,readonly" \ - --mount "type=volume,src=${CONTAINER_NAME}_ccache,dst=$CCACHE_DIR" \ - --mount "type=volume,src=${CONTAINER_NAME}_depends,dst=$DEPENDS_DIR/built" \ - --mount "type=volume,src=${CONTAINER_NAME}_depends_sources,dst=$DEPENDS_DIR/sources" \ - --mount "type=volume,src=${CONTAINER_NAME}_previous_releases,dst=$PREVIOUS_RELEASES_DIR" \ + --mount "${CI_CCACHE_MOUNT}" \ + --mount "${CI_DEPENDS_MOUNT}" \ + --mount "${CI_DEPENDS_SOURCES_MOUNT}" \ + --mount "${CI_PREVIOUS_RELEASES_MOUNT}" \ --env-file /tmp/env-$USER-$CONTAINER_NAME \ --name "$CONTAINER_NAME" \ --network ci-ip6net \ From 79e197b17536b52647599ad9b3f09d2556f14385 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 11 Jun 2024 15:50:21 -0400 Subject: [PATCH 19/39] build: Suppress warnings from boost and capnproto in multiprocess code Without this change there are errors from boost like: /ci_container_base/depends/i686-pc-linux-gnu/include/boost/signals2/expired_slot.hpp:23:28: error: 'what' overrides a member function but is not marked 'override' [-Werror,-Wsuggest-override] /ci_container_base/depends/i686-pc-linux-gnu/include/boost/signals2/detail/signal_template.hpp:750:32: error: 'lock_pimpl' overrides a member function but is not marked 'override' [-Werror,-Wsuggest-override] /ci_container_base/depends/i686-pc-linux-gnu/include/boost/signals2/connection.hpp:150:22: error: 'connected' overrides a member function but is not marked 'override' [-Werror,-Wsuggest-override] There do not seem to be errors from capnproto currently, but add a suppression for it, too, to be consistent with other libraries. --- configure.ac | 3 +++ src/Makefile.am | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ab369cc98a..6e040f5aa0 100644 --- a/configure.ac +++ b/configure.ac @@ -1392,6 +1392,9 @@ if test "$with_libmultiprocess" = "yes" || test "$with_libmultiprocess" = "auto" PKG_CHECK_MODULES([LIBMULTIPROCESS], [libmultiprocess], [ libmultiprocess_found=yes; libmultiprocess_prefix=`$PKG_CONFIG --variable=prefix libmultiprocess`; + if test "$suppress_external_warnings" != "no" ; then + LIBMULTIPROCESS_CFLAGS=SUPPRESS_WARNINGS($LIBMULTIPROCESS_CFLAGS) + fi ], [true]) elif test "$with_libmultiprocess" != "no"; then AC_MSG_ERROR([--with-libmultiprocess=$with_libmultiprocess value is not yes, auto, or no]) diff --git a/src/Makefile.am b/src/Makefile.am index 4a1973aa87..fa1612b5b8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1088,7 +1088,7 @@ libbitcoin_ipc_a_SOURCES = \ ipc/process.cpp \ ipc/process.h \ ipc/protocol.h -libbitcoin_ipc_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +libbitcoin_ipc_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) libbitcoin_ipc_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) $(LIBMULTIPROCESS_CFLAGS) include $(MPGEN_PREFIX)/include/mpgen.mk From 7839503b309c107e8229475a8fbf66601b0e7e8e Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 3 Apr 2024 15:45:47 +0100 Subject: [PATCH 20/39] zmq: use #ifdef ENABLE_ZMQ --- src/init.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 5bb82dc320..0f6a775a75 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -109,7 +109,7 @@ #include -#if ENABLE_ZMQ +#ifdef ENABLE_ZMQ #include #include #include @@ -364,7 +364,7 @@ void Shutdown(NodeContext& node) client->stop(); } -#if ENABLE_ZMQ +#ifdef ENABLE_ZMQ if (g_zmq_notification_interface) { if (node.validation_signals) node.validation_signals->UnregisterValidationInterface(g_zmq_notification_interface.get()); g_zmq_notification_interface.reset(); @@ -578,7 +578,7 @@ void SetupServerArgs(ArgsManager& argsman) g_wallet_init_interface.AddWalletOptions(argsman); -#if ENABLE_ZMQ +#ifdef ENABLE_ZMQ argsman.AddArg("-zmqpubhashblock=
", "Enable publish hash block in
", ArgsManager::ALLOW_ANY, OptionsCategory::ZMQ); argsman.AddArg("-zmqpubhashtx=
", "Enable publish hash transaction in
", ArgsManager::ALLOW_ANY, OptionsCategory::ZMQ); argsman.AddArg("-zmqpubrawblock=
", "Enable publish raw block in
", ArgsManager::ALLOW_ANY, OptionsCategory::ZMQ); @@ -1200,7 +1200,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) for (const auto& client : node.chain_clients) { client->registerRpcs(); } -#if ENABLE_ZMQ +#ifdef ENABLE_ZMQ RegisterZMQRPCCommands(tableRPC); #endif @@ -1472,7 +1472,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return InitError(ResolveErrMsg("externalip", strAddr)); } -#if ENABLE_ZMQ +#ifdef ENABLE_ZMQ g_zmq_notification_interface = CZMQNotificationInterface::Create( [&chainman = node.chainman](std::vector& block, const CBlockIndex& index) { assert(chainman); From 40cd7585a042938937b5964c9c264e2bf4a80742 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 4 Apr 2024 12:14:35 +0100 Subject: [PATCH 21/39] randomenv: use ifdef over if randomenv.cpp:48:5: warning: 'HAVE_VM_VM_PARAM_H' is not defined, evaluates to 0 [-Wundef] randomenv.cpp:51:5: warning: 'HAVE_SYS_RESOURCES_H' is not defined, evaluates to 0 [-Wundef] randomenv.cpp:424:5: error: 'HAVE_SYSCTL' is not defined, evaluates to 0 [-Werror,-Wundef] --- src/randomenv.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/randomenv.cpp b/src/randomenv.cpp index aeec959c28..49033deef2 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -42,15 +42,15 @@ #if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS #include #endif -#if HAVE_SYSCTL +#ifdef HAVE_SYSCTL #include -#if HAVE_VM_VM_PARAM_H +#ifdef HAVE_VM_VM_PARAM_H #include #endif -#if HAVE_SYS_RESOURCES_H +#ifdef HAVE_SYS_RESOURCES_H #include #endif -#if HAVE_SYS_VMMETER_H +#ifdef HAVE_SYS_VMMETER_H #include #endif #endif @@ -162,7 +162,7 @@ void AddPath(CSHA512& hasher, const char *path) } #endif -#if HAVE_SYSCTL +#ifdef HAVE_SYSCTL template void AddSysctl(CSHA512& hasher) { @@ -274,7 +274,7 @@ void RandAddDynamicEnv(CSHA512& hasher) AddFile(hasher, "/proc/self/status"); #endif -#if HAVE_SYSCTL +#ifdef HAVE_SYSCTL # ifdef CTL_KERN # if defined(KERN_PROC) && defined(KERN_PROC_ALL) AddSysctl(hasher); @@ -419,7 +419,7 @@ void RandAddStaticEnv(CSHA512& hasher) // For MacOS/BSDs, gather data through sysctl instead of /proc. Not all of these // will exist on every system. -#if HAVE_SYSCTL +#ifdef HAVE_SYSCTL # ifdef CTL_HW # ifdef HW_MACHINE AddSysctl(hasher); From 82b43955f7948b225bebd08851a616d17f70a926 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 10 Jun 2024 09:44:31 +0100 Subject: [PATCH 22/39] refactor: use #ifdef HAVE_SOCKADDR_UN ```bash init.cpp:526:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 526 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ init.cpp:541:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 541 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ init.cpp:1318:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 1318 | #if HAVE_SOCKADDR_UN ``` ``` netbase.cpp:26:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 26 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ netbase.cpp:221:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 221 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ netbase.cpp:496:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 496 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ netbase.cpp:531:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 531 | #if HAVE_SOCKADDR_UN | ^~~~~~~~~~~~~~~~ netbase.cpp:639:5: error: "HAVE_SOCKADDR_UN" is not defined, evaluates to 0 [-Werror=undef] 639 | #if HAVE_SOCKADDR_UN ``` --- src/init.cpp | 6 +++--- src/netbase.cpp | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 0f6a775a75..16cfa296cc 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -531,7 +531,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-maxreceivebuffer=", strprintf("Maximum per-connection receive buffer, *1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxsendbuffer=", strprintf("Maximum per-connection memory usage for the send buffer, *1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxuploadtarget=", strprintf("Tries to keep outbound traffic under the given target per 24h. Limit does not apply to peers with 'download' permission or blocks created within past week. 0 = no limit (default: %s). Optional suffix units [k|K|m|M|g|G|t|T] (default: M). Lowercase is 1000 base while uppercase is 1024 base", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN argsman.AddArg("-onion=", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy). May be a local file path prefixed with 'unix:'.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); #else argsman.AddArg("-onion=", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -544,7 +544,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-port=", strprintf("Listen for connections on (default: %u, testnet: %u, signet: %u, regtest: %u). Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN argsman.AddArg("-proxy=", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled). May be a local file path prefixed with 'unix:' if the proxy supports it.", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_ELISION, OptionsCategory::CONNECTION); #else argsman.AddArg("-proxy=", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_ELISION, OptionsCategory::CONNECTION); @@ -1325,7 +1325,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) std::string host_out; uint16_t port_out{0}; if (!SplitHostPort(socket_addr, port_out, host_out)) { -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN // Allow unix domain sockets for some options e.g. unix:/some/file/path if (!unix || socket_addr.find(ADDR_PREFIX_UNIX) != 0) { return InitError(InvalidPortErrMsg(arg, socket_addr)); diff --git a/src/netbase.cpp b/src/netbase.cpp index fcbdb43e2a..f5f0997ba6 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -23,7 +23,7 @@ #include #include -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN #include #endif @@ -218,7 +218,7 @@ CService LookupNumeric(const std::string& name, uint16_t portDefault, DNSLookupF bool IsUnixSocketPath(const std::string& name) { -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN if (name.find(ADDR_PREFIX_UNIX) != 0) return false; // Split off "unix:" prefix @@ -527,7 +527,7 @@ std::unique_ptr CreateSockOS(int domain, int type, int protocol) return nullptr; } -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN if (domain == AF_UNIX) return sock; #endif @@ -638,7 +638,7 @@ std::unique_ptr Proxy::Connect() const if (!m_is_unix_socket) return ConnectDirectly(proxy, /*manual_connection=*/true); -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN auto sock = CreateSock(AF_UNIX, SOCK_STREAM, 0); if (!sock) { LogPrintLevel(BCLog::NET, BCLog::Level::Error, "Cannot create a socket for connecting to %s\n", m_unix_socket_path); From e3dc64f4990a15df3fd6147831f66fc2a31c71ad Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 3 Apr 2024 15:29:29 +0100 Subject: [PATCH 23/39] build: add -Wundef "Warn if an undefined identifier is evaluated in an #if directive. Such identifiers are replaced with zero." --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index 6e040f5aa0..23b8870d43 100644 --- a/configure.ac +++ b/configure.ac @@ -405,6 +405,7 @@ AX_CHECK_COMPILE_FLAG([-Wimplicit-fallthrough], [WARN_CXXFLAGS="$WARN_CXXFLAGS - AX_CHECK_COMPILE_FLAG([-Wunreachable-code], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunreachable-code"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wdocumentation], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdocumentation"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wself-assign], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wself-assign"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wundef], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wundef"], [], [$CXXFLAG_WERROR]) dnl Some compilers (gcc) ignore unknown -Wno-* options, but warn about all dnl unknown options if any other warning is produced. Test the -Wfoo case, and From da205dda14d7e71fe0567944117362c1b820ba8f Mon Sep 17 00:00:00 2001 From: Max Edwards Date: Fri, 21 Jun 2024 16:33:01 +0100 Subject: [PATCH 24/39] ci: increase available ccache size to 300MB --- ci/test/00_setup_env_native_asan.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 0fd169f523..23d9180f96 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -26,3 +26,4 @@ export BITCOIN_CONFIG="--enable-usdt --enable-zmq --with-incompatible-bdb --with CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' \ --with-sanitizers=address,float-divide-by-zero,integer,undefined \ CC='clang-18 -ftrivial-auto-var-init=pattern' CXX='clang++-18 -ftrivial-auto-var-init=pattern'" +export CCACHE_MAXSIZE=300M From 72b226882fe2348a9a66aee1d8d21b4e2d275e68 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 19 Jun 2024 14:27:35 -0300 Subject: [PATCH 25/39] wallet: notify when preset + automatic inputs exceed max weight This also avoids signing all inputs prior to erroring out. --- src/wallet/spend.cpp | 7 ++++ test/functional/wallet_fundrawtransaction.py | 33 +++++++++++++++++++ test/functional/wallet_send.py | 34 ++++++++++++++++++++ 3 files changed, 74 insertions(+) diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 4cbcfdb60f..2fce051f54 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -799,6 +799,13 @@ util::Result SelectCoins(const CWallet& wallet, CoinsResult& av op_selection_result->RecalculateWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); + + // Verify we haven't exceeded the maximum allowed weight + int max_inputs_weight = MAX_STANDARD_TX_WEIGHT - (coin_selection_params.tx_noinputs_size * WITNESS_SCALE_FACTOR); + if (op_selection_result->GetWeight() > max_inputs_weight) { + return util::Error{_("The combination of the pre-selected inputs and the wallet automatic inputs selection exceeds the transaction maximum weight. " + "Please try sending a smaller amount or manually consolidating your wallet's UTXOs")}; + } } return op_selection_result; } diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index 71c883f166..3c1b2deb1d 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -114,6 +114,7 @@ def run_test(self): self.test_add_inputs_default_value() self.test_preset_inputs_selection() self.test_weight_calculation() + self.test_weight_limits() self.test_change_position() self.test_simple() self.test_simple_two_coins() @@ -1312,6 +1313,38 @@ def test_weight_calculation(self): self.nodes[2].unloadwallet("test_weight_calculation") + def test_weight_limits(self): + self.log.info("Test weight limits") + + self.nodes[2].createwallet("test_weight_limits") + wallet = self.nodes[2].get_wallet_rpc("test_weight_limits") + + outputs = [] + for _ in range(1472): + outputs.append({wallet.getnewaddress(address_type="legacy"): 0.1}) + txid = self.nodes[0].send(outputs=outputs)["txid"] + self.generate(self.nodes[0], 1) + + # 272 WU per input (273 when high-s); picking 1471 inputs will exceed the max standard tx weight. + rawtx = wallet.createrawtransaction([], [{wallet.getnewaddress(): 0.1 * 1471}]) + + # 1) Try to fund transaction only using the preset inputs + input_weights = [] + for i in range(1471): + input_weights.append({"txid": txid, "vout": i, "weight": 273}) + assert_raises_rpc_error(-4, "Transaction too large", wallet.fundrawtransaction, hexstring=rawtx, input_weights=input_weights) + + # 2) Let the wallet fund the transaction + assert_raises_rpc_error(-4, "The inputs size exceeds the maximum weight. Please try sending a smaller amount or manually consolidating your wallet's UTXOs", + wallet.fundrawtransaction, hexstring=rawtx) + + # 3) Pre-select some inputs and let the wallet fill-up the remaining amount + inputs = input_weights[0:1000] + assert_raises_rpc_error(-4, "The combination of the pre-selected inputs and the wallet automatic inputs selection exceeds the transaction maximum weight. Please try sending a smaller amount or manually consolidating your wallet's UTXOs", + wallet.fundrawtransaction, hexstring=rawtx, input_weights=inputs) + + self.nodes[2].unloadwallet("test_weight_limits") + def test_include_unsafe(self): self.log.info("Test fundrawtxn with unsafe inputs") diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 0a0a8dba0d..bbb0d658d9 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -577,5 +577,39 @@ def run_test(self): # but rounded to nearest integer, it should be the same as the target fee rate assert_equal(round(actual_fee_rate_sat_vb), target_fee_rate_sat_vb) + # Check tx creation size limits + self.test_weight_limits() + + def test_weight_limits(self): + self.log.info("Test weight limits") + + self.nodes[1].createwallet("test_weight_limits") + wallet = self.nodes[1].get_wallet_rpc("test_weight_limits") + + # Generate future inputs; 272 WU per input (273 when high-s). + # Picking 1471 inputs will exceed the max standard tx weight. + outputs = [] + for _ in range(1472): + outputs.append({wallet.getnewaddress(address_type="legacy"): 0.1}) + self.nodes[0].send(outputs=outputs) + self.generate(self.nodes[0], 1) + + # 1) Try to fund transaction only using the preset inputs + inputs = wallet.listunspent() + assert_raises_rpc_error(-4, "Transaction too large", + wallet.send, outputs=[{wallet.getnewaddress(): 0.1 * 1471}], options={"inputs": inputs, "add_inputs": False}) + + # 2) Let the wallet fund the transaction + assert_raises_rpc_error(-4, "The inputs size exceeds the maximum weight. Please try sending a smaller amount or manually consolidating your wallet's UTXOs", + wallet.send, outputs=[{wallet.getnewaddress(): 0.1 * 1471}]) + + # 3) Pre-select some inputs and let the wallet fill-up the remaining amount + inputs = inputs[0:1000] + assert_raises_rpc_error(-4, "The combination of the pre-selected inputs and the wallet automatic inputs selection exceeds the transaction maximum weight. Please try sending a smaller amount or manually consolidating your wallet's UTXOs", + wallet.send, outputs=[{wallet.getnewaddress(): 0.1 * 1471}], options={"inputs": inputs, "add_inputs": True}) + + self.nodes[1].unloadwallet("test_weight_limits") + + if __name__ == '__main__': WalletSendTest().main() From c0b5ea5901d0ed005bca345e5b2de8a502f6af75 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 24 Jun 2024 14:28:23 +0100 Subject: [PATCH 26/39] build: Drop redundant `sys/sysctl.h` header check The `AC_CHECK_HEADERS` macro defines `HAVE_SYS_SYSCTL_H` if the `sys/sysctl.h` header is found. However, in the source code, this header is guarded by `HAVE_SYSCTL` and `HAVE_SYSCTL_ARND` macros, which have their own checks. Since `HAVE_SYS_SYSCTL_H` is not used, we can skip the `AC_CHECK_HEADERS(... sys/sysctl.h ...)` check. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ab369cc98a..f449904972 100644 --- a/configure.ac +++ b/configure.ac @@ -896,7 +896,7 @@ if test "$TARGET_OS" = "darwin"; then AX_CHECK_LINK_FLAG([-Wl,-fixup_chains], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,-fixup_chains"], [], [$LDFLAG_WERROR]) fi -AC_CHECK_HEADERS([sys/select.h sys/prctl.h sys/sysctl.h vm/vm_param.h sys/vmmeter.h sys/resources.h]) +AC_CHECK_HEADERS([sys/select.h sys/prctl.h vm/vm_param.h sys/vmmeter.h sys/resources.h]) AC_CHECK_DECLS([getifaddrs, freeifaddrs],[CHECK_SOCKET],, [#include From 1408944d2ec9f78e62bf91a5e5a50317ba3060c5 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 25 Jun 2024 15:01:00 +0100 Subject: [PATCH 27/39] Squashed 'src/secp256k1/' changes from 06bff6dec8..4af241b320 4af241b320 Merge bitcoin-core/secp256k1#1535: build: Replace hardcoded "auto" value with default one f473c959f0 Merge bitcoin-core/secp256k1#1543: cmake: Do not modify build types when integrating by downstream project d403eea484 Merge bitcoin-core/secp256k1#1546: cmake: Rename `SECP256K1_LATE_CFLAGS` and switch to Bitcoin Core's approach d7ae25ce6f Merge bitcoin-core/secp256k1#1550: fix: typos in secp256k1.c 0e2fadb20c fix: typos in secp256k1.c 69b2192ad4 Merge bitcoin-core/secp256k1#1545: cmake: Do not set `CTEST_TEST_TARGET_ALIAS` 5dd637f3cf Merge bitcoin-core/secp256k1#1548: README: mention ellswift module 7454a53736 README: mention ellswift module 4706be2cd0 cmake: Reimplement `SECP256K1_APPEND_CFLAGS` using Bitcoin Core approach c2764dbb99 cmake: Rename `SECP256K1_LATE_CFLAGS` to `SECP256K1_APPEND_CFLAGS` f87a3589f4 cmake: Do not set `CTEST_TEST_TARGET_ALIAS` 158f9e5eae cmake: Do not modify build types when integrating by downstream project 35c0fdc86b Merge bitcoin-core/secp256k1#1529: cmake: Fix cache issue when integrating by downstream project 4392f0f717 Merge bitcoin-core/secp256k1#1533: tests: refactor: tidy up util functions (#1491) bedffd53d8 Merge bitcoin-core/secp256k1#1488: ci: Add native macOS arm64 job 4b8d5eeacf Merge bitcoin-core/secp256k1#1532: cmake: Disable eager MSan in ctime_tests f55703ba49 autotools: Delete unneeded compiler test 396e885886 autotools: Align MSan checking code with CMake's implementation abde59f52d cmake: Report more compiler details in summary 7abf979a43 cmake: Disable `ctime_tests` if build with `-fsanitize=memory` 4d9645bee0 cmake: Remove "AUTO" value of `SECP256K1_ECMULT_GEN_KB` option a06805ee74 cmake: Remove "AUTO" value of `SECP256K1_ECMULT_WINDOW_SIZE` option 1791f6fce4 Merge bitcoin-core/secp256k1#1517: autotools: Disable eager MSan in ctime_tests 26b94ee92a autotools: Remove "auto" value of `--with-ecmult-gen-kb` option 122dbaeb37 autotools: Remove "auto" value of `--with-ecmult-window` option e73f6f8fd9 tests: refactor: drop `secp256k1_` prefix from testrand.h functions 0ee7453a99 tests: refactor: add `testutil_` prefix to testutil.h functions 0c6bc76dcd tests: refactor: move `random_` helpers from tests.c to testutil.h 0fef8479be tests: refactor: rename `random_field_element_magnitude` -> `random_fe_magnitude` 59db007f0f tests: refactor: rename `random_group_element_...` -> `random_ge_...` ebfb82ee2f ci: Add job with -fsanitize-memory-param-retval e1bef0961c configure: Move "experimental" warning to bottom 55e5d975db autotools: Disable eager MSan in ctime_tests ec4c002faa cmake: Simplify `PROJECT_IS_TOP_LEVEL` emulation cae9a7ad14 cmake: Do not set emulated PROJECT_IS_TOP_LEVEL as cache variable 218f0cc93b ci: Add native macOS arm64 job git-subtree-dir: src/secp256k1 git-subtree-split: 4af241b32099067464e015fa66daac5096206dea --- .cirrus.yml | 4 +- .github/workflows/ci.yml | 74 ++- CMakeLists.txt | 99 ++-- README.md | 1 + build-aux/m4/bitcoin_secp.m4 | 16 + cmake/AllTargetsCompileOptions.cmake | 12 - cmake/CheckMemorySanitizer.cmake | 18 + configure.ac | 65 +-- src/modules/ecdh/tests_impl.h | 6 +- src/modules/ellswift/tests_impl.h | 38 +- src/modules/extrakeys/tests_impl.h | 36 +- src/modules/recovery/tests_impl.h | 8 +- .../schnorrsig/tests_exhaustive_impl.h | 6 +- src/modules/schnorrsig/tests_impl.h | 50 +- src/secp256k1.c | 4 +- src/testrand.h | 22 +- src/testrand_impl.h | 42 +- src/tests.c | 458 +++++++----------- src/tests_exhaustive.c | 10 +- src/testutil.h | 121 ++++- 20 files changed, 596 insertions(+), 494 deletions(-) delete mode 100644 cmake/AllTargetsCompileOptions.cmake create mode 100644 cmake/CheckMemorySanitizer.cmake diff --git a/.cirrus.yml b/.cirrus.yml index 6e77403bf5..4bd15511a4 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -10,8 +10,8 @@ env: MAKEFLAGS: -j4 BUILD: check ### secp256k1 config - ECMULTWINDOW: auto - ECMULTGENKB: auto + ECMULTWINDOW: 15 + ECMULTGENKB: 22 ASM: no WIDEMUL: auto WITH_VALGRIND: yes diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d246682044..ade94e1eec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,8 +21,8 @@ env: MAKEFLAGS: '-j4' BUILD: 'check' ### secp256k1 config - ECMULTWINDOW: 'auto' - ECMULTGENKB: 'auto' + ECMULTWINDOW: 15 + ECMULTGENKB: 22 ASM: 'no' WIDEMUL: 'auto' WITH_VALGRIND: 'yes' @@ -485,18 +485,24 @@ jobs: matrix: configuration: - env_vars: + CTIMETESTS: 'yes' CFLAGS: '-fsanitize=memory -fsanitize-recover=memory -g' - env_vars: ECMULTGENKB: 2 ECMULTWINDOW: 2 + CTIMETESTS: 'yes' CFLAGS: '-fsanitize=memory -fsanitize-recover=memory -g -O3' + - env_vars: + # -fsanitize-memory-param-retval is clang's default, but our build system disables it + # when ctime_tests when enabled. + CFLAGS: '-fsanitize=memory -fsanitize-recover=memory -fsanitize-memory-param-retval -g' + CTIMETESTS: 'no' env: ECDH: 'yes' RECOVERY: 'yes' SCHNORRSIG: 'yes' ELLSWIFT: 'yes' - CTIMETESTS: 'yes' CC: 'clang' SECP256K1_TEST_ITERS: 32 ASM: 'no' @@ -585,10 +591,10 @@ jobs: run: env if: ${{ always() }} - macos-native: - name: "x86_64: macOS Monterey" + x86_64-macos-native: + name: "x86_64: macOS Monterey, Valgrind" # See: https://github.com/actions/runner-images#available-images. - runs-on: macos-12 # Use M1 once available https://github.com/github/roadmap/issues/528 + runs-on: macos-12 env: CC: 'clang' @@ -644,6 +650,62 @@ jobs: run: env if: ${{ always() }} + arm64-macos-native: + name: "ARM64: macOS Sonoma" + # See: https://github.com/actions/runner-images#available-images. + runs-on: macos-14 + + env: + CC: 'clang' + HOMEBREW_NO_AUTO_UPDATE: 1 + HOMEBREW_NO_INSTALL_CLEANUP: 1 + WITH_VALGRIND: 'no' + CTIMETESTS: 'no' + + strategy: + fail-fast: false + matrix: + env_vars: + - { WIDEMUL: 'int64', RECOVERY: 'yes', ECDH: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes' } + - { WIDEMUL: 'int128_struct', ECMULTGENPRECISION: 2, ECMULTWINDOW: 4 } + - { WIDEMUL: 'int128', ECDH: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes' } + - { WIDEMUL: 'int128', RECOVERY: 'yes' } + - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes' } + - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes', CC: 'gcc' } + - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes', CPPFLAGS: '-DVERIFY' } + - BUILD: 'distcheck' + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Install Homebrew packages + run: | + brew install automake libtool gcc + ln -s $(brew --prefix gcc)/bin/gcc-?? /usr/local/bin/gcc + + - name: CI script + env: ${{ matrix.env_vars }} + run: ./ci/ci.sh + + - run: cat tests.log || true + if: ${{ always() }} + - run: cat noverify_tests.log || true + if: ${{ always() }} + - run: cat exhaustive_tests.log || true + if: ${{ always() }} + - run: cat ctime_tests.log || true + if: ${{ always() }} + - run: cat bench.log || true + if: ${{ always() }} + - run: cat config.log || true + if: ${{ always() }} + - run: cat test_env.log || true + if: ${{ always() }} + - name: CI env + run: env + if: ${{ always() }} + win64-native: name: ${{ matrix.configuration.job_name }} # See: https://github.com/actions/runner-images#available-images. diff --git a/CMakeLists.txt b/CMakeLists.txt index 88994d828a..7e3465a75b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -18,15 +18,14 @@ project(libsecp256k1 ) if(CMAKE_VERSION VERSION_LESS 3.21) - get_directory_property(parent_directory PARENT_DIRECTORY) - if(parent_directory) - set(PROJECT_IS_TOP_LEVEL OFF CACHE INTERNAL "Emulates CMake 3.21+ behavior.") - set(${PROJECT_NAME}_IS_TOP_LEVEL OFF CACHE INTERNAL "Emulates CMake 3.21+ behavior.") + # Emulates CMake 3.21+ behavior. + if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR) + set(PROJECT_IS_TOP_LEVEL ON) + set(${PROJECT_NAME}_IS_TOP_LEVEL ON) else() - set(PROJECT_IS_TOP_LEVEL ON CACHE INTERNAL "Emulates CMake 3.21+ behavior.") - set(${PROJECT_NAME}_IS_TOP_LEVEL ON CACHE INTERNAL "Emulates CMake 3.21+ behavior.") + set(PROJECT_IS_TOP_LEVEL OFF) + set(${PROJECT_NAME}_IS_TOP_LEVEL OFF) endif() - unset(parent_directory) endif() # The library version is based on libtool versioning of the ABI. The set of @@ -92,21 +91,15 @@ if(SECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS) add_compile_definitions(USE_EXTERNAL_DEFAULT_CALLBACKS=1) endif() -set(SECP256K1_ECMULT_WINDOW_SIZE "AUTO" CACHE STRING "Window size for ecmult precomputation for verification, specified as integer in range [2..24]. \"AUTO\" is a reasonable setting for desktop machines (currently 15). [default=AUTO]") -set_property(CACHE SECP256K1_ECMULT_WINDOW_SIZE PROPERTY STRINGS "AUTO" 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24) +set(SECP256K1_ECMULT_WINDOW_SIZE 15 CACHE STRING "Window size for ecmult precomputation for verification, specified as integer in range [2..24]. The default value is a reasonable setting for desktop machines (currently 15). [default=15]") +set_property(CACHE SECP256K1_ECMULT_WINDOW_SIZE PROPERTY STRINGS 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24) include(CheckStringOptionValue) check_string_option_value(SECP256K1_ECMULT_WINDOW_SIZE) -if(SECP256K1_ECMULT_WINDOW_SIZE STREQUAL "AUTO") - set(SECP256K1_ECMULT_WINDOW_SIZE 15) -endif() add_compile_definitions(ECMULT_WINDOW_SIZE=${SECP256K1_ECMULT_WINDOW_SIZE}) -set(SECP256K1_ECMULT_GEN_KB "AUTO" CACHE STRING "The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms). Larger values result in possibly better signing or key generation performance at the cost of a larger table. Valid choices are 2, 22, 86. \"AUTO\" is a reasonable setting for desktop machines (currently 22). [default=AUTO]") -set_property(CACHE SECP256K1_ECMULT_GEN_KB PROPERTY STRINGS "AUTO" 2 22 86) +set(SECP256K1_ECMULT_GEN_KB 22 CACHE STRING "The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms). Larger values result in possibly better signing or key generation performance at the cost of a larger table. Valid choices are 2, 22, 86. The default value is a reasonable setting for desktop machines (currently 22). [default=22]") +set_property(CACHE SECP256K1_ECMULT_GEN_KB PROPERTY STRINGS 2 22 86) check_string_option_value(SECP256K1_ECMULT_GEN_KB) -if(SECP256K1_ECMULT_GEN_KB STREQUAL "AUTO") - set(SECP256K1_ECMULT_GEN_KB 22) -endif() if(SECP256K1_ECMULT_GEN_KB EQUAL 2) add_compile_definitions(COMB_BLOCKS=2) add_compile_definitions(COMB_TEETH=5) @@ -214,23 +207,25 @@ mark_as_advanced( CMAKE_SHARED_LINKER_FLAGS_COVERAGE ) -get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) -set(default_build_type "RelWithDebInfo") -if(is_multi_config) - set(CMAKE_CONFIGURATION_TYPES "${default_build_type}" "Release" "Debug" "MinSizeRel" "Coverage" CACHE STRING - "Supported configuration types." - FORCE - ) -else() - set_property(CACHE CMAKE_BUILD_TYPE PROPERTY - STRINGS "${default_build_type}" "Release" "Debug" "MinSizeRel" "Coverage" - ) - if(NOT CMAKE_BUILD_TYPE) - message(STATUS "Setting build type to \"${default_build_type}\" as none was specified") - set(CMAKE_BUILD_TYPE "${default_build_type}" CACHE STRING - "Choose the type of build." +if(PROJECT_IS_TOP_LEVEL) + get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) + set(default_build_type "RelWithDebInfo") + if(is_multi_config) + set(CMAKE_CONFIGURATION_TYPES "${default_build_type}" "Release" "Debug" "MinSizeRel" "Coverage" CACHE STRING + "Supported configuration types." FORCE ) + else() + set_property(CACHE CMAKE_BUILD_TYPE PROPERTY + STRINGS "${default_build_type}" "Release" "Debug" "MinSizeRel" "Coverage" + ) + if(NOT CMAKE_BUILD_TYPE) + message(STATUS "Setting build type to \"${default_build_type}\" as none was specified") + set(CMAKE_BUILD_TYPE "${default_build_type}" CACHE STRING + "Choose the type of build." + FORCE + ) + endif() endif() endif() @@ -263,10 +258,17 @@ endif() set(CMAKE_C_VISIBILITY_PRESET hidden) -# Ask CTest to create a "check" target (e.g., make check) as alias for the "test" target. -# CTEST_TEST_TARGET_ALIAS is not documented but supposed to be user-facing. -# See: https://gitlab.kitware.com/cmake/cmake/-/commit/816c9d1aa1f2b42d40c81a991b68c96eb12b6d2 -set(CTEST_TEST_TARGET_ALIAS check) +set(print_msan_notice) +if(SECP256K1_BUILD_CTIME_TESTS) + include(CheckMemorySanitizer) + check_memory_sanitizer(msan_enabled) + if(msan_enabled) + try_append_c_flags(-fno-sanitize-memory-param-retval) + set(print_msan_notice YES) + endif() + unset(msan_enabled) +endif() + include(CTest) # We do not use CTest's BUILD_TESTING because a single toggle for all tests is too coarse for our needs. mark_as_advanced(BUILD_TESTING) @@ -274,14 +276,16 @@ if(SECP256K1_BUILD_BENCHMARK OR SECP256K1_BUILD_TESTS OR SECP256K1_BUILD_EXHAUST enable_testing() endif() -set(SECP256K1_LATE_CFLAGS "" CACHE STRING "Compiler flags that are added to the command line after all other flags added by the build system.") -include(AllTargetsCompileOptions) +set(SECP256K1_APPEND_CFLAGS "" CACHE STRING "Compiler flags that are appended to the command line after all other flags added by the build system. This variable is intended for debugging and special builds.") +if(SECP256K1_APPEND_CFLAGS) + # Appending to this low-level rule variable is the only way to + # guarantee that the flags appear at the end of the command line. + string(APPEND CMAKE_C_COMPILE_OBJECT " ${SECP256K1_APPEND_CFLAGS}") +endif() add_subdirectory(src) -all_targets_compile_options(src "${SECP256K1_LATE_CFLAGS}") if(SECP256K1_BUILD_EXAMPLES) add_subdirectory(examples) - all_targets_compile_options(examples "${SECP256K1_LATE_CFLAGS}") endif() message("\n") @@ -332,7 +336,7 @@ message("Valgrind .............................. ${SECP256K1_VALGRIND}") get_directory_property(definitions COMPILE_DEFINITIONS) string(REPLACE ";" " " definitions "${definitions}") message("Preprocessor defined macros ........... ${definitions}") -message("C compiler ............................ ${CMAKE_C_COMPILER}") +message("C compiler ............................ ${CMAKE_C_COMPILER_ID} ${CMAKE_C_COMPILER_VERSION}, ${CMAKE_C_COMPILER}") message("CFLAGS ................................ ${CMAKE_C_FLAGS}") get_directory_property(compile_options COMPILE_OPTIONS) string(REPLACE ";" " " compile_options "${compile_options}") @@ -355,10 +359,17 @@ else() message(" - LDFLAGS for executables ............ ${CMAKE_EXE_LINKER_FLAGS_DEBUG}") message(" - LDFLAGS for shared libraries ....... ${CMAKE_SHARED_LINKER_FLAGS_DEBUG}") endif() -if(SECP256K1_LATE_CFLAGS) - message("SECP256K1_LATE_CFLAGS ................. ${SECP256K1_LATE_CFLAGS}") +if(SECP256K1_APPEND_CFLAGS) + message("SECP256K1_APPEND_CFLAGS ............... ${SECP256K1_APPEND_CFLAGS}") +endif() +message("") +if(print_msan_notice) + message( + "Note:\n" + " MemorySanitizer detected, tried to add -fno-sanitize-memory-param-retval to compile options\n" + " to avoid false positives in ctime_tests. Pass -DSECP256K1_BUILD_CTIME_TESTS=OFF to avoid this.\n" + ) endif() -message("\n") if(SECP256K1_EXPERIMENTAL) message( " ******\n" diff --git a/README.md b/README.md index 6e88eb4ecb..e8d4123ab9 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ Features: * Optional module for public key recovery. * Optional module for ECDH key exchange. * Optional module for Schnorr signatures according to [BIP-340](https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki). +* Optional module for ElligatorSwift key exchange according to [BIP-324](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki). Implementation details ---------------------- diff --git a/build-aux/m4/bitcoin_secp.m4 b/build-aux/m4/bitcoin_secp.m4 index 11adef4f22..048267fa6e 100644 --- a/build-aux/m4/bitcoin_secp.m4 +++ b/build-aux/m4/bitcoin_secp.m4 @@ -45,6 +45,22 @@ fi AC_MSG_RESULT($has_valgrind) ]) +AC_DEFUN([SECP_MSAN_CHECK], [ +AC_MSG_CHECKING(whether MemorySanitizer is enabled) +AC_COMPILE_IFELSE([AC_LANG_SOURCE([[ + #if defined(__has_feature) + # if __has_feature(memory_sanitizer) + /* MemorySanitizer is enabled. */ + # elif + # error "MemorySanitizer is disabled." + # endif + #else + # error "__has_feature is not defined." + #endif + ]])], [msan_enabled=yes], [msan_enabled=no]) +AC_MSG_RESULT([$msan_enabled]) +]) + dnl SECP_TRY_APPEND_CFLAGS(flags, VAR) dnl Append flags to VAR if CC accepts them. AC_DEFUN([SECP_TRY_APPEND_CFLAGS], [ diff --git a/cmake/AllTargetsCompileOptions.cmake b/cmake/AllTargetsCompileOptions.cmake deleted file mode 100644 index 6e420e0fde..0000000000 --- a/cmake/AllTargetsCompileOptions.cmake +++ /dev/null @@ -1,12 +0,0 @@ -# Add compile options to all targets added in the subdirectory. -function(all_targets_compile_options dir options) - get_directory_property(targets DIRECTORY ${dir} BUILDSYSTEM_TARGETS) - separate_arguments(options) - set(compiled_target_types STATIC_LIBRARY SHARED_LIBRARY OBJECT_LIBRARY EXECUTABLE) - foreach(target ${targets}) - get_target_property(type ${target} TYPE) - if(type IN_LIST compiled_target_types) - target_compile_options(${target} PRIVATE ${options}) - endif() - endforeach() -endfunction() diff --git a/cmake/CheckMemorySanitizer.cmake b/cmake/CheckMemorySanitizer.cmake new file mode 100644 index 0000000000..d9ef681e65 --- /dev/null +++ b/cmake/CheckMemorySanitizer.cmake @@ -0,0 +1,18 @@ +include_guard(GLOBAL) +include(CheckCSourceCompiles) + +function(check_memory_sanitizer output) + set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY) + check_c_source_compiles(" + #if defined(__has_feature) + # if __has_feature(memory_sanitizer) + /* MemorySanitizer is enabled. */ + # elif + # error \"MemorySanitizer is disabled.\" + # endif + #else + # error \"__has_feature is not defined.\" + #endif + " HAVE_MSAN) + set(${output} ${HAVE_MSAN} PARENT_SCOPE) +endfunction() diff --git a/configure.ac b/configure.ac index 8b62e1dbfd..adae189787 100644 --- a/configure.ac +++ b/configure.ac @@ -203,22 +203,22 @@ AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_wide AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm32|no|auto], [assembly to use (experimental: arm32) [default=auto]])],[req_asm=$withval], [req_asm=auto]) -AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE|auto], +AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE], [window size for ecmult precomputation for verification, specified as integer in range [2..24].] [Larger values result in possibly better performance at the cost of an exponentially larger precomputed table.] [The table will store 2^(SIZE-1) * 64 bytes of data but can be larger in memory due to platform-specific padding and alignment.] [A window size larger than 15 will require you delete the prebuilt precomputed_ecmult.c file so that it can be rebuilt.] [For very large window sizes, use "make -j 1" to reduce memory use during compilation.] -["auto" is a reasonable setting for desktop machines (currently 15). [default=auto]] +[The default value is a reasonable setting for desktop machines (currently 15). [default=15]] )], -[req_ecmult_window=$withval], [req_ecmult_window=auto]) +[set_ecmult_window=$withval], [set_ecmult_window=15]) -AC_ARG_WITH([ecmult-gen-kb], [AS_HELP_STRING([--with-ecmult-gen-kb=2|22|86|auto], +AC_ARG_WITH([ecmult-gen-kb], [AS_HELP_STRING([--with-ecmult-gen-kb=2|22|86], [The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms).] [Larger values result in possibly better signing/keygeneration performance at the cost of a larger table.] -["auto" is a reasonable setting for desktop machines (currently 22). [default=auto]] +[The default value is a reasonable setting for desktop machines (currently 22). [default=22]] )], -[req_ecmult_gen_kb=$withval], [req_ecmult_gen_kb=auto]) +[set_ecmult_gen_kb=$withval], [set_ecmult_gen_kb=22]) AC_ARG_WITH([valgrind], [AS_HELP_STRING([--with-valgrind=yes|no|auto], [Build with extra checks for running inside Valgrind [default=auto]] @@ -247,6 +247,20 @@ if test x"$enable_ctime_tests" = x"auto"; then enable_ctime_tests=$enable_valgrind fi +print_msan_notice=no +if test x"$enable_ctime_tests" = x"yes"; then + SECP_MSAN_CHECK + # MSan on Clang >=16 reports unitialized memory in function parameters and return values, even if + # the uninitalized variable is never actually "used". This is called "eager" checking, and it's + # sounds like good idea for normal use of MSan. However, it yields many false positives in the + # ctime_tests because many return values depend on secret (i.e., "uninitialized") values, and + # we're only interested in detecting branches (which count as "uses") on secret data. + if test x"$msan_enabled" = x"yes"; then + SECP_TRY_APPEND_CFLAGS([-fno-sanitize-memory-param-retval], SECP_CFLAGS) + print_msan_notice=yes + fi +fi + if test x"$enable_coverage" = x"yes"; then SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOVERAGE=1" SECP_CFLAGS="-O0 --coverage $SECP_CFLAGS" @@ -335,14 +349,7 @@ auto) ;; esac -# Set ecmult window size -if test x"$req_ecmult_window" = x"auto"; then - set_ecmult_window=15 -else - set_ecmult_window=$req_ecmult_window -fi - -error_window_size=['window size for ecmult precomputation not an integer in range [2..24] or "auto"'] +error_window_size=['window size for ecmult precomputation not an integer in range [2..24]'] case $set_ecmult_window in ''|*[[!0-9]]*) # no valid integer @@ -357,13 +364,6 @@ case $set_ecmult_window in ;; esac -# Set ecmult gen kb -if test x"$req_ecmult_gen_kb" = x"auto"; then - set_ecmult_gen_kb=22 -else - set_ecmult_gen_kb=$req_ecmult_gen_kb -fi - case $set_ecmult_gen_kb in 2) SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOMB_BLOCKS=2 -DCOMB_TEETH=5" @@ -375,7 +375,7 @@ case $set_ecmult_gen_kb in SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOMB_BLOCKS=43 -DCOMB_TEETH=6" ;; *) - AC_MSG_ERROR(['ecmult gen table size not 2, 22, 86 or "auto"']) + AC_MSG_ERROR(['ecmult gen table size not 2, 22 or 86']) ;; esac @@ -426,12 +426,7 @@ fi ### Check for --enable-experimental if necessary ### -if test x"$enable_experimental" = x"yes"; then - AC_MSG_NOTICE([******]) - AC_MSG_NOTICE([WARNING: experimental build]) - AC_MSG_NOTICE([Experimental features do not have stable APIs or properties, and may not be safe for production use.]) - AC_MSG_NOTICE([******]) -else +if test x"$enable_experimental" = x"no"; then if test x"$set_asm" = x"arm32"; then AC_MSG_ERROR([ARM32 assembly is experimental. Use --enable-experimental to allow.]) fi @@ -492,3 +487,17 @@ echo " CPPFLAGS = $CPPFLAGS" echo " SECP_CFLAGS = $SECP_CFLAGS" echo " CFLAGS = $CFLAGS" echo " LDFLAGS = $LDFLAGS" + +if test x"$print_msan_notice" = x"yes"; then + echo + echo "Note:" + echo " MemorySanitizer detected, tried to add -fno-sanitize-memory-param-retval to SECP_CFLAGS" + echo " to avoid false positives in ctime_tests. Pass --disable-ctime-tests to avoid this." +fi + +if test x"$enable_experimental" = x"yes"; then + echo + echo "WARNING: Experimental build" + echo " Experimental features do not have stable APIs or properties, and may not be safe for" + echo " production use." +fi diff --git a/src/modules/ecdh/tests_impl.h b/src/modules/ecdh/tests_impl.h index 6be96eacbe..3c3acdaf8c 100644 --- a/src/modules/ecdh/tests_impl.h +++ b/src/modules/ecdh/tests_impl.h @@ -56,7 +56,7 @@ static void test_ecdh_generator_basepoint(void) { size_t point_ser_len = sizeof(point_ser); secp256k1_scalar s; - random_scalar_order(&s); + testutil_random_scalar_order(&s); secp256k1_scalar_get_b32(s_b32, &s); CHECK(secp256k1_ec_pubkey_create(CTX, &point[0], s_one) == 1); @@ -95,7 +95,7 @@ static void test_bad_scalar(void) { secp256k1_pubkey point; /* Create random point */ - random_scalar_order(&rand); + testutil_random_scalar_order(&rand); secp256k1_scalar_get_b32(s_rand, &rand); CHECK(secp256k1_ec_pubkey_create(CTX, &point, s_rand) == 1); @@ -127,7 +127,7 @@ static void test_result_basepoint(void) { CHECK(secp256k1_ecdh(CTX, out_base, &point, s_one, NULL, NULL) == 1); for (i = 0; i < 2 * COUNT; i++) { - random_scalar_order(&rand); + testutil_random_scalar_order(&rand); secp256k1_scalar_get_b32(s, &rand); secp256k1_scalar_inverse(&rand, &rand); secp256k1_scalar_get_b32(s_inv, &rand); diff --git a/src/modules/ellswift/tests_impl.h b/src/modules/ellswift/tests_impl.h index f96e3a1268..ed5658f34c 100644 --- a/src/modules/ellswift/tests_impl.h +++ b/src/modules/ellswift/tests_impl.h @@ -229,9 +229,9 @@ void run_ellswift_tests(void) { secp256k1_ge g, g2; secp256k1_pubkey pubkey, pubkey2; /* Generate random public key and random randomizer. */ - random_group_element_test(&g); + testutil_random_ge_test(&g); secp256k1_pubkey_save(&pubkey, &g); - secp256k1_testrand256(rnd32); + testrand256(rnd32); /* Convert the public key to ElligatorSwift and back. */ secp256k1_ellswift_encode(CTX, ell64, &pubkey, rnd32); secp256k1_ellswift_decode(CTX, &pubkey2, ell64); @@ -249,8 +249,8 @@ void run_ellswift_tests(void) { unsigned char ell64[64]; int ret; /* Generate random secret key and random randomizer. */ - if (i & 1) secp256k1_testrand256_test(auxrnd32); - random_scalar_order_test(&sec); + if (i & 1) testrand256_test(auxrnd32); + testutil_random_scalar_order_test(&sec); secp256k1_scalar_get_b32(sec32, &sec); /* Construct ElligatorSwift-encoded public keys for that key. */ ret = secp256k1_ellswift_create(CTX, ell64, sec32, (i & 1) ? auxrnd32 : NULL); @@ -271,11 +271,11 @@ void run_ellswift_tests(void) { secp256k1_pubkey pub; int ret; /* Generate random secret key. */ - random_scalar_order_test(&sec); + testutil_random_scalar_order_test(&sec); secp256k1_scalar_get_b32(sec32, &sec); /* Generate random ElligatorSwift encoding for the remote key and decode it. */ - secp256k1_testrand256_test(ell64); - secp256k1_testrand256_test(ell64 + 32); + testrand256_test(ell64); + testrand256_test(ell64 + 32); secp256k1_ellswift_decode(CTX, &pub, ell64); secp256k1_pubkey_load(CTX, &dec, &pub); secp256k1_gej_set_ge(&decj, &dec); @@ -313,18 +313,18 @@ void run_ellswift_tests(void) { data = NULL; } else { hash_function = secp256k1_ellswift_xdh_hash_function_prefix; - secp256k1_testrand256_test(prefix64); - secp256k1_testrand256_test(prefix64 + 32); + testrand256_test(prefix64); + testrand256_test(prefix64 + 32); data = prefix64; } /* Generate random secret keys and random randomizers. */ - secp256k1_testrand256_test(auxrnd32a); - secp256k1_testrand256_test(auxrnd32b); - random_scalar_order_test(&seca); + testrand256_test(auxrnd32a); + testrand256_test(auxrnd32b); + testutil_random_scalar_order_test(&seca); /* Draw secb uniformly at random to make sure that the secret keys * differ */ - random_scalar_order(&secb); + testutil_random_scalar_order(&secb); secp256k1_scalar_get_b32(sec32a, &seca); secp256k1_scalar_get_b32(sec32b, &secb); @@ -349,13 +349,13 @@ void run_ellswift_tests(void) { /* Verify that the shared secret doesn't match if other side's public key is incorrect. */ /* For A (using a bad public key for B): */ memcpy(ell64b_bad, ell64b, sizeof(ell64a_bad)); - secp256k1_testrand_flip(ell64b_bad, sizeof(ell64b_bad)); + testrand_flip(ell64b_bad, sizeof(ell64b_bad)); ret = secp256k1_ellswift_xdh(CTX, share32_bad, ell64a, ell64b_bad, sec32a, 0, hash_function, data); CHECK(ret); /* Mismatching encodings don't get detected by secp256k1_ellswift_xdh. */ CHECK(secp256k1_memcmp_var(share32_bad, share32a, 32) != 0); /* For B (using a bad public key for A): */ memcpy(ell64a_bad, ell64a, sizeof(ell64a_bad)); - secp256k1_testrand_flip(ell64a_bad, sizeof(ell64a_bad)); + testrand_flip(ell64a_bad, sizeof(ell64a_bad)); ret = secp256k1_ellswift_xdh(CTX, share32_bad, ell64a_bad, ell64b, sec32b, 1, hash_function, data); CHECK(ret); CHECK(secp256k1_memcmp_var(share32_bad, share32b, 32) != 0); @@ -363,12 +363,12 @@ void run_ellswift_tests(void) { /* Verify that the shared secret doesn't match if the private key is incorrect. */ /* For A: */ memcpy(sec32a_bad, sec32a, sizeof(sec32a_bad)); - secp256k1_testrand_flip(sec32a_bad, sizeof(sec32a_bad)); + testrand_flip(sec32a_bad, sizeof(sec32a_bad)); ret = secp256k1_ellswift_xdh(CTX, share32_bad, ell64a, ell64b, sec32a_bad, 0, hash_function, data); CHECK(!ret || secp256k1_memcmp_var(share32_bad, share32a, 32) != 0); /* For B: */ memcpy(sec32b_bad, sec32b, sizeof(sec32b_bad)); - secp256k1_testrand_flip(sec32b_bad, sizeof(sec32b_bad)); + testrand_flip(sec32b_bad, sizeof(sec32b_bad)); ret = secp256k1_ellswift_xdh(CTX, share32_bad, ell64a, ell64b, sec32b_bad, 1, hash_function, data); CHECK(!ret || secp256k1_memcmp_var(share32_bad, share32b, 32) != 0); @@ -376,7 +376,7 @@ void run_ellswift_tests(void) { /* Verify that the shared secret doesn't match when a different encoding of the same public key is used. */ /* For A (changing B's public key): */ memcpy(auxrnd32b_bad, auxrnd32b, sizeof(auxrnd32b_bad)); - secp256k1_testrand_flip(auxrnd32b_bad, sizeof(auxrnd32b_bad)); + testrand_flip(auxrnd32b_bad, sizeof(auxrnd32b_bad)); ret = secp256k1_ellswift_create(CTX, ell64b_bad, sec32b, auxrnd32b_bad); CHECK(ret); ret = secp256k1_ellswift_xdh(CTX, share32_bad, ell64a, ell64b_bad, sec32a, 0, hash_function, data); @@ -384,7 +384,7 @@ void run_ellswift_tests(void) { CHECK(secp256k1_memcmp_var(share32_bad, share32a, 32) != 0); /* For B (changing A's public key): */ memcpy(auxrnd32a_bad, auxrnd32a, sizeof(auxrnd32a_bad)); - secp256k1_testrand_flip(auxrnd32a_bad, sizeof(auxrnd32a_bad)); + testrand_flip(auxrnd32a_bad, sizeof(auxrnd32a_bad)); ret = secp256k1_ellswift_create(CTX, ell64a_bad, sec32a, auxrnd32a_bad); CHECK(ret); ret = secp256k1_ellswift_xdh(CTX, share32_bad, ell64a_bad, ell64b, sec32b, 1, hash_function, data); diff --git a/src/modules/extrakeys/tests_impl.h b/src/modules/extrakeys/tests_impl.h index 45521d1742..ab4ef4a74b 100644 --- a/src/modules/extrakeys/tests_impl.h +++ b/src/modules/extrakeys/tests_impl.h @@ -23,9 +23,9 @@ static void test_xonly_pubkey(void) { int pk_parity; int i; - secp256k1_testrand256(sk); + testrand256(sk); memset(ones32, 0xFF, 32); - secp256k1_testrand256(xy_sk); + testrand256(xy_sk); CHECK(secp256k1_ec_pubkey_create(CTX, &pk, sk) == 1); CHECK(secp256k1_xonly_pubkey_from_pubkey(CTX, &xonly_pk, &pk_parity, &pk) == 1); @@ -95,7 +95,7 @@ static void test_xonly_pubkey(void) { * the curve) then xonly_pubkey_parse should fail as well. */ for (i = 0; i < COUNT; i++) { unsigned char rand33[33]; - secp256k1_testrand256(&rand33[1]); + testrand256(&rand33[1]); rand33[0] = SECP256K1_TAG_PUBKEY_EVEN; if (!secp256k1_ec_pubkey_parse(CTX, &pk, rand33, 33)) { memset(&xonly_pk, 1, sizeof(xonly_pk)); @@ -152,8 +152,8 @@ static void test_xonly_pubkey_tweak(void) { int i; memset(overflows, 0xff, sizeof(overflows)); - secp256k1_testrand256(tweak); - secp256k1_testrand256(sk); + testrand256(tweak); + testrand256(sk); CHECK(secp256k1_ec_pubkey_create(CTX, &internal_pk, sk) == 1); CHECK(secp256k1_xonly_pubkey_from_pubkey(CTX, &internal_xonly_pk, &pk_parity, &internal_pk) == 1); @@ -190,7 +190,7 @@ static void test_xonly_pubkey_tweak(void) { /* Invalid pk with a valid tweak */ memset(&internal_xonly_pk, 0, sizeof(internal_xonly_pk)); - secp256k1_testrand256(tweak); + testrand256(tweak); CHECK_ILLEGAL(CTX, secp256k1_xonly_pubkey_tweak_add(CTX, &output_pk, &internal_xonly_pk, tweak)); CHECK(secp256k1_memcmp_var(&output_pk, zeros64, sizeof(output_pk)) == 0); } @@ -209,8 +209,8 @@ static void test_xonly_pubkey_tweak_check(void) { unsigned char tweak[32]; memset(overflows, 0xff, sizeof(overflows)); - secp256k1_testrand256(tweak); - secp256k1_testrand256(sk); + testrand256(tweak); + testrand256(sk); CHECK(secp256k1_ec_pubkey_create(CTX, &internal_pk, sk) == 1); CHECK(secp256k1_xonly_pubkey_from_pubkey(CTX, &internal_xonly_pk, &pk_parity, &internal_pk) == 1); @@ -256,7 +256,7 @@ static void test_xonly_pubkey_tweak_recursive(void) { unsigned char tweak[N_PUBKEYS - 1][32]; int i; - secp256k1_testrand256(sk); + testrand256(sk); CHECK(secp256k1_ec_pubkey_create(CTX, &pk[0], sk) == 1); /* Add tweaks */ for (i = 0; i < N_PUBKEYS - 1; i++) { @@ -292,7 +292,7 @@ static void test_keypair(void) { memset(overflows, 0xFF, sizeof(overflows)); /* Test keypair_create */ - secp256k1_testrand256(sk); + testrand256(sk); CHECK(secp256k1_keypair_create(CTX, &keypair, sk) == 1); CHECK(secp256k1_memcmp_var(zeros96, &keypair, sizeof(keypair)) != 0); CHECK(secp256k1_keypair_create(CTX, &keypair, sk) == 1); @@ -311,7 +311,7 @@ static void test_keypair(void) { CHECK(secp256k1_memcmp_var(zeros96, &keypair, sizeof(keypair)) == 0); /* Test keypair_pub */ - secp256k1_testrand256(sk); + testrand256(sk); CHECK(secp256k1_keypair_create(CTX, &keypair, sk) == 1); CHECK(secp256k1_keypair_pub(CTX, &pk, &keypair) == 1); CHECK_ILLEGAL(CTX, secp256k1_keypair_pub(CTX, NULL, &keypair)); @@ -330,7 +330,7 @@ static void test_keypair(void) { CHECK(secp256k1_memcmp_var(&pk, &pk_tmp, sizeof(pk)) == 0); /** Test keypair_xonly_pub **/ - secp256k1_testrand256(sk); + testrand256(sk); CHECK(secp256k1_keypair_create(CTX, &keypair, sk) == 1); CHECK(secp256k1_keypair_xonly_pub(CTX, &xonly_pk, &pk_parity, &keypair) == 1); CHECK_ILLEGAL(CTX, secp256k1_keypair_xonly_pub(CTX, NULL, &pk_parity, &keypair)); @@ -353,7 +353,7 @@ static void test_keypair(void) { CHECK(pk_parity == pk_parity_tmp); /* Test keypair_seckey */ - secp256k1_testrand256(sk); + testrand256(sk); CHECK(secp256k1_keypair_create(CTX, &keypair, sk) == 1); CHECK(secp256k1_keypair_sec(CTX, sk_tmp, &keypair) == 1); CHECK_ILLEGAL(CTX, secp256k1_keypair_sec(CTX, NULL, &keypair)); @@ -381,8 +381,8 @@ static void test_keypair_add(void) { int i; CHECK(sizeof(zeros96) == sizeof(keypair)); - secp256k1_testrand256(sk); - secp256k1_testrand256(tweak); + testrand256(sk); + testrand256(tweak); memset(overflows, 0xFF, 32); CHECK(secp256k1_keypair_create(CTX, &keypair, sk) == 1); @@ -407,7 +407,7 @@ static void test_keypair_add(void) { for (i = 0; i < COUNT; i++) { secp256k1_scalar scalar_tweak; secp256k1_keypair keypair_tmp; - secp256k1_testrand256(sk); + testrand256(sk); CHECK(secp256k1_keypair_create(CTX, &keypair, sk) == 1); memcpy(&keypair_tmp, &keypair, sizeof(keypair)); /* Because sk may be negated before adding, we need to try with tweak = @@ -423,7 +423,7 @@ static void test_keypair_add(void) { /* Invalid keypair with a valid tweak */ memset(&keypair, 0, sizeof(keypair)); - secp256k1_testrand256(tweak); + testrand256(tweak); CHECK_ILLEGAL(CTX, secp256k1_keypair_xonly_tweak_add(CTX, &keypair, tweak)); CHECK(secp256k1_memcmp_var(&keypair, zeros96, sizeof(keypair)) == 0); /* Only seckey part of keypair invalid */ @@ -446,7 +446,7 @@ static void test_keypair_add(void) { unsigned char sk32[32]; int pk_parity; - secp256k1_testrand256(tweak); + testrand256(tweak); CHECK(secp256k1_keypair_xonly_pub(CTX, &internal_pk, NULL, &keypair) == 1); CHECK(secp256k1_keypair_xonly_tweak_add(CTX, &keypair, tweak) == 1); CHECK(secp256k1_keypair_xonly_pub(CTX, &output_pk, &pk_parity, &keypair) == 1); diff --git a/src/modules/recovery/tests_impl.h b/src/modules/recovery/tests_impl.h index 728ccfed8d..7a28a3ce65 100644 --- a/src/modules/recovery/tests_impl.h +++ b/src/modules/recovery/tests_impl.h @@ -25,7 +25,7 @@ static int recovery_test_nonce_function(unsigned char *nonce32, const unsigned c } /* On the next run, return a valid nonce, but flip a coin as to whether or not to fail signing. */ memset(nonce32, 1, 32); - return secp256k1_testrand_bits(1); + return testrand_bits(1); } static void test_ecdsa_recovery_api(void) { @@ -106,8 +106,8 @@ static void test_ecdsa_recovery_end_to_end(void) { /* Generate a random key and message. */ { secp256k1_scalar msg, key; - random_scalar_order_test(&msg); - random_scalar_order_test(&key); + testutil_random_scalar_order_test(&msg); + testutil_random_scalar_order_test(&key); secp256k1_scalar_get_b32(privkey, &key); secp256k1_scalar_get_b32(message, &msg); } @@ -141,7 +141,7 @@ static void test_ecdsa_recovery_end_to_end(void) { CHECK(secp256k1_memcmp_var(&pubkey, &recpubkey, sizeof(pubkey)) == 0); /* Serialize/destroy/parse signature and verify again. */ CHECK(secp256k1_ecdsa_recoverable_signature_serialize_compact(CTX, sig, &recid, &rsignature[4]) == 1); - sig[secp256k1_testrand_bits(6)] += 1 + secp256k1_testrand_int(255); + sig[testrand_bits(6)] += 1 + testrand_int(255); CHECK(secp256k1_ecdsa_recoverable_signature_parse_compact(CTX, &rsignature[4], sig, recid) == 1); CHECK(secp256k1_ecdsa_recoverable_signature_convert(CTX, &signature[4], &rsignature[4]) == 1); CHECK(secp256k1_ecdsa_verify(CTX, &signature[4], message, &pubkey) == 0); diff --git a/src/modules/schnorrsig/tests_exhaustive_impl.h b/src/modules/schnorrsig/tests_exhaustive_impl.h index bc31d81107..601b54975d 100644 --- a/src/modules/schnorrsig/tests_exhaustive_impl.h +++ b/src/modules/schnorrsig/tests_exhaustive_impl.h @@ -104,7 +104,7 @@ static void test_exhaustive_schnorrsig_verify(const secp256k1_context *ctx, cons while (e_count_done < EXHAUSTIVE_TEST_ORDER) { secp256k1_scalar e; unsigned char msg32[32]; - secp256k1_testrand256(msg32); + testrand256(msg32); secp256k1_schnorrsig_challenge(&e, sig64, msg32, sizeof(msg32), pk32); /* Only do work if we hit a challenge we haven't tried before. */ if (!e_done[e]) { @@ -120,7 +120,7 @@ static void test_exhaustive_schnorrsig_verify(const secp256k1_context *ctx, cons expect_valid = actual_k != -1 && s != EXHAUSTIVE_TEST_ORDER && (s == (actual_k + actual_d * e) % EXHAUSTIVE_TEST_ORDER); } else { - secp256k1_testrand256(sig64 + 32); + testrand256(sig64 + 32); expect_valid = 0; } valid = secp256k1_schnorrsig_verify(ctx, sig64, msg32, sizeof(msg32), &pubkeys[d - 1]); @@ -161,7 +161,7 @@ static void test_exhaustive_schnorrsig_sign(const secp256k1_context *ctx, unsign /* Generate random messages until all challenges have been tried. */ while (e_count_done < EXHAUSTIVE_TEST_ORDER) { secp256k1_scalar e; - secp256k1_testrand256(msg32); + testrand256(msg32); secp256k1_schnorrsig_challenge(&e, xonly_pubkey_bytes[k - 1], msg32, sizeof(msg32), xonly_pubkey_bytes[d - 1]); /* Only do work if we hit a challenge we haven't tried before. */ if (!e_done[e]) { diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h index 8ada90a87b..aa4fc38270 100644 --- a/src/modules/schnorrsig/tests_impl.h +++ b/src/modules/schnorrsig/tests_impl.h @@ -15,7 +15,7 @@ static void nonce_function_bip340_bitflip(unsigned char **args, size_t n_flip, size_t n_bytes, size_t msglen, size_t algolen) { unsigned char nonces[2][32]; CHECK(nonce_function_bip340(nonces[0], args[0], msglen, args[1], args[2], args[3], algolen, args[4]) == 1); - secp256k1_testrand_flip(args[n_flip], n_bytes); + testrand_flip(args[n_flip], n_bytes); CHECK(nonce_function_bip340(nonces[1], args[0], msglen, args[1], args[2], args[3], algolen, args[4]) == 1); CHECK(secp256k1_memcmp_var(nonces[0], nonces[1], 32) != 0); } @@ -50,10 +50,10 @@ static void run_nonce_function_bip340_tests(void) { secp256k1_nonce_function_bip340_sha256_tagged_aux(&sha_optimized); test_sha256_eq(&sha, &sha_optimized); - secp256k1_testrand256(msg); - secp256k1_testrand256(key); - secp256k1_testrand256(pk); - secp256k1_testrand256(aux_rand); + testrand256(msg); + testrand256(key); + testrand256(pk); + testrand256(aux_rand); /* Check that a bitflip in an argument results in different nonces. */ args[0] = msg; @@ -76,12 +76,12 @@ static void run_nonce_function_bip340_tests(void) { CHECK(nonce_function_bip340(nonce, msg, msglen, key, pk, NULL, 0, NULL) == 0); CHECK(nonce_function_bip340(nonce, msg, msglen, key, pk, algo, algolen, NULL) == 1); /* Other algo is fine */ - secp256k1_testrand_bytes_test(algo, algolen); + testrand_bytes_test(algo, algolen); CHECK(nonce_function_bip340(nonce, msg, msglen, key, pk, algo, algolen, NULL) == 1); for (i = 0; i < COUNT; i++) { unsigned char nonce2[32]; - uint32_t offset = secp256k1_testrand_int(msglen - 1); + uint32_t offset = testrand_int(msglen - 1); size_t msglen_tmp = (msglen + offset) % msglen; size_t algolen_tmp; @@ -90,7 +90,7 @@ static void run_nonce_function_bip340_tests(void) { CHECK(secp256k1_memcmp_var(nonce, nonce2, 32) != 0); /* Different algolen gives different nonce */ - offset = secp256k1_testrand_int(algolen - 1); + offset = testrand_int(algolen - 1); algolen_tmp = (algolen + offset) % algolen; CHECK(nonce_function_bip340(nonce2, msg, msglen, key, pk, algo, algolen_tmp, NULL) == 1); CHECK(secp256k1_memcmp_var(nonce, nonce2, 32) != 0); @@ -116,10 +116,10 @@ static void test_schnorrsig_api(void) { secp256k1_schnorrsig_extraparams extraparams = SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT; secp256k1_schnorrsig_extraparams invalid_extraparams = {{ 0 }, NULL, NULL}; - secp256k1_testrand256(sk1); - secp256k1_testrand256(sk2); - secp256k1_testrand256(sk3); - secp256k1_testrand256(msg); + testrand256(sk1); + testrand256(sk2); + testrand256(sk3); + testrand256(msg); CHECK(secp256k1_keypair_create(CTX, &keypairs[0], sk1) == 1); CHECK(secp256k1_keypair_create(CTX, &keypairs[1], sk2) == 1); CHECK(secp256k1_keypair_create(CTX, &keypairs[2], sk3) == 1); @@ -813,8 +813,8 @@ static void test_schnorrsig_sign(void) { secp256k1_schnorrsig_extraparams extraparams = SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT; unsigned char aux_rand[32]; - secp256k1_testrand256(sk); - secp256k1_testrand256(aux_rand); + testrand256(sk); + testrand256(aux_rand); CHECK(secp256k1_keypair_create(CTX, &keypair, sk)); CHECK(secp256k1_keypair_xonly_pub(CTX, &pk, NULL, &keypair)); CHECK(secp256k1_schnorrsig_sign32(CTX, sig, msg, &keypair, NULL) == 1); @@ -861,12 +861,12 @@ static void test_schnorrsig_sign_verify(void) { secp256k1_xonly_pubkey pk; secp256k1_scalar s; - secp256k1_testrand256(sk); + testrand256(sk); CHECK(secp256k1_keypair_create(CTX, &keypair, sk)); CHECK(secp256k1_keypair_xonly_pub(CTX, &pk, NULL, &keypair)); for (i = 0; i < N_SIGS; i++) { - secp256k1_testrand256(msg[i]); + testrand256(msg[i]); CHECK(secp256k1_schnorrsig_sign32(CTX, sig[i], msg[i], &keypair, NULL)); CHECK(secp256k1_schnorrsig_verify(CTX, sig[i], msg[i], sizeof(msg[i]), &pk)); } @@ -874,19 +874,19 @@ static void test_schnorrsig_sign_verify(void) { { /* Flip a few bits in the signature and in the message and check that * verify and verify_batch (TODO) fail */ - size_t sig_idx = secp256k1_testrand_int(N_SIGS); - size_t byte_idx = secp256k1_testrand_bits(5); - unsigned char xorbyte = secp256k1_testrand_int(254)+1; + size_t sig_idx = testrand_int(N_SIGS); + size_t byte_idx = testrand_bits(5); + unsigned char xorbyte = testrand_int(254)+1; sig[sig_idx][byte_idx] ^= xorbyte; CHECK(!secp256k1_schnorrsig_verify(CTX, sig[sig_idx], msg[sig_idx], sizeof(msg[sig_idx]), &pk)); sig[sig_idx][byte_idx] ^= xorbyte; - byte_idx = secp256k1_testrand_bits(5); + byte_idx = testrand_bits(5); sig[sig_idx][32+byte_idx] ^= xorbyte; CHECK(!secp256k1_schnorrsig_verify(CTX, sig[sig_idx], msg[sig_idx], sizeof(msg[sig_idx]), &pk)); sig[sig_idx][32+byte_idx] ^= xorbyte; - byte_idx = secp256k1_testrand_bits(5); + byte_idx = testrand_bits(5); msg[sig_idx][byte_idx] ^= xorbyte; CHECK(!secp256k1_schnorrsig_verify(CTX, sig[sig_idx], msg[sig_idx], sizeof(msg[sig_idx]), &pk)); msg[sig_idx][byte_idx] ^= xorbyte; @@ -916,9 +916,9 @@ static void test_schnorrsig_sign_verify(void) { { /* Test varying message lengths */ unsigned char msg_large[32 * 8]; - uint32_t msglen = secp256k1_testrand_int(sizeof(msg_large)); + uint32_t msglen = testrand_int(sizeof(msg_large)); for (i = 0; i < sizeof(msg_large); i += 32) { - secp256k1_testrand256(&msg_large[i]); + testrand256(&msg_large[i]); } CHECK(secp256k1_schnorrsig_sign_custom(CTX, sig[0], msg_large, msglen, &keypair, NULL) == 1); CHECK(secp256k1_schnorrsig_verify(CTX, sig[0], msg_large, msglen, &pk) == 1); @@ -942,7 +942,7 @@ static void test_schnorrsig_taproot(void) { unsigned char sig[64]; /* Create output key */ - secp256k1_testrand256(sk); + testrand256(sk); CHECK(secp256k1_keypair_create(CTX, &keypair, sk) == 1); CHECK(secp256k1_keypair_xonly_pub(CTX, &internal_pk, NULL, &keypair) == 1); /* In actual taproot the tweak would be hash of internal_pk */ @@ -952,7 +952,7 @@ static void test_schnorrsig_taproot(void) { CHECK(secp256k1_xonly_pubkey_serialize(CTX, output_pk_bytes, &output_pk) == 1); /* Key spend */ - secp256k1_testrand256(msg); + testrand256(msg); CHECK(secp256k1_schnorrsig_sign32(CTX, sig, msg, &keypair, NULL) == 1); /* Verify key spend */ CHECK(secp256k1_xonly_pubkey_parse(CTX, &output_pk, output_pk_bytes) == 1); diff --git a/src/secp256k1.c b/src/secp256k1.c index 7a32769408..72d725a74e 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -76,7 +76,7 @@ const secp256k1_context *secp256k1_context_no_precomp = &secp256k1_context_stati /* Helper function that determines if a context is proper, i.e., is not the static context or a copy thereof. * - * This is intended for "context" functions such as secp256k1_context_clone. Function which need specific + * This is intended for "context" functions such as secp256k1_context_clone. Functions that need specific * features of a context should still check for these features directly. For example, a function that needs * ecmult_gen should directly check for the existence of the ecmult_gen context. */ static int secp256k1_context_is_proper(const secp256k1_context* ctx) { @@ -544,7 +544,7 @@ static int secp256k1_ecdsa_sign_inner(const secp256k1_context* ctx, secp256k1_sc break; } is_nonce_valid = secp256k1_scalar_set_b32_seckey(&non, nonce32); - /* The nonce is still secret here, but it being invalid is is less likely than 1:2^255. */ + /* The nonce is still secret here, but it being invalid is less likely than 1:2^255. */ secp256k1_declassify(ctx, &is_nonce_valid, sizeof(is_nonce_valid)); if (is_nonce_valid) { ret = secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, r, s, &sec, &msg, &non, recid); diff --git a/src/testrand.h b/src/testrand.h index 721099d039..3c1ed3d45d 100644 --- a/src/testrand.h +++ b/src/testrand.h @@ -12,37 +12,37 @@ /* A non-cryptographic RNG used only for test infrastructure. */ /** Seed the pseudorandom number generator for testing. */ -SECP256K1_INLINE static void secp256k1_testrand_seed(const unsigned char *seed16); +SECP256K1_INLINE static void testrand_seed(const unsigned char *seed16); /** Generate a pseudorandom number in the range [0..2**32-1]. */ -SECP256K1_INLINE static uint32_t secp256k1_testrand32(void); +SECP256K1_INLINE static uint32_t testrand32(void); /** Generate a pseudorandom number in the range [0..2**64-1]. */ -SECP256K1_INLINE static uint64_t secp256k1_testrand64(void); +SECP256K1_INLINE static uint64_t testrand64(void); /** Generate a pseudorandom number in the range [0..2**bits-1]. Bits must be 1 or * more. */ -SECP256K1_INLINE static uint64_t secp256k1_testrand_bits(int bits); +SECP256K1_INLINE static uint64_t testrand_bits(int bits); /** Generate a pseudorandom number in the range [0..range-1]. */ -static uint32_t secp256k1_testrand_int(uint32_t range); +static uint32_t testrand_int(uint32_t range); /** Generate a pseudorandom 32-byte array. */ -static void secp256k1_testrand256(unsigned char *b32); +static void testrand256(unsigned char *b32); /** Generate a pseudorandom 32-byte array with long sequences of zero and one bits. */ -static void secp256k1_testrand256_test(unsigned char *b32); +static void testrand256_test(unsigned char *b32); /** Generate pseudorandom bytes with long sequences of zero and one bits. */ -static void secp256k1_testrand_bytes_test(unsigned char *bytes, size_t len); +static void testrand_bytes_test(unsigned char *bytes, size_t len); /** Flip a single random bit in a byte array */ -static void secp256k1_testrand_flip(unsigned char *b, size_t len); +static void testrand_flip(unsigned char *b, size_t len); /** Initialize the test RNG using (hex encoded) array up to 16 bytes, or randomly if hexseed is NULL. */ -static void secp256k1_testrand_init(const char* hexseed); +static void testrand_init(const char* hexseed); /** Print final test information. */ -static void secp256k1_testrand_finish(void); +static void testrand_finish(void); #endif /* SECP256K1_TESTRAND_H */ diff --git a/src/testrand_impl.h b/src/testrand_impl.h index fe97620435..07564f7f3f 100644 --- a/src/testrand_impl.h +++ b/src/testrand_impl.h @@ -17,7 +17,7 @@ static uint64_t secp256k1_test_state[4]; -SECP256K1_INLINE static void secp256k1_testrand_seed(const unsigned char *seed16) { +SECP256K1_INLINE static void testrand_seed(const unsigned char *seed16) { static const unsigned char PREFIX[19] = "secp256k1 test init"; unsigned char out32[32]; secp256k1_sha256 hash; @@ -40,7 +40,7 @@ SECP256K1_INLINE static uint64_t rotl(const uint64_t x, int k) { return (x << k) | (x >> (64 - k)); } -SECP256K1_INLINE static uint64_t secp256k1_testrand64(void) { +SECP256K1_INLINE static uint64_t testrand64(void) { /* Test-only Xoshiro256++ RNG. See https://prng.di.unimi.it/ */ const uint64_t result = rotl(secp256k1_test_state[0] + secp256k1_test_state[3], 23) + secp256k1_test_state[0]; const uint64_t t = secp256k1_test_state[1] << 17; @@ -53,16 +53,16 @@ SECP256K1_INLINE static uint64_t secp256k1_testrand64(void) { return result; } -SECP256K1_INLINE static uint64_t secp256k1_testrand_bits(int bits) { +SECP256K1_INLINE static uint64_t testrand_bits(int bits) { if (bits == 0) return 0; - return secp256k1_testrand64() >> (64 - bits); + return testrand64() >> (64 - bits); } -SECP256K1_INLINE static uint32_t secp256k1_testrand32(void) { - return secp256k1_testrand64() >> 32; +SECP256K1_INLINE static uint32_t testrand32(void) { + return testrand64() >> 32; } -static uint32_t secp256k1_testrand_int(uint32_t range) { +static uint32_t testrand_int(uint32_t range) { uint32_t mask = 0; uint32_t range_copy; /* Reduce range by 1, changing its meaning to "maximum value". */ @@ -76,15 +76,15 @@ static uint32_t secp256k1_testrand_int(uint32_t range) { } /* Generation loop. */ while (1) { - uint32_t val = secp256k1_testrand64() & mask; + uint32_t val = testrand64() & mask; if (val <= range) return val; } } -static void secp256k1_testrand256(unsigned char *b32) { +static void testrand256(unsigned char *b32) { int i; for (i = 0; i < 4; ++i) { - uint64_t val = secp256k1_testrand64(); + uint64_t val = testrand64(); b32[0] = val; b32[1] = val >> 8; b32[2] = val >> 16; @@ -97,14 +97,14 @@ static void secp256k1_testrand256(unsigned char *b32) { } } -static void secp256k1_testrand_bytes_test(unsigned char *bytes, size_t len) { +static void testrand_bytes_test(unsigned char *bytes, size_t len) { size_t bits = 0; memset(bytes, 0, len); while (bits < len * 8) { int now; uint32_t val; - now = 1 + (secp256k1_testrand_bits(6) * secp256k1_testrand_bits(5) + 16) / 31; - val = secp256k1_testrand_bits(1); + now = 1 + (testrand_bits(6) * testrand_bits(5) + 16) / 31; + val = testrand_bits(1); while (now > 0 && bits < len * 8) { bytes[bits / 8] |= val << (bits % 8); now--; @@ -113,15 +113,15 @@ static void secp256k1_testrand_bytes_test(unsigned char *bytes, size_t len) { } } -static void secp256k1_testrand256_test(unsigned char *b32) { - secp256k1_testrand_bytes_test(b32, 32); +static void testrand256_test(unsigned char *b32) { + testrand_bytes_test(b32, 32); } -static void secp256k1_testrand_flip(unsigned char *b, size_t len) { - b[secp256k1_testrand_int(len)] ^= (1 << secp256k1_testrand_bits(3)); +static void testrand_flip(unsigned char *b, size_t len) { + b[testrand_int(len)] ^= (1 << testrand_bits(3)); } -static void secp256k1_testrand_init(const char* hexseed) { +static void testrand_init(const char* hexseed) { unsigned char seed16[16] = {0}; if (hexseed && strlen(hexseed) != 0) { int pos = 0; @@ -155,12 +155,12 @@ static void secp256k1_testrand_init(const char* hexseed) { } printf("random seed = %02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x\n", seed16[0], seed16[1], seed16[2], seed16[3], seed16[4], seed16[5], seed16[6], seed16[7], seed16[8], seed16[9], seed16[10], seed16[11], seed16[12], seed16[13], seed16[14], seed16[15]); - secp256k1_testrand_seed(seed16); + testrand_seed(seed16); } -static void secp256k1_testrand_finish(void) { +static void testrand_finish(void) { unsigned char run32[32]; - secp256k1_testrand256(run32); + testrand256(run32); printf("random run = %02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x\n", run32[0], run32[1], run32[2], run32[3], run32[4], run32[5], run32[6], run32[7], run32[8], run32[9], run32[10], run32[11], run32[12], run32[13], run32[14], run32[15]); } diff --git a/src/tests.c b/src/tests.c index dab47608c2..6b401e52c0 100644 --- a/src/tests.c +++ b/src/tests.c @@ -96,122 +96,6 @@ static void uncounting_illegal_callback_fn(const char* str, void* data) { (*p)--; } -static void random_field_element_magnitude(secp256k1_fe *fe, int m) { - secp256k1_fe zero; - int n = secp256k1_testrand_int(m + 1); - secp256k1_fe_normalize(fe); - if (n == 0) { - return; - } - secp256k1_fe_clear(&zero); - secp256k1_fe_negate(&zero, &zero, 0); - secp256k1_fe_mul_int_unchecked(&zero, n - 1); - secp256k1_fe_add(fe, &zero); -#ifdef VERIFY - CHECK(fe->magnitude == n); -#endif -} - -static void random_fe_test(secp256k1_fe *x) { - unsigned char bin[32]; - do { - secp256k1_testrand256_test(bin); - if (secp256k1_fe_set_b32_limit(x, bin)) { - return; - } - } while(1); -} - -static void random_fe_non_zero_test(secp256k1_fe *fe) { - do { - random_fe_test(fe); - } while(secp256k1_fe_is_zero(fe)); -} - -static void random_fe_magnitude(secp256k1_fe *fe) { - random_field_element_magnitude(fe, 8); -} - -static void random_ge_x_magnitude(secp256k1_ge *ge) { - random_field_element_magnitude(&ge->x, SECP256K1_GE_X_MAGNITUDE_MAX); -} - -static void random_ge_y_magnitude(secp256k1_ge *ge) { - random_field_element_magnitude(&ge->y, SECP256K1_GE_Y_MAGNITUDE_MAX); -} - -static void random_gej_x_magnitude(secp256k1_gej *gej) { - random_field_element_magnitude(&gej->x, SECP256K1_GEJ_X_MAGNITUDE_MAX); -} - -static void random_gej_y_magnitude(secp256k1_gej *gej) { - random_field_element_magnitude(&gej->y, SECP256K1_GEJ_Y_MAGNITUDE_MAX); -} - -static void random_gej_z_magnitude(secp256k1_gej *gej) { - random_field_element_magnitude(&gej->z, SECP256K1_GEJ_Z_MAGNITUDE_MAX); -} - -static void random_group_element_test(secp256k1_ge *ge) { - secp256k1_fe fe; - do { - random_fe_test(&fe); - if (secp256k1_ge_set_xo_var(ge, &fe, secp256k1_testrand_bits(1))) { - secp256k1_fe_normalize(&ge->y); - break; - } - } while(1); - ge->infinity = 0; -} - -static void random_group_element_jacobian_test(secp256k1_gej *gej, const secp256k1_ge *ge) { - secp256k1_fe z2, z3; - random_fe_non_zero_test(&gej->z); - secp256k1_fe_sqr(&z2, &gej->z); - secp256k1_fe_mul(&z3, &z2, &gej->z); - secp256k1_fe_mul(&gej->x, &ge->x, &z2); - secp256k1_fe_mul(&gej->y, &ge->y, &z3); - gej->infinity = ge->infinity; -} - -static void random_gej_test(secp256k1_gej *gej) { - secp256k1_ge ge; - random_group_element_test(&ge); - random_group_element_jacobian_test(gej, &ge); -} - -static void random_scalar_order_test(secp256k1_scalar *num) { - do { - unsigned char b32[32]; - int overflow = 0; - secp256k1_testrand256_test(b32); - secp256k1_scalar_set_b32(num, b32, &overflow); - if (overflow || secp256k1_scalar_is_zero(num)) { - continue; - } - break; - } while(1); -} - -static void random_scalar_order(secp256k1_scalar *num) { - do { - unsigned char b32[32]; - int overflow = 0; - secp256k1_testrand256(b32); - secp256k1_scalar_set_b32(num, b32, &overflow); - if (overflow || secp256k1_scalar_is_zero(num)) { - continue; - } - break; - } while(1); -} - -static void random_scalar_order_b32(unsigned char *b32) { - secp256k1_scalar num; - random_scalar_order(&num); - secp256k1_scalar_get_b32(b32, &num); -} - static void run_xoshiro256pp_tests(void) { { size_t i; @@ -233,9 +117,9 @@ static void run_xoshiro256pp_tests(void) { 0x4C, 0xCC, 0xC1, 0x18, 0xB2, 0xD8, 0x8F, 0xEF, 0x43, 0x26, 0x15, 0x57, 0x37, 0x00, 0xEF, 0x30, }; - secp256k1_testrand_seed(seed16); + testrand_seed(seed16); for (i = 0; i < 17; i++) { - secp256k1_testrand256(buf32); + testrand256(buf32); } CHECK(secp256k1_memcmp_var(buf32, buf32_expected, sizeof(buf32)) == 0); } @@ -444,14 +328,14 @@ static void run_proper_context_tests(int use_prealloc) { CHECK(context_eq(my_ctx, my_ctx_fresh)); /*** attempt to use them ***/ - random_scalar_order_test(&msg); - random_scalar_order_test(&key); + testutil_random_scalar_order_test(&msg); + testutil_random_scalar_order_test(&key); secp256k1_ecmult_gen(&my_ctx->ecmult_gen_ctx, &pubj, &key); secp256k1_ge_set_gej(&pub, &pubj); /* obtain a working nonce */ do { - random_scalar_order_test(&nonce); + testutil_random_scalar_order_test(&nonce); } while(!secp256k1_ecdsa_sig_sign(&my_ctx->ecmult_gen_ctx, &sigr, &sigs, &key, &msg, &nonce, NULL)); /* try signing */ @@ -608,7 +492,7 @@ static void run_sha256_known_output_tests(void) { CHECK(secp256k1_memcmp_var(out, outputs[i], 32) == 0); /* 2. Run: split the input bytestrings randomly before writing */ if (strlen(inputs[i]) > 0) { - int split = secp256k1_testrand_int(strlen(inputs[i])); + int split = testrand_int(strlen(inputs[i])); secp256k1_sha256_initialize(&hasher); j = repeat[i]; while (j > 0) { @@ -769,7 +653,7 @@ static void run_hmac_sha256_tests(void) { secp256k1_hmac_sha256_finalize(&hasher, out); CHECK(secp256k1_memcmp_var(out, outputs[i], 32) == 0); if (strlen(inputs[i]) > 0) { - int split = secp256k1_testrand_int(strlen(inputs[i])); + int split = testrand_int(strlen(inputs[i])); secp256k1_hmac_sha256_initialize(&hasher, (const unsigned char*)(keys[i]), strlen(keys[i])); secp256k1_hmac_sha256_write(&hasher, (const unsigned char*)(inputs[i]), split); secp256k1_hmac_sha256_write(&hasher, (const unsigned char*)(inputs[i] + split), strlen(inputs[i]) - split); @@ -969,7 +853,7 @@ static void signed30_to_uint16(uint16_t* out, const secp256k1_modinv32_signed30* static void mutate_sign_signed30(secp256k1_modinv32_signed30* x) { int i; for (i = 0; i < 16; ++i) { - int pos = secp256k1_testrand_bits(3); + int pos = testrand_bits(3); if (x->v[pos] > 0 && x->v[pos + 1] <= 0x3fffffff) { x->v[pos] -= 0x40000000; x->v[pos + 1] += 1; @@ -1061,7 +945,7 @@ static void mutate_sign_signed62(secp256k1_modinv64_signed62* x) { static const int64_t M62 = (int64_t)(UINT64_MAX >> 2); int i; for (i = 0; i < 8; ++i) { - int pos = secp256k1_testrand_bits(2); + int pos = testrand_bits(2); if (x->v[pos] > 0 && x->v[pos + 1] <= M62) { x->v[pos] -= (M62 + 1); x->v[pos + 1] += 1; @@ -1774,8 +1658,8 @@ static void run_modinv_tests(void) { /* generate random xd and md, so that md is odd, md>1, xd 256) { now = 256 - i; } @@ -2194,7 +2078,7 @@ static void scalar_test(void) { secp256k1_scalar b; int i; /* Test add_bit. */ - int bit = secp256k1_testrand_bits(8); + int bit = testrand_bits(8); secp256k1_scalar_set_int(&b, 1); CHECK(secp256k1_scalar_is_one(&b)); for (i = 0; i < bit; i++) { @@ -2287,7 +2171,7 @@ static void run_scalar_set_b32_seckey_tests(void) { secp256k1_scalar s2; /* Usually set_b32 and set_b32_seckey give the same result */ - random_scalar_order_b32(b32); + testutil_random_scalar_order_b32(b32); secp256k1_scalar_set_b32(&s1, b32, NULL); CHECK(secp256k1_scalar_set_b32_seckey(&s2, b32) == 1); CHECK(secp256k1_scalar_eq(&s1, &s2) == 1); @@ -2948,7 +2832,7 @@ static void run_scalar_tests(void) { static void random_fe_non_square(secp256k1_fe *ns) { secp256k1_fe r; - random_fe_non_zero(ns); + testutil_random_fe_non_zero(ns); if (secp256k1_fe_sqrt(&r, ns)) { secp256k1_fe_negate(ns, ns, 1); } @@ -3125,12 +3009,12 @@ static void run_field_misc(void) { for (i = 0; i < 1000 * COUNT; i++) { secp256k1_fe_storage xs, ys, zs; if (i & 1) { - random_fe(&x); + testutil_random_fe(&x); } else { - random_fe_test(&x); + testutil_random_fe_test(&x); } - random_fe_non_zero(&y); - v = secp256k1_testrand_bits(15); + testutil_random_fe_non_zero(&y); + v = testrand_bits(15); /* Test that fe_add_int is equivalent to fe_set_int + fe_add. */ secp256k1_fe_set_int(&q, v); /* q = v */ z = x; /* z = x */ @@ -3268,14 +3152,14 @@ static void run_fe_mul(void) { int i; for (i = 0; i < 100 * COUNT; ++i) { secp256k1_fe a, b, c, d; - random_fe(&a); - random_fe_magnitude(&a); - random_fe(&b); - random_fe_magnitude(&b); - random_fe_test(&c); - random_fe_magnitude(&c); - random_fe_test(&d); - random_fe_magnitude(&d); + testutil_random_fe(&a); + testutil_random_fe_magnitude(&a, 8); + testutil_random_fe(&b); + testutil_random_fe_magnitude(&b, 8); + testutil_random_fe_test(&c); + testutil_random_fe_magnitude(&c, 8); + testutil_random_fe_test(&d); + testutil_random_fe_magnitude(&d, 8); test_fe_mul(&a, &a, 1); test_fe_mul(&c, &c, 1); test_fe_mul(&a, &b, 0); @@ -3297,7 +3181,7 @@ static void run_sqr(void) { secp256k1_fe_normalize(&x); /* Check that (x+y)*(x-y) = x^2 - y*2 for some random values y */ - random_fe_test(&y); + testutil_random_fe_test(&y); lhs = x; secp256k1_fe_add(&lhs, &y); /* lhs = x+y */ @@ -3351,7 +3235,7 @@ static void run_sqrt(void) { int j; random_fe_non_square(&ns); for (j = 0; j < COUNT; j++) { - random_fe(&x); + testutil_random_fe(&x); secp256k1_fe_sqr(&s, &x); CHECK(secp256k1_fe_is_square_var(&s)); test_sqrt(&s, &x); @@ -3665,7 +3549,7 @@ static void run_inverse_tests(void) /* test 128*count random inputs; half with testrand256_test, half with testrand256 */ for (testrand = 0; testrand <= 1; ++testrand) { for (i = 0; i < 64 * COUNT; ++i) { - (testrand ? secp256k1_testrand256_test : secp256k1_testrand256)(b32); + (testrand ? testrand256_test : testrand256)(b32); secp256k1_scalar_set_b32(&x_scalar, b32, NULL); secp256k1_fe_set_b32_mod(&x_fe, b32); for (var = 0; var <= 1; ++var) { @@ -3731,8 +3615,8 @@ static void test_hsort(size_t element_len) { /* Test hsort with array of random length n */ for (i = 0; i < COUNT; i++) { - int n = secp256k1_testrand_int(NUM); - secp256k1_testrand_bytes_test(elements, n*element_len); + int n = testrand_int(NUM); + testrand_bytes_test(elements, n*element_len); secp256k1_hsort(elements, n, element_len, test_hsort_cmp, &data); test_hsort_is_sorted(elements, n, element_len); } @@ -3792,7 +3676,7 @@ static void test_ge(void) { for (i = 0; i < runs; i++) { int j, k; secp256k1_ge g; - random_group_element_test(&g); + testutil_random_ge_test(&g); if (i >= runs - 2) { secp256k1_ge_mul_lambda(&g, &ge[1]); CHECK(!secp256k1_ge_eq_var(&g, &ge[1])); @@ -3805,15 +3689,15 @@ static void test_ge(void) { secp256k1_ge_neg(&ge[3 + 4 * i], &g); secp256k1_ge_neg(&ge[4 + 4 * i], &g); secp256k1_gej_set_ge(&gej[1 + 4 * i], &ge[1 + 4 * i]); - random_group_element_jacobian_test(&gej[2 + 4 * i], &ge[2 + 4 * i]); + testutil_random_ge_jacobian_test(&gej[2 + 4 * i], &ge[2 + 4 * i]); secp256k1_gej_set_ge(&gej[3 + 4 * i], &ge[3 + 4 * i]); - random_group_element_jacobian_test(&gej[4 + 4 * i], &ge[4 + 4 * i]); + testutil_random_ge_jacobian_test(&gej[4 + 4 * i], &ge[4 + 4 * i]); for (j = 0; j < 4; j++) { - random_ge_x_magnitude(&ge[1 + j + 4 * i]); - random_ge_y_magnitude(&ge[1 + j + 4 * i]); - random_gej_x_magnitude(&gej[1 + j + 4 * i]); - random_gej_y_magnitude(&gej[1 + j + 4 * i]); - random_gej_z_magnitude(&gej[1 + j + 4 * i]); + testutil_random_ge_x_magnitude(&ge[1 + j + 4 * i]); + testutil_random_ge_y_magnitude(&ge[1 + j + 4 * i]); + testutil_random_gej_x_magnitude(&gej[1 + j + 4 * i]); + testutil_random_gej_y_magnitude(&gej[1 + j + 4 * i]); + testutil_random_gej_z_magnitude(&gej[1 + j + 4 * i]); } for (j = 0; j < 4; ++j) { @@ -3828,14 +3712,14 @@ static void test_ge(void) { } /* Generate random zf, and zfi2 = 1/zf^2, zfi3 = 1/zf^3 */ - random_fe_non_zero_test(&zf); - random_fe_magnitude(&zf); + testutil_random_fe_non_zero_test(&zf); + testutil_random_fe_magnitude(&zf, 8); secp256k1_fe_inv_var(&zfi3, &zf); secp256k1_fe_sqr(&zfi2, &zfi3); secp256k1_fe_mul(&zfi3, &zfi3, &zfi2); /* Generate random r */ - random_fe_non_zero_test(&r); + testutil_random_fe_non_zero_test(&r); for (i1 = 0; i1 < 1 + 4 * runs; i1++) { int i2; @@ -3865,8 +3749,8 @@ static void test_ge(void) { secp256k1_ge ge2_zfi = ge[i2]; /* the second term with x and y rescaled for z = 1/zf */ secp256k1_fe_mul(&ge2_zfi.x, &ge2_zfi.x, &zfi2); secp256k1_fe_mul(&ge2_zfi.y, &ge2_zfi.y, &zfi3); - random_ge_x_magnitude(&ge2_zfi); - random_ge_y_magnitude(&ge2_zfi); + testutil_random_ge_x_magnitude(&ge2_zfi); + testutil_random_ge_y_magnitude(&ge2_zfi); secp256k1_gej_add_zinv_var(&resj, &gej[i1], &ge2_zfi, &zf); CHECK(secp256k1_gej_eq_ge_var(&resj, &ref)); } @@ -3922,7 +3806,7 @@ static void test_ge(void) { gej_shuffled[i] = gej[i]; } for (i = 0; i < 4 * runs + 1; i++) { - int swap = i + secp256k1_testrand_int(4 * runs + 1 - i); + int swap = i + testrand_int(4 * runs + 1 - i); if (swap != i) { secp256k1_gej t = gej_shuffled[i]; gej_shuffled[i] = gej_shuffled[swap]; @@ -3942,7 +3826,7 @@ static void test_ge(void) { secp256k1_ge_set_all_gej_var(ge_set_all, gej, 4 * runs + 1); for (i = 0; i < 4 * runs + 1; i++) { secp256k1_fe s; - random_fe_non_zero(&s); + testutil_random_fe_non_zero(&s); secp256k1_gej_rescale(&gej[i], &s); CHECK(secp256k1_gej_eq_ge_var(&gej[i], &ge_set_all[i])); } @@ -3975,7 +3859,7 @@ static void test_ge(void) { /* Test batch gej -> ge conversion with many infinities. */ for (i = 0; i < 4 * runs + 1; i++) { int odd; - random_group_element_test(&ge[i]); + testutil_random_ge_test(&ge[i]); odd = secp256k1_fe_is_odd(&ge[i].x); CHECK(odd == 0 || odd == 1); /* randomly set half the points to infinity */ @@ -4012,7 +3896,7 @@ static void test_intialized_inf(void) { secp256k1_fe zinv; /* Test that adding P+(-P) results in a fully initialized infinity*/ - random_group_element_test(&p); + testutil_random_ge_test(&p); secp256k1_gej_set_ge(&pj, &p); secp256k1_gej_neg(&npj, &pj); @@ -4125,14 +4009,14 @@ static void run_gej(void) { secp256k1_gej_set_infinity(&b); test_gej_cmov(&a, &b); - random_gej_test(&a); + testutil_random_gej_test(&a); test_gej_cmov(&a, &b); test_gej_cmov(&b, &a); b = a; test_gej_cmov(&a, &b); - random_gej_test(&b); + testutil_random_gej_test(&b); test_gej_cmov(&a, &b); test_gej_cmov(&b, &a); } @@ -4140,12 +4024,12 @@ static void run_gej(void) { /* Tests for secp256k1_gej_eq_var */ for (i = 0; i < COUNT; i++) { secp256k1_fe fe; - random_gej_test(&a); - random_gej_test(&b); + testutil_random_gej_test(&a); + testutil_random_gej_test(&b); CHECK(!secp256k1_gej_eq_var(&a, &b)); b = a; - random_fe_non_zero_test(&fe); + testutil_random_fe_non_zero_test(&fe); secp256k1_gej_rescale(&a, &fe); CHECK(secp256k1_gej_eq_var(&a, &b)); } @@ -4162,7 +4046,7 @@ static void test_ec_combine(void) { int i; for (i = 1; i <= 6; i++) { secp256k1_scalar s; - random_scalar_order_test(&s); + testutil_random_scalar_order_test(&s); secp256k1_scalar_add(&sum, &sum, &s); secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &Qj, &s); secp256k1_ge_set_gej(&Q, &Qj); @@ -4222,7 +4106,7 @@ static void run_group_decompress(void) { int i; for (i = 0; i < COUNT * 4; i++) { secp256k1_fe fe; - random_fe_test(&fe); + testutil_random_fe_test(&fe); test_group_decompress(&fe); } } @@ -4370,7 +4254,7 @@ static void test_point_times_order(const secp256k1_gej *point) { secp256k1_ge res3; unsigned char pub[65]; size_t psize = 65; - random_scalar_order_test(&x); + testutil_random_scalar_order_test(&x); secp256k1_scalar_negate(&nx, &x); secp256k1_ecmult(&res1, point, &x, &x); /* calc res1 = x * point + x * G; */ secp256k1_ecmult(&res2, point, &nx, &nx); /* calc res2 = (order - x) * point + (order - x) * G; */ @@ -4431,13 +4315,13 @@ static void test_ecmult_target(const secp256k1_scalar* target, int mode) { secp256k1_gej pj, p1j, p2j, ptj; /* Generate random n1,n2 such that n1+n2 = -target. */ - random_scalar_order_test(&n1); + testutil_random_scalar_order_test(&n1); secp256k1_scalar_add(&n2, &n1, target); secp256k1_scalar_negate(&n2, &n2); /* Generate a random input point. */ if (mode != 0) { - random_group_element_test(&p); + testutil_random_ge_test(&p); secp256k1_gej_set_ge(&pj, &p); } @@ -4529,8 +4413,8 @@ static void ecmult_const_commutativity(void) { secp256k1_gej res2; secp256k1_ge mid1; secp256k1_ge mid2; - random_scalar_order_test(&a); - random_scalar_order_test(&b); + testutil_random_scalar_order_test(&a); + testutil_random_scalar_order_test(&b); secp256k1_ecmult_const(&res1, &secp256k1_ge_const_g, &a); secp256k1_ecmult_const(&res2, &secp256k1_ge_const_g, &b); @@ -4551,9 +4435,9 @@ static void ecmult_const_mult_zero_one(void) { secp256k1_ge point; secp256k1_ge inf; - random_scalar_order_test(&s); + testutil_random_scalar_order_test(&s); secp256k1_scalar_negate(&negone, &secp256k1_scalar_one); - random_group_element_test(&point); + testutil_random_ge_test(&point); secp256k1_ge_set_infinity(&inf); /* 0*point */ @@ -4606,7 +4490,7 @@ static void ecmult_const_edges(void) { secp256k1_scalar_add(&q, &q, &scalars_near_split_bounds[i - 1]); secp256k1_scalar_add(&q, &q, &scalars_near_split_bounds[i - 1]); } - random_group_element_test(&point); + testutil_random_ge_test(&point); secp256k1_ecmult_const(&res, &point, &q); ecmult_const_check_result(&point, &q, &res); } @@ -4623,12 +4507,12 @@ static void ecmult_const_mult_xonly(void) { secp256k1_scalar q; int res; /* Random base point. */ - random_group_element_test(&base); + testutil_random_ge_test(&base); /* Random scalar to multiply it with. */ - random_scalar_order_test(&q); + testutil_random_scalar_order_test(&q); /* If i is odd, n=d*base.x for random non-zero d */ if (i & 1) { - random_fe_non_zero_test(&d); + testutil_random_fe_non_zero_test(&d); secp256k1_fe_mul(&n, &base.x, &d); } else { n = base.x; @@ -4650,14 +4534,14 @@ static void ecmult_const_mult_xonly(void) { secp256k1_fe x, n, d, r; int res; secp256k1_scalar q; - random_scalar_order_test(&q); + testutil_random_scalar_order_test(&q); /* Generate random X coordinate not on the curve. */ do { - random_fe_test(&x); + testutil_random_fe_test(&x); } while (secp256k1_ge_x_on_curve_var(&x)); /* If i is odd, n=d*x for random non-zero d. */ if (i & 1) { - random_fe_non_zero_test(&d); + testutil_random_fe_non_zero_test(&d); secp256k1_fe_mul(&n, &x, &d); } else { n = x; @@ -4740,10 +4624,10 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi for (ncount = 0; ncount < COUNT; ncount++) { secp256k1_ge ptg; secp256k1_gej ptgj; - random_scalar_order(&sc[0]); - random_scalar_order(&sc[1]); + testutil_random_scalar_order(&sc[0]); + testutil_random_scalar_order(&sc[1]); - random_group_element_test(&ptg); + testutil_random_ge_test(&ptg); secp256k1_gej_set_ge(&ptgj, &ptg); pt[0] = ptg; pt[1] = secp256k1_ge_const_g; @@ -4780,7 +4664,7 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi for (j = 0; j < 3; j++) { for (i = 0; i < 32; i++) { - random_scalar_order(&sc[i]); + testutil_random_scalar_order(&sc[i]); secp256k1_ge_set_infinity(&pt[i]); } CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, sizes[j])); @@ -4789,7 +4673,7 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi for (j = 0; j < 3; j++) { for (i = 0; i < 32; i++) { - random_group_element_test(&ptg); + testutil_random_ge_test(&ptg); pt[i] = ptg; secp256k1_scalar_set_int(&sc[i], 0); } @@ -4798,9 +4682,9 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi } for (j = 0; j < 3; j++) { - random_group_element_test(&ptg); + testutil_random_ge_test(&ptg); for (i = 0; i < 16; i++) { - random_scalar_order(&sc[2*i]); + testutil_random_scalar_order(&sc[2*i]); secp256k1_scalar_negate(&sc[2*i + 1], &sc[2*i]); pt[2 * i] = ptg; pt[2 * i + 1] = ptg; @@ -4809,9 +4693,9 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi CHECK(ecmult_multi(&CTX->error_callback, scratch, &r, &secp256k1_scalar_zero, ecmult_multi_callback, &data, sizes[j])); CHECK(secp256k1_gej_is_infinity(&r)); - random_scalar_order(&sc[0]); + testutil_random_scalar_order(&sc[0]); for (i = 0; i < 16; i++) { - random_group_element_test(&ptg); + testutil_random_ge_test(&ptg); sc[2*i] = sc[0]; sc[2*i+1] = sc[0]; @@ -4823,13 +4707,13 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi CHECK(secp256k1_gej_is_infinity(&r)); } - random_group_element_test(&ptg); + testutil_random_ge_test(&ptg); secp256k1_scalar_set_int(&sc[0], 0); pt[0] = ptg; for (i = 1; i < 32; i++) { pt[i] = ptg; - random_scalar_order(&sc[i]); + testutil_random_scalar_order(&sc[i]); secp256k1_scalar_add(&sc[0], &sc[0], &sc[i]); secp256k1_scalar_negate(&sc[i], &sc[i]); } @@ -4843,11 +4727,11 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi size_t i; secp256k1_gej_set_infinity(&r); - random_scalar_order(&sc[0]); + testutil_random_scalar_order(&sc[0]); for (i = 0; i < 20; i++) { secp256k1_ge ptg; sc[i] = sc[0]; - random_group_element_test(&ptg); + testutil_random_ge_test(&ptg); pt[i] = ptg; secp256k1_gej_add_ge_var(&r, &r, &pt[i], NULL); } @@ -4865,9 +4749,9 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi secp256k1_scalar rs; secp256k1_scalar_set_int(&rs, 0); - random_group_element_test(&ptg); + testutil_random_ge_test(&ptg); for (i = 0; i < 20; i++) { - random_scalar_order(&sc[i]); + testutil_random_scalar_order(&sc[i]); pt[i] = ptg; secp256k1_scalar_add(&rs, &rs, &sc[i]); } @@ -4880,8 +4764,8 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi /* Sanity check that zero scalars don't cause problems */ for (ncount = 0; ncount < 20; ncount++) { - random_scalar_order(&sc[ncount]); - random_group_element_test(&pt[ncount]); + testutil_random_scalar_order(&sc[ncount]); + testutil_random_ge_test(&pt[ncount]); } secp256k1_scalar_clear(&sc[0]); @@ -4902,7 +4786,7 @@ static void test_ecmult_multi(secp256k1_scratch *scratch, secp256k1_ecmult_multi secp256k1_ge ptg; secp256k1_gej ptgj; - random_group_element_test(&ptg); + testutil_random_ge_test(&ptg); secp256k1_gej_set_ge(&ptgj, &ptg); for(t0i = 0; t0i < TOP; t0i++) { @@ -4973,29 +4857,29 @@ static int test_ecmult_multi_random(secp256k1_scratch *scratch) { int i; /* Which multiplication function to use */ - int fn = secp256k1_testrand_int(3); + int fn = testrand_int(3); secp256k1_ecmult_multi_func ecmult_multi = fn == 0 ? secp256k1_ecmult_multi_var : fn == 1 ? secp256k1_ecmult_strauss_batch_single : secp256k1_ecmult_pippenger_batch_single; /* Simulate exponentially distributed num. */ - int num_bits = 2 + secp256k1_testrand_int(6); + int num_bits = 2 + testrand_int(6); /* Number of (scalar, point) inputs (excluding g). */ - int num = secp256k1_testrand_int((1 << num_bits) + 1); + int num = testrand_int((1 << num_bits) + 1); /* Number of those which are nonzero. */ - int num_nonzero = secp256k1_testrand_int(num + 1); + int num_nonzero = testrand_int(num + 1); /* Whether we're aiming to create an input with nonzero expected result. */ - int nonzero_result = secp256k1_testrand_bits(1); + int nonzero_result = testrand_bits(1); /* Whether we will provide nonzero g multiplicand. In some cases our hand * is forced here based on num_nonzero and nonzero_result. */ int g_nonzero = num_nonzero == 0 ? nonzero_result : num_nonzero == 1 && !nonzero_result ? 1 : - (int)secp256k1_testrand_bits(1); + (int)testrand_bits(1); /* Which g_scalar pointer to pass into ecmult_multi(). */ - const secp256k1_scalar* g_scalar_ptr = (g_nonzero || secp256k1_testrand_bits(1)) ? &g_scalar : NULL; + const secp256k1_scalar* g_scalar_ptr = (g_nonzero || testrand_bits(1)) ? &g_scalar : NULL; /* How many EC multiplications were performed in this function. */ int mults = 0; /* How many randomization steps to apply to the input list. */ - int rands = (int)secp256k1_testrand_bits(3); + int rands = (int)testrand_bits(3); if (rands > num_nonzero) rands = num_nonzero; secp256k1_gej_set_infinity(&expected); @@ -5004,11 +4888,11 @@ static int test_ecmult_multi_random(secp256k1_scratch *scratch) { if (g_nonzero) { /* If g_nonzero, set g_scalar to nonzero value r. */ - random_scalar_order_test(&g_scalar); + testutil_random_scalar_order_test(&g_scalar); if (!nonzero_result) { /* If expected=0 is desired, add a (a*r, -(1/a)*g) term to compensate. */ CHECK(num_nonzero > filled); - random_scalar_order_test(&sc_tmp); + testutil_random_scalar_order_test(&sc_tmp); secp256k1_scalar_mul(&scalars[filled], &sc_tmp, &g_scalar); secp256k1_scalar_inverse_var(&sc_tmp, &sc_tmp); secp256k1_scalar_negate(&sc_tmp, &sc_tmp); @@ -5020,8 +4904,8 @@ static int test_ecmult_multi_random(secp256k1_scratch *scratch) { if (nonzero_result && filled < num_nonzero) { /* If a nonzero result is desired, and there is space, add a random nonzero term. */ - random_scalar_order_test(&scalars[filled]); - random_group_element_test(&ge_tmp); + testutil_random_scalar_order_test(&scalars[filled]); + testutil_random_ge_test(&ge_tmp); secp256k1_gej_set_ge(&gejs[filled], &ge_tmp); ++filled; } @@ -5040,12 +4924,12 @@ static int test_ecmult_multi_random(secp256k1_scratch *scratch) { /* Add entries to scalars,gejs so that there are num of them. All the added entries * either have scalar=0 or point=infinity, so these do not change the expected result. */ while (filled < num) { - if (secp256k1_testrand_bits(1)) { + if (testrand_bits(1)) { secp256k1_gej_set_infinity(&gejs[filled]); - random_scalar_order_test(&scalars[filled]); + testutil_random_scalar_order_test(&scalars[filled]); } else { secp256k1_scalar_set_int(&scalars[filled], 0); - random_group_element_test(&ge_tmp); + testutil_random_ge_test(&ge_tmp); secp256k1_gej_set_ge(&gejs[filled], &ge_tmp); } ++filled; @@ -5059,7 +4943,7 @@ static int test_ecmult_multi_random(secp256k1_scratch *scratch) { secp256k1_scalar v, iv; /* Shuffle the entries. */ for (j = 0; j < num_nonzero; ++j) { - int k = secp256k1_testrand_int(num_nonzero - j); + int k = testrand_int(num_nonzero - j); if (k != 0) { secp256k1_gej gej = gejs[j]; secp256k1_scalar sc = scalars[j]; @@ -5079,7 +4963,7 @@ static int test_ecmult_multi_random(secp256k1_scratch *scratch) { } /* Transform the last input: a*P -> (v*a) * ((1/v)*P). */ CHECK(num_nonzero >= 1); - random_scalar_order_test(&v); + testutil_random_scalar_order_test(&v); secp256k1_scalar_inverse(&iv, &v); secp256k1_scalar_mul(&scalars[num_nonzero - 1], &scalars[num_nonzero - 1], &v); secp256k1_ecmult(&gejs[num_nonzero - 1], &gejs[num_nonzero - 1], &iv, NULL); @@ -5088,7 +4972,7 @@ static int test_ecmult_multi_random(secp256k1_scratch *scratch) { /* Shuffle all entries (0..num-1). */ for (i = 0; i < num; ++i) { - int j = secp256k1_testrand_int(num - i); + int j = testrand_int(num - i); if (j != 0) { secp256k1_gej gej = gejs[i]; secp256k1_scalar sc = scalars[i]; @@ -5118,8 +5002,8 @@ static void test_ecmult_multi_batch_single(secp256k1_ecmult_multi_func ecmult_mu ecmult_multi_data data; secp256k1_scratch *scratch_empty; - random_group_element_test(&pt); - random_scalar_order(&sc); + testutil_random_ge_test(&pt); + testutil_random_scalar_order(&sc); data.sc = ≻ data.pt = &pt; @@ -5150,7 +5034,7 @@ static void test_secp256k1_pippenger_bucket_window_inv(void) { * for a given scratch space. */ static void test_ecmult_multi_pippenger_max_points(void) { - size_t scratch_size = secp256k1_testrand_bits(8); + size_t scratch_size = testrand_bits(8); size_t max_size = secp256k1_pippenger_scratch_size(secp256k1_pippenger_bucket_window_inv(PIPPENGER_MAX_BUCKET_WINDOW-1)+512, 12); secp256k1_scratch *scratch; size_t n_points_supported; @@ -5244,15 +5128,15 @@ static void test_ecmult_multi_batching(void) { secp256k1_gej_set_infinity(&r2); /* Get random scalars and group elements and compute result */ - random_scalar_order(&scG); + testutil_random_scalar_order(&scG); secp256k1_ecmult(&r2, &r2, &secp256k1_scalar_zero, &scG); for(i = 0; i < n_points; i++) { secp256k1_ge ptg; secp256k1_gej ptgj; - random_group_element_test(&ptg); + testutil_random_ge_test(&ptg); secp256k1_gej_set_ge(&ptgj, &ptg); pt[i] = ptg; - random_scalar_order(&sc[i]); + testutil_random_scalar_order(&sc[i]); secp256k1_ecmult(&ptgj, &ptgj, &sc[i], NULL); secp256k1_gej_add_var(&r2, &r2, &ptgj, NULL); } @@ -5464,7 +5348,7 @@ static void run_wnaf(void) { test_fixed_wnaf_small(); /* Random tests */ for (i = 0; i < COUNT; i++) { - random_scalar_order(&n); + testutil_random_scalar_order(&n); test_wnaf(&n, 4+(i%10)); test_fixed_wnaf(&n, 4 + (i % 10)); } @@ -5645,9 +5529,9 @@ static void test_ecmult_gen_blind(void) { secp256k1_gej pgej2; secp256k1_ge p; secp256k1_ge pge; - random_scalar_order_test(&key); + testutil_random_scalar_order_test(&key); secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &pgej, &key); - secp256k1_testrand256(seed32); + testrand256(seed32); b = CTX->ecmult_gen_ctx.scalar_offset; p = CTX->ecmult_gen_ctx.ge_offset; secp256k1_ecmult_gen_blind(&CTX->ecmult_gen_ctx, seed32); @@ -5741,7 +5625,7 @@ static void run_endomorphism_tests(void) { for (i = 0; i < 100U * COUNT; ++i) { secp256k1_scalar full; - random_scalar_order_test(&full); + testutil_random_scalar_order_test(&full); test_scalar_split(&full); } for (i = 0; i < sizeof(scalars_near_split_bounds) / sizeof(scalars_near_split_bounds[0]); ++i) { @@ -6327,7 +6211,7 @@ static void run_eckey_negate_test(void) { unsigned char seckey[32]; unsigned char seckey_tmp[32]; - random_scalar_order_b32(seckey); + testutil_random_scalar_order_b32(seckey); memcpy(seckey_tmp, seckey, 32); /* Verify negation changes the key and changes it back */ @@ -6351,7 +6235,7 @@ static void run_eckey_negate_test(void) { /* Negating an overflowing seckey fails and the seckey is zeroed. In this * test, the seckey has 16 random bytes to ensure that ec_seckey_negate * doesn't just set seckey to a constant value in case of failure. */ - random_scalar_order_b32(seckey); + testutil_random_scalar_order_b32(seckey); memset(seckey, 0xFF, 16); memset(seckey_tmp, 0, 32); CHECK(secp256k1_ec_seckey_negate(CTX, seckey) == 0); @@ -6361,7 +6245,7 @@ static void run_eckey_negate_test(void) { static void random_sign(secp256k1_scalar *sigr, secp256k1_scalar *sigs, const secp256k1_scalar *key, const secp256k1_scalar *msg, int *recid) { secp256k1_scalar nonce; do { - random_scalar_order_test(&nonce); + testutil_random_scalar_order_test(&nonce); } while(!secp256k1_ecdsa_sig_sign(&CTX->ecmult_gen_ctx, sigr, sigs, key, msg, &nonce, recid)); } @@ -6373,11 +6257,11 @@ static void test_ecdsa_sign_verify(void) { secp256k1_scalar sigr, sigs; int getrec; int recid; - random_scalar_order_test(&msg); - random_scalar_order_test(&key); + testutil_random_scalar_order_test(&msg); + testutil_random_scalar_order_test(&key); secp256k1_ecmult_gen(&CTX->ecmult_gen_ctx, &pubj, &key); secp256k1_ge_set_gej(&pub, &pubj); - getrec = secp256k1_testrand_bits(1); + getrec = testrand_bits(1); /* The specific way in which this conditional is written sidesteps a potential bug in clang. See the commit messages of the commit that introduced this comment for details. */ if (getrec) { @@ -6470,8 +6354,8 @@ static void test_ecdsa_end_to_end(void) { /* Generate a random key and message. */ { secp256k1_scalar msg, key; - random_scalar_order_test(&msg); - random_scalar_order_test(&key); + testutil_random_scalar_order_test(&msg); + testutil_random_scalar_order_test(&key); secp256k1_scalar_get_b32(privkey, &key); secp256k1_scalar_get_b32(message, &msg); } @@ -6481,7 +6365,7 @@ static void test_ecdsa_end_to_end(void) { CHECK(secp256k1_ec_pubkey_create(CTX, &pubkey, privkey) == 1); /* Verify exporting and importing public key. */ - CHECK(secp256k1_ec_pubkey_serialize(CTX, pubkeyc, &pubkeyclen, &pubkey, secp256k1_testrand_bits(1) == 1 ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED)); + CHECK(secp256k1_ec_pubkey_serialize(CTX, pubkeyc, &pubkeyclen, &pubkey, testrand_bits(1) == 1 ? SECP256K1_EC_COMPRESSED : SECP256K1_EC_UNCOMPRESSED)); memset(&pubkey, 0, sizeof(pubkey)); CHECK(secp256k1_ec_pubkey_parse(CTX, &pubkey, pubkeyc, pubkeyclen) == 1); @@ -6493,19 +6377,19 @@ static void test_ecdsa_end_to_end(void) { CHECK(secp256k1_memcmp_var(&pubkey_tmp, &pubkey, sizeof(pubkey)) == 0); /* Verify private key import and export. */ - CHECK(ec_privkey_export_der(CTX, seckey, &seckeylen, privkey, secp256k1_testrand_bits(1) == 1)); + CHECK(ec_privkey_export_der(CTX, seckey, &seckeylen, privkey, testrand_bits(1) == 1)); CHECK(ec_privkey_import_der(CTX, privkey2, seckey, seckeylen) == 1); CHECK(secp256k1_memcmp_var(privkey, privkey2, 32) == 0); /* Optionally tweak the keys using addition. */ - if (secp256k1_testrand_int(3) == 0) { + if (testrand_int(3) == 0) { int ret1; int ret2; int ret3; unsigned char rnd[32]; unsigned char privkey_tmp[32]; secp256k1_pubkey pubkey2; - secp256k1_testrand256_test(rnd); + testrand256_test(rnd); memcpy(privkey_tmp, privkey, 32); ret1 = secp256k1_ec_seckey_tweak_add(CTX, privkey, rnd); ret2 = secp256k1_ec_pubkey_tweak_add(CTX, &pubkey, rnd); @@ -6522,14 +6406,14 @@ static void test_ecdsa_end_to_end(void) { } /* Optionally tweak the keys using multiplication. */ - if (secp256k1_testrand_int(3) == 0) { + if (testrand_int(3) == 0) { int ret1; int ret2; int ret3; unsigned char rnd[32]; unsigned char privkey_tmp[32]; secp256k1_pubkey pubkey2; - secp256k1_testrand256_test(rnd); + testrand256_test(rnd); memcpy(privkey_tmp, privkey, 32); ret1 = secp256k1_ec_seckey_tweak_mul(CTX, privkey, rnd); ret2 = secp256k1_ec_pubkey_tweak_mul(CTX, &pubkey, rnd); @@ -6591,7 +6475,7 @@ static void test_ecdsa_end_to_end(void) { /* Serialize/destroy/parse DER and verify again. */ siglen = 74; CHECK(secp256k1_ecdsa_signature_serialize_der(CTX, sig, &siglen, &signature[0]) == 1); - sig[secp256k1_testrand_int(siglen)] += 1 + secp256k1_testrand_int(255); + sig[testrand_int(siglen)] += 1 + testrand_int(255); CHECK(secp256k1_ecdsa_signature_parse_der(CTX, &signature[0], sig, siglen) == 0 || secp256k1_ecdsa_verify(CTX, &signature[0], message, &pubkey) == 0); } @@ -6601,23 +6485,23 @@ static void test_random_pubkeys(void) { secp256k1_ge elem2; unsigned char in[65]; /* Generate some randomly sized pubkeys. */ - size_t len = secp256k1_testrand_bits(2) == 0 ? 65 : 33; - if (secp256k1_testrand_bits(2) == 0) { - len = secp256k1_testrand_bits(6); + size_t len = testrand_bits(2) == 0 ? 65 : 33; + if (testrand_bits(2) == 0) { + len = testrand_bits(6); } if (len == 65) { - in[0] = secp256k1_testrand_bits(1) ? 4 : (secp256k1_testrand_bits(1) ? 6 : 7); + in[0] = testrand_bits(1) ? 4 : (testrand_bits(1) ? 6 : 7); } else { - in[0] = secp256k1_testrand_bits(1) ? 2 : 3; + in[0] = testrand_bits(1) ? 2 : 3; } - if (secp256k1_testrand_bits(3) == 0) { - in[0] = secp256k1_testrand_bits(8); + if (testrand_bits(3) == 0) { + in[0] = testrand_bits(8); } if (len > 1) { - secp256k1_testrand256(&in[1]); + testrand256(&in[1]); } if (len > 33) { - secp256k1_testrand256(&in[33]); + testrand256(&in[33]); } if (secp256k1_eckey_pubkey_parse(&elem, in, len)) { unsigned char out[65]; @@ -6639,7 +6523,7 @@ static void test_random_pubkeys(void) { CHECK(secp256k1_eckey_pubkey_parse(&elem2, in, size)); CHECK(secp256k1_ge_eq_var(&elem2, &elem)); /* Check that the X9.62 hybrid type is checked. */ - in[0] = secp256k1_testrand_bits(1) ? 6 : 7; + in[0] = testrand_bits(1) ? 6 : 7; res = secp256k1_eckey_pubkey_parse(&elem2, in, size); if (firstb == 2 || firstb == 3) { if (in[0] == firstb + 4) { @@ -6718,7 +6602,7 @@ static void permute(size_t *arr, size_t n) { size_t i; for (i = n - 1; i >= 1; i--) { size_t tmp, j; - j = secp256k1_testrand_int(i + 1); + j = testrand_int(i + 1); tmp = arr[i]; arr[i] = arr[j]; arr[j] = tmp; @@ -6728,7 +6612,7 @@ static void permute(size_t *arr, size_t n) { static void rand_pk(secp256k1_pubkey *pk) { unsigned char seckey[32]; secp256k1_keypair keypair; - secp256k1_testrand256(seckey); + testrand256(seckey); CHECK(secp256k1_keypair_create(CTX, &keypair, seckey) == 1); CHECK(secp256k1_keypair_pub(CTX, pk, &keypair) == 1); } @@ -6944,27 +6828,27 @@ static void assign_big_endian(unsigned char *ptr, size_t ptrlen, uint32_t val) { static void damage_array(unsigned char *sig, size_t *len) { int pos; - int action = secp256k1_testrand_bits(3); + int action = testrand_bits(3); if (action < 1 && *len > 3) { /* Delete a byte. */ - pos = secp256k1_testrand_int(*len); + pos = testrand_int(*len); memmove(sig + pos, sig + pos + 1, *len - pos - 1); (*len)--; return; } else if (action < 2 && *len < 2048) { /* Insert a byte. */ - pos = secp256k1_testrand_int(1 + *len); + pos = testrand_int(1 + *len); memmove(sig + pos + 1, sig + pos, *len - pos); - sig[pos] = secp256k1_testrand_bits(8); + sig[pos] = testrand_bits(8); (*len)++; return; } else if (action < 4) { /* Modify a byte. */ - sig[secp256k1_testrand_int(*len)] += 1 + secp256k1_testrand_int(255); + sig[testrand_int(*len)] += 1 + testrand_int(255); return; } else { /* action < 8 */ /* Modify a bit. */ - sig[secp256k1_testrand_int(*len)] ^= 1 << secp256k1_testrand_bits(3); + sig[testrand_int(*len)] ^= 1 << testrand_bits(3); return; } } @@ -6977,23 +6861,23 @@ static void random_ber_signature(unsigned char *sig, size_t *len, int* certainly int n; *len = 0; - der = secp256k1_testrand_bits(2) == 0; + der = testrand_bits(2) == 0; *certainly_der = der; *certainly_not_der = 0; - indet = der ? 0 : secp256k1_testrand_int(10) == 0; + indet = der ? 0 : testrand_int(10) == 0; for (n = 0; n < 2; n++) { /* We generate two classes of numbers: nlow==1 "low" ones (up to 32 bytes), nlow==0 "high" ones (32 bytes with 129 top bits set, or larger than 32 bytes) */ - nlow[n] = der ? 1 : (secp256k1_testrand_bits(3) != 0); + nlow[n] = der ? 1 : (testrand_bits(3) != 0); /* The length of the number in bytes (the first byte of which will always be nonzero) */ - nlen[n] = nlow[n] ? secp256k1_testrand_int(33) : 32 + secp256k1_testrand_int(200) * secp256k1_testrand_bits(3) / 8; + nlen[n] = nlow[n] ? testrand_int(33) : 32 + testrand_int(200) * testrand_bits(3) / 8; CHECK(nlen[n] <= 232); /* The top bit of the number. */ - nhbit[n] = (nlow[n] == 0 && nlen[n] == 32) ? 1 : (nlen[n] == 0 ? 0 : secp256k1_testrand_bits(1)); + nhbit[n] = (nlow[n] == 0 && nlen[n] == 32) ? 1 : (nlen[n] == 0 ? 0 : testrand_bits(1)); /* The top byte of the number (after the potential hardcoded 16 0xFF characters for "high" 32 bytes numbers) */ - nhbyte[n] = nlen[n] == 0 ? 0 : (nhbit[n] ? 128 + secp256k1_testrand_bits(7) : 1 + secp256k1_testrand_int(127)); + nhbyte[n] = nlen[n] == 0 ? 0 : (nhbit[n] ? 128 + testrand_bits(7) : 1 + testrand_int(127)); /* The number of zero bytes in front of the number (which is 0 or 1 in case of DER, otherwise we extend up to 300 bytes) */ - nzlen[n] = der ? ((nlen[n] == 0 || nhbit[n]) ? 1 : 0) : (nlow[n] ? secp256k1_testrand_int(3) : secp256k1_testrand_int(300 - nlen[n]) * secp256k1_testrand_bits(3) / 8); + nzlen[n] = der ? ((nlen[n] == 0 || nhbit[n]) ? 1 : 0) : (nlow[n] ? testrand_int(3) : testrand_int(300 - nlen[n]) * testrand_bits(3) / 8); if (nzlen[n] > ((nlen[n] == 0 || nhbit[n]) ? 1 : 0)) { *certainly_not_der = 1; } @@ -7002,7 +6886,7 @@ static void random_ber_signature(unsigned char *sig, size_t *len, int* certainly nlenlen[n] = nlen[n] + nzlen[n] < 128 ? 0 : (nlen[n] + nzlen[n] < 256 ? 1 : 2); if (!der) { /* nlenlen[n] max 127 bytes */ - int add = secp256k1_testrand_int(127 - nlenlen[n]) * secp256k1_testrand_bits(4) * secp256k1_testrand_bits(4) / 256; + int add = testrand_int(127 - nlenlen[n]) * testrand_bits(4) * testrand_bits(4) / 256; nlenlen[n] += add; if (add != 0) { *certainly_not_der = 1; @@ -7016,7 +6900,7 @@ static void random_ber_signature(unsigned char *sig, size_t *len, int* certainly CHECK(tlen <= 856); /* The length of the garbage inside the tuple. */ - elen = (der || indet) ? 0 : secp256k1_testrand_int(980 - tlen) * secp256k1_testrand_bits(3) / 8; + elen = (der || indet) ? 0 : testrand_int(980 - tlen) * testrand_bits(3) / 8; if (elen != 0) { *certainly_not_der = 1; } @@ -7024,7 +6908,7 @@ static void random_ber_signature(unsigned char *sig, size_t *len, int* certainly CHECK(tlen <= 980); /* The length of the garbage after the end of the tuple. */ - glen = der ? 0 : secp256k1_testrand_int(990 - tlen) * secp256k1_testrand_bits(3) / 8; + glen = der ? 0 : testrand_int(990 - tlen) * testrand_bits(3) / 8; if (glen != 0) { *certainly_not_der = 1; } @@ -7039,7 +6923,7 @@ static void random_ber_signature(unsigned char *sig, size_t *len, int* certainly } else { int tlenlen = tlen < 128 ? 0 : (tlen < 256 ? 1 : 2); if (!der) { - int add = secp256k1_testrand_int(127 - tlenlen) * secp256k1_testrand_bits(4) * secp256k1_testrand_bits(4) / 256; + int add = testrand_int(127 - tlenlen) * testrand_bits(4) * testrand_bits(4) / 256; tlenlen += add; if (add != 0) { *certainly_not_der = 1; @@ -7090,13 +6974,13 @@ static void random_ber_signature(unsigned char *sig, size_t *len, int* certainly nlen[n]--; } /* Generate remaining random bytes of number */ - secp256k1_testrand_bytes_test(sig + *len, nlen[n]); + testrand_bytes_test(sig + *len, nlen[n]); *len += nlen[n]; nlen[n] = 0; } /* Generate random garbage inside tuple. */ - secp256k1_testrand_bytes_test(sig + *len, elen); + testrand_bytes_test(sig + *len, elen); *len += elen; /* Generate end-of-contents bytes. */ @@ -7108,7 +6992,7 @@ static void random_ber_signature(unsigned char *sig, size_t *len, int* certainly CHECK(tlen + glen <= 1121); /* Generate random garbage outside tuple. */ - secp256k1_testrand_bytes_test(sig + *len, glen); + testrand_bytes_test(sig + *len, glen); *len += glen; tlen += glen; CHECK(tlen <= 1121); @@ -7771,7 +7655,7 @@ int main(int argc, char **argv) { run_xoshiro256pp_tests(); /* find random seed */ - secp256k1_testrand_init(argc > 2 ? argv[2] : NULL); + testrand_init(argc > 2 ? argv[2] : NULL); /*** Setup test environment ***/ @@ -7780,9 +7664,9 @@ int main(int argc, char **argv) { /* Randomize the context only with probability 15/16 to make sure we test without context randomization from time to time. TODO Reconsider this when recalibrating the tests. */ - if (secp256k1_testrand_bits(4)) { + if (testrand_bits(4)) { unsigned char rand32[32]; - secp256k1_testrand256(rand32); + testrand256(rand32); CHECK(secp256k1_context_randomize(CTX, rand32)); } /* Make a writable copy of secp256k1_context_static in order to test the effect of API functions @@ -7909,7 +7793,7 @@ int main(int argc, char **argv) { free(STATIC_CTX); secp256k1_context_destroy(CTX); - secp256k1_testrand_finish(); + testrand_finish(); printf("no problems found\n"); return 0; diff --git a/src/tests_exhaustive.c b/src/tests_exhaustive.c index 5843b3e1f5..6efa88982e 100644 --- a/src/tests_exhaustive.c +++ b/src/tests_exhaustive.c @@ -171,7 +171,7 @@ static void test_exhaustive_ecmult(const secp256k1_ge *group, const secp256k1_ge CHECK(secp256k1_fe_equal(&tmpf, &group[(i * j) % EXHAUSTIVE_TEST_ORDER].x)); /* Test secp256k1_ecmult_const_xonly with all curve X coordinates, with random xd. */ - random_fe_non_zero(&xd); + testutil_random_fe_non_zero(&xd); secp256k1_fe_mul(&xn, &xd, &group[i].x); ret = secp256k1_ecmult_const_xonly(&tmpf, &xn, &xd, &ng, 0); CHECK(ret); @@ -375,7 +375,7 @@ int main(int argc, char** argv) { printf("test count = %i\n", count); /* find random seed */ - secp256k1_testrand_init(argc > 2 ? argv[2] : NULL); + testrand_init(argc > 2 ? argv[2] : NULL); /* set up split processing */ if (argc > 4) { @@ -395,7 +395,7 @@ int main(int argc, char** argv) { while (count--) { /* Build context */ ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE); - secp256k1_testrand256(rand32); + testrand256(rand32); CHECK(secp256k1_context_randomize(ctx, rand32)); /* Generate the entire group */ @@ -408,7 +408,7 @@ int main(int argc, char** argv) { /* Set a different random z-value for each Jacobian point, except z=1 is used in the last iteration. */ secp256k1_fe z; - random_fe(&z); + testutil_random_fe(&z); secp256k1_gej_rescale(&groupj[i], &z); } @@ -459,7 +459,7 @@ int main(int argc, char** argv) { secp256k1_context_destroy(ctx); } - secp256k1_testrand_finish(); + testrand_finish(); printf("no problems found\n"); return 0; diff --git a/src/testutil.h b/src/testutil.h index 4e2cb7d5b3..8296a5fb99 100644 --- a/src/testutil.h +++ b/src/testutil.h @@ -7,23 +7,136 @@ #define SECP256K1_TESTUTIL_H #include "field.h" +#include "group.h" #include "testrand.h" #include "util.h" -static void random_fe(secp256k1_fe *x) { +static void testutil_random_fe(secp256k1_fe *x) { unsigned char bin[32]; do { - secp256k1_testrand256(bin); + testrand256(bin); if (secp256k1_fe_set_b32_limit(x, bin)) { return; } } while(1); } -static void random_fe_non_zero(secp256k1_fe *nz) { +static void testutil_random_fe_non_zero(secp256k1_fe *nz) { do { - random_fe(nz); + testutil_random_fe(nz); } while (secp256k1_fe_is_zero(nz)); } +static void testutil_random_fe_magnitude(secp256k1_fe *fe, int m) { + secp256k1_fe zero; + int n = testrand_int(m + 1); + secp256k1_fe_normalize(fe); + if (n == 0) { + return; + } + secp256k1_fe_clear(&zero); + secp256k1_fe_negate(&zero, &zero, 0); + secp256k1_fe_mul_int_unchecked(&zero, n - 1); + secp256k1_fe_add(fe, &zero); +#ifdef VERIFY + CHECK(fe->magnitude == n); +#endif +} + +static void testutil_random_fe_test(secp256k1_fe *x) { + unsigned char bin[32]; + do { + testrand256_test(bin); + if (secp256k1_fe_set_b32_limit(x, bin)) { + return; + } + } while(1); +} + +static void testutil_random_fe_non_zero_test(secp256k1_fe *fe) { + do { + testutil_random_fe_test(fe); + } while(secp256k1_fe_is_zero(fe)); +} + +static void testutil_random_ge_x_magnitude(secp256k1_ge *ge) { + testutil_random_fe_magnitude(&ge->x, SECP256K1_GE_X_MAGNITUDE_MAX); +} + +static void testutil_random_ge_y_magnitude(secp256k1_ge *ge) { + testutil_random_fe_magnitude(&ge->y, SECP256K1_GE_Y_MAGNITUDE_MAX); +} + +static void testutil_random_gej_x_magnitude(secp256k1_gej *gej) { + testutil_random_fe_magnitude(&gej->x, SECP256K1_GEJ_X_MAGNITUDE_MAX); +} + +static void testutil_random_gej_y_magnitude(secp256k1_gej *gej) { + testutil_random_fe_magnitude(&gej->y, SECP256K1_GEJ_Y_MAGNITUDE_MAX); +} + +static void testutil_random_gej_z_magnitude(secp256k1_gej *gej) { + testutil_random_fe_magnitude(&gej->z, SECP256K1_GEJ_Z_MAGNITUDE_MAX); +} + +static void testutil_random_ge_test(secp256k1_ge *ge) { + secp256k1_fe fe; + do { + testutil_random_fe_test(&fe); + if (secp256k1_ge_set_xo_var(ge, &fe, testrand_bits(1))) { + secp256k1_fe_normalize(&ge->y); + break; + } + } while(1); + ge->infinity = 0; +} + +static void testutil_random_ge_jacobian_test(secp256k1_gej *gej, const secp256k1_ge *ge) { + secp256k1_fe z2, z3; + testutil_random_fe_non_zero_test(&gej->z); + secp256k1_fe_sqr(&z2, &gej->z); + secp256k1_fe_mul(&z3, &z2, &gej->z); + secp256k1_fe_mul(&gej->x, &ge->x, &z2); + secp256k1_fe_mul(&gej->y, &ge->y, &z3); + gej->infinity = ge->infinity; +} + +static void testutil_random_gej_test(secp256k1_gej *gej) { + secp256k1_ge ge; + testutil_random_ge_test(&ge); + testutil_random_ge_jacobian_test(gej, &ge); +} + +static void testutil_random_scalar_order_test(secp256k1_scalar *num) { + do { + unsigned char b32[32]; + int overflow = 0; + testrand256_test(b32); + secp256k1_scalar_set_b32(num, b32, &overflow); + if (overflow || secp256k1_scalar_is_zero(num)) { + continue; + } + break; + } while(1); +} + +static void testutil_random_scalar_order(secp256k1_scalar *num) { + do { + unsigned char b32[32]; + int overflow = 0; + testrand256(b32); + secp256k1_scalar_set_b32(num, b32, &overflow); + if (overflow || secp256k1_scalar_is_zero(num)) { + continue; + } + break; + } while(1); +} + +static void testutil_random_scalar_order_b32(unsigned char *b32) { + secp256k1_scalar num; + testutil_random_scalar_order(&num); + secp256k1_scalar_get_b32(b32, &num); +} + #endif /* SECP256K1_TESTUTIL_H */ From 3ab25201909bece9066ac6191670bcee09791d54 Mon Sep 17 00:00:00 2001 From: Ben Westgate Date: Wed, 22 May 2024 22:37:31 -0500 Subject: [PATCH 28/39] contrib: Fixup verify-binaries OS platform parsing Parse platform strings with "-" or '.' correctly such as "linux-gnu" or "x86_64-linux-gnu.tar.gz" to download the matching files or file. String partition() is used to tolerate more dashes. Update `VERSION_EXAMPLE` with a new string parsed correctly now. Fix "-aarch64" interpreted as a release candidate due to sub-string "rc", causing all downloads to fail. Now "rc" must immediately follow first "-" to indicate an [-rc] string. Local variables `version_rc`, `version_os` renamed to `rc`, `platform`. If "-rcN" is specified, `platform` is reassigned to remove the '-rcN'. Changes are useful to only download one bitcoin core binary on slow connections. Making `verify.py pub` more intuitive, robust, and versatile. Closes #30145 When user types a platform string not found in any filename lets help and say the platform closest to what they typed in a `f"No files matched the platform specified. Did you mean: {closest_match}"` log. Improves UX when unaware how we name our files. Uses the difflib Python built-in which was already imported elsewhere. Update test.py to test single file verification verify-binaries/verify.py can accept an entire filename filter for its "-platform" parameter now so let us test that it succeeds and downloads and verifies only one file. `verify.py pub 22.0-x86_64-linux-gnu.tar.gz` should get and verify only the requested binary. It is placed before the existing wide verification as it is a faster test and possibly easier to break. Update doc with examples now possible after bugfix Add example to show release candidates now work with "-platform" strings containing "-" and string provided can be from the middle of filename: `./contrib/verify-binaries/verify.py --json pub 23.0-rc5-linux-gnu` Change example 5 to not match example 3. New examples to show platform can now be provided specifically enough to download only a single binary down to its file extension: `./contrib/verify-binaries/verify.py pub 25.2-x86_64-linux` `./contrib/verify-binaries/verify.py pub 24.1-rc1-darwin` `./contrib/verify-binaries/verify.py pub 27.0-win64-setup.exe` This is the most common use if not verifying all files so users see it as the first example for "only download the binaries for a certain architecture and/or platform". Downloading one file is intuitively what most will think this meant and this change delivers on that expectation. Co-authored-by: stickies-v --- contrib/verify-binaries/README.md | 10 ++++++---- contrib/verify-binaries/test.py | 15 +++++++++++++++ contrib/verify-binaries/verify.py | 26 +++++++++++--------------- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/contrib/verify-binaries/README.md b/contrib/verify-binaries/README.md index 04d683e69b..0f3e16a5bc 100644 --- a/contrib/verify-binaries/README.md +++ b/contrib/verify-binaries/README.md @@ -50,6 +50,7 @@ Get JSON output and don't prompt for user input (no auto key import): ```sh ./contrib/verify-binaries/verify.py --json pub 22.0-x86 +./contrib/verify-binaries/verify.py --json pub 23.0-rc5-linux-gnu ``` Rely only on local GPG state and manually specified keys, while requiring a @@ -57,14 +58,15 @@ threshold of at least 10 trusted signatures: ```sh ./contrib/verify-binaries/verify.py \ --trusted-keys 74E2DEF5D77260B98BC19438099BAD163C70FBFA,9D3CC86A72F8494342EA5FD10A41BDC3F4FAFF1C \ - --min-good-sigs 10 pub 22.0-x86 + --min-good-sigs 10 pub 22.0-linux ``` -If you only want to download the binaries for a certain platform, add the corresponding suffix, e.g.: +If you only want to download the binaries for a certain architecture and/or platform, add the corresponding suffix, e.g.: ```sh -./contrib/verify-binaries/verify.py pub 24.0.1-darwin -./contrib/verify-binaries/verify.py pub 23.1-rc1-win64 +./contrib/verify-binaries/verify.py pub 25.2-x86_64-linux +./contrib/verify-binaries/verify.py pub 24.1-rc1-darwin +./contrib/verify-binaries/verify.py pub 27.0-win64-setup.exe ``` If you do not want to keep the downloaded binaries, specify the cleanup option. diff --git a/contrib/verify-binaries/test.py b/contrib/verify-binaries/test.py index 22d718ece3..875606ec22 100755 --- a/contrib/verify-binaries/test.py +++ b/contrib/verify-binaries/test.py @@ -12,6 +12,21 @@ def main(): expect_code(run_verify("", "pub", '0.32.awefa.12f9h'), 11, "Malformed version should fail") expect_code(run_verify('--min-good-sigs 20', "pub", "22.0"), 9, "--min-good-sigs 20 should fail") + print("- testing verification (22.0-x86_64-linux-gnu.tar.gz)", flush=True) + _220_x86_64_linux_gnu = run_verify("--json", "pub", "22.0-x86_64-linux-gnu.tar.gz") + try: + result = json.loads(_220_x86_64_linux_gnu.stdout.decode()) + except Exception: + print("failed on 22.0-x86_64-linux-gnu.tar.gz --json:") + print_process_failure(_220_x86_64_linux_gnu) + raise + + expect_code(_220_x86_64_linux_gnu, 0, "22.0-x86_64-linux-gnu.tar.gz should succeed") + v = result['verified_binaries'] + assert result['good_trusted_sigs'] + assert len(v) == 1 + assert v['bitcoin-22.0-x86_64-linux-gnu.tar.gz'] == '59ebd25dd82a51638b7a6bb914586201e67db67b919b2a1ff08925a7936d1b16' + print("- testing verification (22.0)", flush=True) _220 = run_verify("--json", "pub", "22.0") try: diff --git a/contrib/verify-binaries/verify.py b/contrib/verify-binaries/verify.py index 12e6e10d8a..6c07b36c9d 100755 --- a/contrib/verify-binaries/verify.py +++ b/contrib/verify-binaries/verify.py @@ -97,23 +97,17 @@ def bool_from_env(key, default=False) -> bool: VERSION_FORMAT = ".[.][-rc[0-9]][-platform]" -VERSION_EXAMPLE = "22.0-x86_64 or 23.1-rc1-darwin" +VERSION_EXAMPLE = "22.0 or 23.1-rc1-darwin.dmg or 27.0-x86_64-linux-gnu" def parse_version_string(version_str): - parts = version_str.split('-') - version_base = parts[0] - version_rc = "" - version_os = "" - if len(parts) == 2: # "-rcN" or "version-platform" - if "rc" in parts[1]: - version_rc = parts[1] - else: - version_os = parts[1] - elif len(parts) == 3: # "-rcN-platform" - version_rc = parts[1] - version_os = parts[2] + # "[-rcN][-platform]" + version_base, _, platform = version_str.partition('-') + rc = "" + if platform.startswith("rc"): # "-rcN[-platform]" + rc, _, platform = platform.partition('-') + # else "" or "-platform" - return version_base, version_rc, version_os + return version_base, rc, platform def download_with_wget(remote_file, local_file): @@ -514,7 +508,9 @@ def cleanup(): # Extract hashes and filenames hashes_to_verify = parse_sums_file(SUMS_FILENAME, [os_filter]) if not hashes_to_verify: - log.error("no files matched the platform specified") + available_versions = ["-".join(line[1].split("-")[2:]) for line in parse_sums_file(SUMS_FILENAME, [])] + closest_match = difflib.get_close_matches(os_filter, available_versions, cutoff=0, n=1)[0] + log.error(f"No files matched the platform specified. Did you mean: {closest_match}") return ReturnCode.NO_BINARIES_MATCH # remove binaries that are known not to be hosted by bitcoincore.org From 1556d21599a250297d5f20e5249c970340ab08bc Mon Sep 17 00:00:00 2001 From: Roman Zeyde Date: Sat, 22 Jun 2024 11:46:10 +0300 Subject: [PATCH 29/39] rest: don't copy data when sending binary response Also, change `HTTPRequest::WriteReply` to accept `std::span`. --- src/httpserver.cpp | 5 +++-- src/httpserver.h | 9 +++++++-- src/rest.cpp | 20 +++++++------------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index b1d4dc9234..b6c6db8b35 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -634,7 +635,7 @@ void HTTPRequest::WriteHeader(const std::string& hdr, const std::string& value) * Replies must be sent in the main loop in the main http thread, * this cannot be done from worker threads. */ -void HTTPRequest::WriteReply(int nStatus, const std::string& strReply) +void HTTPRequest::WriteReply(int nStatus, std::span reply) { assert(!replySent && req); if (m_interrupt) { @@ -643,7 +644,7 @@ void HTTPRequest::WriteReply(int nStatus, const std::string& strReply) // Send event to main http thread to send reply message struct evbuffer* evb = evhttp_request_get_output_buffer(req); assert(evb); - evbuffer_add(evb, strReply.data(), strReply.size()); + evbuffer_add(evb, reply.data(), reply.size()); auto req_copy = req; HTTPEvent* ev = new HTTPEvent(eventBase, true, [req_copy, nStatus]{ evhttp_send_reply(req_copy, nStatus, nullptr, nullptr); diff --git a/src/httpserver.h b/src/httpserver.h index 9a49877f09..991081bab8 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -7,6 +7,7 @@ #include #include +#include #include namespace util { @@ -123,12 +124,16 @@ class HTTPRequest /** * Write HTTP reply. * nStatus is the HTTP status code to send. - * strReply is the body of the reply. Keep it empty to send a standard message. + * reply is the body of the reply. Keep it empty to send a standard message. * * @note Can be called only once. As this will give the request back to the * main thread, do not call any other HTTPRequest methods after calling this. */ - void WriteReply(int nStatus, const std::string& strReply = ""); + void WriteReply(int nStatus, std::string_view reply = "") + { + WriteReply(nStatus, std::as_bytes(std::span{reply.data(), reply.size()})); + } + void WriteReply(int nStatus, std::span reply); }; /** Get the query parameter value from request uri for a specified key, or std::nullopt if the key diff --git a/src/rest.cpp b/src/rest.cpp index d43018f5af..4abbc4d2ca 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -248,9 +248,8 @@ static bool rest_headers(const std::any& context, ssHeader << pindex->GetBlockHeader(); } - std::string binaryHeader = ssHeader.str(); req->WriteHeader("Content-Type", "application/octet-stream"); - req->WriteReply(HTTP_OK, binaryHeader); + req->WriteReply(HTTP_OK, ssHeader); return true; } @@ -321,9 +320,8 @@ static bool rest_block(const std::any& context, switch (rf) { case RESTResponseFormat::BINARY: { - const std::string binaryBlock{block_data.begin(), block_data.end()}; req->WriteHeader("Content-Type", "application/octet-stream"); - req->WriteReply(HTTP_OK, binaryBlock); + req->WriteReply(HTTP_OK, std::as_bytes(std::span{block_data})); return true; } @@ -451,9 +449,8 @@ static bool rest_filter_header(const std::any& context, HTTPRequest* req, const ssHeader << header; } - std::string binaryHeader = ssHeader.str(); req->WriteHeader("Content-Type", "application/octet-stream"); - req->WriteReply(HTTP_OK, binaryHeader); + req->WriteReply(HTTP_OK, ssHeader); return true; } case RESTResponseFormat::HEX: { @@ -548,9 +545,8 @@ static bool rest_block_filter(const std::any& context, HTTPRequest* req, const s DataStream ssResp{}; ssResp << filter; - std::string binaryResp = ssResp.str(); req->WriteHeader("Content-Type", "application/octet-stream"); - req->WriteReply(HTTP_OK, binaryResp); + req->WriteReply(HTTP_OK, ssResp); return true; } case RESTResponseFormat::HEX: { @@ -729,9 +725,8 @@ static bool rest_tx(const std::any& context, HTTPRequest* req, const std::string DataStream ssTx; ssTx << TX_WITH_WITNESS(tx); - std::string binaryTx = ssTx.str(); req->WriteHeader("Content-Type", "application/octet-stream"); - req->WriteReply(HTTP_OK, binaryTx); + req->WriteReply(HTTP_OK, ssTx); return true; } @@ -900,10 +895,9 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std:: // use exact same output as mentioned in Bip64 DataStream ssGetUTXOResponse{}; ssGetUTXOResponse << active_height << active_hash << bitmap << outs; - std::string ssGetUTXOResponseString = ssGetUTXOResponse.str(); req->WriteHeader("Content-Type", "application/octet-stream"); - req->WriteReply(HTTP_OK, ssGetUTXOResponseString); + req->WriteReply(HTTP_OK, ssGetUTXOResponse); return true; } @@ -981,7 +975,7 @@ static bool rest_blockhash_by_height(const std::any& context, HTTPRequest* req, DataStream ss_blockhash{}; ss_blockhash << pblockindex->GetBlockHash(); req->WriteHeader("Content-Type", "application/octet-stream"); - req->WriteReply(HTTP_OK, ss_blockhash.str()); + req->WriteReply(HTTP_OK, ss_blockhash); return true; } case RESTResponseFormat::HEX: { From 83a9bef0e2acad7655e23d30e1c52412f380d93d Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 25 Jun 2024 13:28:36 +0200 Subject: [PATCH 30/39] Add missing include for mining interface Needed for std::unique_ptr --- src/interfaces/mining.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index b96881f67c..1f603399bf 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_INTERFACES_MINING_H #define BITCOIN_INTERFACES_MINING_H +#include #include #include From 75ce7637ad75af890581660c0bb3565c3c03bd6c Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 25 Jun 2024 13:33:35 +0200 Subject: [PATCH 31/39] refactor: testBlockValidity make out argument last --- src/interfaces/mining.h | 5 +++-- src/node/interfaces.cpp | 2 +- src/rpc/mining.cpp | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 1f603399bf..bc98448833 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -45,6 +45,7 @@ class Mining * @returns a block template */ virtual std::unique_ptr createNewBlock(const CScript& script_pub_key, bool use_mempool = true) = 0; + /** * Processes new block. A valid new block is automatically relayed to peers. * @@ -63,12 +64,12 @@ class Mining * Only works on top of our current best block. * Does not check proof-of-work. * - * @param[out] state details of why a block failed to validate * @param[in] block the block to validate * @param[in] check_merkle_root call CheckMerkleRoot() + * @param[out] state details of why a block failed to validate * @returns false if any of the checks fail */ - virtual bool testBlockValidity(BlockValidationState& state, const CBlock& block, bool check_merkle_root = true) = 0; + virtual bool testBlockValidity(const CBlock& block, bool check_merkle_root, BlockValidationState& state) = 0; //! Get internal node context. Useful for RPC and testing, //! but not accessible across processes. diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index e0bab6e22e..931b98e59c 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -870,7 +870,7 @@ class MinerImpl : public Mining return context()->mempool->GetTransactionsUpdated(); } - bool testBlockValidity(BlockValidationState& state, const CBlock& block, bool check_merkle_root) override + bool testBlockValidity(const CBlock& block, bool check_merkle_root, BlockValidationState& state) override { LOCK(::cs_main); return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, chainman().ActiveChain().Tip(), /*fCheckPOW=*/false, check_merkle_root); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 2b93c18965..fe91d1ed89 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -390,7 +390,7 @@ static RPCHelpMan generateblock() LOCK(cs_main); BlockValidationState state; - if (!miner.testBlockValidity(state, block, /*check_merkle_root=*/false)) { + if (!miner.testBlockValidity(block, /*check_merkle_root=*/false, state)) { throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("testBlockValidity failed: %s", state.ToString())); } } @@ -713,7 +713,7 @@ static RPCHelpMan getblocktemplate() return "inconclusive-not-best-prevblk"; } BlockValidationState state; - miner.testBlockValidity(state, block); + miner.testBlockValidity(block, /*check_merkle_root=*/true, state); return BIP22ValidationResult(state); } From 5fb2b704897fe10b5bd5ed754a5afd2ddc4a9e1d Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Wed, 26 Jun 2024 12:27:06 +0200 Subject: [PATCH 32/39] Drop unneeded lock from createNewBlock This was added in 4bf2e361da1964f7c278b4939967a0e5afde20b0, but BlockAssembler::CreateNewBlock already locks cs_main internally. --- src/node/interfaces.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 931b98e59c..5e87bf234a 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -881,7 +881,6 @@ class MinerImpl : public Mining BlockAssembler::Options options; ApplyArgsManOptions(gArgs, options); - LOCK(::cs_main); return BlockAssembler{chainman().ActiveChainstate(), use_mempool ? context()->mempool.get() : nullptr, options}.CreateNewBlock(script_pub_key); } From f6dc6db44ddc22ade96a69a02908f14cfb279a37 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Wed, 26 Jun 2024 12:29:33 +0200 Subject: [PATCH 33/39] refactor: use CHECK_NONFATAL to avoid single-use symbol --- src/rpc/mining.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index fe91d1ed89..d9fd82d708 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -667,9 +667,7 @@ static RPCHelpMan getblocktemplate() ChainstateManager& chainman = EnsureChainman(node); Mining& miner = EnsureMining(node); LOCK(cs_main); - std::optional maybe_tip{miner.getTipHash()}; - CHECK_NONFATAL(maybe_tip); - uint256 tip{maybe_tip.value()}; + uint256 tip{CHECK_NONFATAL(miner.getTipHash()).value()}; std::string strMode = "template"; UniValue lpval = NullUniValue; From a74b0f93efa1d9eaf5abc2f6591c44a632aec6ed Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Wed, 26 Jun 2024 13:42:55 +0200 Subject: [PATCH 34/39] Have testBlockValidity hold cs_main instead of caller The goal of interfaces is to eventually run in their own process, so we can't use EXCLUSIVE_LOCKS_REQUIRED in their declaration. However TestBlockValidaty will crash (in its call to ConnectBlock) if the tip changes from under the proposed block. Have the testBlockValidity implementation hold the lock instead, and non-fatally check for this condition. --- src/interfaces/mining.h | 2 +- src/node/interfaces.cpp | 11 +++++++++-- src/rpc/mining.cpp | 4 ---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index bc98448833..7d71a01450 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -67,7 +67,7 @@ class Mining * @param[in] block the block to validate * @param[in] check_merkle_root call CheckMerkleRoot() * @param[out] state details of why a block failed to validate - * @returns false if any of the checks fail + * @returns false if it does not build on the current tip, or any of the checks fail */ virtual bool testBlockValidity(const CBlock& block, bool check_merkle_root, BlockValidationState& state) = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 5e87bf234a..fa151407fa 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -872,8 +872,15 @@ class MinerImpl : public Mining bool testBlockValidity(const CBlock& block, bool check_merkle_root, BlockValidationState& state) override { - LOCK(::cs_main); - return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, chainman().ActiveChain().Tip(), /*fCheckPOW=*/false, check_merkle_root); + LOCK(cs_main); + CBlockIndex* tip{chainman().ActiveChain().Tip()}; + // Fail if the tip updated before the lock was taken + if (block.hashPrevBlock != tip->GetBlockHash()) { + state.Error("Block does not connect to current chain tip."); + return false; + } + + return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, tip, /*fCheckPOW=*/false, check_merkle_root); } std::unique_ptr createNewBlock(const CScript& script_pub_key, bool use_mempool) override diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index d9fd82d708..7e420dcd9b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -371,8 +371,6 @@ static RPCHelpMan generateblock() ChainstateManager& chainman = EnsureChainman(node); { - LOCK(cs_main); - std::unique_ptr blocktemplate{miner.createNewBlock(coinbase_script, /*use_mempool=*/false)}; if (!blocktemplate) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block"); @@ -387,8 +385,6 @@ static RPCHelpMan generateblock() RegenerateCommitments(block, chainman); { - LOCK(cs_main); - BlockValidationState state; if (!miner.testBlockValidity(block, /*check_merkle_root=*/false, state)) { throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("testBlockValidity failed: %s", state.ToString())); From 7df03f1a923e239cea8c9b0d603a9eb00863a40c Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Thu, 27 Jun 2024 10:00:17 +0100 Subject: [PATCH 35/39] util: add perm string helper functions PermsToSymbolicString will convert from fs::perms to string type 'rwxrwxrwx'. InterpretPermString will convert from a user-supplied "perm string" such as 'owner', 'group' or 'all, into appropriate fs::perms. --- src/util/fs_helpers.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ src/util/fs_helpers.h | 14 ++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp index 8952f20f79..41c8fe3b8f 100644 --- a/src/util/fs_helpers.cpp +++ b/src/util/fs_helpers.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -269,3 +270,42 @@ bool TryCreateDirectories(const fs::path& p) // create_directories didn't create the directory, it had to have existed already return false; } + +std::string PermsToSymbolicString(fs::perms p) +{ + std::string perm_str(9, '-'); + + auto set_perm = [&](size_t pos, fs::perms required_perm, char letter) { + if ((p & required_perm) != fs::perms::none) { + perm_str[pos] = letter; + } + }; + + set_perm(0, fs::perms::owner_read, 'r'); + set_perm(1, fs::perms::owner_write, 'w'); + set_perm(2, fs::perms::owner_exec, 'x'); + set_perm(3, fs::perms::group_read, 'r'); + set_perm(4, fs::perms::group_write, 'w'); + set_perm(5, fs::perms::group_exec, 'x'); + set_perm(6, fs::perms::others_read, 'r'); + set_perm(7, fs::perms::others_write, 'w'); + set_perm(8, fs::perms::others_exec, 'x'); + + return perm_str; +} + +std::optional InterpretPermString(const std::string& s) +{ + if (s == "owner") { + return fs::perms::owner_read | fs::perms::owner_write; + } else if (s == "group") { + return fs::perms::owner_read | fs::perms::owner_write | + fs::perms::group_read; + } else if (s == "all") { + return fs::perms::owner_read | fs::perms::owner_write | + fs::perms::group_read | + fs::perms::others_read; + } else { + return std::nullopt; + } +} diff --git a/src/util/fs_helpers.h b/src/util/fs_helpers.h index ea3778eac3..28dd6d979d 100644 --- a/src/util/fs_helpers.h +++ b/src/util/fs_helpers.h @@ -12,6 +12,7 @@ #include #include #include +#include /** * Ensure file contents are fully committed to disk, using a platform-specific @@ -62,6 +63,19 @@ void ReleaseDirectoryLocks(); bool TryCreateDirectories(const fs::path& p); fs::path GetDefaultDataDir(); +/** Convert fs::perms to symbolic string of the form 'rwxrwxrwx' + * + * @param[in] p the perms to be converted + * @return Symbolic permissions string + */ +std::string PermsToSymbolicString(fs::perms p); +/** Interpret a custom permissions level string as fs::perms + * + * @param[in] s Permission level string + * @return Permissions as fs::perms + */ +std::optional InterpretPermString(const std::string& s); + #ifdef WIN32 fs::path GetSpecialFolderPath(int nFolder, bool fCreate = true); #endif From f467aede78533dac60a118e1566138d65522c213 Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Mon, 8 Jan 2024 15:02:44 +0000 Subject: [PATCH 36/39] init: add option for rpccookie permissions Add a bitcoind launch option `-rpccookieperms` to configure the file permissions of the cookie on Unix systems. --- src/httprpc.cpp | 19 +++++++++++++++++-- src/init.cpp | 1 + src/rpc/request.cpp | 21 +++++++++++++++------ src/rpc/request.h | 4 +++- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index c72dbf10bc..66e0591c44 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -11,6 +11,8 @@ #include #include #include +#include +#include #include #include #include @@ -19,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -244,8 +247,20 @@ static bool InitRPCAuthentication() { if (gArgs.GetArg("-rpcpassword", "") == "") { - LogPrintf("Using random cookie authentication.\n"); - if (!GenerateAuthCookie(&strRPCUserColonPass)) { + LogInfo("Using random cookie authentication.\n"); + + std::optional cookie_perms{std::nullopt}; + auto cookie_perms_arg{gArgs.GetArg("-rpccookieperms")}; + if (cookie_perms_arg) { + auto perm_opt = InterpretPermString(*cookie_perms_arg); + if (!perm_opt) { + LogInfo("Invalid -rpccookieperms=%s; must be one of 'owner', 'group', or 'all'.\n", *cookie_perms_arg); + return false; + } + cookie_perms = *perm_opt; + } + + if (!GenerateAuthCookie(&strRPCUserColonPass, cookie_perms)) { return false; } } else { diff --git a/src/init.cpp b/src/init.cpp index fbf25a0341..6bbc0827bd 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -650,6 +650,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-rpcbind=[:port]", "Bind to given address to listen for JSON-RPC connections. Do not expose the RPC server to untrusted networks such as the public internet! This option is ignored unless -rpcallowip is also passed. Port is optional and overrides -rpcport. Use [host]:port notation for IPv6. This option can be specified multiple times (default: 127.0.0.1 and ::1 i.e., localhost)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::RPC); argsman.AddArg("-rpcdoccheck", strprintf("Throw a non-fatal error at runtime if the documentation for an RPC is incorrect (default: %u)", DEFAULT_RPC_DOC_CHECK), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC); argsman.AddArg("-rpccookiefile=", "Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)", ArgsManager::ALLOW_ANY, OptionsCategory::RPC); + argsman.AddArg("-rpccookieperms=", strprintf("Set permissions on the RPC auth cookie file so that it is readable by [owner|group|all] (default: owner [via umask 0077])"), ArgsManager::ALLOW_ANY, OptionsCategory::RPC); argsman.AddArg("-rpcpassword=", "Password for JSON-RPC connections", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::RPC); argsman.AddArg("-rpcport=", strprintf("Listen for JSON-RPC connections on (default: %u, testnet: %u, signet: %u, regtest: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort(), signetBaseParams->RPCPort(), regtestBaseParams->RPCPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::RPC); argsman.AddArg("-rpcservertimeout=", strprintf("Timeout during HTTP requests (default: %d)", DEFAULT_HTTP_SERVER_TIMEOUT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC); diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index b7acd62ee3..eae7c4ba18 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -5,12 +5,11 @@ #include -#include - #include #include #include #include +#include #include #include @@ -82,7 +81,7 @@ static fs::path GetAuthCookieFile(bool temp=false) static bool g_generated_cookie = false; -bool GenerateAuthCookie(std::string *cookie_out) +bool GenerateAuthCookie(std::string* cookie_out, std::optional cookie_perms) { const size_t COOKIE_SIZE = 32; unsigned char rand_pwd[COOKIE_SIZE]; @@ -96,7 +95,7 @@ bool GenerateAuthCookie(std::string *cookie_out) fs::path filepath_tmp = GetAuthCookieFile(true); file.open(filepath_tmp); if (!file.is_open()) { - LogPrintf("Unable to open cookie authentication file %s for writing\n", fs::PathToString(filepath_tmp)); + LogInfo("Unable to open cookie authentication file %s for writing\n", fs::PathToString(filepath_tmp)); return false; } file << cookie; @@ -104,11 +103,21 @@ bool GenerateAuthCookie(std::string *cookie_out) fs::path filepath = GetAuthCookieFile(false); if (!RenameOver(filepath_tmp, filepath)) { - LogPrintf("Unable to rename cookie authentication file %s to %s\n", fs::PathToString(filepath_tmp), fs::PathToString(filepath)); + LogInfo("Unable to rename cookie authentication file %s to %s\n", fs::PathToString(filepath_tmp), fs::PathToString(filepath)); return false; } + if (cookie_perms) { + std::error_code code; + fs::permissions(filepath, cookie_perms.value(), fs::perm_options::replace, code); + if (code) { + LogInfo("Unable to set permissions on cookie authentication file %s\n", fs::PathToString(filepath_tmp)); + return false; + } + } + g_generated_cookie = true; - LogPrintf("Generated RPC authentication cookie %s\n", fs::PathToString(filepath)); + LogInfo("Generated RPC authentication cookie %s\n", fs::PathToString(filepath)); + LogInfo("Permissions used for cookie: %s\n", PermsToSymbolicString(fs::status(filepath).permissions())); if (cookie_out) *cookie_out = cookie; diff --git a/src/rpc/request.h b/src/rpc/request.h index a682c58d96..c7e723d962 100644 --- a/src/rpc/request.h +++ b/src/rpc/request.h @@ -7,9 +7,11 @@ #define BITCOIN_RPC_REQUEST_H #include +#include #include #include +#include UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id); UniValue JSONRPCReplyObj(const UniValue& result, const UniValue& error, const UniValue& id); @@ -17,7 +19,7 @@ std::string JSONRPCReply(const UniValue& result, const UniValue& error, const Un UniValue JSONRPCError(int code, const std::string& message); /** Generate a new RPC authentication cookie and write it to disk */ -bool GenerateAuthCookie(std::string *cookie_out); +bool GenerateAuthCookie(std::string* cookie_out, std::optional cookie_perms=std::nullopt); /** Read the RPC authentication cookie from disk */ bool GetAuthCookie(std::string *cookie_out); /** Delete RPC authentication cookie from disk */ From d2afa2690cceb0012b2aa1960e1cfa497f3103fa Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Tue, 9 Jan 2024 12:36:29 +0000 Subject: [PATCH 37/39] test: add rpccookieperms test Tests various perms on non-Windows OSes --- test/functional/rpc_users.py | 39 ++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index 66cdd7cf9a..153493fbab 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -11,12 +11,15 @@ ) import http.client +import os +import platform import urllib.parse import subprocess from random import SystemRandom import string import configparser import sys +from typing import Optional def call_with_auth(node, user, password): @@ -84,6 +87,40 @@ def test_auth(self, node, user, password): self.log.info('Wrong...') assert_equal(401, call_with_auth(node, user + 'wrong', password + 'wrong').status) + def test_rpccookieperms(self): + p = {"owner": 0o600, "group": 0o640, "all": 0o644} + + if platform.system() == 'Windows': + self.log.info(f"Skip cookie file permissions checks as OS detected as: {platform.system()=}") + return + + self.log.info('Check cookie file permissions can be set using -rpccookieperms') + + cookie_file_path = self.nodes[1].chain_path / '.cookie' + PERM_BITS_UMASK = 0o777 + + def test_perm(perm: Optional[str]): + if not perm: + perm = 'owner' + self.restart_node(1) + else: + self.restart_node(1, extra_args=[f"-rpccookieperms={perm}"]) + + file_stat = os.stat(cookie_file_path) + actual_perms = file_stat.st_mode & PERM_BITS_UMASK + expected_perms = p[perm] + assert_equal(expected_perms, actual_perms) + + # Remove any leftover rpc{user|password} config options from previous tests + self.nodes[1].replace_in_config([("rpcuser", "#rpcuser"), ("rpcpassword", "#rpcpassword")]) + + self.log.info('Check default cookie permission') + test_perm(None) + + self.log.info('Check custom cookie permissions') + for perm in ["owner", "group", "all"]: + test_perm(perm) + def run_test(self): self.conf_setup() self.log.info('Check correctness of the rpcauth config option') @@ -115,6 +152,8 @@ def run_test(self): (self.nodes[0].chain_path / ".cookie.tmp").mkdir() self.nodes[0].assert_start_raises_init_error(expected_msg=init_error) + self.test_rpccookieperms() + if __name__ == '__main__': HTTPBasicsTest().main() From 73f0a6cbd0b628675028fbd5a37eff8115e7ccfe Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Mon, 22 Apr 2024 12:24:09 +0100 Subject: [PATCH 38/39] doc: detail -rpccookieperms option Co-authored-by: tdb3 <106488469+tdb3@users.noreply.github.com> --- doc/init.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/init.md b/doc/init.md index 7f79027718..d0b73593c0 100644 --- a/doc/init.md +++ b/doc/init.md @@ -35,8 +35,10 @@ it will use a special cookie file for authentication. The cookie is generated wi content when the daemon starts, and deleted when it exits. Read access to this file controls who can access it through RPC. -By default the cookie is stored in the data directory, but it's location can be overridden -with the option '-rpccookiefile'. +By default the cookie is stored in the data directory, but its location can be +overridden with the option `-rpccookiefile`. Default file permissions for the +cookie are "owner" (i.e. user read/writeable) via default application-wide file +umask of `0077`, but these can be overridden with the `-rpccookieperms` option. This allows for running bitcoind without having to do any manual configuration. From 5d2fb14bafe4e80c0a482d99e5ebde07c477f000 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 29 Jun 2024 00:10:25 +0200 Subject: [PATCH 39/39] test: p2p: check that connecting to ourself leads to disconnect The "connect to ourself" detection logic has been first introduced by Satoshi in October 2009, together with a couple of other changes and a version bump to "v0.1.6 BETA" (see commit cc0b4c3b62367a2aebe5fc1f4d0ed4b97e9c2ac9). --- test/functional/p2p_handshake.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/functional/p2p_handshake.py b/test/functional/p2p_handshake.py index dd19fe9333..673322ab10 100755 --- a/test/functional/p2p_handshake.py +++ b/test/functional/p2p_handshake.py @@ -17,6 +17,7 @@ NODE_WITNESS, ) from test_framework.p2p import P2PInterface +from test_framework.util import p2p_port # Desirable service flags for outbound non-pruned and pruned peers. Note that @@ -88,6 +89,12 @@ def run_test(self): with node.assert_debug_log([f"feeler connection completed"]): self.add_outbound_connection(node, "feeler", NODE_NONE, wait_for_disconnect=True) + self.log.info("Check that connecting to ourself leads to immediate disconnect") + with node.assert_debug_log(["connected to self", "disconnecting"]): + node_listen_addr = f"127.0.0.1:{p2p_port(0)}" + node.addconnection(node_listen_addr, "outbound-full-relay", self.options.v2transport) + self.wait_until(lambda: len(node.getpeerinfo()) == 0) + if __name__ == '__main__': P2PHandshakeTest().main()