Disable boost multi index safe mode by default when configuring with
--enable-debug.
This option can cause transactions to take a long time to be accepted
into the mempool under certain conditions; iterator destruction takes
O(n) time vs O(1) as they are stored in a singly linked list. See
27586 for more information.
Re-enable it on the CI builds which previously had it enabled.
Re-enable it on the msan fuzz target so that we have fuzz tasks testing
with it enabeld and disabled in this repo.
Github-Pull: #27724
Rebased-From: 59c89447499bd9d6202269879555b8bc37373aa2
glibc 2.33 introduced a new fortification level, _FORTIFY_SOURCE=3.
Which improves the coverage of cases where _FORTIFY_SOURCE can use _chk
functions. For example, using GCC 13 and glibc 2.36 (Fedora Rawhide),
compiling master:
```bash
nm -C src/bitcoind | grep _chk
U __fprintf_chk@GLIBC_2.17
U __memcpy_chk@GLIBC_2.17
U __snprintf_chk@GLIBC_2.17
U __sprintf_chk@GLIBC_2.17
U __stack_chk_fail@GLIBC_2.17
U __stack_chk_guard@GLIBC_2.17
U __vsnprintf_chk@GLIBC_2.17
objdump -d src/bitcoind | grep "_chk@plt" | wc -l
33
```
vs this branch:
```bash
nm -C src/bitcoind | grep _chk
U __fprintf_chk@GLIBC_2.17
U __memcpy_chk@GLIBC_2.17
U __memset_chk@GLIBC_2.17
U __snprintf_chk@GLIBC_2.17
U __sprintf_chk@GLIBC_2.17
U __stack_chk_fail@GLIBC_2.17
U __stack_chk_guard@GLIBC_2.17
U __vsnprintf_chk@GLIBC_2.17
objdump -d src/bitcoind | grep "_chk@plt" | wc -l
61
```
Usage of level 3 requires LLVM/Clang 9+, or GCC 12+. Older
compilers/glibc will still use _FORTIFY_SOURCE=2. For example, in the
glibc we currently use for Linux release builds (2.24), FORTIFY_LEVEL is
determined using the following:
```c
```
so any value > 1 will turn on _FORTIFY_SOURCE=2.
https://sourceware.org/pipermail/libc-alpha/2021-February/122207.htmlhttps://developers.redhat.com/blog/2021/04/16/broadening-compiler-checks-for-buffer-overflows-in-_fortify_source
b3b673f7048cce1d1368819abb0b58b7c6699fa5 mapport: require miniupnpc API version 17 or later (fanquake)
Pull request description:
Version 17 is currently the latest version, see: https://github.com/miniupnp/miniupnp/blob/master/miniupnpc/apiversions.txt, and has been available since the release of 2.1. 2.1 or newer is readily available across all distros, see https://repology.org/project/miniupnpc/versions, so drop support for the older API versions.
Split out of #22644.
ACKs for top commit:
hebasto:
ACK b3b673f7048cce1d1368819abb0b58b7c6699fa5, tested on Ubuntu 20.04 w/ and w/o [`libminiupnpc-dev`](https://packages.ubuntu.com/focal/libminiupnpc-dev) package.
TheCharlatan:
ACK b3b673f7048cce1d1368819abb0b58b7c6699fa5
Tree-SHA512: f53b36b82462c4ea83d9b83413dca8097885d1620f7ca0a53a79d6b3d3cf37c7773828b23f4278ccfcc3b14fcb0faffa35f60191b519b04570f3d2783d0303e2
Even though all other targets are disabled, we still need Boost CPPFLAGS
(use_boost) to compile. This currently works everywhere, except on arm
macOS (where the include path is pretty non-standard), because
generally, the Boost include path is generic, i.e `/usr/include`.
d4c59da8d6e3aac14306249aa12332fed55efebd build: Avoid `BOOST_NO_CXX98_FUNCTION_BASE` macro redefinition (Hennadii Stepanov)
Pull request description:
With GCC 12 and Boost 1.81 (from depends) having multiple warnings:
```
In file included from /home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/config.hpp:48:
/home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/config/stdlib/libstdcpp3.hpp:397:9: warning: 'BOOST_NO_CXX98_FUNCTION_BASE' macro redefined [-Wmacro-redefined]
#define BOOST_NO_CXX98_FUNCTION_BASE
^
<command line>:8:9: note: previous definition is here
#define BOOST_NO_CXX98_FUNCTION_BASE 1
^
1 warning generated.
```
This PR fixes those warnings.
Defining of the `BOOST_NO_CXX98_FUNCTION_BASE` macro was introduced in https://github.com/bitcoin/bitcoin/pull/25436, but since https://github.com/boostorg/config/pull/430, it is required to check it before adding.
ACKs for top commit:
fanquake:
ACK d4c59da8d6e3aac14306249aa12332fed55efebd - it works now.
Tree-SHA512: 53b9ddcf8dad729638ed41251e30c80f2d7d1ae3ffe47466865834f1f10184fe0881abeb339b3e46c270c3eb11fb63d19ab12cc9461bf5c2be12b4763c1b1c34
d51f0fa4b7b19281efe65aacf414845c661d0a13 doc: add release notes for 26896 (fanquake)
2b248798d96f794db08b7725730b5fb4e00b9b10 build: remove --enable-upnp-default from configure (fanquake)
02f5a5e7b5fd7ba35e407d4409202a0e0fed003c build: remove --enable-natpmp-default from configure (fanquake)
25a0e8ba0b31d8bd265df0589fe49241a60d0fc2 Remove configure-time setting of DEFAULT_UPNP (fanquake)
06562e5fa771dab275a9cab4914cd64d961a52bc Remove configure-time setting of DEFAULT_NATPMP (fanquake)
Pull request description:
This PR removes the `--enable-upnp-default` and `--enable-natpmp-default` options from configure.
It's odd to me that we maintain configure-time options for setting the default port-forwarding runtime state (but no other similar options), and I'm not sure what use-case it satisfies, that can't be achieved by multiple other means. I also doubt that we'll ever restart using these in release builds, or turning on any of this by default.
I think the only scenario these options would be used is when you want to compile your own binaries (we don't use them in Guix), with port-forwarding on by default, but otherwise can't or don't want to use a `.conf` file, can't or don't want to pass command line options at runtime, and also don't want to modify the source code?
ACKs for top commit:
hebasto:
ACK d51f0fa4b7b19281efe65aacf414845c661d0a13, rebased and comments have been addressed since my recent [review](https://github.com/bitcoin/bitcoin/pull/26896#pullrequestreview-1273910740).
TheCharlatan:
ACK d51f0fa4b7b19281efe65aacf414845c661d0a13
Tree-SHA512: 481decd8bddd8b03b7319591e3acf189f7b6b96c9a9a8c5bc1a3f8ec00d0b8f9b52d2f5c28a298a2ec947cfe9611cfd184e393ccb2e4e21bfce86ca7d4de60d3
0f883df7a5430d6b229a2f190fe7daab24802ebf build: fix configuring with only bitcoin-util (fanquake)
Pull request description:
Fixes the issue presented in #25037 in a single (easily backportable) diff, with no additional refactoring/changes.
Can be tested with:
```bash
./configure \
--disable-tests \
--disable-bench \
--without-libs \
--without-daemon \
--without-gui \
--disable-fuzz-binary \
--without-utils \
--enable-util-util
```
ACKs for top commit:
TheCharlatan:
tACK 0f883df7a5430d6b229a2f190fe7daab24802ebf
hebasto:
ACK 0f883df7a5430d6b229a2f190fe7daab24802ebf, tested on Ubuntu 22.04.
Tree-SHA512: 3682712405c360852c4edd90c171e21302154bf8789252c64083974a5c873cf04d97e8721c7916d5b2dafa6acd2b8dc32deecf550e90e03bcbbabbbbf75ce959
202291722300b86f36e97de7960d40a32544c2d1 Add secp256k1_selftest call (Pieter Wuille)
3bfca788b0dae879bfc745cc52c2cb6edc49fd70 Remove explicit enabling of default modules (Pieter Wuille)
4462cb04986d77eddcfc6e8f75e04dc278a8147a Adapt to libsecp256k1 API changes (Pieter Wuille)
9d47e7b71b2805430e8c7b43816efd225a6ccd8c Squashed 'src/secp256k1/' changes from 44c2452fd3..21ffe4b22a (Pieter Wuille)
Pull request description:
Now that libsecp256k1 has a release (https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-December/021271.html), update the subtree to match it.
The changes themselves are not very impactful for Bitcoin Core, but include:
* It's no longer needed to specify whether contexts are for signing or verification or both (all contexts support everything), so make use of that in this PR.
* Verification operations can use the static context now, removing the need for some infrastructure in pubkey.cpp to make sure a context exists.
* Most modules are now enabled by default, so we can drop explicit enabling for them.
* CI improvements (in particular, MSVC and more recent MacOS)
* Introduction of an internal int128 type, which has no effect for GCC/Clang builds, but enables 128-bit multiplication in MSVC, giving a ~20% speedup there (but still slower than GCC/Clang).
* Release process changes (process documentation, changelog, ...).
ACKs for top commit:
Sjors:
ACK 202291722300b86f36e97de7960d40a32544c2d1, but 4462cb04986d77eddcfc6e8f75e04dc278a8147a could use more eyes on it.
achow101:
ACK 202291722300b86f36e97de7960d40a32544c2d1
jonasnick:
utACK 202291722300b86f36e97de7960d40a32544c2d1
Tree-SHA512: 8a9fe28852abe74abd6f96fef16a94d5a427b1d99bff4caab1699014d24698aab9b966a5364a46ed1001c07a7c1d825154ed4e6557c7decce952b77330a8616b
Fixes the issue presented in #25037 in a single (easily backportable)
diff, with no additional refactoring/changes.
Can be tested with:
```bash
./configure \
--disable-tests \
--disable-bench \
--without-libs \
--without-daemon \
--without-gui \
--disable-fuzz-binary \
--without-utils \
--enable-util-util
```
These headers are already included in a default set which are checked
early during configure.
We already use at least sys/types.h and unistd.h unconditionally in
configure.
98868633d1d5cd2591b9613545bd2ce2e81af212 Bugfix: configure: bitcoin-{cli,tx,util} don't need UPnP, NAT-PMP, or ZMQ (Luke Dashjr)
Pull request description:
As with #23345, these other tools likewise don't use various deps.
ACKs for top commit:
achow101:
ACK 98868633d1d5cd2591b9613545bd2ce2e81af212
Tree-SHA512: 4be056b8e0c9f69834229aa257187457de1bc34214d320b770834e21ecc1f0ca7aa7b9689fba525928947bfabbb461528795f709014fb9618b82f088fe64f271
d216d714aae36e6f1c95f82aef81a0be74dee2f3 Revert "build: Use Homebrew's sqlite package if it is available" (fanquake)
Pull request description:
This reverts ee7b84e63cbeadd5e680d69ff0548275581e9241 from #20527.
That change was made without any rationale, maybe other than, a brew
installed version might be newer, and that's "better". However when
building from source on macOS, it just results in drastically worse
performance, and issues / confusion like #25724.
The difference in performance can be observed using the example from https://github.com/bitcoin/bitcoin/issues/25724#issuecomment-1213554922,
but minified i.e:
```bash
time src/bitcoin-cli createwallet speedy true
time src/bitcoin-cli importdescriptors '[
{"desc":"raw(00145846369f3d6ba366d6f5a903fb5cf4dca3763c0e)#k9wh6v62","timestamp":"now"},
{"desc":"raw(001420800aabf13f3a4c4ce3ce4c66cecf1d17f21a6e)#6m0hlfh4","timestamp":"now"},
{"desc":"raw(0014c6bf9715e06d73ebf9b3b02d5cc48d24d8bbabc1)#wyavh36r","timestamp":"now"},
{"desc":"raw(00141ba7807b3f46af113beaea5c698428ce7138cd8a)#jctdsups","timestamp":"now"},
{"desc":"raw(00140c1bd27f10fff01b36ddf3c1febaa1acff19b080)#9s6nc3pk","timestamp":"now"},
{"desc":"raw(00141226e31987e4bc2e63c0ee12908f675e40464b20)#9pp7qm39","timestamp":"now"},
{"desc":"raw(0014f73f149f7503960a5e849c6ee7a8a8c336f631cb)#qtkxv9fc","timestamp":"now"},
{"desc":"raw(0014c8ccb4d81ffc769fc5fdd8d7eed69b0e0cae5749)#hn39qayv","timestamp":"now"},
{"desc":"raw(001498565aead2d67a22a6021d55210f2a917fc22169)#6ar3vwsx","timestamp":"now"},
{"desc":"raw(001403013248ac0cd9eabe176cad162cda2a19f771e1)#4m47mukd","timestamp":"now"}
]'
```
Running master, when building from souce and using brew installed
sqlite, this takes ~3.4s. With this PR, the same operation takes ~0.3s.
Resolves the "build from source" portion of #25724. Building from
depends is still not ideal, however I have some other changes that might
help improve things in that case.
Related performance issue reports:
* https://github.com/bitcoindevkit/bdk/issues/749
* https://bitcoin.stackexchange.com/questions/113898/bitcoin-v23-is-10-times-slower-than-v22-on-macos-for-basic-regtest-tests
* https://github.com/bitcoin/bitcoin/issues/25724
* https://github.com/bitcoin/bitcoin/pull/25985#issuecomment-1245942400
ACKs for top commit:
achow101:
ACK d216d714aae36e6f1c95f82aef81a0be74dee2f3
jarolrod:
ACK d216d714aae36e6f1c95f82aef81a0be74dee2f3
hebasto:
ACK d216d714aae36e6f1c95f82aef81a0be74dee2f3, I have reviewed the code and it looks OK, I agree it can be merged. No conflicts with our build [docs](d216d714aa/doc/build-osx.md (descriptor-wallet-support)).
Tree-SHA512: 1bb4b44385b11fa9fe66edd7449278f9e47a6cc679b7111f9adf17db94c34e29c9cceafc917454e134420db40b24b56da29226af6f43e6dbeff822b79b77ed60
We currently perform the same check twice, to put the same set of flags
in two different variables. Split the checks so we test for crc and crypto
extensions independently.
If we don't want to split, we should just delete the second AX_CHECK_COMPILE_FLAG
check, and set ARM_CRC_CXXFLAGS & ARM_CRC_CXXFLAGS at the same time.
We already use a mix of <cstdlib> and stdlib.h unconditionally throughout
the codebase.
Us checking this header also duplicates work already done by autotools.
Currently stdlib.h is checked for 3 times during a ./configure run, after
this change, at least it's only twice.
We already use a mix of <cstdio> and stdio.h unconditionally throughout
the codebase.
Us checking this header also duplicates work already done by autotools.
Currently stdio.h is checked for 3 times during a ./configure run, after
this change, at least it's only twice.
We don't include strings.h anywhere.
This is also already checked for by autoconf, so us checking for it just
means a 3rd existence check during ./configure.
This reverts ee7b84e63cbeadd5e680d69ff0548275581e9241 from #20527.
This change was made without any rationale, maybe other than a brew
installed version might be newer, and that's "better". However when
building from source on macOS, it just results in drastically worse
perofrmance, and results in issues / confusions like #25724.
Resolves the "build from source" portion of #25724. Building from
depends is still not ideal, however I have some other changes that might
help improve things in that case.
The difference in performance can be observed using the example from
https://github.com/bitcoin/bitcoin/issues/25724#issuecomment-1213554922,
but minified to only 10 descriptors. i.e:
```bash
time src/bitcoin-cli createwallet speedy true
time src/bitcoin-cli importdescriptors '[
{"desc":"raw(00145846369f3d6ba366d6f5a903fb5cf4dca3763c0e)#k9wh6v62","timestamp":"now"},
{"desc":"raw(001420800aabf13f3a4c4ce3ce4c66cecf1d17f21a6e)#6m0hlfh4","timestamp":"now"},
{"desc":"raw(0014c6bf9715e06d73ebf9b3b02d5cc48d24d8bbabc1)#wyavh36r","timestamp":"now"},
{"desc":"raw(00141ba7807b3f46af113beaea5c698428ce7138cd8a)#jctdsups","timestamp":"now"},
{"desc":"raw(00140c1bd27f10fff01b36ddf3c1febaa1acff19b080)#9s6nc3pk","timestamp":"now"},
{"desc":"raw(00141226e31987e4bc2e63c0ee12908f675e40464b20)#9pp7qm39","timestamp":"now"},
{"desc":"raw(0014f73f149f7503960a5e849c6ee7a8a8c336f631cb)#qtkxv9fc","timestamp":"now"},
{"desc":"raw(0014c8ccb4d81ffc769fc5fdd8d7eed69b0e0cae5749)#hn39qayv","timestamp":"now"},
{"desc":"raw(001498565aead2d67a22a6021d55210f2a917fc22169)#6ar3vwsx","timestamp":"now"},
{"desc":"raw(001403013248ac0cd9eabe176cad162cda2a19f771e1)#4m47mukd","timestamp":"now"}
]'
```
Running master, when building from souce and using brew installed
sqlite, this takes ~3.4s. With this PR, the same operation takes ~0.3s.
8df063e53712b69a346a83c599ca9cb83905b70b build: Fix help string for `--enable-external-signer` configure option (Hennadii Stepanov)
Pull request description:
This PR is a follow up of bitcoin/bitcoin#24065 and fixes the help string according to the actual default value 816ca01650/configure.ac (L324-L327)
ACKs for top commit:
kristapsk:
cr utACK 8df063e53712b69a346a83c599ca9cb83905b70b
jarolrod:
ACK 8df063e53712b69a346a83c599ca9cb83905b70b
Tree-SHA512: ad3f457a53c9238ddd8ded9efd1224e564e6cb9da8b7ff7733a11e32a7daad5c0f6c6223509218f44944a874470cb0d2447897662eaf4e78c763b30785717c50
9aeeb75cf91fdfb03206284e900887211dc76ed3 Add symlinks for hardcoded Makefiles in out of tree builds (Pablo Greco)
Pull request description:
When doing out of tree builds, some hardwired Makefiles are not symlinked, which makes it a bit more uncomfortable to run some instances of make.
There's no "real" functionality loss without this patch because the symlinked files are just for quick access to thinks in the main Makefile
ACKs for top commit:
hebasto:
ACK 9aeeb75cf91fdfb03206284e900887211dc76ed3, tested on Ubuntu 22.04.
Tree-SHA512: 656f73c387584cee34f66b3f95993267a40b915762949c7a84b73ba2ea8d37b7b5850733377110e0110ed2f7da64e6a5f9b303812080fe7815154dbb40c8a44c
