b919efadff depends: Use default macos clang compiler (Russell Yanofsky)
d54f64c6c7 Add multiprocess travis configuration (Russell Yanofsky)
787f40668d Set LD_LIBRARY_PATH consistently in travis tests (Russell Yanofsky)
d630646662 libmultiprocess depends build (Russell Yanofsky)
e6e44eedd5 Multiprocess build changes (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
This splits autotools, depends build, and travis changes out of #10102, so code changes and build system changes can be reviewed separately.
ACKs for top commit:
hebasto:
re-ACK b919efadff, rebased only since my [previous](https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-605514556) review.
Tree-SHA512: ebc5e403cc99a0d9629ed7fe1595e01d57e6d1255cbf03968a3196ff6f528f734c78060fdc065724ee1f923bcc5aa2b29470fcb36a7f15957eb57c76d58178a4
01a3392b1b Drop bitcoin-wallet dependency on libevent (Russell Yanofsky)
0660119ac3 Drop unintended bitcoin-tx dependency on libevent (Russell Yanofsky)
Pull request description:
This fixes compile errors trying to build bitcoin-tx and bitcoin-wallet without libevent, which were reported by Luke Dashjr in https://github.com/bitcoin/bitcoin/issues/18465
The fix avoiding `bitcoin-tx` dependency on libevent just adds a conditional build rule. This is implemented in the first commit (more details in commit description).
The fix avoiding `bitcoin-wallet` dependency on libevent requires minor code changes, because `bitcoin-wallet` (unlike `bitcoin-tx`) links against code that calls `urlDecode` / `evhttp_uridecode`. This fix is implemented in the second commit (again details in the commit description).
ACKs for top commit:
jonasschnelli:
utACK 01a3392b1b.
Tree-SHA512: d2245e912ab494cccceeb427a1eca8e55b01a0006ff93eebcfb5461ae7cecd1083ac2de443d9db036b18bdc6f0fb615546caaa20c585046f66d234937f74870a
5df0877f91 test: update and harden interface_bitcoin_cli tests (Jon Atack)
75019774c9 cli -getinfo: use getbalances instead of deprecated getwalletinfo balance (Jon Atack)
Pull request description:
Extracted from #18453 to preserve that PR as a discussion on multiwallet RPC/CLI.
This PR updates `bitcoin-cli -getinfo` to fetch the wallet balance from `getbalances` in order to no longer depend on `getwalletinfo.balance` which was deprecated a year ago in facfb41.
I found this when removing the getwalletinfo() `balance`, `unconfirmed_balance`, and `immature_balance` fields to see what broke from depending on them.
I didn't see any perceivable change in `-getinfo` run time from the change.
Test coverage for this change is provided by `test/functional/interface_bitcoin_cli.py`, which the second commit updates to (a) no longer depend on getwalletinfo.balances and (b) test the -getinfo blockcount and balance fields against non-default, non-zero values.
ACKs for top commit:
robot-visions:
ACK 5df0877
MarcoFalke:
ACK 5df0877
vasild:
re-ACK 5df0877f9
promag:
Code review ACK 5df0877f91.
theStack:
ACK 5df0877f91
Tree-SHA512: 0dd8c62f915b1c0112e42b132dcf74a141bdd1f51e7c17d4a698b374ec296f4f9836f7058dbe237cf24f9bfb32ea5000e14f7089e2e86472d9c6a175be26e910
Now that 0.20 branch has been split off, master is 0.20.99 (pre-0.21).
Also clean out release notes.
Tree-SHA512: bba6133ae9708f75206c8934901b9f9909a233330f4dfefb3c24175bf8e11631cdc89a5d24a22421a73083f7eb743e977db8020b87dfbd3c1e6043929a19a285
fad691cafe rpc: Make verifychain default values static, not depend on global args (MarcoFalke)
Pull request description:
This fixes several issues:
* The documentation is not compile-time static and depends on run-time arguments, making it impossible to host it on a static resource like a website or pdf. See also a similar change in the wallet rpc code: #18499
* The same call (relying on default values) will run different code on different machines, depending on the command line args that were used to start the server. This might lead to hard-to-debug-remote issues.
This is a small behaviour change, and I will add release notes.
ACKs for top commit:
theStack:
ACK fad691cafe
promag:
Code review ACK fad691cafe.
Tree-SHA512: 1c7a253ff0ec13a973b10d3777b71c70954ded5805b65a3ab06317327014de4cd0601d71d30c6ce89a581722c150cb5567acc1bd3e0c789cb51bab6ef0dcfc4a
7fcdec0f32 Remove PID file at the very end (Hennadii Stepanov)
Pull request description:
While reproducing the bug from #18517, I've noticed that the `bitcoind.pid` file has already been removed when the `bitcoind` hangs.
This PR makes `Shutdown()` keep the `bitcoind.pid` file available until the end.
ACKs for top commit:
MarcoFalke:
ACK 7fcdec0f32
emilengler:
utACK 7fcdec0f32
promag:
Code review ACK 7fcdec0f32.
theStack:
Code review ACK 7fcdec0f32
Tree-SHA512: 9732ef34e137dbee70a06d922b316b8ea7b9a1c959cf8861b6940cd789336dc19ee468a4c3a28d95d1458076a48270c676b0ff27fec30cf57eced6ddab0a2a9b
fa1da3d4bf test: Add basic addr relay test (MarcoFalke)
fa1793c1c4 net: Pass connman const when relaying address (MarcoFalke)
fa47a0b003 net: Make addr relay mockable (MarcoFalke)
Pull request description:
As usual:
* Switch to std::chrono time to be type-safe and mockable
* Add basic test that relies on mocktime to add code coverage
ACKs for top commit:
naumenkogs:
utACK fa1da3d
promag:
ACK fa1da3d4bf (fabe56e44b6f683e24e37246a7a8851190947cb3 before https://github.com/bitcoin/bitcoin/pull/18454#issuecomment-607866453), fa5bf23d527a450e72c2bf13d013e5393b664ca3 was dropped since last review.
Tree-SHA512: 0552bf8fcbe375baa3cab62acd8c23b2994efa47daff818ad1116d0ffaa0b9e520dc1bca2bbc68369b25584e85e54861fe6fd0968de4f503b95439c099df9bd7
478c11dde3 Correct scripted-diff example link (Yahia Chiheb)
Pull request description:
ACKs for top commit:
fanquake:
ACK 478c11dde3
Tree-SHA512: 3bc741a79db9bd7abb17ef11f697b768565ec01303a5823ee6a7d8dfa6e888a99a15e9eda69f97a912abc3fd56a54f698f9a580596511bc9bcf62a6870b273f6
5ca90f8b59 scripts: add MACHO lazy bindings check to security-check.py (fanquake)
Pull request description:
This is a slightly belated follow up to #17686 and some discussion with Cory. It's not entirely clear if we should make this change due to the way the macOS dynamic loader appears to work. However I'm opening this for some discussion. Also related to #17768.
#### Issue:
[`LD64`](https://opensource.apple.com/source/ld64/) doesn't set the [MH_BINDATLOAD](https://opensource.apple.com/source/xnu/xnu-6153.11.26/EXTERNAL_HEADERS/mach-o/loader.h.auto.html) bit in the header of MACHO executables, when building with `-bind_at_load`. This is in contradiction to the [documentation](https://opensource.apple.com/source/ld64/ld64-450.3/doc/man/man1/ld.1.auto.html):
```bash
-bind_at_load
Sets a bit in the mach header of the resulting binary which tells dyld to
bind all symbols when the binary is loaded, rather than lazily.
```
The [`ld` in Apples cctools](https://opensource.apple.com/source/cctools/cctools-927.0.2/ld/layout.c.auto.html) does set the bit, however the [cctools-port](https://github.com/tpoechtrager/cctools-port/) that we use for release builds, bundles `LD64`.
However; even if the linker hasn't set that bit, the dynamic loader ([`dyld`](https://opensource.apple.com/source/dyld/)) doesn't seem to ever check for it, and from what I understand, it looks at a different part of the header when determining whether to lazily load symbols.
Note that our release binaries are currently working as expected, and no lazy loading occurs.
#### Example:
Using a small program, we can observe the behaviour of the dynamic loader.
Conducted using:
```bash
clang++ --version
Apple clang version 11.0.0 (clang-1100.0.33.17)
Target: x86_64-apple-darwin18.7.0
ld -v
@(#)PROGRAM:ld PROJECT:ld64-530
BUILD 18:57:17 Dec 13 2019
LTO support using: LLVM version 11.0.0, (clang-1100.0.33.17) (static support for 23, runtime is 23)
TAPI support using: Apple TAPI version 11.0.0 (tapi-1100.0.11)
```
```cpp
#include <iostream>
int main() {
std::cout << "Hello World!\n";
return 0;
}
```
Compile and check the MACHO header:
```bash
clang++ test.cpp -o test
otool -vh test
...
Mach header
magic cputype cpusubtype caps filetype ncmds sizeofcmds flags
MH_MAGIC_64 X86_64 ALL LIB64 EXECUTE 16 1424 NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE
# Run and dump dynamic loader bindings:
DYLD_PRINT_BINDINGS=1 DYLD_PRINT_TO_FILE=no_bind.txt ./test
Hello World!
```
Recompile with `-bind_at_load`. Note still no `BINDATLOAD` flag:
```bash
clang++ test.cpp -o test -Wl,-bind_at_load
otool -vh test
Mach header
magic cputype cpusubtype caps filetype ncmds sizeofcmds flags
MH_MAGIC_64 X86_64 ALL LIB64 EXECUTE 16 1424 NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE
...
DYLD_PRINT_BINDINGS=1 DYLD_PRINT_TO_FILE=bind.txt ./test
Hello World!
```
If we diff the outputs, you can see that `dyld` doesn't perform any lazy bindings when the binary is compiled with `-bind_at_load`, even if the `BINDATLOAD` flag is not set:
```diff
@@ -1,11 +1,27 @@
+dyld: bind: test:0x103EDF030 = libc++.1.dylib:__ZNKSt3__16locale9use_facetERNS0_2idE, *0x103EDF030 = 0x7FFF70C9FA58
+dyld: bind: test:0x103EDF038 = libc++.1.dylib:__ZNKSt3__18ios_base6getlocEv, *0x103EDF038 = 0x7FFF70CA12C2
+dyld: bind: test:0x103EDF068 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryC1ERS3_, *0x103EDF068 = 0x7FFF70CA12B6
+dyld: bind: test:0x103EDF070 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryD1Ev, *0x103EDF070 = 0x7FFF70CA1528
+dyld: bind: test:0x103EDF080 = libc++.1.dylib:__ZNSt3__16localeD1Ev, *0x103EDF080 = 0x7FFF70C9FAE6
<trim>
-dyld: lazy bind: test:0x10D4AC0C8 = libsystem_platform.dylib:_strlen, *0x10D4AC0C8 = 0x7FFF73C5C6E0
-dyld: lazy bind: test:0x10D4AC068 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryC1ERS3_, *0x10D4AC068 = 0x7FFF70CA12B6
-dyld: lazy bind: test:0x10D4AC038 = libc++.1.dylib:__ZNKSt3__18ios_base6getlocEv, *0x10D4AC038 = 0x7FFF70CA12C2
-dyld: lazy bind: test:0x10D4AC030 = libc++.1.dylib:__ZNKSt3__16locale9use_facetERNS0_2idE, *0x10D4AC030 = 0x7FFF70C9FA58
-dyld: lazy bind: test:0x10D4AC080 = libc++.1.dylib:__ZNSt3__16localeD1Ev, *0x10D4AC080 = 0x7FFF70C9FAE6
-dyld: lazy bind: test:0x10D4AC070 = libc++.1.dylib:__ZNSt3__113basic_ostreamIcNS_11char_traitsIcEEE6sentryD1Ev, *0x10D4AC070 = 0x7FFF70CA1528
```
Note: `dyld` also has a `DYLD_BIND_AT_LAUNCH=1` environment variable, that when set, will force any lazy bindings to be non-lazy:
```bash
dyld: forced lazy bind: test:0x10BEC8068 = libc++.1.dylib:__ZNSt3__113basic_ostream
```
#### Thoughts:
After looking at the dyld source, I can't find any checks for `MH_BINDATLOAD`. You can see the flags it does check for, such as MH_PIE or MH_BIND_TO_WEAK [here](https://opensource.apple.com/source/dyld/dyld-732.8/src/ImageLoaderMachO.cpp.auto.html).
It seems that the lazy binding of any symbols depends on whether or not [lazy_bind_size](https://opensource.apple.com/source/xnu/xnu-6153.11.26/EXTERNAL_HEADERS/mach-o/loader.h.auto.html) from the `LC_DYLD_INFO_ONLY` load command is > 0. Which was mentioned in [#17686](https://github.com/bitcoin/bitcoin/pull/17686#issue-350216254).
#### Changes:
This PR is one of [Corys commits](7b6ba26178), that I've rebased and modified to make build. I've also included an addition to the `security-check.py` script to check for the flag.
However, given the above, I'm not entirely sure this patch is the correct approach. If the linker no-longer inserts it, and the dynamic loader doesn't look for it, there might be little benefit to setting it. Or, maybe this is an oversight from Apple and needs some upstream discussion. Looking for some thoughts / Concept ACK/NACK.
One alternate approach we could take is to drop the patch and modify security-check.py to look for `lazy_bind_size` == 0 in the `LC_DYLD_INFO_ONLY` load command, using `otool -l`.
ACKs for top commit:
theuni:
ACK 5ca90f8b59
Tree-SHA512: 444022ea9d19ed74dd06dc2ab3857a9c23fbc2f6475364e8552d761b712d684b3a7114d144f20de42328d1a99403b48667ba96885121392affb2e05b834b6e1c
b1d24d1d03 Reorder the test instructions by number (Pieter Wuille)
c2ccadc26a Merge and generalize case 3 and case 6 (Pieter Wuille)
402ad5aaca Only run sanity check once at the end (Pieter Wuille)
eda8309bfc Assert immediately rather than caching failure (Pieter Wuille)
55608455cb Make a fuzzer-based copy of the prevector randomized test (Pieter Wuille)
Pull request description:
The current prevector test effectively randomly generates a number of operations to perform on a prevector and a normal vector, and checks consistency between the two.
By converting this into a fuzzer the operations can be targetted rather than random.
ACKs for top commit:
MarcoFalke:
ACK b1d24d1d03🍬
Tree-SHA512: 2b5c62abcd5fee94f42db03400531484d98c59e7f4308e0e683c61aabcd9ce42f85c5d058d2d5e7f8221124f71d2112b6a5f3c80e5d0fdae265a70647747e92f
faede1b293 test: Properly raise FailedToStartError when rpc shutdown before warmup finished (MarcoFalke)
Pull request description:
Should fix issues such as https://travis-ci.org/github/bitcoin/bitcoin/jobs/671910152#L7034
Top commit has no ACKs.
Tree-SHA512: ac659f29c5ec91985c916b734e24911cbf4e2c5c4b1f1891a7e6c2d2511ec285167550fb03848eee4a7a3cbc9f8cdb0c766f4e881d9e44368c7415d007006368
6136a96cdf ci: Rename RUN_CI_ON_HOST to DANGER_RUN_CI_ON_HOST (Hennadii Stepanov)
97ba77aa8e ci: Add native s390x (Hennadii Stepanov)
Pull request description:
Unlike the Docker wrapped solution (#17591) this PR suggests running on host system directly.
This approach makes builds quick and stable (see: #18106).
The excerpt from the Travis log:
```
...
Running on host system without docker wrapper
...
Byte Order: Big Endian
...
```
ACKs for top commit:
MarcoFalke:
ACK 6136a96cdf
Tree-SHA512: 1b591de13e38d10a35217e1de11cbd648a359d18d16eed166fac18ea5788b58cc9fc6d407086ed342b99e57e479efd951a0ea693710177e500eb116316b9a788
fa7af33b4c ci: Run unit tests sequential once (MarcoFalke)
fa68a3e764 appveyor: Enable minimal unit test logging to aid debugging (MarcoFalke)
Pull request description:
Fixes#16976
Top commit has no ACKs.
Tree-SHA512: 1f1ee8776a67afa8c1c5a16ef170c9975b6a486c087c8eba12e97d23382befd1c2801622ec70ca8e4cd1fbedce1dec46be67677ceaf07f35f1d3f3bead0200f0
cdfb8e7afa tests: Add fuzzing harness for HTTPRequest, libevent's evhttp and related functions (practicalswift)
Pull request description:
Add fuzzing harness for `HTTPRequest`, `libevent`'s `evhttp` and related functions.
ACKs for top commit:
laanwj:
ACK cdfb8e7afa
Tree-SHA512: da481afed5eb3232d3f3d0583094e56050e6234223dfcb356d8567fe0616336eb1b78c5e6821325fc9767e385e5dfaf3c96f0d35ffdb67f18d74f9a9a9464e24
7777e3624f scripted-diff: Replace strCommand with msg_type (MarcoFalke)
Pull request description:
Receiving a message is not a command, but simply a message of some type
ACKs for top commit:
promag:
ACK 7777e3624f.
naumenkogs:
ACK 7777e36
practicalswift:
ACK 7777e3624f -- I've always thought the `strCommand` name is confusing :)
theStack:
ACK 7777e36
Tree-SHA512: 662bac579064c621191916274314b85111cfb4df488f00893ceb16def1c47af4b2a0f34cd7349722099b5a9d23160edb8eb999841f1d64af3e0da02e4870b4bf
283bd72156 tests: Add coverage of {,Incremental}DynamicUsage(const std::set<X, Y>& s) to existing fuzzer (practicalswift)
bf76000493 tests: Add fuzzing harness for classes/functions in cuckoocache.h (practicalswift)
57890b2555 tests: Add fuzzing harness for classes/functions in checkqueue.h (practicalswift)
2df5701e90 tests: Add coverage of GetVirtualTransactionSize(...) to existing fuzzer (practicalswift)
7b9a2dc864 tests: Add fuzzing harness for AdditionOverflow(...) (practicalswift)
44fb2a596b tests: Add fuzzing harness for FeeFilterRounder (practicalswift)
Pull request description:
Includes:
```
tests: Add fuzzing harness for FeeFilterRounder
tests: Add fuzzing harness for classes/functions in checkqueue.h
tests: Add fuzzing harness for classes/functions in cuckoocache.h
tests: Add coverage of {,Incremental}DynamicUsage(const std::set<X, Y>& s) to existing fuzzer
tests: Add coverage of GetVirtualTransactionSize(...) to existing fuzzer
tests: Add fuzzing harness for AdditionOverflow(...)
```
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core.
ACKs for top commit:
MarcoFalke:
ACK 283bd72156
Tree-SHA512: 2361edfb5c47741b22d9fb996836c5250c5a26bc5e956039ea6a0c55ba2d36c78f241d66f85bc02f5b85b9b83d5fde56a5c4702b9d1b7ac4a9a3ae391ca79eaa
13d2a33537 Fix unregister_all_during_call cleanup (Russell Yanofsky)
Pull request description:
Use `TestingSetup` fixture to fix `unregister_all_during_call` test not calling `UnregisterBackgroundSignalScheduler`, which could trigger an assert in `RegisterBackgroundSignalScheduler` when called in later tests
Failure reported by fanquake https://github.com/bitcoin/bitcoin/pull/18551#issuecomment-610974251
ACKs for top commit:
MarcoFalke:
ACK 13d2a33537 if appveyor unit tests pass
Tree-SHA512: d2ec8ff14c54d97903af50031abfac1f38ec1c3aabc90371cfd5b79481fa69d3d77f339bfdf7d2178fd85e83402f72eda7cf4d339e5bbfa7e6e1a68836643b93
da0842dcd4 build: Update ax_boost_mase.m4 to the latest serial (Hennadii Stepanov)
Pull request description:
Picked from the upstream 90814f1895Fix#17010.
This PR is [alternative](https://github.com/bitcoin/bitcoin/issues/17010#issuecomment-610651736) to #18501.
ACKs for top commit:
laanwj:
ACK da0842dcd4
Tree-SHA512: 5e43e12c524e4ea6b967c9be02c81a75948eac6cf55b819e3339222a2e3414731581d40af3524ad865abae7c5247c190448ebf2aa5e0d9a338edb501cc23ba38
Use TestingSetup fixture to fix unregister_all_during_call test not calling
UnregisterBackgroundSignalScheduler, which could trigger an assert in
RegisterBackgroundSignalScheduler when called in later tests
Failure reported by fanquake <fanquake@gmail.com>
https://github.com/bitcoin/bitcoin/pull/18551#issuecomment-610974251
2276339a17 Add test for UnregisterAllValidationInterfaces bug (Russell Yanofsky)
3c61abbbc8 Do not clear validationinterface entries being executed (Pieter Wuille)
Pull request description:
The previous code for MainSignalsInstance::Clear would decrement the reference
count of every interface, including ones that were already Unregister()ed but
still being executed.
This fixes the issue pointed out here: https://github.com/bitcoin/bitcoin/pull/18524/files#r404395685 . It's not currently observable.
ACKs for top commit:
jonasschnelli:
utACK 2276339a17 - reviewed code and test (thanks @ryanofsky for adding the test).
MarcoFalke:
ACK 2276339a17🎎
ryanofsky:
Code review ACK 2276339a17. No change to bugfix, just rebased and new test commit added since last review
Tree-SHA512: c1d68e7c681a45c6cadc84e407c2266bcb4b12d34264e1232a61c4eadb74b551231c5a3b1d041de39f507aef4dfa7d4589b8bfe1833f069c739c6270d2a05dbe
The previous code for MainSignalsInstance::Clear would decrement the reference
count of every interface, including ones that were already Unregister()ed but
still being executed.
fa1a92224d rpc: Avoid initialization-order-fiasco on static CRPCCommand tables (MarcoFalke)
Pull request description:
Currently the fiasco is only theoretical because all content of the table are compile-time constants. However, the fiasco materializes should they ever become run-time constants (e.g. #18531).
ACKs for top commit:
promag:
ACK fa1a92224d.
practicalswift:
ACK fa1a92224d -- fiasco bad :)
Tree-SHA512: cccadb0ad56194599b74f04264d74c34fa865958580a850efc6474bbdc56f30cadce6b2e9a6ad5472ff46c3f4c793366acd8090fad409a45b25d961f2d89da19
7a2ecf16df Wallet: Change IsMine check in CWallet::DelAddressBook from assert to failure (Luke Dashjr)
2952c46b92 Wallet: Replace CAddressBookData.name with GetLabel() method (Luke Dashjr)
d7092c392e QA: Test that change doesn't turn into non-change when spent in an avoid-reuse wallet (Luke Dashjr)
Pull request description:
Follow-up to #18192, not strictly necessary for 0.20
ACKs for top commit:
MarcoFalke:
re-ACK 7a2ecf16df, only change is adding an assert_equal in the test 🔰
jnewbery:
utACK 7a2ecf16df
Tree-SHA512: e0933ee40f705b751697dc27249e1868ed4874254b174ebdd0a7150125d8c818402e66df2371718c7eeb90e67ee2317215fb260aa9b9d7b9b45ee436de2988ff
56fe839e4e qt: Fix Window -> Minimize menu item (Hennadii Stepanov)
Pull request description:
Now Window -> Minimize menu item is broken on Linux.
Steps to reproduce:
1. start `bitcoin-qt`
2. activate Window -> Minimize menu item with a keyboard (not by a shortcut) or a mouse
**Expected behavior**
The main window gets minimized.
**Actual behavior**
The main window still unchanged. Even worse: the menu widget becomes a separate window:
This PR does not touch the macOS specific code as `qApp->focusWindow()` seems work on macOS flawlessly.
ACKs for top commit:
promag:
Tested ACK 56fe839e4e on bionic with qt 5.9.5.
Tree-SHA512: 3582e44ba181d859f5994b9cddc6ce1b60aa1db520a31dd3a0684336c79d558d7410ce7a1ab5b0860c6431b54d8acc3aa16e399717b4c70839861e6b6c4290c0
