d6fae988ef guix: fix vmov alignment issues with gcc 10.3.0 & mingw-w64 (fanquake)
Pull request description:
This introduces a patch to our GCC (10.3.0) mingw-w64 compiler, in Guix, to make
it avoid using aligned vmov instructions. This works around a longstanding issue
in GCC, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54412, which was recently
discovered to be causing issues, see #24726.
Note that distros like Debian are also patching around this issue, and that is
where this patch comes from. This would also explain why we haven't run into this
problem earlier, in development builds. See:
https://salsa.debian.org/mingw-w64-team/gcc-mingw-w64/-/blob/master/debian/patches/vmov-alignment.patch.
Fixes#24726.
Alternative to #24727.
See also:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=939559
ACKs for top commit:
laanwj:
Concept and code review ACK d6fae988ef
hebasto:
ACK d6fae988ef, tested Guix ` bitcoin-d6fae988eff7-win64.zip` artifact on Windows 11 Pro 21H2:
Tree-SHA512: f522efd8e604ab1d9f9c385147f6f488767cfe66f08a1c8b4ff67d448e065f8f2334bf825d99e7fe9571ada9038002b08434585f639120cb29b2e314da7b556e
When no build dir is specified, cmake will warn:
```bash
Preprocessing libmultiprocess...
Configuring libmultiprocess...
CMake Warning:
No source or binary directory provided. Both will be assumed to be the
same as the current working directory, but note that this warning will
become a fatal error in future CMake releases.
```
It's unclear if this will actually ever become an error, but it's also easy
enough to just supply the directory, and save this maybe breaking in
future.
7f6042849c build, qt: use one patch per line in depends/packages/qt.mk (Pavol Rusnak)
826cbc470f build, qt: drop fix_no_printer.patch (Hennadii Stepanov)
ef20add4c9 build, qt: bump Qt5 version to 5.15.3 (Pavol Rusnak)
Pull request description:
build, qt: bump Qt5 version to 5.15.3
Qt 5.15.3 release is a patch release made on the top of Qt 5.15.2. As a patch
release, Qt 5.15.3 does not add any new functionality but provides bug fixes
and other improvements.
https://code.qt.io/cgit/qt/qtreleasenotes.git/about/qt/5.15.3/release-note.md
* dropped patches:
- patches/qt/dont_use_avx_android_x86_64.patch
- patches/qt/fix_bigsur_style.patch
* adjusted patches:
- patches/qt/fix_android_jni_static.patch
- patches/qt/fix_limits_header.patch
- patches/qt/use_android_ndk23.patch
ACKs for top commit:
hebasto:
ACK 7f6042849c
fanquake:
ACK 7f6042849c
Tree-SHA512: dd79475901bc9636fb0ce2424f63ddfe8ab5f85f7f35ac64b0e8708042793c19663be1abdcaef6be95e30bae8aa9e6da4389d768de5c102ded8ab61b3d02b07b
37a16ffd70 refactor: fix clang-tidy named args usage (fanquake)
Pull request description:
> Incorrect named args are source of bugs, like https://github.com/bitcoin/bitcoin/pull/22979.
> To allow them being checked by clang-tidy, use a format it can understand.
Picks up #23545, with some additional changes and some feedback addressed.
With these changes invoking `./autogen.sh && ./configure CC=clang-12 CXX=clang++-12 && make clean && bear make -j9 && ( cd ./src/ && run-clang-tidy-12 -j9 )` no-longer results in named argument errors out of `clang-tidy`.
Ultimately I think we should just add `clang-tidy-*` jobs to the CI and automate things away.
ACKs for top commit:
MarcoFalke:
cr ACK 37a16ffd70
Tree-SHA512: 9bfc0d006eb187755b4fdb0bd92cee9266fc0816be42065ef7dcd885b9020ff12e3cdd7ca3a831613a56a0206d448e690ee4e1fa37628fa2013860e17f416ff3
0d01272cd8 build: don't use Boost multi_index serialization (fanquake)
Pull request description:
We don't use the serialization or archiving facilities of multi_index.
So globally disable support, which gives a minor improvement in build
time, i.e less preprocessing work, given we don't link any Boost libs.
See: https://www.boost.org/doc/libs/1_78_0/libs/multi_index/doc/tutorial/creation.html
> Serialization capabilities are automatically provided by just linking with the appropriate Boost.Serialization library module: it is not necessary to explicitly include any header from Boost.Serialization, apart from those declaring the type of archive used in the process. If not used, however, serialization support can be disabled by globally defining the macro BOOST_MULTI_INDEX_DISABLE_SERIALIZATION. Disabling serialization for Boost.MultiIndex can yield a small improvement in build times, and may be necessary in those defective compilers that fail to correctly process Boost.Serialization headers.
ACKs for top commit:
MarcoFalke:
cr ACK 0d01272cd8
Tree-SHA512: 87c664a2f142dc6b8f8598341f9829be3fda8cf614d73cc9a894c8033ee40c6daa9b50f4049ecb1f1e3aaf342568d9a5f5c65af1e04c36ee3a9cb46eca95767b
This was added to support compilation on macOS 10.10, our minimum
required macOS is now 10.15. macOS has also supported it since 10.11.
See https://github.com/bitcoin/bitcoin/pull/9063.
0c64401324 Revert "qt: Do not use QObject::tr plural syntax for numbers with a unit symbol" (Luke Dashjr)
Pull request description:
Apparently this got forgotten. Maybe too late for 23.x (it's a bugfix, but changes translation strings).
This reverts commit 3adde72bc9 (#296)
per [GChuf](https://github.com/bitcoin-core/gui/pull/296#issuecomment-962516055)
>I can confirm for slovenian and other slavic languages that we do have 3 or 4 different ways of saying "%n GB needed%, depending on the actual number of gigabytes. Similar to english "is/are". There's no way to cover all cases ... this is exactly why transifex allows you to have more than 2 options.
ACKs for top commit:
hebasto:
ACK 0c64401324, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: c01bae44a32b3ec324f2f9b8e4923bbb2e83bbd1460b745c5c911b98a9b2806fcbf815cfb19a1f1a7038c5c14312e102e7df8744c9002ef784b36d158e08eb14
3d70c05868 build: remove faketime unsetting and comments from configure.ac (fanquake)
Pull request description:
We no-longer use [`faketime`](https://github.com/wolfcw/libfaketime) (it used to be required in gitian), so as far as I'm aware, there is no need for us to unset `FAKETIME` or mention it in our build docs.
ACKs for top commit:
laanwj:
Code review ACK 3d70c05868
prusnak:
Approach ACK 3d70c05
hebasto:
ACK 3d70c05868, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 9cf89d63b81119f3d2f02975a66ec0b93e861993fdb0e4f70538e3be6e0047dc09ce87ef2de40cbf877647a21706b39ddf07240c77765278d383d7a7878cc7eb
The removed code was intended to catch issues with event_enable_debug_logging which was not available prior to libevent 2.1.1. This is not necessary since the minimum libevent version was bumped to 2.1.8.
e8fc236da7 refactor: add missing std:: includes to threadnames.cpp (fanquake)
87f3c04cc5 doc: remove incorrect mention of PR_GET_NAME (fanquake)
Pull request description:
By removing the whole comment. These `#include // For` comments are near impossible
to maintain, pollute diffs, and generally don't add a lot of value.
While here, also add the missing `std::` includes.
ACKs for top commit:
junderw:
LGTM ACK e8fc236
Tree-SHA512: d29aff40c94f59c42f295a5738bc5ff2f4a2f2e6d270cc505f27d56d07d272597e2f8403d72fe45775661e1a1fc2af9fc52aeaeb41263bd3e9dfe255332383c8
Qt 5.15.3 release is a patch release made on the top of Qt 5.15.2. As a patch
release, Qt 5.15.3 does not add any new functionality but provides bug fixes
and other improvements.
https://code.qt.io/cgit/qt/qtreleasenotes.git/about/qt/5.15.3/release-note.md
* dropped patches:
- patches/qt/dont_use_avx_android_x86_64.patch
- patches/qt/fix_bigsur_style.patch
* adjusted patches:
- patches/qt/fix_android_jni_static.patch
- patches/qt/fix_limits_header.patch
- patches/qt/use_android_ndk23.patch
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
7762c5683f build: Fix "ERR: Unsigned tarballs do not exist" (Hennadii Stepanov)
Pull request description:
This was missed in 53dd6165b8 (bitcoin/bitcoin#24549).
ACKs for top commit:
laanwj:
ACK 7762c5683f
fanquake:
ACK 7762c5683f - this should have been a part of #24549.
Tree-SHA512: 526e4e6ee1463003059b9703030c5b0537445783c45a205024c9c9e3493da967733153a1dc80594fed15a336805c1cf14c8d13cd106186a7544f51d0d542f335
bf77fea3c1 test: fix incorrect named args in txpackage tests (fanquake)
Pull request description:
Final non-scripted-diff commit split from #24661.
Could be tested with: `./autogen.sh && ./configure CC=clang-12 CXX=clang++-12 && make clean && bear make -j9 && ( cd ./src/ && run-clang-tidy-12 -j9 )`.
Motivation:
> Incorrect named args are source of bugs, like https://github.com/bitcoin/bitcoin/pull/22979.
> To allow them being checked by clang-tidy, use a format it can understand.
ACKs for top commit:
ajtowns:
ACK bf77fea3c1
Tree-SHA512: a13bfb5fc70424b13fbeec7f164d7a0d3b72b27ebec11dfd4115b7782a0037f26e9349e06eef8a6b17b8f529e0c7f43ae37a9c252bde65706dd164704d207d5f
This is currently causing the same failure in two different PRs:
```bash
duplicate symbol 'lcQpaFonts()' in:
/tmp/cirrus-ci-build/depends/x86_64-apple-darwin/lib/libQt5ThemeSupport.a(qgenericunixthemes.o)
/tmp/cirrus-ci-build/depends/x86_64-apple-darwin/lib/libQt5FontDatabaseSupport.a(qfontengine_coretext.o)
ld: 1 duplicate symbol for architecture x86_64
```
```bash
x86_64-apple-darwin-ld: error: duplicate symbol: __Z10lcQpaFontsv
>>> defined in /tmp/cirrus-ci-build/depends/x86_64-apple-darwin/lib/libQt5ThemeSupport.a(qgenericunixthemes.o)
>>> defined in /tmp/cirrus-ci-build/depends/x86_64-apple-darwin/lib/libQt5FontDatabaseSupport.a(qfontengine_coretext.o)
```
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
21520b9551 fuzz: add target for coinselection (Martin Zumsande)
Pull request description:
This adds a fuzz target for the coinselection algorithms by creating random `OutputGroup`s and running all three coin selection algorithms for them.
It does not fuzz higher-level wallet logic for selecting eligible coins (as in `SelectCoins()`), thought it probably would make sense to have a fuzz target for that too.
ACKs for top commit:
achow101:
ACK 21520b9551
vasild:
ACK 21520b9551
Tree-SHA512: c763003cf5ff5317f929d3d0b2f06fa739ae41dd642042d9a5c5c96e6cb9b349a6c7aeabc77bc2b846d12c8bcb60e07ee20a9f38539429c65723ab76aeee6b2e
0c12f0116c wallet: Postpone NotifyWalletLoaded() for encrypted wallets (Hennadii Stepanov)
aeee419c6a wallet, refactor: Add wallet::NotifyWalletLoaded() function (Hennadii Stepanov)
Pull request description:
Fixesbitcoin-core/gui#571.
`CWallet::Create()` notifies about wallet loading too early, that results the notification goes before `DescriptorScriptPubKeyMan`s were created and added to an encrypted wallet.
And `interfaces::Wallet::taprootEnabled()` in ecf692b466/src/qt/receivecoinsdialog.cpp (L100-L102) erroneously returns `false` for just created encrypted descriptor wallets.
ACKs for top commit:
Sjors:
utACK 0c12f0116c
achow101:
ACK 0c12f0116c
Tree-SHA512: 2694bacd12748cd5f6c95d9d3bf8bcf4502ee67fecd8d057f33236b72069c61401b08f49deb013fc71c3f1e51ae16bdfd827ddcbc2a083d7044589be7a78982e
71038a151e rpc: Fix documentation assertion for `getrawtransaction` (laanwj)
Pull request description:
When `getrawtransaction` is successfully used on a coinbase transaction, there is an assertion error. This is very unlikely but happens in the `interface_usdt_utxocache.py` test in #24358.
This does the following:
- Add missing "coinbase" documentation.
- Synchronize documentation between `getrawtransaction` and `decoderawtransaction`, the two users of `TxToUniv` that have detailed documentation. `decodepsbt` and `getblock` also uses it but fortunately elides this block.
- Change "vout[].amount" to `STR_AMOUNT` for consistency.
- Add maintainer comment to keep the two places synchronized. It might be possible to get smarter with deduplication, but there are some extra fields that prevent the obvious way.
ACKs for top commit:
jonatack:
ACK 71038a151e
Tree-SHA512: 962236130455d805190ff9a5c971e4e25c17db35614a90ce340264ec953b0ad7fb814eb33ae430b5073955a8a350f72bdd67ba93e35f9c70e5175b836a767a35
d6bc2322ed test: -peerblockfilters without -blockfilterindex raises an error (brunoerg)
Pull request description:
This PR adds test coverage for the following init error:
2a3e8fb359/src/init.cpp (L850)
Setting -peerblockfilters without -blockfilterindex should raise an error when initializing.
ACKs for top commit:
ccdle12:
Tested ACK d6bc2322ed
Tree-SHA512: e740c2ccde6bb1bb8381bb676a6d01bd5746cf9ce0c8dadd62067a6b9b380027bfe8b8cdeae9846a0ab18385f3dc5dff607fe5274cb55107d47470db00015fb2
17648493df doc: Speed up functional test runs using ramdisk (willcl-ark)
Pull request description:
Using a ramdisk for the functional tests can give noticable speedups for developers and reviewers.
Local testing with an 8GB ramdisk saw a full test run using `test/functional/test_runner.py --jobs=100 --cachedir=/mnt/tmp/cache --tmpdir=/mnt/tmp` reduced from ~280 seconds to ~99 seconds.
Possible bikeshedding opportunity to be had over whether this might best fit into `doc/productivity.md`, but IMO more people will likely see it (and it will therefore be more useful) if it is here.
It seems best to select `tmpfs` over `ramfs` as `ramfs` can grow dynamically (good) but cannot be limited in size and might cause the system to hang if you run out of ram (bad), whereas `tmpfs` is size-limited and will overflow into swap.
ACKs for top commit:
josibake:
ACK 17648493df
jamesob:
ACK 17648493df
Tree-SHA512: b8e0846d4558a7a33fbb7cd190e30c36182db36095e1c1feae8c10a12042cff9d97739964bd9211d8564231dc99b4be5eed806d12a1d11dfa908157d7f26cc67
9563a645c2 refactor: add stdd:: includes to core_write (fanquake)
8b9efebb0a refactor: use named args when ScriptToUniv or TxToUniv are invoked (Michael Dietz)
22f25a6116 refactor: prefer snake case, TxToUniv arg hashBlock renamed block_hash (Michael Dietz)
828a094ecf refactor: merge ScriptPubKeyToUniv & ScriptToUniv into one function (Michael Dietz)
Pull request description:
I've cherry-picked some of the commits out of #22924, and made minor changes (like fixing named args).
ACKs for top commit:
MarcoFalke:
re-ACK 9563a645c2🕓
Tree-SHA512: 4f0e5b45c14cbf68b9e389bbe1211c125d95cbd3da5205b1cff6a4c44f15b15039ba2a5b25cd7e2580d9169404f1b7ff620d8a7e01f6112e3cb153ecfaef8916
2ef47ba6c5 util/check: stop using lambda for Assert/Assume (Anthony Towns)
7c9fe25c16 wallet: move Assert() check into constructor (Anthony Towns)
Pull request description:
Using a lambda creates a couple of odd namespacing issues, in particular making clang's thread safety analysis less helpful, and confusing gcc when calling member functions. Fix this by not using a lambda.
Fixes#21596Fixes#24654
ACKs for top commit:
MarcoFalke:
ACK 2ef47ba6c5🚢
jonatack:
Tested re-ACK 2ef47ba6c5
Tree-SHA512: 4bdbf3215f3d14472df0552362c5eebe8b7eea2d0928a8a41109edd4e0c5f95de6f8220eb2fee8506874e352c003907faf5ef344174795939306a618157b1bae
