65273fa0e74f0c11dfbf0645dd962bdc779ea558 Clear m_addr_known before our periodic self-advertisement (Suhas Daftuar)
Pull request description:
We use a rolling bloom filter to track which addresses we've previously sent a peer, but after #7125 we no longer clear it every day before our own announcement. This looks to me like an oversight which has the effect of reducing the frequency with which we actually self-announce our own address, so this reintroduces resetting that filter.
ACKs for top commit:
naumenkogs:
ACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
laanwj:
Code review ACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
sipa:
utACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
Tree-SHA512: 602c155fb6d2249b054fcb6f1c0dd17143605ceb87132286bbd90babf26d258ff6c41f9925482c17e2be41805d33f9b83926cb447f394969ffecd4bccfa0a64f
fa275e1539941b49fe206ff0bf110e3362bec6bb test: Fix intermittent feature_taproot issue (MarcoFalke)
Pull request description:
The nodes might disconnect (e.g. due to "Timeout downloading block" https://cirrus-ci.com/task/5313800947630080?command=ci#L1763) and the test fails to continue.
Fix that by reconnecting the nodes.
ACKs for top commit:
laanwj:
code review ACK fa275e1539941b49fe206ff0bf110e3362bec6bb
Tree-SHA512: 2871183c8058d8292c9c4ef56ea3d19d5616ca712ebdaabb6609f8c9cd2e16c9ac2ce26aa1e94b346872b7b6fec56b59af151af83de3a5aa08bed01bfcc7187a
fa8abdc9953e381715493b259908e246914793b0 rpc: Use FeeModes doc helper in estimatesmartfee (MarcoFalke)
Pull request description:
Not sure why this doesn't use the doc helper, probably an oversight?
ACKs for top commit:
laanwj:
Code review ACK fa8abdc9953e381715493b259908e246914793b0
Tree-SHA512: 1f2dc8356e3476ddcf9cafafa7f9865ad95bed1e3067c0edab8e3c483e374bdbdbecc066167554b4a1b479e28f6a52c4ae6a75a70c67ee4e1ff4f3ba36b04001
4e28753f60613ecd35cdef87bef5f99c302c3fbd feestimator: encapsulate estimation file logic (Antoine Poinsot)
e8ea6ad9c16997bdc7e22a20eca16e234290b7ff init: don't create a CBlockPolicyEstimator if we don't relay transactions (Antoine Poinsot)
86ff2cf202bfb9d9b50800b8ffe3fead3f77f5fa Remove the remaining fee estimation globals (Antoine Poinsot)
03bfeee957ab7e3b6aece82b9561774648094f54 interface: remove unused estimateSmartFee method from node (Antoine Poinsot)
Pull request description:
If the `blocksonly` mode is turned on after running with transaction
relay enabled for a while, the fee estimation will serve outdated data
to both the internal wallet and to external applications that might be
feerate-sensitive and make use of `estimatesmartfee` (for example a
Lightning Network node).
This has already caused issues (for example https://github.com/bitcoin/bitcoin/issues/16840 (C-lightning), or https://github.com/lightningnetwork/lnd/issues/2562 (LND)) and it seems prudent to fail rather than to give inaccurate values.
This fixes#16840, and closes#16890 which tried to fix the symptoms (RPC) but not the cause as mentioned by sdaftuar :
> If this is a substantial problem, then I would think we should take action to protect our own wallet users as well (rather than hide the results of what our fee estimation would do!).
ACKs for top commit:
MarcoFalke:
re-ACK 4e28753f60 👋
jnewbery:
utACK 4e28753f60613ecd35cdef87bef5f99c302c3fbd
Tree-SHA512: c869cf03b86d8194002970bbc84662dae76874967949b9be0d9a4511a1eabcb1627c38aca3154da9dcece1a4c49ec02bd4f9fcca2ec310986e07904559e63ba8
fa0f4157098ea68169ced44730986d0ed2c3a5aa net: Assume that SetCommonVersion is called at most once per peer (MarcoFalke)
Pull request description:
This restores the check removed in https://github.com/bitcoin/bitcoin/pull/17785#discussion_r503224381
Instead of using `error`, which was used previously, it uses a newly introduced `Assume()`. `error` had several issues:
* It logs unconditionally to the debug log
* It doesn't abort the program when the error is hit in tests
ACKs for top commit:
practicalswift:
cr ACK fa0f4157098ea68169ced44730986d0ed2c3a5aa: patch looks correct
jnewbery:
utACK fa0f4157098ea68169ced44730986d0ed2c3a5aa
Tree-SHA512: cd7424a9485775e8c7093b725f8f52a90d47485185e79bac80f7810e450d0b3fda608d8805e9239094929f7bad2dca3fe772fb78ae606c2399d15405521e136b
6690adba08006739da0060eb4937126bdfa1181a Warn when binaries are built from a dirty branch. (Tyler Chambers)
Pull request description:
- Adjusted `--version` flag behavior in bitcoind and bitcoin-wallet to have the same behavior.
- Added `--version` flag to bitcoin-tx to match.
- Added functionality in gen-manpages.sh to error when attempting to generate man pages for binaries built from a dirty branch.
mitigates problem with issue #20412
ACKs for top commit:
laanwj:
Tested ACK 6690adba08006739da0060eb4937126bdfa1181a
Tree-SHA512: b5ca509f1a57f66808c2bebc4b710ca00c6fec7b5ebd7eef58018e28e716f5f2358e36551b8a4df571bf3204baed565a297aeefb93990e7a99add502b97ee1b8
c23f6f84efa2fe7e7168a5d41341f3a7c5598f70 Add depends qt fix for ARM macs (Jonas Schnelli)
Pull request description:
With this, depends builds fine on macOS 11 on an Apple Silicon Mac (ARM64).
ACKs for top commit:
laanwj:
Code review ACK c23f6f84efa2fe7e7168a5d41341f3a7c5598f70
Tree-SHA512: a8354cec99969cff9e7dab150c335050ddb4b3c93a9f12a4db5e8046f02b11ce692ac17c2b96cbbe7f380c1aa110b15b8d6d48d51bc9c560282c702e99fd8a8d
1816327e533d359c237c53eb6440b2f3a7cbf4fa p2p: Put disconnecting logs into BCLog::NET category (Hennadii Stepanov)
Pull request description:
It's too noisy:
```
$ cat debug.log | wc -l
28529
$ cat debug.log | grep "Disconnecting and discouraging peer" | wc -l
10177
```
ACKs for top commit:
MarcoFalke:
noban, addnode and local peers are still unconditionally logged (as they should), but this one can go into a category, so cr-ACK 1816327e533d359c237c53eb6440b2f3a7cbf4fa
practicalswift:
ACK 1816327e533d359c237c53eb6440b2f3a7cbf4fa for the reasons MarcoFalke gave above.
ajtowns:
ACK 1816327e533d359c237c53eb6440b2f3a7cbf4fa
Tree-SHA512: c312c1009090840659b2cb1364d8ad9b6ab8e742fc462aef169996d93c76c248507639a00257ed9d73a6916c01176b1793491b2305e92fdded5f9de0935b6ba6
ed1bbcefeaafef460b6e5609de85e13c12e0d2a4 contrib: add MACHO tests to symbol-check tests (fanquake)
5bab08df175db3a4283596515e498fc5a4d0dab9 contrib: Add test for ELF symbol-check (Wladimir J. van der Laan)
Pull request description:
Check both failure cases:
- Use a glibc symbol from a version that is too new
- Use a symbol from a library that is not in the allowlist
And also check a conforming binary.
Adding a similar check for Windows PE can be done in a separate PR.
ACKs for top commit:
fanquake:
ACK ed1bbcefeaafef460b6e5609de85e13c12e0d2a4
Tree-SHA512: fd437612e003922465fe1396efa1fa3a64bd1c7b0a514d2a0a7a0caaaa9fb5cb43e0ed7caec15eb0a3508692c9eb3212d7ba3c7e8180b942dd3e50616ad6e557
cb0b7125c14bf97394bd8b43bf2abfb943bb1cf9 doc: libbitcoinconsensus: add missing error code description, fix NBitcoin link (Sebastian Falbesoner)
Pull request description:
This PR improves the libbitcoinconsensus description in `shared-libraries.md` in two ways:
* adds the missing error code description for `bitcoinconsensus_ERR_INVALID_FLAGS` (introduced by commit 5ca8ef299a08aae91d5061750533694b58d810b2, PR #8976)
* updates and fixes the link to the NBitcoin implementation (introduced by commit 3361edd01046aab137928d1e6e2468dbfc4b946e, PR #6430)
* the owner of the `NBitcoin` github repository changed from `NicolasDorier` to `MetacoSA` (redirection still worked though)
* instead of dynamically referring to a file in master with a fixed line number (which is obviously always quickly outdated), use a permalink with a file numbers area
ACKs for top commit:
MarcoFalke:
cr ACK cb0b7125c14bf97394bd8b43bf2abfb943bb1cf9
harding:
Code (documentation) review ACK cb0b7125c14bf97394bd8b43bf2abfb943bb1cf9. Text is clear and seems accurate, and the link checks out.
Tree-SHA512: 9840458db6fb40e71c9852104aefcec5abbaf5054c6123701181dd477cea8c81d3647f376b67692159adf577c9b6305b05b784728bf9f14a753fab5898075a4e
fa6af312277bb1b7e57d9b764d411c5b0873829f test: Document why syncwithvalidationinterfacequeue is needed in tests (MarcoFalke)
fa135a13b8ddaa117bd090ec43a3eab3a95755c1 Revert "test: Add missing sync_all to wallet_balance test" (MarcoFalke)
Pull request description:
syncwithvalidationinterfacequeue is a hidden test-only RPC, so it should not be used when it is not needed. Thus, either remove it or explain why it is needed.
ACKs for top commit:
fjahr:
Code review ACK fa6af312277bb1b7e57d9b764d411c5b0873829f
Tree-SHA512: de30db4ab521184091ee5beeab02989138cf7cf05088f766a2fb106151b239310b63d5380cb79e2a072f72c5ae9513aecae8eb9c1c7be713771585c3cb04d63a
12dcdaaa54b9a4c55384e6b04379da8125fa6e7d Don't make "in" parameters look like "out"/"in-out" parameters: pass by ref to const instead of ref to non-const (practicalswift)
Pull request description:
Don't make "in" parameters look like "out"/"in-out" parameters: pass by ref to const instead of ref to non-const.
ACKs for top commit:
MarcoFalke:
cr ACK 12dcdaaa54b9a4c55384e6b04379da8125fa6e7d
fjahr:
Code review ACK 12dcdaaa54b9a4c55384e6b04379da8125fa6e7d
hebasto:
ACK 12dcdaaa54b9a4c55384e6b04379da8125fa6e7d, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: bf9e476dc27bfbd856b42cc4720fb2d4b74e64039776ce067691aaf536360b36902ef72988c7914505df5bf0ed2f1557e0eba52b87935d3a17db865ea17f6093
773c42b265fb2212b5cb8785b7226a206d063543 tests: Test that a fully signed tx given to signrawtx is unchanged (Andrew Chow)
Pull request description:
Tests that a fully signed transaction given to `signrawtransactionwithwallet` is both unchanged and marked as complete. This tests for a regression in 0.20 where the transaction would not be marked as complete.
ACKs for top commit:
MarcoFalke:
ACK 773c42b265 🕶
Tree-SHA512: b32afd5f2667c817ee59c37af78cb37352e7c22d0271833d446fd61f11b354974694128a3b408f214db459caef3fd67ecdbf1664280ac2de602215f05aa4a6de
fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2 refactor: Use C++17 std::array where possible (MarcoFalke)
Pull request description:
Using the C++11 std::array with explicit template parameters is problematic because overshooting the size will fill the memory with default constructed types.
For example,
```cpp
#include <array>
#include <iostream>
int main()
{
std::array<int, 3> a{1, 2};
for (const auto& i : a) {
std::cout << i << std::endl; // prints "1 2 0"
}
}
```
ACKs for top commit:
jonasschnelli:
Code Review ACK fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2
practicalswift:
cr ACK fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2
vasild:
ACK fac7ab1d
promag:
Code review ACK fac7ab1d5b58fb9cfd80d5cf74ac4d2e5cb8eff2.
Tree-SHA512: ef7e872340226e0d6160e6fd66c6ca78b2ef9c245fa0ab27fe4777aac9fba8d5aaa154da3d27b65dec39a6a63d07f1063c3a8ffb667a98ab137756a1a0af2656
e373959d6fe90cc4507024a6b31a706bfc5bd0c8 Android : Ensure pic build for bdb (Block Mechanic)
Pull request description:
This pr ensures android builds for the BDB dependency have the pic flag enabled. Android builds were failing to link with reloc errors.
ACKs for top commit:
laanwj:
Code review ACK e373959d6fe90cc4507024a6b31a706bfc5bd0c8
jonasschnelli:
utACK e373959d6fe90cc4507024a6b31a706bfc5bd0c8
Tree-SHA512: 68319ed7cc0bd295eaa87dd53ba051daeb1456bc3ab9b48ca0c4b831a9c8da1073480478efde73689f0e403e37409a8459229264656f05ba5fef6c257a74f977
faa05854f832405231c9198787a4eafe2cd4c5f0 util: Remove probably misleading TODO (MarcoFalke)
fac5efe730ab5b8694920547203deefc5252d294 util: Add Assume() identity function (MarcoFalke)
fa861569dcfff9e72686dc5f30b1faa645b4d54e util: Allow Assert(...) to be used in all contexts (practicalswift)
Pull request description:
This is needed for #20138. Please refer to the added documentation for motivation.
ACKs for top commit:
practicalswift:
cr ACK faa05854f832405231c9198787a4eafe2cd4c5f0
jnewbery:
utACK faa05854f832405231c9198787a4eafe2cd4c5f0
hebasto:
ACK faa05854f832405231c9198787a4eafe2cd4c5f0, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 72165fbd898b92ab9a79b070993fa1faa86c2e3545b6645e72c652bda295d5107bc298d0482bf3aaf0926fc0c3e6418a445c0e073b08568c44231f547f76a688
86e6add5ca7d425faa314103df6617044a910ab3 doc: rename CODEOWNERS to REVIEWERS (Adam Jonas)
Pull request description:
This PR renames the CODEOWNERS file to REVIEWERS and works with DrahtBot to leave a comment requesting a review.
Testing of the functionality was done on https://github.com/adamjonas/bitcoin-codeowners-sandbox/pulls.
~EDIT: [after further testing of a fake organization](https://github.com/jonasorg/bitcoin-codeowners-sandbox-org/pulls), it appears that in order to be automatically tagged, the reviewer requires write level permissions. This is undocumented and will obviously close the circle of who can be tagged and so this PR reverts the addition of the file in #18949.~
~This removes a line not being parsed in the CODEOWNERS file introduced in #18949. While the pattern [checked out](https://github.com/bitcoin/bitcoin/pull/18949#issuecomment-648859076) using `git ls-files`, it is preventing the CODEOWNERS file from automatically tagging reviewers. For future modifications to this file, note that [any line failing to parse causes the entire file to stop identifying codeowners](http://www.benjaminoakes.com/git/2018/08/10/Testing-changes-to-GitHub-CODEOWNERS/#:~:text=warning).~
~I experimented in a [sandbox repo](https://github.com/adamjonas/bitcoin-codeowners-sandbox/pulls) to discover the offending line and this change appears to fix the problem.~
ACKs for top commit:
laanwj:
Doesn't look like github is going to fix this any time soon and besides REVIEWERS is a better name so ACK 86e6add5ca7d425faa314103df6617044a910ab3
Tree-SHA512: b3425eca4be62f829f0edcfa08232df310680835c2f60e9fa36956a6b59a2b0cd0ab580c572c87affccd843fbd849f4f99e4b3d9d7b6070ab117953f04e17f5a
d3ef947524a07f8d7fbad5b95781ab6cacb1cb49 build: Check that Homebrew's berkeley-db4 package is actually installed (Hennadii Stepanov)
Pull request description:
On master (a0489f3472f3799dc1ece32a59556fd239c4c14b) the `configure` script is not able to determine that Homebrew's `berkeley-db4` package is uninstalled. This causes a compile error on macOS.
With this PR, and with the [uninstalled](https://stackoverflow.com/questions/20802320/detect-if-homebrew-package-is-installed) `berkeley-db4` package:
```
% ./configure -q
configure: error: libdb_cxx headers missing, Bitcoin Core requires this library for BDB wallet support (--without-bdb to disable BDB wallet support)
```
Related #20478.
ACKs for top commit:
promag:
Tested ACK d3ef947524a07f8d7fbad5b95781ab6cacb1cb49.
willcl-ark:
tACK d3ef947524a07f8d7fbad5b95781ab6cacb1cb49
jonasschnelli:
utACK d3ef947524a07f8d7fbad5b95781ab6cacb1cb49
Tree-SHA512: 8dc532e08249ec63bd357594aa458d314b6e8537fc63f5b1d509c84d0d71d5b1f70172caa1a7efe2fc8af31c829e7982a0695cf3fbe5cbc477019550269915e1
2f6fe4e4e9e9e35e713c0a20cf891b023592110a ci: Build with --enable-werror by default, and document exceptions (Hennadii Stepanov)
Pull request description:
This PR prevents introducing of new compiler warnings in the master branch, e.g., #19986, #20162.
ACKs for top commit:
practicalswift:
cr ACK 2f6fe4e4e9e9e35e713c0a20cf891b023592110a: patch looks correct
MarcoFalke:
re-ACK 2f6fe4e4e9 🏏
vasild:
ACK 2f6fe4e
Tree-SHA512: 23b5feb5bc472658c992d882ef61af23496f25adaa19f9c79bfaef5d2db273d44981aa93b1631a7d37cb58755283c1dacf3f2d68e501522d3fa8c965ab646d19
Tests that a fully signed transaction given to
signrawtransactionwithwallet is both unchanged and marked as complete.
This tests for a regression in 0.20 where the transaction would not be
marked as complete.
This behavior was apparently inadvertently broken in 5400ef6; without this
change our daily self-announcements frequently go unsent, because our
address is still in the peer's rolling bloom filter (for potentially many
days, depending on addr traffic).
cadb77a6ab8a3e6f56062cfaec4dd8168c71b39d net: Add compat.h header for htonl function (Hennadii Stepanov)
f796f0057bc7dad8e7065831b07f432fc0fb9f08 net: Drop unneeded headers when compat.h included (Hennadii Stepanov)
467c34644861a5267601255650e27c7aadab31dc net: Drop unneeded Windows headers in compat.h (Hennadii Stepanov)
Pull request description:
It is the `compat.h` header's job to provide platform-agnostic interfaces for internet operations.
No need in `#include <arpa/inet.h>` scattered around.
ACKs for top commit:
practicalswift:
re-ACK cadb77a6ab8a3e6f56062cfaec4dd8168c71b39d: patch looks even better
laanwj:
Code review ACK cadb77a6ab8a3e6f56062cfaec4dd8168c71b39d
Tree-SHA512: 625ff90b2806310ab856a6ca1ddb6d9a85aa70f342b323e8525a711dd12219a1ecec8373ec1dca5a0653ffb11f9b421753887b25615d991ba3132c1cca6a3c6e
2c69381f3de5091e103cb8bef299aba321503e7c Removed redundant git pull from appveyor config. (Aaron Clauson)
8b99e609e7da5dd3601e9214d8f869e96108fffe Adjusted msvc compiler and linker settings to remove optimisations that are causing sporadic ABI issues on Visual Studio updates. (Aaron Clauson)
Pull request description:
The motivation for this PR is twofold:
1. Update the Qt binaries used by the appveyor CI job after a recent update to Visual Studio 2019 used in the Appveyor build image resulted in ABI incompatibilities,
2. Remove optimisations and debug information from the Bitcoin Core `Release` msvc build to reduce the chance of future ABI incompatibility issues for future Visual Studio updates.
The changes made in this PR are:
- Changed appveyor config file hash to use a new version of Qt pre-compiled binaries built for Visual Studio 2019 v16.8.1.
- Adjusted msvc compiler and linker settings to remove optimisations and debug information generation to help avoid future ABI issues on Visual Studio updates.
- Tidied up debug and release configuration blocks in common project file to avoid duplication.
- Updated appveyor config to use latest Visual Studio 2019 image.
- Bumped vcpkg version to tag `2020.11-1` for binary caching feature*.
See #20392 for related discussion.
*Binary caching is a new [vcpkg feature](https://github.com/microsoft/vcpkg/blob/master/docs/specifications/binarycaching.md) that allows dependency caching. This PR is not using the feature but by updating the vcpkg version it means it can be optionally used by other contributors in their own Appveyor configs. By caching the vcpkg dependencies using this feature my build times are reduced by approx 10 minutes.
ACKs for top commit:
MarcoFalke:
Concept ACK 2c69381f3de5091e103cb8bef299aba321503e7c
laanwj:
Concept ACK 2c69381f3de5091e103cb8bef299aba321503e7c
Tree-SHA512: 372f5b8b3b5f7e56b78045f9fc06a22fd9f5366d06e99e2f1eaad6d741680da74d0e6371e7bc580cb41f4d6696b2101b950167309d33e98242578b458ace813a
c82d15b6d189983ebf82f35ade33d830870f25c6 depends: Do not force Precompiled Headers (PCH) for building Qt on Linux (Hennadii Stepanov)
Pull request description:
On CentOS 8 (Cirrus CI job) the forced `-pch` option breaks Qt build.
Removing `-pch` option does not affect build time for other systems:
- master (e2ff5e7b35d71195278d2a2ed9485f141de33d7a):
```
$ time make -j 9 -C depends/ qt
...
Caching qt...
make: Leaving directory '/home/hebasto/guix/GitHub/bitcoin/depends'
real 4m22,359s
user 18m3,719s
sys 1m24,769s
```
- this PR:
```
$ time make -j 9 -C depends/ qt
...
Caching qt...
make: Leaving directory '/home/hebasto/guix/GitHub/bitcoin/depends'
real 4m14,862s
user 18m3,355s
sys 1m24,506s
```
Qt docs: https://doc.qt.io/qt-5/qmake-precompiledheaders.htmlFixes#20423
ACKs for top commit:
MarcoFalke:
review ACK c82d15b6d189983ebf82f35ade33d830870f25c6
Tree-SHA512: 0f2a3712e90de881d00f8e56c363edde33dd4f5c117df5744ab4e51d0a8146331de7236bc8329d68ddd91535cd853e68ee80ef4cceb6a909786abfd8881b01e8
This moves the fee_estimates file management to the CBlockPolicyEstimator
Flush() method.
Co-authored-by: John Newbery <john@johnnewbery.com>
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
This moves the CBlockPolicyEstimator to the NodeContext, which get rids
of two globals and allows us to conditionally create the
CBlockPolicyEstimator (and to remove a circular dep).
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
Check both failure cases:
- Use a glibc symbol from a version that is too new
- Use a symbol from a library that is not in the allowlist
And also check a conforming binary.
Adding a similar check for Windows PE can be done in a separate PR.
fad7be584ffaf8099cc099d9378ba831c9483260 test: Fix intermittent p2p_finerprint issue (MarcoFalke)
Pull request description:
A single sync_with_ping can't be used to drop a block announcement, as the block might be sent *after* the ping has been responded to.
Fix that by waiting for the block.
ACKs for top commit:
theStack:
ACK fad7be584ffaf8099cc099d9378ba831c9483260
Tree-SHA512: d43ba9d07273486858f65a26326cc6637ef743bf7b400e5048ba7eac266fb1893283e6503dd49f179caa1abab2977315fb70ba9fba34be9a817a74259d8e4034
fa5c4f12f5d017d8dbe7700ca9688189eacbf32b ci: Adjust cirrus ci task names (MarcoFalke)
Pull request description:
The task names are too long for GitHub to display them properly without truncation in the "checks-view". Fix that by using a new naming scheme:
* Native builds don't mention "x86_64 Linux", as it is redundant, they do mention the OS (bionic or focal) in the name suffix
* Cross builds mention the target in the prefix and the OS (always bionic) in the suffix
* the macos native build simply says "macos native"
ACKs for top commit:
practicalswift:
ACK fa5c4f12f5d017d8dbe7700ca9688189eacbf32b: patch looks correct!
hebasto:
ACK fa5c4f12f5d017d8dbe7700ca9688189eacbf32b, I have reviewed the code and it looks OK, I agree it can be merged.
RiccardoMasutti:
ACK fa5c4f1
Tree-SHA512: 856deb0577c97c70069ef1d369991addc49522135c0ad9e382218fd79ba3d55a95d6c601288dcef0510764b92fbd30a9d7de32b08dc5be55482deab14049b892
1e62350ca20898189904a88dfef9ea11ddcd8626 refactor: Improve use of explicit keyword (Fabian Jahr)
c502a6dbfb854ca827a5a3925394f9e09d29b898 lint: Use c++17 std in cppcheck linter (Fabian Jahr)
Pull request description:
I found the `extended-lint-cppcheck` linter still uses `std=c++11` when reviewing #20471. The only difference in the output after this change is one line is missing:
```
src/script/descriptor.cpp:159:5: warning: Struct 'PubkeyProvider' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
```
After some digging, I am still not sure why this one is ignored with c++17 when 40 other`noExplicitConstructor` warnings were still appearing.
In the second commit, I fix these warnings, adding `explicit` where appropriate and adding fixes to ignore otherwise.
ACKs for top commit:
practicalswift:
cr ACK 1e62350ca20898189904a88dfef9ea11ddcd8626: patch looks correct!
MarcoFalke:
review ACK 1e62350ca20898189904a88dfef9ea11ddcd8626
Tree-SHA512: dff7b324429a57160e217cf38d9ddbb6e70c6cb3d3e3e0bd4013d88e07afc2292c3df94d0acf7122e9d486322821682ecf15c8f2724a78667764c05d47f89a12
Tidied up debug and release configuration blocks in common project file to avoid duplication.
Updated appveyor config to use latest Visual Studio 2019 image.
Changed appveyor config file hash to use a new version of Qt pre-compiled binaries built for Visual Studio 2019 v16.8.1.
Bumped vcpkg version to tag '2020.11-1' for binary caching feature.
See #20392 for related discussion.
