aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-07-03walletdb: don't reinitialize desc cache with multiple cache entriesAndrew Chow
When loading descriptor caches, we would accidentally reinitialize the descriptor cache when seeing that one already exists. This should have only been initializing the cache when one does not exist. However this code itself is unnecessary as the act of looking up the cache to add to it will initialize it if it didn't already exist. This issue could be hit by trying to load a wallet that had imported a multisig descriptor. The wallet would fail to load. A test has been added to wallet_importdescriptors.py to catch this case. Another test case has also been added to check that loading a wallet with only single key descriptors works.
2020-07-03Merge #19424: ci: Run tsan ci config on cirrusMarcoFalke
fa8e6df282af0d396d75b03721f1b59a520ced19 ci: Run tsan ci config on cirrus (MarcoFalke) Pull request description: Fixes bitcoin-core/gui#12 Copied description from #19321: Currently it is not possible to use travis in forked repositories due to the 50 minute limit on builds. A fresh build (uncached) of the thread sanitizer config takes more than 50 minutes. One approach to fix this could be to throw away tests until the run time is less than 50 minutes. However, the risk of being blind of failures in the thrown away tests is not worth the gain. Also, to detect them, one has to run the tsan configuration nightly and failures could only be detected post-merge. Another approach would be to ask travis support to raise the limit for a forked repository. This is a tedious and manual one-by-one process, so I'd rather not. Finally, a different ci provider can be used, since the config files are designed to be platform-agnostic. This is what I picked. I kept all settings identical to the travis machine for now. Both providers run in the google cloud, so this should be a "move-only". ACKs for top commit: fanquake: ACK fa8e6df282af0d396d75b03721f1b59a520ced19 - my understanding is that test coverage remains the same. Just swapping providers to work-around the Travis time-limit in other repos. Tree-SHA512: 26fed248a4f743107160d3b9e5df57fa0be280fd065ae6fece83d254f59d58ccf3e11a245519d158da109c47b053f62ee8756215008541973c65dc28c4efb748
2020-07-03Merge #19413: refactor: Remove confusing BlockIndex globalMarcoFalke
fa0dfdf447d5b84a1849dc823d8508463600136a refactor: Remove confusing BlockIndex global (MarcoFalke) Pull request description: The global `::BlockIndex()` is problematic for several reasons: * It returns a mutable reference to the block tree, without the appropriate lock annotation (`m_block_index` is guarded by `cs_main`). The current code is fine, but in the future this might lead to accidental races and data corruption. * The rpc server shouldn't rely on node globals, but rather a context that is passed in to the RPC method. * Tests might want to spin up their own block tree, and thus should also not rely on a single global. Fix all issues by removing the global ACKs for top commit: promag: Code review ACK fa0dfdf447d5b84a1849dc823d8508463600136a. jonatack: re-ACK fa0dfdf Tree-SHA512: 8f158fc5e1c67e73588a21c25677b3fa0fe442313b13ec24b87054806c59607d6ba0c062a865ce3e0ee568706bd0d1faa84febda21aff5bcd65dab172f74c52f
2020-07-03Merge #19407: doc: explain why passing -mlinker-version is required when ↵fanquake
cross-compiling a8d39b88406e2047746366355666b5f603105a2e doc: explain why passing -mlinker-version is required (fanquake) Pull request description: I have been down a 🐇 hole. Closes #19359. When Clang is compiled, [a check is run](https://github.com/llvm/llvm-project/blob/release/8.x/clang/CMakeLists.txt#L353) to define `HOST_LINK_VERSION` as the output of `$CMAKE_LINKER -v`. Note the this is the version of the linker being used to compile Clang itself.. and this check is only run when compiling Clang for macOS. In the Clang driver, if `HOST_LINK_VERSION` has been defined, there is some additional runtime functionality. An `-mlinker-version` argument, with the value of `HOST_LINK_VERSION` [will be added to the linker arguments](https://github.com/llvm/llvm-project/blob/89de0d8dfbb9a6ff1f8b141ed70b563ecc094878/clang/lib/Driver/Driver.cpp#L382), if `-mlinker-version` has not been passed in by the user. This is a bit weird, as by default, you are setting `-mlinker-version` to the version of the linker that was used to build the Clang binary, not the linker which will be used when compiling. The commit which introduced the functionality, https://github.com/llvm/llvm-project/commit/628fcf4e3b79ba9f41dd1b49b1aba7a20b68fc0e, described it as a "hack", that should be replaced. However, that was 10 years ago, and the behaviour is still here. In the Darwin driver, [a check is done](https://github.com/llvm/llvm-project/blob/89de0d8dfbb9a6ff1f8b141ed70b563ecc094878/clang/lib/Driver/ToolChains/Darwin.cpp#L208) for the `-mlinker-version` argument. If there is no argument, the version will default to `0`. Given the above, this should never happen when using Clang for macOS. A series of comparisons are then performed, to check whether the linker version is modern enough to enable certain features, like [`-demangle`](https://github.com/llvm/llvm-project/blob/89de0d8dfbb9a6ff1f8b141ed70b563ecc094878/clang/lib/Driver/ToolChains/Darwin.cpp#L215). ### What this means #### macOS A Clang compiled for macOS, i.e `clang+llvm-8.0.0-x86_64-apple-darwin`, will have `HOST_LINKER_VERSION` set to the version of the linker used to compile Clang itself. At runtime, `-mlinker-version=HOST_LINKER_VERSION` will be added to the linker args, if `-mlinker-version` wasn't passed in. In the Darwin driver, additional arguments, like `-demangle`, will be added to the linker arguments, because `HOST_LINKER_VERSION` was likely some very modern version of `lld` or `ld64`. #### Linux (cross compilation in depends) A Clang compiled for Linux, i.e `clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04`, which we now use for macOS builds in depends, will behave differently. As it's built for Linux, `HOST_LINKER_VERSION` was not defined at compile time, and there will be no default behaviour of appending `-mlinker-version=HOST_LINKER_VERSION` to the linker args. Thus, unless you pass in `-mlinker-version` yourself, when the version checks are done in the Darwin driver, no modern linker features will be enabled, as the version will have defaulted to `0`. Therefore, it's important that we continue to pass `-mlinker-version="our LD64 version"` as part of our compilation flags, if we want to have "modern" linker features enabled for our macOS builds. #### Summary [Clang 8](https://releases.llvm.org/download.html#8.0.0). Building a macOS binary. Link line with path arguments trimmed. | | default behaviour | `-mlinker-version=100` (`-demangle threshold`) | `-mlinker-version=530` | | - | --------------- | --------------------- | ---------------------- | | macOS Clang | `-demangle -lto_library ../libLTO.dylib -no_deduplicate -dynamic -arch x86_64 -macosx_version_min 10.15.0 -o a.out ../test-b8b9b3.o -lc++ -lSystem ../libclang_rt.osx.a` | `-demangle -dynamic -arch x86_64 -macosx_version_min 10.15.0 -o a.out ../test-a66966.o -lc++ -lSystem ../libclang_rt.osx.a` | same as default | | Linux Clang | `-dynamic -arch x86_64 -macosx_version_min 10.12.0 -o a.out ../test-bfce57.o -lc++ -lSystem` | `-demangle -dynamic -arch x86_64 -macosx_version_min 10.12.0 -o a.out ../test-a846a3.o -lc++ -lSystem` | `-demangle -lto_library ../libLTO.dylib -no_deduplicate -dynamic -arch x86_64 -macosx_version_min 10.12.0 -o a.out ../test-de0280.o -lc++ -lSystem` | Note: Most links here are pointing to the 8.x branch of LLVM/Clang, as we are using that version in depends. Note: To add a little more confusion, you wont see `-mlinker-version X` in your compile flags, you'll see [`-target-linker-version X`](https://github.com/llvm/llvm-project/blob/431daedee4dcce0c096c400dbf8e64dfe7254fb6/clang/lib/Driver/ToolChains/Clang.cpp#L4777). ACKs for top commit: laanwj: ACK a8d39b88406e2047746366355666b5f603105a2e Tree-SHA512: 92f93079a5e59a0d561e74336b5cb03e3bf5a34437f5850283b9128c7624494b8285ec16290b1fa8103fe87f8789a53ce44b17902b8c1db5fde24d74b76fb168
2020-07-03Merge #18307: build: Require pkg-config for all of the hostsfanquake
92bc268e4af4ebcbde08567ea00e019ac509a769 build: Detect missed pkg-config early (Hennadii Stepanov) 1739eb23d8a6d272e70f95342323b6fe48b8eb6c build: Drop unused use_pkgconfig variable (Hennadii Stepanov) a661449a2eeaf88efda36b6a84084dcbfe5b24eb build: Drop use_pkgconfig check for libmultiprocess check (Hennadii Stepanov) 90b95e7929463d6127c1b24fe1bf457d750a045c build: Drop dead non-pkg-config code for libevent check (Hennadii Stepanov) 44a14afbb889633a6c9a322a5aeca2e1b2cbdbd8 build: Drop dead non-pkg-config code for qrencode check (Hennadii Stepanov) 10cbae0c399302b0f8b1aa847c4246ba60bf25ee build: Drop dead non-pkg-config code for ZMQ check (Hennadii Stepanov) 06cfc9cadf7c5dd43147e6525a348d5f2d299422 build: Fix indentation in UNIVALUE check (Hennadii Stepanov) 6fd2118e777d11cbc81a45313d1a7d6400e34f3f build: Drop dead non-pkg-config code for UNIVALUE check (Hennadii Stepanov) e9edbe4dbd8c24a779de7d92e5f10c870aab5511 build: Always use pkg-config (Hennadii Stepanov) 9e2e753b0605c8cd826381a362f0c7de56eea81f build: Always define ZMQ_STATIC for MinGW (Hennadii Stepanov) Pull request description: This PR: - is based on #18297 (already merged) - drops all of the non-pkg-config paths from the `configure` script Ref: #17768 ACKs for top commit: fanquake: ACK 92bc268e4af4ebcbde08567ea00e019ac509a769. I re-gitian-built. There are a couple follow-ups that I'll PR shortly. Thanks for addressing my feedback above. I took too long to get back to this. laanwj: ACK 92bc268e4af4ebcbde08567ea00e019ac509a769 Tree-SHA512: 83c2d9cf03518867a1ebf7e26a8fc5b6dd8962ef983fe0d84e0c7eb74717f4c36a834da02faf0e503ffd87167005351671cf040c0d4ddae57ee152a6ff84012b
2020-07-03Merge #18649: tests: Add std::locale::global to list of locale dependent ↵fanquake
functions in lint-locale-dependence.sh 54b5eb2b149a1f2a4a1dbdb9e0648c5a390d8e22 tests: Add std::locale::global to list of locale dependent functions in lint-locale-dependence.sh (practicalswift) Pull request description: Add `std::locale::global` to list of locale dependent functions in `lint-locale-dependence.sh`. We currently flag `setlocale(...)` as locale dependent, but prior to this commit we didn't flag `std::locale::global(...)` as such. In addition to setting the global C++ locale `std::locale::global(...)` also does the equivalent of `std::setlocale(LC_ALL, ...);`. Thus the functionality of `std::locale::global(...)` is a superset of `setlocale(...)` :) ACKs for top commit: MarcoFalke: ACK 54b5eb2b149a1f2a4a1dbdb9e0648c5a390d8e22, fine with me Tree-SHA512: bcf2f1c765add6ed09c3debca968b75eeea81602503f109c0f76ec98635911d453f4834a39e741703c3d470f123178e8952191a9b1a3429394b99c07765dcf1f
2020-07-03Merge #19215: psbt: Include and allow both non_witness_utxo and witness_utxo ↵Samuel Dobson
for segwit inputs 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 tests: Check that segwit inputs in psbt have both UTXO types (Andrew Chow) 46004790588c24174a0bec49b540d158ce163ffd psbt: always put a non_witness_utxo and don't remove it (Andrew Chow) 5279d8bc07d601fe6a67ad665fbc7591fe73c7de psbt: Allow both non_witness_utxo and witness_utxo (Andrew Chow) 72f6bec1da198764d4648a10a61c485e7ab65e9e rpc: show both UTXOs in decodepsbt (Andrew Chow) Pull request description: Due to recent changes to hardware wallets, the full previous transaction will need to be provided for segwit inputs. Since some software may be checking for the existence of a `witness_utxo` to determine whether to produce a segwit signature, we keep that field to ease the transition. Because all of the sanity checks implemented by the `IsSane` functions were related to having mixed segwit and non-segwit data in a PSBT, those functions are removed as those checks are no longer proper. Some tests are updated/removed to accommodate this and a simple test added to check that both UTXOs are being added to segwit inputs. As discussed in the wallet IRC meeting, our own signer will not require `non_witness_utxo` for segwit inputs. ACKs for top commit: Sjors: utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 (didn't retest compared to 836d6fc, but fortunately HWI's CI tracks our master branch, with a bunch of hardware wallet simulators) ryanofsky: Code review re-ACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3. No changes since last review, but now I understand the context better. I think it would good to improve the comments as suggested https://github.com/bitcoin/bitcoin/pull/19215#discussion_r447889473 and maybe refer to meshcollider: utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 Tree-SHA512: ccc1fd3c16ac3859f5aca4fa489bd40f68be0b81bbdc4dd51188bbf28827a8642dc8b605a37318e5f16cf40f1c4910052dace2f27eca21bb58435f02a443e940
2020-07-02ci: Run tsan ci config on cirrusMarcoFalke
2020-07-02Merge #18288: build: Add MemorySanitizer (MSan) in Travis to detect use of ↵MarcoFalke
uninitialized memory 870f0cd2a0534d54bba18190e9f024f88e832933 build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory (practicalswift) Pull request description: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory. First UBSan, then ASan followed by TSan... and now: yes, the wait is over -- **MSan is finally here!** :) Some historical context: * 2017: Continuous compilation with Clang Thread Safety analysis enabled (#10866, #10923) * 2018: Continuous testing with trapping on signed integer overflows (`-ftrapv`) (#12686) * 2018: Continuous testing of use of locale dependent functions (#13041) * 2018: Continuous testing of format strings (#13705) * 2018: Continuous compilation with MSVC `TreatWarningAsError` (#14151) * 2018: Continuous testing under UndefinedBehaviorSanitizer – UBSan (#14252, #14673, #17006) * 2018: Continuous testing under AddressSanitizer – ASan (#14794, #17205, #17674) * 2018: Continuous testing under ThreadSanitizer – TSan (#14829) * 2019: Continuous testing in an unsigned char environment (`-funsigned-char`) (#15134) * 2019: Continuous compile-time testing of assumptions we're making (#15391) * 2019: Continuous testing of fuzz test cases under Valgrind (#17633, #18159, #18166) * 2020: Finally... MemorySanitizer – MSAN! :) What is the next step? What tools should we add to CI to keep bugs from entering `master`? :) ACKs for top commit: MarcoFalke: ACK 870f0cd2a0534d54bba18190e9f024f88e832933 Tree-SHA512: 38327c8b75679d97d469fe42e704cacd1217447a5a603701dd8a58ee50b3be2c10248f8d68a479ed081c0c4b254589d3081c9183f991640b06ef689061f75578
2020-07-02Merge #19422: ci: Add tsan suppression for race in walletMarcoFalke
fa12d8d3edfbed8d5ce746e75af94eb92372f6b7 ci: Add tsan suppression for race in wallet (MarcoFalke) Pull request description: Workaround to fix #19417 (Intermittent CI failure) Top commit has no ACKs. Tree-SHA512: 2d68783d6db1bf425ce830cb23eab2f7fa3b9ee18cfb08665e4187196af571547206646dc6dfac0b4444e3dc6c4c13ae45efb09607d2d50df20a3d0a4eec98bd
2020-07-02Merge #19396: refactor: Remove confusing OutputType::CHANGE_AUTOWladimir J. van der Laan
fa927ff884ae373c676deed63180a8d238872cdc Enable Wswitch for OutputType (MarcoFalke) faddad71f648ed99734f4f8811bd4bc7232ca670 Remove confusing OutputType::CHANGE_AUTO (MarcoFalke) fa2eb383522249a5f4d48726c520cec5de496614 interfaces: Remove unused getDefaultChangeType (MarcoFalke) Pull request description: `OutputType::CHANGE_AUTO` is problematic for several reasons: * An output that is not change must never be described by `CHANGE_AUTO`. Simply allowing that option makes the code confusing and review harder than it needs to be. * To make review even harder, `CHANGE_AUTO` requires `-Wswitch` to be disabled for `OutputType` Fix both issues by removing `CHANGE_AUTO` and then enabling `-Wswitch` for `OutputType` ACKs for top commit: promag: Code review ACK fa927ff884ae373c676deed63180a8d238872cdc. laanwj: Code review ACK fa927ff884ae373c676deed63180a8d238872cdc Tree-SHA512: 24fd809757aa343866c94dafe9a7130b50cda4f77c97666d407f99b813f75b115a7d8e688a6bc2a737e87cba64ddd4e43f2b3c5538fd35fabb5845807bb39134
2020-07-01Merge #19427: ci: Run all tests on native mac againMarcoFalke
fa23fbb42fa710e6be405596e2abfc3a289d020d ci: Run all tests on native mac again (MarcoFalke) Pull request description: They should pass again after f6072e601af68f3eee307478ad22ff3960680656 ACKs for top commit: practicalswift: ACK fa23fbb42fa710e6be405596e2abfc3a289d020d -- Travis is happy and so am I Tree-SHA512: 49c16b6056d4e67d12a202744e1c56fee2788830213fe4a195955ad44c6b8ecce768a591463ffa0048821959a75b6fad4178629a8866c4a26799c4c8c13e933d
2020-07-01Merge bitcoin-core/gui#17: doc: Remove outdated comment in TransactionTablePrivMarcoFalke
faebb60b8d009e52d585cffd0f124a0f2fd1a66d 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 faebb60b8d009e52d585cffd0f124a0f2fd1a66d, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: cd6df24d49d17e58049ac9b261c5e07c8e85ed1aacb547b13c0e55139339d7fcc3b1f766ea2e27d758ea77deadc01f7e28781be1515323c82b9012cee8fd488b
2020-07-01Merge #19428: Fix boost/C++17 compile failuresMarcoFalke
fa575f34614f189d5b083d7efa6925d968f4df11 wallet: Replace boost::none with nullopt (MarcoFalke) fac7bdb75e69f731b89e848c11931b4087440283 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 fa575f34614f189d5b083d7efa6925d968f4df11 Tree-SHA512: 028e0f0a96c68f6e3394263dd720f0288fff6584592fdf9a7d9551b8358ee64f64b7c5cb802cc866eaa435e0247b66a5a5e54bfdc61a7c9769f287cfd3509966
2020-07-01Enable Wswitch for OutputTypeMarcoFalke
2020-07-01Remove confusing OutputType::CHANGE_AUTOMarcoFalke
2020-07-01wallet: Replace boost::none with nulloptMarcoFalke
2020-07-01script: Fix boost/C++17 compile failureMarcoFalke
script/standard.cpp:297:48: error: temporary of type 'boost::static_visitor<CScript>' has protected destructor return boost::apply_visitor(CScriptVisitor{}, dest); ^ /usr/include/boost/variant/static_visitor.hpp:53:5: note: declared protected here ~static_visitor() = default; ^ 1 error generated.
2020-07-01ci: Run all tests on native mac againMarcoFalke
2020-07-01Merge #19408: doc: Windows WSL build recommendation to temporarily disable ↵Wladimir J. van der Laan
Win32 PE support 21a65756f558a2bd50e71c6dfb04143533e59f76 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 21a65756f558a2bd50e71c6dfb04143533e59f76 Tree-SHA512: afb014be5a63fa9a827aed30acb2faab15feed34ed89c788a7f6ae6ab1b2238f99e075e6e281d0cc581914db3a4ecc3d5a3d26442f11a520e4e457a40e75e533
2020-07-01Merge #19028: test: Set -logthreadnames in unit testsWladimir J. van der Laan
99993489da9bc003b823bcab10e5f5297b369431 test: Set -logthreadnames in unit tests (MarcoFalke) fa4ea997b4da1ae0afafba223fff9efbeefaf555 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 99993489da9bc003b823bcab10e5f5297b369431 Tree-SHA512: 3bdbfc211da146da64b50b0826246aff5c611a84b69ab896a55b3c9d1adc92c5975da36ab92aee577df82e229c4326b477f4105bfdd1a5df4c9a0b018cf61602
2020-07-01Merge #19257: build: remove BIP70 configure optionWladimir J. van der Laan
c4ffcf07af19cd0c600b11dabd94e7e9d31ad072 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 c4ffcf07af19cd0c600b11dabd94e7e9d31ad072 MarcoFalke: ACK c4ffcf07af19cd0c600b11dabd94e7e9d31ad072 with or without the "catch-all reject" Tree-SHA512: a5dd4231ed97c9dd1984fb90d69a8725df2fdda0b963269b0575601c74528e5d820a4a863c428f8ede86eaae2a1606671fe1fcebdeb96b1023f7a5f899270284
2020-07-01Merge #19256: gui: change combiner for signals to optional_last_valueWladimir J. van der Laan
f1a0314c537791f202dfb7c1209f0e04ba7988c3 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 f1a0314c537791f202dfb7c1209f0e04ba7988c3 Tree-SHA512: 3600f85019a3591b141dc9207f8a7e66d16d9996cf97fdf08f5133a212d55c591955ab835ffbdca20b5d62711578bc305d5525c75546fa957f180192e2a80c1e
2020-07-01Merge #19308: wallet: BerkeleyBatch Handle cursor internallyWladimir J. van der Laan
ca24edfbc1941ed0a3c9586416dae4e84794eb66 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 ca24edfbc1941ed0a3c9586416dae4e84794eb66. Changes since last review: StartCursor rename, moving CloseCursor calls near returns promag: Code review ACK ca24edfbc1941ed0a3c9586416dae4e84794eb66. Tree-SHA512: f029b498c7f275aedca53ce7ade7cb99c82975fd6cad17346a4990fb3bcc54e2a5309b32053bd13def9ee464d331b036ac79abb8fc4fa561170c6cfc85283447
2020-07-01gui: change combiner for signals to optional_last_valueCory Fields
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>
2020-07-01Merge #19331: build: Do not include server symbols in walletWladimir J. van der Laan
faca73000fa8975c28f6be8be01957c1ae94ea62 ci: Install fixed version of clang-format for linters (MarcoFalke) fa4695da4c69646b58a8fa0b6b30146bb234deb8 build: Sort Makefile.am after renaming file (MarcoFalke) cccc2784a3bb10fa8e43be7e68207cafb12bd915 scripted-diff: Move ui_interface to the node lib (MarcoFalke) fa72ca6a9d90d66012765b0043fd819698b94ba8 qt: Remove unused includes (MarcoFalke) fac96e6450d595fe67168cb7afa7692da6cc9973 wallet: Do not include server symbols (MarcoFalke) fa0f6c58c1c6d10f04c4e65a424cc51ebca50a8c 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 faca73000fa8975c28f6be8be01957c1ae94ea62 hebasto: re-ACK faca73000fa8975c28f6be8be01957c1ae94ea62, since the [previous](https://github.com/bitcoin/bitcoin/pull/19331#pullrequestreview-434420539) review: Tree-SHA512: e9731f249425aaea50b6db5fc7622e10078cf006721bb87989cac190a2ff224412f6f8a7dd83efd018835302337611f5839e29e15bef366047ed591cef58dfb4
2020-07-01Merge #19368: test: improve functional tests compatibility with BSD/macOSWladimir J. van der Laan
3a7e79478ab41af7c53ce14d9fca9815bffe1f73 test: retry when write to a socket fails on macOS (Ivan Metlushko) 8cf9d15b823d91d2a74fc83832fccca2219342c9 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=html https://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 3a7e79478ab41af7c53ce14d9fca9815bffe1f73 Tree-SHA512: fefbe40ce94ab29f18bbbed2a434194b1384ffa5279b1d04db7a3708e3dd422bd9e450f1db3f95a1a851fac5a626ab533c6ebcfd7ede96f8ccae9e6f3e9fff92
2020-07-01Merge #19375: build: target Windows 7 when building libevent and fix ipv6 usageWladimir J. van der Laan
eb6b73540d1ee7ff5a6874dd0e35f9b30b68e3b8 build: pass _WIN32_WINNT=0x0601 when building libevent for Windows (fanquake) 03e056edcd1a7f7197a29068c52fa33fce12f7d7 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 eb6b73540d1ee7ff5a6874dd0e35f9b30b68e3b8 Tree-SHA512: e1e50f194911301981edaed0c216ed4efb9ebd4a1f9bc9b9f85bec7140b66c45c8666fd5db4aad359596559d4a08ab7c920e9d9736f3ecdbb841afc54e40586e
2020-06-30ci: Add tsan suppression for race in walletMarcoFalke
2020-06-30doc: Remove outdated comment in TransactionTablePrivMarcoFalke
2020-06-30build: pass _WIN32_WINNT=0x0601 when building libevent for Windowsfanquake
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.
2020-06-29refactor: Remove confusing BlockIndex globalMarcoFalke
2020-06-29Merge #19399: refactor: Replace RecursiveMutex with Mutex in rpc/server.cppWladimir J. van der Laan
6fdfeebcc79df62c8bf1cf4b6e9e97d6aefb3eb3 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 6fdfeebcc79df62c8bf1cf4b6e9e97d6aefb3eb3 MarcoFalke: ACK 6fdfeebcc79df62c8bf1cf4b6e9e97d6aefb3eb3 Tree-SHA512: 05a8ac58c0cd6a3c9afad9e06ad78059642e3e97715e129f379c0bf6dccdb58e70d05d965f23e7432fd3f02d7f97967a778ffb8e424837891d9d785a9e98964c
2020-06-29Merge #19412: test: move TEST_RUNNER_EXTRA into native tsan setupMarcoFalke
a92e48b02df545e620a7b1de74f647f46413d3fb 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 a92e48b02df545e620a7b1de74f647f46413d3fb practicalswift: ACK a92e48b02df545e620a7b1de74f647f46413d3fb -- patch looks correct hebasto: ACK a92e48b02df545e620a7b1de74f647f46413d3fb, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 86057bef2cc87c6acdbbf94f8cd7a5147510448c3e67aacde8daf247e3ccf649cfc5afbbd10693e084f426042d98150616c0e49bfa5f32b949dff9cebd2fd95d
2020-06-29Merge #19300: wallet: Handle concurrent wallet loadingMarcoFalke
9b009fae6e2eb0ab2ee7ce7882c3556a9ac363a7 qa: Test concurrent wallet loading (João Barbosa) b9971ae5853c1d62e09d976a8705f4f731290d85 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 9b009fae6e2eb0ab2ee7ce7882c3556a9ac363a7 I have not reviewed the code hebasto: ACK 9b009fae6e2eb0ab2ee7ce7882c3556a9ac363a7, tested on Linux Mint 20 (x86_64): ryanofsky: Code review good-but-not-ideal ACK 9b009fae6e2eb0ab2ee7ce7882c3556a9ac363a7 Tree-SHA512: 0ccd77b03c0926e4c4e51efb31e193b93cb4b9ffe8bac6bb018f7344c55dfd939b873b8cf5e657dca73e6202eb75aa672de2acb787cc133184b0b3b51e47b972
2020-06-29Merge #19333: refactor: Fix clang compile failureWladimir J. van der Laan
fa3b35a189c4a4fd9667ef0af1c7059471ac8b01 ci: Add test for clang-3.8 C++11 support (MarcoFalke) faa7431fee45b26f7ac2f5fd0b8874cb6afafbd4 refactor: Fix clang compile failure (MarcoFalke) Pull request description: Fix ``` script/standard.cpp:278:22: error: default initialization of an object of const type 'const (anonymous namespace)::CScriptVisitor' without a user-provided default constructor const CScriptVisitor g_script_visitor; ^ {} 1 error generated. ACKs for top commit: laanwj: ACK fa3b35a189c4a4fd9667ef0af1c7059471ac8b01 Tree-SHA512: b3251208945b44530224aadbc10fef1260b479c0b43a5e345501fbfd3579a9fe354b946090e023232852bbb99759da4429b58b137b7b286ddac6bd7960851f7f
2020-06-29test: move TEST_RUNNER_EXTRA into native tsan setupfanquake
2020-06-29Merge #19403: build: improve __builtin_clz* detectionWladimir J. van der Laan
9952242c03fe587b5dff46a9f770e319146103bf 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:100: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 9952242c03fe587b5dff46a9f770e319146103bf laanwj: ACK 9952242c03fe587b5dff46a9f770e319146103bf Tree-SHA512: 695abb1a694a01a25aaa483b4fffa7d598842f2ba4fe8630fbed9ce5450b915c33bf34bb16ad16a16b702dd7c91ebf49fe509a2498b9e28254fe0ec5177bbac0
2020-06-29Merge #19204: p2p: Reduce inv traffic during IBDMarcoFalke
fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72 net: Avoid wasting inv traffic during IBD (MarcoFalke) fa06d7e93489e61078cfb95ab767c001536a6e10 refactor: block import implies IsInitialBlockDownload (MarcoFalke) faba65e696a88e5626e587f4e63fa15500cbe4d0 Add ChainstateManager::ActiveChainstate (MarcoFalke) fabf3d64ff2bd14f762810316144bb9fd69c517c 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 fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72 ([`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 https://github.com/bitcoin/bitcoin/commit/fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72 jonatack: re-ACK fa525e4 checked diff `git range-diff 19612ca fa8a66c fa525e4`, re-reviewed, ran tests, ran a custom p2p IBD behavior test at https://github.com/jonatack/bitcoin/commit/9321e0f223ea87d21f6fa42b61bcc8d40a0943de. hebasto: re-ACK fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72, only rebased since the [previous](https://github.com/bitcoin/bitcoin/pull/19204#pullrequestreview-429519667) review (verified with `git range-diff`). Tree-SHA512: 2c22a5def9822396fca45d808b165b636f1143c4bdb2eaa5c7e977f1f18e8b10c86d4c180da488def38416cf3076a26de15014dfd4d86b2a7e5af88c74afb8eb
2020-06-29Merge #19394: build: Remove unused RES_IMAGESMarcoFalke
53361ddc7591f35df36cc71d11c24f826f209e11 [build] Remove unused RES_IMAGES (Bushstar) Pull request description: Remove RES_IMAGES. Seems to be unused since 2015 in the commit below. https://github.com/bitcoin/bitcoin/commit/98c222b5aa94543fce683b989356b0d8ad1f1d22#diff-9a4f3a253de77bf90b107bdf5283ebc3R317 The src/qt/res/images to which it was used with is no longer present either. ACKs for top commit: hebasto: ACK 53361ddc7591f35df36cc71d11c24f826f209e11, tested on Linux Mint 20 (x86_64). Tree-SHA512: d2f09ae225a4c6c171e1aae4c4a444064dc0502e96130e04ccb718f9fcf611a287c56630dec3e9a8937b5e29040d931a237da36180d2343c23cef30359e46323
2020-06-29Merge #19367: doc: Span pitfallsWladimir J. van der Laan
fab57e2b9bc4577fcfcd9fbddbc35d96046c5d88 doc: Mention Span in developer-notes.md (Pieter Wuille) 3502a60418858a8281ddf2f9cd59daa8f01d2fa8 doc: Document Span pitfalls (Pieter Wuille) Pull request description: This is an attempt to document pitfalls with the use of `Span`, following up on comments like https://github.com/bitcoin/bitcoin/pull/18468#issuecomment-622846597 and https://github.com/bitcoin/bitcoin/pull/18468#discussion_r442998211 ACKs for top commit: laanwj: ACK fab57e2b9bc4577fcfcd9fbddbc35d96046c5d88 Tree-SHA512: 8f6f277d6d88921852334853c2b7ced97e83d3222ce40c9fe12dfef508945f26269b90ae091439ebffddf03f939797cb28126b2387f77959069ef8909c25ab53
2020-06-29Add Windows WSL build recommendation to temporarily disable Win32 PE support.Aaron Clauson
2020-06-29doc: explain why passing -mlinker-version is requiredfanquake
2020-06-29Merge #19371: ci: Increase test timeout for sanitizer configsfanquake
fa74a54fad7abfe3b0c98c5a6e4780d63d35b13f ci: Increase test timeout for sanitizer configs (MarcoFalke) Pull request description: Hopefully fixes #19369 ACKs for top commit: practicalswift: ACK fa74a54fad7abfe3b0c98c5a6e4780d63d35b13f -- patch looks correct! fanquake: ACK fa74a54fad7abfe3b0c98c5a6e4780d63d35b13f - 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
2020-06-29Merge #19344: docs: update testgen usage examplefanquake
e12e970df6fcae08ff8008812cdeef600d6b2db8 docs: match usage text to script and location (Peter Bushnell) Pull request description: Update the usage text in the README to match the usage text in the Python script. https://github.com/bitcoin/bitcoin/blob/02b26ba1c119c7732f09f09e3b94f75effa569c0/contrib/testgen/gen_key_io_test_vectors.py#L9 https://github.com/bitcoin/bitcoin/blob/02b26ba1c119c7732f09f09e3b94f75effa569c0/contrib/testgen/gen_key_io_test_vectors.py#L10 Also to match the file names in the actual destination. https://github.com/bitcoin/bitcoin/blob/02b26ba1c119c7732f09f09e3b94f75effa569c0/src/test/data/key_io_valid.json https://github.com/bitcoin/bitcoin/blob/02b26ba1c119c7732f09f09e3b94f75effa569c0/src/test/data/key_io_invalid.json Following the README usage text generates new files when the user is likely to have wanted to update the existing files. ACKs for top commit: fanquake: ACK e12e970df6fcae08ff8008812cdeef600d6b2db8 - this looks correct. Tree-SHA512: b7ab61e19a54597a8fbd1844b9cfaef78879e53b882eefe4e0140fa115674df7f061e468835186963b89c963244a17d922f2ad0829b10f62b84f02019ee33edb
2020-06-29Merge #19390: doc/REST-interface: Remove stale infofanquake
fd9c213c6e42cedd8a03c2f721ff46790cded76b doc/REST-interface: Remove stale info (Luke Dashjr) Pull request description: Clean merge to 0.19+ ACKs for top commit: fanquake: ACK fd9c213c6e42cedd8a03c2f721ff46790cded76b MarcoFalke: ACK fd9c213c6e42cedd8a03c2f721ff46790cded76b Tree-SHA512: ac3ffaa72226380ed8b8ab505518d0dc4350bfcc4625dfd27a2350fbb972a8d2bb4255307926eb331c49232023bcb283a659f0d87e4ecbf654982341242f7d36
2020-06-29Merge #19356: build: Fix search for brew-installed BDB 4 on OS Xfanquake
8578c6fccd11404412d2c60f9bede311b79ca0d0 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 8578c6fccd11404412d2c60f9bede311b79ca0d0. Tree-SHA512: a28f48fc81a25736f7e77c663f21cd9a6ae1cd115682031c5aa695c94cb5afa11920330a60cd6a54832822a2aec1eb23123ac2e2dcd4f0b3835aef9c9339ac97
2020-06-29[build] Remove unused RES_IMAGESBushstar
2020-06-29build: improve builtin_clz* detectionfanquake
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:100: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.
2020-06-28Merge #19114: scripted-diff: TxoutType C++11 scoped enum classMarcoFalke
fa32adf9dc25540ad27f5b82654c7057d7738627 scripted-diff: TxoutType C++11 scoped enum class (MarcoFalke) fa95a694c492b267e4038674fd3f338dd215ab48 doc: Update outdated txnouttype documentation (MarcoFalke) fa58469c770d8c935a86462634e4e8cd806aa6e3 rpc: Properly use underlying type in GetAllOutputTypes (MarcoFalke) fa41c657022b8f99c8e6718a0e33c5838c412a0b 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 fa32adf9dc25540ad27f5b82654c7057d7738627 -- patch looks correct hebasto: re-ACK fa32adf9dc25540ad27f5b82654c7057d7738627, since fa5997bd6fc82e16b597ea96e3c5c665f1f174ab (https://github.com/bitcoin/bitcoin/pull/19114#pullrequestreview-421425198) rebased only (verified with `git range-diff`). Tree-SHA512: f42a9db47f9be89fa4bdd8d2fb05a16726286d8b12e3d87327b67d723f91c7d5a57deb4b2ddae9e1d16fee7a5f8c00828b6dc8909c5db680fc5e0a3cf07cd465
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-08 03:58:18 +0000