fa23fbb42f ci: Run all tests on native mac again (MarcoFalke)
Pull request description:
They should pass again after f6072e601a
ACKs for top commit:
practicalswift:
ACK fa23fbb42f -- Travis is happy and so am I
Tree-SHA512: 49c16b6056d4e67d12a202744e1c56fee2788830213fe4a195955ad44c6b8ecce768a591463ffa0048821959a75b6fad4178629a8866c4a26799c4c8c13e933d
faebb60b8d doc: Remove outdated comment in TransactionTablePriv (MarcoFalke)
Pull request description:
Locks are no longer taken upfront, so remove the outdated comment
ACKs for top commit:
hebasto:
ACK faebb60b8d, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: cd6df24d49d17e58049ac9b261c5e07c8e85ed1aacb547b13c0e55139339d7fcc3b1f766ea2e27d758ea77deadc01f7e28781be1515323c82b9012cee8fd488b
fa575f3461 wallet: Replace boost::none with nullopt (MarcoFalke)
fac7bdb75e script: Fix boost/C++17 compile failure (MarcoFalke)
Pull request description:
Compiling with C++17 enabled, but not the latest version of boost (e.g. 1.69) will result in a compile failure. I know that C++17 is not "officially" supported yet, especially not with all boost versions, since C++17 is meant to replace boost, but some of my systems can no longer compile Bitcoin Core and upstreaming the patches avoids others running into the same issue.
ACKs for top commit:
sipa:
utACK fa575f3461
Tree-SHA512: 028e0f0a96c68f6e3394263dd720f0288fff6584592fdf9a7d9551b8358ee64f64b7c5cb802cc866eaa435e0247b66a5a5e54bfdc61a7c9769f287cfd3509966
21a65756f5 Add Windows WSL build recommendation to temporarily disable Win32 PE support. (Aaron Clauson)
Pull request description:
This is a solution for the issues described in #17277 and #18348
When cross compiling Bitcoin Code for Windows the `Autoconf` configure scripts attempt to execute Win32 PE files. The configure scripts expect the attempt to fail, however, WSL supports forking the execution of Win32 PE files out to the underlying Windows OS. This can result in the executions failing for unanticipated reasons, which is the case in the two referenced issues.
This PR adds an explanatory note and additional instructions to temporarily disable WLS's Win32 support.
ACKs for top commit:
laanwj:
ACK 21a65756f5
Tree-SHA512: afb014be5a63fa9a827aed30acb2faab15feed34ed89c788a7f6ae6ab1b2238f99e075e6e281d0cc581914db3a4ecc3d5a3d26442f11a520e4e457a40e75e533
99993489da test: Set -logthreadnames in unit tests (MarcoFalke)
fa4ea997b4 init: Setup scheduler in tests and init in exactly the same way (MarcoFalke)
Pull request description:
Generally the unit tests are single threaded, with the exception of the script check threads, the schedule, and optionally indexer threads.
Like the functional tests, the thread name can serve additional debug information, so set `-logthreadnames` in unit tests.
Can be tested with
```
./src/test/test_bitcoin -l test_suite -t validation_tests/test_combiner_all -- DEBUG_LOG_OUT
ACKs for top commit:
laanwj:
ACK 99993489da
Tree-SHA512: 3bdbfc211da146da64b50b0826246aff5c611a84b69ab896a55b3c9d1adc92c5975da36ab92aee577df82e229c4326b477f4105bfdd1a5df4c9a0b018cf61602
c4ffcf07af build: remove BIP70 configure option (fanquake)
Pull request description:
This was left in after #17165, so that anyone who had been compiling
with (already disabled by default) BIP70 would realise that support
had been completely removed in 0.20.0. However we should be able to
remove it for 0.21.0.
ACKs for top commit:
jnewbery:
utACK c4ffcf07af
MarcoFalke:
ACK c4ffcf07af with or without the "catch-all reject"
Tree-SHA512: a5dd4231ed97c9dd1984fb90d69a8725df2fdda0b963269b0575601c74528e5d820a4a863c428f8ede86eaae2a1606671fe1fcebdeb96b1023f7a5f899270284
f1a0314c53 gui: change combiner for signals to optional_last_value (Cory Fields)
Pull request description:
[`optional_last_value`](https://www.boost.org/doc/libs/1_73_0/doc/html/boost/signals2/optional_last_value.html), which does not throw, has replaced `last_value` as
Boosts default combiner. Besides being better supported, it also doesn't
trigger gcc's `-Wmaybe-unitialized` warning, presumably because exceptions no
longer bubble-up out of signals:
```bash
In file included from ui_interface.cpp:9:
/bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp: In member function 'boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::result_type boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::operator()(Args ...) [with Combiner = boost::signals2::last_value<bool>; Group = int; GroupCompare = std::less<int>; SlotFunction = boost::function<bool(const bilingual_str&, const std::__cxx11::basic_string<char>&, unsigned int)>; ExtendedSlotFunction = boost::function<bool(const boost::signals2::connection&, const bilingual_str&, const std::__cxx11::basic_string<char>&, unsigned int)>; Mutex = boost::signals2::mutex; R = bool; Args = {const bilingual_str&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, unsigned int}]':
/bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized]
if(value) return value.get();
^
/bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:43:21: note: '*((void*)& value +1)' was declared here
optional<T> value;
^~~~~
/bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp: In member function 'boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::result_type boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::operator()(Args ...) [with Combiner = boost::signals2::last_value<bool>; Group = int; GroupCompare = std::less<int>; SlotFunction = boost::function<bool(const bilingual_str&, const std::__cxx11::basic_string<char>&, const std::__cxx11::basic_string<char>&, unsigned int)>; ExtendedSlotFunction = boost::function<bool(const boost::signals2::connection&, const bilingual_str&, const std::__cxx11::basic_string<char>&, const std::__cxx11::basic_string<char>&, unsigned int)>; Mutex = boost::signals2::mutex; R = bool; Args = {const bilingual_str&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, unsigned int}]':
/bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized]
if(value) return value.get();
^
/bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:43:21: note: '*((void*)& value +1)' was declared here
optional<T> value;
^~~~~
```
The change in default happened in [Boost 1.39.0](https://www.boost.org/users/history/version_1_39_0.html) (along with the introduction of the Signals2 library.
More information is also available here https://www.boost.org/doc/libs/1_73_0/doc/html/signals2/rationale.html#id-1.3.36.9.4:
> The default combiner for Boost.Signals2 has changed from the last_value combiner used by default in the original Boost.Signals library.
> This is because last_value requires that at least 1 slot be connected to the signal when it is invoked (except for the last_value<void> specialization).
> In a multi-threaded environment where signal invocations and slot connections and disconnections may be happening concurrently, it is difficult to fulfill this requirement. When using optional_last_value, there is no requirement for slots to be connected when a signal is invoked, since in that case the combiner may simply return an empty boost::optional.
ACKs for top commit:
laanwj:
ACK f1a0314c53
Tree-SHA512: 3600f85019a3591b141dc9207f8a7e66d16d9996cf97fdf08f5133a212d55c591955ab835ffbdca20b5d62711578bc305d5525c75546fa957f180192e2a80c1e
ca24edfbc1 walletdb: Handle cursor internally (Andrew Chow)
Pull request description:
Instead of returning a Dbc (BDB cursor object) and having the caller deal with the cursor, make BerkeleyBatch handle the cursor internally.
Split from #18971
ACKs for top commit:
ryanofsky:
Code review ACK ca24edfbc1. Changes since last review: StartCursor rename, moving CloseCursor calls near returns
promag:
Code review ACK ca24edfbc1.
Tree-SHA512: f029b498c7f275aedca53ce7ade7cb99c82975fd6cad17346a4990fb3bcc54e2a5309b32053bd13def9ee464d331b036ac79abb8fc4fa561170c6cfc85283447
optional_last_value, which does not throw, has replaced optional_value as
boost's default combiner. Besides being better supported, it also doesn't
trigger gcc's -Wmaybe-unitialized warning, presumably because exceptions no
longer bubble-up out of signals:
```bash
boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized]
if(value) return value.get();
```
The change in default happened in Boost 1.39.0 (along with the
introduction of the signals 2 library. More information is available here:
https://www.boost.org/doc/libs/1_73_0/doc/html/signals2/rationale.html#id-1.3.36.9.4
and here:
https://www.boost.org/doc/libs/1_73_0/doc/html/boost/signals2/optional_last_value.html
Co-authored-by: fanquake <fanquake@gmail.com>
faca73000f ci: Install fixed version of clang-format for linters (MarcoFalke)
fa4695da4c build: Sort Makefile.am after renaming file (MarcoFalke)
cccc2784a3 scripted-diff: Move ui_interface to the node lib (MarcoFalke)
fa72ca6a9d qt: Remove unused includes (MarcoFalke)
fac96e6450 wallet: Do not include server symbols (MarcoFalke)
fa0f6c58c1 Revert "Fix link error with --enable-debug" (MarcoFalke)
Pull request description:
This reverts a hacky workaround from commit b83cc0f, which only happens to work due to compiler optimizations. Then, it actually fixes the linker error.
The underlying problem is that the wallet includes symbols from the server (ui_interface), which usually results in linker failures. Though, in this specific case the linker failures have not been observed (unless `-O0`) because our compilers were smart enough to strip unused symbols.
Fix the underlying problem by creating a new header-only with the needed symbol and move ui_interface to node to clarify that this is part of libbitcoin_server.
ACKs for top commit:
Sjors:
ACK faca730
laanwj:
ACK faca73000f
hebasto:
re-ACK faca73000f, since the [previous](https://github.com/bitcoin/bitcoin/pull/19331#pullrequestreview-434420539) review:
Tree-SHA512: e9731f249425aaea50b6db5fc7622e10078cf006721bb87989cac190a2ff224412f6f8a7dd83efd018835302337611f5839e29e15bef366047ed591cef58dfb4
3a7e79478a test: retry when write to a socket fails on macOS (Ivan Metlushko)
8cf9d15b82 test: use pgrep for better compatibility (Ivan Metlushko)
Pull request description:
Rationale: a few minor changes to make experience of running tests on macOS a bit better
1.`pidof` is not available on BSD/macOS, while `pgrep` is present on BSD, Linux and macOS
2. Add retry as a workaround for a weird behavior when writing to a socket (https://bugs.python.org/issue33450). Stacktrace attached
Man pages:
https://www.freebsd.org/cgi/man.cgi?query=pgrep&apropos=0&sektion=1&manpath=FreeBSD+6.0-RELEASE&arch=default&format=htmlhttps://man7.org/linux/man-pages/man1/pgrep.1.html
Related to #19281
Stacktrace example:
```
...
33/161 - feature_abortnode.py failed, Duration: 63 s
stdout:
2020-06-11T10:46:43.947000Z TestFramework (INFO): Initializing test directory /var/folders/2q/d5w9zh614r7g5c8r74ln3g400000gq/T/test_runner_₿_🏃_20200611_174102/feature_abortnode_128
2020-06-11T10:46:45.199000Z TestFramework (INFO): Waiting for crash
2020-06-11T10:47:15.921000Z TestFramework (INFO): Node crashed - now verifying restart fails
2020-06-11T10:47:47.068000Z TestFramework (INFO): Stopping nodes
[node 1] Cleaning up leftover process
stderr:
Traceback (most recent call last):
File "/Users/xxx/Projects/bitcoin/test/functional/feature_abortnode.py", line 50, in <module>
AbortNodeTest().main()
File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_framework.py", line 142, in main
exit_code = self.shutdown()
File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_framework.py", line 266, in shutdown
self.stop_nodes()
File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_framework.py", line 515, in stop_nodes
node.stop_node(wait=wait)
File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/test_node.py", line 318, in stop_node
self.stop(wait=wait)
File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/authproxy.py", line 142, in __call__
response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
File "/Users/xxx/Projects/bitcoin/test/functional/test_framework/authproxy.py", line 107, in _request
self.__conn.request(method, path, postdata, headers)
File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 1107, in request
self._send_request(method, url, body, headers)
File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 1152, in _send_request
self.endheaders(body)
File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 1103, in endheaders
self._send_output(message_body)
File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 936, in _send_output
self.send(message_body)
File "/Users/xxx/.pyenv/versions/3.5.6/lib/python3.5/http/client.py", line 908, in send
self.sock.sendall(data)
OSError: [Errno 41] Protocol wrong type for socket
```
ACKs for top commit:
laanwj:
ACK 3a7e79478a
Tree-SHA512: fefbe40ce94ab29f18bbbed2a434194b1384ffa5279b1d04db7a3708e3dd422bd9e450f1db3f95a1a851fac5a626ab533c6ebcfd7ede96f8ccae9e6f3e9fff92
eb6b73540d build: pass _WIN32_WINNT=0x0601 when building libevent for Windows (fanquake)
03e056edcd depends: Patch libevent build to fix IPv6 -rpcbind on Windows (Luke Dashjr)
Pull request description:
TLDR: This poaches a commit from #18287 and adds one more to adjust the Windows version targeted when building libevent. These changes combined should fully fix ipv6 usage with the RPC server on Windows.
---
Binding the RPC server to a ipv6 address does not currently work on Windows.
We currently try and bind to `127.0.0.1` and `::1` [by default](https://github.com/bitcoin/bitcoin/blob/master/src/httpserver.cpp#L304).
On Windows you'll see lines like this in debug.log:
```bash
2020-06-24T01:49:04Z libevent: getaddrinfo: nodename nor servname provided, or not known
2020-06-24T01:49:04Z Binding RPC on address ::1 port 8332 failed
```
This issue was bought up in, and supposedly fixed by #18287, however the two people that tested it, both said that it didn't fix the problem. I think I now understand why that change alone is incomplete.
Our call into libevent starts with [evhttp_bind_socket_with_handle()](https://github.com/bitcoin/bitcoin/blob/master/src/httpserver.cpp#L325):
```bash
evhttp_bind_socket_with_handle()
bind_socket()
make_addrinfo()
evutil_getaddrinfo()
if #USE_NATIVE_GETADDRINFO
#ifndef AI_ADDRCONFIG
evutil_adjust_hints_for_addrconfig_()
evutil_check_interfaces()
evutil_check_ifaddrs()
evutil_found_ifaddr()
// miss identifies ipv6 as ipv4?
#endif
evutil_getaddrinfo_common_()
```
The problem is falling into ["#ifndef AI_ADDRCONFIG"](https://github.com/libevent/libevent/blob/master/evutil.c#L1580):
```cpp
#ifndef AI_ADDRCONFIG
/* Not every system has AI_ADDRCONFIG, so fake it. */
if (hints.ai_family == PF_UNSPEC &&
(hints.ai_flags & EVUTIL_AI_ADDRCONFIG)) {
evutil_adjust_hints_for_addrconfig_(&hints);
}
#endif
```
When this occurs, hints end up being adjusted, and it seems that ipv6 addresses end up being mis-identified as ipv4?
However this shouldn't happen, as these `AI_` definitions are available on Windows.
The issue is that in evutil.c, `_WIN32_WINNT` [is set to `0x501`](https://github.com/libevent/libevent/blob/master/evutil.c#L45) (XP).
This obviously predates Vista (`0x0600`), which is when the `AI_ADDRCONFIG` definition (and others) became [available](https://docs.microsoft.com/en-us/windows/win32/api/ws2def/ns-ws2def-addrinfoa).
The change here will override libevents internal D_WIN32_WINNT defines. This should be ok, because it's only making "more" of the Windows API available. It's also aligned with what we do in our own configure, we pass [`D_WIN32_WINNT=0x0601`](https://github.com/bitcoin/bitcoin/blob/master/configure.ac#L610). We also now use linker flags to restrict our binary from running on a Windows version [earlier than Windows 7](https://github.com/bitcoin/bitcoin/blob/master/configure.ac#L621).
The combined fixes can be tested by running:
`bitcoind -rpcbind=::1 rpcallowip='0.0.0.0/0' -debug=http`
and then querying it using:
`bitcoin-cli -rpcconnect=::1 getblockchaininfo`
TODO:
- [x] Open an issue upstream. https://github.com/libevent/libevent/issues/1041
ACKs for top commit:
laanwj:
ACK eb6b73540d
Tree-SHA512: e1e50f194911301981edaed0c216ed4efb9ebd4a1f9bc9b9f85bec7140b66c45c8666fd5db4aad359596559d4a08ab7c920e9d9736f3ecdbb841afc54e40586e
This enables of the use of AI_* definitions in the Windows headers,
specifically AI_ADDRCONFIG, which fixes an issue with libevent and
ipv6 on Windows.
It also aligns with what we define in configure when building Core.
6fdfeebcc7 refactor: Replace RecursiveMutex with Mutex in rpc/server.cpp (Hennadii Stepanov)
Pull request description:
The functions that could lock this mutex, i.e., `SetRPCWarmupStatus()`, `SetRPCWarmupFinished()`, `RPCIsInWarmup()`, `CRPCTable::execute()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_rpc_warmup_mutex` could be a non-recursive mutex.
Related to #19303.
ACKs for top commit:
laanwj:
ACK 6fdfeebcc7
MarcoFalke:
ACK 6fdfeebcc7
Tree-SHA512: 05a8ac58c0cd6a3c9afad9e06ad78059642e3e97715e129f379c0bf6dccdb58e70d05d965f23e7432fd3f02d7f97967a778ffb8e424837891d9d785a9e98964c
a92e48b02d test: move TEST_RUNNER_EXTRA into native tsan setup (fanquake)
Pull request description:
`feature_block.py` is being run in the tsan job, i.e [here](https://travis-ci.org/github/bitcoin/bitcoin/jobs/703122309), even though it should be excluded. My hasty assumption is that this will fix it. In any case, all other instances of `TEST_RUNNER_EXTRA` seem to have moved out of `.travis.yml` and into the different CI configurations.
ACKs for top commit:
MarcoFalke:
ACK a92e48b02d
practicalswift:
ACK a92e48b02d -- patch looks correct
hebasto:
ACK a92e48b02d, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 86057bef2cc87c6acdbbf94f8cd7a5147510448c3e67aacde8daf247e3ccf649cfc5afbbd10693e084f426042d98150616c0e49bfa5f32b949dff9cebd2fd95d
9b009fae6e qa: Test concurrent wallet loading (João Barbosa)
b9971ae585 wallet: Handle concurrent wallet loading (João Barbosa)
Pull request description:
This PR handles concurrent wallet loading.
This can be tested by running in parallel the following script a couple of times:
```sh
for i in {1..10}
do
src/bitcoin-cli -regtest loadwallet foo
src/bitcoin-cli -regtest unloadwallet foo
done
```
Eventually the error occurs:
```
error code: -4
error message:
Wallet already being loading.
```
For reference, loading and already loaded wallet gives:
```
error code: -4
error message:
Wallet file verification failed. Error loading wallet w1. Duplicate -wallet filename specified.
```
Fixes#19232.
ACKs for top commit:
MarcoFalke:
Concept ACK 9b009fae6e I have not reviewed the code
hebasto:
ACK 9b009fae6e, tested on Linux Mint 20 (x86_64):
ryanofsky:
Code review good-but-not-ideal ACK 9b009fae6e
Tree-SHA512: 0ccd77b03c0926e4c4e51efb31e193b93cb4b9ffe8bac6bb018f7344c55dfd939b873b8cf5e657dca73e6202eb75aa672de2acb787cc133184b0b3b51e47b972
9952242c03 build: improve builtin_clz* detection (fanquake)
Pull request description:
Fixes#19402.
The way we currently test for `__builtin_clz*` support with `AC_CHECK_DECLS` does not work with Clang:
```bash
configure:21492: clang++-10 -std=c++11 -c -g -O2 -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS conftest.cpp >&5
conftest.cpp💯10: error: builtin functions must be directly called
(void) __builtin_clz;
^
1 error generated.
```
This also removes the `__builtin_clz()` check, as we don't actually use it anywhere, and it's trvial to re-add detection if we do start using it at some point. If this is controversial then I'll add a test for it as well.
ACKs for top commit:
sipa:
ACK 9952242c03
laanwj:
ACK 9952242c03
Tree-SHA512: 695abb1a694a01a25aaa483b4fffa7d598842f2ba4fe8630fbed9ce5450b915c33bf34bb16ad16a16b702dd7c91ebf49fe509a2498b9e28254fe0ec5177bbac0
fa525e4d1c net: Avoid wasting inv traffic during IBD (MarcoFalke)
fa06d7e934 refactor: block import implies IsInitialBlockDownload (MarcoFalke)
faba65e696 Add ChainstateManager::ActiveChainstate (MarcoFalke)
fabf3d64ff test: Add FeeFilterRounder test (MarcoFalke)
Pull request description:
Tx-inv messages are ignored during IBD, so it would be nice if we told peers to not send them in the first place. Do that by sending two `feefilter` messages: One when the connection is made (and the node is in IBD), and another one when the node leaves IBD.
ACKs for top commit:
jamesob:
ACK fa525e4d1c ([`jamesob/ackr/19204.1.MarcoFalke.p2p_reduce_inv_traffic_d`](https://github.com/jamesob/bitcoin/tree/ackr/19204.1.MarcoFalke.p2p_reduce_inv_traffic_d))
naumenkogs:
utACK fa525e4
gzhao408:
ACK fa525e4d1c
jonatack:
re-ACK fa525e4 checked diff `git range-diff 19612ca fa8a66c fa525e4`, re-reviewed, ran tests, ran a custom p2p IBD behavior test at 9321e0f223.
hebasto:
re-ACK fa525e4d1c, only rebased since the [previous](https://github.com/bitcoin/bitcoin/pull/19204#pullrequestreview-429519667) review (verified with `git range-diff`).
Tree-SHA512: 2c22a5def9822396fca45d808b165b636f1143c4bdb2eaa5c7e977f1f18e8b10c86d4c180da488def38416cf3076a26de15014dfd4d86b2a7e5af88c74afb8eb
53361ddc75 [build] Remove unused RES_IMAGES (Bushstar)
Pull request description:
Remove RES_IMAGES. Seems to be unused since 2015 in the commit below.
98c222b5aa (diff-9a4f3a253de77bf90b107bdf5283ebc3R317)
The src/qt/res/images to which it was used with is no longer present either.
ACKs for top commit:
hebasto:
ACK 53361ddc75, tested on Linux Mint 20 (x86_64).
Tree-SHA512: d2f09ae225a4c6c171e1aae4c4a444064dc0502e96130e04ccb718f9fcf611a287c56630dec3e9a8937b5e29040d931a237da36180d2343c23cef30359e46323
fa74a54fad ci: Increase test timeout for sanitizer configs (MarcoFalke)
Pull request description:
Hopefully fixes#19369
ACKs for top commit:
practicalswift:
ACK fa74a54fad -- patch looks correct!
fanquake:
ACK fa74a54fad - the test failure here is a different issue, and the problem referenced by this PR hasn't occurred, so I think this can be merged. It's also fixing the use of `--factor` which was replaced in #18986.
Tree-SHA512: bec44fff454f20b7c5f8a461560d2496765dea61186027cc0cdce5ac55be0488b6f7f172fec49b89fe59a75b455501e2b4ae91a98c4a17d5c1a722846d2b3b60
8578c6fccd build: Fix search for brew-installed BDB 4 on OS X (Glenn Willen)
Pull request description:
~~NOTE: This PR contains one important fix that I need (to make Bitcoin Core build cleanly on my system without shenanigans), plus some related general cleanup that is not really necessary, and could be annoying. (I am prepared to defend my argument that BDB_CFLAGS is wrong here, and BDB_CPPFLAGS is right, but this could bite anybody who has gotten in the habit of -- or scripted -- setting the former.)~~
Ok, I have been convinced that I was too clever with the refactor and I have removed it. Now it's just the tiny change to fix the build on my local machine.
---
On OS X, when searching Homebrew keg-only packages for BDB 4.8, if we find it,
use BDB_CPPFLAGS and BDB_LIBS instead of CFLAGS and LIBS for the result. This
is (1) more correct, and (2) necessary in order to give this location
priority over other directories in the include search path, which may include
system include directories with other versions of BDB.
ACKs for top commit:
theuni:
ACK 8578c6fccd.
Tree-SHA512: a28f48fc81a25736f7e77c663f21cd9a6ae1cd115682031c5aa695c94cb5afa11920330a60cd6a54832822a2aec1eb23123ac2e2dcd4f0b3835aef9c9339ac97
The way we currently test with AC_CHECK_DECLS do not work with Clang:
```bash
configure:21492: clang++-10 -std=c++11 -c -g -O2 -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS conftest.cpp >&5
conftest.cpp💯10: error: builtin functions must be directly called
(void) __builtin_clz;
^
1 error generated.
```
This also removes the __builtin_clz() check, as we don't actually use
it anywhere, and it's trvial to re-add detection if we do start using
it at some point.
fa32adf9dc scripted-diff: TxoutType C++11 scoped enum class (MarcoFalke)
fa95a694c4 doc: Update outdated txnouttype documentation (MarcoFalke)
fa58469c77 rpc: Properly use underlying type in GetAllOutputTypes (MarcoFalke)
fa41c65702 rpc: Simplify GetAllOutputTypes with the Join helper (MarcoFalke)
Pull request description:
Non-scoped enums can accidentally and silently decay into an integral type. Also, the symbol names of the keys are exported to the surrounding (usually global) namespace.
Fix both issues by switching to an `enum class TxoutType` in a (mostly) scripted-diff.
ACKs for top commit:
practicalswift:
ACK fa32adf9dc -- patch looks correct
hebasto:
re-ACK fa32adf9dc, since fa5997bd6fc82e16b597ea96e3c5c665f1f174ab (https://github.com/bitcoin/bitcoin/pull/19114#pullrequestreview-421425198) rebased only (verified with `git range-diff`).
Tree-SHA512: f42a9db47f9be89fa4bdd8d2fb05a16726286d8b12e3d87327b67d723f91c7d5a57deb4b2ddae9e1d16fee7a5f8c00828b6dc8909c5db680fc5e0a3cf07cd465
6d35d0d18f doc: add release note for -getinfo displaying multiwallet balances (Jon Atack)
Pull request description:
Release note for #18594. This is one of the commits from #19089, which had one concept ACK and approach ACK since late May. It seems better to submit the changes atomically.
Top commit has no ACKs.
Tree-SHA512: 38616d14b02c39f4ee4b93bf14f72043423cef177b595e85181bc9dc610fbe19d8271f2d2c9e5e17bb46423ffe27746e8e510b13a23ae6fd0e5bc4418a00dafa
0ac09c9793 qt: Do not truncate node flag strings in debugwindow.ui peers details tab. (saibato)
Pull request description:
Fix: When fiddling around with new node flags other than the usual.
I saw that not all possible node flag strings i.e. the UNKNOWN[..] where
visible in peers details tab.
Since v18.2 fixed size was set to 300 and sliding is thereby limited.
A fix on my old linux cruft and small screen was to set minimumSize width to -1 or 0.
Qt will then autosize the slider to the max string length.
Thereby i had full display of all flags inclusive sliding without to fullscreen the window.
Not sure if this is even an issue for those who can afford big screens or high res macs?
Feedback welcome.
BTW: nice side effect now again easy to scroll trough long version names of the node.
can't wait to see strings like /Satoshi:0.23.99/NOX2NOX4NOX32 or what ever fits in the version string.
ACKs for top commit:
hebasto:
ACK 0ac09c9793, tested on Linux Mint 20 (x86_64, Qt 5.12.8).
promag:
Tested ACK 0ac09c9793 on macos.
Tree-SHA512: a1601b5e35f10b1fd9407b28142ca00c1b985a822be5d23be4d7d3376211450f06e17f962c44b8b40977f8f8bbbb701cac1c5abb4afb3618da76385dfac848a3
d906aaa117 qt: Fix regression in TransactionTableModel (Hennadii Stepanov)
Pull request description:
Since https://github.com/bitcoin/bitcoin/pull/17993 a crash is possible on exit.
Steps to reproduce:
- precondition: the old chain
- start `bitcoin-qt`
- wait until sync
- on main window: Menu -> File -> Quit
- crash
This PR is based on ryanofsky's [suggestion](https://github.com/bitcoin-core/gui/issues/7#issuecomment-646639251).
Fixes#7.
ACKs for top commit:
promag:
Code review ACK d906aaa117.
ryanofsky:
Code review ACK d906aaa117. Only changes are squashing, adding assert and adding const
vasild:
ACK d906aaa1
Tree-SHA512: 99a475fd90dff50407a58537fdc6099a2a074018e9078452bf86defc1a4b9e546aa94f916d242355900b21638c6cfef845598a5282661a9343556c4514eb155f
1087807b2b tests: Provide main(...) function in fuzzer (practicalswift)
Pull request description:
Provide `main(...)` function in fuzzer. Allow building uninstrumented harnesses with only `--enable-fuzz`.
This PR restores the behaviour to how things worked prior to #18008. #18008 worked around an macOS specific issue but did it in a way which unnecessarily affected platforms not in need of the workaround :)
Before this patch:
```
# Build uninstrumented fuzzing harness (no libFuzzer/AFL/other-fuzzer-instrumentation)
$ ./configure --enable-fuzz
$ make
CXXLD test/fuzz/span
/usr/lib/gcc/x86_64-linux-gnu/7/../../../x86_64-linux-gnu/Scrt1.o: In function `_start':
(.text+0x20): undefined reference to `main'
collect2: error: ld returned 1 exit status
Makefile:7244: recipe for target 'test/fuzz/span' failed
make[2]: *** [test/fuzz/span] Error 1
make[2]: *** Waiting for unfinished jobs....
$
```
After this patch:
```
# Build uninstrumented fuzzing harness (no libFuzzer/AFL/other-fuzzer-instrumentation)
$ ./configure --enable-fuzz
$ make
$ echo foo | src/test/fuzz/span
$
```
The examples above show the change in non-macOS functionality. macOS functionality is unaffected by this patch.
ACKs for top commit:
MarcoFalke:
ACK 1087807b2b
Tree-SHA512: 9c16ea32ffd378057c4fae9d9124636d11e3769374d340f68a1b761b9e3e3b8a33579e60425293c96b8911405d8b96ac3ed378e669ea4c47836af06892aca73d
25dac9fa65 doc: add release notes for explicit fee estimators and bumpfee change (Karl-Johan Alm)
05227a3554 tests for bumpfee / estimate_modes (Karl-Johan Alm)
3404c1b753 policy: optional FeeEstimateMode param to CFeeRate::ToString (Karl-Johan Alm)
6fcf448430 rpc/wallet: add two explicit modes to estimate_mode (Karl-Johan Alm)
b188d80c2d MOVEONLY: Make FeeEstimateMode available to CFeeRate (Karl-Johan Alm)
5d1a411eb1 fees: add FeeModes doc helper function (Karl-Johan Alm)
91f6d2bc8f rpc/wallet: add conf_target as alias to confTarget in bumpfee (Karl-Johan Alm)
69158b41fc added CURRENCY_ATOM to express minimum indivisible unit (Karl-Johan Alm)
Pull request description:
This lets users pick their own fees when using `sendtoaddress`/`sendmany` if they prefer this over the estimators.
ACKs for top commit:
Sjors:
re-utACK 25dac9fa65: rebased, more fancy C++,
jonatack:
ACK 25dac9fa65 I think this should be merged after all this time, even though it looks to me like there are needed follow-ups, fixes and test coverage to be added (see further down), which I don't mind helping out with, if wanted.
fjahr:
Code review ACK 25dac9fa65
Tree-SHA512: f31177e6cabf3187a43cdfe93477144f8e8385c7344613743cbbd16e8490d53ff5144aec7b9de6c9a65eb855b55e0f99d7f164dee4b6bf3cfea4dce51cf11d33
