From 5a3f1a87c6d6e1c6f7937b25f03d113801a87b48 Mon Sep 17 00:00:00 2001 From: nanlour Date: Mon, 1 Apr 2024 14:13:06 +1100 Subject: [PATCH 001/530] Fix #29767, set m_synced = true after Commit() --- src/index/base.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 13d8ba5a01..a203ce4a9f 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -162,9 +162,9 @@ void BaseIndex::Sync() const CBlockIndex* pindex_next = WITH_LOCK(cs_main, return NextSyncBlock(pindex, m_chainstate->m_chain)); if (!pindex_next) { SetBestBlockIndex(pindex); - m_synced = true; // No need to handle errors in Commit. See rationale above. Commit(); + m_synced = true; break; } if (pindex_next->pprev != pindex && !Rewind(pindex, pindex_next->pprev)) { From 345e1c679362bc2f0053009779f19b413af7b663 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 31 Mar 2024 10:24:20 +0100 Subject: [PATCH 002/530] bench: Disable WalletCreate* benchmarks when building with MSVC --- src/bench/wallet_create.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp index 2e06b0df95..888d033d95 100644 --- a/src/bench/wallet_create.cpp +++ b/src/bench/wallet_create.cpp @@ -51,9 +51,14 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted) static void WalletCreatePlain(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/false); } static void WalletCreateEncrypted(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/true); } +#ifndef _MSC_VER +// TODO: Being built with MSVC, the fs::remove_all() call in +// the WalletCreate() fails with the error "The process cannot +// access the file because it is being used by another process." #ifdef USE_SQLITE BENCHMARK(WalletCreatePlain, benchmark::PriorityLevel::LOW); BENCHMARK(WalletCreateEncrypted, benchmark::PriorityLevel::LOW); #endif +#endif } // namespace wallet From a37095b6d2330179d759203f9180782931fede78 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 2 Jan 2024 16:34:50 -0500 Subject: [PATCH 003/530] Add AutoFile::seek and tell It's useful to be able to seek to a specific position in a file. Allow AutoFile to seek by using fseek. It's also useful to be able to get the current position in a file. Allow AutoFile to tell by using ftell. --- src/streams.cpp | 22 ++++++++++++++++++++++ src/streams.h | 3 +++ 2 files changed, 25 insertions(+) diff --git a/src/streams.cpp b/src/streams.cpp index 6921dad677..cdd36a86fe 100644 --- a/src/streams.cpp +++ b/src/streams.cpp @@ -21,6 +21,28 @@ std::size_t AutoFile::detail_fread(Span dst) } } +void AutoFile::seek(int64_t offset, int origin) +{ + if (IsNull()) { + throw std::ios_base::failure("AutoFile::seek: file handle is nullptr"); + } + if (std::fseek(m_file, offset, origin) != 0) { + throw std::ios_base::failure(feof() ? "AutoFile::seek: end of file" : "AutoFile::seek: fseek failed"); + } +} + +int64_t AutoFile::tell() +{ + if (IsNull()) { + throw std::ios_base::failure("AutoFile::tell: file handle is nullptr"); + } + int64_t r{std::ftell(m_file)}; + if (r < 0) { + throw std::ios_base::failure("AutoFile::tell: ftell failed"); + } + return r; +} + void AutoFile::read(Span dst) { if (detail_fread(dst) != dst.size()) { diff --git a/src/streams.h b/src/streams.h index c001cd7866..794d9c5980 100644 --- a/src/streams.h +++ b/src/streams.h @@ -472,6 +472,9 @@ class AutoFile /** Implementation detail, only used internally. */ std::size_t detail_fread(Span dst); + void seek(int64_t offset, int origin); + int64_t tell(); + // // Stream subset // From 5bc3beac3c588157c73d406b7f16f7611140b41b Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 2 Apr 2024 09:37:10 +0100 Subject: [PATCH 004/530] [doc] add historical release notes for 26.1 --- doc/release-notes/release-notes-26.1.md | 106 ++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 doc/release-notes/release-notes-26.1.md diff --git a/doc/release-notes/release-notes-26.1.md b/doc/release-notes/release-notes-26.1.md new file mode 100644 index 0000000000..b5c6e63007 --- /dev/null +++ b/doc/release-notes/release-notes-26.1.md @@ -0,0 +1,106 @@ +26.1 Release Notes +================== + +Bitcoin Core version 26.1 is now available from: + + + +This release includes various bug fixes and performance +improvements, as well as updated translations. + +Please report bugs using the issue tracker at GitHub: + + + +To receive security and update notifications, please subscribe to: + + + +How to Upgrade +============== + +If you are running an older version, shut it down. Wait until it has completely +shut down (which might take a few minutes in some cases), then run the +installer (on Windows) or just copy over `/Applications/Bitcoin-Qt` (on macOS) +or `bitcoind`/`bitcoin-qt` (on Linux). + +Upgrading directly from a version of Bitcoin Core that has reached its EOL is +possible, but it might take some time if the data directory needs to be migrated. Old +wallet versions of Bitcoin Core are generally supported. + +Compatibility +============== + +Bitcoin Core is supported and extensively tested on operating systems +using the Linux kernel, macOS 11.0+, and Windows 7 and newer. Bitcoin +Core should also work on most other Unix-like systems but is not as +frequently tested on them. It is not recommended to use Bitcoin Core on +unsupported systems. + +Notable changes +=============== + +### Wallet + +- #28994 wallet: skip BnB when SFFO is enabled +- #28920 wallet: birth time update during tx scanning +- #29176 wallet: Fix use-after-free in WalletBatch::EraseRecords +- #29510 wallet: getrawchangeaddress and getnewaddress failures should not affect keypools for descriptor wallets + +### RPC + +- #29003 rpc: fix getrawtransaction segfault +- #28784 rpc: keep .cookie file if it was not generated + +### Logs + +- #29227 log mempool loading progress + +### P2P and network changes + +- #29200 net: create I2P sessions using both ECIES-X25519 and ElGamal encryption +- #29412 p2p: Don't process mutated blocks +- #29524 p2p: Don't consider blocks mutated if they don't connect to known prev block + +### Build + +- #29127 Use hardened runtime on macOS release builds. +- #29195 build: Fix -Xclang -internal-isystem option + +### CI + +- #28992 ci: Use Ubuntu 24.04 Noble for asan,tsan,tidy,fuzz +- #29080 ci: Set HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK to avoid unrelated failures +- #29610 ci: Fix "macOS native" job + +### Miscellaneous + +- #28391 refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access +- #29179 test: wallet rescan with reorged parent + IsFromMe child in mempool +- #28791 snapshots: don't core dump when running -checkblockindex after loadtxoutset +- #29357 test: Drop x modifier in fsbridge::fopen call for MinGW builds +- #29529 fuzz: restrict fopencookie usage to Linux & FreeBSD + +Credits +======= + +Thanks to everyone who directly contributed to this release: + +- dergoegge +- fanquake +- furszy +- glozow +- Greg Sanders +- Hennadii Stepanov +- Jon Atack +- MarcoFalke +- Mark Friedenbach +- Martin Zumsande +- Murch +- Roman Zeyde +- stickies-v +- UdjinM6 + +As well as to everyone that helped with translations on +[Transifex](https://www.transifex.com/bitcoin/bitcoin/). + From ce43513a41d495ea5358a5abeee82707d5398109 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 2 Apr 2024 10:58:25 +0200 Subject: [PATCH 005/530] ci: Temporarily disable bpfcc-tools --- ci/test/00_setup_env_native_asan.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 313fd44f9e..2db1776a80 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -9,7 +9,7 @@ export LC_ALL=C.UTF-8 export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" # Only install BCC tracing packages in Cirrus CI. if [[ "${CIRRUS_CI}" == "true" ]]; then - BPFCC_PACKAGE="bpfcc-tools linux-headers-$(uname --kernel-release)" + BPFCC_PACKAGE="" # Temporarily disabled "bpfcc-tools linux-headers-$(uname --kernel-release)" export CI_CONTAINER_CAP="--privileged -v /sys/kernel:/sys/kernel:rw" else BPFCC_PACKAGE="" From dde2afd3751c16644ec6fd54d92ab09fcbcef68e Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 2 Apr 2024 13:21:58 +0100 Subject: [PATCH 006/530] guix: remove errant leftover from #29648 We no longer build a lib, so a non-existent dir is causing builds to fail. --- contrib/guix/libexec/build.sh | 7 ------- 1 file changed, 7 deletions(-) diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index fe4707509b..b1f3f26578 100644 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -316,12 +316,6 @@ mkdir -p "$DISTSRC" ( cd installed - case "$HOST" in - *mingw*) - mv --target-directory="$DISTNAME"/lib/ "$DISTNAME"/bin/*.dll - ;; - esac - # Prune libtool and object archives find . -name "lib*.la" -delete find . -name "lib*.a" -delete @@ -335,7 +329,6 @@ mkdir -p "$DISTSRC" # Split binaries and libraries from their debug symbols { find "${DISTNAME}/bin" -type f -executable -print0 - find "${DISTNAME}/lib" -type f -print0 } | xargs -0 -P"$JOBS" -I{} "${DISTSRC}/contrib/devtools/split-debug.sh" {} {} {}.dbg ;; esac From 31bca30ca01392573dcf9ac96017227a58df4c30 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Tue, 2 Apr 2024 14:29:24 +0100 Subject: [PATCH 007/530] Drop Windows Socket dependency for `randomenv.cpp` This change drops a dependency on the ws2_32 library for our libbitcoinkernel, which may not be desirable. --- src/randomenv.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/randomenv.cpp b/src/randomenv.cpp index 25529496b4..39f11a9cbf 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -357,10 +358,19 @@ void RandAddStaticEnv(CSHA512& hasher) hasher << &hasher << &RandAddStaticEnv << &malloc << &errno << &environ; // Hostname +#ifdef WIN32 + constexpr DWORD max_size = MAX_COMPUTERNAME_LENGTH + 1; + char hname[max_size]; + DWORD size = max_size; + if (GetComputerNameA(hname, &size) != 0) { + hasher.Write(UCharCast(hname), size); + } +#else char hname[256]; if (gethostname(hname, 256) == 0) { hasher.Write((const unsigned char*)hname, strnlen(hname, 256)); } +#endif #if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS // Network interfaces From 76040cb748dabbc6fd8c71e63acae27f3b12a031 Mon Sep 17 00:00:00 2001 From: Christopher Bergqvist Date: Tue, 2 Apr 2024 09:39:14 +0200 Subject: [PATCH 008/530] test: Bump timeouts in feature_index_prune and wallet_importdescriptors Timeout issues where encountered when running functional tests with `--jobs=16 --extended`. Line in `feature_index_prune.py` took 101.6s, 96.6s, 103.0s across 3 runs on my machine, default limit is 60. Line in the `wallet_importdescriptors.py --descriptors` took 5.4s, 5.7s, 6.0s across 3 runs. --- test/functional/feature_index_prune.py | 2 +- test/functional/wallet_importdescriptors.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/feature_index_prune.py b/test/functional/feature_index_prune.py index a5658a96ba..f4aedb394a 100755 --- a/test/functional/feature_index_prune.py +++ b/test/functional/feature_index_prune.py @@ -31,7 +31,7 @@ def sync_index(self, height): expected_stats = { 'coinstatsindex': {'synced': True, 'best_block_height': height} } - self.wait_until(lambda: self.nodes[1].getindexinfo() == expected_stats) + self.wait_until(lambda: self.nodes[1].getindexinfo() == expected_stats, timeout=150) expected = {**expected_filter, **expected_stats} self.wait_until(lambda: self.nodes[2].getindexinfo() == expected) diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index e3c8964120..636a7bbf3e 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -688,7 +688,7 @@ def run_test(self): encrypted_wallet.walletpassphrase("passphrase", 99999) with concurrent.futures.ThreadPoolExecutor(max_workers=1) as thread: - with self.nodes[0].assert_debug_log(expected_msgs=["Rescan started from block 2e14eaec9745ec9690602feddf650eb6e436d32a3ae8453cf6a90ef1d53a6c42... (slow variant inspecting all blocks)"], timeout=5): + with self.nodes[0].assert_debug_log(expected_msgs=["Rescan started from block 0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206... (slow variant inspecting all blocks)"], timeout=10): importing = thread.submit(encrypted_wallet.importdescriptors, requests=[descriptor]) # Set the passphrase timeout to 1 to test that the wallet remains unlocked during the rescan From b64d15e5cbe9fdb291b12dedd3a8a641abe4b0f1 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 29 Feb 2024 10:01:10 -0500 Subject: [PATCH 009/530] depends: add -g to DEBUG=1 flags --- depends/hosts/darwin.mk | 2 +- depends/hosts/linux.mk | 2 +- depends/hosts/mingw32.mk | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk index 29ad7ef252..8beedcc98a 100644 --- a/depends/hosts/darwin.mk +++ b/depends/hosts/darwin.mk @@ -105,7 +105,7 @@ endif darwin_release_CFLAGS=-O2 darwin_release_CXXFLAGS=$(darwin_release_CFLAGS) -darwin_debug_CFLAGS=-O1 +darwin_debug_CFLAGS=-O1 -g darwin_debug_CXXFLAGS=$(darwin_debug_CFLAGS) darwin_cmake_system=Darwin diff --git a/depends/hosts/linux.mk b/depends/hosts/linux.mk index 8be23be57d..9cd28f0deb 100644 --- a/depends/hosts/linux.mk +++ b/depends/hosts/linux.mk @@ -10,7 +10,7 @@ endif linux_release_CFLAGS=-O2 linux_release_CXXFLAGS=$(linux_release_CFLAGS) -linux_debug_CFLAGS=-O1 +linux_debug_CFLAGS=-O1 -g linux_debug_CXXFLAGS=$(linux_debug_CFLAGS) linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_ENABLE_DEBUG_MODE=1 diff --git a/depends/hosts/mingw32.mk b/depends/hosts/mingw32.mk index 15aa7cd25a..4c657358f6 100644 --- a/depends/hosts/mingw32.mk +++ b/depends/hosts/mingw32.mk @@ -14,7 +14,7 @@ endif mingw32_release_CFLAGS=-O2 mingw32_release_CXXFLAGS=$(mingw32_release_CFLAGS) -mingw32_debug_CFLAGS=-O1 +mingw32_debug_CFLAGS=-O1 -g mingw32_debug_CXXFLAGS=$(mingw32_debug_CFLAGS) mingw32_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC From c71d974551a3efa9215645bd48cc8312507bf11c Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 29 Feb 2024 10:01:46 -0500 Subject: [PATCH 010/530] depends: remove -g from sqlite debug flags --- depends/packages/sqlite.mk | 1 - 1 file changed, 1 deletion(-) diff --git a/depends/packages/sqlite.mk b/depends/packages/sqlite.mk index 7d175ec4bb..15bc0f4d7a 100644 --- a/depends/packages/sqlite.mk +++ b/depends/packages/sqlite.mk @@ -8,7 +8,6 @@ define $(package)_set_vars $(package)_config_opts=--disable-shared --disable-readline --disable-dynamic-extensions --enable-option-checking $(package)_config_opts+= --disable-rtree --disable-fts4 --disable-fts5 # We avoid using `--enable-debug` because it overrides CFLAGS, a behavior we want to prevent. -$(package)_cflags_debug += -g $(package)_cppflags_debug += -DSQLITE_DEBUG $(package)_cppflags+=-DSQLITE_DQS=0 -DSQLITE_DEFAULT_MEMSTATUS=0 -DSQLITE_OMIT_DEPRECATED $(package)_cppflags+=-DSQLITE_OMIT_SHARED_CACHE -DSQLITE_OMIT_JSON -DSQLITE_LIKE_DOESNT_MATCH_BLOBS From da5e71b9800ddbca2c66e89b0441868455235b7f Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sat, 16 Mar 2024 15:02:29 +0000 Subject: [PATCH 011/530] build, depends: Fix `libmultiprocess` cross-compilation This change prevents building all default targets that include `mpgen`, which expectedly fails to link when cross-compiling. --- depends/packages/libmultiprocess.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/depends/packages/libmultiprocess.mk b/depends/packages/libmultiprocess.mk index 765d649377..c292c49bfb 100644 --- a/depends/packages/libmultiprocess.mk +++ b/depends/packages/libmultiprocess.mk @@ -20,7 +20,7 @@ define $(package)_config_cmds endef define $(package)_build_cmds - $(MAKE) + $(MAKE) multiprocess endef define $(package)_stage_cmds From 1093da67289acae89dc330c9a3ef4b742fea780a Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 3 Apr 2024 12:16:01 +0100 Subject: [PATCH 012/530] guix: Remove another leftover from #29648 --- contrib/guix/libexec/build.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index b1f3f26578..30e4c9ef1a 100644 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -320,13 +320,10 @@ mkdir -p "$DISTSRC" find . -name "lib*.la" -delete find . -name "lib*.a" -delete - # Prune pkg-config files - rm -rf "${DISTNAME}/lib/pkgconfig" - case "$HOST" in *darwin*) ;; *) - # Split binaries and libraries from their debug symbols + # Split binaries from their debug symbols { find "${DISTNAME}/bin" -type f -executable -print0 } | xargs -0 -P"$JOBS" -I{} "${DISTSRC}/contrib/devtools/split-debug.sh" {} {} {}.dbg From 939262869a63faea29b9b5580b9117f805bcaf22 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 4 Apr 2024 12:05:16 +0100 Subject: [PATCH 013/530] Squashed 'src/secp256k1/' changes from efe85c70a2..d8311688bd d8311688bd Merge bitcoin-core/secp256k1#1515: ci: Note affected clangs in comment on ASLR quirk a85e2233e7 ci: Note affected clangs in comment on ASLR quirk 4b77fec67a Merge bitcoin-core/secp256k1#1512: msan: notate more variable assignments from assembly code f7f0184ba1 msan: notate more variable assignments from assembly code a61339149f change inconsistent array param to pointer 05bfab69ae Merge bitcoin-core/secp256k1#1507: ci: Add workaround for ASLR bug in sanitizers a5e8ab2484 ci: Add sanitizer env variables to debug output 84a93de4d2 ci: Add workaround for ASLR bug in sanitizers 427e86b9ed Merge bitcoin-core/secp256k1#1490: tests: improve fe_sqr test (issue #1472) 2028069df2 doc: clarify input requirements for secp256k1_fe_mul 11420a7a28 tests: improve fe_sqr test cdc9a6258e Merge bitcoin-core/secp256k1#1489: tests: add missing fe comparison checks for inverse field test cases d926510cf7 Merge bitcoin-core/secp256k1#1496: msan: notate variable assignments from assembly code 31ba404944 msan: notate variable assignments from assembly code e7ea32e30a msan: Add SECP256K1_CHECKMEM_MSAN_DEFINE which applies to memory sanitizer and not valgrind e7bdddd9c9 refactor: rename `check_fe_equal` -> `fe_equal` 00111c9c56 tests: add missing fe comparison checks for inverse field test cases 0653a25d50 Merge bitcoin-core/secp256k1#1486: ci: Update cache action 94a14d5290 ci: Update cache action 2483627299 Merge bitcoin-core/secp256k1#1483: cmake: Recommend native CMake commands in README 5ad3aa3dcd Merge bitcoin-core/secp256k1#1484: tests: Drop redundant _scalar_check_overflow calls 51df2d9ab3 tests: Drop redundant _scalar_check_overflow calls 3777e3f36a cmake: Recommend native CMake commands in README e4af41c61b Merge bitcoin-core/secp256k1#1249: cmake: Add `SECP256K1_LATE_CFLAGS` configure option 3bf4d68fc0 Merge bitcoin-core/secp256k1#1482: build: Clean up handling of module dependencies e6822678ea build: Error if required module explicitly off 89ec583ccf build: Clean up handling of module dependencies 44378867a0 Merge bitcoin-core/secp256k1#1468: v0.4.1 release aftermath a9db9f2d75 Merge bitcoin-core/secp256k1#1480: Get rid of untested sizeof(secp256k1_ge_storage) == 64 code path 74b7c3b53e Merge bitcoin-core/secp256k1#1476: include: make docs more consistent b37fdb28ce check-abi: Minor UI improvements ad5f589a94 check-abi: Default to HEAD for new version 9fb7e2f156 release process: Style and formatting nits ba5d72d626 assumptions: Use new STATIC_ASSERT macro e53c2d9ffc Require that sizeof(secp256k1_ge_storage) == 64 d0ba2abbff util: Add STATIC_ASSERT macro da7bc1b803 include: in doc, remove article in front of "pointer" aa3dd5280b include: make doc about ctx more consistent e3f690015a include: remove obvious "cannot be NULL" doc d373bf6d08 Merge bitcoin-core/secp256k1#1474: tests: restore scalar_mul test 79e094517c Merge bitcoin-core/secp256k1#1473: Fix typos 3dbfb48946 tests: restore scalar_mul test d77170a88d Fix typos e7053d065b release process: Add email step 429d21dc79 release process: Run sanity checks on release PR 42f8c51402 cmake: Add `SECP256K1_LATE_CFLAGS` configure option git-subtree-dir: src/secp256k1 git-subtree-split: d8311688bd383d3a923a1b11789cded3cc8e5e03 --- .../install-homebrew-valgrind/action.yml | 2 +- .../actions/run-in-docker-action/action.yml | 5 + CONTRIBUTING.md | 2 +- README.md | 6 +- ci/ci.sh | 3 +- configure.ac | 126 +++--------------- doc/release-process.md | 79 +++++------ src/assumptions.h | 104 ++++++++------- src/checkmem.h | 7 + src/secp256k1/CMakeLists.txt | 41 ++++-- .../cmake/AllTargetsCompileOptions.cmake | 12 ++ src/secp256k1/contrib/lax_der_parsing.h | 4 +- src/secp256k1/include/secp256k1.h | 66 ++++----- src/secp256k1/include/secp256k1_ecdh.h | 2 +- src/secp256k1/include/secp256k1_ellswift.h | 4 +- src/secp256k1/include/secp256k1_extrakeys.h | 10 +- .../include/secp256k1_preallocated.h | 14 +- src/secp256k1/include/secp256k1_recovery.h | 20 +-- src/secp256k1/include/secp256k1_schnorrsig.h | 4 +- src/secp256k1/src/field.h | 4 +- .../src/modules/ellswift/tests_impl.h | 8 +- src/secp256k1/src/scalar_4x64_impl.h | 59 +++++--- src/secp256k1/src/scalar_impl.h | 4 +- src/secp256k1/src/secp256k1.c | 39 ++---- src/secp256k1/src/tests.c | 67 ++++++---- src/secp256k1/src/util.h | 16 ++- src/secp256k1/tools/check-abi.sh | 27 ++-- 27 files changed, 366 insertions(+), 369 deletions(-) create mode 100644 src/secp256k1/cmake/AllTargetsCompileOptions.cmake diff --git a/.github/actions/install-homebrew-valgrind/action.yml b/.github/actions/install-homebrew-valgrind/action.yml index 094ff891f7..ce10eb2686 100644 --- a/.github/actions/install-homebrew-valgrind/action.yml +++ b/.github/actions/install-homebrew-valgrind/action.yml @@ -16,7 +16,7 @@ runs: cat valgrind_fingerprint shell: bash - - uses: actions/cache@v3 + - uses: actions/cache@v4 id: cache with: path: ${{ env.CI_HOMEBREW_CELLAR_VALGRIND }} diff --git a/.github/actions/run-in-docker-action/action.yml b/.github/actions/run-in-docker-action/action.yml index dbfaa4fece..74933686a0 100644 --- a/.github/actions/run-in-docker-action/action.yml +++ b/.github/actions/run-in-docker-action/action.yml @@ -36,6 +36,11 @@ runs: load: true cache-from: type=gha + - # Workaround for https://github.com/google/sanitizers/issues/1614 . + # The underlying issue has been fixed in clang 18.1.3. + run: sudo sysctl -w vm.mmap_rnd_bits=28 + shell: bash + - # Tell Docker to pass environment variables in `env` into the container. run: > docker run \ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a5e457913a..5fbf7332c9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,7 +44,7 @@ The Contributor Workflow & Peer Review in libsecp256k1 are similar to Bitcoin Co In addition, libsecp256k1 tries to maintain the following coding conventions: -* No runtime heap allocation (e.g., no `malloc`) unless explicitly requested by the caller (via `secp256k1_context_create` or `secp256k1_scratch_space_create`, for example). Morever, it should be possible to use the library without any heap allocations. +* No runtime heap allocation (e.g., no `malloc`) unless explicitly requested by the caller (via `secp256k1_context_create` or `secp256k1_scratch_space_create`, for example). Moreover, it should be possible to use the library without any heap allocations. * The tests should cover all lines and branches of the library (see [Test coverage](#coverage)). * Operations involving secret data should be tested for being constant time with respect to the secrets (see [src/ctime_tests.c](src/ctime_tests.c)). * Local variables containing secret data should be cleared explicitly to try to delete secrets from memory. diff --git a/README.md b/README.md index cb9b7bdff9..a45d239c13 100644 --- a/README.md +++ b/README.md @@ -101,9 +101,9 @@ To maintain a pristine source tree, CMake encourages to perform an out-of-source $ mkdir build && cd build $ cmake .. - $ make - $ make check # run the test suite - $ sudo make install # optional + $ cmake --build . + $ ctest # run the test suite + $ sudo cmake --build . --target install # optional To compile optional modules (such as Schnorr signatures), you need to run `cmake` with additional flags (such as `-DSECP256K1_ENABLE_MODULE_SCHNORRSIG=ON`). Run `cmake .. -LH` to see the full list of available flags. diff --git a/ci/ci.sh b/ci/ci.sh index 9cc715955e..3999af4f1c 100755 --- a/ci/ci.sh +++ b/ci/ci.sh @@ -17,7 +17,8 @@ print_environment() { SECP256K1_TEST_ITERS BENCH SECP256K1_BENCH_ITERS CTIMETESTS\ EXAMPLES \ HOST WRAPPER_CMD \ - CC CFLAGS CPPFLAGS AR NM + CC CFLAGS CPPFLAGS AR NM \ + UBSAN_OPTIONS ASAN_OPTIONS LSAN_OPTIONS do eval "isset=\${$var+x}" if [ -n "$isset" ]; then diff --git a/configure.ac b/configure.ac index 6e90aa6122..b28d9fe94c 100755 --- a/configure.ac +++ b/configure.ac @@ -1541,126 +1541,34 @@ fi dnl ZMQ check -if test "$use_zmq" = "yes"; then - PKG_CHECK_MODULES([ZMQ], [libzmq >= 4], - AC_DEFINE([ENABLE_ZMQ], [1], [Define this symbol to enable ZMQ functions]), - [AC_MSG_WARN([libzmq version 4.x or greater not found, disabling]) - use_zmq=no]) +# Processing must be done in a reverse topological sorting of the dependency graph +# (dependent module first). +if test x"$enable_module_ellswift" = x"yes"; then + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_ELLSWIFT=1" fi -if test "$use_zmq" = "yes"; then - dnl Assume libzmq was built for static linking - case $host in - *mingw*) - ZMQ_CFLAGS="$ZMQ_CFLAGS -DZMQ_STATIC" - ;; - esac -fi - -AM_CONDITIONAL([ENABLE_ZMQ], [test "$use_zmq" = "yes"]) - -dnl libmultiprocess library check - -libmultiprocess_found=no -if test "$with_libmultiprocess" = "yes" || test "$with_libmultiprocess" = "auto"; then - PKG_CHECK_MODULES([LIBMULTIPROCESS], [libmultiprocess], [ - libmultiprocess_found=yes; - libmultiprocess_prefix=`$PKG_CONFIG --variable=prefix libmultiprocess`; - ], [true]) -elif test "$with_libmultiprocess" != "no"; then - AC_MSG_ERROR([--with-libmultiprocess=$with_libmultiprocess value is not yes, auto, or no]) -fi - -dnl Enable multiprocess check - -if test "$enable_multiprocess" = "yes"; then - if test "$libmultiprocess_found" != "yes"; then - AC_MSG_ERROR([--enable-multiprocess=yes option specified but libmultiprocess library was not found. May need to install libmultiprocess library, or specify install path with PKG_CONFIG_PATH environment variable. Running 'pkg-config --debug libmultiprocess' may be helpful for debugging.]) +if test x"$enable_module_schnorrsig" = x"yes"; then + if test x"$enable_module_extrakeys" = x"no"; then + AC_MSG_ERROR([Module dependency error: You have disabled the extrakeys module explicitly, but it is required by the schnorrsig module.]) fi - build_multiprocess=yes -elif test "$enable_multiprocess" = "auto"; then - build_multiprocess=$libmultiprocess_found -else - build_multiprocess=no + enable_module_extrakeys=yes + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_SCHNORRSIG=1" fi -AM_CONDITIONAL([BUILD_MULTIPROCESS], [test "$build_multiprocess" = "yes"]) -AM_CONDITIONAL([BUILD_BGL_NODE], [test "$build_multiprocess" = "yes"]) -AM_CONDITIONAL([BUILD_BGL_GUI], [test "$build_multiprocess" = "yes"]) - -dnl codegen tools check - -if test "$build_multiprocess" != "no"; then - if test "$with_mpgen" = "yes" || test "$with_mpgen" = "auto"; then - MPGEN_PREFIX="$libmultiprocess_prefix" - elif test "$with_mpgen" != "no"; then - MPGEN_PREFIX="$with_mpgen"; - fi - AC_SUBST(MPGEN_PREFIX) +if test x"$enable_module_extrakeys" = x"yes"; then + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_EXTRAKEYS=1" fi -AC_MSG_CHECKING([whether to build BGLd]) -AM_CONDITIONAL([BUILD_BGLD], [test $build_BGLd = "yes"]) -AC_MSG_RESULT($build_BGLd) - -AC_MSG_CHECKING([whether to build BGL-cli]) -AM_CONDITIONAL([BUILD_BGL_CLI], [test $build_BGL_cli = "yes"]) -AC_MSG_RESULT($build_BGL_cli) - -AC_MSG_CHECKING([whether to build BGL-tx]) -AM_CONDITIONAL([BUILD_BGL_TX], [test $build_BGL_tx = "yes"]) -AC_MSG_RESULT($build_BGL_tx) - -AC_MSG_CHECKING([whether to build BGL-wallet]) -AM_CONDITIONAL([BUILD_BGL_WALLET], [test $build_BGL_wallet = "yes"]) -AC_MSG_RESULT($build_BGL_wallet) - -AC_MSG_CHECKING([whether to build BGL-util]) -AM_CONDITIONAL([BUILD_BGL_UTIL], [test $build_BGL_util = "yes"]) -AC_MSG_RESULT($build_BGL_util) - -AC_MSG_CHECKING([whether to build experimental BGL-chainstate]) -if test "$build_BGL_chainstate" = "yes"; then - if test "$build_experimental_kernel_lib" = "no"; then - AC_MSG_ERROR([experimental BGL-chainstate cannot be built without the experimental BGLkernel library. Use --with-experimental-kernel-lib]); - fi +if test x"$enable_module_recovery" = x"yes"; then + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_RECOVERY=1" fi -AM_CONDITIONAL([BUILD_BGL_CHAINSTATE], [test $build_BGL_chainstate = "yes"]) -AC_MSG_RESULT($build_BGL_chainstate) -AM_CONDITIONAL([BUILD_BGL_KERNEL_LIB], [test "$build_experimental_kernel_lib" != "no" && ( test "$build_experimental_kernel_lib" = "yes" || test "$build_BGL_chainstate" = "yes" )]) - -AC_LANG_POP - -if test "$use_ccache" != "no"; then - AC_MSG_CHECKING([if ccache should be used]) - if test "$CCACHE" = ""; then - if test "$use_ccache" = "yes"; then - AC_MSG_ERROR([ccache not found.]); - else - use_ccache=no - fi - else - use_ccache=yes - CC="$ac_cv_path_CCACHE $CC" - CXX="$ac_cv_path_CCACHE $CXX" - fi - AC_MSG_RESULT($use_ccache) - if test "$use_ccache" = "yes"; then - AX_CHECK_COMPILE_FLAG([-fdebug-prefix-map=A=B], [DEBUG_CXXFLAGS="$DEBUG_CXXFLAGS -fdebug-prefix-map=\$(abs_top_srcdir)=."], [], [$CXXFLAG_WERROR]) - AX_CHECK_PREPROC_FLAG([-fmacro-prefix-map=A=B], [DEBUG_CPPFLAGS="$DEBUG_CPPFLAGS -fmacro-prefix-map=\$(abs_top_srcdir)=."], [], [$CXXFLAG_WERROR]) - fi +if test x"$enable_module_ecdh" = x"yes"; then + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_ECDH=1" fi -dnl enable wallet -AC_MSG_CHECKING([if wallet should be enabled]) -if test "$enable_wallet" != "no"; then - AC_MSG_RESULT([yes]) - AC_DEFINE_UNQUOTED([ENABLE_WALLET],[1],[Define to 1 to enable wallet functions]) - enable_wallet=yes - -else - AC_MSG_RESULT([no]) +if test x"$enable_external_default_callbacks" = x"yes"; then + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DUSE_EXTERNAL_DEFAULT_CALLBACKS=1" fi dnl enable upnp support diff --git a/doc/release-process.md b/doc/release-process.md index bf7e2de070..e1ef33d299 100644 --- a/doc/release-process.md +++ b/doc/release-process.md @@ -1,4 +1,4 @@ -# Release Process +# Release process This document outlines the process for releasing versions of the form `$MAJOR.$MINOR.$PATCH`. @@ -17,8 +17,9 @@ This process also assumes that there will be no minor releases for old major rel * Update manpages (see previous section) * Write release notes (see "Write the release notes" below). -## Sanity Checks -Perform these checks before creating a release: +## Sanity checks +Perform these checks when reviewing the release PR (see below): + * On both the master branch and the new release branch: - update `CLIENT_VERSION_MAJOR` in [`configure.ac`](../configure.ac) @@ -121,7 +122,7 @@ git fetch origin "v${VERSION}" git checkout "v${VERSION}" popd ``` -<<<<<<< HEAD + Ensure your guix.sigs are up-to-date if you wish to `guix-verify` your builds against other `guix-attest` signatures. @@ -153,28 +154,30 @@ pushd ./guix.sigs git add "${VERSION}/${SIGNER}"/noncodesigned.SHA256SUMS{,.asc} git commit -m "Add attestations by ${SIGNER} for ${VERSION} non-codesigned" popd -======= + +1. Ensure `make distcheck` doesn't fail. + ```shell + ./autogen.sh && ./configure --enable-dev-mode && make distcheck + ``` + 2. Check installation with autotools: -```shell -dir=$(mktemp -d) -./autogen.sh && ./configure --prefix=$dir && make clean && make install && ls -RlAh $dir -gcc -o ecdsa examples/ecdsa.c $(PKG_CONFIG_PATH=$dir/lib/pkgconfig pkg-config --cflags --libs libsecp256k1) -Wl,-rpath,"$dir/lib" && ./ecdsa -``` + ```shell + dir=$(mktemp -d) + ./autogen.sh && ./configure --prefix=$dir && make clean && make install && ls -RlAh $dir + gcc -o ecdsa examples/ecdsa.c $(PKG_CONFIG_PATH=$dir/lib/pkgconfig pkg-config --cflags --libs libsecp256k1) -Wl,-rpath,"$dir/lib" && ./ecdsa + ``` 3. Check installation with CMake: -```shell -dir=$(mktemp -d) -build=$(mktemp -d) -cmake -B $build -DCMAKE_INSTALL_PREFIX=$dir && cmake --build $build --target install && ls -RlAh $dir -gcc -o ecdsa examples/ecdsa.c -I $dir/include -L $dir/lib*/ -l secp256k1 -Wl,-rpath,"$dir/lib",-rpath,"$dir/lib64" && ./ecdsa ->>>>>>> 29fde0223a... Squashed 'src/secp256k1/' changes from 199d27cea3..efe85c70a2 -``` -4. Use the [`check-abi.sh`](/tools/check-abi.sh) tool to ensure there are no unexpected ABI incompatibilities and that the version number and release notes accurately reflect all potential ABI changes. To run this tool, the `abi-dumper` and `abi-compliance-checker` packages are required. - -```shell -tools/check-abi.sh -``` + ```shell + dir=$(mktemp -d) + build=$(mktemp -d) + cmake -B $build -DCMAKE_INSTALL_PREFIX=$dir && cmake --build $build --target install && ls -RlAh $dir + gcc -o ecdsa examples/ecdsa.c -I $dir/include -L $dir/lib*/ -l secp256k1 -Wl,-rpath,"$dir/lib",-rpath,"$dir/lib64" && ./ecdsa + ``` +4. Use the [`check-abi.sh`](/tools/check-abi.sh) tool to verify that there are no unexpected ABI incompatibilities and that the version number and the release notes accurately reflect all potential ABI changes. To run this tool, the `abi-dumper` and `abi-compliance-checker` packages are required. + ```shell + tools/check-abi.sh + ``` -<<<<<<< HEAD Then open a Pull Request to the [guix.sigs repository](https://github.com/bitcoin-core/guix.sigs). ## Codesigning @@ -350,27 +353,29 @@ Notes: * adding a section for the release (make sure that the version number is a link to a diff between the previous and new version), * removing the `[Unreleased]` section header, and * including an entry for `### ABI Compatibility` if it doesn't exist, - * sets `_PKG_VERSION_IS_RELEASE` to `true` in `configure.ac`, and - * if this is not a patch release - * updates `_PKG_VERSION_*` and `_LIB_VERSION_*` in `configure.ac` and + * sets `_PKG_VERSION_IS_RELEASE` to `true` in `configure.ac`, and, + * if this is not a patch release, + * updates `_PKG_VERSION_*` and `_LIB_VERSION_*` in `configure.ac`, and * updates `project(libsecp256k1 VERSION ...)` and `${PROJECT_NAME}_LIB_VERSION_*` in `CMakeLists.txt`. -2. After the PR is merged, tag the commit and push it: +2. Perform the [sanity checks](#sanity-checks) on the PR branch. +3. After the PR is merged, tag the commit, and push the tag: ``` RELEASE_COMMIT= git tag -s v$MAJOR.$MINOR.$PATCH -m "libsecp256k1 $MAJOR.$MINOR.$PATCH" $RELEASE_COMMIT git push git@github.com:bitcoin-core/secp256k1.git v$MAJOR.$MINOR.$PATCH ``` -3. Open a PR to the master branch with a commit (using message `"release cleanup: bump version after $MAJOR.$MINOR.$PATCH"`, for example) that +4. Open a PR to the master branch with a commit (using message `"release cleanup: bump version after $MAJOR.$MINOR.$PATCH"`, for example) that * sets `_PKG_VERSION_IS_RELEASE` to `false` and increments `_PKG_VERSION_PATCH` and `_LIB_VERSION_REVISION` in `configure.ac`, * increments the `$PATCH` component of `project(libsecp256k1 VERSION ...)` and `${PROJECT_NAME}_LIB_VERSION_REVISION` in `CMakeLists.txt`, and * adds an `[Unreleased]` section header to the [CHANGELOG.md](../CHANGELOG.md). If other maintainers are not present to approve the PR, it can be merged without ACKs. -4. Create a new GitHub release with a link to the corresponding entry in [CHANGELOG.md](../CHANGELOG.md). +5. Create a new GitHub release with a link to the corresponding entry in [CHANGELOG.md](../CHANGELOG.md). +6. Send an announcement email to the bitcoin-dev mailing list. ## Maintenance release -Note that bugfixes only need to be backported to releases for which no compatible release without the bug exists. +Note that bug fixes need to be backported only to releases for which no compatible release without the bug exists. 1. If there's no maintenance branch `$MAJOR.$MINOR`, create one: ``` @@ -378,20 +383,18 @@ Note that bugfixes only need to be backported to releases for which no compatibl git push git@github.com:bitcoin-core/secp256k1.git $MAJOR.$MINOR ``` 2. Open a pull request to the `$MAJOR.$MINOR` branch that - * includes the bugfixes, + * includes the bug fixes, * finalizes the release notes similar to a regular release, * increments `_PKG_VERSION_PATCH` and `_LIB_VERSION_REVISION` in `configure.ac` and the `$PATCH` component of `project(libsecp256k1 VERSION ...)` and `${PROJECT_NAME}_LIB_VERSION_REVISION` in `CMakeLists.txt` (with commit message `"release: bump versions for $MAJOR.$MINOR.$PATCH"`, for example). -3. After the PRs are merged, update the release branch and tag the commit: +3. Perform the [sanity checks](#sanity-checks) on the PR branch. +4. After the PRs are merged, update the release branch, tag the commit, and push the tag: ``` git checkout $MAJOR.$MINOR && git pull git tag -s v$MAJOR.$MINOR.$PATCH -m "libsecp256k1 $MAJOR.$MINOR.$PATCH" - ``` -4. Push tag: - ``` git push git@github.com:bitcoin-core/secp256k1.git v$MAJOR.$MINOR.$PATCH ``` -5. Create a new GitHub release with a link to the corresponding entry in [CHANGELOG.md](../CHANGELOG.md). -6. Open PR to the master branch that includes a commit (with commit message `"release notes: add $MAJOR.$MINOR.$PATCH"`, for example) that adds release notes to [CHANGELOG.md](../CHANGELOG.md). ->>>>>>> 29fde0223a... Squashed 'src/secp256k1/' changes from 199d27cea3..efe85c70a2 +6. Create a new GitHub release with a link to the corresponding entry in [CHANGELOG.md](../CHANGELOG.md). +7. Send an announcement email to the bitcoin-dev mailing list. +8. Open PR to the master branch that includes a commit (with commit message `"release notes: add $MAJOR.$MINOR.$PATCH"`, for example) that adds release notes to [CHANGELOG.md](../CHANGELOG.md). diff --git a/src/assumptions.h b/src/assumptions.h index 77204de2b8..50c150f080 100644 --- a/src/assumptions.h +++ b/src/assumptions.h @@ -16,65 +16,69 @@ reduce the odds of experiencing an unwelcome surprise. */ -struct secp256k1_assumption_checker { - /* This uses a trick to implement a static assertion in C89: a type with an array of negative size is not - allowed. */ - int dummy_array[( - /* Bytes are 8 bits. */ - (CHAR_BIT == 8) && +#if defined(__has_attribute) +# if __has_attribute(__unavailable__) +__attribute__((__unavailable__("Don't call this function. It only exists because STATIC_ASSERT cannot be used outside a function."))) +# endif +#endif +static void secp256k1_assumption_checker(void) { + /* Bytes are 8 bits. */ + STATIC_ASSERT(CHAR_BIT == 8); - /* No integer promotion for uint32_t. This ensures that we can multiply uintXX_t values where XX >= 32 - without signed overflow, which would be undefined behaviour. */ - (UINT_MAX <= UINT32_MAX) && + /* No integer promotion for uint32_t. This ensures that we can multiply uintXX_t values where XX >= 32 + without signed overflow, which would be undefined behaviour. */ + STATIC_ASSERT(UINT_MAX <= UINT32_MAX); - /* Conversions from unsigned to signed outside of the bounds of the signed type are - implementation-defined. Verify that they function as reinterpreting the lower - bits of the input in two's complement notation. Do this for conversions: - - from uint(N)_t to int(N)_t with negative result - - from uint(2N)_t to int(N)_t with negative result - - from int(2N)_t to int(N)_t with negative result - - from int(2N)_t to int(N)_t with positive result */ + /* Conversions from unsigned to signed outside of the bounds of the signed type are + implementation-defined. Verify that they function as reinterpreting the lower + bits of the input in two's complement notation. Do this for conversions: + - from uint(N)_t to int(N)_t with negative result + - from uint(2N)_t to int(N)_t with negative result + - from int(2N)_t to int(N)_t with negative result + - from int(2N)_t to int(N)_t with positive result */ - /* To int8_t. */ - ((int8_t)(uint8_t)0xAB == (int8_t)-(int8_t)0x55) && - ((int8_t)(uint16_t)0xABCD == (int8_t)-(int8_t)0x33) && - ((int8_t)(int16_t)(uint16_t)0xCDEF == (int8_t)(uint8_t)0xEF) && - ((int8_t)(int16_t)(uint16_t)0x9234 == (int8_t)(uint8_t)0x34) && + /* To int8_t. */ + STATIC_ASSERT(((int8_t)(uint8_t)0xAB == (int8_t)-(int8_t)0x55)); + STATIC_ASSERT((int8_t)(uint16_t)0xABCD == (int8_t)-(int8_t)0x33); + STATIC_ASSERT((int8_t)(int16_t)(uint16_t)0xCDEF == (int8_t)(uint8_t)0xEF); + STATIC_ASSERT((int8_t)(int16_t)(uint16_t)0x9234 == (int8_t)(uint8_t)0x34); - /* To int16_t. */ - ((int16_t)(uint16_t)0xBCDE == (int16_t)-(int16_t)0x4322) && - ((int16_t)(uint32_t)0xA1B2C3D4 == (int16_t)-(int16_t)0x3C2C) && - ((int16_t)(int32_t)(uint32_t)0xC1D2E3F4 == (int16_t)(uint16_t)0xE3F4) && - ((int16_t)(int32_t)(uint32_t)0x92345678 == (int16_t)(uint16_t)0x5678) && + /* To int16_t. */ + STATIC_ASSERT((int16_t)(uint16_t)0xBCDE == (int16_t)-(int16_t)0x4322); + STATIC_ASSERT((int16_t)(uint32_t)0xA1B2C3D4 == (int16_t)-(int16_t)0x3C2C); + STATIC_ASSERT((int16_t)(int32_t)(uint32_t)0xC1D2E3F4 == (int16_t)(uint16_t)0xE3F4); + STATIC_ASSERT((int16_t)(int32_t)(uint32_t)0x92345678 == (int16_t)(uint16_t)0x5678); - /* To int32_t. */ - ((int32_t)(uint32_t)0xB2C3D4E5 == (int32_t)-(int32_t)0x4D3C2B1B) && - ((int32_t)(uint64_t)0xA123B456C789D012ULL == (int32_t)-(int32_t)0x38762FEE) && - ((int32_t)(int64_t)(uint64_t)0xC1D2E3F4A5B6C7D8ULL == (int32_t)(uint32_t)0xA5B6C7D8) && - ((int32_t)(int64_t)(uint64_t)0xABCDEF0123456789ULL == (int32_t)(uint32_t)0x23456789) && + /* To int32_t. */ + STATIC_ASSERT((int32_t)(uint32_t)0xB2C3D4E5 == (int32_t)-(int32_t)0x4D3C2B1B); + STATIC_ASSERT((int32_t)(uint64_t)0xA123B456C789D012ULL == (int32_t)-(int32_t)0x38762FEE); + STATIC_ASSERT((int32_t)(int64_t)(uint64_t)0xC1D2E3F4A5B6C7D8ULL == (int32_t)(uint32_t)0xA5B6C7D8); + STATIC_ASSERT((int32_t)(int64_t)(uint64_t)0xABCDEF0123456789ULL == (int32_t)(uint32_t)0x23456789); - /* To int64_t. */ - ((int64_t)(uint64_t)0xB123C456D789E012ULL == (int64_t)-(int64_t)0x4EDC3BA928761FEEULL) && -#if defined(SECP256K1_WIDEMUL_INT128) - ((int64_t)(((uint128_t)0xA1234567B8901234ULL << 64) + 0xC5678901D2345678ULL) == (int64_t)-(int64_t)0x3A9876FE2DCBA988ULL) && - (((int64_t)(int128_t)(((uint128_t)0xB1C2D3E4F5A6B7C8ULL << 64) + 0xD9E0F1A2B3C4D5E6ULL)) == (int64_t)(uint64_t)0xD9E0F1A2B3C4D5E6ULL) && - (((int64_t)(int128_t)(((uint128_t)0xABCDEF0123456789ULL << 64) + 0x0123456789ABCDEFULL)) == (int64_t)(uint64_t)0x0123456789ABCDEFULL) && + /* To int64_t. */ + STATIC_ASSERT((int64_t)(uint64_t)0xB123C456D789E012ULL == (int64_t)-(int64_t)0x4EDC3BA928761FEEULL); +#if defined(SECP256K1_INT128_NATIVE) + STATIC_ASSERT((int64_t)(((uint128_t)0xA1234567B8901234ULL << 64) + 0xC5678901D2345678ULL) == (int64_t)-(int64_t)0x3A9876FE2DCBA988ULL); + STATIC_ASSERT(((int64_t)(int128_t)(((uint128_t)0xB1C2D3E4F5A6B7C8ULL << 64) + 0xD9E0F1A2B3C4D5E6ULL)) == (int64_t)(uint64_t)0xD9E0F1A2B3C4D5E6ULL); + STATIC_ASSERT(((int64_t)(int128_t)(((uint128_t)0xABCDEF0123456789ULL << 64) + 0x0123456789ABCDEFULL)) == (int64_t)(uint64_t)0x0123456789ABCDEFULL); - /* To int128_t. */ - ((int128_t)(((uint128_t)0xB1234567C8901234ULL << 64) + 0xD5678901E2345678ULL) == (int128_t)(-(int128_t)0x8E1648B3F50E80DCULL * 0x8E1648B3F50E80DDULL + 0x5EA688D5482F9464ULL)) && + /* To int128_t. */ + STATIC_ASSERT((int128_t)(((uint128_t)0xB1234567C8901234ULL << 64) + 0xD5678901E2345678ULL) == (int128_t)(-(int128_t)0x8E1648B3F50E80DCULL * 0x8E1648B3F50E80DDULL + 0x5EA688D5482F9464ULL)); #endif - /* Right shift on negative signed values is implementation defined. Verify that it - acts as a right shift in two's complement with sign extension (i.e duplicating - the top bit into newly added bits). */ - ((((int8_t)0xE8) >> 2) == (int8_t)(uint8_t)0xFA) && - ((((int16_t)0xE9AC) >> 4) == (int16_t)(uint16_t)0xFE9A) && - ((((int32_t)0x937C918A) >> 9) == (int32_t)(uint32_t)0xFFC9BE48) && - ((((int64_t)0xA8B72231DF9CF4B9ULL) >> 19) == (int64_t)(uint64_t)0xFFFFF516E4463BF3ULL) && -#if defined(SECP256K1_WIDEMUL_INT128) - ((((int128_t)(((uint128_t)0xCD833A65684A0DBCULL << 64) + 0xB349312F71EA7637ULL)) >> 39) == (int128_t)(((uint128_t)0xFFFFFFFFFF9B0674ULL << 64) + 0xCAD0941B79669262ULL)) && + /* Right shift on negative signed values is implementation defined. Verify that it + acts as a right shift in two's complement with sign extension (i.e duplicating + the top bit into newly added bits). */ + STATIC_ASSERT((((int8_t)0xE8) >> 2) == (int8_t)(uint8_t)0xFA); + STATIC_ASSERT((((int16_t)0xE9AC) >> 4) == (int16_t)(uint16_t)0xFE9A); + STATIC_ASSERT((((int32_t)0x937C918A) >> 9) == (int32_t)(uint32_t)0xFFC9BE48); + STATIC_ASSERT((((int64_t)0xA8B72231DF9CF4B9ULL) >> 19) == (int64_t)(uint64_t)0xFFFFF516E4463BF3ULL); +#if defined(SECP256K1_INT128_NATIVE) + STATIC_ASSERT((((int128_t)(((uint128_t)0xCD833A65684A0DBCULL << 64) + 0xB349312F71EA7637ULL)) >> 39) == (int128_t)(((uint128_t)0xFFFFFFFFFF9B0674ULL << 64) + 0xCAD0941B79669262ULL)); #endif - 1) * 2 - 1]; -}; + + /* This function is not supposed to be called. */ + VERIFY_CHECK(0); +} #endif /* SECP256K1_ASSUMPTIONS_H */ diff --git a/src/checkmem.h b/src/checkmem.h index f2169decfc..7e333ce5f3 100644 --- a/src/checkmem.h +++ b/src/checkmem.h @@ -30,6 +30,8 @@ * - SECP256K1_CHECKMEM_DEFINE(p, len): * - marks the len-byte memory pointed to by p as defined data (public data, in the * context of constant-time checking). + * - SECP256K1_CHECKMEM_MSAN_DEFINE(p, len): + * - Like SECP256K1_CHECKMEM_DEFINE, but applies only to memory_sanitizer. * */ @@ -48,11 +50,16 @@ # define SECP256K1_CHECKMEM_ENABLED 1 # define SECP256K1_CHECKMEM_UNDEFINE(p, len) __msan_allocated_memory((p), (len)) # define SECP256K1_CHECKMEM_DEFINE(p, len) __msan_unpoison((p), (len)) +# define SECP256K1_CHECKMEM_MSAN_DEFINE(p, len) __msan_unpoison((p), (len)) # define SECP256K1_CHECKMEM_CHECK(p, len) __msan_check_mem_is_initialized((p), (len)) # define SECP256K1_CHECKMEM_RUNNING() (1) # endif #endif +#if !defined SECP256K1_CHECKMEM_MSAN_DEFINE +# define SECP256K1_CHECKMEM_MSAN_DEFINE(p, len) SECP256K1_CHECKMEM_NOOP((p), (len)) +#endif + /* If valgrind integration is desired (through the VALGRIND define), implement the * SECP256K1_CHECKMEM_* macros using valgrind. */ #if !defined SECP256K1_CHECKMEM_ENABLED diff --git a/src/secp256k1/CMakeLists.txt b/src/secp256k1/CMakeLists.txt index 71b99b3125..de62ac787e 100644 --- a/src/secp256k1/CMakeLists.txt +++ b/src/secp256k1/CMakeLists.txt @@ -51,29 +51,40 @@ endif() option(SECP256K1_INSTALL "Enable installation." ${PROJECT_IS_TOP_LEVEL}) -option(SECP256K1_ENABLE_MODULE_ECDH "Enable ECDH module." ON) -if(SECP256K1_ENABLE_MODULE_ECDH) - add_compile_definitions(ENABLE_MODULE_ECDH=1) -endif() +## Modules +# We declare all options before processing them, to make sure we can express +# dependendencies while processing. +option(SECP256K1_ENABLE_MODULE_ECDH "Enable ECDH module." ON) option(SECP256K1_ENABLE_MODULE_RECOVERY "Enable ECDSA pubkey recovery module." OFF) -if(SECP256K1_ENABLE_MODULE_RECOVERY) - add_compile_definitions(ENABLE_MODULE_RECOVERY=1) -endif() - option(SECP256K1_ENABLE_MODULE_EXTRAKEYS "Enable extrakeys module." ON) option(SECP256K1_ENABLE_MODULE_SCHNORRSIG "Enable schnorrsig module." ON) +option(SECP256K1_ENABLE_MODULE_ELLSWIFT "Enable ElligatorSwift module." ON) + +# Processing must be done in a topological sorting of the dependency graph +# (dependent module first). +if(SECP256K1_ENABLE_MODULE_ELLSWIFT) + add_compile_definitions(ENABLE_MODULE_ELLSWIFT=1) +endif() + if(SECP256K1_ENABLE_MODULE_SCHNORRSIG) + if(DEFINED SECP256K1_ENABLE_MODULE_EXTRAKEYS AND NOT SECP256K1_ENABLE_MODULE_EXTRAKEYS) + message(FATAL_ERROR "Module dependency error: You have disabled the extrakeys module explicitly, but it is required by the schnorrsig module.") + endif() set(SECP256K1_ENABLE_MODULE_EXTRAKEYS ON) add_compile_definitions(ENABLE_MODULE_SCHNORRSIG=1) endif() + if(SECP256K1_ENABLE_MODULE_EXTRAKEYS) add_compile_definitions(ENABLE_MODULE_EXTRAKEYS=1) endif() -option(SECP256K1_ENABLE_MODULE_ELLSWIFT "Enable ElligatorSwift module." ON) -if(SECP256K1_ENABLE_MODULE_ELLSWIFT) - add_compile_definitions(ENABLE_MODULE_ELLSWIFT=1) +if(SECP256K1_ENABLE_MODULE_RECOVERY) + add_compile_definitions(ENABLE_MODULE_RECOVERY=1) +endif() + +if(SECP256K1_ENABLE_MODULE_ECDH) + add_compile_definitions(ENABLE_MODULE_ECDH=1) endif() option(SECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS "Enable external default callback functions." OFF) @@ -254,9 +265,14 @@ 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) + 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") @@ -330,6 +346,9 @@ 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}") +endif() message("\n") if(SECP256K1_EXPERIMENTAL) message( diff --git a/src/secp256k1/cmake/AllTargetsCompileOptions.cmake b/src/secp256k1/cmake/AllTargetsCompileOptions.cmake new file mode 100644 index 0000000000..6e420e0fde --- /dev/null +++ b/src/secp256k1/cmake/AllTargetsCompileOptions.cmake @@ -0,0 +1,12 @@ +# 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/src/secp256k1/contrib/lax_der_parsing.h b/src/secp256k1/contrib/lax_der_parsing.h index 034a38e6a0..37c8c691f2 100644 --- a/src/secp256k1/contrib/lax_der_parsing.h +++ b/src/secp256k1/contrib/lax_der_parsing.h @@ -67,8 +67,8 @@ extern "C" { * * Returns: 1 when the signature could be parsed, 0 otherwise. * Args: ctx: a secp256k1 context object - * Out: sig: a pointer to a signature object - * In: input: a pointer to the signature to be parsed + * Out: sig: pointer to a signature object + * In: input: pointer to the signature to be parsed * inputlen: the length of the array pointed to be input * * This function will accept any valid DER encoded signature, even if the diff --git a/src/secp256k1/include/secp256k1.h b/src/secp256k1/include/secp256k1.h index 936f0b42b7..f4053f2a93 100644 --- a/src/secp256k1/include/secp256k1.h +++ b/src/secp256k1/include/secp256k1.h @@ -265,7 +265,7 @@ SECP256K1_API void secp256k1_selftest(void); * memory allocation entirely, see secp256k1_context_static and the functions in * secp256k1_preallocated.h. * - * Returns: a newly created context object. + * Returns: pointer to a newly created context object. * In: flags: Always set to SECP256K1_CONTEXT_NONE (see below). * * The only valid non-deprecated flag in recent library versions is @@ -296,8 +296,8 @@ SECP256K1_API secp256k1_context *secp256k1_context_create( * Cloning secp256k1_context_static is not possible, and should not be emulated by * the caller (e.g., using memcpy). Create a new context instead. * - * Returns: a newly created context object. - * Args: ctx: an existing context to copy (not secp256k1_context_static) + * Returns: pointer to a newly created context object. + * Args: ctx: pointer to a context to copy (not secp256k1_context_static). */ SECP256K1_API secp256k1_context *secp256k1_context_clone( const secp256k1_context *ctx @@ -313,7 +313,7 @@ SECP256K1_API secp256k1_context *secp256k1_context_clone( * behaviour is undefined. In that case, secp256k1_context_preallocated_destroy must * be used instead. * - * Args: ctx: an existing context to destroy, constructed using + * Args: ctx: pointer to a context to destroy, constructed using * secp256k1_context_create or secp256k1_context_clone * (i.e., not secp256k1_context_static). */ @@ -350,8 +350,8 @@ SECP256K1_API void secp256k1_context_destroy( * fails. In this case, the corresponding default handler will be called with * the data pointer argument set to NULL. * - * Args: ctx: an existing context object. - * In: fun: a pointer to a function to call when an illegal argument is + * Args: ctx: pointer to a context object. + * In: fun: pointer to a function to call when an illegal argument is * passed to the API, taking a message and an opaque pointer. * (NULL restores the default handler.) * data: the opaque pointer to pass to fun above, must be NULL for the default handler. @@ -377,8 +377,8 @@ SECP256K1_API void secp256k1_context_set_illegal_callback( * for that). After this callback returns, anything may happen, including * crashing. * - * Args: ctx: an existing context object. - * In: fun: a pointer to a function to call when an internal error occurs, + * Args: ctx: pointer to a context object. + * In: fun: pointer to a function to call when an internal error occurs, * taking a message and an opaque pointer (NULL restores the * default handler, see secp256k1_context_set_illegal_callback * for details). @@ -395,7 +395,7 @@ SECP256K1_API void secp256k1_context_set_error_callback( /** Create a secp256k1 scratch space object. * * Returns: a newly created scratch space. - * Args: ctx: an existing context object. + * Args: ctx: pointer to a context object. * In: size: amount of memory to be available as scratch space. Some extra * (<100 bytes) will be allocated for extra accounting. */ @@ -407,7 +407,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT secp256k1_scratch_space *secp256k1_sc /** Destroy a secp256k1 scratch space. * * The pointer may not be used afterwards. - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object. * scratch: space to destroy */ SECP256K1_API void secp256k1_scratch_space_destroy( @@ -419,7 +419,7 @@ SECP256K1_API void secp256k1_scratch_space_destroy( * * Returns: 1 if the public key was fully valid. * 0 if the public key could not be parsed or is invalid. - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object. * Out: pubkey: pointer to a pubkey object. If 1 is returned, it is set to a * parsed version of input. If not, its value is undefined. * In: input: pointer to a serialized public key @@ -439,14 +439,14 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_parse( /** Serialize a pubkey object into a serialized byte sequence. * * Returns: 1 always. - * Args: ctx: a secp256k1 context object. - * Out: output: a pointer to a 65-byte (if compressed==0) or 33-byte (if + * Args: ctx: pointer to a context object. + * Out: output: pointer to a 65-byte (if compressed==0) or 33-byte (if * compressed==1) byte array to place the serialized key * in. - * In/Out: outputlen: a pointer to an integer which is initially set to the + * In/Out: outputlen: pointer to an integer which is initially set to the * size of output, and is overwritten with the written * size. - * In: pubkey: a pointer to a secp256k1_pubkey containing an + * In: pubkey: pointer to a secp256k1_pubkey containing an * initialized public key. * flags: SECP256K1_EC_COMPRESSED if serialization should be in * compressed format, otherwise SECP256K1_EC_UNCOMPRESSED. @@ -464,7 +464,7 @@ SECP256K1_API int secp256k1_ec_pubkey_serialize( * Returns: <0 if the first public key is less than the second * >0 if the first public key is greater than the second * 0 if the two public keys are equal - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object * In: pubkey1: first public key to compare * pubkey2: second public key to compare */ @@ -477,9 +477,9 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_cmp( /** Parse an ECDSA signature in compact (64 bytes) format. * * Returns: 1 when the signature could be parsed, 0 otherwise. - * Args: ctx: a secp256k1 context object - * Out: sig: a pointer to a signature object - * In: input64: a pointer to the 64-byte array to parse + * Args: ctx: pointer to a context object + * Out: sig: pointer to a signature object + * In: input64: pointer to the 64-byte array to parse * * The signature must consist of a 32-byte big endian R value, followed by a * 32-byte big endian S value. If R or S fall outside of [0..order-1], the @@ -498,9 +498,9 @@ SECP256K1_API int secp256k1_ecdsa_signature_parse_compact( /** Parse a DER ECDSA signature. * * Returns: 1 when the signature could be parsed, 0 otherwise. - * Args: ctx: a secp256k1 context object - * Out: sig: a pointer to a signature object - * In: input: a pointer to the signature to be parsed + * Args: ctx: pointer to a context object + * Out: sig: pointer to a signature object + * In: input: pointer to the signature to be parsed * inputlen: the length of the array pointed to be input * * This function will accept any valid DER encoded signature, even if the @@ -520,13 +520,13 @@ SECP256K1_API int secp256k1_ecdsa_signature_parse_der( /** Serialize an ECDSA signature in DER format. * * Returns: 1 if enough space was available to serialize, 0 otherwise - * Args: ctx: a secp256k1 context object - * Out: output: a pointer to an array to store the DER serialization - * In/Out: outputlen: a pointer to a length integer. Initially, this integer + * Args: ctx: pointer to a context object + * Out: output: pointer to an array to store the DER serialization + * In/Out: outputlen: pointer to a length integer. Initially, this integer * should be set to the length of output. After the call * it will be set to the length of the serialization (even * if 0 was returned). - * In: sig: a pointer to an initialized signature object + * In: sig: pointer to an initialized signature object */ SECP256K1_API int secp256k1_ecdsa_signature_serialize_der( const secp256k1_context *ctx, @@ -538,9 +538,9 @@ SECP256K1_API int secp256k1_ecdsa_signature_serialize_der( /** Serialize an ECDSA signature in compact (64 byte) format. * * Returns: 1 - * Args: ctx: a secp256k1 context object - * Out: output64: a pointer to a 64-byte array to store the compact serialization - * In: sig: a pointer to an initialized signature object + * Args: ctx: pointer to a context object + * Out: output64: pointer to a 64-byte array to store the compact serialization + * In: sig: pointer to an initialized signature object * * See secp256k1_ecdsa_signature_parse_compact for details about the encoding. */ @@ -554,7 +554,7 @@ SECP256K1_API int secp256k1_ecdsa_signature_serialize_compact( * * Returns: 1: correct signature * 0: incorrect or unparseable signature - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object * In: sig: the signature being verified. * msghash32: the 32-byte message hash being verified. * The verifier must make sure to apply a cryptographic @@ -585,12 +585,12 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_verify( /** Convert a signature to a normalized lower-S form. * * Returns: 1 if sigin was not normalized, 0 if it already was. - * Args: ctx: a secp256k1 context object - * Out: sigout: a pointer to a signature to fill with the normalized form, + * Args: ctx: pointer to a context object + * Out: sigout: pointer to a signature to fill with the normalized form, * or copy if the input was already normalized. (can be NULL if * you're only interested in whether the input was already * normalized). - * In: sigin: a pointer to a signature to check/normalize (can be identical to sigout) + * In: sigin: pointer to a signature to check/normalize (can be identical to sigout) * * With ECDSA a third-party can forge a second distinct signature of the same * message, given a single initial signature, but without knowing the key. This diff --git a/src/secp256k1/include/secp256k1_ecdh.h b/src/secp256k1/include/secp256k1_ecdh.h index 515e174299..4d9da3461d 100644 --- a/src/secp256k1/include/secp256k1_ecdh.h +++ b/src/secp256k1/include/secp256k1_ecdh.h @@ -39,7 +39,7 @@ SECP256K1_API const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_de * 0: scalar was invalid (zero or overflow) or hashfp returned 0 * Args: ctx: pointer to a context object. * Out: output: pointer to an array to be filled by hashfp. - * In: pubkey: a pointer to a secp256k1_pubkey containing an initialized public key. + * In: pubkey: pointer to a secp256k1_pubkey containing an initialized public key. * seckey: a 32-byte scalar with which to multiply the point. * hashfp: pointer to a hash function. If NULL, * secp256k1_ecdh_hash_function_sha256 is used diff --git a/src/secp256k1/include/secp256k1_ellswift.h b/src/secp256k1/include/secp256k1_ellswift.h index f79bd88396..ae37287f82 100644 --- a/src/secp256k1/include/secp256k1_ellswift.h +++ b/src/secp256k1/include/secp256k1_ellswift.h @@ -87,7 +87,7 @@ SECP256K1_API const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_xdh_ * Returns: 1 always. * Args: ctx: pointer to a context object * Out: ell64: pointer to a 64-byte array to be filled - * In: pubkey: a pointer to a secp256k1_pubkey containing an + * In: pubkey: pointer to a secp256k1_pubkey containing an * initialized public key * rnd32: pointer to 32 bytes of randomness * @@ -169,7 +169,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ellswift_create( * (will not be NULL) * ell_b64: pointer to the 64-byte encoded public key of party B * (will not be NULL) - * seckey32: a pointer to our 32-byte secret key + * seckey32: pointer to our 32-byte secret key * party: boolean indicating which party we are: zero if we are * party A, non-zero if we are party B. seckey32 must be * the private key corresponding to that party's ell_?64. diff --git a/src/secp256k1/include/secp256k1_extrakeys.h b/src/secp256k1/include/secp256k1_extrakeys.h index 002153eccd..f74d067d6f 100644 --- a/src/secp256k1/include/secp256k1_extrakeys.h +++ b/src/secp256k1/include/secp256k1_extrakeys.h @@ -39,7 +39,7 @@ typedef struct { * Returns: 1 if the public key was fully valid. * 0 if the public key could not be parsed or is invalid. * - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object. * Out: pubkey: pointer to a pubkey object. If 1 is returned, it is set to a * parsed version of input. If not, it's set to an invalid value. * In: input32: pointer to a serialized xonly_pubkey. @@ -54,9 +54,9 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_parse( * * Returns: 1 always. * - * Args: ctx: a secp256k1 context object. - * Out: output32: a pointer to a 32-byte array to place the serialized key in. - * In: pubkey: a pointer to a secp256k1_xonly_pubkey containing an initialized public key. + * Args: ctx: pointer to a context object. + * Out: output32: pointer to a 32-byte array to place the serialized key in. + * In: pubkey: pointer to a secp256k1_xonly_pubkey containing an initialized public key. */ SECP256K1_API int secp256k1_xonly_pubkey_serialize( const secp256k1_context *ctx, @@ -69,7 +69,7 @@ SECP256K1_API int secp256k1_xonly_pubkey_serialize( * Returns: <0 if the first public key is less than the second * >0 if the first public key is greater than the second * 0 if the two public keys are equal - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object. * In: pubkey1: first public key to compare * pubkey2: second public key to compare */ diff --git a/src/secp256k1/include/secp256k1_preallocated.h b/src/secp256k1/include/secp256k1_preallocated.h index 2f67031ec9..4fdb0c7cb4 100644 --- a/src/secp256k1/include/secp256k1_preallocated.h +++ b/src/secp256k1/include/secp256k1_preallocated.h @@ -52,8 +52,8 @@ SECP256K1_API size_t secp256k1_context_preallocated_size( * in the memory. In simpler words, the prealloc pointer (or any pointer derived * from it) should not be used during the lifetime of the context object. * - * Returns: a newly created context object. - * In: prealloc: a pointer to a rewritable contiguous block of memory of + * Returns: pointer to newly created context object. + * In: prealloc: pointer to a rewritable contiguous block of memory of * size at least secp256k1_context_preallocated_size(flags) * bytes, as detailed above. * flags: which parts of the context to initialize. @@ -70,7 +70,7 @@ SECP256K1_API secp256k1_context *secp256k1_context_preallocated_create( * caller-provided memory. * * Returns: the required size of the caller-provided memory block. - * In: ctx: an existing context to copy. + * In: ctx: pointer to a context to copy. */ SECP256K1_API size_t secp256k1_context_preallocated_clone_size( const secp256k1_context *ctx @@ -89,9 +89,9 @@ SECP256K1_API size_t secp256k1_context_preallocated_clone_size( * Cloning secp256k1_context_static is not possible, and should not be emulated by * the caller (e.g., using memcpy). Create a new context instead. * - * Returns: a newly created context object. - * Args: ctx: an existing context to copy (not secp256k1_context_static). - * In: prealloc: a pointer to a rewritable contiguous block of memory of + * Returns: pointer to a newly created context object. + * Args: ctx: pointer to a context to copy (not secp256k1_context_static). + * In: prealloc: pointer to a rewritable contiguous block of memory of * size at least secp256k1_context_preallocated_size(flags) * bytes, as detailed above. */ @@ -116,7 +116,7 @@ SECP256K1_API secp256k1_context *secp256k1_context_preallocated_clone( * preallocated pointer given to secp256k1_context_preallocated_create or * secp256k1_context_preallocated_clone. * - * Args: ctx: an existing context to destroy, constructed using + * Args: ctx: pointer to a context to destroy, constructed using * secp256k1_context_preallocated_create or * secp256k1_context_preallocated_clone * (i.e., not secp256k1_context_static). diff --git a/src/secp256k1/include/secp256k1_recovery.h b/src/secp256k1/include/secp256k1_recovery.h index 16f0c71287..ba7ae31017 100644 --- a/src/secp256k1/include/secp256k1_recovery.h +++ b/src/secp256k1/include/secp256k1_recovery.h @@ -28,9 +28,9 @@ typedef struct { /** Parse a compact ECDSA signature (64 bytes + recovery id). * * Returns: 1 when the signature could be parsed, 0 otherwise - * Args: ctx: a secp256k1 context object - * Out: sig: a pointer to a signature object - * In: input64: a pointer to a 64-byte compact signature + * Args: ctx: pointer to a context object + * Out: sig: pointer to a signature object + * In: input64: pointer to a 64-byte compact signature * recid: the recovery id (0, 1, 2 or 3) */ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_parse_compact( @@ -43,9 +43,9 @@ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_parse_compact( /** Convert a recoverable signature into a normal signature. * * Returns: 1 - * Args: ctx: a secp256k1 context object. - * Out: sig: a pointer to a normal signature. - * In: sigin: a pointer to a recoverable signature. + * Args: ctx: pointer to a context object. + * Out: sig: pointer to a normal signature. + * In: sigin: pointer to a recoverable signature. */ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_convert( const secp256k1_context *ctx, @@ -56,10 +56,10 @@ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_convert( /** Serialize an ECDSA signature in compact format (64 bytes + recovery id). * * Returns: 1 - * Args: ctx: a secp256k1 context object. - * Out: output64: a pointer to a 64-byte array of the compact signature. - * recid: a pointer to an integer to hold the recovery id. - * In: sig: a pointer to an initialized signature object. + * Args: ctx: pointer to a context object. + * Out: output64: pointer to a 64-byte array of the compact signature. + * recid: pointer to an integer to hold the recovery id. + * In: sig: pointer to an initialized signature object. */ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_serialize_compact( const secp256k1_context *ctx, diff --git a/src/secp256k1/include/secp256k1_schnorrsig.h b/src/secp256k1/include/secp256k1_schnorrsig.h index 8ccccd24f8..5398655f13 100644 --- a/src/secp256k1/include/secp256k1_schnorrsig.h +++ b/src/secp256k1/include/secp256k1_schnorrsig.h @@ -169,11 +169,11 @@ SECP256K1_API int secp256k1_schnorrsig_sign_custom( * * Returns: 1: correct signature * 0: incorrect signature - * Args: ctx: a secp256k1 context object, initialized for verification. + * Args: ctx: pointer to a context object. * In: sig64: pointer to the 64-byte signature to verify. * msg: the message being verified. Can only be NULL if msglen is 0. * msglen: length of the message - * pubkey: pointer to an x-only public key to verify with (cannot be NULL) + * pubkey: pointer to an x-only public key to verify with */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_schnorrsig_verify( const secp256k1_context *ctx, diff --git a/src/secp256k1/src/field.h b/src/secp256k1/src/field.h index bd589bf8a8..8c65a3aff6 100644 --- a/src/secp256k1/src/field.h +++ b/src/secp256k1/src/field.h @@ -255,8 +255,8 @@ static void secp256k1_fe_add(secp256k1_fe *r, const secp256k1_fe *a); /** Multiply two field elements. * * On input, a and b must be valid field elements; r does not need to be initialized. - * r and a may point to the same object, but neither can be equal to b. The magnitudes - * of a and b must not exceed 8. + * r and a may point to the same object, but neither may point to the object pointed + * to by b. The magnitudes of a and b must not exceed 8. * Performs {r = a * b} * On output, r will have magnitude 1, but won't be normalized. */ diff --git a/src/secp256k1/src/modules/ellswift/tests_impl.h b/src/secp256k1/src/modules/ellswift/tests_impl.h index 7d1efbc492..f96e3a1268 100644 --- a/src/secp256k1/src/modules/ellswift/tests_impl.h +++ b/src/secp256k1/src/modules/ellswift/tests_impl.h @@ -188,9 +188,9 @@ void run_ellswift_tests(void) { CHECK(ret == ((testcase->enc_bitmap >> c) & 1)); if (ret) { secp256k1_fe x2; - CHECK(check_fe_equal(&t, &testcase->encs[c])); + CHECK(fe_equal(&t, &testcase->encs[c])); secp256k1_ellswift_xswiftec_var(&x2, &testcase->u, &testcase->encs[c]); - CHECK(check_fe_equal(&testcase->x, &x2)); + CHECK(fe_equal(&testcase->x, &x2)); } } } @@ -203,7 +203,7 @@ void run_ellswift_tests(void) { CHECK(ret); ret = secp256k1_pubkey_load(CTX, &ge, &pubkey); CHECK(ret); - CHECK(check_fe_equal(&testcase->x, &ge.x)); + CHECK(fe_equal(&testcase->x, &ge.x)); CHECK(secp256k1_fe_is_odd(&ge.y) == testcase->odd_y); } for (i = 0; (unsigned)i < sizeof(ellswift_xdh_tests_bip324) / sizeof(ellswift_xdh_tests_bip324[0]); ++i) { @@ -290,7 +290,7 @@ void run_ellswift_tests(void) { secp256k1_ecmult(&resj, &decj, &sec, NULL); secp256k1_ge_set_gej(&res, &resj); /* Compare. */ - CHECK(check_fe_equal(&res.x, &share_x)); + CHECK(fe_equal(&res.x, &share_x)); } /* Verify the joint behavior of secp256k1_ellswift_xdh */ for (i = 0; i < 200 * COUNT; i++) { diff --git a/src/secp256k1/src/scalar_4x64_impl.h b/src/secp256k1/src/scalar_4x64_impl.h index c9f190e712..dc60d2caf7 100644 --- a/src/secp256k1/src/scalar_4x64_impl.h +++ b/src/secp256k1/src/scalar_4x64_impl.h @@ -460,6 +460,14 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l) : "S"(l), "i"(SECP256K1_N_C_0), "i"(SECP256K1_N_C_1) : "rax", "rdx", "r8", "r9", "r10", "r11", "r12", "r13", "r14", "cc"); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m0, sizeof(m0)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m1, sizeof(m1)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m2, sizeof(m2)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m3, sizeof(m3)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m4, sizeof(m4)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m5, sizeof(m5)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m6, sizeof(m6)); + /* Reduce 385 bits into 258. */ __asm__ __volatile__( /* Preload */ @@ -539,6 +547,12 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l) : "g"(m0), "g"(m1), "g"(m2), "g"(m3), "g"(m4), "g"(m5), "g"(m6), "i"(SECP256K1_N_C_0), "i"(SECP256K1_N_C_1) : "rax", "rdx", "r8", "r9", "r10", "r11", "r12", "r13", "cc"); + SECP256K1_CHECKMEM_MSAN_DEFINE(&p0, sizeof(p0)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&p1, sizeof(p1)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&p2, sizeof(p2)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&p3, sizeof(p3)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&p4, sizeof(p4)); + /* Reduce 258 bits into 256. */ __asm__ __volatile__( /* Preload */ @@ -584,6 +598,10 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l) : "=g"(c) : "g"(p0), "g"(p1), "g"(p2), "g"(p3), "g"(p4), "D"(r), "i"(SECP256K1_N_C_0), "i"(SECP256K1_N_C_1) : "rax", "rdx", "r8", "r9", "r10", "cc", "memory"); + + SECP256K1_CHECKMEM_MSAN_DEFINE(r, sizeof(*r)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&c, sizeof(c)); + #else uint128_t c; uint64_t c0, c1, c2; @@ -657,7 +675,7 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l) secp256k1_scalar_reduce(r, c + secp256k1_scalar_check_overflow(r)); } -static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, const secp256k1_scalar *b) { +static void secp256k1_scalar_mul_512(uint64_t *l8, const secp256k1_scalar *a, const secp256k1_scalar *b) { #ifdef USE_ASM_X86_64 const uint64_t *pb = b->d; __asm__ __volatile__( @@ -672,7 +690,7 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c /* (rax,rdx) = a0 * b0 */ "movq %%r15, %%rax\n" "mulq %%r11\n" - /* Extract l0 */ + /* Extract l8[0] */ "movq %%rax, 0(%%rsi)\n" /* (r8,r9,r10) = (rdx) */ "movq %%rdx, %%r8\n" @@ -690,7 +708,7 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c "addq %%rax, %%r8\n" "adcq %%rdx, %%r9\n" "adcq $0, %%r10\n" - /* Extract l1 */ + /* Extract l8[1] */ "movq %%r8, 8(%%rsi)\n" "xorq %%r8, %%r8\n" /* (r9,r10,r8) += a0 * b2 */ @@ -711,7 +729,7 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c "addq %%rax, %%r9\n" "adcq %%rdx, %%r10\n" "adcq $0, %%r8\n" - /* Extract l2 */ + /* Extract l8[2] */ "movq %%r9, 16(%%rsi)\n" "xorq %%r9, %%r9\n" /* (r10,r8,r9) += a0 * b3 */ @@ -740,7 +758,7 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c "addq %%rax, %%r10\n" "adcq %%rdx, %%r8\n" "adcq $0, %%r9\n" - /* Extract l3 */ + /* Extract l8[3] */ "movq %%r10, 24(%%rsi)\n" "xorq %%r10, %%r10\n" /* (r8,r9,r10) += a1 * b3 */ @@ -761,7 +779,7 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c "addq %%rax, %%r8\n" "adcq %%rdx, %%r9\n" "adcq $0, %%r10\n" - /* Extract l4 */ + /* Extract l8[4] */ "movq %%r8, 32(%%rsi)\n" "xorq %%r8, %%r8\n" /* (r9,r10,r8) += a2 * b3 */ @@ -776,51 +794,54 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c "addq %%rax, %%r9\n" "adcq %%rdx, %%r10\n" "adcq $0, %%r8\n" - /* Extract l5 */ + /* Extract l8[5] */ "movq %%r9, 40(%%rsi)\n" /* (r10,r8) += a3 * b3 */ "movq %%r15, %%rax\n" "mulq %%r14\n" "addq %%rax, %%r10\n" "adcq %%rdx, %%r8\n" - /* Extract l6 */ + /* Extract l8[6] */ "movq %%r10, 48(%%rsi)\n" - /* Extract l7 */ + /* Extract l8[7] */ "movq %%r8, 56(%%rsi)\n" : "+d"(pb) - : "S"(l), "D"(a->d) + : "S"(l8), "D"(a->d) : "rax", "rbx", "rcx", "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", "cc", "memory"); + + SECP256K1_CHECKMEM_MSAN_DEFINE(l8, sizeof(*l8) * 8); + #else /* 160 bit accumulator. */ uint64_t c0 = 0, c1 = 0; uint32_t c2 = 0; - /* l[0..7] = a[0..3] * b[0..3]. */ + /* l8[0..7] = a[0..3] * b[0..3]. */ muladd_fast(a->d[0], b->d[0]); - extract_fast(l[0]); + extract_fast(l8[0]); muladd(a->d[0], b->d[1]); muladd(a->d[1], b->d[0]); - extract(l[1]); + extract(l8[1]); muladd(a->d[0], b->d[2]); muladd(a->d[1], b->d[1]); muladd(a->d[2], b->d[0]); - extract(l[2]); + extract(l8[2]); muladd(a->d[0], b->d[3]); muladd(a->d[1], b->d[2]); muladd(a->d[2], b->d[1]); muladd(a->d[3], b->d[0]); - extract(l[3]); + extract(l8[3]); muladd(a->d[1], b->d[3]); muladd(a->d[2], b->d[2]); muladd(a->d[3], b->d[1]); - extract(l[4]); + extract(l8[4]); muladd(a->d[2], b->d[3]); muladd(a->d[3], b->d[2]); - extract(l[5]); + extract(l8[5]); muladd_fast(a->d[3], b->d[3]); - extract_fast(l[6]); + extract_fast(l8[6]); VERIFY_CHECK(c1 == 0); - l[7] = c0; + l8[7] = c0; #endif } diff --git a/src/secp256k1/src/scalar_impl.h b/src/secp256k1/src/scalar_impl.h index bbba83e937..972d8041b0 100644 --- a/src/secp256k1/src/scalar_impl.h +++ b/src/secp256k1/src/scalar_impl.h @@ -229,7 +229,7 @@ static void secp256k1_scalar_split_lambda(secp256k1_scalar * SECP256K1_RESTRICT * <= {triangle inequality} * a1*|k*b2/n - c1| + a2*|k*(-b1)/n - c2| * < {Lemma 1 and Lemma 2} - * a1*(2^-1 + epslion1) + a2*(2^-1 + epsilon2) + * a1*(2^-1 + epsilon1) + a2*(2^-1 + epsilon2) * < {rounding up to an integer} * (a1 + a2 + 1)/2 * < {rounding up to a power of 2} @@ -247,7 +247,7 @@ static void secp256k1_scalar_split_lambda(secp256k1_scalar * SECP256K1_RESTRICT * <= {triangle inequality} * (-b1)*|k*b2/n - c1| + b2*|k*(-b1)/n - c2| * < {Lemma 1 and Lemma 2} - * (-b1)*(2^-1 + epslion1) + b2*(2^-1 + epsilon2) + * (-b1)*(2^-1 + epsilon1) + b2*(2^-1 + epsilon2) * < {rounding up to an integer} * (-b1 + b2)/2 + 1 * < {rounding up to a power of 2} diff --git a/src/secp256k1/src/secp256k1.c b/src/secp256k1/src/secp256k1.c index 4c11e7f0b8..15a5eede67 100644 --- a/src/secp256k1/src/secp256k1.c +++ b/src/secp256k1/src/secp256k1.c @@ -237,36 +237,25 @@ static SECP256K1_INLINE void secp256k1_declassify(const secp256k1_context* ctx, } static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge, const secp256k1_pubkey* pubkey) { - if (sizeof(secp256k1_ge_storage) == 64) { - /* When the secp256k1_ge_storage type is exactly 64 byte, use its - * representation inside secp256k1_pubkey, as conversion is very fast. - * Note that secp256k1_pubkey_save must use the same representation. */ - secp256k1_ge_storage s; - memcpy(&s, &pubkey->data[0], sizeof(s)); - secp256k1_ge_from_storage(ge, &s); - } else { - /* Otherwise, fall back to 32-byte big endian for X and Y. */ - secp256k1_fe x, y; - ARG_CHECK(secp256k1_fe_set_b32_limit(&x, pubkey->data)); - ARG_CHECK(secp256k1_fe_set_b32_limit(&y, pubkey->data + 32)); - secp256k1_ge_set_xy(ge, &x, &y); - } + secp256k1_ge_storage s; + + /* We require that the secp256k1_ge_storage type is exactly 64 bytes. + * This is formally not guaranteed by the C standard, but should hold on any + * sane compiler in the real world. */ + STATIC_ASSERT(sizeof(secp256k1_ge_storage) == 64); + memcpy(&s, &pubkey->data[0], 64); + secp256k1_ge_from_storage(ge, &s); ARG_CHECK(!secp256k1_fe_is_zero(&ge->x)); return 1; } static void secp256k1_pubkey_save(secp256k1_pubkey* pubkey, secp256k1_ge* ge) { - if (sizeof(secp256k1_ge_storage) == 64) { - secp256k1_ge_storage s; - secp256k1_ge_to_storage(&s, ge); - memcpy(&pubkey->data[0], &s, sizeof(s)); - } else { - VERIFY_CHECK(!secp256k1_ge_is_infinity(ge)); - secp256k1_fe_normalize_var(&ge->x); - secp256k1_fe_normalize_var(&ge->y); - secp256k1_fe_get_b32(pubkey->data, &ge->x); - secp256k1_fe_get_b32(pubkey->data + 32, &ge->y); - } + secp256k1_ge_storage s; + + STATIC_ASSERT(sizeof(secp256k1_ge_storage) == 64); + VERIFY_CHECK(!secp256k1_ge_is_infinity(ge)); + secp256k1_ge_to_storage(&s, ge); + memcpy(&pubkey->data[0], &s, 64); } int secp256k1_ec_pubkey_parse(const secp256k1_context* ctx, secp256k1_pubkey* pubkey, const unsigned char *input, size_t inputlen) { diff --git a/src/secp256k1/src/tests.c b/src/secp256k1/src/tests.c index d7da8dd3a9..67608a6641 100644 --- a/src/secp256k1/src/tests.c +++ b/src/secp256k1/src/tests.c @@ -2928,20 +2928,18 @@ static void run_scalar_tests(void) { secp256k1_scalar_set_b32(&r2, res[i][1], &overflow); CHECK(!overflow); secp256k1_scalar_mul(&z, &x, &y); - CHECK(!secp256k1_scalar_check_overflow(&z)); CHECK(secp256k1_scalar_eq(&r1, &z)); if (!secp256k1_scalar_is_zero(&y)) { secp256k1_scalar_inverse(&zz, &y); - CHECK(!secp256k1_scalar_check_overflow(&zz)); secp256k1_scalar_inverse_var(&zzv, &y); CHECK(secp256k1_scalar_eq(&zzv, &zz)); secp256k1_scalar_mul(&z, &z, &zz); - CHECK(!secp256k1_scalar_check_overflow(&z)); CHECK(secp256k1_scalar_eq(&x, &z)); secp256k1_scalar_mul(&zz, &zz, &y); - CHECK(!secp256k1_scalar_check_overflow(&zz)); CHECK(secp256k1_scalar_eq(&secp256k1_scalar_one, &zz)); } + secp256k1_scalar_mul(&z, &x, &x); + CHECK(secp256k1_scalar_eq(&r2, &z)); } } } @@ -2956,7 +2954,7 @@ static void random_fe_non_square(secp256k1_fe *ns) { } } -static int check_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) { +static int fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) { secp256k1_fe an = *a; secp256k1_fe bn = *b; secp256k1_fe_normalize_weak(&an); @@ -3093,7 +3091,7 @@ static void run_field_half(void) { #endif secp256k1_fe_normalize_weak(&u); secp256k1_fe_add(&u, &u); - CHECK(check_fe_equal(&t, &u)); + CHECK(fe_equal(&t, &u)); /* Check worst-case input: ensure the LSB is 1 so that P will be added, * which will also cause all carries to be 1, since all limbs that can @@ -3112,7 +3110,7 @@ static void run_field_half(void) { #endif secp256k1_fe_normalize_weak(&u); secp256k1_fe_add(&u, &u); - CHECK(check_fe_equal(&t, &u)); + CHECK(fe_equal(&t, &u)); } } @@ -3139,7 +3137,7 @@ static void run_field_misc(void) { secp256k1_fe_add(&z, &q); /* z = x+v */ q = x; /* q = x */ secp256k1_fe_add_int(&q, v); /* q = x+v */ - CHECK(check_fe_equal(&q, &z)); + CHECK(fe_equal(&q, &z)); /* Test the fe equality and comparison operations. */ CHECK(secp256k1_fe_cmp_var(&x, &x) == 0); CHECK(secp256k1_fe_equal(&x, &x)); @@ -3199,27 +3197,27 @@ static void run_field_misc(void) { secp256k1_fe_add(&y, &x); z = x; secp256k1_fe_mul_int(&z, 3); - CHECK(check_fe_equal(&y, &z)); + CHECK(fe_equal(&y, &z)); secp256k1_fe_add(&y, &x); secp256k1_fe_add(&z, &x); - CHECK(check_fe_equal(&z, &y)); + CHECK(fe_equal(&z, &y)); z = x; secp256k1_fe_mul_int(&z, 5); secp256k1_fe_mul(&q, &x, &fe5); - CHECK(check_fe_equal(&z, &q)); + CHECK(fe_equal(&z, &q)); secp256k1_fe_negate(&x, &x, 1); secp256k1_fe_add(&z, &x); secp256k1_fe_add(&q, &x); - CHECK(check_fe_equal(&y, &z)); - CHECK(check_fe_equal(&q, &y)); + CHECK(fe_equal(&y, &z)); + CHECK(fe_equal(&q, &y)); /* Check secp256k1_fe_half. */ z = x; secp256k1_fe_half(&z); secp256k1_fe_add(&z, &z); - CHECK(check_fe_equal(&x, &z)); + CHECK(fe_equal(&x, &z)); secp256k1_fe_add(&z, &z); secp256k1_fe_half(&z); - CHECK(check_fe_equal(&x, &z)); + CHECK(fe_equal(&x, &z)); } } @@ -3288,18 +3286,31 @@ static void run_fe_mul(void) { } static void run_sqr(void) { - secp256k1_fe x, s; + int i; + secp256k1_fe x, y, lhs, rhs, tmp; - { - int i; - secp256k1_fe_set_int(&x, 1); - secp256k1_fe_negate(&x, &x, 1); + secp256k1_fe_set_int(&x, 1); + secp256k1_fe_negate(&x, &x, 1); - for (i = 1; i <= 512; ++i) { - secp256k1_fe_mul_int(&x, 2); - secp256k1_fe_normalize(&x); - secp256k1_fe_sqr(&s, &x); - } + for (i = 1; i <= 512; ++i) { + secp256k1_fe_mul_int(&x, 2); + secp256k1_fe_normalize(&x); + + /* Check that (x+y)*(x-y) = x^2 - y*2 for some random values y */ + random_fe_test(&y); + + lhs = x; + secp256k1_fe_add(&lhs, &y); /* lhs = x+y */ + secp256k1_fe_negate(&tmp, &y, 1); /* tmp = -y */ + secp256k1_fe_add(&tmp, &x); /* tmp = x-y */ + secp256k1_fe_mul(&lhs, &lhs, &tmp); /* lhs = (x+y)*(x-y) */ + + secp256k1_fe_sqr(&rhs, &x); /* rhs = x^2 */ + secp256k1_fe_sqr(&tmp, &y); /* tmp = y^2 */ + secp256k1_fe_negate(&tmp, &tmp, 1); /* tmp = -y^2 */ + secp256k1_fe_add(&rhs, &tmp); /* rhs = x^2 - y^2 */ + + CHECK(fe_equal(&lhs, &rhs)); } } @@ -3621,9 +3632,9 @@ static void run_inverse_tests(void) for (i = 0; (size_t)i < sizeof(fe_cases)/sizeof(fe_cases[0]); ++i) { for (var = 0; var <= 1; ++var) { test_inverse_field(&x_fe, &fe_cases[i][0], var); - check_fe_equal(&x_fe, &fe_cases[i][1]); + CHECK(fe_equal(&x_fe, &fe_cases[i][1])); test_inverse_field(&x_fe, &fe_cases[i][1], var); - check_fe_equal(&x_fe, &fe_cases[i][0]); + CHECK(fe_equal(&x_fe, &fe_cases[i][0])); } } for (i = 0; (size_t)i < sizeof(scalar_cases)/sizeof(scalar_cases[0]); ++i) { @@ -4564,7 +4575,7 @@ static void ecmult_const_mult_xonly(void) { /* Check that resj's X coordinate corresponds with resx. */ secp256k1_fe_sqr(&v, &resj.z); secp256k1_fe_mul(&v, &v, &resx); - CHECK(check_fe_equal(&v, &resj.x)); + CHECK(fe_equal(&v, &resj.x)); } /* Test that secp256k1_ecmult_const_xonly correctly rejects X coordinates not on curve. */ diff --git a/src/secp256k1/src/util.h b/src/secp256k1/src/util.h index 187bf1c5e0..154d9ebcf1 100644 --- a/src/secp256k1/src/util.h +++ b/src/secp256k1/src/util.h @@ -51,13 +51,27 @@ static void print_buf_plain(const unsigned char *buf, size_t len) { # define SECP256K1_INLINE inline # endif +/** Assert statically that expr is true. + * + * This is a statement-like macro and can only be used inside functions. + */ +#define STATIC_ASSERT(expr) do { \ + switch(0) { \ + case 0: \ + /* If expr evaluates to 0, we have two case labels "0", which is illegal. */ \ + case /* ERROR: static assertion failed */ (expr): \ + ; \ + } \ +} while(0) + /** Assert statically that expr is an integer constant expression, and run stmt. * * Useful for example to enforce that magnitude arguments are constant. */ #define ASSERT_INT_CONST_AND_DO(expr, stmt) do { \ switch(42) { \ - case /* ERROR: integer argument is not constant */ expr: \ + /* C allows only integer constant expressions as case labels. */ \ + case /* ERROR: integer argument is not constant */ (expr): \ break; \ default: ; \ } \ diff --git a/src/secp256k1/tools/check-abi.sh b/src/secp256k1/tools/check-abi.sh index 8f6119cd8e..55c945ac16 100755 --- a/src/secp256k1/tools/check-abi.sh +++ b/src/secp256k1/tools/check-abi.sh @@ -3,17 +3,19 @@ set -eu default_base_version="$(git describe --match "v*.*.*" --abbrev=0)" -default_new_version="master" +default_new_version="HEAD" display_help_and_exit() { - echo "Usage: $0 " + echo "Usage: $0 [ []]" echo "" echo "Description: This script uses the ABI Compliance Checker tool to determine if the ABI" echo " of a new version of libsecp256k1 has changed in a backward-incompatible way." echo "" echo "Options:" - echo " base_ver Specify the base version (default: $default_base_version)" - echo " new_ver Specify the new version (default: $default_new_version)" + echo " base_ver Specify the base version as a git commit-ish" + echo " (default: most recent reachable tag matching \"v.*.*\", currently \"$default_base_version\")" + echo " new_ver Specify the new version as a git commit-ish" + echo " (default: $default_new_version)" echo " -h, --help Display this help message" exit 0 } @@ -23,9 +25,11 @@ if [ "$#" -eq 0 ]; then new_version="$default_new_version" elif [ "$#" -eq 1 ] && { [ "$1" = "-h" ] || [ "$1" = "--help" ]; }; then display_help_and_exit -elif [ "$#" -eq 2 ]; then +elif [ "$#" -eq 1 ] || [ "$#" -eq 2 ]; then base_version="$1" - new_version="$2" + if [ "$#" -eq 2 ]; then + new_version="$2" + fi else echo "Invalid usage. See help:" echo "" @@ -33,7 +37,8 @@ else fi checkout_and_build() { - git worktree add -d "$1" "$2" + _orig_dir="$(pwd)" + git worktree add --detach "$1" "$2" cd "$1" mkdir build && cd build cmake -S .. --preset dev-mode \ @@ -45,20 +50,18 @@ checkout_and_build() { -DSECP256K1_BUILD_EXAMPLES=OFF cmake --build . -j "$(nproc)" abi-dumper src/libsecp256k1.so -o ABI.dump -lver "$2" + cd "$_orig_dir" } echo "Comparing $base_version (base version) to $new_version (new version)" echo -original_dir="$(pwd)" - -base_source_dir=$(mktemp -d) +base_source_dir="$(mktemp -d)" checkout_and_build "$base_source_dir" "$base_version" -new_source_dir=$(mktemp -d) +new_source_dir="$(mktemp -d)" checkout_and_build "$new_source_dir" "$new_version" -cd "$original_dir" abi-compliance-checker -lib libsecp256k1 -old "${base_source_dir}/build/ABI.dump" -new "${new_source_dir}/build/ABI.dump" git worktree remove "$base_source_dir" git worktree remove "$new_source_dir" From 5d24f45426e0290c5cfec10be9cd72717ac703fa Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Tue, 26 Mar 2024 16:03:41 +0100 Subject: [PATCH 014/530] test: Extends wait_for_getheaders so a specific block hash can be checked Previously, `wait_for_getheaders` would check whether a node had received **any** getheaders message. This implied that, if a test needed to check for a specific block hash within a headers message, it had to make sure that it was checking the desired message. This normally involved having to manually clear `last_message`. This method, apart from being too verbose, was error prone, given an undesired `getheaders` would make tests pass. This adds the ability to check for a specific block_hash within the last `getheaders` message. --- test/functional/mining_basic.py | 2 +- test/functional/p2p_compactblocks.py | 2 +- test/functional/p2p_initial_headers_sync.py | 15 +++++---------- test/functional/p2p_segwit.py | 3 +-- test/functional/p2p_sendheaders.py | 21 +++++++++++---------- test/functional/test_framework/p2p.py | 16 +++++++++------- 6 files changed, 28 insertions(+), 31 deletions(-) diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index 7c9ff9ba59..9911a82cdd 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -308,7 +308,7 @@ def chain_tip(b_hash, *, status='headers-only', branchlen=1): # Should ask for the block from a p2p node, if they announce the header as well: peer = node.add_p2p_connection(P2PDataStore()) - peer.wait_for_getheaders(timeout=5) # Drop the first getheaders + peer.wait_for_getheaders(timeout=5, block_hash=block.hashPrevBlock) peer.send_blocks_and_test(blocks=[block], node=node) # Must be active now: assert chain_tip(block.hash, status='active', branchlen=0) in node.getchaintips() diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index 9fd96d51e3..4a8ff3eb7f 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -387,7 +387,7 @@ def test_compactblock_requests(self, test_node): if announce == "inv": test_node.send_message(msg_inv([CInv(MSG_BLOCK, block.sha256)])) - test_node.wait_until(lambda: "getheaders" in test_node.last_message, timeout=30) + test_node.wait_for_getheaders(timeout=30) test_node.send_header_for_blocks([block]) else: test_node.send_header_for_blocks([block]) diff --git a/test/functional/p2p_initial_headers_sync.py b/test/functional/p2p_initial_headers_sync.py index 2f591a3b58..35743a0818 100755 --- a/test/functional/p2p_initial_headers_sync.py +++ b/test/functional/p2p_initial_headers_sync.py @@ -38,9 +38,10 @@ def announce_random_block(self, peers): def run_test(self): self.log.info("Adding a peer to node0") peer1 = self.nodes[0].add_p2p_connection(P2PInterface()) + best_block_hash = int(self.nodes[0].getbestblockhash(), 16) # Wait for peer1 to receive a getheaders - peer1.wait_for_getheaders() + peer1.wait_for_getheaders(block_hash=best_block_hash) # An empty reply will clear the outstanding getheaders request, # allowing additional getheaders requests to be sent to this peer in # the future. @@ -60,17 +61,12 @@ def run_test(self): assert "getheaders" not in peer2.last_message assert "getheaders" not in peer3.last_message - with p2p_lock: - peer1.last_message.pop("getheaders", None) - self.log.info("Have all peers announce a new block") self.announce_random_block(all_peers) self.log.info("Check that peer1 receives a getheaders in response") - peer1.wait_for_getheaders() + peer1.wait_for_getheaders(block_hash=best_block_hash) peer1.send_message(msg_headers()) # Send empty response, see above - with p2p_lock: - peer1.last_message.pop("getheaders", None) self.log.info("Check that exactly 1 of {peer2, peer3} received a getheaders in response") count = 0 @@ -80,7 +76,6 @@ def run_test(self): if "getheaders" in p.last_message: count += 1 peer_receiving_getheaders = p - p.last_message.pop("getheaders", None) p.send_message(msg_headers()) # Send empty response, see above assert_equal(count, 1) @@ -89,14 +84,14 @@ def run_test(self): self.announce_random_block(all_peers) self.log.info("Check that peer1 receives a getheaders in response") - peer1.wait_for_getheaders() + peer1.wait_for_getheaders(block_hash=best_block_hash) self.log.info("Check that the remaining peer received a getheaders as well") expected_peer = peer2 if peer2 == peer_receiving_getheaders: expected_peer = peer3 - expected_peer.wait_for_getheaders() + expected_peer.wait_for_getheaders(block_hash=best_block_hash) self.log.info("Success!") diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index f48d5bd44a..ec24f5f77d 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -201,14 +201,13 @@ def announce_tx_and_wait_for_getdata(self, tx, success=True, use_wtxid=False): def announce_block_and_wait_for_getdata(self, block, use_header, timeout=60): with p2p_lock: self.last_message.pop("getdata", None) - self.last_message.pop("getheaders", None) msg = msg_headers() msg.headers = [CBlockHeader(block)] if use_header: self.send_message(msg) else: self.send_message(msg_inv(inv=[CInv(MSG_BLOCK, block.sha256)])) - self.wait_for_getheaders(timeout=timeout) + self.wait_for_getheaders(block_hash=block.hashPrevBlock, timeout=timeout) self.send_message(msg) self.wait_for_getdata([block.sha256], timeout=timeout) diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py index 2cf724596f..8a203f3470 100755 --- a/test/functional/p2p_sendheaders.py +++ b/test/functional/p2p_sendheaders.py @@ -311,6 +311,7 @@ def test_nonnull_locators(self, test_node, inv_node): # Now that we've synced headers, headers announcements should work tip = self.mine_blocks(1) + expected_hash = tip inv_node.check_last_inv_announcement(inv=[tip]) test_node.check_last_headers_announcement(headers=[tip]) @@ -334,7 +335,10 @@ def test_nonnull_locators(self, test_node, inv_node): if j == 0: # Announce via inv test_node.send_block_inv(tip) - test_node.wait_for_getheaders() + if i == 0: + test_node.wait_for_getheaders(block_hash=expected_hash) + else: + assert "getheaders" not in test_node.last_message # Should have received a getheaders now test_node.send_header_for_blocks(blocks) # Test that duplicate inv's won't result in duplicate @@ -521,6 +525,7 @@ def test_nonnull_locators(self, test_node, inv_node): self.log.info("Part 5: Testing handling of unconnecting headers") # First we test that receipt of an unconnecting header doesn't prevent # chain sync. + expected_hash = tip for i in range(10): self.log.debug("Part 5.{}: starting...".format(i)) test_node.last_message.pop("getdata", None) @@ -533,15 +538,14 @@ def test_nonnull_locators(self, test_node, inv_node): block_time += 1 height += 1 # Send the header of the second block -> this won't connect. - with p2p_lock: - test_node.last_message.pop("getheaders", None) test_node.send_header_for_blocks([blocks[1]]) - test_node.wait_for_getheaders() + test_node.wait_for_getheaders(block_hash=expected_hash) test_node.send_header_for_blocks(blocks) test_node.wait_for_getdata([x.sha256 for x in blocks]) [test_node.send_message(msg_block(x)) for x in blocks] test_node.sync_with_ping() assert_equal(int(self.nodes[0].getbestblockhash(), 16), blocks[1].sha256) + expected_hash = blocks[1].sha256 blocks = [] # Now we test that if we repeatedly don't send connecting headers, we @@ -556,13 +560,12 @@ def test_nonnull_locators(self, test_node, inv_node): for i in range(1, MAX_NUM_UNCONNECTING_HEADERS_MSGS): # Send a header that doesn't connect, check that we get a getheaders. - with p2p_lock: - test_node.last_message.pop("getheaders", None) test_node.send_header_for_blocks([blocks[i]]) - test_node.wait_for_getheaders() + test_node.wait_for_getheaders(block_hash=expected_hash) # Next header will connect, should re-set our count: test_node.send_header_for_blocks([blocks[0]]) + expected_hash = blocks[0].sha256 # Remove the first two entries (blocks[1] would connect): blocks = blocks[2:] @@ -571,10 +574,8 @@ def test_nonnull_locators(self, test_node, inv_node): # before we get disconnected. Should be 5*MAX_NUM_UNCONNECTING_HEADERS_MSGS for i in range(5 * MAX_NUM_UNCONNECTING_HEADERS_MSGS - 1): # Send a header that doesn't connect, check that we get a getheaders. - with p2p_lock: - test_node.last_message.pop("getheaders", None) test_node.send_header_for_blocks([blocks[i % len(blocks)]]) - test_node.wait_for_getheaders() + test_node.wait_for_getheaders(block_hash=expected_hash) # Eventually this stops working. test_node.send_header_for_blocks([blocks[-1]]) diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 2fcfc23e4a..16037a1a55 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -644,15 +644,17 @@ def test_function(): self.wait_until(test_function, timeout=timeout) - def wait_for_getheaders(self, *, timeout=60): - """Waits for a getheaders message. + def wait_for_getheaders(self, block_hash=None, *, timeout=60): + """Waits for a getheaders message containing a specific block hash. - Receiving any getheaders message will satisfy the predicate. the last_message["getheaders"] - value must be explicitly cleared before calling this method, or this will return - immediately with success. TODO: change this method to take a hash value and only - return true if the correct block header has been requested.""" + If no block hash is provided, checks whether any getheaders message has been received by the node.""" def test_function(): - return self.last_message.get("getheaders") + last_getheaders = self.last_message.pop("getheaders", None) + if block_hash is None: + return last_getheaders + if last_getheaders is None: + return False + return block_hash == last_getheaders.locator.vHave[0] self.wait_until(test_function, timeout=timeout) From c2ff9792495acd1b36d2457ef0b5b38bdd2d8ebd Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Thu, 4 Apr 2024 14:55:46 +0200 Subject: [PATCH 015/530] test: Fix debug recommendation in argsman_tests --- src/test/argsman_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/argsman_tests.cpp b/src/test/argsman_tests.cpp index 1f46efe464..340208a1c9 100644 --- a/src/test/argsman_tests.cpp +++ b/src/test/argsman_tests.cpp @@ -904,7 +904,7 @@ BOOST_FIXTURE_TEST_CASE(util_ArgsMerge, ArgsMergeTestingSetup) // If check below fails, should manually dump the results with: // - // ARGS_MERGE_TEST_OUT=results.txt ./test_bitcoin --run_test=util_tests/util_ArgsMerge + // ARGS_MERGE_TEST_OUT=results.txt ./test_bitcoin --run_test=argsman_tests/util_ArgsMerge // // And verify diff against previous results to make sure the changes are expected. // @@ -1007,7 +1007,7 @@ BOOST_FIXTURE_TEST_CASE(util_ChainMerge, ChainMergeTestingSetup) // If check below fails, should manually dump the results with: // - // CHAIN_MERGE_TEST_OUT=results.txt ./test_bitcoin --run_test=util_tests/util_ChainMerge + // CHAIN_MERGE_TEST_OUT=results.txt ./test_bitcoin --run_test=argsman_tests/util_ChainMerge // // And verify diff against previous results to make sure the changes are expected. // From 5930f983fd7558b430ecdd0e284a1088fc56bb18 Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Thu, 4 Apr 2024 21:08:38 +0100 Subject: [PATCH 016/530] gui: don't permit port in proxy IP option Fixes: #809 Previously it was possible through the GUI to enter an IP address:port into the "Proxy IP" configuration box. After the node was restarted the errant setting would prevent the node starting back up until manually removed from settings.json. --- src/qt/optionsdialog.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index c9abeca9a2..7fc118bcd8 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include @@ -434,7 +435,10 @@ QValidator(parent) QValidator::State ProxyAddressValidator::validate(QString &input, int &pos) const { Q_UNUSED(pos); - // Validate the proxy + uint16_t port{0}; + std::string hostname; + if (!SplitHostPort(input.toStdString(), port, hostname) || port != 0) return QValidator::Invalid; + CService serv(LookupNumeric(input.toStdString(), DEFAULT_GUI_PROXY_PORT)); Proxy addrProxy = Proxy(serv, true); if (addrProxy.IsValid()) From d917a8e9223b110176ba16bc03c2a28bd5244048 Mon Sep 17 00:00:00 2001 From: brunoerg Date: Tue, 2 Apr 2024 09:01:15 -0300 Subject: [PATCH 017/530] doc: i2p: improve `-i2pacceptincoming` mention --- doc/i2p.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/i2p.md b/doc/i2p.md index b6c07388b7..b769a74df4 100644 --- a/doc/i2p.md +++ b/doc/i2p.md @@ -79,8 +79,8 @@ one of the networks has issues. ## Persistent vs transient I2P addresses The first time Bitcoin Core connects to the I2P router, it automatically -generates a persistent I2P address and its corresponding private key by default -or if `-i2pacceptincoming=1` is set. The private key is saved in a file named +generates a persistent I2P address and its corresponding private key by default, +unless `-i2pacceptincoming=0` is set. The private key is saved in a file named `i2p_private_key` in the Bitcoin Core data directory. The persistent I2P address is used for making outbound connections and accepting inbound connections. From b831807e7261c7c835730b9f2b15710a5e182603 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Mon, 1 Apr 2024 14:03:35 +0200 Subject: [PATCH 018/530] test: introduce and use `calculate_input_weight` helper Rather than manually estimating an input's weight by adding up all the involved components (fixed-size skeleton, compact-serialized lengths, and the actual scriptSig / witness stack items) we can simply take use of the serialization classes `CTxIn` / `CTxInWitness` instead, to achieve the same with significantly less code. The new helper is used in the functional tests rpc_psbt.py and wallet_send.py, where the previous manual estimation code was duplicated. --- test/functional/rpc_psbt.py | 17 ++++---------- test/functional/test_framework/wallet_util.py | 18 +++++++++++++++ test/functional/wallet_send.py | 23 ++++++------------- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 3906d80da0..d2087519f3 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -16,8 +16,6 @@ CTxIn, CTxOut, MAX_BIP125_RBF_SEQUENCE, - WITNESS_SCALE_FACTOR, - ser_compact_size, ) from test_framework.psbt import ( PSBT, @@ -42,6 +40,7 @@ find_vout_for_address, ) from test_framework.wallet_util import ( + calculate_input_weight, generate_keypair, get_generate_key, ) @@ -752,17 +751,9 @@ def test_psbt_input_keys(psbt_input, keys): input_idx = i break psbt_in = dec["inputs"][input_idx] - # Calculate the input weight - # (prevout + sequence + length of scriptSig + scriptsig) * WITNESS_SCALE_FACTOR + len of num scriptWitness stack items + (length of stack item + stack item) * N stack items - # Note that occasionally this weight estimate may be slightly larger or smaller than the real weight - # as sometimes ECDSA signatures are one byte shorter than expected with a probability of 1/128 - len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0 - len_scriptsig += len(ser_compact_size(len_scriptsig)) - len_scriptwitness = (sum([(len(x) // 2) + len(ser_compact_size(len(x) // 2)) for x in psbt_in["final_scriptwitness"]]) + len(ser_compact_size(len(psbt_in["final_scriptwitness"])))) if "final_scriptwitness" in psbt_in else 0 - len_prevout_txid = 32 - len_prevout_index = 4 - len_sequence = 4 - input_weight = ((len_prevout_txid + len_prevout_index + len_sequence + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness + scriptsig_hex = psbt_in["final_scriptSig"]["hex"] if "final_scriptSig" in psbt_in else "" + witness_stack_hex = psbt_in["final_scriptwitness"] if "final_scriptwitness" in psbt_in else None + input_weight = calculate_input_weight(scriptsig_hex, witness_stack_hex) low_input_weight = input_weight // 2 high_input_weight = input_weight * 2 diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py index 44811918bf..8104701ff3 100755 --- a/test/functional/test_framework/wallet_util.py +++ b/test/functional/test_framework/wallet_util.py @@ -15,6 +15,11 @@ script_to_p2wsh, ) from test_framework.key import ECKey +from test_framework.messages import ( + CTxIn, + CTxInWitness, + WITNESS_SCALE_FACTOR, +) from test_framework.script_util import ( key_to_p2pkh_script, key_to_p2wpkh_script, @@ -123,6 +128,19 @@ def generate_keypair(compressed=True, wif=False): privkey = bytes_to_wif(privkey.get_bytes(), compressed) return privkey, pubkey +def calculate_input_weight(scriptsig_hex, witness_stack_hex=None): + """Given a scriptSig and a list of witness stack items for an input in hex format, + calculate the total input weight. If the input has no witness data, + `witness_stack_hex` can be set to None.""" + tx_in = CTxIn(scriptSig=bytes.fromhex(scriptsig_hex)) + witness_size = 0 + if witness_stack_hex is not None: + tx_inwit = CTxInWitness() + for witness_item_hex in witness_stack_hex: + tx_inwit.scriptWitness.stack.append(bytes.fromhex(witness_item_hex)) + witness_size = len(tx_inwit.serialize()) + return len(tx_in.serialize()) * WITNESS_SCALE_FACTOR + witness_size + class WalletUnlock(): """ A context manager for unlocking a wallet with a passphrase and automatically locking it afterward. diff --git a/test/functional/wallet_send.py b/test/functional/wallet_send.py index 06dff7fe25..9c72d21512 100755 --- a/test/functional/wallet_send.py +++ b/test/functional/wallet_send.py @@ -9,10 +9,6 @@ from test_framework.authproxy import JSONRPCException from test_framework.descriptors import descsum_create -from test_framework.messages import ( - ser_compact_size, - WITNESS_SCALE_FACTOR, -) from test_framework.test_framework import BGLTestFramework from test_framework.util import ( assert_equal, @@ -21,7 +17,10 @@ assert_raises_rpc_error, count_bytes, ) -from test_framework.wallet_util import generate_keypair +from test_framework.wallet_util import ( + calculate_input_weight, + generate_keypair, +) class WalletSendTest(BGLTestFramework): @@ -544,17 +543,9 @@ def run_test(self): input_idx = i break psbt_in = dec["inputs"][input_idx] - # Calculate the input weight - # (prevout + sequence + length of scriptSig + scriptsig) * WITNESS_SCALE_FACTOR + len of num scriptWitness stack items + (length of stack item + stack item) * N stack items - # Note that occasionally this weight estimate may be slightly larger or smaller than the real weight - # as sometimes ECDSA signatures are one byte shorter than expected with a probability of 1/128 - len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0 - len_scriptsig += len(ser_compact_size(len_scriptsig)) - len_scriptwitness = (sum([(len(x) // 2) + len(ser_compact_size(len(x) // 2)) for x in psbt_in["final_scriptwitness"]]) + len(ser_compact_size(len(psbt_in["final_scriptwitness"])))) if "final_scriptwitness" in psbt_in else 0 - len_prevout_txid = 32 - len_prevout_index = 4 - len_sequence = 4 - input_weight = ((len_prevout_txid + len_prevout_index + len_sequence + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness + scriptsig_hex = psbt_in["final_scriptSig"]["hex"] if "final_scriptSig" in psbt_in else "" + witness_stack_hex = psbt_in["final_scriptwitness"] if "final_scriptwitness" in psbt_in else None + input_weight = calculate_input_weight(scriptsig_hex, witness_stack_hex) # Input weight error conditions assert_raises_rpc_error( From 91db0f5a60a3188d3aa1d297e16942884fb23b63 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Mon, 1 Apr 2024 15:01:38 +0200 Subject: [PATCH 019/530] test: add unit tests for `calculate_input_weight` --- test/functional/test_framework/wallet_util.py | 40 +++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 41 insertions(+) diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py index 8104701ff3..d30b00f4a7 100755 --- a/test/functional/test_framework/wallet_util.py +++ b/test/functional/test_framework/wallet_util.py @@ -4,6 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Useful util functions for testing the wallet""" from collections import namedtuple +import unittest from test_framework.address import ( byte_to_base58, @@ -159,3 +160,42 @@ def __enter__(self): def __exit__(self, *args): _ = args self.wallet.walletlock() + + +class TestFrameworkWalletUtil(unittest.TestCase): + def test_calculate_input_weight(self): + SKELETON_BYTES = 32 + 4 + 4 # prevout-txid, prevout-index, sequence + SMALL_LEN_BYTES = 1 # bytes needed for encoding scriptSig / witness item lenghts < 253 + LARGE_LEN_BYTES = 3 # bytes needed for encoding scriptSig / witness item lengths >= 253 + + # empty scriptSig, no witness + self.assertEqual(calculate_input_weight(""), + (SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + self.assertEqual(calculate_input_weight("", None), + (SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + # small scriptSig, no witness + scriptSig_small = "00"*252 + self.assertEqual(calculate_input_weight(scriptSig_small, None), + (SKELETON_BYTES + SMALL_LEN_BYTES + 252) * WITNESS_SCALE_FACTOR) + # small scriptSig, empty witness stack + self.assertEqual(calculate_input_weight(scriptSig_small, []), + (SKELETON_BYTES + SMALL_LEN_BYTES + 252) * WITNESS_SCALE_FACTOR + SMALL_LEN_BYTES) + # large scriptSig, no witness + scriptSig_large = "00"*253 + self.assertEqual(calculate_input_weight(scriptSig_large, None), + (SKELETON_BYTES + LARGE_LEN_BYTES + 253) * WITNESS_SCALE_FACTOR) + # large scriptSig, empty witness stack + self.assertEqual(calculate_input_weight(scriptSig_large, []), + (SKELETON_BYTES + LARGE_LEN_BYTES + 253) * WITNESS_SCALE_FACTOR + SMALL_LEN_BYTES) + # empty scriptSig, 5 small witness stack items + self.assertEqual(calculate_input_weight("", ["00", "11", "22", "33", "44"]), + ((SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + SMALL_LEN_BYTES + 5 * SMALL_LEN_BYTES + 5) + # empty scriptSig, 253 small witness stack items + self.assertEqual(calculate_input_weight("", ["00"]*253), + ((SKELETON_BYTES + SMALL_LEN_BYTES) * WITNESS_SCALE_FACTOR) + LARGE_LEN_BYTES + 253 * SMALL_LEN_BYTES + 253) + # small scriptSig, 3 large witness stack items + self.assertEqual(calculate_input_weight(scriptSig_small, ["00"*253]*3), + ((SKELETON_BYTES + SMALL_LEN_BYTES + 252) * WITNESS_SCALE_FACTOR) + SMALL_LEN_BYTES + 3 * LARGE_LEN_BYTES + 3*253) + # large scriptSig, 3 large witness stack items + self.assertEqual(calculate_input_weight(scriptSig_large, ["00"*253]*3), + ((SKELETON_BYTES + LARGE_LEN_BYTES + 253) * WITNESS_SCALE_FACTOR) + SMALL_LEN_BYTES + 3 * LARGE_LEN_BYTES + 3*253) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 9b999ada6f..919805f646 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -85,6 +85,7 @@ "crypto.ripemd160", "script", "segwit_addr", + "wallet_util", ] EXTENDED_SCRIPTS = [ From 588a4ed3bd39cfd065d4e3c116cb352da267f762 Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Wed, 19 Apr 2023 16:34:39 -0400 Subject: [PATCH 020/530] depends: switch libnatpmp to CMake Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> --- depends/packages/libnatpmp.mk | 20 +++++++------------- depends/patches/libnatpmp/no_libtool.patch | 15 --------------- 2 files changed, 7 insertions(+), 28 deletions(-) delete mode 100644 depends/patches/libnatpmp/no_libtool.patch diff --git a/depends/packages/libnatpmp.mk b/depends/packages/libnatpmp.mk index caa809ec0b..b018c5d45b 100644 --- a/depends/packages/libnatpmp.mk +++ b/depends/packages/libnatpmp.mk @@ -2,25 +2,19 @@ package=libnatpmp $(package)_version=07004b97cf691774efebe70404cf22201e4d330d $(package)_download_path=https://github.com/miniupnp/libnatpmp/archive $(package)_file_name=$($(package)_version).tar.gz -$(package)_sha256_hash=9321953ceb39d07c25463e266e50d0ae7b64676bb3a986d932b18881ed94f1fb -$(package)_patches=no_libtool.patch +$(package)_sha256_hash=ef84979950dfb3556705b63c9cd6c95501b75e887fba466234b187f3c9029669 +$(package)_build_subdir=build -define $(package)_set_vars - $(package)_build_opts=CC="$($(package)_cc)" - $(package)_build_opts_mingw32=CPPFLAGS=-DNATPMP_STATICLIB - $(package)_build_env+=CFLAGS="$($(package)_cflags) $($(package)_cppflags)" AR="$($(package)_ar)" -endef - -define $(package)_preprocess_cmds - patch -p1 < $($(package)_patch_dir)/no_libtool.patch +define $(package)_config_cmds + $($(package)_cmake) -S .. -B . endef define $(package)_build_cmds - $(MAKE) libnatpmp.a $($(package)_build_opts) + $(MAKE) natpmp endef define $(package)_stage_cmds - mkdir -p $($(package)_staging_prefix_dir)/include $($(package)_staging_prefix_dir)/lib &&\ - install *.h $($(package)_staging_prefix_dir)/include &&\ + mkdir -p $($(package)_staging_prefix_dir)/include $($(package)_staging_prefix_dir)/lib && \ + install ../natpmp.h ../natpmp_declspec.h $($(package)_staging_prefix_dir)/include && \ install libnatpmp.a $($(package)_staging_prefix_dir)/lib endef diff --git a/depends/patches/libnatpmp/no_libtool.patch b/depends/patches/libnatpmp/no_libtool.patch deleted file mode 100644 index 2b9f01f6eb..0000000000 --- a/depends/patches/libnatpmp/no_libtool.patch +++ /dev/null @@ -1,15 +0,0 @@ -diff -ruN libnatpmp-07004b97cf691774efebe70404cf22201e4d330d/Makefile libnatpmp-07004b97cf691774efebe70404cf22201e4d330d.new/Makefile ---- libnatpmp-07004b97cf691774efebe70404cf22201e4d330d/Makefile 2022-07-05 07:49:50.000000000 +0000 -+++ libnatpmp-07004b97cf691774efebe70404cf22201e4d330d.new/Makefile 2024-01-23 20:59:35.674821779 +0000 -@@ -197,11 +197,7 @@ - $(CC) $(LDFLAGS) -o $@ $^ $(LDLIBS) - - $(STATICLIB): $(LIBOBJS) --ifneq (, $(findstring darwin, $(OS))) -- $(LIBTOOL) -static -o $@ $? --else - $(AR) crs $@ $? --endif - - $(SHAREDLIB): $(LIBOBJS) - ifneq (, $(findstring darwin, $(OS))) From b109c6e3bcca51a1737eb829f491a2d735a56e38 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 22 Mar 2024 17:29:22 +0000 Subject: [PATCH 021/530] depends: libnatpmp f2433bec24ca3d3f22a8a7840728a3ac177f94ba This includes once CMake related change I upstreamed: https://github.com/miniupnp/libnatpmp/pull/43. --- depends/packages/libnatpmp.mk | 4 ++-- doc/dependencies.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/depends/packages/libnatpmp.mk b/depends/packages/libnatpmp.mk index b018c5d45b..50e38b2f48 100644 --- a/depends/packages/libnatpmp.mk +++ b/depends/packages/libnatpmp.mk @@ -1,9 +1,9 @@ package=libnatpmp -$(package)_version=07004b97cf691774efebe70404cf22201e4d330d +$(package)_version=f2433bec24ca3d3f22a8a7840728a3ac177f94ba $(package)_download_path=https://github.com/miniupnp/libnatpmp/archive $(package)_file_name=$($(package)_version).tar.gz $(package)_sha256_hash=ef84979950dfb3556705b63c9cd6c95501b75e887fba466234b187f3c9029669 -$(package)_build_subdir=build +$(package)_patches=no_libtool.patch define $(package)_config_cmds $($(package)_cmake) -S .. -B . diff --git a/doc/dependencies.md b/doc/dependencies.md index d3afd98e88..6e115bed5c 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -35,7 +35,7 @@ You can find installation instructions in the `build-*.md` file for your platfor ### Networking | Dependency | Releases | Version used | Minimum required | Runtime | | --- | --- | --- | --- | --- | -| [libnatpmp](../depends/packages/libnatpmp.mk) | [link](https://github.com/miniupnp/libnatpmp/) | commit [07004b9...](https://github.com/bitcoin/bitcoin/pull/25917) | | No | +| [libnatpmp](../depends/packages/libnatpmp.mk) | [link](https://github.com/miniupnp/libnatpmp/) | commit [f2433be...](https://github.com/bitcoin/bitcoin/pull/29708) | | No | | [MiniUPnPc](../depends/packages/miniupnpc.mk) | [link](https://miniupnp.tuxfamily.org/) | [2.2.2](https://github.com/bitcoin/bitcoin/pull/20421) | 2.1 | No | ### Notifications From 7987806f36eec575357c48eeff9be0b4cb7ebd11 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:00:08 +0100 Subject: [PATCH 022/530] ci: Drop duplicated compiler flags --- ci/test/00_setup_env_native_fuzz_with_msan.sh | 2 +- ci/test/00_setup_env_native_msan.sh | 2 +- ci/test/00_setup_env_native_tsan.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh index 8250bd520a..02a6a6a5f2 100755 --- a/ci/test/00_setup_env_native_fuzz_with_msan.sh +++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh @@ -17,7 +17,7 @@ export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" -export BGL_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening --with-asm=no CFLAGS='${MSAN_FLAGS}' CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" +export BGL_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening --with-asm=no CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'" export USE_MEMORY_SANITIZER="true" export RUN_UNIT_TESTS="false" export RUN_FUNCTIONAL_TESTS="false" diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh index 8030dc9958..3f9c55c3da 100644 --- a/ci/test/00_setup_env_native_msan.sh +++ b/ci/test/00_setup_env_native_msan.sh @@ -17,7 +17,7 @@ export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" -export BGL_CONFIG="--with-sanitizers=memory --disable-hardening --with-asm=no CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" +export BGL_CONFIG="--with-sanitizers=memory --disable-hardening --with-asm=no" export USE_MEMORY_SANITIZER="true" export RUN_FUNCTIONAL_TESTS="false" export CCACHE_MAXSIZE=250M diff --git a/ci/test/00_setup_env_native_tsan.sh b/ci/test/00_setup_env_native_tsan.sh index 23ab1ae000..3fcaa8c6c6 100755 --- a/ci/test/00_setup_env_native_tsan.sh +++ b/ci/test/00_setup_env_native_tsan.sh @@ -11,4 +11,4 @@ export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" export PACKAGES="clang-18 llvm-18 libclang-rt-18-dev libc++abi-18-dev libc++-18-dev python3-zmq" export DEP_OPTS="CC=clang-18 CXX='clang++-18 -stdlib=libc++'" export GOAL="install" -export BITCOIN_CONFIG="--enable-zmq CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION' CXXFLAGS='-g' --with-sanitizers=thread" +export BITCOIN_CONFIG="--enable-zmq CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION' --with-sanitizers=thread" From e4f6abe8fe6488f4314d4b6f41ba8ce98f28801b Mon Sep 17 00:00:00 2001 From: Cory Fields Date: Thu, 4 Apr 2024 19:56:21 +0000 Subject: [PATCH 023/530] crypto: chacha20: always use our fallback timingsafe_bcmp rather than libc's Looking at apple/freebsd/openbsd sources, their implementations match our naive fallback. It's not worth the hassle of using a platform-specific function for no gain. --- configure.ac | 2 -- src/crypto/chacha20poly1305.cpp | 13 ++----------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index b28d9fe94c..7d1a9504fd 100755 --- a/configure.ac +++ b/configure.ac @@ -968,8 +968,6 @@ AC_CHECK_DECLS([setsid]) AC_CHECK_DECLS([pipe2]) -AC_CHECK_FUNCS([timingsafe_bcmp]) - AC_MSG_CHECKING([for __builtin_clzl]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ ]], [[ (void) __builtin_clzl(0); diff --git a/src/crypto/chacha20poly1305.cpp b/src/crypto/chacha20poly1305.cpp index bdef957c87..b969bb1a29 100644 --- a/src/crypto/chacha20poly1305.cpp +++ b/src/crypto/chacha20poly1305.cpp @@ -2,10 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif - #include #include @@ -30,10 +26,7 @@ void AEADChaCha20Poly1305::SetKey(Span key) noexcept namespace { -#ifndef HAVE_TIMINGSAFE_BCMP -#define HAVE_TIMINGSAFE_BCMP - -int timingsafe_bcmp(const unsigned char* b1, const unsigned char* b2, size_t n) noexcept +int timingsafe_bcmp_internal(const unsigned char* b1, const unsigned char* b2, size_t n) noexcept { const unsigned char *p1 = b1, *p2 = b2; int ret = 0; @@ -42,8 +35,6 @@ int timingsafe_bcmp(const unsigned char* b1, const unsigned char* b2, size_t n) return (ret != 0); } -#endif - /** Compute poly1305 tag. chacha20 must be set to the right nonce, block 0. Will be at block 1 after. */ void ComputeTag(ChaCha20& chacha20, Span aad, Span cipher, Span tag) noexcept { @@ -97,7 +88,7 @@ bool AEADChaCha20Poly1305::Decrypt(Span cipher, Span Date: Tue, 26 Mar 2024 17:15:47 +0000 Subject: [PATCH 024/530] ci: use LLVM 18.1.3 in MSAN jobs --- ci/test/01_base_install.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh index 6f1498963a..25962a53e5 100755 --- a/ci/test/01_base_install.sh +++ b/ci/test/01_base_install.sh @@ -36,7 +36,7 @@ if [ -n "$PIP_PACKAGES" ]; then fi if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then - ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b "llvmorg-18.1.1" /msan/llvm-project + ${CI_RETRY_EXE} git clone --depth=1 https://github.com/llvm/llvm-project -b "llvmorg-18.1.3" /msan/llvm-project cmake -G Ninja -B /msan/clang_build/ \ -DLLVM_ENABLE_PROJECTS="clang" \ From cf3f388cfff0f6983da4d4b79a0de1e14dbc48d0 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 26 Mar 2024 16:28:43 +0000 Subject: [PATCH 025/530] ci: remove --with-asm usage (secp256k1) --- ci/test/00_setup_env_native_fuzz_with_msan.sh | 2 +- ci/test/00_setup_env_native_msan.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh index 02a6a6a5f2..2ed54b9e7e 100755 --- a/ci/test/00_setup_env_native_fuzz_with_msan.sh +++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh @@ -17,7 +17,7 @@ export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" -export BGL_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening --with-asm=no CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'" +export BGL_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'" export USE_MEMORY_SANITIZER="true" export RUN_UNIT_TESTS="false" export RUN_FUNCTIONAL_TESTS="false" diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh index 3f9c55c3da..81bab3d15a 100644 --- a/ci/test/00_setup_env_native_msan.sh +++ b/ci/test/00_setup_env_native_msan.sh @@ -17,7 +17,7 @@ export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" -export BGL_CONFIG="--with-sanitizers=memory --disable-hardening --with-asm=no" +export BGL_CONFIG="--with-sanitizers=memory --disable-hardening" export USE_MEMORY_SANITIZER="true" export RUN_FUNCTIONAL_TESTS="false" export CCACHE_MAXSIZE=250M From a45582af262a1c43b8fd6f11465325ab466995bd Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sat, 6 Apr 2024 15:46:53 +0100 Subject: [PATCH 026/530] refactor, bench, fuzz: Drop unneeded `UCharCast` calls The `CKey::Set()` template function handles `std::byte` just fine. --- src/bench/bip324_ecdh.cpp | 2 +- src/bench/ellswift.cpp | 2 +- src/test/fuzz/util/descriptor.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bench/bip324_ecdh.cpp b/src/bench/bip324_ecdh.cpp index 659da0f08e..fb10c2957e 100644 --- a/src/bench/bip324_ecdh.cpp +++ b/src/bench/bip324_ecdh.cpp @@ -27,7 +27,7 @@ static void BIP324_ECDH(benchmark::Bench& bench) bench.batch(1).unit("ecdh").run([&] { CKey key; - key.Set(UCharCast(key_data.data()), UCharCast(key_data.data()) + 32, true); + key.Set(key_data.data(), key_data.data() + 32, true); EllSwiftPubKey our_ellswift(our_ellswift_data); EllSwiftPubKey their_ellswift(their_ellswift_data); diff --git a/src/bench/ellswift.cpp b/src/bench/ellswift.cpp index 82e8dec982..9441b4863e 100644 --- a/src/bench/ellswift.cpp +++ b/src/bench/ellswift.cpp @@ -17,7 +17,7 @@ static void EllSwiftCreate(benchmark::Bench& bench) bench.batch(1).unit("pubkey").run([&] { auto ret = key.EllSwiftCreate(MakeByteSpan(entropy)); /* Use the first 32 bytes of the ellswift encoded public key as next private key. */ - key.Set(UCharCast(ret.data()), UCharCast(ret.data()) + 32, true); + key.Set(ret.data(), ret.data() + 32, true); assert(key.IsValid()); /* Use the last 32 bytes of the ellswift encoded public key as next entropy. */ std::copy(ret.begin() + 32, ret.begin() + 64, MakeWritableByteSpan(entropy).begin()); diff --git a/src/test/fuzz/util/descriptor.cpp b/src/test/fuzz/util/descriptor.cpp index df78bdf314..0fed2bc5e1 100644 --- a/src/test/fuzz/util/descriptor.cpp +++ b/src/test/fuzz/util/descriptor.cpp @@ -15,7 +15,7 @@ void MockedDescriptorConverter::Init() { // an extended one. if (IdIsCompPubKey(i) || IdIsUnCompPubKey(i) || IdIsXOnlyPubKey(i) || IdIsConstPrivKey(i)) { CKey privkey; - privkey.Set(UCharCast(key_data.begin()), UCharCast(key_data.end()), !IdIsUnCompPubKey(i)); + privkey.Set(key_data.begin(), key_data.end(), !IdIsUnCompPubKey(i)); if (IdIsCompPubKey(i) || IdIsUnCompPubKey(i)) { CPubKey pubkey{privkey.GetPubKey()}; keys_str[i] = HexStr(pubkey); From de0788a9529add051fe7b5eb0fc939f397d24853 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sat, 6 Apr 2024 16:03:39 +0100 Subject: [PATCH 027/530] fuzz, refactor: Deduplicate fuzz binary path creation --- test/fuzz/test_runner.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index b3edb0e253..cb5e1a710e 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -104,9 +104,11 @@ def main(): logging.error("Must have fuzz executable built") sys.exit(1) + fuzz_bin=os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz') + # Build list of tests test_list_all = parse_test_list( - fuzz_bin=os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz'), + fuzz_bin=fuzz_bin, source_dir=config['environment']['SRCDIR'], ) @@ -151,7 +153,7 @@ def main(): try: help_output = subprocess.run( args=[ - os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz'), + fuzz_bin, '-help=1', ], env=get_fuzz_env(target=test_list_selection[0], source_dir=config['environment']['SRCDIR']), @@ -173,7 +175,7 @@ def main(): return generate_corpus( fuzz_pool=fuzz_pool, src_dir=config['environment']['SRCDIR'], - build_dir=config["environment"]["BUILDDIR"], + fuzz_bin=fuzz_bin, corpus_dir=args.corpus_dir, targets=test_list_selection, ) @@ -184,7 +186,7 @@ def main(): corpus=args.corpus_dir, test_list=test_list_selection, src_dir=config['environment']['SRCDIR'], - build_dir=config["environment"]["BUILDDIR"], + fuzz_bin=fuzz_bin, merge_dirs=[Path(m_dir) for m_dir in args.m_dir], ) return @@ -194,7 +196,7 @@ def main(): corpus=args.corpus_dir, test_list=test_list_selection, src_dir=config['environment']['SRCDIR'], - build_dir=config["environment"]["BUILDDIR"], + fuzz_bin=fuzz_bin, using_libfuzzer=using_libfuzzer, use_valgrind=args.valgrind, empty_min_time=args.empty_min_time, @@ -237,7 +239,7 @@ def transform_rpc_target(targets, src_dir): return targets -def generate_corpus(*, fuzz_pool, src_dir, build_dir, corpus_dir, targets): +def generate_corpus(*, fuzz_pool, src_dir, fuzz_bin, corpus_dir, targets): """Generates new corpus. Run {targets} without input, and outputs the generated corpus to @@ -270,7 +272,7 @@ def job(command, t, t_env): os.makedirs(target_corpus_dir, exist_ok=True) use_value_profile = int(random.random() < .3) command = [ - os.path.join(build_dir, 'src', 'test', 'fuzz', 'fuzz'), + fuzz_bin, "-rss_limit_mb=8000", "-max_total_time=6000", "-reload=0", @@ -283,12 +285,12 @@ def job(command, t, t_env): future.result() -def merge_inputs(*, fuzz_pool, corpus, test_list, src_dir, build_dir, merge_dirs): +def merge_inputs(*, fuzz_pool, corpus, test_list, src_dir, fuzz_bin, merge_dirs): logging.info(f"Merge the inputs from the passed dir into the corpus_dir. Passed dirs {merge_dirs}") jobs = [] for t in test_list: args = [ - os.path.join(build_dir, 'src', 'test', 'fuzz', 'fuzz'), + fuzz_bin, '-rss_limit_mb=8000', '-set_cover_merge=1', # set_cover_merge is used instead of -merge=1 to reduce the overall @@ -325,13 +327,13 @@ def job(t, args): future.result() -def run_once(*, fuzz_pool, corpus, test_list, src_dir, build_dir, using_libfuzzer, use_valgrind, empty_min_time): +def run_once(*, fuzz_pool, corpus, test_list, src_dir, fuzz_bin, using_libfuzzer, use_valgrind, empty_min_time): jobs = [] for t in test_list: corpus_path = corpus / t os.makedirs(corpus_path, exist_ok=True) args = [ - os.path.join(build_dir, 'src', 'test', 'fuzz', 'fuzz'), + fuzz_bin, ] empty_dir = not any(corpus_path.iterdir()) if using_libfuzzer: From a6b0ccb2300945f3e4f001587c2c635f92b511ff Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sat, 6 Apr 2024 16:06:52 +0100 Subject: [PATCH 028/530] fuzz: Introduce `BITCOINFUZZ` environment variable The `BITCOINFUZZ` environment variable allows to override the default path to the fuzz binary. It complements the already existing set of variables used by tests: - BITCOIND - BITCOINCLI - BITCOINUTIL - BITCOINWALLET --- test/fuzz/test_runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index cb5e1a710e..558d63e85c 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -104,7 +104,7 @@ def main(): logging.error("Must have fuzz executable built") sys.exit(1) - fuzz_bin=os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz') + fuzz_bin=os.getenv("BITCOINFUZZ", default=os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz')) # Build list of tests test_list_all = parse_test_list( From a639417e7d3bbd861f9d9de8a290425d62afc23f Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 6 Apr 2024 18:17:26 +0200 Subject: [PATCH 029/530] test: remove immediate tx relay workaround in wallet_groups.py Reverts commit ab4efad51b9ba276ffeb6871931e13772493f7cc (PR #26970). This workaround is not needed anymore, as since #27114 the test sets the noban permission for both in- and outbound connections via the `noban_tx_relay` setting, and we don't have to rely on these topology hacks anymore. See commit c985eb854cc86deb747caea5283c17cf51b6a983. --- test/functional/wallet_groups.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/functional/wallet_groups.py b/test/functional/wallet_groups.py index 7e3e7e426b..d9b0e2c34d 100755 --- a/test/functional/wallet_groups.py +++ b/test/functional/wallet_groups.py @@ -42,11 +42,6 @@ def skip_test_if_missing_module(self): def run_test(self): self.log.info("Setting up") - # To take full use of immediate tx relay, all nodes need to be reachable - # via inbound peers, i.e. connect first to last to close the circle - # (the default test network topology looks like this: - # node0 <-- node1 <-- node2 <-- node3 <-- node4 <-- node5) - self.connect_nodes(0, self.num_nodes - 1) # Mine some coins self.generate(self.nodes[0], COINBASE_MATURITY + 1) From 0124adbfc917088be9da80d577105dd1670a56f0 Mon Sep 17 00:00:00 2001 From: AngusP Date: Mon, 25 Mar 2024 20:13:35 +0100 Subject: [PATCH 030/530] refactor: Simplify `extra_txn` to be a vec of CTransactionRef instead of a vec of pair All `CTransactionRef` have `.GetWitnessHash()` that returns a cached `const Wtxid` (since fac1223a568fa1ad6dd602350598eed278d115e8), so we don't need to pass transaction refs around with their IDs as they're easy to get from a ref. --- src/blockencodings.cpp | 11 +++++++---- src/blockencodings.h | 2 +- src/net_processing.cpp | 4 ++-- src/test/blockencodings_tests.cpp | 2 +- src/test/fuzz/partially_downloaded_block.cpp | 4 ++-- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 1e940e8f03..16a56b1852 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -47,7 +47,7 @@ uint64_t CBlockHeaderAndShortTxIDs::GetShortID(const uint256& txhash) const { -ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, const std::vector>& extra_txn) { +ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, const std::vector& extra_txn) { if (cmpctblock.header.IsNull() || (cmpctblock.shorttxids.empty() && cmpctblock.prefilledtxn.empty())) return READ_STATUS_INVALID; if (cmpctblock.shorttxids.size() + cmpctblock.prefilledtxn.size() > MAX_BLOCK_WEIGHT / MIN_SERIALIZABLE_TRANSACTION_WEIGHT) @@ -134,11 +134,14 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c } for (size_t i = 0; i < extra_txn.size(); i++) { - uint64_t shortid = cmpctblock.GetShortID(extra_txn[i].first); + if (extra_txn[i] == nullptr) { + continue; + } + uint64_t shortid = cmpctblock.GetShortID(extra_txn[i]->GetWitnessHash()); std::unordered_map::iterator idit = shorttxids.find(shortid); if (idit != shorttxids.end()) { if (!have_txn[idit->second]) { - txn_available[idit->second] = extra_txn[i].second; + txn_available[idit->second] = extra_txn[i]; have_txn[idit->second] = true; mempool_count++; extra_count++; @@ -150,7 +153,7 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c // Note that we don't want duplication between extra_txn and mempool to // trigger this case, so we compare witness hashes first if (txn_available[idit->second] && - txn_available[idit->second]->GetWitnessHash() != extra_txn[i].second->GetWitnessHash()) { + txn_available[idit->second]->GetWitnessHash() != extra_txn[i]->GetWitnessHash()) { txn_available[idit->second].reset(); mempool_count--; extra_count--; diff --git a/src/blockencodings.h b/src/blockencodings.h index 68c71a9600..effedf5374 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -142,7 +142,7 @@ class PartiallyDownloadedBlock { explicit PartiallyDownloadedBlock(CTxMemPool* poolIn) : pool(poolIn) {} // extra_txn is a list of extra transactions to look at, in form - ReadStatus InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, const std::vector>& extra_txn); + ReadStatus InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, const std::vector& extra_txn); bool IsTxAvailable(size_t index) const; ReadStatus FillBlock(CBlock& block, const std::vector& vtx_missing); }; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6996af38cb..39ffff97d2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1006,7 +1006,7 @@ class PeerManagerImpl final : public PeerManager /** Orphan/conflicted/etc transactions that are kept for compact block reconstruction. * The last -blockreconstructionextratxn/DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN of * these are kept in a ring buffer */ - std::vector> vExtraTxnForCompact GUARDED_BY(g_msgproc_mutex); + std::vector vExtraTxnForCompact GUARDED_BY(g_msgproc_mutex); /** Offset into vExtraTxnForCompact to insert the next tx */ size_t vExtraTxnForCompactIt GUARDED_BY(g_msgproc_mutex) = 0; @@ -1802,7 +1802,7 @@ void PeerManagerImpl::AddToCompactExtraTransactions(const CTransactionRef& tx) return; if (!vExtraTxnForCompact.size()) vExtraTxnForCompact.resize(m_opts.max_extra_txs); - vExtraTxnForCompact[vExtraTxnForCompactIt] = std::make_pair(tx->GetWitnessHash(), tx); + vExtraTxnForCompact[vExtraTxnForCompactIt] = tx; vExtraTxnForCompactIt = (vExtraTxnForCompactIt + 1) % m_opts.max_extra_txs; } diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 763f0f897e..5ccdfe2f7a 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -14,7 +14,7 @@ #include -std::vector> extra_txn; +std::vector extra_txn; BOOST_FIXTURE_TEST_SUITE(blockencodings_tests, RegTestingSetup) diff --git a/src/test/fuzz/partially_downloaded_block.cpp b/src/test/fuzz/partially_downloaded_block.cpp index 4a4b46da60..ab75afe066 100644 --- a/src/test/fuzz/partially_downloaded_block.cpp +++ b/src/test/fuzz/partially_downloaded_block.cpp @@ -60,7 +60,7 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb) // The coinbase is always available available.insert(0); - std::vector> extra_txn; + std::vector extra_txn; for (size_t i = 1; i < block->vtx.size(); ++i) { auto tx{block->vtx[i]}; @@ -68,7 +68,7 @@ FUZZ_TARGET(partially_downloaded_block, .init = initialize_pdb) bool add_to_mempool{fuzzed_data_provider.ConsumeBool()}; if (add_to_extra_txn) { - extra_txn.emplace_back(tx->GetWitnessHash(), tx); + extra_txn.emplace_back(tx); available.insert(i); } From d490259361592c531e126209e3a6660d64f6a442 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 1 Apr 2024 18:13:18 +0200 Subject: [PATCH 031/530] depends: add the new LLVM debug macro `LIBCPP_HARDENING_MODE` is the new macro, the previous one was removed in LLVM 18. See https://libcxx.llvm.org/Hardening.html. --- depends/hosts/linux.mk | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/depends/hosts/linux.mk b/depends/hosts/linux.mk index 9cd28f0deb..f5ce2bb0b8 100644 --- a/depends/hosts/linux.mk +++ b/depends/hosts/linux.mk @@ -13,7 +13,11 @@ linux_release_CXXFLAGS=$(linux_release_CFLAGS) linux_debug_CFLAGS=-O1 -g linux_debug_CXXFLAGS=$(linux_debug_CFLAGS) -linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_ENABLE_DEBUG_MODE=1 +# https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode.html +linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC + +# https://libcxx.llvm.org/Hardening.html +linux_debug_CPPFLAGS+=-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG ifeq (86,$(findstring 86,$(build_arch))) i686_linux_CC=gcc -m32 From 84a98ce4e4f4c138bdedea504cc3ae7e5b8a7ca8 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Thu, 21 Mar 2024 11:35:11 +0000 Subject: [PATCH 032/530] [clang-tidy] Enable the misc-no-recursion check Co-authored-by: stickies-v Co-authored-by: Gloria Zhao --- doc/developer-notes.md | 2 ++ src/.clang-tidy | 1 + src/merkleblock.cpp | 3 +++ src/node/interfaces.cpp | 1 + src/qt/optionsmodel.cpp | 19 +++++++++++++++++++ src/rpc/util.cpp | 6 ++++++ src/rpc/util.h | 2 ++ src/script/descriptor.cpp | 9 +++++++++ src/test/miniscript_tests.cpp | 1 + src/test/validation_block_tests.cpp | 1 + src/univalue/include/univalue.h | 1 + src/univalue/lib/univalue_write.cpp | 3 +++ src/util/string.h | 1 + src/wallet/receive.cpp | 1 + src/wallet/rpc/addresses.cpp | 3 +++ src/wallet/rpc/backup.cpp | 1 + src/wallet/scriptpubkeyman.cpp | 1 + 17 files changed, 56 insertions(+) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 497a09e36e..0721f81f00 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -115,6 +115,8 @@ code. Use `reinterpret_cast` and `const_cast` as appropriate. - Prefer [`list initialization ({})`](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-list) where possible. For example `int x{0};` instead of `int x = 0;` or `int x(0);` + - Recursion is checked by clang-tidy and thus must be made explicit. Use + `NOLINTNEXTLINE(misc-no-recursion)` to suppress the check. For function calls a namespace should be specified explicitly, unless such functions have been declared within it. Otherwise, [argument-dependent lookup](https://en.cppreference.com/w/cpp/language/adl), also known as ADL, could be diff --git a/src/.clang-tidy b/src/.clang-tidy index e4b789dcaa..a00400f083 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -6,6 +6,7 @@ bugprone-string-constructor, bugprone-use-after-move, bugprone-lambda-function-name, misc-unused-using-decls, +misc-no-recursion, modernize-use-default-member-init, modernize-use-emplace, modernize-use-noexcept, diff --git a/src/merkleblock.cpp b/src/merkleblock.cpp index c75f5c5e60..669c6e3b70 100644 --- a/src/merkleblock.cpp +++ b/src/merkleblock.cpp @@ -54,6 +54,7 @@ CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter* filter, const std: txn = CPartialMerkleTree(vHashes, vMatch); } +// NOLINTNEXTLINE(misc-no-recursion) uint256 CPartialMerkleTree::CalcHash(int height, unsigned int pos, const std::vector &vTxid) { //we can never have zero txs in a merkle block, we always need the coinbase tx //if we do not have this assert, we can hit a memory access violation when indexing into vTxid @@ -74,6 +75,7 @@ uint256 CPartialMerkleTree::CalcHash(int height, unsigned int pos, const std::ve } } +// NOLINTNEXTLINE(misc-no-recursion) void CPartialMerkleTree::TraverseAndBuild(int height, unsigned int pos, const std::vector &vTxid, const std::vector &vMatch) { // determine whether this node is the parent of at least one matched txid bool fParentOfMatch = false; @@ -92,6 +94,7 @@ void CPartialMerkleTree::TraverseAndBuild(int height, unsigned int pos, const st } } +// NOLINTNEXTLINE(misc-no-recursion) uint256 CPartialMerkleTree::TraverseAndExtract(int height, unsigned int pos, unsigned int &nBitsUsed, unsigned int &nHashUsed, std::vector &vMatch, std::vector &vnIndex) { if (nBitsUsed >= vBits.size()) { // overflowed the bits array - failure diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index ef239ae6df..cbea6a036d 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -406,6 +406,7 @@ class NodeImpl : public Node NodeContext* m_context{nullptr}; }; +// NOLINTNEXTLINE(misc-no-recursion) bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock& lock, const CChain& active, const BlockManager& blockman) { if (!index) return false; diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index a4d99c3b07..59d14f242d 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -360,6 +360,7 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in return successful; } +// NOLINTNEXTLINE(misc-no-recursion) QVariant OptionsModel::getOption(OptionID option, const std::string& suffix) const { auto setting = [&]{ return node().getPersistentSetting(SettingName(option) + suffix); }; @@ -455,6 +456,24 @@ QVariant OptionsModel::getOption(OptionID option, const std::string& suffix) con } } +QFont OptionsModel::getFontForChoice(const FontChoice& fc) +{ + QFont f; + if (std::holds_alternative(fc)) { + f = GUIUtil::fixedPitchFont(fc != UseBestSystemFont); + f.setWeight(QFont::Bold); + } else { + f = std::get(fc); + } + return f; +} + +QFont OptionsModel::getFontForMoney() const +{ + return getFontForChoice(m_font_money); +} + +// NOLINTNEXTLINE(misc-no-recursion) bool OptionsModel::setOption(OptionID option, const QVariant& value, const std::string& suffix) { auto changed = [&] { return value.isValid() && value != getOption(option, suffix); }; diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 92a5175c9c..12a1e55e8b 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -416,6 +416,7 @@ struct Sections { /** * Recursive helper to translate an RPCArg into sections */ + // NOLINTNEXTLINE(misc-no-recursion) void Push(const RPCArg& arg, const size_t current_indent = 5, const OuterType outer_type = OuterType::NONE) { const auto indent = std::string(current_indent, ' '); @@ -965,6 +966,7 @@ std::string RPCArg::ToDescriptionString(bool is_named_arg) const return ret; } +// NOLINTNEXTLINE(misc-no-recursion) void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const int current_indent) const { // Indentation @@ -1098,6 +1100,7 @@ static std::optional ExpectedType(RPCResult::Type type) NONFATAL_UNREACHABLE(); } +// NOLINTNEXTLINE(misc-no-recursion) UniValue RPCResult::MatchesType(const UniValue& result) const { if (m_skip_type_check) { @@ -1176,6 +1179,7 @@ void RPCResult::CheckInnerDoc() const CHECK_NONFATAL(inner_needed != m_inner.empty()); } +// NOLINTNEXTLINE(misc-no-recursion) std::string RPCArg::ToStringObj(const bool oneline) const { std::string res; @@ -1214,6 +1218,7 @@ std::string RPCArg::ToStringObj(const bool oneline) const NONFATAL_UNREACHABLE(); } +// NOLINTNEXTLINE(misc-no-recursion) std::string RPCArg::ToString(const bool oneline) const { if (oneline && !m_opts.oneline_description.empty()) { @@ -1240,6 +1245,7 @@ std::string RPCArg::ToString(const bool oneline) const case Type::OBJ: case Type::OBJ_NAMED_PARAMS: case Type::OBJ_USER_KEYS: { + // NOLINTNEXTLINE(misc-no-recursion) const std::string res = Join(m_inner, ",", [&](const RPCArg& i) { return i.ToStringObj(oneline); }); if (m_type == Type::OBJ) { return "{" + res + "}"; diff --git a/src/rpc/util.h b/src/rpc/util.h index 02b4092028..a7c5d666a4 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -162,6 +162,7 @@ struct RPCArgOptions { //!< methods set the also_positional flag and read values from both positions. }; +// NOLINTNEXTLINE(misc-no-recursion) struct RPCArg { enum class Type { OBJ, @@ -271,6 +272,7 @@ struct RPCArg { std::string ToDescriptionString(bool is_named_arg) const; }; +// NOLINTNEXTLINE(misc-no-recursion) struct RPCResult { enum class Type { OBJ, diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index a0e755afac..a11d4dcbd5 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -599,6 +599,7 @@ class DescriptorImpl : public Descriptor COMPAT, // string calculation that mustn't change over time to stay compatible with previous software versions }; + // NOLINTNEXTLINE(misc-no-recursion) bool IsSolvable() const override { for (const auto& arg : m_subdescriptor_args) { @@ -607,6 +608,7 @@ class DescriptorImpl : public Descriptor return true; } + // NOLINTNEXTLINE(misc-no-recursion) bool IsRange() const final { for (const auto& pubkey : m_pubkey_args) { @@ -618,6 +620,7 @@ class DescriptorImpl : public Descriptor return false; } + // NOLINTNEXTLINE(misc-no-recursion) virtual bool ToStringSubScriptHelper(const SigningProvider* arg, std::string& ret, const StringType type, const DescriptorCache* cache = nullptr) const { size_t pos = 0; @@ -630,6 +633,7 @@ class DescriptorImpl : public Descriptor return true; } + // NOLINTNEXTLINE(misc-no-recursion) virtual bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type, const DescriptorCache* cache = nullptr) const { std::string extra = ToStringExtra(); @@ -682,6 +686,7 @@ class DescriptorImpl : public Descriptor return ret; } + // NOLINTNEXTLINE(misc-no-recursion) bool ExpandHelper(int pos, const SigningProvider& arg, const DescriptorCache* read_cache, std::vector& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache) const { std::vector> entries; @@ -723,6 +728,7 @@ class DescriptorImpl : public Descriptor return ExpandHelper(pos, DUMMY_SIGNING_PROVIDER, &read_cache, output_scripts, out, nullptr); } + // NOLINTNEXTLINE(misc-no-recursion) void ExpandPrivate(int pos, const SigningProvider& provider, FlatSigningProvider& out) const final { for (const auto& p : m_pubkey_args) { @@ -750,6 +756,7 @@ class DescriptorImpl : public Descriptor std::optional MaxSatisfactionElems() const override { return {}; } + // NOLINTNEXTLINE(misc-no-recursion) void GetPubKeys(std::set& pubkeys, std::set& ext_pubs) const override { for (const auto& p : m_pubkey_args) { @@ -1579,6 +1586,7 @@ struct KeyParser { }; /** Parse a script in a particular context. */ +// NOLINTNEXTLINE(misc-no-recursion) std::unique_ptr ParseScript(uint32_t& key_exp_index, Span& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error) { using namespace spanparsing; @@ -1886,6 +1894,7 @@ std::unique_ptr InferMultiA(const CScript& script, ParseScriptCo return std::make_unique(match->first, std::move(keys)); } +// NOLINTNEXTLINE(misc-no-recursion) std::unique_ptr InferScript(const CScript& script, ParseScriptContext ctx, const SigningProvider& provider) { if (ctx == ParseScriptContext::P2TR && script.size() == 34 && script[0] == 32 && script[33] == OP_CHECKSIG) { diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp index 1f28e61bc9..a3699f84b6 100644 --- a/src/test/miniscript_tests.cpp +++ b/src/test/miniscript_tests.cpp @@ -297,6 +297,7 @@ using miniscript::operator"" _mst; using Node = miniscript::Node; /** Compute all challenges (pubkeys, hashes, timelocks) that occur in a given Miniscript. */ +// NOLINTNEXTLINE(misc-no-recursion) std::set FindChallenges(const NodeRef& ref) { std::set chal; for (const auto& key : ref->keys) { diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 316ab86c2b..69f4e305ab 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -127,6 +127,7 @@ std::shared_ptr MinerTestingSetup::BadBlock(const uint256& prev_ha return ret; } +// NOLINTNEXTLINE(misc-no-recursion) void MinerTestingSetup::BuildChain(const uint256& root, int height, const unsigned int invalid_rate, const unsigned int branch_rate, const unsigned int max_size, std::vector>& blocks) { if (height <= 0 || blocks.size() >= max_size) return; diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index 88a1e60306..e2ff88899e 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -18,6 +18,7 @@ #include #include +// NOLINTNEXTLINE(misc-no-recursion) class UniValue { public: enum VType { VNULL, VOBJ, VARR, VSTR, VNUM, VBOOL, }; diff --git a/src/univalue/lib/univalue_write.cpp b/src/univalue/lib/univalue_write.cpp index 4a3cbba20f..4a2219061a 100644 --- a/src/univalue/lib/univalue_write.cpp +++ b/src/univalue/lib/univalue_write.cpp @@ -27,6 +27,7 @@ static std::string json_escape(const std::string& inS) return outS; } +// NOLINTNEXTLINE(misc-no-recursion) std::string UniValue::write(unsigned int prettyIndent, unsigned int indentLevel) const { @@ -66,6 +67,7 @@ static void indentStr(unsigned int prettyIndent, unsigned int indentLevel, std:: s.append(prettyIndent * indentLevel, ' '); } +// NOLINTNEXTLINE(misc-no-recursion) void UniValue::writeArray(unsigned int prettyIndent, unsigned int indentLevel, std::string& s) const { s += "["; @@ -88,6 +90,7 @@ void UniValue::writeArray(unsigned int prettyIndent, unsigned int indentLevel, s s += "]"; } +// NOLINTNEXTLINE(misc-no-recursion) void UniValue::writeObject(unsigned int prettyIndent, unsigned int indentLevel, std::string& s) const { s += "{"; diff --git a/src/util/string.h b/src/util/string.h index 1e7cf31157..9917afb15e 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -65,6 +65,7 @@ void ReplaceAll(std::string& in_out, const std::string& search, const std::strin * @param unary_op Apply this operator to each item */ template +// NOLINTNEXTLINE(misc-no-recursion) auto Join(const C& container, const S& separator, UnaryOp unary_op) { decltype(unary_op(*container.begin())) ret; diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index ea3ffff549..c164266f80 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -253,6 +253,7 @@ bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx, const isminef return (CachedTxGetDebit(wallet, wtx, filter) > 0); } +// NOLINTNEXTLINE(misc-no-recursion) bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set& trusted_parents) { AssertLockHeld(wallet.cs_wallet); diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index ca70ca0776..162f871141 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -395,6 +395,7 @@ class DescribeWalletAddressVisitor public: const SigningProvider * const provider; + // NOLINTNEXTLINE(misc-no-recursion) void ProcessSubScript(const CScript& subscript, UniValue& obj) const { // Always present: script type and redeemscript @@ -445,6 +446,7 @@ class DescribeWalletAddressVisitor return obj; } + // NOLINTNEXTLINE(misc-no-recursion) UniValue operator()(const ScriptHash& scripthash) const { UniValue obj(UniValue::VOBJ); @@ -465,6 +467,7 @@ class DescribeWalletAddressVisitor return obj; } + // NOLINTNEXTLINE(misc-no-recursion) UniValue operator()(const WitnessV0ScriptHash& id) const { UniValue obj(UniValue::VOBJ); diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 4b340ca79d..3f60edd209 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -854,6 +854,7 @@ enum class ScriptContext // Analyse the provided scriptPubKey, determining which keys and which redeem scripts from the ImportData struct are needed to spend it, and mark them as used. // Returns an error string, or the empty string for success. +// NOLINTNEXTLINE(misc-no-recursion) static std::string RecurseImportData(const CScript& script, ImportData& import_data, const ScriptContext script_ctx) { // Use Solver to obtain script type and parsed pubkeys or hashes: diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 59171f6db7..b42275fe4b 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -97,6 +97,7 @@ bool HaveKeys(const std::vector& pubkeys, const LegacyScriptPubKeyMan& //! @param recurse_scripthash whether to recurse into nested p2sh and p2wsh //! scripts or simply treat any script that has been //! stored in the keystore as spendable +// NOLINTNEXTLINE(misc-no-recursion) IsMineResult IsMineInner(const LegacyScriptPubKeyMan& keystore, const CScript& scriptPubKey, IsMineSigVersion sigversion, bool recurse_scripthash=true) { IsMineResult ret = IsMineResult::NO; From 880a9ed427cbcf27b88a1e70cbb609d1492b6376 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 4 Apr 2024 13:32:15 +0100 Subject: [PATCH 033/530] guix: remove gcc-toolchain static from Windows build The libs in this dir are the following: ```bash ls /gnu/store/2vnbkrdin4rrf7ygnr80mlcglin4qqa4-gcc-toolchain-12.3.0-static/lib/lib libanl.a libc.a libdl.a libm.a libBrokenLocale.a libcrypt.a libg.a libmcheck.a libpthread.a librt.a libresolv.a libutil.a ``` These do not need to be propogated into the Windows build environment. --- contrib/guix/libexec/build.sh | 9 +++++++++ contrib/guix/manifest.scm | 1 - 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index 30e4c9ef1a..1ce194bc69 100644 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -74,6 +74,15 @@ export CPLUS_INCLUDE_PATH="${NATIVE_GCC}/include/c++:${NATIVE_GCC}/include" export OBJC_INCLUDE_PATH="${NATIVE_GCC}/include" export OBJCPLUS_INCLUDE_PATH="${NATIVE_GCC}/include/c++:${NATIVE_GCC}/include" +case "$HOST" in + *darwin*) export LIBRARY_PATH="${NATIVE_GCC}/lib" ;; + *mingw*) export LIBRARY_PATH="${NATIVE_GCC}/lib" ;; + *) + NATIVE_GCC_STATIC="$(store_path gcc-toolchain static)" + export LIBRARY_PATH="${NATIVE_GCC}/lib:${NATIVE_GCC_STATIC}/lib" + ;; +esac + # Set environment variables to point the CROSS toolchain to the right # includes/libs for $HOST case "$HOST" in diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm index 4a5542d45a..a06ca39484 100644 --- a/contrib/guix/manifest.scm +++ b/contrib/guix/manifest.scm @@ -517,7 +517,6 @@ inspecting signatures in Mach-O binaries.") (cond ((string-suffix? "-mingw32" target) (list ;; Native GCC 12 toolchain gcc-toolchain-12 - (list gcc-toolchain-12 "static") zip (make-mingw-pthreads-cross-toolchain "x86_64-w64-mingw32") nsis-x86_64 From 257921e3f08ba45df772781adc1658cd5e4e22e3 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 8 Apr 2024 05:12:28 -0400 Subject: [PATCH 034/530] doc: 25.2 historical release notes --- doc/release-notes/release-notes-25.2.md | 74 +++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 doc/release-notes/release-notes-25.2.md diff --git a/doc/release-notes/release-notes-25.2.md b/doc/release-notes/release-notes-25.2.md new file mode 100644 index 0000000000..3f050ebaef --- /dev/null +++ b/doc/release-notes/release-notes-25.2.md @@ -0,0 +1,74 @@ +25.2 Release Notes +================== + +Bitcoin Core version 25.2 is now available from: + + + +This release includes various bug fixes and performance +improvements, as well as updated translations. + +Please report bugs using the issue tracker at GitHub: + + + +To receive security and update notifications, please subscribe to: + + + +How to Upgrade +============== + +If you are running an older version, shut it down. Wait until it has completely +shut down (which might take a few minutes in some cases), then run the +installer (on Windows) or just copy over `/Applications/Bitcoin-Qt` (on macOS) +or `bitcoind`/`bitcoin-qt` (on Linux). + +Upgrading directly from a version of Bitcoin Core that has reached its EOL is +possible, but it might take some time if the data directory needs to be migrated. Old +wallet versions of Bitcoin Core are generally supported. + +Compatibility +============== + +Bitcoin Core is supported and extensively tested on operating systems +using the Linux kernel, macOS 10.15+, and Windows 7 and newer. Bitcoin +Core should also work on most other Unix-like systems but is not as +frequently tested on them. It is not recommended to use Bitcoin Core on +unsupported systems. + +Notable changes +=============== + +### Gui + +- gui#774 Fix crash on selecting "Mask values" in transaction view + +### RPC + +- #29003 rpc: fix getrawtransaction segfault + +### Wallet + +- #29176 wallet: Fix use-after-free in WalletBatch::EraseRecords +- #29510 wallet: `getrawchangeaddress` and `getnewaddress` failures should not affect keypools for descriptor wallets + +### P2P and network changes + +- #29412 p2p: Don't process mutated blocks +- #29524 p2p: Don't consider blocks mutated if they don't connect to known prev block + +Credits +======= + +Thanks to everyone who directly contributed to this release: + +- Martin Zumsande +- Sebastian Falbesoner +- MarcoFalke +- UdjinM6 +- dergoegge +- Greg Sanders + +As well as to everyone that helped with translations on +[Transifex](https://www.transifex.com/bitcoin/bitcoin/). From 298f05f4253c2f557573335c127bd84ea11c9ea1 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Mon, 8 Apr 2024 15:05:07 +0200 Subject: [PATCH 035/530] fill_mempool: remove subtest-specific comment --- test/functional/mempool_limit.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index eb64542fbd..0f3c316fd3 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -47,8 +47,6 @@ def fill_mempool(self): # Generate UTXOs to flood the mempool # 1 to create a tx initially that will be evicted from the mempool later # 75 transactions each with a fee rate higher than the previous one - # And 1 more to verify that this tx does not get added to the mempool with a fee rate less than the mempoolminfee - # And 2 more for the package cpfp test self.generate(miniwallet, 1 + (num_of_batches * tx_batch_size)) # Mine 99 blocks so that the UTXOs are allowed to be spent From a9143e8b6cc34bc8026ced9294ab38cd6d1faf03 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 9 Apr 2024 09:17:34 +0200 Subject: [PATCH 036/530] Revert "ci: Temporarily disable bpfcc-tools" This reverts commit fac012c7262f036e9b6f5800e57dcd63870a871c. --- ci/test/00_setup_env_native_asan.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 2db1776a80..313fd44f9e 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -9,7 +9,7 @@ export LC_ALL=C.UTF-8 export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" # Only install BCC tracing packages in Cirrus CI. if [[ "${CIRRUS_CI}" == "true" ]]; then - BPFCC_PACKAGE="" # Temporarily disabled "bpfcc-tools linux-headers-$(uname --kernel-release)" + BPFCC_PACKAGE="bpfcc-tools linux-headers-$(uname --kernel-release)" export CI_CONTAINER_CAP="--privileged -v /sys/kernel:/sys/kernel:rw" else BPFCC_PACKAGE="" From d54c064b730221379c5df1f527b60fbc5ba8fa71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Tue, 9 Apr 2024 11:21:57 +0200 Subject: [PATCH 037/530] Change MAC_OSX macro to __APPLE__ in crypto package --- src/crypto/sha256.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/crypto/sha256.cpp b/src/crypto/sha256.cpp index 00d95ce59e..30da3a0480 100644 --- a/src/crypto/sha256.cpp +++ b/src/crypto/sha256.cpp @@ -20,7 +20,7 @@ #include #endif -#if defined(MAC_OSX) && defined(ENABLE_ARM_SHANI) +#if defined(__APPLE__) && defined(ENABLE_ARM_SHANI) #include #include #endif @@ -674,7 +674,7 @@ std::string SHA256AutoDetect(sha256_implementation::UseImplementation use_implem #endif #endif -#if defined(MAC_OSX) +#if defined(__APPLE__) int val = 0; size_t len = sizeof(val); if (sysctlbyname("hw.optional.arm.FEAT_SHA256", &val, &len, nullptr, 0) == 0) { From 1a68668cbb451aa3a328866564ae3c52c4cf71d6 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Mon, 8 Apr 2024 15:07:38 +0200 Subject: [PATCH 038/530] Move fill_mempool to util function --- test/functional/mempool_limit.py | 52 ++------------------------ test/functional/test_framework/util.py | 39 +++++++++++++++++++ 2 files changed, 43 insertions(+), 48 deletions(-) diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index 0f3c316fd3..fb6aacbb4a 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -6,7 +6,6 @@ from decimal import Decimal -from test_framework.blocktools import COINBASE_MATURITY from test_framework.p2p import P2PTxInvStore from test_framework.test_framework import BGLTestFramework from test_framework.util import ( @@ -14,8 +13,7 @@ assert_fee_amount, assert_greater_than, assert_raises_rpc_error, - create_lots_of_big_transactions, - gen_return_txouts, + fill_mempool, ) from test_framework.wallet import ( COIN, @@ -34,48 +32,6 @@ def set_test_params(self): ]] self.supports_cli = False - def fill_mempool(self): - """Fill mempool until eviction.""" - self.log.info("Fill the mempool until eviction is triggered and the mempoolminfee rises") - txouts = gen_return_txouts() - node = self.nodes[0] - miniwallet = self.wallet - relayfee = node.getnetworkinfo()['relayfee'] - - tx_batch_size = 1 - num_of_batches = 75 - # Generate UTXOs to flood the mempool - # 1 to create a tx initially that will be evicted from the mempool later - # 75 transactions each with a fee rate higher than the previous one - self.generate(miniwallet, 1 + (num_of_batches * tx_batch_size)) - - # Mine 99 blocks so that the UTXOs are allowed to be spent - self.generate(node, COINBASE_MATURITY - 1) - - self.log.debug("Create a mempool tx that will be evicted") - tx_to_be_evicted_id = miniwallet.send_self_transfer(from_node=node, fee_rate=relayfee)["txid"] - - # Increase the tx fee rate to give the subsequent transactions a higher priority in the mempool - # The tx has an approx. vsize of 65k, i.e. multiplying the previous fee rate (in sats/kvB) - # by 130 should result in a fee that corresponds to 2x of that fee rate - base_fee = relayfee * 130 - - self.log.debug("Fill up the mempool with txs with higher fee rate") - with node.assert_debug_log(["rolling minimum fee bumped"]): - for batch_of_txid in range(num_of_batches): - fee = (batch_of_txid + 1) * base_fee - create_lots_of_big_transactions(miniwallet, node, fee, tx_batch_size, txouts) - - self.log.debug("The tx should be evicted by now") - # The number of transactions created should be greater than the ones present in the mempool - assert_greater_than(tx_batch_size * num_of_batches, len(node.getrawmempool())) - # Initial tx created should not be present in the mempool anymore as it had a lower fee rate - assert tx_to_be_evicted_id not in node.getrawmempool() - - self.log.debug("Check that mempoolminfee is larger than minrelaytxfee") - assert_equal(node.getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) - assert_greater_than(node.getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) - def test_rbf_carveout_disallowed(self): node = self.nodes[0] @@ -137,7 +93,7 @@ def test_mid_package_eviction(self): assert_equal(node.getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) assert_equal(node.getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) - self.fill_mempool() + fill_mempool(self, node, self.wallet) current_info = node.getmempoolinfo() mempoolmin_feerate = current_info["mempoolminfee"] @@ -227,7 +183,7 @@ def test_mid_package_replacement(self): assert_equal(node.getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) assert_equal(node.getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) - self.fill_mempool() + fill_mempool(self, node, self.wallet) current_info = node.getmempoolinfo() mempoolmin_feerate = current_info["mempoolminfee"] @@ -301,7 +257,7 @@ def run_test(self): assert_equal(node.getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) assert_equal(node.getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) - self.fill_mempool() + fill_mempool(self, node, self.wallet) # Deliberately try to create a tx with a fee less than the minimum mempool fee to assert that it does not get added to the mempool self.log.info('Create a mempool tx that will not pass mempoolminfee') diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 08d898b586..dbb85e3178 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -497,6 +497,45 @@ def check_node_connections(*, node, num_in, num_out): assert_equal(info["connections_in"], num_in) assert_equal(info["connections_out"], num_out) +def fill_mempool(test_framework, node, miniwallet): + """Fill mempool until eviction.""" + test_framework.log.info("Fill the mempool until eviction is triggered and the mempoolminfee rises") + txouts = gen_return_txouts() + relayfee = node.getnetworkinfo()['relayfee'] + + tx_batch_size = 1 + num_of_batches = 75 + # Generate UTXOs to flood the mempool + # 1 to create a tx initially that will be evicted from the mempool later + # 75 transactions each with a fee rate higher than the previous one + test_framework.generate(miniwallet, 1 + (num_of_batches * tx_batch_size)) + + # Mine COINBASE_MATURITY - 1 blocks so that the UTXOs are allowed to be spent + test_framework.generate(node, 100 - 1) + + test_framework.log.debug("Create a mempool tx that will be evicted") + tx_to_be_evicted_id = miniwallet.send_self_transfer(from_node=node, fee_rate=relayfee)["txid"] + + # Increase the tx fee rate to give the subsequent transactions a higher priority in the mempool + # The tx has an approx. vsize of 65k, i.e. multiplying the previous fee rate (in sats/kvB) + # by 130 should result in a fee that corresponds to 2x of that fee rate + base_fee = relayfee * 130 + + test_framework.log.debug("Fill up the mempool with txs with higher fee rate") + with node.assert_debug_log(["rolling minimum fee bumped"]): + for batch_of_txid in range(num_of_batches): + fee = (batch_of_txid + 1) * base_fee + create_lots_of_big_transactions(miniwallet, node, fee, tx_batch_size, txouts) + + test_framework.log.debug("The tx should be evicted by now") + # The number of transactions created should be greater than the ones present in the mempool + assert_greater_than(tx_batch_size * num_of_batches, len(node.getrawmempool())) + # Initial tx created should not be present in the mempool anymore as it had a lower fee rate + assert tx_to_be_evicted_id not in node.getrawmempool() + + test_framework.log.debug("Check that mempoolminfee is larger than minrelaytxfee") + assert_equal(node.getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) + assert_greater_than(node.getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) # Transaction/Block functions ############################# From 341476b146ccfdf93ba411bfbf60c06d82472e36 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Wed, 27 Mar 2024 09:30:05 -0400 Subject: [PATCH 039/530] fuzz: Add coverage for client_maxfeerate --- src/test/fuzz/package_eval.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index c801c2e325..c201118bce 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -276,8 +276,14 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool) // (the package is a test accept and ATMP is a submission). auto single_submit = txs.size() == 1 && fuzzed_data_provider.ConsumeBool(); + // Exercise client_maxfeerate logic + std::optional client_maxfeerate{}; + if (fuzzed_data_provider.ConsumeBool()) { + client_maxfeerate = CFeeRate(fuzzed_data_provider.ConsumeIntegralInRange(-1, 50 * COIN), 100); + } + const auto result_package = WITH_LOCK(::cs_main, - return ProcessNewPackage(chainstate, tx_pool, txs, /*test_accept=*/single_submit, /*client_maxfeerate=*/{})); + return ProcessNewPackage(chainstate, tx_pool, txs, /*test_accept=*/single_submit, client_maxfeerate)); // Always set bypass_limits to false because it is not supported in ProcessNewPackage and // can be a source of divergence. From 759d59d6c76354f0ac9e6c552e1dcb901e7b3436 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Tue, 26 Mar 2024 10:35:44 -0400 Subject: [PATCH 040/530] AcceptMultipleTransactions: Fix workspace client_maxfeerate If we do not set the Failure for the workspace when there is a client_maxfeerate related error, we hit an Assume() to the contrary. Properly set it. --- src/validation.cpp | 4 +- test/functional/rpc_packages.py | 67 +++++++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index bd006db969..03b25cd36d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1366,7 +1366,9 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: // Individual modified feerate exceeded caller-defined max; abort // N.B. this doesn't take into account CPFPs. Chunk-aware validation may be more robust. if (args.m_client_maxfeerate && CFeeRate(ws.m_modified_fees, ws.m_vsize) > args.m_client_maxfeerate.value()) { - package_state.Invalid(PackageValidationResult::PCKG_TX, "max feerate exceeded"); + // Need to set failure here both individually and at package level + ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "max feerate exceeded", ""); + package_state.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); // Exit early to avoid doing pointless work. Update the failed tx result; the rest are unfinished. results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); return PackageMempoolAcceptResult(package_state, std::move(results)); diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index acd5e83a8a..1d2c06f418 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -18,6 +18,7 @@ assert_equal, assert_fee_amount, assert_raises_rpc_error, + fill_mempool, ) from test_framework.wallet import ( DEFAULT_FEE, @@ -81,7 +82,8 @@ def run_test(self): self.test_conflicting() self.test_rbf() self.test_submitpackage() - self.test_maxfeerate_maxburn_submitpackage() + self.test_maxfeerate_submitpackage() + self.test_maxburn_submitpackage() def test_independent(self, coin): self.log.info("Test multiple independent transactions in a package") @@ -357,7 +359,7 @@ def test_submitpackage(self): assert_equal(res["tx-results"][sec_wtxid]["error"], "version") peer.wait_for_broadcast([first_wtxid]) - def test_maxfeerate_maxburn_submitpackage(self): + def test_maxfeerate_submitpackage(self): node = self.nodes[0] # clear mempool deterministic_address = node.get_deterministic_priv_key().address @@ -368,23 +370,72 @@ def test_maxfeerate_maxburn_submitpackage(self): minrate_btc_kvb = min([chained_txn["fee"] / chained_txn["tx"].get_vsize() * 1000 for chained_txn in chained_txns]) chain_hex = [t["hex"] for t in chained_txns] pkg_result = node.submitpackage(chain_hex, maxfeerate=minrate_btc_kvb - Decimal("0.00000001")) + + # First tx failed in single transaction evaluation, so package message is generic + assert_equal(pkg_result["package_msg"], "transaction failed") assert_equal(pkg_result["tx-results"][chained_txns[0]["wtxid"]]["error"], "max feerate exceeded") assert_equal(pkg_result["tx-results"][chained_txns[1]["wtxid"]]["error"], "bad-txns-inputs-missingorspent") assert_equal(node.getrawmempool(), []) + # Make chain of two transactions where parent doesn't make minfee threshold + # but child is too high fee + # Lower mempool limit to make it easier to fill_mempool + self.restart_node(0, extra_args=[ + "-datacarriersize=100000", + "-maxmempool=5", + "-persistmempool=0", + ]) + + fill_mempool(self, node, self.wallet) + + minrelay = node.getmempoolinfo()["minrelaytxfee"] + parent = self.wallet.create_self_transfer( + fee_rate=minrelay, + ) + + child = self.wallet.create_self_transfer( + fee_rate=DEFAULT_FEE, + utxo_to_spend=parent["new_utxo"], + ) + + pkg_result = node.submitpackage([parent["hex"], child["hex"]], maxfeerate=DEFAULT_FEE - Decimal("0.00000001")) + + # Child is connected even though parent is invalid and still reports fee exceeded + # this implies sub-package evaluation of both entries together. + assert_equal(pkg_result["package_msg"], "transaction failed") + assert "mempool min fee not met" in pkg_result["tx-results"][parent["wtxid"]]["error"] + assert_equal(pkg_result["tx-results"][child["wtxid"]]["error"], "max feerate exceeded") + assert parent["txid"] not in node.getrawmempool() + assert child["txid"] not in node.getrawmempool() + + # Reset maxmempool, datacarriersize, reset dynamic mempool minimum feerate, and empty mempool. + self.restart_node(0) + + assert_equal(node.getrawmempool(), []) + + def test_maxburn_submitpackage(self): + node = self.nodes[0] + + assert_equal(node.getrawmempool(), []) + self.log.info("Submitpackage maxburnamount arg testing") - tx = tx_from_hex(chain_hex[1]) + chained_txns_burn = self.wallet.create_self_transfer_chain(chain_length=2) + chained_burn_hex = [t["hex"] for t in chained_txns_burn] + + tx = tx_from_hex(chained_burn_hex[1]) tx.vout[-1].scriptPubKey = b'a' * 10001 # scriptPubKey bigger than 10k IsUnspendable - chain_hex = [chain_hex[0], tx.serialize().hex()] + chained_burn_hex = [chained_burn_hex[0], tx.serialize().hex()] # burn test is run before any package evaluation; nothing makes it in and we get broader exception - assert_raises_rpc_error(-25, "Unspendable output exceeds maximum configured by user", node.submitpackage, chain_hex, 0, chained_txns[1]["new_utxo"]["value"] - Decimal("0.00000001")) + assert_raises_rpc_error(-25, "Unspendable output exceeds maximum configured by user", node.submitpackage, chained_burn_hex, 0, chained_txns_burn[1]["new_utxo"]["value"] - Decimal("0.00000001")) assert_equal(node.getrawmempool(), []) + minrate_btc_kvb_burn = min([chained_txn_burn["fee"] / chained_txn_burn["tx"].get_vsize() * 1000 for chained_txn_burn in chained_txns_burn]) + # Relax the restrictions for both and send it; parent gets through as own subpackage - pkg_result = node.submitpackage(chain_hex, maxfeerate=minrate_btc_kvb, maxburnamount=chained_txns[1]["new_utxo"]["value"]) - assert "error" not in pkg_result["tx-results"][chained_txns[0]["wtxid"]] + pkg_result = node.submitpackage(chained_burn_hex, maxfeerate=minrate_btc_kvb_burn, maxburnamount=chained_txns_burn[1]["new_utxo"]["value"]) + assert "error" not in pkg_result["tx-results"][chained_txns_burn[0]["wtxid"]] assert_equal(pkg_result["tx-results"][tx.getwtxid()]["error"], "scriptpubkey") - assert_equal(node.getrawmempool(), [chained_txns[0]["txid"]]) + assert_equal(node.getrawmempool(), [chained_txns_burn[0]["txid"]]) if __name__ == "__main__": RPCPackagesTest().main() \ No newline at end of file From 7ed3b6dd8f01613929882500644efccc1caa2bc8 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Mon, 8 Apr 2024 15:10:16 +0200 Subject: [PATCH 041/530] fill_mempool: assertions and docsctring update --- test/functional/test_framework/util.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index dbb85e3178..dab96758bf 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -498,11 +498,21 @@ def check_node_connections(*, node, num_in, num_out): assert_equal(info["connections_out"], num_out) def fill_mempool(test_framework, node, miniwallet): - """Fill mempool until eviction.""" + """Fill mempool until eviction. + + Allows for simpler testing of scenarios with floating mempoolminfee > minrelay + Requires -datacarriersize=100000 and + -maxmempool=5. + It will not ensure mempools become synced as it + is based on a single node and assumes -minrelaytxfee + is 1 sat/vbyte. + """ test_framework.log.info("Fill the mempool until eviction is triggered and the mempoolminfee rises") txouts = gen_return_txouts() relayfee = node.getnetworkinfo()['relayfee'] + assert_equal(relayfee, Decimal('0.00001000')) + tx_batch_size = 1 num_of_batches = 75 # Generate UTXOs to flood the mempool From aef8e3032b522ef46e3b55cd364b5d54e172e612 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 9 Apr 2024 15:58:05 +0200 Subject: [PATCH 042/530] ci: disable _FORTIFY_SOURCE with MSAN --- ci/test/00_setup_env_native_fuzz_with_msan.sh | 3 ++- ci/test/00_setup_env_native_msan.sh | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh index 2ed54b9e7e..070e49c909 100755 --- a/ci/test/00_setup_env_native_fuzz_with_msan.sh +++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh @@ -17,7 +17,8 @@ export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" -export BGL_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'" +# _FORTIFY_SOURCE is not compatible with MSAN. +export BGL_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE -U_FORTIFY_SOURCE'" export USE_MEMORY_SANITIZER="true" export RUN_UNIT_TESTS="false" export RUN_FUNCTIONAL_TESTS="false" diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh index 81bab3d15a..600f16a5a6 100644 --- a/ci/test/00_setup_env_native_msan.sh +++ b/ci/test/00_setup_env_native_msan.sh @@ -17,7 +17,8 @@ export PACKAGES="ninja-build" # BDB generates false-positives and will be removed in future export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'" export GOAL="install" -export BGL_CONFIG="--with-sanitizers=memory --disable-hardening" +# _FORTIFY_SOURCE is not compatible with MSAN. +export BGL_CONFIG="--with-sanitizers=memory CPPFLAGS='-U_FORTIFY_SOURCE'" export USE_MEMORY_SANITIZER="true" export RUN_FUNCTIONAL_TESTS="false" export CCACHE_MAXSIZE=250M From 3bc93913dd3b291a2e1a16cc6a16038e2675e2ec Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 5 Mar 2024 18:46:15 +0100 Subject: [PATCH 043/530] depends: remove no longer needed patch for Boost::Process As Boost::Process has been replaced by cpp-subprocess (PR #28981), this patch touches an unused code part and is hence not needed anymore. --- depends/packages/boost.mk | 5 ---- depends/patches/boost/process_macos_sdk.patch | 27 ------------------- 2 files changed, 32 deletions(-) delete mode 100644 depends/patches/boost/process_macos_sdk.patch diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index ab43764b38..ebc097d686 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -3,11 +3,6 @@ $(package)_version=1.81.0 $(package)_download_path=https://boostorg.jfrog.io/artifactory/main/release/$($(package)_version)/source/ $(package)_file_name=boost_$(subst .,_,$($(package)_version)).tar.bz2 $(package)_sha256_hash=71feeed900fbccca04a3b4f2f84a7c217186f28a940ed8b7ed4725986baf99fa -$(package)_patches=process_macos_sdk.patch - -define $(package)_preprocess_cmds - patch -p1 < $($(package)_patch_dir)/process_macos_sdk.patch -endef define $(package)_stage_cmds mkdir -p $($(package)_staging_prefix_dir)/include && \ diff --git a/depends/patches/boost/process_macos_sdk.patch b/depends/patches/boost/process_macos_sdk.patch deleted file mode 100644 index ebc556d972..0000000000 --- a/depends/patches/boost/process_macos_sdk.patch +++ /dev/null @@ -1,27 +0,0 @@ -Fix Boost Process compilation with macOS 14 SDK. -Can be dropped with Boost 1.84.0. -https://github.com/boostorg/process/pull/343. -https://github.com/boostorg/process/issues/342. - -diff --git a/boost/process/detail/posix/handles.hpp b/boost/process/detail/posix/handles.hpp -index cd9e1ce5a..304e77b1c 100644 ---- a/boost/process/detail/posix/handles.hpp -+++ b/boost/process/detail/posix/handles.hpp -@@ -33,7 +33,7 @@ inline std::vector get_handles(std::error_code & ec) - else - ec.clear(); - -- auto my_fd = ::dirfd(dir.get()); -+ auto my_fd = dirfd(dir.get()); - - struct ::dirent * ent_p; - -@@ -117,7 +117,7 @@ struct limit_handles_ : handler_base_ext - return; - } - -- auto my_fd = ::dirfd(dir); -+ auto my_fd = dirfd(dir); - struct ::dirent * ent_p; - - while ((ent_p = readdir(dir)) != nullptr) From 6ae53e73b0eb21eba7bd85eb3ff50f72079f9f50 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 10 Apr 2024 12:21:22 +0200 Subject: [PATCH 044/530] guix: replace GCC unaligned VMOV patch with binutils patch Rather than invasively patching GCC. Given we have binutils 2.38 available, we can patch it to flip the default for `-muse-unaligned-vector-move`. --- contrib/guix/manifest.scm | 10 +- .../patches/binutils-unaligned-default.patch | 22 ++ contrib/guix/patches/vmov-alignment.patch | 288 ------------------ 3 files changed, 28 insertions(+), 292 deletions(-) create mode 100644 contrib/guix/patches/binutils-unaligned-default.patch delete mode 100644 contrib/guix/patches/vmov-alignment.patch diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm index a06ca39484..a65a6c835c 100644 --- a/contrib/guix/manifest.scm +++ b/contrib/guix/manifest.scm @@ -110,13 +110,15 @@ desirable for building BGL Core release binaries." (define (gcc-mingw-patches gcc) (package-with-extra-patches gcc - (search-our-patches "gcc-remap-guix-store.patch" - "vmov-alignment.patch" - "gcc-broken-longjmp.patch"))) + (search-our-patches "gcc-remap-guix-store.patch"))) + +(define (binutils-mingw-patches binutils) + (package-with-extra-patches binutils + (search-our-patches "binutils-unaligned-default.patch"))) (define (make-mingw-pthreads-cross-toolchain target) "Create a cross-compilation toolchain package for TARGET" - (let* ((xbinutils (cross-binutils target)) + (let* ((xbinutils (binutils-mingw-patches (cross-binutils target))) (pthreads-xlibc mingw-w64-x86_64-winpthreads) (pthreads-xgcc (cross-gcc target #:xgcc (gcc-mingw-patches mingw-w64-base-gcc) diff --git a/contrib/guix/patches/binutils-unaligned-default.patch b/contrib/guix/patches/binutils-unaligned-default.patch new file mode 100644 index 0000000000..d1bc71aee1 --- /dev/null +++ b/contrib/guix/patches/binutils-unaligned-default.patch @@ -0,0 +1,22 @@ +commit 6537181f59ed186a341db621812a6bc35e22eaf6 +Author: fanquake +Date: Wed Apr 10 12:15:52 2024 +0200 + + build: turn on -muse-unaligned-vector-move by default + + This allows us to avoid (more invasively) patching GCC, to avoid + unaligned instruction use. + +diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c +index e0632681477..14a9653abdf 100644 +--- a/gas/config/tc-i386.c ++++ b/gas/config/tc-i386.c +@@ -801,7 +801,7 @@ static unsigned int no_cond_jump_promotion = 0; + static unsigned int sse2avx; + + /* Encode aligned vector move as unaligned vector move. */ +-static unsigned int use_unaligned_vector_move; ++static unsigned int use_unaligned_vector_move = 1; + + /* Encode scalar AVX instructions with specific vector length. */ + static enum diff --git a/contrib/guix/patches/vmov-alignment.patch b/contrib/guix/patches/vmov-alignment.patch deleted file mode 100644 index 96e1cb7cd1..0000000000 --- a/contrib/guix/patches/vmov-alignment.patch +++ /dev/null @@ -1,288 +0,0 @@ -Description: Use unaligned VMOV instructions -Author: Stephen Kitt -Bug-Debian: https://bugs.debian.org/939559 -See also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54412 - -Based on a patch originally by Claude Heiland-Allen - ---- a/gcc/config/i386/sse.md -+++ b/gcc/config/i386/sse.md -@@ -1058,17 +1058,11 @@ - { - if (FLOAT_MODE_P (GET_MODE_INNER (mode))) - { -- if (misaligned_operand (operands[1], mode)) -- return "vmovu\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; -- else -- return "vmova\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; -+ return "vmovu\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; - } - else - { -- if (misaligned_operand (operands[1], mode)) -- return "vmovdqu\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; -- else -- return "vmovdqa\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; -+ return "vmovdqu\t{%1, %0%{%3%}%N2|%0%{%3%}%N2, %1}"; - } - } - [(set_attr "type" "ssemov") -@@ -1184,17 +1178,11 @@ - { - if (FLOAT_MODE_P (GET_MODE_INNER (mode))) - { -- if (misaligned_operand (operands[0], mode)) -- return "vmovu\t{%1, %0%{%2%}|%0%{%2%}, %1}"; -- else -- return "vmova\t{%1, %0%{%2%}|%0%{%2%}, %1}"; -+ return "vmovu\t{%1, %0%{%2%}|%0%{%2%}, %1}"; - } - else - { -- if (misaligned_operand (operands[0], mode)) -- return "vmovdqu\t{%1, %0%{%2%}|%0%{%2%}, %1}"; -- else -- return "vmovdqa\t{%1, %0%{%2%}|%0%{%2%}, %1}"; -+ return "vmovdqu\t{%1, %0%{%2%}|%0%{%2%}, %1}"; - } - } - [(set_attr "type" "ssemov") -@@ -7806,7 +7794,7 @@ - "TARGET_SSE && !(MEM_P (operands[0]) && MEM_P (operands[1]))" - "@ - %vmovlps\t{%1, %0|%q0, %1} -- %vmovaps\t{%1, %0|%0, %1} -+ %vmovups\t{%1, %0|%0, %1} - %vmovlps\t{%1, %d0|%d0, %q1}" - [(set_attr "type" "ssemov") - (set_attr "prefix" "maybe_vex") -@@ -13997,29 +13985,15 @@ - switch (mode) - { - case E_V8DFmode: -- if (misaligned_operand (operands[2], mode)) -- return "vmovupd\t{%2, %x0|%x0, %2}"; -- else -- return "vmovapd\t{%2, %x0|%x0, %2}"; -+ return "vmovupd\t{%2, %x0|%x0, %2}"; - case E_V16SFmode: -- if (misaligned_operand (operands[2], mode)) -- return "vmovups\t{%2, %x0|%x0, %2}"; -- else -- return "vmovaps\t{%2, %x0|%x0, %2}"; -+ return "vmovups\t{%2, %x0|%x0, %2}"; - case E_V8DImode: -- if (misaligned_operand (operands[2], mode)) -- return which_alternative == 2 ? "vmovdqu64\t{%2, %x0|%x0, %2}" -+ return which_alternative == 2 ? "vmovdqu64\t{%2, %x0|%x0, %2}" - : "vmovdqu\t{%2, %x0|%x0, %2}"; -- else -- return which_alternative == 2 ? "vmovdqa64\t{%2, %x0|%x0, %2}" -- : "vmovdqa\t{%2, %x0|%x0, %2}"; - case E_V16SImode: -- if (misaligned_operand (operands[2], mode)) -- return which_alternative == 2 ? "vmovdqu32\t{%2, %x0|%x0, %2}" -+ return which_alternative == 2 ? "vmovdqu32\t{%2, %x0|%x0, %2}" - : "vmovdqu\t{%2, %x0|%x0, %2}"; -- else -- return which_alternative == 2 ? "vmovdqa32\t{%2, %x0|%x0, %2}" -- : "vmovdqa\t{%2, %x0|%x0, %2}"; - default: - gcc_unreachable (); - } -@@ -21225,63 +21199,27 @@ - switch (get_attr_mode (insn)) - { - case MODE_V16SF: -- if (misaligned_operand (operands[1], mode)) -- return "vmovups\t{%1, %t0|%t0, %1}"; -- else -- return "vmovaps\t{%1, %t0|%t0, %1}"; -+ return "vmovups\t{%1, %t0|%t0, %1}"; - case MODE_V8DF: -- if (misaligned_operand (operands[1], mode)) -- return "vmovupd\t{%1, %t0|%t0, %1}"; -- else -- return "vmovapd\t{%1, %t0|%t0, %1}"; -+ return "vmovupd\t{%1, %t0|%t0, %1}"; - case MODE_V8SF: -- if (misaligned_operand (operands[1], mode)) -- return "vmovups\t{%1, %x0|%x0, %1}"; -- else -- return "vmovaps\t{%1, %x0|%x0, %1}"; -+ return "vmovups\t{%1, %x0|%x0, %1}"; - case MODE_V4DF: -- if (misaligned_operand (operands[1], mode)) -- return "vmovupd\t{%1, %x0|%x0, %1}"; -- else -- return "vmovapd\t{%1, %x0|%x0, %1}"; -+ return "vmovupd\t{%1, %x0|%x0, %1}"; - case MODE_XI: -- if (misaligned_operand (operands[1], mode)) -- { -- if (which_alternative == 2) -- return "vmovdqu\t{%1, %t0|%t0, %1}"; -- else if (GET_MODE_SIZE (mode) == 8) -- return "vmovdqu64\t{%1, %t0|%t0, %1}"; -- else -- return "vmovdqu32\t{%1, %t0|%t0, %1}"; -- } -+ if (which_alternative == 2) -+ return "vmovdqu\t{%1, %t0|%t0, %1}"; -+ else if (GET_MODE_SIZE (mode) == 8) -+ return "vmovdqu64\t{%1, %t0|%t0, %1}"; - else -- { -- if (which_alternative == 2) -- return "vmovdqa\t{%1, %t0|%t0, %1}"; -- else if (GET_MODE_SIZE (mode) == 8) -- return "vmovdqa64\t{%1, %t0|%t0, %1}"; -- else -- return "vmovdqa32\t{%1, %t0|%t0, %1}"; -- } -+ return "vmovdqu32\t{%1, %t0|%t0, %1}"; - case MODE_OI: -- if (misaligned_operand (operands[1], mode)) -- { -- if (which_alternative == 2) -- return "vmovdqu\t{%1, %x0|%x0, %1}"; -- else if (GET_MODE_SIZE (mode) == 8) -- return "vmovdqu64\t{%1, %x0|%x0, %1}"; -- else -- return "vmovdqu32\t{%1, %x0|%x0, %1}"; -- } -+ if (which_alternative == 2) -+ return "vmovdqu\t{%1, %x0|%x0, %1}"; -+ else if (GET_MODE_SIZE (mode) == 8) -+ return "vmovdqu64\t{%1, %x0|%x0, %1}"; - else -- { -- if (which_alternative == 2) -- return "vmovdqa\t{%1, %x0|%x0, %1}"; -- else if (GET_MODE_SIZE (mode) == 8) -- return "vmovdqa64\t{%1, %x0|%x0, %1}"; -- else -- return "vmovdqa32\t{%1, %x0|%x0, %1}"; -- } -+ return "vmovdqu32\t{%1, %x0|%x0, %1}"; - default: - gcc_unreachable (); - } ---- a/gcc/config/i386/i386.cc -+++ b/gcc/config/i386/i386.cc -@@ -5418,17 +5418,15 @@ ix86_get_ssemov (rtx *operands, unsigned size, - { - case opcode_int: - if (scalar_mode == E_HFmode) -- opcode = (misaligned_p -- ? (TARGET_AVX512BW ? "vmovdqu16" : "vmovdqu64") -- : "vmovdqa64"); -+ opcode = TARGET_AVX512BW ? "vmovdqu16" : "vmovdqu64"; - else -- opcode = misaligned_p ? "vmovdqu32" : "vmovdqa32"; -+ opcode = "vmovdqu32"; - break; - case opcode_float: -- opcode = misaligned_p ? "vmovups" : "vmovaps"; -+ opcode = "vmovups"; - break; - case opcode_double: -- opcode = misaligned_p ? "vmovupd" : "vmovapd"; -+ opcode = "vmovupd"; - break; - } - } -@@ -5438,29 +5436,21 @@ ix86_get_ssemov (rtx *operands, unsigned size, - { - case E_HFmode: - if (evex_reg_p) -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu16" -- : "vmovdqu64") -- : "vmovdqa64"); -+ opcode = TARGET_AVX512BW ? "vmovdqu16" : "vmovdqu64"; - else -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu16" -- : "%vmovdqu") -- : "%vmovdqa"); -+ opcode = TARGET_AVX512BW ? "vmovdqu16" : "%vmovdqu"; - break; - case E_SFmode: -- opcode = misaligned_p ? "%vmovups" : "%vmovaps"; -+ opcode = "%vmovups"; - break; - case E_DFmode: -- opcode = misaligned_p ? "%vmovupd" : "%vmovapd"; -+ opcode = "%vmovupd"; - break; - case E_TFmode: - if (evex_reg_p) -- opcode = misaligned_p ? "vmovdqu64" : "vmovdqa64"; -+ opcode = "vmovdqu64"; - else -- opcode = misaligned_p ? "%vmovdqu" : "%vmovdqa"; -+ opcode = "%vmovdqu"; - break; - default: - gcc_unreachable (); -@@ -5472,48 +5462,32 @@ ix86_get_ssemov (rtx *operands, unsigned size, - { - case E_QImode: - if (evex_reg_p) -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu8" -- : "vmovdqu64") -- : "vmovdqa64"); -+ opcode = TARGET_AVX512BW ? "vmovdqu8" : "vmovdqu64"; - else -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu8" -- : "%vmovdqu") -- : "%vmovdqa"); -+ opcode = TARGET_AVX512BW ? "vmovdqu8" : "%vmovdqu"; - break; - case E_HImode: - if (evex_reg_p) -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu16" -- : "vmovdqu64") -- : "vmovdqa64"); -+ opcode = TARGET_AVX512BW ? "vmovdqu16" : "vmovdqu64"; - else -- opcode = (misaligned_p -- ? (TARGET_AVX512BW -- ? "vmovdqu16" -- : "%vmovdqu") -- : "%vmovdqa"); -+ opcode = TARGET_AVX512BW ? "vmovdqu16" : "%vmovdqu"; - break; - case E_SImode: - if (evex_reg_p) -- opcode = misaligned_p ? "vmovdqu32" : "vmovdqa32"; -+ opcode = "vmovdqu32"; - else -- opcode = misaligned_p ? "%vmovdqu" : "%vmovdqa"; -+ opcode = "%vmovdqu"; - break; - case E_DImode: - case E_TImode: - case E_OImode: - if (evex_reg_p) -- opcode = misaligned_p ? "vmovdqu64" : "vmovdqa64"; -+ opcode = "vmovdqu64"; - else -- opcode = misaligned_p ? "%vmovdqu" : "%vmovdqa"; -+ opcode = "%vmovdqu"; - break; - case E_XImode: -- opcode = misaligned_p ? "vmovdqu64" : "vmovdqa64"; -+ opcode = "vmovdqu64"; - break; - default: - gcc_unreachable (); From 1b6db1e0e1fade23cf3d06c58a70c6872a9fc17e Mon Sep 17 00:00:00 2001 From: dergoegge Date: Fri, 25 Nov 2022 15:31:04 +0000 Subject: [PATCH 045/530] [net processing] Move nTimeOffset to net_processing --- src/net.cpp | 1 - src/net.h | 2 -- src/net_processing.cpp | 10 +++++++--- src/net_processing.h | 1 + src/qt/guiutil.cpp | 4 ++-- src/qt/guiutil.h | 4 ++-- src/qt/rpcconsole.cpp | 2 +- src/rpc/net.cpp | 2 +- 8 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 7928298c22..c7f670b1e9 100755 --- a/src/net.cpp +++ b/src/net.cpp @@ -599,7 +599,6 @@ void CNode::CopyStats(CNodeStats& stats) X(m_last_tx_time); X(m_last_block_time); X(m_connected); - X(nTimeOffset); X(m_addr_name); X(nVersion); { diff --git a/src/net.h b/src/net.h index 1154773b76..69be0f7507 100644 --- a/src/net.h +++ b/src/net.h @@ -191,7 +191,6 @@ class CNodeStats std::chrono::seconds m_last_tx_time; std::chrono::seconds m_last_block_time; std::chrono::seconds m_connected; - int64_t nTimeOffset; std::string m_addr_name; int nVersion; std::string cleanSubVer; @@ -703,7 +702,6 @@ class CNode std::atomic m_last_recv{0s}; //! Unix epoch time at peer connection const std::chrono::seconds m_connected; - std::atomic nTimeOffset{0}; // Address of this peer const CAddress addr; // Bind address of our side of the connection diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 39ffff97d2..b712e4cdb2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -390,6 +390,10 @@ struct Peer { /** Whether this peer wants invs or headers (when possible) for block announcements */ bool m_prefers_headers GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; + /** Time offset computed during the version handshake based on the + * timestamp the peer sent in the version message. */ + std::atomic m_time_offset{0}; + explicit Peer(NodeId id, ServiceFlags our_services) : m_id{id} , m_our_services{our_services} @@ -1792,6 +1796,7 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c stats.presync_height = peer->m_headers_sync->GetPresyncHeight(); } } + stats.time_offset = peer->m_time_offset; return true; } @@ -3666,12 +3671,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, peer->m_starting_height, addrMe.ToStringAddrPort(), fRelay, pfrom.GetId(), remoteAddr, (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : "")); - int64_t nTimeOffset = nTime - GetTime(); - pfrom.nTimeOffset = nTimeOffset; + peer->m_time_offset = nTime - GetTime(); if (!pfrom.IsInboundConn()) { // Don't use timedata samples from inbound peers to make it // harder for others to tamper with our adjusted time. - AddTimeData(pfrom.addr, nTimeOffset); + AddTimeData(pfrom.addr, peer->m_time_offset); } // If the peer is old enough to have the old alert system, send it the final alert. diff --git a/src/net_processing.h b/src/net_processing.h index 93f7322a96..9e802ca9eb 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -41,6 +41,7 @@ struct CNodeStateStats { bool m_addr_relay_enabled{false}; ServiceFlags their_services; int64_t presync_height{-1}; + int64_t time_offset{0}; }; class PeerManager : public CValidationInterface, public NetEventsInterface diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 53ee4a0340..4638e7b487 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -764,9 +764,9 @@ QString formatPingTime(std::chrono::microseconds ping_time) QObject::tr("%1 ms").arg(QString::number((int)(count_microseconds(ping_time) / 1000), 10)); } -QString formatTimeOffset(int64_t nTimeOffset) +QString formatTimeOffset(int64_t time_offset) { - return QObject::tr("%1 s").arg(QString::number((int)nTimeOffset, 10)); + return QObject::tr("%1 s").arg(QString::number((int)time_offset, 10)); } QString formatNiceTimeOffset(qint64 secs) diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 677d113eae..134ddb9644 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -240,8 +240,8 @@ namespace GUIUtil /** Format a CNodeStats.m_last_ping_time into a user-readable string or display N/A, if 0 */ QString formatPingTime(std::chrono::microseconds ping_time); - /** Format a CNodeCombinedStats.nTimeOffset into a user-readable string */ - QString formatTimeOffset(int64_t nTimeOffset); + /** Format a CNodeStateStats.time_offset into a user-readable string */ + QString formatTimeOffset(int64_t time_offset); QString formatNiceTimeOffset(qint64 secs); diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 1c2804dda9..8e5458750c 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1196,7 +1196,6 @@ void RPCConsole::updateDetailWidget() ui->peerBytesRecv->setText(GUIUtil::formatBytes(stats->nodeStats.nRecvBytes)); ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.m_last_ping_time)); ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.m_min_ping_time)); - ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStats.nTimeOffset)); if (stats->nodeStats.nVersion) { ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion)); } @@ -1228,6 +1227,7 @@ void RPCConsole::updateDetailWidget() // This check fails for example if the lock was busy and // nodeStateStats couldn't be fetched. if (stats->fNodeStateStatsAvailable) { + ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStateStats.time_offset)); ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStateStats.their_services)); // Sync height is init to -1 if (stats->nodeStateStats.nSyncHeight > -1) { diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index f935a3b08f..410769b1fe 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -239,7 +239,7 @@ static RPCHelpMan getpeerinfo() obj.pushKV("bytessent", stats.nSendBytes); obj.pushKV("bytesrecv", stats.nRecvBytes); obj.pushKV("conntime", count_seconds(stats.m_connected)); - obj.pushKV("timeoffset", stats.nTimeOffset); + obj.pushKV("timeoffset", statestats.time_offset); if (stats.m_last_ping_time > 0us) { obj.pushKV("pingtime", Ticks(stats.m_last_ping_time)); } From f330b6138f7c673d13df21de58e5e2fe24c17324 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Sat, 3 Feb 2024 17:15:41 +0000 Subject: [PATCH 046/530] [net processing] Use std::chrono for type-safe time offsets --- src/net_processing.cpp | 6 +++--- src/net_processing.h | 4 +++- src/qt/rpcconsole.cpp | 3 ++- src/rpc/net.cpp | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b712e4cdb2..cd17106c9b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -392,7 +392,7 @@ struct Peer { /** Time offset computed during the version handshake based on the * timestamp the peer sent in the version message. */ - std::atomic m_time_offset{0}; + std::atomic m_time_offset{0s}; explicit Peer(NodeId id, ServiceFlags our_services) : m_id{id} @@ -3671,11 +3671,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, peer->m_starting_height, addrMe.ToStringAddrPort(), fRelay, pfrom.GetId(), remoteAddr, (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : "")); - peer->m_time_offset = nTime - GetTime(); + peer->m_time_offset = NodeSeconds{std::chrono::seconds{nTime}} - Now(); if (!pfrom.IsInboundConn()) { // Don't use timedata samples from inbound peers to make it // harder for others to tamper with our adjusted time. - AddTimeData(pfrom.addr, peer->m_time_offset); + AddTimeData(pfrom.addr, Ticks(peer->m_time_offset.load())); } // If the peer is old enough to have the old alert system, send it the final alert. diff --git a/src/net_processing.h b/src/net_processing.h index 9e802ca9eb..26b7806ce1 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -9,6 +9,8 @@ #include #include +#include + class AddrMan; class CChainParams; class CTxMemPool; @@ -41,7 +43,7 @@ struct CNodeStateStats { bool m_addr_relay_enabled{false}; ServiceFlags their_services; int64_t presync_height{-1}; - int64_t time_offset{0}; + std::chrono::seconds time_offset{0}; }; class PeerManager : public CValidationInterface, public NetEventsInterface diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 8e5458750c..5a5a2b6e49 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -1227,7 +1228,7 @@ void RPCConsole::updateDetailWidget() // This check fails for example if the lock was busy and // nodeStateStats couldn't be fetched. if (stats->fNodeStateStatsAvailable) { - ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStateStats.time_offset)); + ui->timeoffset->setText(GUIUtil::formatTimeOffset(Ticks(stats->nodeStateStats.time_offset))); ui->peerServices->setText(GUIUtil::formatServicesStr(stats->nodeStateStats.their_services)); // Sync height is init to -1 if (stats->nodeStateStats.nSyncHeight > -1) { diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 410769b1fe..7194d63248 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -239,7 +239,7 @@ static RPCHelpMan getpeerinfo() obj.pushKV("bytessent", stats.nSendBytes); obj.pushKV("bytesrecv", stats.nRecvBytes); obj.pushKV("conntime", count_seconds(stats.m_connected)); - obj.pushKV("timeoffset", statestats.time_offset); + obj.pushKV("timeoffset", Ticks(statestats.time_offset)); if (stats.m_last_ping_time > 0us) { obj.pushKV("pingtime", Ticks(stats.m_last_ping_time)); } From 3dbcc4beead7b4d67169945fc6a447894b2b6d67 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Mon, 27 Nov 2023 17:22:41 +0000 Subject: [PATCH 047/530] [net processing] Introduce PeerManagerInfo For querying statistics/info from PeerManager. The median outbound time offset is the only initial field. --- src/net_processing.cpp | 8 ++++++++ src/net_processing.h | 7 +++++++ src/rpc/net.cpp | 4 ++-- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index cd17106c9b..cf654511ac 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -517,6 +517,7 @@ class PeerManagerImpl final : public PeerManager std::optional FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + PeerManagerInfo GetInfo() const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool IgnoresIncomingTxs() override { return m_opts.ignore_incoming_txs; } void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); @@ -1801,6 +1802,13 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c return true; } +PeerManagerInfo PeerManagerImpl::GetInfo() const +{ + return PeerManagerInfo{ + .median_outbound_time_offset = m_outbound_time_offsets.Median(), + }; +} + void PeerManagerImpl::AddToCompactExtraTransactions(const CTransactionRef& tx) { if (m_opts.max_extra_txs <= 0) diff --git a/src/net_processing.h b/src/net_processing.h index 26b7806ce1..46bdf42b76 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -46,6 +46,10 @@ struct CNodeStateStats { std::chrono::seconds time_offset{0}; }; +struct PeerManagerInfo { + std::chrono::seconds median_outbound_time_offset{0s}; +}; + class PeerManager : public CValidationInterface, public NetEventsInterface { public: @@ -86,6 +90,9 @@ class PeerManager : public CValidationInterface, public NetEventsInterface /** Get statistics from node state */ virtual bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const = 0; + /** Get peer manager info. */ + virtual PeerManagerInfo GetInfo() const = 0; + /** Whether this node ignores txs received over p2p. */ virtual bool IgnoresIncomingTxs() = 0; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 7194d63248..2e21a68f58 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include @@ -679,9 +678,10 @@ static RPCHelpMan getnetworkinfo() obj.pushKV("localservicesnames", GetServicesNames(services)); } if (node.peerman) { + auto peerman_info{node.peerman->GetInfo()}; obj.pushKV("localrelay", !node.peerman->IgnoresIncomingTxs()); + obj.pushKV("timeoffset", Ticks(peerman_info.median_outbound_time_offset)); } - obj.pushKV("timeoffset", GetTimeOffset()); if (node.connman) { obj.pushKV("networkactive", node.connman->GetNetworkActive()); obj.pushKV("connections", node.connman->GetNodeCount(ConnectionDirection::Both)); From a09e0541672723281daa848da758d3fa91d38a58 Mon Sep 17 00:00:00 2001 From: dergoegge Date: Wed, 29 Nov 2023 12:03:49 +0000 Subject: [PATCH 048/530] [net processing] Move IgnoresIncomingTxs to PeerManagerInfo --- src/net_processing.cpp | 2 +- src/net_processing.h | 4 +--- src/rpc/net.cpp | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index cf654511ac..1da58b09d3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -518,7 +518,6 @@ class PeerManagerImpl final : public PeerManager EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); PeerManagerInfo GetInfo() const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - bool IgnoresIncomingTxs() override { return m_opts.ignore_incoming_txs; } void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void SetBestBlock(int height, std::chrono::seconds time) override @@ -1806,6 +1805,7 @@ PeerManagerInfo PeerManagerImpl::GetInfo() const { return PeerManagerInfo{ .median_outbound_time_offset = m_outbound_time_offsets.Median(), + .ignores_incoming_txs = m_opts.ignore_incoming_txs, }; } diff --git a/src/net_processing.h b/src/net_processing.h index 46bdf42b76..bbcf030a98 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -48,6 +48,7 @@ struct CNodeStateStats { struct PeerManagerInfo { std::chrono::seconds median_outbound_time_offset{0s}; + bool ignores_incoming_txs{false}; }; class PeerManager : public CValidationInterface, public NetEventsInterface @@ -93,9 +94,6 @@ class PeerManager : public CValidationInterface, public NetEventsInterface /** Get peer manager info. */ virtual PeerManagerInfo GetInfo() const = 0; - /** Whether this node ignores txs received over p2p. */ - virtual bool IgnoresIncomingTxs() = 0; - /** Relay transaction to all peers. */ virtual void RelayTransaction(const uint256& txid, const uint256& wtxid) = 0; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 2e21a68f58..fc3baf2adf 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -679,7 +679,7 @@ static RPCHelpMan getnetworkinfo() } if (node.peerman) { auto peerman_info{node.peerman->GetInfo()}; - obj.pushKV("localrelay", !node.peerman->IgnoresIncomingTxs()); + obj.pushKV("localrelay", !peerman_info.ignores_incoming_txs); obj.pushKV("timeoffset", Ticks(peerman_info.median_outbound_time_offset)); } if (node.connman) { From a94abdef48f409c4bdc042139dc454e65825c984 Mon Sep 17 00:00:00 2001 From: stickies-v Date: Sun, 4 Feb 2024 05:08:26 +0000 Subject: [PATCH 049/530] Remove timedata With the introduction and usage of TimeOffsets, there is no more need for this file. Remove it. --- src/Makefile.am | 2 - src/Makefile.test.include | 4 +- src/addrman_impl.h | 1 - src/init.cpp | 2 - src/net_processing.cpp | 6 +- src/test/denialofservice_tests.cpp | 2 - src/test/fuzz/timedata.cpp | 31 -------- src/test/net_tests.cpp | 5 -- src/test/timedata_tests.cpp | 105 -------------------------- src/timedata.cpp | 116 ----------------------------- src/timedata.h | 82 -------------------- 11 files changed, 5 insertions(+), 351 deletions(-) delete mode 100644 src/test/fuzz/timedata.cpp delete mode 100644 src/test/timedata_tests.cpp delete mode 100644 src/timedata.cpp delete mode 100644 src/timedata.h diff --git a/src/Makefile.am b/src/Makefile.am index 62c3fd513f..bbd7932d5c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -280,7 +280,6 @@ BGL_CORE_H = \ support/lockedpool.h \ sync.h \ threadsafety.h \ - timedata.h \ torcontrol.h \ txdb.h \ txmempool.h \ @@ -459,7 +458,6 @@ libBGL_node_a_SOURCES = \ rpc/txoutproof.cpp \ script/sigcache.cpp \ signet.cpp \ - timedata.cpp \ torcontrol.cpp \ txdb.cpp \ txmempool.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index cd34e64b55..978d06d594 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -151,7 +151,7 @@ BGL_TESTS =\ test/streams_tests.cpp \ test/sync_tests.cpp \ test/system_tests.cpp \ - test/timedata_tests.cpp \ + test/timeoffsets_tests.cpp \ test/torcontrol_tests.cpp \ test/transaction_tests.cpp \ test/translation_tests.cpp \ @@ -380,7 +380,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/string.cpp \ test/fuzz/strprintf.cpp \ test/fuzz/system.cpp \ - test/fuzz/timedata.cpp \ + test/fuzz/timeoffsets.cpp \ test/fuzz/torcontrol.cpp \ test/fuzz/transaction.cpp \ test/fuzz/tx_in.cpp \ diff --git a/src/addrman_impl.h b/src/addrman_impl.h index f6bfc17a0c..07d9640a66 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -11,7 +11,6 @@ #include #include #include -#include #include #include diff --git a/src/init.cpp b/src/init.cpp index ca6cd69510..6d4827018c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -67,7 +67,6 @@ #include #include