eacedfb0230978748cbcfb13817fed7e7c756ba7 scripts: add additional type annotations to security-check.py (fanquake)
83d063e9541cc9ea41ea86919eb9435c73efb14e scripts: add run_command to security-check.py (fanquake)
13f606b4f940e5820ff21ea62fc27a5a91774b05 scripts: remove NONFATAL from security-check.py (fanquake)
061acf62a15ad3dbb9f055b7c2569b9832ed623a scripts: no-longer check for 32 bit windows in security-check.py (fanquake)
Pull request description:
* Remove 32-bit Windows checks.
* Remove NONFATAL checking. Added in #8249, however unused since #13764.
* Add `run_command` to de-duplicate all of the subprocess calls. Mentioned in #18713.
* Add additional type annotations.
* Print stderr when there is an issue running a command.
ACKs for top commit:
laanwj:
ACK eacedfb0230978748cbcfb13817fed7e7c756ba7
Tree-SHA512: 69a7ccfdf346ee202b3e8f940634c5daed1d2b5a5d15ac9800252866ba3284ec66e391a66a0b341f5a4e5e8482fe1b614d4671e8e766112ff059405081184a85
3e38023af724a76972d39cbccfb0bba4c54a0323 scripts: add PE .reloc section check to security-check.py (fanquake)
Pull request description:
The `ld` in binutils has historically had a few issues with PE binaries, there's a good summary in this [thread](https://sourceware.org/bugzilla/show_bug.cgi?id=19011).
One issue in particular was `ld` stripping the `.reloc` section out of PE binaries, even though it's required for functioning ASLR. This was [reported by a Tor developer in 2014](https://sourceware.org/bugzilla/show_bug.cgi?id=17321) and they have been patching their [own binutils](https://gitweb.torproject.org/builders/tor-browser-build.git/tree/projects/binutils) ever since. However their patch only made it into binutils at the [start of this year](https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=dc9bd8c92af67947db44b3cb428c050259b15cd0). It adds an `--enable-reloc-section` flag, which is turned on by default if you are using `--dynamic-base`. In the mean time this issue has also been worked around by other projects, such as FFmpeg, see [this commit](91b668acd6).
I have checked our recent supported Windows release binaries, and they do contain a `.reloc` section. From what I understand, we are using all the right compile/linker flags, including `-pie` & `-fPIE`, and have never run into the crashing/entrypoint issues that other projects might have seen.
One other thing worth noting here, it how Debian/Ubuntu patch the binutils that they distribute, because that's what we end up using in our gitian builds.
In the binutils-mingw-w64 in Bionic (18.04), which we currently use in gitian, PE hardening options/security flags are enabled by default. See the [changelog](https://changelogs.ubuntu.com/changelogs/pool/universe/b/binutils-mingw-w64/binutils-mingw-w64_8ubuntu1/changelog) and the [relevant commit](452b3013b8).
However in Focal (20.04), this has now been reversed. PE hardening options are no-longer the default. See the [changelog](https://changelogs.ubuntu.com/changelogs/pool/universe/b/binutils-mingw-w64/binutils-mingw-w64_8.8/changelog) and [relevant commit](7bd8b2fbc2), which cites same .reloc issue mentioned here.
Given that we explicitly specify/opt-in to everything that we want to use, the defaults aren't necessarily an issue for us. However I think it highlights the importance of continuing to be explicit about what we want, and not falling-back or relying on upstream.
This was also prompted by the possibility of us doing link time garbage collection, see #18579 & #18605. It seemed some sanity checks would be worthwhile in-case the linker goes haywire while garbage collecting.
I think Guix is going to bring great benefits when dealing with these kinds of issues. Carl you might have something to say in that regard.
ACKs for top commit:
dongcarl:
ACK 3e38023af724a76972d39cbccfb0bba4c54a0323
Tree-SHA512: af14d63bdb334bde548dd7de3e0946556b7e2598d817b56eb4e75b3f56c705c26aa85dd9783134c4b6a7aeb7cb4de567eed996e94d533d31511f57ed332287da
8334ee31f868f0f9baf0920d14d20174ed889dbe scripts: add MACHO LAZY_BINDINGS test to test-security-check.py (fanquake)
7b99c7454cdb74cd9cd7a5eedc2fb9d0a19df456 scripts: add MACHO Canary check to security-check.py (fanquake)
Pull request description:
7b99c7454cdb74cd9cd7a5eedc2fb9d0a19df456 uses `otool -Iv` to check for `___stack_chk_fail` in the macOS binaries. Similar to the [ELF check](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/security-check.py#L105). Note that looking for a triple underscore prefixed function (as opposed to two for ELF) is correct for the macOS binaries. i.e:
```bash
otool -Iv bitcoind | grep chk
0x00000001006715b8 509 ___memcpy_chk
0x00000001006715be 510 ___snprintf_chk
0x00000001006715c4 511 ___sprintf_chk
0x00000001006715ca 512 ___stack_chk_fail
0x00000001006715d6 517 ___vsnprintf_chk
0x0000000100787898 513 ___stack_chk_guard
```
8334ee31f868f0f9baf0920d14d20174ed889dbe is a follow up to #18295 and adds test cases to `test-security-check.py` that for some reason I didn't add at the time. I'll sort out #18434 so that we can run these tests in the CI.
ACKs for top commit:
practicalswift:
ACK 8334ee31f868f0f9baf0920d14d20174ed889dbe: Mitigations are important. Important things are worth asserting :)
jonasschnelli:
utACK 8334ee31f868f0f9baf0920d14d20174ed889dbe.
Tree-SHA512: 1aa5ded34bbd187eddb112b27278deb328bfc21ac82316b20fab6ad894f223b239a76b53dab0ac1770d194c1760fcc40d4da91ec09959ba4fc8eadedb173936a
5ca90f8b598978437340bb8467f527b9edfb2bbf 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 5ca90f8b598978437340bb8467f527b9edfb2bbf
Tree-SHA512: 444022ea9d19ed74dd06dc2ab3857a9c23fbc2f6475364e8552d761b712d684b3a7114d144f20de42328d1a99403b48667ba96885121392affb2e05b834b6e1c
7142d50ac33e0ad7d24e49e04c1fc7e3e769ed46 scripts: rename test_64bit_PE to test_PE (fanquake)
edaca2dd123cef958699c07ab248cf0ffc71af07 scripts: add MACHO NX check to security-check.py (fanquake)
1a4e9f32efcc5f6a74290446dc58784fd85c7b31 scripts: add MACHO tests to test-security-check.py (fanquake)
Pull request description:
Adds tests for the MACHO checks in security-check.py:
ac579ada7e/contrib/devtools/security-check.py (L212-L214)
I'm planning on following up with more checks in security-check.py, and corresponding tests in test-security-check.py.
Note that you'll probably have to be on macOS to run them. You can run just this suite with `python3 test-security-check.py TestSecurityChecks.test_MACHO`.
ACKs for top commit:
laanwj:
ACK 7142d50ac33e0ad7d24e49e04c1fc7e3e769ed46
Tree-SHA512: ace3ca9f6df5d4fedd5988938fb7dc7563ec7dc587aa275f780b5f51e9b8d7d6f7768e0a1e05ce438510a07b8640aba92c76847b30c2990f46c66b78a0acf960
ea3c7e585c382998212fd7f41114462a8168a734 test: Remove libssl-dev packages from CI scripts (Wladimir J. van der Laan)
7ea55264b9d60325bc7a5c15d78e9063de145970 test: remove lsan suppression for libcrypto (Wladimir J. van der Laan)
2d7066527a456f8e1f4f603fe104b0bd9d864559 build: remove libcrypto as internal dependency in libbitcoinconsensus.pc (Wladimir J. van der Laan)
278751ea11f2cfe68b0c98f504f65586720cb5a4 doc: Remove ssl as a required dependency from build-unix (Wladimir J. van der Laan)
Pull request description:
Some doc and build cleanups following #17265.
I intentionally left the libssl-dev install in `gitian-win-signer.yml`, as it's necessary for the ossl signer.
ACKs for top commit:
MarcoFalke:
ACK ea3c7e585c382998212fd7f41114462a8168a734 🗯
jamesob:
ACK ea3c7e585c
practicalswift:
ACK ea3c7e585c382998212fd7f41114462a8168a734 - nice!
fanquake:
ACK ea3c7e585c382998212fd7f41114462a8168a734 - thanks.
Tree-SHA512: 67ea35bdd6d6e512d69e6734713534c88cae033a2ed695677ea15c3e3d5ff570374e342775c88e60877fa43a19047853e7b2a433e2c9a4349a5c423726a7457e
7c9e821c4e6cb186208ead9c8df616d1f393a49a scripts: add MACHO NOUNDEFS check to security-check.py (fanquake)
4ca92dc6d3f3e487d63286d8871d1829b3d279ff scripts: add MACHO PIE check to security-check.py (fanquake)
Pull request description:
This uses `otool -vh` to print the mach header and look for the `PIE` flag:
```bash
otool -vh src/bitcoind
Mach header
magic cputype cpusubtype caps filetype ncmds sizeofcmds flags
MH_MAGIC_64 X86_64 ALL LIB64 EXECUTE 24 2544 NOUNDEFS DYLDLINK TWOLEVEL WEAK_DEFINES BINDS_TO_WEAK PIE
```
From [`mach-o/loader.h`](https://opensource.apple.com/source/cctools/cctools-927.0.2/include/mach-o/loader.h.auto.html):
```c
#define MH_PIE 0x200000 /* When this bit is set, the OS will
load the main executable at a
random address. Only used in
MH_EXECUTE filetypes. */
```
ACKs for top commit:
laanwj:
code review ACK 7c9e821c4e6cb186208ead9c8df616d1f393a49a
Tree-SHA512: 5ba2f60440d0e31c70371a355c91ca4f723d80f7287d04e2098bf5b11892cc74216ff8f1454603c4db9675d4f7983614843b992b8dcfca0309aadf2aa7ab2e4b
8f7b93047581c67f2133cdb8c7845471de66c30f Drop the leading 0 from the version number (Andrew Chow)
Pull request description:
Removes the leading 0 from the version number. The minor version, which we had been using as the major version, is now the major version. The revision, which we had been using as the minor version, is now the minor version. The revision number is dropped. The build number is promoted to being part of the version number. This also avoids issues where it was accidentally not included in the version number.
The CLIENT_VERSION remains the same format as previous as previously, as the Major version was 0 so it never actually got included in it.
The user agent string formatter is updated to follow this new versioning.
***
Honestly I'm just tired of all of the people asking for "1.0" that maybe this'll shut them up. Skip the whole 1.0 thing and go straight to version 22.0!
Also, this means that the terminology we commonly use lines up with how the variables are named. So major versions are actually bumping the major version number, etc.
ACKs for top commit:
jnewbery:
Code review ACK 8f7b930475
MarcoFalke:
review ACK 8f7b93047581c67f2133cdb8c7845471de66c30f 🎻
Tree-SHA512: b5c3fae14d4c0a9c0ab3b1db7c949ecc0ac3537646306b13d98dd0efc17c489cdd16d43f0a24aaa28e9c4a92ea360500e05480a335b03f9fb308010cdd93a436
* compat: remove bswap_* check on macOS
This was originally added in #9366 to fix the gui build, as
Protobuf would also define these macros. Now that we're no-longer
using Protobuf, remove the additional check.
* build: skip building OpenSSL lib_ssl
* build: remove OpenSSL from Qt build
More info available from:
https://doc.qt.io/qt-5/ssl.html#enabling-and-disabling-ssl-support
* build: remove EVP_MD_CTX_new detection
This was added in #9475 to fix LibreSSL compatibility for
BIP70, so is no longer required.
* build: remove SSL lib detection
* gui: update BIP70 support message
* build: remove BIP70 entries from macOS Info.plist
* gui: remove payment request file handling from OpenURI dialog
* gui: remove BIP70 Support
* build: remove protobuf from depends and contrib
655d52a0cd6b3df738dc9843e25a95b8930b4628 contrib: Specify wb mode when creating mac sdk (João Barbosa)
Pull request description:
Fix the warning:
```
./contrib/macdeploy/gen-sdk:84: FutureWarning: GzipFile was opened for writing, but this will change in future Python releases. Specify the mode argument for opening it for writing.
```
ACKs for top commit:
fanquake:
Tested ACK 655d52a0cd6b3df738dc9843e25a95b8930b4628 with Python 3.10.
Tree-SHA512: 095cd301f211531a8b8f50e7915fe13c7ab0b278fb23dc50a03625cfae9e2fd7a0d8c315fced4af3011552e3e455ce562b7f717a0ed096c4433ddcf24f22b2c9
7fb7acfc206b4bf8c296d72b66f3bd4fe342fd87 Set init stop timeout to 10 min (setpill)
Pull request description:
`bitcoind` can take a long time to flush its db cache to disk upon
shutdown. Systemd sends a `SIGKILL` after a timeout, causing unclean
shutdowns and triggering a long "Rolling forward" at the next startup.
Disabling the timeout should prevent this from happening, and does not
break systemd's `restart` logic.
Addresses #13736.
ACKs for top commit:
instagibbs:
utACK 7fb7acfc20
Tree-SHA512: 16e0ce5a9ecf0628f8d93d68db3f5a78ab36021d9bede05a90c84f144db2e87e17707a6eb910cb7c018c265ce2c81d43de2988bd79e4a2d8554515db8fb5aa36
fac86ac7b3ceac2f884412c7a9f4bd5bab5e3916 scripted-diff: Add missed copyright headers (Hennadii Stepanov)
6fde9d5e47fc9a1042b3fb68031eab5bf55e508d script: Update EXLUDE list in copyright_header.py (Hennadii Stepanov)
1998152f15fd2b0e83f5068c375a34feaf73db8c script: Add empty line after C++ copyright (Hennadii Stepanov)
071f2fc204f542c5a287ca8835115a2ee0bf2f50 script: Add ability to insert copyright to *.sh (Hennadii Stepanov)
Pull request description:
This PR improves `contrib/devtools/copyright_header.py` script and adds copyright headers to the files in `src` and `test` directories with two exceptions:
- [`src/reverse_iterator.h`](https://github.com/bitcoin/bitcoin/blob/master/src/reverse_iterator.h) (added to exceptions)
- [`src/test/fuzz/FuzzedDataProvider.h`](https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/FuzzedDataProvider.h) (added to exceptions)
On master 5622d8f3156a293e61d0964c33d4b21d8c9fd5e0:
```
$ ./contrib/devtools/copyright_header.py report . | grep zero
25 with zero copyrights
```
With this PR:
```
$ ./contrib/devtools/copyright_header.py report . | grep zero
2 with zero copyrights
```
~I am uncertain about our copyright policy with `build_msvc` and `contrib` directories content, so they are out of scope of this PR.~
ACKs for top commit:
MarcoFalke:
ACK fac86ac7b3ceac2f884412c7a9f4bd5bab5e3916
Tree-SHA512: d7832c4a7a1a3b7806119775b40ec35d7982f49ff0e6199b8cee4c0e0a36e68d51728b6ee9924b1c161df4bc6105bd93391b79d42914357fa522f499cb113fa8
f8f772dc495f7d3ee02d75d42ee22ceb90c3a693 macdeploy: alternative info to download the macOS SDK (Antoine Poinsot)
Pull request description:
The previous link wasn't accessible for me, this adds some instructions
given to me by Hebasto on #bitcoin-core-builds as well as a shasum for
the archive to quickly check the downloaded one is the right one before
processing with the entire Guix build.
ACKs for top commit:
fanquake:
ACK f8f772dc495f7d3ee02d75d42ee22ceb90c3a693
Tree-SHA512: 620160b593ed8fa4ae4a748b8e72d67b93ff0ec9e6b8ef3c3ac5402c1c48ec0ac325a527b6278cdf84aaf51ba8194d4c366c412ffad141d0412add2710efcff5
7087440894a9daa7de806c5aa42d83ad60759c65 depends: native_ds_store 1.3.0 (fanquake)
Pull request description:
`ds_store` [now takes advantage](36fb607940) of Pythons ability to decode binary [plists](https://docs.python.org/3/library/plistlib.html) (since 3.4), so we can drop its biplist dependency.
The call to `biplist.Data()` in `custom_dsstore.py` doesn't seem to do anything, and from what I can tell can just be removed. i.e:
```diff
diff --git a/contrib/macdeploy/custom_dsstore.py b/contrib/macdeploy/custom_dsstore.py
index dc1c1882d..e475bc6c3 100755
--- a/contrib/macdeploy/custom_dsstore.py
+++ b/contrib/macdeploy/custom_dsstore.py
@@ -47,6 +47,7 @@ alias.volume.disk_image_alias.target.filename = package_name_ns + '.temp.dmg'
alias.volume.disk_image_alias.target.carbon_path = 'Macintosh HD:Users:\x00bitcoinuser:\x00Documents:\x00bitcoin:\x00bitcoin:\x00' + package_name_ns + '.temp.dmg'
alias.volume.disk_image_alias.target.posix_path = 'Users/bitcoinuser/Documents/bitcoin/bitcoin/' + package_name_ns + '.temp.dmg'
alias.target.carbon_path = package_name_ns + ':.background:\x00background.tiff'
+assert(biplist.Data(alias.to_bytes()) == alias.to_bytes())
icvp['backgroundImageAlias'] = biplist.Data(alias.to_bytes())
ds['.']['icvp'] = icvp
```
ACKs for top commit:
laanwj:
ACK 7087440894a9daa7de806c5aa42d83ad60759c65
Tree-SHA512: 8ba3cf561937efe4a3daae8b0cb4de3bf9e425b3a9244161b09d94ee2b1bd4c3e21315fa70e495b19a052aabdc1731b3b6f346b63272d72d2762ced83237d02f
6aab7649d30b19d136a27f1287fd2c8b00fb460c doc: Fix whitespace errs in .md files, bitcoin.conf, Info.plist.in, and find_bdb48.m4 (Jon Layton)
Pull request description:
Although there is an existing `test/lint/lint-whitespace.sh` linter, it only prevents new errors from being introduced. This commit removes all existing whitespace errors from Core markdown files (skips `src/crypto/ctaes/`, `leveldb/`, and `doc/release-notes/`), `bitcoin.conf`, and `Info.plist.in`.
Further formatting could be done on the markdown documents, but seeing as there several coexisting styles that break a few `markdownlint` rules, a first step would be to define and add a linter to Travis. For now, the small fix is made.
ACKs for top commit:
fanquake:
ACK 6aab7649d30b19d136a27f1287fd2c8b00fb460c - Thanks for following up. Hopefully we now never have to deal with whitespace again.
Tree-SHA512: 810cc31ae4364b2dedf85783e67315d7b4e11589e4b32c599606e1b1ba8de0663bcae9ddb1bd8c9762a3636a2d65bdcd64ec22d2e90943f374a0c9574b77ca23
* optimize: adjust ci dockerfile for faster building
* remove old comment
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* remove unneeded semicolons
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* format: make each installed package it's own line to minimize conflicts
* sort the installed packages (and a fix)
Signed-off-by: Pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* optimize: somehow optimize circular-dependencies.py
Signed-off-by: pasta <pasta@dashboost.org>
* optimize: use parallel if available to lint in parallel
Signed-off-by: pasta <pasta@dashboost.org>
* suggestions
* more suggestions
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* src/evo/evodb.cpp:57:29: warning: Assert statement calls a function which may have desired side effects: 'IsClean'. [assertWithSideEffect]
* src/llmq/quorums.cpp:635:37: note: Null pointer dereference
src/llmq/quorums.cpp:635:37: warning: Either the condition 'pFrom==nullptr' is redundant or there is possible null pointer dereference: pFrom. [nullPointerRedundantCheck]
src/llmq/quorums.cpp:636:81: note: Assuming that condition 'pFrom==nullptr' is not redundant
* fix a bunch of cppcheck warnings
* cppcheck: run on many more files. Enable all checks except a few ignored ones.
ignored
```
"Consider using std::transform algorithm instead of a raw loop."
"Consider using std::accumulate algorithm instead of a raw loop."
```
* ci: build specific version of cppcheck instead of install from apt
* ci: use cppcheck 2.4, remove commented out line, fix symlink
cppcheck 2.6 is latest, however causes issues
```
src/spork.cpp:135:51: warning: Analysis failed. If the code is valid then please report this failure. [cppcheckError]
```
cppcheck 2.5 appears to get into an infinite loop
* no need to check presence before insertion
* use if-init, remove redundant check
* remove redundant check
* don't remove cmake? fix macOs depends build?
* cppcheck: one per line, alphabetize
* remove duplicate cmake install
