b11d35b5e2 Fixed wget call in gitian-build.py (willyk)
Pull request description:
Missing comma makes the gitian-builder script to download osslsigncode-2.0.tar.gz as osslsigncode-2.0.tar.gz-N, which makes the subsequent calls fail when building window binaries
ACKs for top commit:
laanwj:
ACK b11d35b5e2
promag:
ACK b11d35b5e2.
hebasto:
ACK b11d35b5e2, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: a74f6bec9e405fe57c2e53cc4ea0cb3546521b295478d02ae81a36306262e349e14395e46fd8106e72cc6a5bb80a88c8eff1ea38816af49c8b169c5715e84b0d
c8becb8280 depends: add ability to skip building qrencode (fanquake)
Pull request description:
Similar to other depends packages, add the ability to skip building `qrencode` by passing `NO_QR=1`. Same as #16089.
ACKs for top commit:
promag:
ACK c8becb8280.
hebasto:
ACK c8becb8280, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 86c7a87a31b1b2e65be2b79f533ce49f8b0074cf31331411cb3d32bb542d0b99e69605482ad75e4d1be5f2c8c613f17ba9ff17195a6b48f45365f5eb35df8bf9
711e0449cf ci: Remove trusty build (Hennadii Stepanov)
7f3ae22468 ci: Add CentOS 7 build (Hennadii Stepanov)
Pull request description:
Arguably, CentOS is the most conservative distro of all the popular ones. Thus, it could be a good way to check the Bitcoin Core compatibility with aged dependencies.
Currently, CentOS 7 has:
- Berkeley DB == 4.8.30
- Boost == 1.53.0
- GCC == 4.8.5
- libevent == 2.0.21 < minimum required [2.0.22](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md), but tests passed
- MiniUPnPc == 2.0
- Python == 3.6.8
- qrencode == 3.4.1
- Qt == 5.9.7
- ZeroMQ == 4.1.4
~Please note that this PR is based on the bugfix #17634.~
Also trusty build has been removed for the following reasons:
- https://github.com/bitcoin/bitcoin/issues/17628#issuecomment-559448201:
> Maybe it'd make sense to replace Ubuntu Trusty with Centos 7 as the "check ancient backward compatibililty" Travis run. It's supported until 2024, apparently.
- https://github.com/bitcoin/bitcoin/pull/17635#discussion_r354811792:
> Our travis is currently running at its limit and this doesn't seem like it is adding a lot new coverage compared to the other builds.
Close#17628
ACKs for top commit:
MarcoFalke:
ACK 711e0449cf🚠
Tree-SHA512: 614ec8394943f482a5867067f7119bffd052924a51e32ffda9a08e10c392c4a955a3539e2f8907cb65bfd9347dadf0ba62f6d1530bbc49927c347360a5a7f73c
Removes SetCrypted() and fUseCrypto as we don't need them anymore.
SetCrypted calls in LegacyScriptPubKeyMan are replaced with mapKeys.empty()
IsCrypted() is changed to just call HasEncryptionKeys()
886f1731be Key pool: Fix omitted pre-split count in GetKeyPoolSize (Andrew Chow)
386a994b85 Key pool: Change ReturnDestination interface to take address instead of key (Andrew Chow)
ba41aa4969 Key pool: Move LearnRelated and GetDestination calls (Andrew Chow)
65833a7407 Add OutputType and CPubKey parameters to KeepDestination (Andrew Chow)
9fcf8ce7ae Rename Keep/ReturnKey to Keep/ReturnDestination and remove the wrapper (Andrew Chow)
596f6460f9 Key pool: Move CanGetAddresses call (Andrew Chow)
Pull request description:
* The `pwallet->CanGetAddresses()` call in `ReserveDestination::GetReservedDestination` to `LegacyScriptPubKeyMan::GetReservedDestination` so that the sanity check results in a failure when a `ScriptPubKeyMan` individually cannot get a destination, not when any of the `ScriptPubKeyMan`s can't.
* `ScriptPubKeyMan::GetReservedDestination` is changed to return the destination so that future `ScriptPubKeyMan`s can return destinations constructed in other ways. This is implemented for `LegacyScriptPubKeyMan` by moving key-to-destination code from `CWallet` to `LegacyScriptPubKeyMan`
* In order for `ScriptPubKeyMan` to be generic and work with future `ScriptPubKeyMan`s, `ScriptPubKeyMan::ReturnDestination` is changed to take a `CTxDestination` instead of a `CPubKey`. Since `LegacyScriptPubKeyMan` still deals with keys internally, a new map `m_reserved_key_to_index` is added in order to track the keypool indexes that have been reserved.
* A bug is fixed in how the total keypool size is calculated as it was omitting `set_pre_split_keypool` which is a bug.
Split from #17261
ACKs for top commit:
ryanofsky:
Code review ACK 886f1731be. Only change is moving earlier fix to a better commit (same end result).
promag:
Code review ACK 886f1731be.
instagibbs:
code review re-ACK 886f1731be
Sjors:
Code review re-ACK 886f1731be
Tree-SHA512: f4be290759f63fdc920d5c02bd0d09acc4b06a5f053787d4afcd3c921b2e35d2bd97617fadae015da853dc189f559fb8d2c6e58d53e4cabfac9af151cd97ad19
d8daa8f371 pubkey: Assert CPubKey's ECCVerifyHandle precondition (practicalswift)
Pull request description:
Assert `CPubKey`'s `ECCVerifyHandle` precondition.
This makes it more clear for fuzzing harness writers and others that `ECCVerifyHandle` is expected to be held when interacting with `CPubKey`.
Related PR #17274.
ACKs for top commit:
sipa:
ACK d8daa8f371
Tree-SHA512: 9e74086599799dc9b5c3fb8357445b662e5bf896d826af63d6d6b6ddb616612966f3bb5de3bd3ae0e692c47de85672f64b8ab6d3a1c45899dc25ba46990b5ec7
Last update was in 2017.
Updates tinyformat to upstream commit 705e3f4e1de922069bf715746d35bd2364b1f98f.
Re-apply bitcoin core specific changes.
No changes that affect our use, as far as I can see, but this gets rid
of the gcc `-Wimplicit-fallthrough` warnings, at least.
bd44711e1b build: pass -dead_strip_dylibs to ld on macOS (fanquake)
Pull request description:
This strips some unused dylibs from bitcoin-qt.
```diff
otool -L src/qt/bitcoin-qt
/usr/lib/libSystem.B.dylib
- /System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration
/System/Library/Frameworks/IOKit.framework/Versions/A/IOKit
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation
/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit
/System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
-/System/Library/Frameworks/Security.framework/Versions/A/Security
/System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration
/System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics
/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL
-/System/Library/Frameworks/AGL.framework/Versions/A/AGL
/System/Library/Frameworks/Carbon.framework/Versions/A/Carbon
/usr/lib/libc++.1.dylib
/System/Library/Frameworks/CFNetwork.framework/Versions/A/CFNetwork
/System/Library/Frameworks/CoreText.framework/Versions/A/CoreText
/System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO
/usr/lib/libobjc.A.dylib
```
`AGL` - ObjC wrapper for OpenGL.
`DiskArbitration` - mount/unmount notifications and events.
`Security` - low level security operations, authentication services.
From `man ld`:
```
Remove dylibs that are unreachable by the entry point or exported symbols.
That is, suppresses the generation of load command commands for dylibs
which supplied no symbols during the link. This option should not be
used when linking against a dylib which is required at runtime for
some indirect reason such as the dylib has an important initializer.
```
ACKs for top commit:
theuni:
ACK bd44711e1b.
Tree-SHA512: 9592ce2966d28cb6c58e01efd401f56a4baa5dc5be5313f4fe8454632b578608be65a23c8602772049cd4655a9cb020fdd40d6622a244c301920d8c3db43f99a
1f9d5af4f1 tests: Add initialization order fiasco detection in Travis (practicalswift)
Pull request description:
Add initialization order fiasco detection in Travis :)
Context: https://github.com/bitcoin/bitcoin/pull/17670#issuecomment-562035813
This would have caught the `events_hasher` initialization order issue introduced in #17573 and fixed in #17670.
Output in case of an initialization order fiasco:
```
==7934==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x557098d79200 at pc 0x55709796b9a3 bp 0x7ffde524dc30 sp 0x7ffde524dc28
READ of size 8 at 0x557098d79200 thread T0
#0 0x55709796b9a2 in CSHA256::Finalize(unsigned char*) src/crypto/sha256.cpp:667:25
#1 0x5570978150e9 in SeedEvents(CSHA512&) src/random.cpp:462:19
#2 0x5570978145e1 in SeedSlow(CSHA512&) src/random.cpp:482:5
#3 0x5570978149a3 in SeedStartup(CSHA512&, (anonymous namespace)::RNGState&) src/random.cpp:527:5
#4 0x55709781102d in ProcRand(unsigned char*, int, RNGLevel) src/random.cpp:571:9
#5 0x557097810d19 in GetRandBytes(unsigned char*, int) src/random.cpp:576:59
#6 0x557096c2f9d5 in (anonymous namespace)::CSignatureCache::CSignatureCache() src/script/sigcache.cpp:34:9
#7 0x557096511977 in __cxx_global_var_init.7 src/script/sigcache.cpp:67:24
#8 0x5570965119f8 in _GLOBAL__sub_I_sigcache.cpp src/script/sigcache.cpp
#9 0x557097bba4ac in __libc_csu_init (src/bitcoind+0x18554ac)
#10 0x7f214b1c2b27 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:266
#11 0x5570965347d9 in _start (src/bitcoind+0x1cf7d9)
0x557098d79200 is located 96 bytes inside of global variable 'events_hasher' defined in 'random.cpp:456:16' (0x557098d791a0) of size 104
registered at:
#0 0x557096545dfd in __asan_register_globals compiler-rt/lib/asan/asan_globals.cpp:360:3
#1 0x557097817f8b in asan.module_ctor (src/bitcoind+0x14b2f8b)
SUMMARY: AddressSanitizer: initialization-order-fiasco src/crypto/sha256.cpp:667:25 in CSHA256::Finalize(unsigned char*)
```
ACKs for top commit:
promag:
Tested ACK 1f9d5af4f1, got
MarcoFalke:
ACK 1f9d5af4f1👔
Tree-SHA512: f24ac0a313df7549193bd7f4fcfdf9b72bdfc6a6ee31d0b08e6d0752e5108fbd532106b6c86377ae0641258c9adb4921872e5d9a0154c0284e03315e0777102c
8bda0960f9 Move events_hasher into RNGState() (Pieter Wuille)
Pull request description:
This moves `events_hasher` and `events_mutex` into `RNGState()` in random.cpp. This guarantees (through the existing `GetRNGState()` function) that the mutex is always created before any events are added, even when that happens inside global initializers.
Fixes the issue reported here: https://github.com/bitcoin/bitcoin/pull/17573#issuecomment-561828251, and includes the annotation from #17666).
ACKs for top commit:
MarcoFalke:
re-ACK 8bda0960f9🥈
sipsorcery:
re-ACK 8bda0960f9.
Tree-SHA512: 78702d668764df19e9d61d87d82eca71cceca87d5351b740e13e732a1c18a3d53d7fbaaf63245266da597370bfebec9fa6a4749c15ec5a78dcfe6122c33553ed
dddd09eb33 test: Wait until mempool is loaded in wallet_abandonconflict (MarcoFalke)
Pull request description:
This might or might not fix intermittent issues such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/28724018#L4091
I believe the mempool was not loaded fully after the restart, in which case it was not dumped either on the next restart. Thus, the previous mempool was attempted to be loaded a second time, which succeeded and contained the txs.
ACKs for top commit:
laanwj:
ACK dddd09eb33
Tree-SHA512: ab7061f946b5e5388f825dddceadb125f5197b24af3a7fcf1e700235d106a323419a56bfb4d84a2e27442e0de63e540c623b704343d83a98deaab3c02fcbdcbe
Currently they are identical, but in the future we might want to turn
the mempool into a unique_ptr. Replacing the global with the mempool
pointer from the node context simplifies this step.
55b2cb199c random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake)
461e547877 doc: correct random.h docs after #17270 (fanquake)
Pull request description:
The usage of `MilliSleep()` in SeedPeriodic (previously SeedSleep) was
[removed](d61f2bb076) in #17270, meaning it, and its users can now be marked `noexcept`.
This also corrects the docs in random.h for some of the changes in #17270.
ACKs for top commit:
practicalswift:
ACK 55b2cb199c
laanwj:
ACK 55b2cb199c
sipa:
ACK 55b2cb199c
Tree-SHA512: 672d369796e7c4f9b4d98dc545e5454999fa1bef373871994a26041d6163c58909e2255e4f820d3ef011679aa3392754eb57477306a89f5fd3d57e2bd7f0811a
This strips some unused dylibs from bitcoin-qt.
From man ld:
Remove dylibs that are unreachable by the entry point or exported symbols.
That is, suppresses the generation of load command commands for dylibs
which supplied no symbols during the link. This option should not be
used when linking against a dylib which is required at runtime for
some indirect reason such as the dylib has an important initializer.
fa40e48c50 ci: Remove unparseable lines from supp file for old xenial clang tsan (MarcoFalke)
fa1bfc476c ci: ubsan report_error_type=1 and add suppressions (MarcoFalke)
fa69cef13e test: Print stderr when subprocess fails (MarcoFalke)
2222c30586 test: Use char instead of unsigned char (MarcoFalke)
faa8023ce9 ci: Bump to clang-8 for asan build to avoid segfaults on ppc64le (MarcoFalke)
Pull request description:
Use clang-8 instead of default clang (which is clang-6 on Bionic) to avoid spurious segfaults when running the ci system on ppc64le
ACKs for top commit:
practicalswift:
ACK fa40e48c50 assuming Travis is happy -- diff looks correct :)
Tree-SHA512: f4f26232d3a0ef38da245869340f723d279a3db9823befbc735fb5a00024dae041c7306d7ae55d2488e6f86aa96cdea155b007aefb561fba505141e8dbc717dc
