Age | Commit message (Collapse) | Author |
|
b9ee63c71b751fd67da777774ea8c0b27e7db2f8 Make descriptor test deterministic (David Reikher)
Pull request description:
This is an improvement to a test, inspired by #14343 - removing non determinism from a test.
The test `descriptor_test` is non-deterministic, as it relies on the `MaybeUseHInsteadOfApostrophy` function which randomly either swaps all apostrophes with 'h' or doesn't at all in a descriptor. This fix makes both cases always run, if an apostrophe is found in a test descriptor.
This does not reduce test coverage but removes the non-determinism.
Additionally, the `MaybeUseHInsteadOfApostrophy` function removed the checksum if found at the end of a descriptor when the apostrophes are swapped by 'h's, since after being swapped the checksum is no longer correct. I instead added re-calculation of the checksum using the `DescriptorChecksum` function, which adds coverage for the case of a descriptors having 'h's instead of apostrophes and a checksum. This was previously lacking.
To achieve this I had to move `DescriptorChecksum` and `PolyMod` out of the anonymous namespace in descriptor.cpp to make `DescriptorChecksum` accessible in descriptor_tests.cpp.
All tests complete successfully (functional as well as unit tests).
ACKs for top commit:
achow101:
Code Review ACK b9ee63c71b751fd67da777774ea8c0b27e7db2f8
Tree-SHA512: 992c73a6644a07bfe7c72301ee2666f3c4845a012aaedd7a099a05cea8bdac84fa8280b28e44a7856260c00c0be1a6f1b6768f5694c2a22edf4c489e53fec424
|
|
faebf6271467048dc8a9a0c526a0f8565023a966 rpc: Use Join helper in rpc/util (MarcoFalke)
fa8cd6f9c13319baca467864661982a3dfb2320c util: Add Join helper to join a list of strings (MarcoFalke)
Pull request description:
We have a lot of enumerations in the code and sometimes those enumerations need to be mentioned in the RPC or command line documentation. Previously, each caller would have a couple of lines inline to join the strings or the joined string is hardcoded in the documentation. A helper to join strings would make code such as https://github.com/bitcoin/bitcoin/pull/16629#discussion_r315852446 less verbose and easier to read.
Also, warnings commonly accumulate in complex RPCs, since a warning doesn't lead to an early return. A helper to join those warnings would make code such as https://github.com/bitcoin/bitcoin/pull/16394/files#r309324997 less verbose and easier to read.
ACKs for top commit:
practicalswift:
ACK faebf6271467048dc8a9a0c526a0f8565023a966
Tree-SHA512: 80f2db86a05c63b686f510585c1c631250271a8958fd71fafaac91559ffd2ec25d609bf7d53412ba27f87eff5893ac9dd9c2f296fc0c73581556e1d6a734a36f
|
|
153d9dd9acffa95bb97a4b1579bd20237fdc9c52 refactor: replace qLowerBound & qUpperBound with std:: upper_bound & lower_bound (fanquake)
59373e3e94015316bcaa03a7b9c2e6f442641720 refactor: replace qSort with std::sort (fanquake)
fea33cbbdfb4673033f3414bf1613591ff654aac refactor: replace qStableSort with std::stable_sort (fanquake)
Pull request description:
`qStablesort`, `qSort`, `qLowerBound` and `qUpperBound` have been marked as obsolete since at least Qt 5.9: [Obsolete Members for QtAlgorithms](https://doc.qt.io/qt-5.9/qtalgorithms-obsolete.html).
This pull request replaces their usage with the suggested `std::` replacements.
This also removes some warning spam when compiling against newer Qt (5.13.0 via brew):
```bash
CXX qt/libbitcoinqt_a-walletcontroller.o
qt/transactiontablemodel.cpp:96:52: warning: 'qLowerBound<QList<TransactionRecord>::iterator, uint256, TxLessThan>' is deprecated: Use std::lower_bound [-Wdeprecated-declarations]
QList<TransactionRecord>::iterator lower = qLowerBound(
qt/transactiontablemodel.cpp:98:52: warning: 'qUpperBound<QList<TransactionRecord>::iterator, uint256, TxLessThan>' is deprecated: Use std::upper_bound [-Wdeprecated-declarations]
QList<TransactionRecord>::iterator upper = qUpperBound(
```
```bash
CXX qt/libbitcoinqt_a-moc_walletcontroller.o
qt/bantablemodel.cpp:64:13: warning: 'qStableSort<QList<CCombinedBan>::iterator, BannedNodeLessThan>' is deprecated: Use std::stable_sort [-Wdeprecated-declarations]
qStableSort(cachedBanlist.begin(), cachedBanlist.end(), BannedNodeLessThan(sortColumn, sortOrder));
```
```bash
CXX qt/libbitcoinqt_a-sendcoinsentry.o
qt/recentrequeststablemodel.cpp:205:5: warning: 'qSort<QList<RecentRequestEntry>::iterator, RecentRequestEntryLessThan>' is deprecated: Use std::sort [-Wdeprecated-declarations]
qSort(list.begin(), list.end(), RecentRequestEntryLessThan(column, order));
```
ACKs for top commit:
hebasto:
ACK 153d9dd9acffa95bb97a4b1579bd20237fdc9c52
promag:
ACK 153d9dd9acffa95bb97a4b1579bd20237fdc9c52.
jonasschnelli:
utACK 153d9dd9acffa95bb97a4b1579bd20237fdc9c52
Tree-SHA512: 22f7290ed798ce8b0f5f313405377845d4c8e48dc8687be7464e27fff53363b451a40e9e18910a8c3b4b9d4dcc236a366c92e7d171fcb8576c48f149a1886c26
|
|
|
|
|
|
|
|
6011c9d72d1df5c2cd09de6f85c21eb4f7eb1ba8 QA: fix rpc_setban.py race (Jonas Schnelli)
Pull request description:
The new `rpc_setban.py` test failes regularly on CIs due to a race between injecting the ban and testing the log "on the other side".
The problem is, that the test immediately after the `addnode` command on node0 checks for the `dropped (banned)` entry on node1 (without giving some time).
Adding a 2 seconds sleep seems to solve the race (I guess there is no better event-driven delay).
Example of a failed test: https://bitcoinbuilds.org/index.php?ansilog=bf743910-103f-4b54-9a97-960c471061bd.log#l2906
Top commit has no ACKs.
Tree-SHA512: 680f8ea3e5ddb07e93f824f1aeff4a459e25e6c14715a39fc7670e50506d7cf25925348672c5c2d8ba3e1243ccf5effbc2456bcd094fb96868349f8d26e008f1
|
|
|
|
2dbfb37b407ed23b517f507d78fb77334142dce5 Fix Char as Bool in interfaces (Jeremy Rubin)
Pull request description:
In a few places in src/wallet/wallet.h, we use a char when semantically we want a bool.
This is kind of an issue because it means we can unserialize the same transaction with different fFromMe flags (as differing chars) and evaluate the following section in wallet/wallet.cpp
```c++
if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe)
{
wtx.fFromMe = wtxIn.fFromMe;
fUpdated = true;
}
```
incorrectly (triggering an fUpdated where both fFromMe values represent true, via different chars).
I don't think this is a vulnerability, but it's just a little messy and unsemantic, and could lead to issues with stored wtxIns not being findable in a map by their hash.
The serialize/unserialize code for bool internally uses a char, so it should be safe to make this substitution.
NOTE: Technically, this is a behavior change -- I haven't checked too closely that nowhere is depending on storing information in this char. Theoretically, this could break something because after this change a tx unserialized with such a char would preserve it's value, but now it is converted to a ~true~ canonical bool.
ACKs for top commit:
achow101:
Code review ACK 2dbfb37b407ed23b517f507d78fb77334142dce5
meshcollider:
Code review ACK 2dbfb37b407ed23b517f507d78fb77334142dce5
Tree-SHA512: 8c0dc9cf672aa2276c694facbf50febe7456eaa8bf2bd2504f81a61052264b8b30cdb5326e1936893adc3d33504667aee3c7e207a194c71d87b3e7b5fe199c9d
|
|
Changed MaybeUseHInsteadOfApostrophy to UseHInsteadOfApostrophe.
This function now always replaces apostrophes with 'h'.
The original Check function was renamed to DoCheck and it's
called with an additional parameter which tells it to either
leave the prv and pub arguments as is or replace the apostrophes
with 'h'. The test runs with apostrophes replaced in prv only,
pub only, prv and pub and without replacement at all. Replacement
of apostrophes in a descriptor and then running DoCheck is conditional
on whether apostrophes are found in that descriptor.
Additionally, instead of dropping the checksum recalculate it
after replacing apostrophes with 'h' in the function UseHInsteadOfApostrophe
using the GetDescriptorChecksum function. That way, this also
introduces an indirect unit test to GetDescriptoChecksum.
|
|
clean-local
c84ff23c6daade5145d960e1ac26596ddede2da1 build: ignore macOS make deploy artefacts & add them to clean-local (fanquake)
Pull request description:
ACKs for top commit:
hebasto:
ACK c84ff23c6daade5145d960e1ac26596ddede2da1, tested on Linux Mint 19.2:
dongcarl:
ACK c84ff23c6daade5145d960e1ac26596ddede2da1
Tree-SHA512: 5a6525a5e433a7279807cc02d3f47bb7f4bd5da8142a945b21d3bfa71938cac823569b34b071e21525a817a187fb02a562bb1c5a9761e2019a2b0900a51a8315
|
|
|
|
|
|
17d178fb9463c195c822614eb0245188e52f8371 doc: add release-notes for getmempoolentry weight field addition (fanquake)
9c9cc2bd201afc7d519778cfcdeba5c81faa49f4 qa: Add RPC tests for weight in mempool entry (Daniel Edgecumbe)
54aaa7883cb61d414627248e5e410180bfd8fa67 RPC: add weight to mempool entry output (Daniel Edgecumbe)
Pull request description:
Rebase of #14649 (which itself was a rebase of #11256).
Squash the two test related commits, and swapped out `size` usage for `vsize`.
Added a commit with release notes.
ACKs for top commit:
emilengler:
Concept ACK 17d178f
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/16647/commits/17d178fb9463c195c822614eb0245188e52f8371
meshcollider:
utACK 17d178fb9463c195c822614eb0245188e52f8371
Tree-SHA512: 1d354c9837e0ad0afa40325de9329b9e62688d5eab4d9e1cf9b46d8ae29d08f42d903ab37a41751c2ea8f9034231b21095881b1f5d911cb542b8b06bc85dc7cd
|
|
b6e9ff899677770741d94f1cf4f61ebb13fc453f build: Remove src/obj directory from repository (Wladimir J. van der Laan)
Pull request description:
This directory is automatically created by the build process (in the build target directory, see #16588) and doesn't need to be in the repository nor in the tarballs.
Move associated ignore directive to top-level `.gitignore` file.
ACKs for top commit:
hebasto:
Concept ACK b6e9ff899677770741d94f1cf4f61ebb13fc453f
Tree-SHA512: 5f3f5a0e8f19ecf925eb16cab327c3023b8512731bbaad5875828da7a25fdda1b77f6fbd06c002a383913627dc9b552f09ad27c57bcf0cb020ed3b1f506e5fef
|
|
8616c81f08c8118003e035808b3f237b3bb211a9 doc: initial RapidCheck property-based testing documentation (Jon Atack)
Pull request description:
This commit proposes documentation that would have been helpful to me when setting up RapidCheck to work correctly with Bitcoin Core. I tested these instructions repeatedly with Linux (Debian 4.19 / GCC 8.3) and macOS (10.14.6 / AppleClang 10).
The markdown version can be seen at https://github.com/jonatack/bitcoin/blob/rapidcheck-documentation/doc/rapidcheck.md.
ACKs for top commit:
fanquake:
ACK 8616c81f08c8118003e035808b3f237b3bb211a9 - Thanks for updating.
Tree-SHA512: 9c2a65f0eb99f59e9adfea82855f217727a8a9d30618c1c0bdd2a73ae9c1ee61bc7efd59fc8703a666d182ad4220e22d6034ac3109311e6573dad00dfa4b06ea
|
|
ffea41f5301d5582665cf10ba5c2b9547a1443de Enable all tests in feature_config_args.py (Hennadii Stepanov)
66f5c17f8a3fe06fc65191e379ffc04e43cbbc86 Use CheckDataDirOption() for code uniformity (Hennadii Stepanov)
7e33a18a34b1a9b0f115076c142661d6d30c0585 Fix datadir handling in bitcoin-cli (Hennadii Stepanov)
b28dada37465c0a773cf08b0e6766f0081bcb943 Fix datadir handling in bitcoin-qt (Hennadii Stepanov)
50824093bb2d68fe1393dfd636fab5f8795faa5d Fix datadir handling in bitcoind (Hennadii Stepanov)
740d41ce9f7fdf209366e930bd0fdcc6b1bc6b79 Add CheckDataDirOption() function (Hennadii Stepanov)
c1f325126cf51d28dce8da74bfdf5cd05ab237ea Return absolute path early in AbsPathForConfigVal (Hennadii Stepanov)
Pull request description:
Fix #15240, see: https://github.com/bitcoin/bitcoin/issues/15240#issuecomment-487353760
Fix #15745
Fix broken `feature_config_args.py` tests (disabled by MarcoFalke@fabe28a0cdcfa13e0e595a0905e3642a960d3077). All test are enabled now.
This PR is alternative to #13621.
User's `$HOME` directory is not touched unnecessarily now.
~To make reviewing easier only `bitcoind` code is modified (neither `bitcoin-cli` nor `bitcoin-qt`).~
Refs:
- https://github.com/bitcoin/bitcoin/issues/15745#issuecomment-479852569 by **laanwj**
- #16220
Top commit has no ACKs.
Tree-SHA512: 4a4cda10e0b67c8f374da0c9567003d2b566d948e7f8550fe246868b5794c15010e88ea206009480b9cd2f737f310a15e984f920730448f99a895893bed351df
|
|
fa21737ba7159578299f2b10e181a084ab8887c4 ci: Add environment files for all settings (MarcoFalke)
Pull request description:
This moves all environment settings from travis to files in the ci folder. Now, it is possible to easily run each travis configuration with a single command.
Top commit has no ACKs.
Tree-SHA512: 989c6b62eb3839eb1fa5461e986496e9660167e2438a789c7588a6fee4f9c37b332782c010fe5c7de8f606bcf98dffb2481d2777cbce88f87cc9f0c42fb2d7fc
|
|
20ea9ef6ce9228a5258b99eeeeb40e6dfae2299f [doc] mention whitelist is inbound, and applies to blocksonly (Sjors Provoost)
Pull request description:
* `-whitelist` only impacts inbound nodes (see #9923). This is obvious in the context of allowing those nodes to connect to you, but there are additional whitelist features where this is less obvious, such as mempool relay behavior.
* `whitelistrelay` (on by default) explains that `-blocksonly` makes an exception for transactions from whitelisted nodes, but it wasn't documented (nor obvious imo) the other way around. See also https://github.com/bitcoin/bitcoin/pull/15984#issuecomment-490645552
Top commit has no ACKs.
Tree-SHA512: 03e363a5da5d81ad147d1c7e38bf11114df8bb89bdd66fb551520b25f810efa886ec6e649d3b435c4935e0ae4f39bb718bc7bb5778b9de6aa0b71e970a431af8
|
|
6c27fa0f09e5ec3854f30673d26c6e5e2d27063e scripts: remove github-merge.py (fanquake)
Pull request description:
This script has been moved to the bitcoin-core/bitcoin-maintainer-tools repository,
after discussion in a core dev meeting. The rationale being that it is also useful to
other projects, and thus should be moved to a more general repository.
Meeting log: http://www.erisian.com.au/bitcoin-core-dev/log-2019-08-15.html
See also: https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/28
~~I'll open a follow up PR to port the documentation to the maintainer tools repo.~~
Ported the docs in https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/29
ACKs for top commit:
laanwj:
ACK 6c27fa0f09e5ec3854f30673d26c6e5e2d27063e
Tree-SHA512: 972070c6893043aebe3b62118d2e42d857553202b1eeb9abca55326f580f25e9859de188a1d0c50b9c7d20bdeeddffc00bcf9ad5a76b60add5c8541956d45489
|
|
This script has been moved to the bitcoin-core/bitcoin-maintainer-tools repository,
after discussion in a core dev meeting. The rationale being that it is also useful to
other projects, and thus should be moved to a more general repository.
Meeting log: http://www.erisian.com.au/bitcoin-core-dev/log-2019-08-15.html
See also: https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/28
|
|
b168dd30cf71ac176e271bc610b0b1a79ceaf075 Bugfix: QA: Run tests with UPnP disabled (Luke Dashjr)
Pull request description:
This replaces #16560 by adding `upnp=0` to `bitcoin.conf` rather than passing it to nodes.
> Needed for builds configured with --enable-upnp-default
You can test this change using:
```bash
./configure --enable-upnp-default && make -j6 && test/functional/test_runner.py feature_config_args.py
```
on master the test will fail without this change.
ACKs for top commit:
practicalswift:
ACK b168dd30cf71ac176e271bc610b0b1a79ceaf075 -- diff looks correct
Tree-SHA512: e639dd480dda2cffa19a679018c4bd7e4bd4d0f5e3001d6b407b833e3c166bde98b201063e267b8e45f8a20b0d53ec8bc028bec806b2357f9a7ba314cc4e2d07
|
|
3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb Reformat p2p_permissions.py (nicolas.dorier)
ce7eac3cb0e7d301db75de24e9a7b0af93c61311 [Fix] The default whitelistrelay should be true (nicolas.dorier)
Pull request description:
I thought `whitelistrelay` default was `false` when it is `true`.
The root of the issue come from the fact that all references to `DEFAULT_` are not in the scope of this file, so hard coding of default values are used everywhere in `net.cpp`. I think that in a separate PR we should fix that more fundamentally everywhere.
ACKs for top commit:
promag:
ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb.
Sjors:
re-ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb
Tree-SHA512: f4a75f986fa2adf1a5f1c91605e0d261f7ac5ac8535fb05437d83b8392dbcf5cc1a47d755adcf8ad8dc67a88de28060187200fd3ce06545261a5c7ec0fea831a
|
|
fa0119af226255ce1e0e573a575ab37539ff8693 doc: Refer in rpcbind doc to the manpage (MarcoFalke)
Pull request description:
The help was outdated, so refer to the updated manpage instead.
* closes #14740
* closes #9272
ACKs for top commit:
emilengler:
Concept ACK fa0119a
laanwj:
ACK fa0119af226255ce1e0e573a575ab37539ff8693
fanquake:
ACK fa0119af226255ce1e0e573a575ab37539ff8693
Tree-SHA512: 9836e4f31ece7acae334b47e3104b4595d0d1c1977af386a9463faf49aea9de3ab1c1dc3de248d0ee09e3b13693eaf6902dc82d277062e1bb1980b1117236fbb
|
|
|
|
|
|
|
|
|
|
Needed for builds configured with --enable-upnp-default
|
|
dc1bc1c503233c42850819b544578b756299e071 doc: Add ZMQ dependencies to Fedora build (Hennadii Stepanov)
Pull request description:
Without `zeromq-devel` package on Fedora 30:
```
$ ./configure | grep zmq
configure: WARNING: libzmq version 4.x or greater not found, disabling
with zmq = no
```
With installed `zeromq-devel`:
```
$ ./configure | grep zmq
with zmq = yes
```
Also this PR improves style in the `miniupnpc` related paragraphs.
ACKs for top commit:
emilengler:
Concept ACK dc1bc1c
practicalswift:
ACK dc1bc1c503233c42850819b544578b756299e071 -- diff looks correct
fanquake:
ACK dc1bc1c503233c42850819b544578b756299e071 - tested in a Fedora 30 Docker container. Other changes match the Ubuntu & Debian docs above.
Tree-SHA512: 89b083a6256e8d0bcb9f0b193af84bc680843fdbb4a3e8871086477875250ad0ae214ce08797fc70c0cb04a2969632840c805a2a10a433763ee31cd1aa793bac
|
|
787c9ec0c383cefb83a95433311a27f9c220dca0 Additional tests for other failure cases (Andrew Chow)
6e1ae58298bc0069b95dfcd531c493a3825bded7 Check error messages in descriptor tests (Andrew Chow)
625534d7b1417da926f1ced600855ea818d6e01e Give more errors for specific failure conditions (Andrew Chow)
c325f619dd071b5489989f645e42cace8eb23fb4 Return an error from descriptor Parse that gives more information about what failed (Andrew Chow)
Pull request description:
Because it's nigh impossible to figure out what is wrong with a descriptor otherwise.
ACKs for top commit:
Sjors:
ACK 787c9ec
meshcollider:
Code review ACK 787c9ec0c383cefb83a95433311a27f9c220dca0
Tree-SHA512: 07c5deff127fd65507b96df2e4608b4f918e0cbd20b3b45b9ab5a05c2985a6efa9832079cf1e7f504305bfbb861019e93b96cc1574dd63b2ff0756b5928ece20
|
|
|
|
a6c1fc3cd96e68d00b747ee91ea00dd66cca60e0 build: echo prop tests status during build (Jon Atack)
Pull request description:
Enable users to see if the prop tests are enabled during the build. This can be particularly helpful as property-based tests are silently auto-enabled by default if librapidcheck is found.
Sample build output:
```
Options used to compile and link:
with wallet = yes
with gui / qt = yes
with bip70 = yes
with qr = yes
with zmq = yes
with test = yes
with prop = yes <--- new
with fuzz = no
with bench = no
```
ACKs for top commit:
fanquake:
ACK a6c1fc3cd96e68d00b747ee91ea00dd66cca60e0 - Thanks. Tested `./configure` with and without rapidcheck available.
Tree-SHA512: 460f3b83ee2a03e6dcc53bc326ac210d2484895e11766be117920fbc6fa78cdbcd4267cbceda9447c2f5fa5a7f218865ccc929ac6319308e397c0a5603571ecd
|
|
|
|
|
|
Some failure conditions implicitly fail by failing some other check.
But the error messages are more helpful if they say explicitly what
actually caused the failure, so add those as failure conditions and
errors.
|
|
failed
|
|
26d3fad1093dfc697048313be7a96c9adf723654 Add unmodified-but-with-checksum to getdescriptorinfo (Pieter Wuille)
104b3a5069c937383e6f88f2f3fb804ef61b208f Factor out checksum checking from descriptor parsing (Pieter Wuille)
Pull request description:
ACKs for top commit:
achow101:
Code Review ACK 26d3fad1093dfc697048313be7a96c9adf723654
meshcollider:
re-Code Review ACK 26d3fad1093dfc697048313be7a96c9adf723654
Sjors:
ACK 26d3fad1093dfc697048313be7a96c9adf723654
Tree-SHA512: b7a7f89b64a184927d6f9a0c183a087609983f0c5d5593f78e12db4714e930a4af655db9da4b0c407ea2e24d3b926cef6e1f2a15de502d0d1290a6e046826b99
|
|
|
|
|
|
|
|
d117f4541d4717e83c9396273e92960723622030 Add test for setban (nicolas.dorier)
dc7529abf0197dccb876dc4a93cbdd2ad9f03e5c [Fix] Allow connection of a noban banned peer (nicolas.dorier)
Pull request description:
Reported by @MarcoFalke on https://github.com/bitcoin/bitcoin/pull/16248#discussion_r314026195
The bug would mean that if the peer connecting to you is banned, but whitelisted without specific permissions, it would not be able to connect to the node.
The solution is just to move the same line below.
ACKs for top commit:
Sjors:
Agree inline is more clear. utACK d117f45
MarcoFalke:
ACK d117f4541d4717e83c9396273e92960723622030
Tree-SHA512: 0fed39acb1e8db67bb0bf4c4de3ad034ae776f38d55bd661f1ae0e1a4c6becaf1824ab46ed8279f2f31df3f4b29ff56461d8b167d3e9cece62cfe58b5a912811
|
|
|
|
faba46da07cd8383d0bc841d37ea9cacba60e354 ci: Set --ansi in test_runner (MarcoFalke)
Pull request description:
Fixup to:
* tests: Use colors and dots in test_runner.py output only if standard output is a terminal #16561
ACKs for top commit:
practicalswift:
ACK faba46da07cd8383d0bc841d37ea9cacba60e354 -- diff looks correct
fanquake:
ACK faba46da07cd8383d0bc841d37ea9cacba60e354 - assuming Travis is all green.
Tree-SHA512: 50bf6ec8e7a2987d77821816289fd87458e63237d419a149e5e04027387d2c4b3c23db58977585fda30fbc4b13686a7def0d9e00096b9a8469dea2916fd30565
|
|
|
|
|
|
fa27c55b0593c769b6ad87de0b59df3816d73548 util: Move ResolveErrMsg to util/error (MarcoFalke)
Pull request description:
Pull request https://github.com/bitcoin/bitcoin/pull/16248#discussion_r314035862 duplicated the body of this util function. The whole point of the util function is to be shared, so do that here as a fixup to #16248
ACKs for top commit:
Sjors:
utACK fa27c55
ryanofsky:
utACK fa27c55b0593c769b6ad87de0b59df3816d73548
Tree-SHA512: e2b25ae05082fe9d0ee94bdc7d51f801bd9f78e8fc2b141e9a313e008dbb8a77653fe876e111c802c676859c6b76c37a673d1f8cfbe7ad25607a5ffcffde19fd
|
|
Enable users to see if the prop tests are enabled during the build. This can be particularly helpful as property-based tests are silently auto-enabled by default if librapidcheck is found.
Minor fixes to the docs and help grammar for this option.
|
|
390874c722c5af0b37cb94151ffb49986eab5f7d qt: Remove menu icons (Wladimir J. van der Laan)
Pull request description:
Remove the icons from the application menu. Why remove?
- They are inconsistently applied, some actions had icons, some newer ones don't. Good luck coming up with a sensible icon for everything
- Menu icons don't seem to have a place in modern UI: for example, GNOME, MacOS have stopped showing these a long time ago (see https://github.com/bitcoin/bitcoin/pull/16584#issuecomment-521195090)
- Less bikeshedding opportunity about "what should the icon for this be"
Removed icons:
```
/icons/quit res/icons/quit.png
/icons/about res/icons/about.png
/icons/about_qt res/icons/about_qt.png
/icons/options res/icons/configure.png
/icons/key res/icons/key.png
/icons/verify res/icons/verify.png (also .svg)
/icons/debugwindow res/icons/debugwindow.png
/icons/open res/icons/open.png
/icons/info res/icons/info.png
/icons/filesave res/icons/filesave.png
```
I checked that these icons are used nowhere else.
Removed from the menu not removed from the repository, because still referenced by other parts of the code:
```
/icons/lock_closed
/icons/edit
/icons/address-book
/icons/send
```
ACKs for top commit:
practicalswift:
ACK 390874c722c5af0b37cb94151ffb49986eab5f7d -- diff looks correct
l2a5b1:
ACK 390874c722c5af0b37cb94151ffb49986eab5f7d - Bitcoin Core has a very simple application menu. As long as the menu items describe their actions clearly and unambiguously then the icons alongside the label are redundant and offer very little value, if anything at all.
kallewoof:
ACK 390874c722c5af0b37cb94151ffb49986eab5f7d
jonasschnelli:
utACK 390874c722c5af0b37cb94151ffb49986eab5f7d
Tree-SHA512: dd1c52bed3bc6fb9359d5ea1b229a023dafaf813ae640775cbb433b9886bbc11a7d6a4306bac350b26d45fca9b495e4468630f2a32e185570e05f16a3ce45b47
|
|
|