The "A fatal internal error occurred, see debug.log for details" is
redundant because init.cpp will already show an InitError with a better
error message as well as the hint to check the debug.log
cd34038cbd Switch from Optional<T> to std::optional<T> (C++17). Run clang-format. (practicalswift)
fb559c1170 tests: Fill fuzzing coverage gaps for functions in util/translation.h (practicalswift)
b74f3d6c45 tests: Fill fuzzing coverage gaps for functions in consensus/validation.h (practicalswift)
c0bbf8193d tests: Fill fuzzing coverage gaps for functions in primitives/block.h (practicalswift)
Pull request description:
* Fill fuzzing coverage gaps for functions in `consensus/validation.h`
* Fill fuzzing coverage gaps for functions in `primitives/block.h`
* Fill fuzzing coverage gaps for functions in `util/translation.h`
* Switch from `Optional<T>` to `std::optional<T>` (C++17). Run `clang-format`.
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
Top commit has no ACKs.
Tree-SHA512: d6aa4634c3953ade173589a8239bd230eb317ef897835a8557acb73df01b25e5e17bf46f837838e59ec04c1f3d3b7d1309ba68c8a264d17b938215512c9e6085
8a22fd0114 avoided os-dependant path (Ferdinando M. Ametrano)
Pull request description:
The current code fails on windows because of the forward slashes; using os.path.join solves the problem and it is in general more robust
ACKs for top commit:
MarcoFalke:
ACK 8a22fd0114
Tree-SHA512: 813f27aea33f97c8afac52e716a55fc5d7fb69621023aba99d40df7e1d145e0ec8d1eee49ddd403b219bf0e0e168e0e987b05c78eaef611f744d99bf2fc8bc91
8d306862ef ci: Add fuzzbuzz integration (practicalswift)
Pull request description:
Add fuzzbuzz integration.
Just like #15338 enabled optional FreeBSD building via Cirrus CI (`.cirrus.yml`) this PR adds optional fuzzing via fuzzbuzz (`.fuzzbuzz.yml`).
Having this merged makes is easier for people to fuzz Bitcoin Core (via their forked repos) using their fuzzbuzz account and then hopefully submit coverage increasing corpus additions upstreams to to https://github.com/bitcoin-core/qa-assets.
Historically it has been mostly been me and MarcoFalke who submit test cases to `qa-assets`, but with this change hopefully more people will join the hunt for coverage increasing fuzzing inputs :)
Top commit has no ACKs.
Tree-SHA512: c7d8e354996c673da36cc9add260383c82a5325bfaa7ce6141ad6cd6b7d6adf3a6c900ea2db17fb70147b3625fa7f6a1ff8ba813aeaa299f316d8f6cabb3a65c
0000ea3265 test: Add test for GetRandMillis and GetRandMicros (MarcoFalke)
fa0e5b89cf Add templated GetRandomDuration<> (MarcoFalke)
Pull request description:
A naive implementation of this template is dangerous, because the call site might accidentally omit the template parameter:
```cpp
template <typename D>
D GetRandDur(const D& duration_max)
{
return D{GetRand(duration_max.count())};
}
BOOST_AUTO_TEST_CASE(util_time_GetRandTime)
{
std::chrono::seconds rand_hour = GetRandDur(std::chrono::hours{1});
// Want seconds to be in range [0..1hour), but always get zero :((((
BOOST_CHECK_EQUAL(rand_hour.count(), 0);
}
```
Luckily `std::common_type` is already specialised in the standard lib for `std::chrono::duration` (https://en.cppreference.com/w/cpp/chrono/duration/common_type). And its effect seem to be that the call site must always specify the template argument explicitly.
So instead of implementing the function for each duration type by hand, replace it with a templated version that is safe to use.
ACKs for top commit:
laanwj:
Code review ACK 0000ea3265
promag:
Code review ACK 0000ea3265.
jonatack:
ACK 0000ea3 thanks for the improved documentation. Code review, built, ran `src/test/test_bitcoin -t random_tests -l test_suite` for the new unit tests, `git diff fa05a4c 0000ea3` since previous review:
hebasto:
ACK 0000ea3265 with non-blocking [nit](https://github.com/bitcoin/bitcoin/pull/18781#discussion_r424924671).
Tree-SHA512: e89d46e31452be6ea14269ecbbb2cdd9ae83b4412cd14dff7d1084283092722a2f847cb501e8054394e4a3eff852f9c87f6d694fd008b3f7e8458cb5a3068af7
050e2ee6f2 test: Remove const to work around compiler error on xenial (Wladimir J. van der Laan)
Pull request description:
Fix the following error in travis:
test/validationinterface_tests.cpp:26:36: error: default initialization of an object of const type 'const BlockValidationState' without a user-provided default constructor
const BlockValidationState state_dummy;
ACKs for top commit:
MarcoFalke:
Tested ACK 050e2ee6f2 on xenial with clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)
fanquake:
ACK 050e2ee6f2 - I see why we didn't hit this on master. We are installing the `clang-8` packages for the tsan job. However on the 0.20 branch we are still just installing `clang`, which is 3.8.
Tree-SHA512: 8a1d57289dbe9895ab79f81ca87b4fd723426b8d72f3a34bec9553226fba69f6dc19551c1f1d52db6c4b2652164a02ddc60f3187c3e2ad7bcacb0aaca7fa690a
746736639e [net processing] Only send a getheaders for one block in an INV (John Newbery)
Pull request description:
Headers-first is the primary method of announcement on the network. If a node fell back sending blocks by inv, it's probably for a re-org. The final block hash provided should be the highest, so send a getheaders and then fetch the blocks we need to catch up.
Sending many GETHEADERS messages to the peer would cause them to send a large number of potentially large HEADERS messages with redundant data, which is a waste of bandwidth.
ACKs for top commit:
sipa:
utACK 746736639e
mzumsande:
utACK 746736639e as per ajtowns' reasoning.
naumenkogs:
utACK 7467366
ajtowns:
ACK 746736639e
jonatack:
ACK 746736639e
Tree-SHA512: 59e243b80d3f0873709dfacb2e4ffba34689aad7de31ec7f69a64e0e3a0756235a0150e4082ff5de823949ba4411ee1aed2344b4749b62e0eb1ea906e41f5ea9
eacedfb023 scripts: add additional type annotations to security-check.py (fanquake)
83d063e954 scripts: add run_command to security-check.py (fanquake)
13f606b4f9 scripts: remove NONFATAL from security-check.py (fanquake)
061acf62a1 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 eacedfb023
Tree-SHA512: 69a7ccfdf346ee202b3e8f940634c5daed1d2b5a5d15ac9800252866ba3284ec66e391a66a0b341f5a4e5e8482fe1b614d4671e8e766112ff059405081184a85
fa1f840596 rpcwallet: Replace pwallet-> with wallet. (MarcoFalke)
fa182a8794 rpcwallet: Replace boost::optional<T>::emplace with simple assignment of T{} (MarcoFalke)
Pull request description:
Closes#18943
ACKs for top commit:
laanwj:
ACK fa1f840596
ryanofsky:
Code review ACK fa1f840596 and thanks for using a standalone commit for the fix
promag:
Code review ACK fa1f840596.
hebasto:
ACK fa1f840596, tested on Linux Mint 19.3.
Tree-SHA512: 0838485d1f93f737ce5bf12740669dcafeebb78dbc3fa15dbcc511edce64bf024f60f0497a04149a1e799d893d57b0c9ffe442020c1b9cfc3c69db731f50e712
Fix the following error in travis:
test/validationinterface_tests.cpp:26:36: error: default initialization of an object of const type 'const BlockValidationState' without a user-provided default constructor
const BlockValidationState state_dummy;
7777f2a4bb miner: Avoid stack-use-after-return in validationinterface (MarcoFalke)
fa5ceb25fc test: Remove UninterruptibleSleep from test and replace it by SyncWithValidationInterfaceQueue (MarcoFalke)
fa770ce7fe validationinterface: Rework documentation, Rename pwalletIn to callbacks (MarcoFalke)
fab6d060ce test: Add unregister_validation_interface_race test (MarcoFalke)
Pull request description:
When a validationinterface has itself unregistered in one thread, but is about to get executed in another thread [1], there is a race:
* The validationinterface destructing itself
* The validationinterface getting dereferenced for execution
[1] 64139803f1/src/validationinterface.cpp (L82-L83)
This happens in the miner. More generally it happens everywhere where at least one thread is generating notifications and another one is unregistering a validationinterface.
This issue has been fixed in commit ab31b9d6fe, but the fix has not been applied to the miner.
Example where this happened in practice: https://travis-ci.org/github/bitcoin/bitcoin/jobs/675322230#L4414
ACKs for top commit:
promag:
Code review ACK 7777f2a4bb.
laanwj:
Code review ACK 7777f2a4bb
Tree-SHA512: 8087119243c71ba18a823a63515f3730d127162625d8729024278b447af29e2ff206f4840ee3d90bf84f93a2c5ab73b76c7e7044c83aa93b5b51047a166ec3d3
This commit is (intentionally) adding a broken test. The test is broken
because it registering a subscriber object that can go out of scope
while events are still being sent.
To run the broken test and reproduce the bug:
- Remove comment /** and */
- ./configure --with-sanitizers=address
- export ASAN_OPTIONS=detect_leaks=0
- make
- while ./src/test/test_bitcoin -t validationinterface_tests/unregister_validation_interface_race --catch_system_errors=no ; do true; done
a30b0a24e9 build: enable -Werror=gnu (Vasil Dimov)
Pull request description:
Stop the build if a warning is emitted due to `-Wgnu` and
`--enable-werror` has been used. As usual - this would help notice such
a warning that is about to be introduced in new code.
This is a followup to
https://github.com/bitcoin/bitcoin/pull/18088 build: ensure we aren't using GNU extensions
ACKs for top commit:
practicalswift:
ACK a30b0a24e9
Empact:
ACK a30b0a24e9
Tree-SHA512: f81b71cf3ee4db88b6f664c571075e0d30800a604f067f44273f256695a1dea533779db2ac859dd0a4cd8b66289c3e45f4aff1cfadfa160a1c354237167b05e2
faa26d3744 test: Remove RPCOverloadWrapper boilerplate (MarcoFalke)
Pull request description:
There are too many wrappers in test_node already, so at least the code that implements the wrappers should be as minimal as possible.
ACKs for top commit:
laanwj:
code review ACK faa26d3744
Tree-SHA512: 94e593907de22187524e2445afb3101e40b3b599d4b4015aa8c6ca902d7586ff9daf520828759029d199a3af79e61b96b490a822a5a193ac7bf946beacb11a24
df6bde031b test: remove glibc fdelt sanity check (fanquake)
8bf1540cc2 build: remove fdelt_chk backwards compatibility code (fanquake)
Pull request description:
ae30d40e50
The return type of [`fdelt_chk`](https://sourceware.org/git/?p=glibc.git;a=blob;f=debug/fdelt_chk.c;h=f62ce7349707cb68f55831c1c591fd7387a90258;hb=HEAD) changed from `unsigned long int` to `long int` in glibc 2.16. See [this commit](https://sourceware.org/git/?p=glibc.git;a=commit;h=ceb9e56b3d1f8c1922e0526c2e841373843460e2). Now that we require [glibc >=2.17](https://github.com/bitcoin/bitcoin/pull/17538) we can remove our back-compat code.
ab7bce584a
While looking at the above changes, I noticed that our glibc fdelt sanity check doesn't seem to be checking anything. `fdelt_warn()` also isn't something we'd want to actually "trigger" at runtime, as doing so would cause `bitcoind` to abort.
The comments:
> // trigger: Call FD_SET to trigger __fdelt_chk. FORTIFY_SOURCE must be defined
> // as >0 and optimizations must be set to at least -O2.
suggest calling FD_SET to check the invocation of `fdelt_chk` (this is [aliased with fdelt_warn in glibc](https://sourceware.org/git/?p=glibc.git;a=blob;f=debug/fdelt_chk.c;h=f62ce7349707cb68f55831c1c591fd7387a90258;hb=HEAD)). However just calling `FD_SET()` will not necessarily cause the compiler to insert a call to `fd_warn()`.
Whether or not GCC (recent Clang should work, but may use different heuristics) inserts a call to `fdelt_warn()` depends on if the compiler can determine if the value passed in is a compile time constant (using [`__builtin_constant_p`](https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html)) and whether the value is < 0 or >= `FD_SETSIZE`. The glibc implementation is [here](https://sourceware.org/git/?p=glibc.git;a=blob;f=misc/bits/select2.h;h=7e17430ed94dd1679af10afa3d74795f9c97c0e8;hb=HEAD). This means our check should never cause a call to be inserted.
Compiling master without `--glibc-back-compat` (if you do pass `--glibc-back-compat` the outcome is still the same; however the abort will only happen with >=`FD_SETSIZE` as that is what our [fdelt_warn()](https://github.com/bitcoin/bitcoin/blob/master/src/compat/glibc_compat.cpp#L24) checks for), there are no calls to `fdelt_warn()` inserted by the compiler:
```bash
objdump -dC bitcoind | grep sanity_fdelt
...
0000000000399d20 <sanity_test_fdelt()>:
399d20: 48 81 ec 98 00 00 00 sub $0x98,%rsp
399d27: b9 10 00 00 00 mov $0x10,%ecx
399d2c: 64 48 8b 04 25 28 00 mov %fs:0x28,%rax
399d33: 00 00
399d35: 48 89 84 24 88 00 00 mov %rax,0x88(%rsp)
399d3c: 00
399d3d: 31 c0 xor %eax,%eax
399d3f: 48 89 e7 mov %rsp,%rdi
399d42: fc cld
399d43: f3 48 ab rep stos %rax,%es:(%rdi)
399d46: 48 8b 84 24 88 00 00 mov 0x88(%rsp),%rax
399d4d: 00
399d4e: 64 48 33 04 25 28 00 xor %fs:0x28,%rax
399d55: 00 00
399d57: 75 0d jne 399d66 <sanity_test_fdelt()+0x46>
399d59: b8 01 00 00 00 mov $0x1,%eax
399d5e: 48 81 c4 98 00 00 00 add $0x98,%rsp
399d65: c3 retq
399d66: e8 85 df c8 ff callq 27cf0 <__stack_chk_fail@plt>
399d6b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
```
If you modify the sanity test to pass `-1` or `FD_SETSIZE` to `FD_SET`, you'll see calls to `fdelt_warn` inserted, and the runtime behaviour is an abort as expected.
```diff
diff --git a/src/compat/glibc_sanity_fdelt.cpp b/src/compat/glibc_sanity_fdelt.cpp
index 87140d0c7..16974bfa0 100644
--- a/src/compat/glibc_sanity_fdelt.cpp
+++ b/src/compat/glibc_sanity_fdelt.cpp
@@ -20,7 +20,7 @@ bool sanity_test_fdelt()
{
fd_set fds;
FD_ZERO(&fds);
- FD_SET(0, &fds);
+ FD_SET(FD_SETSIZE, &fds);
return FD_ISSET(0, &fds);
}
#endif
```
```bash
0000000000399d20 <sanity_test_fdelt()>:
399d20: 48 81 ec 98 00 00 00 sub $0x98,%rsp
399d27: b9 10 00 00 00 mov $0x10,%ecx
399d2c: 64 48 8b 04 25 28 00 mov %fs:0x28,%rax
399d33: 00 00
399d35: 48 89 84 24 88 00 00 mov %rax,0x88(%rsp)
399d3c: 00
399d3d: 31 c0 xor %eax,%eax
399d3f: 48 89 e7 mov %rsp,%rdi
399d42: fc cld
399d43: f3 48 ab rep stos %rax,%es:(%rdi)
399d46: 48 c7 c7 ff ff ff ff mov $0xffffffffffffffff,%rdi
399d4d: e8 3e ff ff ff callq 399c90 <__fdelt_warn>
399d52: 0f b6 04 24 movzbl (%rsp),%eax
399d56: 83 e0 01 and $0x1,%eax
399d59: 48 8b 94 24 88 00 00 mov 0x88(%rsp),%rdx
399d60: 00
399d61: 64 48 33 14 25 28 00 xor %fs:0x28,%rdx
399d68: 00 00
399d6a: 75 08 jne 399d74 <sanity_test_fdelt()+0x54>
399d6c: 48 81 c4 98 00 00 00 add $0x98,%rsp
399d73: c3 retq
399d74: e8 77 df c8 ff callq 27cf0 <__stack_chk_fail@plt>
399d79: 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
```
```bash
src/bitcoind
*** buffer overflow detected ***: src/bitcoind terminated
Aborted
```
I think the test should should be removed and replaced (if possible) with additional checks in security-check.py. I was thinking about adding a version of [this script](https://github.com/fanquake/core-review/blob/master/fortify.py) as part of the output, but that needs more thought. I'll address this in a follow up.
ACKs for top commit:
laanwj:
ACK df6bde031b
Tree-SHA512: d8b3af4f4eb2d6c767ca6e72ece51d0ab9042e1bbdfcbbdb7ad713414df21489ba3217662b531b8bfdac0265d2ce5431abfae6e861b6187d182ff26c6e59b32d
0c63f80854 build: Suppress -Wdeprecated-copy warnings (Hennadii Stepanov)
Pull request description:
Tomorrow, on Apr 23 the Ubuntu 20.04 release is expected. It packaged with Qt 5.12 LTS that has a nasty peculiarity to cause modern compilers, including Clang 10.0 and GCC 9.3, to emit spammy `-Wdeprecated-copy` warnings (#15822, #18419).
This PR suppress such warnings _temporarily_, until the [upstream is fixed](https://codereview.qt-project.org/c/qt/qtbase/+/272258).
Here are some affected systems (with system packages):
- Ubuntu 20.04 LTS + Qt 5.12.8 LTS + { Clang 10.0 | GCC 9.3 }
- Fedora 32 + Qt 5.13.2 + Clang 10.0
Reference: [QTBUG-75210](https://bugreports.qt.io/browse/QTBUG-75210)
Also see **fanquake**'s [comment](https://github.com/bitcoin/bitcoin/pull/18738#issuecomment-622956100).
ACKs for top commit:
MarcoFalke:
ACK 0c63f80854 seems fine to disable this warning for the 0.21.0 release temporarily and then enable it for 0.22.0, when boost is removed.
fanquake:
ACK 0c63f80854 - I think it's ok to suppress these for now, given that `-Wdeprecated-copy` is enabled (via `-Wextra`) in GCC 9 and Clang 10. The Qt output is pretty noisy, and there's a few warnings from Boost as well.
Tree-SHA512: 7064a3272bc9eae00b73a16c421ac58be148f374cbef87320e8f092f52761f6e98166eff60346b70867f8a69a9698a79455dc16b42d92f8fbe7c56519571ac08
9f59dde974 rpc: Relock wallet only if most recent callback (João Barbosa)
a2e6db5c4f rpc: Add mutex to guard deadlineTimers (João Barbosa)
Pull request description:
This PR fixes an early relocking race condition from #18811 where old relock callback runs after new wallet unlock code and nRelockTime update but before rpcRunLater call, causing early relock and incorrect nRelockTime time
Issue introduced in #18487.
Fixes#18811.
ACKs for top commit:
MarcoFalke:
ACK 9f59dde974
ryanofsky:
Code review ACK 9f59dde974. No changes since last review except squashing commits.
jonatack:
ACK 9f59dde974
Tree-SHA512: 2f7fc03e5ab6037337f2d82dfad432495cc337c77d07c968ee2355105db6292f24543c03456f5402e0e759577a4327758f9372f7ea29de6d56dc3695fda9b379
a8b5f1b133 gui: Fix manual coin control with multiple wallets loaded (João Barbosa)
Pull request description:
This PR ensures each loaded wallet has a dedicated coin control in the send view which is manipulated by the coin control dialog.
This is an alternative to #17457. Two main differences are:
- scope reduced - no unnecessary changes unrelated to the fix;
- approach taken - coin control instance now belongs to the send view.
All problems raised in #17457 reviews no longer apply due to the approach taken - https://github.com/bitcoin/bitcoin/pull/17457#pullrequestreview-319297589 and https://github.com/bitcoin/bitcoin/pull/17457#issuecomment-555920829)
No change in behavior if only one wallet is loaded.
Closes#15725.
ACKs for top commit:
jonasschnelli:
utACK a8b5f1b133
ryanofsky:
Code review ACK a8b5f1b133. Code changes are very straightforward, just replacing global CCoinControl object with SendCoinsDialog member. Not sure if this means coin control settings are reset between payments. It would be good to note in the PR description or release notes if single wallet behavior is affected
hebasto:
ACK a8b5f1b133
Sjors:
tACK a8b5f1b133
Tree-SHA512: 3ad9c51bab6f28ec0e90efbd6f43fa510c81dafb2eff0b8c3724efcee3e030054a10be013e27cefe35763374c5f6d7af8c02658736964f733d7e38b646b5df65
e8123eae40 gui: Fix itemWalletAddress leak when not tree mode (João Barbosa)
Pull request description:
Taken from #17457, the first commit is a similar to 88a94f7bb8 but for test binary, and the second commit fixes a leak where `CCoinControlWidgetItem` are unnecessarily created and leaked.
ACKs for top commit:
jonasschnelli:
utACK e8123eae40
hebasto:
ACK e8123eae40, tested on Linux Mint 19.3.
Tree-SHA512: 8b43cb29de103842ce5f048de51222919540d3212d2873c16731145e856178644041924ad0e9a58c2ff08f209a9b4ac26dc9965289eb719da233c0984f93631e
839add193b build: Enable -Wsuggest-override (Hennadii Stepanov)
de5e91c303 refactor: Add BerkeleyDatabaseVersion() function (Hennadii Stepanov)
Pull request description:
From GCC [docs](https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gcc/Warning-Options.html):
> `-Wsuggest-override`
> Warn about overriding virtual functions that are not marked with the override keyword.
~This PR is based on #16722 (the first commit).~ See: https://github.com/bitcoin/bitcoin/pull/16722#issuecomment-584111086
ACKs for top commit:
fanquake:
ACK 839add193b
vasild:
ACK 839add193
practicalswift:
ACK 839add193b assuming Travis is happy: patch looks correct
Tree-SHA512: 1e8cc085da30d41536deff9b181962c1882314ab252c2ad958294087ae1e5a0dfa4886bdbe36f21cf6ae71df776a8420f349f007d4b5b49fd79ba98ce308965a
Headers-first is the primary method of announcement on the network. If a
node fell back sending blocks by inv, it's probably for a re-org. The
final block hash provided should be the highest, so send a getheaders
and then fetch the blocks we need to catch up.
e3047edfb6 test: use p2p constants in denial of service tests (fanquake)
25d8264c95 p2p: add MAX_FEELER_CONNECTIONS constant (tryphe)
Pull request description:
Extracted from #16003.
ACKs for top commit:
naumenkogs:
utACK e3047ed
Tree-SHA512: 14fc15292be4db2e825a0331dd189a48713464f622a91c589122c1a7135bcfd37a61e64af1e76d32880ded09c24efd54d3c823467d6c35367a380e0be33bd35f
23083856a5 [test] Add test for cfcheckpt (Jim Posen)
f9e00bb25a [net processing] Message handling for getcfcheckpt. (Jim Posen)
9ccaaba11e [init] Add -peerblockfilters option (Jim Posen)
Pull request description:
Serve cfcheckpt messages if basic block filter index is enabled and `-peercfilters` is set.
`NODE_COMPACT_FILTERS` is not signaled to peers, but functionality can be used for testing and serving pre-configured clients.
ACKs for top commit:
jonatack:
Code review re-ACK 23083856a5 the only change since my review @ 967e2b1 is an update required for #16224 that was merged yesterday.
fjahr:
re-ACK 23083856a5
jkczyz:
re-ACK 23083856a5
ariard:
re-Code Review ACK 2308385
clarkmoody:
Tested ACK 23083856a
MarcoFalke:
re-ACK 23083856a5🌳
theStack:
ACK 23083856a5
Tree-SHA512: 8c751bbd7d1c31a413096462ae025c3d2f3163c7016cbec472a5f5ec267f8dd19a2dfc4d749876d7409c1db546e6fdd16461c6863effcfa0d3e993edcfa92a08
cbd661122e Set LD_LIBRARY_PATH consistently in travis tests (Russell Yanofsky)
fa35c34df7 Remove unused ci configs that have been moved elsewhere (MarcoFalke)
3333cb9699 fuzz: Pass down MAKEJOBS to test_runner (MarcoFalke)
Pull request description:
Just how `MAKEJOBS` is passed down to the functional test `test_runner`, do the same for the fuzz `test_runner`.
Also includes a commit to remove unused config files, which have been moved elsewhere.
Top commit has no ACKs.
Tree-SHA512: 32557102c9e40599b432aeb004c8427e8fbb07cdf4048050cdc8241d1b029aaad306b1131007eeca8315a4f71c38a7efbb833310e056cd11b835676cd19b8902
83da576f44 net: use CMessageHeader::HEADER_SIZE, add missing include (Jon Atack)
Pull request description:
as suggested 16 months ago by Gleb Naumenko in https://github.com/bitcoin/bitcoin/pull/15197#issuecomment-456181865.
`static constexpr CMessageHeader::HEADER_SIZE` is already used in this file, `src/net.cpp`, in 2 instances. This commit replaces the remaining 2 integer values in the file with it and adds the explicit include header.
Co-authored by: Gleb Naumenko <naumenko.gs@gmail.com>
ACKs for top commit:
naumenkogs:
utACK 83da576
practicalswift:
ACK 83da576f44 -- patch looks correct
theStack:
ACK 83da576f44 -- verified that its just magic number elimination refactoring and additionally checked that all tests pass 👍
Tree-SHA512: 5b915483bca4ea162c259865a1b615d73b88a1b1db3f82db05f770d10b8a42494d948f5b21badbcce2d9efa5915b8cbb6af83073867c23d2f152c0d35ac37b96
d97fac422e Add a link from ZMQ doc to ZMQ example in contrib/ (Damian Mee)
Pull request description:
No code changes :). Only a small convenience improvement in zmq doc.
ACKs for top commit:
fanquake:
ACK d97fac422e
Tree-SHA512: f05a8a7a77c0a698637fd24ffc94d0d617743b434f46695a56576a53331ede254aeece416baf3f8275ae4dfad85ae6e14d1920aa32af53150847420a176d90fb
89fea68ffd build: don't pass -w when building for Windows (fanquake)
Pull request description:
This has been around since the introduction of autotools. However at
this point I'm not sure we'd ever want to suppress all warnings when
performing a build, and given that CXX FLAGS will have been overriden
when cross-compiling for Windows (using depends), this would rarely,
if-ever be used anyways.
From https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html:
> -w
>
> Inhibit all warning messages.
ACKs for top commit:
hebasto:
ACK 89fea68ffd
Tree-SHA512: 2b5bdef7fff5c87b28199f5822cab3cdf600c90c01a40db5cd85053eef5dcb5816e2e97ff61a30ff94b4f0c6cb7be22beaef34d82235bdf05ff9da865d40b381
9847e205bf [docs] Improve commenting in ProcessGetData() (John Newbery)
2f032556e0 [test] test that an invalid GETDATA doesn't prevent processing of future messages (Amiti Uttarwar)
e257cf71c8 [net processing] ignore unknown INV types in GETDATA messages (Amiti Uttarwar)
047ceac142 [net processing] ignore tx GETDATA from blocks-only peers (Amiti Uttarwar)
Pull request description:
Currently we'll stall peers that send us an unknown INV type in a GETDATA message. Be a bit more friendly and just drop the invalid request.
Ditto for blocks-relay-only peers that send us a GETDATA for a transaction.
There's a test for the first part. The second is difficult to test in the functional test framework since we aren't able to make blocks-relay-only connections.
ACKs for top commit:
sipa:
utACK 9847e205bf
brakmic:
ACK 9847e205bf
luke-jr:
utACK 9847e205bf
naumenkogs:
utACK 9847e20
ajtowns:
utACK 9847e205bf
Tree-SHA512: 6007f2fd839ffe737727f6fb8e8f083b2d9e05a510748f1d40b8f9be8fdf7b5419a36d8f1039923eec1ba2983e8f6f0436ec5fc196d9f6dcb0657f2ff8ff8e4c
