aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-05-09build: Enable -Werror=sign-compareBen Woosley
Explicitly add -Wsign-compare as well - not required for all compilers, as GCC activates it under -Wall, but may impact clang, etc.
2020-05-09refactor: Rework asmap Interpret to avoid ptrdiff_tBen Woosley
2020-05-08test: Fix outstanding -Wsign-compare errorsBen Woosley
2020-05-08Merge #16224: gui: Bilingual GUI error messagesMarcoFalke
18bd83b1fee2eb47ed4ad05c91f2d6cc311fc9ad util: Cleanup translation.h (Hennadii Stepanov) e95e658b8ec6e02229691a1941d688e96d4df6af doc: Do not translate technical or extremely rare errors (Hennadii Stepanov) 7e923d47ba9891856b86bc9f718cf2f1f773bdf6 Make InitError bilingual (Hennadii Stepanov) 917ca93553917251e0fd59717a347c63cdfd8a14 Make ThreadSafe{MessageBox|Question} bilingual (Hennadii Stepanov) 23b9fa2e5ec0425980301d2eebad81e660a5ea39 gui: Add detailed text to BitcoinGUI::message (Hennadii Stepanov) Pull request description: This is an alternative to #15340 (it works with the `Chain` interface; see: https://github.com/bitcoin/bitcoin/pull/15340#issuecomment-502674004). Refs: - #16218 (partial fix) - https://github.com/bitcoin/bitcoin/pull/15894#issuecomment-487947077 This PR: - makes GUI error messages bilingual: user's native language + untranslated (i.e. English) - insures that only untranslated messages are written to the debug log file and to `stderr` (that is not the case on master). If a translated string is unavailable only an English string appears to a user. Here are some **examples** (updated): ![Screenshot from 2020-04-24 17-08-37](https://user-images.githubusercontent.com/32963518/80222043-e2458780-864e-11ea-83fc-197b7121dba5.png) ![Screenshot from 2020-04-24 17-12-17](https://user-images.githubusercontent.com/32963518/80222051-e5407800-864e-11ea-92f7-dfef1144becd.png) * `qt5ct: using qt5ct plugin` message is my local environment specific; please ignore it. --- Note for reviewers: `InitWarning()` is out of this PR scope. ACKs for top commit: Sjors: re-tACK 18bd83b1fee2eb47ed4ad05c91f2d6cc311fc9ad MarcoFalke: ACK 18bd83b1fee2eb47ed4ad05c91f2d6cc311fc9ad 🐦 Tree-SHA512: 3cc8ec44f84403e54b57d11714c86b0855ed90eb794b5472e432005073354b9e3f7b4e8e7bf347a4c21be47299dbc7170f2d0c4b80e308205ff09596e55a4f96
2020-05-08Merge #18864: Add v0.16.3 backwards compatibility test, bump v0.19.0.1 to ↵MarcoFalke
v0.19.1 d135c294764add81683ba47575f9a5dde7d7c07f [ci] make list of previous releases to download a setting (Sjors Provoost) 9c246b873c74834a121edba00fcaecf0cba6f9b4 [test] backwards compatibility: bump v0.19.0.1 to v0.19.1 (Sjors Provoost) 89a28e02fa46f3d5eb07ab02aa34aa95c6fcee11 [test] add v0.16.3 backwards compatibility test (Sjors Provoost) Pull request description: Thanks to #18774's `adjust_bitcoin_conf_for_pre_17` we can now test backwards compatibility for v0.16.3, both for sync and loading a recent wallet. This PR bumps v0.19.0.1 to v0.19.1. I also made the version list consistent for the `contrib/devtools/previous_release.sh` instruction, between both tests. ACKs for top commit: MarcoFalke: ACK d135c294764add81683ba47575f9a5dde7d7c07f Tree-SHA512: 5ff137a7a934237fa220f1c2807ce9abeeb75929266558bf3e4045bec7dfcd0a8747fa74d700065c568330b18badf58c60c308eb13d1eed444d4bbfe6decc48b
2020-05-08[ci] make list of previous releases to download a settingSjors Provoost
Co-Authored-By: MarcoFalke <falke.marco@gmail.com>
2020-05-07Merge #18905: travis: Remove s390xMarcoFalke
8c705ff1291ef7876ab1a939e2c7312aacc3dc37 travis: Remove s390x (MarcoFalke) Pull request description: Fixes #18868 Fixes #18016 Top commit has no ACKs. Tree-SHA512: 1007b761c7e01dd2b75aa34e92e01a92a84a100c0a3a53863c755d93b2438a74601e3f2083919b8a9cfc92fb104d0c8415fad3f6c9504c76b02ad2e9712660c0
2020-05-07travis: Remove s390xMarcoFalke
2020-05-07Merge #18743: depends: Add --sysroot option to mac os native compile flagsfanquake
1e94a2bcbc5ff8ae61eed9f31317ea534649116d depends: Add --sysroot option to mac os native compile flags (Russell Yanofsky) Pull request description: Catalina SDK clang stopped automatically searching the SDK include paths when invoked without `--sysroot`: - https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-594600985 - https://github.com/Homebrew/homebrew-core/issues/45061 This hasn't been a problem for current native depends packages because are passing their own `--sysroot` values, and hasn't been a problem for current host packages because they use `darwin_` commands instead of `build_darwin_` commands. But the current `build_darwin_CC` and `build_darwin_CXX` commands are still unnecessarily fragile, and incompatible with new native depends packages added in https://github.com/bitcoin/bitcoin/pull/18677. Cory Fields (theuni) suggested in https://github.com/bitcoin/bitcoin/pull/16367#issuecomment-595393546 switching compiler from SDK clang to native clang (from $PATH) to avoid this problem. This is easy and makes a certain amount of sense for building native packages, as opposed to host packages. But Michael (fanquake) pointed out in https://github.com/bitcoin/bitcoin/pull/18677#discussion_r409934309 that it would be inconsistent to switch to non-SDK compilers while still using other SDK tools like `ranlib` and `install_name_tool`. So simplest, minimal fix seems to be just adding the missing `--sysroot` option. ACKs for top commit: ryanofsky: > ACK [1e94a2b](https://github.com/bitcoin/bitcoin/commit/1e94a2bcbc5ff8ae61eed9f31317ea534649116d) - I think this change is ok, and I prefer it to the previous patch. fanquake: ACK 1e94a2bcbc5ff8ae61eed9f31317ea534649116d - I think this change is ok, and I prefer it to the previous patch. Thanks for the summary in the PR description. I played around with Xcode and the CLT; I think previously I didn't fully grok the slight differences between the two. Tree-SHA512: 4d4bbb7f49acb76d934a872a15b4e14f36290b508cb9e728815f959767ec174bcfb6d2ca7dcd995cc550d86980d64d4247ea5ecfca2301f0953006e50744fdb4
2020-05-07Merge #18899: travis: Remove valgrindMarcoFalke
fa082d0a57afedca9122fac4aecd6a3070f06b04 travis: Remove valgrind (MarcoFalke) Pull request description: When the valgrind run was added, it took 2 hours. Travis kindly raised the timeout limit to the maximum possible of 3 hours. Today, a full build of Bitcoin Core with all tests takes more than three hours. Thus, it is impossible to run all tests on travis. Moreover, the feedback loop for developers that create a pull request takes at least 2 hours, but in some cases (when the travis queue is full) until the next day. This is unacceptable. Fix both issues by removing the build from travis. Please note that the `ci/test/` configurations are *not* removed. They will stay in the repo and can be executed anywhere (just not on travis). ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/18899/commits/fa082d0a57afedca9122fac4aecd6a3070f06b04 jnewbery: utACK fa082d0a57afedca9122fac4aecd6a3070f06b04 Tree-SHA512: 9acaa0e2d3926014fadb7dd2e86c4e01df382e9399f6ae99f989fa609da66a77bdd1b75d6ff42d2686f38f730b8564e6dc722aa597a473290c9d30c2abe7ef0f
2020-05-07Merge #18535: build: remove -Qunused-arguments workaround for clang + ccachefanquake
a029805f57fa9a4ab9867c0d1e865675d57537c7 build: remove -Qunused-arguments workaround for clang + ccache (fanquake) Pull request description: This was added in 386efb7695debaf5f0f328a2e4c9abc342161665 to address spammy Clang warnings when building with ccache. The issue was addressed in [ccache 3.2](https://bugzilla.samba.org/show_bug.cgi?id=8118), and from a look at most major distros, it's only Debian Jessie that has a version of ccache older than that ([3.1](https://packages.debian.org/jessie/ccache)). Therefore I think it's acceptable to drop this workaround, and re-enable warnings for unused driver arguments (when compiling using Clang and ccache). ACKs for top commit: hebasto: ACK a029805f57fa9a4ab9867c0d1e865675d57537c7. vasild: utACK a029805f57fa9a4ab9867c0d1e865675d57537c7 Tree-SHA512: f887b9bd12f9c1c8d209943b86e8dafe33cfd1572912f2cafabe08ffe403973e48f0f7289280a8c6db9263c57aad43fbd4bb72f42db762eb090f3b1ef0538f43
2020-05-06travis: Remove valgrindMarcoFalke
2020-05-06Merge #18873: test: Fix intermittent sync_blocks failuresWladimir J. van der Laan
fa3f9a05660687bf4146e089050e944a1d6cbe3c test: Fix intermittent sync_blocks failures (MarcoFalke) Pull request description: Fixes #18872 Fixes #18737 Fixes #18801 See docstring for motivation and description ACKs for top commit: laanwj: Code review ACK fa3f9a05660687bf4146e089050e944a1d6cbe3c Tree-SHA512: acd52d386a6849f7ff1cb1a51a439dc3c76e0e3a4dd8d22030df0ebb09b44497c61c56331ff65724b695d82d86b0ebb24608f9e637008e5dacb7676b0c448889
2020-05-06Merge #17874: build: make linker checks more robustWladimir J. van der Laan
03da4c7781fe52cac48530ca292af4d794ae28e2 build: make linker checks more robust (Cory Fields) Pull request description: Check for a flag to turn linker warnings into errors. When flags are passed to linkers via the compiler driver using a -Wl,-foo flag, linker warnings may be swallowed rather than bubbling up. This is one of [Corys commits](https://github.com/theuni/bitcoin/commit/b9acd3d33e40de5d95901d591a8703a5974195ee) that I've modified to also add `-Wl,-fatal_warnings` for darwin. ACKs for top commit: vasild: re-ACK 03da4c778 Tree-SHA512: 212031d619ed88e52aaae30cf3b711681d72c4d670884406403605d1d86c784c84cb07e2e0d6c30926e659db8f14f8dabd5af3de5291637f8080d6dfee358248
2020-05-06Merge #18512: Improve asmap checks and add sanity checkWladimir J. van der Laan
748977690e0519110cda9628162a7ccf73a5934b Add asmap_direct fuzzer that tests Interpreter directly (Pieter Wuille) 7cf97fda154ba837933eb05be5aeecfb69a06641 Make asmap Interpreter errors fatal and fuzz test it (Pieter Wuille) c81aefc5377888c7ac4f29f570249fd6c2fdb352 Add additional effiency checks to sanity checker (Pieter Wuille) fffd8dca2de39ad4a683f0dce57cdca55ed2f600 Add asmap sanity checker (Pieter Wuille) 5feefbe6e7b6cdd809eba4074d41dc95a7035f7e Improve asmap Interpret checks and document failures (Pieter Wuille) 2b3dbfa5a63cb5a6625ec00294ebd933800f0255 Deal with decoding failures explicitly in asmap Interpret (Pieter Wuille) 1479007a335ab43af46f527d0543e254fc2a8e86 Introduce Instruction enum in asmap (Pieter Wuille) Pull request description: This improves/documents the failure cases inside the asmap interpreter. None of the changes are bug fixes (they only change behavior for corrupted asmap files), but they may make things easier to follow. In a second step, a sanity checker is added that effectively executes every potential code path through the asmap file, checking the same failure cases as the interpreter, and more. It takes around 30 ms to run for me for a 1.2 MB asmap file. I've verified that this accepts asmap files constructed by https://github.com/sipa/asmap/blob/master/buildmap.py with a large dataset, and no longer accepts it with 1 bit changed in it. ACKs for top commit: practicalswift: ACK 748977690e0519110cda9628162a7ccf73a5934b modulo feedback below. jonatack: ACK 748977690e0519110cda9628162a7ccf73a5934b code review, regular build/tests/ran bitcoin with -asmap, fuzz build/ran both fuzzers overnight. fjahr: ACK 748977690e0519110cda9628162a7ccf73a5934b Tree-SHA512: d876df3859735795c857c83e7155ba6851ce839bdfa10c18ce2698022cc493ce024b5578c1828e2a94bcdf2552c2f46c392a251ed086691b41959e62a6970821
2020-05-06Merge #18853: wallet: Fix typo in assert that is compile-time trueWladimir J. van der Laan
fa47cf9d95dc2c2822fc96df16f179176935bf96 wallet: Fix typo in assert that is compile-time true (MarcoFalke) Pull request description: Commit 92bcd70808b9cac56b184903aa6d37baf9641b37 presumably added a check that a `dest` of type `CNoDestination` implies an empty `scriptChange`. However, it accidentally checked for `boost::variant::empty`, which always returns false: https://www.boost.org/doc/libs/1_72_0/doc/html/boost/variant.html#id-1_3_46_5_4_1_1_16_2-bb ACKs for top commit: Sjors: utACK fa47cf9d95dc2c2822fc96df16f179176935bf96 Tree-SHA512: 9626b1e2947039853703932a362c2ee204e002d3344856eb93eef0e0f833401336f2dfa80fd43b83c8ec6eac624e6302aee771fb67aec436ba6483be02b8d615
2020-05-06Merge #18843: build: warn on potentially uninitialized readsWladimir J. van der Laan
71f183a49b714a28622277fa668d8f9f3dac0aae build: warn on potentially uninitialized reads (Vasil Dimov) Pull request description: * Enable `conditional-uninitialized` warning class to show potentially uninitialized reads. * Fix the sole such warning in Bitcoin Core in `GetRdRand()`: `r1` would be set to `0` on `rdrand` failure, so initializing it to `0` is a non-functional change. ACKs for top commit: practicalswift: ACK 71f183a49b714a28622277fa668d8f9f3dac0aae laanwj: ACK 71f183a49b714a28622277fa668d8f9f3dac0aae Tree-SHA512: 2c1d8caacd86424b16a9d92e5df19e0bedb51ae111eecad7e3bfa46447bc88e5fff1f32dacf6c4a28257ebb3d87e79f80f074ce2c523ce08b1a0c0a67ab44204
2020-05-06Merge #18854: doc: Fix typo in Coin doxygen commentWladimir J. van der Laan
fa09110ebb5e485b17a767fca198819fcbe7c16e doc: Fix typo in Coin doxygen comment (MarcoFalke) Pull request description: `CTxOutCompressor` has been renamed in commit 4de934b9b5b4be1bac8fe205f4ee9a79e772dc34, so rename it in the docs as well. ACKs for top commit: laanwj: ACK fa09110ebb5e485b17a767fca198819fcbe7c16e hebasto: ACK fa09110ebb5e485b17a767fca198819fcbe7c16e Tree-SHA512: e16a21ac3112a67ee7d5ffabb3f47103aed8f91fdebf1bf96311cd0b7bdb9b7323ed826bfa95517386d4128ff0ae2c7c13bad047a7c5a0cc2458be7a43119157
2020-05-06build: make linker checks more robustCory Fields
Check for a flag to turn linker warnings into errors. When flags are passed to linkers via the compiler driver using a -Wl,-foo flag, linker warnings may be swallowed rather than bubbling up. Co-authored-by: fanquake <fanquake@gmail.com>
2020-05-06Merge #18806: net: remove is{Empty,Full} flags from CBloomFilter, clarify ↵fanquake
CVE fix 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 net: remove is{Empty,Full} flags from CBloomFilter, clarify CVE fix (Sebastian Falbesoner) Pull request description: The BIP37 bloom filter class `CBloomFilter` contains two flags `isEmpty`/`isFull` together with an update method with the purpose to, according to the comments, "avoid wasting cpu", i.e. the mechanism should serve as an optimization for the trivial cases of empty (all bits zero) or full (all bits one) filters. However, the real reason of adding those flags (introduced with commit https://github.com/bitcoin/bitcoin/commit/37c6389c5a0ca63ae3573440ecdfe95d28ad8f07 by gmaxwell) was a _covert fix_ of [CVE-2013-5700](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-5700), a vulnerability that allowed a divide-by-zero remote node crash. According to gmaxwell himself (https://github.com/bitcoin/bitcoin/pull/9060#issuecomment-257749165): > the IsEmpty/IsFull optimizations were largely a pretextual optimization intended to make unexploitable a remote crash vulnerability (integer division by zero) that existed in the original bloom filtering code without disclosing it. I'm doubtful that they are all that useful. :) For more information on how to trigger this crash, see PR https://github.com/bitcoin/bitcoin/pull/18515 which contains a detailled description and a regression test. It has also been discussed on a [recent PR club meeting on fuzzing](https://bitcoincore.reviews/18521.html). The covert fix code already led to issues and PR based on the wrong assumption that the flags are there for optimization reasons (see #16886 and #16922). This PR gets rid of the flags and the update method and just focuses on the CVE fix itself, i.e. it can be seen as a revert of the covert fix commit modulo the actual fix. ACKs for top commit: meshcollider: utACK 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 laanwj: Concept and code review ACK 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 jkczyz: ACK 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 MarcoFalke: ACK 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 fjahr: Code review ACK 1ad8ea2b73134bdd8d6b50704a019d47ad2191d8 Tree-SHA512: 29f7ff9faece0285e11e16c024851f5bcb772dec64118ccc3f9067ec256267ec8e1b1e3105c7de2a72fd122c3b085e8fc840ab8f4e49813f1cc7a444df1867f7
2020-05-06Merge #18741: guix: Make source tarball using git-archivefanquake
bfe1ba2f5b36056e0c41edf8206b93d3d83098df rel-builds: Specify core.abbrev for git-rev-parse (Carl Dong) 27e63e01cce368d67092de8f0c736927d6f6aa69 build: Accomodate makensis v2.x (Carl Dong) 1f2c39a30e0f82046c7aecddfda3eb99cb536816 guix: Remove logical cores requirement (Carl Dong) a4f6ffa71e335d4b2a6bf525b7f416968f9cd9f7 lint: Also enable source statements for non-gitian (Carl Dong) d256f91cb1b0d6ff5170106b99b0266cbe51f5a2 rel-builds: Directly deploy win installer to OUTDIR (Carl Dong) fa791da02f9684e3fd554b687fb692ae6a23d65a nsis: Specify OutFile path only once (Carl Dong) 14701604d0904bc5bbf1c67de08f8ee6d3215523 guix: Expose GIT_COMMON_DIR in container as readonly (Carl Dong) f5a6ac4f48b18f93050d77bcb23f9cf45ec34647 guix: Make source tarball using git-archive (Carl Dong) 395c1137f630dc495ffb2752a23bc1dfd470ee53 gitian: Limit sourced script to just assignments (Carl Dong) Pull request description: Based on: #18556 Related: https://github.com/bitcoin/bitcoin/pull/17595#discussion_r399728721 ACKs for top commit: fanquake: ACK bfe1ba2f5b36056e0c41edf8206b93d3d83098df - I agree with Carl, and am going to merge this. I'd like for Linux Guix builds to be working again, and we can rebase #18818. Tree-SHA512: c87ada7e3de17ca0b692a91029b86573442ded5780fc081c214773f6b374a0cdbeaf6f6898c36669c2e247ee32aa7f82defb1180f8decac52c65f0c140f18674
2020-05-06Merge #9381: Remove CWalletTx merging logic from AddToWalletSamuel Dobson
28b112e9bd3fd1181c0720306051ba7efca8b436 Get rid of BindWallet (Russell Yanofsky) d002f9d15d938e78360ad906f2d74a249c7e923e Disable CWalletTx copy constructor (Russell Yanofsky) 65b9d8f8ddb5a838454efc8bdd6576f0deb65f6d Avoid copying CWalletTx in LoadToWallet (Russell Yanofsky) bd2fbc7cdbec46400341209f4cb7e69e5b2cee19 Get rid of unneeded CWalletTx::Init parameter (Russell Yanofsky) 2b9cba206594bfbcefcef0c88a0bf793819643bd Remove CWalletTx merging logic from AddToWallet (Russell Yanofsky) Pull request description: This is a pure refactoring, no behavior is changing. Instead of AddToWallet taking a temporary CWalletTx object and then potentially merging it with a pre-existing CWalletTx, have it take a callback so callers can update the pre-existing CWalletTx directly. This makes AddToWallet simpler because now it is only has to be concerned with saving CWalletTx objects and not merging them. This makes AddToWallet calls clearer because they can now make direct updates to CWalletTx entries without having to make temporary objects and then worry about how they will be merged. Motivation for this change came from the bumpfee PR #8456 where we wanted to be able to call AddToWallet to make a simple update to an existing transaction, but were reluctant to, because the existing CWalletTx merging logic did not apply and seemed dangerous try to update as part of that PR. After this refactoring, the bumpfee PR could call AddToWallet safely instead of implementing a duplicate AddToWallet function. This also allows getting rid of the CWalletTx copy constructor to prevent unintentional copying. ACKs for top commit: MarcoFalke: Anyway, re-ACK 28b112e9bd3fd1181c0720306051ba7efca8b436 Tree-SHA512: 528dd088714472a237500b200f4433db850bdb7fc29c5e5d81cae48072061dfb967f7c37edd90b33f24901239f9be982988547c1f8c80abc25fb243fbf7330ef
2020-05-05test: Fix intermittent sync_blocks failuresMarcoFalke
2020-05-05Merge #18885: contrib: Move optimize-pngs.py script to the maintainer repoMarcoFalke
fa13090d2048c9a1e475eb25de756763f8adddb8 contrib: Remove optimize-pngs.py script, which lives in the maintainer repo (MarcoFalke) Pull request description: Moved to https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/master/optimize-pngs.py Bitcoin Core should focus on the full node implementation, not on scripts to compress png images. This script is only used when new PNG files are added to the repo. This happens about once every two years. So fetching the script from the other repo should not be a burden, but removing it from this repo is going to cut down on the meta files we need to maintain in the main repo. ACKs for top commit: practicalswift: ACK fa13090d2048c9a1e475eb25de756763f8adddb8 -- `+0 lines, -82 lines` :) promag: ACK fa13090d2048c9a1e475eb25de756763f8adddb8. hebasto: ACK fa13090d2048c9a1e475eb25de756763f8adddb8, verified that script is already [moved](https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/56). Tree-SHA512: 37d111adae769bcddc6ae88041032d5a2b8b228fec67f555c8333c38de3992f5138b30bea868d7d6d6b7f966a47133e5853134373b149ab23cba3b8b560ecb31
2020-05-05Merge #18879: valgrind: remove outdated suppressionsMarcoFalke
d7120f7f78cda5ed1ab91f83e9b546de68dbee47 valgrind : remove duplicate BCLog::Logger suppression (fanquake) 708e3c7e85a666d5b8da8638a819c0f3973fcca4 valgrind: remove rest_blockhash_by_height suppression (fanquake) Pull request description: https://github.com/bitcoin/bitcoin/commit/708e3c7e85a666d5b8da8638a819c0f3973fcca4: `Suppress rest_blockhash_by_height` should no longer be needed after #18785. https://github.com/bitcoin/bitcoin/commit/d7120f7f78cda5ed1ab91f83e9b546de68dbee47 : Removes a duplicate `Suppress BCLog::Logger::StartLogging()` suppression that was added in #17770. ACKs for top commit: MarcoFalke: ACK d7120f7f78cda5ed1ab91f83e9b546de68dbee47 practicalswift: ACK d7120f7f78cda5ed1ab91f83e9b546de68dbee47 -- patch looks correct and valgrind Travis job is happy Tree-SHA512: 45f5b9fa64bf83cada3cd9ad33c245f660376d5b29f51a2531d83133940090df945f5ef26c5847d6ec024ffab9528d55573c5cf9ca5e73795f9abfc971b3d29b
2020-05-05contrib: Remove optimize-pngs.py script, which lives in the maintainer repoMarcoFalke
https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/master/optimize-pngs.py
2020-05-05Merge #18782: wallet: Make sure no DescriptorScriptPubKeyMan or ↵Samuel Dobson
WalletDescriptor members are left uninitialized after construction 2a780980983f4b4aaae75817e57e7ed308713561 wallet: Make sure no WalletDescriptor members are uninitialized after construction (practicalswift) ff046aeeba8d4f3ff210d37ba020616c12450ab3 wallet: Make sure no DescriptorScriptPubKeyMan members are uninitialized after construction (practicalswift) Pull request description: This is a small folllow-up to #16528 ("Native Descriptor Wallets using DescriptorScriptPubKeyMan") which was merged in to `master` a couple of hours ago. Make sure no `DescriptorScriptPubKeyMan` or `WalletDescriptor` members are left uninitialized after construction. Before this change `bool m_internal` was left uninitialized when using the `DescriptorScriptPubKeyMan(WalletStorage&, WalletDescriptor&)` ctor. The same goes for the now initialized integers which were left uninitialized when using the `WalletDescriptor()` ctor. ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/18782/commits/2a780980983f4b4aaae75817e57e7ed308713561 fjahr: Code review ACK 2a780980983f4b4aaae75817e57e7ed308713561 Sjors: utACK 2a78098 achow101: ACK 2a780980983f4b4aaae75817e57e7ed308713561 brakmic: Code review ACK 2a780980983f4b4aaae75817e57e7ed308713561 meshcollider: utACK 2a780980983f4b4aaae75817e57e7ed308713561 Tree-SHA512: c98e035268fdc7f65a423b73ac0cf010b0ef7c5e679b3cf170c1813efac8ab5c657dcbaf43c746770bea59e4772bfefe4caa834f1175260c39c7f35d92946ba5
2020-05-05valgrind : remove duplicate BCLog::Logger suppressionfanquake
This was added in #17770, but is identical to the supression above.
2020-05-05valgrind: remove rest_blockhash_by_height suppressionfanquake
This should no-longer be necessary after #18785.
2020-05-05util: Cleanup translation.hHennadii Stepanov
2020-05-05doc: Do not translate technical or extremely rare errorsHennadii Stepanov
2020-05-05Make InitError bilingualHennadii Stepanov
2020-05-05Make ThreadSafe{MessageBox|Question} bilingualHennadii Stepanov
2020-05-05gui: Add detailed text to BitcoinGUI::messageHennadii Stepanov
2020-05-05Merge #18088: build: ensure we aren't using GNU extensionsfanquake
0ae8f18dfe143051fec6ae10ea7df10142e3ff2f build: add -Wgnu to compile flags (fanquake) 3a0fd7726b8b916de6cce33bb67f48990575f923 Remove use of non-standard zero variadic macros (Ben Woosley) 49f6178c3e5e3ad54a419da9d8523207da17fc64 Drop unused LOG_TIME_MICROS helper (Ben Woosley) 5d4999951ee32e333b511245862628e80f83b703 prevector: Avoid unnamed struct, which is a GNU extension (DesWurstes) Pull request description: Since we [started using](https://github.com/bitcoin/bitcoin/pull/7165) the `ax_cxx_compile_stdcxx.m4` macro we've been passing `[noext]` to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn _off_ extension support so they would fail to compile". However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former. #### anonymous structs ```bash ./prevector.h:153:9: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct] struct { ``` This is fixed in https://github.com/bitcoin/bitcoin/commit/b849212c1ec01cc8633b8cdcd390da9b1051be0d. #### variadic macros ```bash ./undo.h:57:50: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments] ::Unserialize(s, VARINT(nVersionDummy)); ``` This is taken care of in #18087. The `LOG_TIME_*` macros introduced in #16805 make use of a [GNU extension](https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html). ```bash In file included from validation.cpp:22: ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) ^ ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] ./logging/timer.h:101:92: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments] BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__) ^ 6 warnings generated. ``` This is fixed in 081a0ab64eb442bc85c4d4a4d3bc2c8e97ac2a6d and 612e8e138b97fc5ad2f38847300132a8fc423c3f. #### prevention To ensure that usage doesn't creep back in we can add [`-Wgnu`](https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu) to our compile time flags, which will make Clang warn whenever it encounters GNU extensions. This would close #14130. Also related to #17230, where it's suggested we use a GNU extension, the `gnu::pure` attribute. ACKs for top commit: practicalswift: ACK 0ae8f18dfe143051fec6ae10ea7df10142e3ff2f -- diff looks correct MarcoFalke: ACK 0ae8f18dfe143051fec6ae10ea7df10142e3ff2f vasild: utACK 0ae8f18df dongcarl: ACK 0ae8f18dfe143051fec6ae10ea7df10142e3ff2f Tree-SHA512: c517404681ef8edf04c785731d26105bac9f3c9c958605aa24cbe399c649e7c5ee0c4aa8e714fd2b2d335e2fbea4d571e09b0dec36678ef871f0a6683ba6bb7f
2020-05-04rel-builds: Specify core.abbrev for git-rev-parseCarl Dong
Chose 12 because the kernel uses it: https://public-inbox.org/git/CA+55aFy0_pwtFOYS1Tmnxipw9ZkRNCQHmoYyegO00pjMiZQfbg@mail.gmail.com/raw And also because it's a nice number.
2020-05-04wallet: Fix typo in assert that is compile-time trueMarcoFalke
2020-05-04Merge #18443: lockedpool: avoid sensitive data in core files (FreeBSD)Wladimir J. van der Laan
f85203097f78d9daa1d35c4097a80beab31da2a4 lockedpool: avoid sensitive data in core files (FreeBSD) (Vasil Dimov) Pull request description: This is a followup to 23991ee53 / https://github.com/bitcoin/bitcoin/pull/15600 to also use madvise(2) on FreeBSD to avoid sensitive data allocated with secure_allocator ending up in core files in addition to preventing it from going to the swap. ACKs for top commit: sipa: ACK f85203097f78d9daa1d35c4097a80beab31da2a4 if someone verifies this works as intended on *BSD. laanwj: ACK f85203097f78d9daa1d35c4097a80beab31da2a4 practicalswift: Code-review ACK f85203097f78d9daa1d35c4097a80beab31da2a4 assuming a reviewer with FreeBSD access verifies that the PR goal is achieved :) Tree-SHA512: 2e6d4ab6a9fbe18732c8ba530eacc17f58128c97140758b80c905b5b838922a2bcaa5f9abc45ab69d5a1a2baa0cba322f006048b60a877228e089c7e64dadd2a
2020-05-04Merge #18699: wallet: Avoid translating RPC errorsWladimir J. van der Laan
fa2cce4391b0b1bda325f695bb45f7b565c8e8ea wallet: Remove trailing whitespace from potential translation strings (MarcoFalke) fa59cc1c977cce8f1f28374ac2169970ca78a35f wallet: Report full error message in wallettool (MarcoFalke) fae7776690c37104d2d4949429c5f84e6a33c576 wallet: Avoid translating RPC errors when creating txs (MarcoFalke) fae51a5c6f4270a1088e6295b10a8cc45988ae46 wallet: Avoid translating RPC errors when loading wallets (MarcoFalke) Pull request description: Common errors and warnings should be translated when displayed in the GUI, but not translated when displayed elsewhere. The wallet method `CreateWalletFromFile` does not know its caller, so this commit changes it to return a `bilingual_str` to the caller. Fixes #17072 ACKs for top commit: laanwj: ACK fa2cce4391b0b1bda325f695bb45f7b565c8e8ea, checked that no new translation messages are added compared to master. hebasto: ACK fa2cce4391b0b1bda325f695bb45f7b565c8e8ea Tree-SHA512: c6a943ae9c3689ea3c48c20d26de6e4970de0257a1f1eec57a2bded67a4af9dcc5c45b2d64659d6fb4c4bc4d8103e28483ea3d14bb850df8db0ff9e8e5c77ee2
2020-05-04Merge #18786: init: Remove boost from ThreadImportWladimir J. van der Laan
faec3dc2adc487af97c22408f9f0bfe33f44a230 init: Remove boost from ThreadImport (MarcoFalke) Pull request description: Can be tested by calling `-reindex` or `-loadblock` and then pressing `CTRL`+`C`. Should print something like: ``` ... 2020-04-27T19:34:31Z [loadblk] Reindexing block file blk00005.dat... ^C2020-04-27T19:34:32Z [loadblk] Shutdown requested. Exit ThreadImport 2020-04-27T19:34:32Z [qt-init] Interrupting HTTP server ... ``` ACKs for top commit: laanwj: Code review ACK faec3dc2adc487af97c22408f9f0bfe33f44a230 hebasto: ACK faec3dc2adc487af97c22408f9f0bfe33f44a230, tested on Linux Mint 19.3 (x86_64) both `bitcoind` and `bitcoin-qt` binaries. Tree-SHA512: e105af18d98296d82ec99f48e478cf44577e3c32f7e4b47617a7bc7cbf71d6becb92722f229a1be38d58ad29712704509ad9740d8ab8cd3104cf90057664b437
2020-05-04Merge #18783: tests: Add fuzzing harness for MessageSign, MessageVerify and ↵MarcoFalke
other functions in util/message.h 38e49ded8bd079f8da8b270b39f81cc5cf3ada11 tests: Add fuzzing harness for MessageSign, MessageVerify and other functions in util/message.h (practicalswift) Pull request description: Add fuzzing harness for `MessageSign`, `MessageVerify` and other functions in `util/message.h`. 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 :) ACKs for top commit: vasild: utACK 38e49ded8bd079f8da8b270b39f81cc5cf3ada11 Tree-SHA512: 4f83718365d9c7e772a4ccecb31817bf17117efae2bfaf6e9618ff17908def0c8b97b5fa2504d51ab38b2e6f82c046178dd751495cc37ab4779c0b1ac1a4d211
2020-05-04Merge #18859: Remove CCoinsViewCache::GetValueIn(...)MarcoFalke
b56607a89ba112083f2b0a7b64ab18d66b26e2be Remove CCoinsViewCache::GetValueIn(...) (practicalswift) Pull request description: Remove `CCoinsViewCache::GetValueIn(...)`. Fixes #18858. It seems like `GetValueIn` was added in #748 ("Pay-to-script-hash (OP_EVAL replacement)", merged in 2012) and the last use in validation code was removed in #8498 ("Near-Bugfix: Optimization: Minimize the number of times it is checked that no money...", merged in 2017). `CCoinsViewCache::GetValueIn(…)` performs money summation like this: ```c++ CAmount CCoinsViewCache::GetValueIn(const CTransaction& tx) const { if (tx.IsCoinBase()) return 0; CAmount nResult = 0; for (unsigned int i = 0; i < tx.vin.size(); i++) nResult += AccessCoin(tx.vin[i].prevout).out.nValue; return nResult; } ``` Note that no check is done to make sure that the resulting `nResult` is such that it stays within the money bounds (`MoneyRange(nResult)`), or that the summation does not trigger a signed integer overflow. Proof of concept output: ``` coins.cpp:243:17: runtime error: signed integer overflow: 9223200000000000000 + 2100000000000000 cannot be represented in type 'long' GetValueIn = -9221444073709551616 ``` Proof of concept code: ```c++ CMutableTransaction mutable_transaction; mutable_transaction.vin.resize(4393); Coin coin; coin.out.nValue = MAX_MONEY; assert(MoneyRange(coin.out.nValue)); CCoinsCacheEntry coins_cache_entry; coins_cache_entry.coin = coin; coins_cache_entry.flags = CCoinsCacheEntry::DIRTY; CCoinsView backend_coins_view; CCoinsViewCache coins_view_cache{&backend_coins_view}; CCoinsMap coins_map; coins_map.emplace(COutPoint{}, std::move(coins_cache_entry)); coins_view_cache.BatchWrite(coins_map, {}); const CAmount total_value_in = coins_view_cache.GetValueIn(CTransaction{mutable_transaction}); std::cout << "GetValueIn = " << total_value_in << std::endl; ``` ACKs for top commit: MarcoFalke: ACK b56607a89ba112083f2b0a7b64ab18d66b26e2be promag: Code review ACK b56607a89ba112083f2b0a7b64ab18d66b26e2be. jb55: ACK b56607a89ba112083f2b0a7b64ab18d66b26e2be hebasto: ACK b56607a89ba112083f2b0a7b64ab18d66b26e2be, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged. Tree-SHA512: 2c8402b5753ec96703d12c57c3eda8eccf999ed3519134a87faaf0838cfe44b94ef384296af2a524c06c8756c0245418d181af9083548e360905fac9d79215e6
2020-05-04[test] backwards compatibility: bump v0.19.0.1 to v0.19.1Sjors Provoost
2020-05-04[test] add v0.16.3 backwards compatibility testSjors Provoost
2020-05-04Merge #15768: gui: Add close window shortcutJonas Schnelli
f5a3a5b9ab362c58fa424261f313aa9cf46d2a98 gui: Add close window shortcut (Miguel Herranz) Pull request description: CMD+W is the standard shortcut in macOS to close a window without exiting the program. This adds support to use the shortcut in both main and debug windows. ACKs for top commit: jonasschnelli: Tested ACK f5a3a5b9ab362c58fa424261f313aa9cf46d2a98 hebasto: ACK f5a3a5b9ab362c58fa424261f313aa9cf46d2a98, tested on Linux Mint 19.3 by manually opening available dialogs and sub-windows, and applying the `Ctrl+W` shortcut. Also tested with "Minimize on close" option enabled / disabled. Tree-SHA512: 39851f6680cf97c334d5759c6f8597cb45685359417493ff8b0566672edbd32303fa15ac4260ec8ab5ea1458a600a329153014f25609e1db9cf399aa851ae2f9
2020-05-03Remove CCoinsViewCache::GetValueIn(...)practicalswift
2020-05-03build: warn on potentially uninitialized readsVasil Dimov
Enable -Wconditional-uninitialized to warn on potentially uninitialized reads. Fix the sole such warning in Bitcoin Core in GetRdRand(): r1 would be set to 0 on rdrand failure, so initializing it to 0 is a non-functional change. From "Intel 64 and IA-32 ArchitecturesSoftware Developer's Manual" [1], page 1711: "CF=1 indicates that the data in the destination is valid. Otherwise CF=0 and the data in the destination operand will be returned as zeros for the specified width." [1] https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf
2020-05-03Merge #18855: tests: feature_backwards_compatibility.py test downgrade after ↵MarcoFalke
upgrade 489ebfd7a16443d8263c048d55622da297df7c39 tests: feature_backwards_compatibility.py test downgrade after upgrade (Andrew Chow) Pull request description: After upgrading the node, try to go back to the original version to make sure that using a newer node version does not prevent the wallet file from being downgraded again. ACKs for top commit: MarcoFalke: ACK 489ebfd7a16443d8263c048d55622da297df7c39 Tree-SHA512: 86231de6514b3657912fd9d6621212166fd2b29b591fc97120092c548babcf1d6f50b5bd103b28cecde395a26809134f01c1a198725596c3626420de3fd1f017
2020-05-03Merge #18839: test: Fix intermittent issuesMarcoFalke
fab7ee39900d128cd305f530b643e2e62999ec74 test: Fix p2p_leak intermittent issue (MarcoFalke) fa8614aea9b8dc0f135a1221c7435014adb71c89 test: Fix intermittent p2p_segwit issue (MarcoFalke) Pull request description: Fixes #18801 Fixes #18802 ACKs for top commit: practicalswift: ACK fab7ee39900d128cd305f530b643e2e62999ec74 -- diff looks correct Tree-SHA512: b5d0473ec1133aded127bef4c0c90f6b55906327cb49cbbd0776c4971cd9d5c9375fb70cc3dc8dd1f4bd8b1245c4414e803dc0d29ed4553b00eb131d27c9c128
2020-05-03Merge #18617: test: add factor option to adjust test timeoutsMarcoFalke
2742c3428633b6ceaab6714635dc3adb74bf121b test: add factor option to adjust test timeouts (Harris) Pull request description: This PR adds a new option **factor** that can be used to adjust timeouts in various functional tests. Several timeouts and functions from `authproxy`, `mininode`, `test_node` and `util` have been adapted to use this option. The factor-option definition is located in `test_framework.py`. Fixes https://github.com/bitcoin/bitcoin/issues/18266 Also Fixes https://github.com/bitcoin/bitcoin/issues/18834 ACKs for top commit: MarcoFalke: Thanks! ACK 2742c3428633b6ceaab6714635dc3adb74bf121b Tree-SHA512: 6d8421933ba2ac1b7db00b70cf2bc242d9842c48121c11aadc30b0985c4a174c86a127d6402d0cd73b993559d60d4f747872d21f9510cf4806e008349780d3ef