29c9e2c2d2015ade47ed4497926363dea3f9c59b wallet: Do not iterate a directory if having an error while accessing it (Hennadii Stepanov)
Pull request description:
On Windows when `ListDatabases` tries to iterate any system folder, e.g., "System Volume Information", it falls into an infinite loop.
This PR fixes this bug. Now the `debug.log` contains:
```
2021-05-12T09:07:53Z ListDatabases: Access is denied D:/System Volume Information -- skipping.
```
An easy way to reproduce the bug and test this PR is to pass the `-walletdir=D:\` command-line option, and run the `listwalletdir` RPC, or File -> Open Wallet in the GUI menu.
Fixes#20081.
Fixes#21136.
Fixes#21904.
Also https://bitcoin.stackexchange.com/questions/99243/listwalletdir-access-is-denied-d-system-volume-information
ACKs for top commit:
prayank23:
ACK 29c9e2c2d2
promag:
Code review ACK 29c9e2c2d2015ade47ed4497926363dea3f9c59b.
meshcollider:
Code review ACK 29c9e2c2d2015ade47ed4497926363dea3f9c59b
Tree-SHA512: b851c88e6d09626f4cb81acc2fa59a563b2aee64582963285715bf785c64b872e8bf738aa6b27bdbaf4c3e5c8565c2dc2c802135f9aa1f48b4b913435bc5d793
9c891b64ffd14bc8216dbd5eb60816043af265b6 net: initialize nMessageSize to max uint32_t instead of -1 (eugene)
Pull request description:
nMessageSize is uint32_t and is set to -1. This will warn with `-fsanitize=implicit-integer-sign-change` when V1TransportDeserializer calls into the ctor. This pull initializes nMessageSize to `numeric_limits<uint32_t>::max()` instead and removes the ubsan suppression.
ACKs for top commit:
laanwj:
Code review ACK 9c891b64ffd14bc8216dbd5eb60816043af265b6
promag:
Code review ACK 9c891b64ffd14bc8216dbd5eb60816043af265b6.
Tree-SHA512: f05173d9553a01d207a5a7f8ff113d9e11354c50b494a67d44d3931c151581599a9da4e28f40edd113f4698ea9115e6092b2a5b7329c841426726772076c1493
fa95555a491dc01952703f476836e607ac34eab4 fuzz: Limit max insertions in timedata fuzz test (MarcoFalke)
Pull request description:
It is debatable whether a size of the median filter other than `200` (the only size used in production) should be fuzzed. For now add a minimal patch to cap the max insertions. Otherwise the complexity is N^2 log(N), where N is the size of the fuzz input.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34167
ACKs for top commit:
practicalswift:
cr ACK fa95555a491dc01952703f476836e607ac34eab4: patch looks correct
Tree-SHA512: be7737e9f4c906053e355641de84dde31fed37ed6be4c5e92e602ca7675dffdaf06b7063b9235ef541b05d3d5fd689c99479317473bb15cb5271b8baabffd0f2
facfc0f65dd0a7d54c0f6d56bff793e57b12ee12 fuzz: Remove strprintf test cases that are known to fail (MarcoFalke)
Pull request description:
They are still waiting to be fixed (see https://github.com/c42f/tinyformat/issues/70 ), so no need for us to carry them around in our source code. They can be added back once upstream is fixed.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34082
ACKs for top commit:
laanwj:
Code review ACK facfc0f65dd0a7d54c0f6d56bff793e57b12ee12
Tree-SHA512: d9d3d35555b6d58740a041ae45797ca85149f60990e2ed632c5dadf363e1d2362d2447681d7ceaa1fbffcd6e7bc8da5bc15d3923b68829a86c25b364a599afc8
fa872c9af397837bb17859b5f43adec71239682a depends: Fix id_string invocations (Carl Dong)
Pull request description:
Closes: #21242
```
Reproduced from depends/Makefile comment:
When invoking a shell, GNU Make special-cases exit code 127 (command not
found) by not capturing the output but instead passing it through. This
is not done for any other exit code.
Therefore, we require a "|| true" to avoid this behaviour when in an
environment where the build_* or host_* may not exist yet.
```
ACKs for top commit:
laanwj:
Concept and light code review ACK fa872c9af397837bb17859b5f43adec71239682a
Tree-SHA512: 9ce88381aec579d956572cf70c4f69dc5a3873f0d2af14a71cf24814192a89452b8280258bed8cca804e4bd2644db056d213ab733df46a10560a47079524d8ac
83a425d25af033086744c1c8c892015014ed46bd compressor: use a prevector in compressed script serialization (William Casarin)
Pull request description:
This function was doing millions of unnecessary heap allocations during IBD.
I'm start to catalog unnecessary heap allocations as a pet project of mine: as-zero-as-possible-alloc IBD. This is one small step.
before:
after:
~should I type alias this?~ *I type aliased it*
This is a part of the Zero Allocations Project #18849 (ZAP1). This code came up as a place where many allocations occur.
ACKs for top commit:
Empact:
ACK 83a425d25a
elichai:
tACK 83a425d25af033086744c1c8c892015014ed46bd
sipa:
utACK 83a425d25af033086744c1c8c892015014ed46bd
Tree-SHA512: f0ffa6ab0ea1632715b0b76362753f9f6935f05cdcc80d85566774401155a3c57ad45a687942a1806d3503858f0bb698da9243746c8e2edb8fdf13611235b0e0
804ac106313eb52d3a86f42c681b42acf90974c8 remove unnecessary newline from initWarning() argument (Larry Ruane)
Pull request description:
Run: `src/bitcoind -wallet=nosuchfile`
Without this patch, `debug.log` contains:
```
2021-03-23T21:19:16Z init message: Verifying wallet(s)...
2021-03-23T21:19:16Z Warning: Skipping -wallet path that doesn't exist. Failed to load database path '/home/larry/.bitcoin/wallets/nosuchfile'. Path does not exist.
2021-03-23T21:19:16Z init message: Loading banlist...
```
With this patch, the empty line isn't present. This PR fixes a similar problem with `src/bitcoind -conf=nosuchfile`
ACKs for top commit:
practicalswift:
cr ACK 804ac106313eb52d3a86f42c681b42acf90974c8: patch looks correct!
jarolrod:
tACK 804ac106313eb52d3a86f42c681b42acf90974c8, nice catch!
theStack:
Code-review ACK 804ac106313eb52d3a86f42c681b42acf90974c8
Tree-SHA512: dfcbaaa72ca24ac40233ac56840cfba8827853711d3df6e229ce940686f2ebf8bf0560bafcaa73a4d82d179a5050af0d3cabdc47b3b1dfd6aaadf718a6635f11
5294f0d5a94cc7beaf692131fba0cad8beec9f13 refactor: return std::nullopt instead of {} (fanquake)
Pull request description:
In #21415 [we decided](https://github.com/bitcoin/bitcoin/pull/21415#issuecomment-800236640) to return `std::optional` rather than `{}` for
uninitialized values. This PR replaces the two remaining usages of `{}`
with `std::nullopt`.
As a side-effect, this also quells the spurious GCC 10.2.x warning that
we've had reported quite a few times. i.e #21318, #21248, #20797.
```bash
txmempool.cpp: In member function ‘CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<uint256>&) const’:
txmempool.cpp:898:13: warning: ‘<anonymous>’ may be used uninitialized in this function [-Wmaybe-uninitialized]
898 | return {};
| ^
```
ACKs for top commit:
hebasto:
ACK 5294f0d5a94cc7beaf692131fba0cad8beec9f13, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 5b776be79ab26e5a3a5fc2b463b394ea5ce6797ed5558424873fa4ecee2898170eff76d6da9d69394d28f8f98974117fc63b922a3e19c52f5294c83073e79bb0
3d086f42ab58f0f5984101d5285d5a707e3dc8e7 test: add ParseUInt16() test coverage (Jon Atack)
Pull request description:
`ParseUInt16()` was just added in #21328.
ACKs for top commit:
practicalswift:
cr ACK 3d086f42ab58f0f5984101d5285d5a707e3dc8e7: patch looks correct & more coverage is better than less coverage
Tree-SHA512: bf7f96deb7c1531419565907f0ea8a8e32b368d4b823a3e80928b2c118edbf643ea06e357b4b5504a89f855caeed289daa9f823c740231ed6ad1b8ed00285ce8
8dd5946c0b7aa8f3976b14a5de4ce84b80a9c32a add functional test (Larry Ruane)
b5a80fa7e487c37b7ac0e3874a2fabade41b9ca8 util: Handle HTTP_SERVICE_UNAVAILABLE in bitcoin-cli (Hennadii Stepanov)
Pull request description:
If `bitcoind` is processing 16 RPC requests, attempting to submit another request using `bitcoin-cli` produces this less-than-helpful error message: `error: couldn't parse reply from server`. This PR changes the error to: `error: server response: Work queue depth exceeded`.
ACKs for top commit:
fjahr:
tACK 8dd5946c0b7aa8f3976b14a5de4ce84b80a9c32a
luke-jr:
utACK 8dd5946c0b7aa8f3976b14a5de4ce84b80a9c32a (no changes since previous utACK)
hebasto:
re-ACK 8dd5946c0b7aa8f3976b14a5de4ce84b80a9c32a, only suggested changes since my [previous](https://github.com/bitcoin/bitcoin/pull/18335#pullrequestreview-460621350) review.
darosior:
ACK 8dd5946c0b7aa8f3976b14a5de4ce84b80a9c32a
Tree-SHA512: 33e25f6ff05d9b56fae2bdb68b132557bb8e995f5438ac4fbbc53c304c5152a98aa43c43600c31d8a6a2830cbd48bf8ec7d89dce50190b29ec00a43830126913
9086e0dd3c924b6c9a5ad05799d30d97b9ced3be build: set Unicode true for NSIS installer (fanquake)
Pull request description:
Now that we are using Focal for gitian builds, and have [NSIS 3.0+ available](https://packages.ubuntu.com/focal/nsis) (also in Guix), we can create installers that [support unicode](https://nsis.sourceforge.io/Docs/Chapter4.html#aunicodetarget).
Unicode is only becoming the NSIS default [beginning with the 3.07 release](https://nsis.sourceforge.io/Docs/AppendixF.html#v3.07-cl), so we need to set this attribute to get support.
Should close: #13817
Gitian builds:
```bash
b8553615b6b4be5e4459e03796e700b30b5d198a7f184f27be6983ff901b5592 bitcoin-9086e0dd3c92-win-unsigned.tar.gz
a6b024a5a68e0196e8e118168c918285e820f2d0ffe9c38db680580459da8bf3 bitcoin-9086e0dd3c92-win64-debug.zip
ff4003d4f61127c707e44b5235eaf924b30351f20cde27e775131982a1b4cf92 bitcoin-9086e0dd3c92-win64-setup-unsigned.exe
1876bee55fa9ea99b91203975c13d0ad8a046b4b58068bde41c977fd1d12de13 bitcoin-9086e0dd3c92-win64.zip
000f2778f8f166a89b4ab35f155156c1c34800be6e47d29b5308043c50128392 src/bitcoin-9086e0dd3c92.tar.gz
d650a9b8f2dd1df777bf42439dfcbcf6bc358e30ec148b9992a18b39f76b1ecf bitcoin-core-win-22-res.yml
```
ACKs for top commit:
laanwj:
ACK 9086e0dd3c924b6c9a5ad05799d30d97b9ced3be
hebasto:
ACK 9086e0dd3c924b6c9a5ad05799d30d97b9ced3be, tested on Windows 10 Pro (20H2, build 19042.804):
Tree-SHA512: cc7b7ca05877571d0a29a7d36e40279f54d886d8ab27facfa722c2ee95a1fc06c2bad8ef1eb1980d283ae981659d737021a46c8f4618e24d510b5ab384990e09
## Issue being fixed or feature implemented
the problem with retries implemented in #4793 is that they don't really
do anything besides fetching results of a failed job multiple times 🙈
## What was done?
partially reverted changes done in #4793, implemented actual job
restart. dropped `--sleep` and `--retries` and added `--attempts`
instead.
## How Has This Been Tested?
Pick any test, add some randomly failing expression somewhere and run it
with some high number of retries.
For example:
```diff
diff --git a/test/functional/feature_dip0020_activation.py b/test/functional/feature_dip0020_activation.py
index 471e4fdc66..b56a954b78 100755
--- a/test/functional/feature_dip0020_activation.py
+++ b/test/functional/feature_dip0020_activation.py
@@ -69,6 +69,7 @@ class DIP0020ActivationTest(BitcoinTestFramework):
# Should be spendable now
tx0id = self.node.sendrawtransaction(tx0_hex)
assert tx0id in set(self.node.getrawmempool())
+ assert int(tx0id[0], 16) < 4
if __name__ == '__main__':
```
On develop:
```
./test/functional/test_runner.py feature_dip0020_activation.py --retries=100 --sleep=0
```
if this fails on the first run, it keeps "failing" (simply fetching the
same results actually) till the end.
With this patch:
```
./test/functional/test_runner.py feature_dip0020_activation.py --attempts=100
```
if this fails on the first run, it can actually succeed after a few
attempts now, unless you are extremely unlucky ofc 😄
Also, check [ci results in my repo
](https://cdn.artifacts.gitlab-static.net/93/b4/93b4f8b17e5dcccab1afee165b4d74d90f05800caf65d6c48a83a1a78c979587/2023_04_08/4081291268/4478867166/job.log?response-content-type=text%2Fplain%3B%20charset%3Dutf-8&response-content-disposition=inline&Expires=1680945516&KeyName=gprd-artifacts-cdn&Signature=2d4mHCJBbgRaTDiSQ6kKIy1PdIM=).
Note:
```
...
feature_dip3_v19.py failed at attempt 1/3, Duration: 159s
...
4/179 - [1mfeature_dip3_v19.py[0m passed, Duration: 244 s
...
feature_llmq_hpmn.py failed at attempt 1/3, Duration: 284s
...
feature_llmq_hpmn.py failed at attempt 2/3, Duration: 296s
...
11/179 - [1mfeature_llmq_hpmn.py[0m failed, Duration: 233 s
...
```
An example with 2 tests failing initially and then passing:
https://gitlab.com/dashpay/dash/-/jobs/4089689970
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
## What was done?
Added v20 BIP9 style fork structure along with utility functions.
Since several features coming depending on that fork status, we needed
to group them into one
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
CI should not be running that many jobs in "FAST_MODE"
https://github.com/dashpay/dash/blob/develop/.gitlab-ci.yml#L5
## What was done?
assign `.skip-in-fast-mode-template` template to more jobs
## How Has This Been Tested?
ran CI in my repo
before: https://gitlab.com/UdjinM6/dash/-/pipelines/832175740
after: https://gitlab.com/UdjinM6/dash/-/pipelines/832176325
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [ ] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
This adds a check that ensures branches merge cleanly into master via a
ff-only
## What was done?
Added a GitHub action created via gpt-4 :)
## How Has This Been Tested?
https://github.com/PastaPastaPasta/dash/actions/runs/4703432752/jobs/8341923994
and
https://github.com/PastaPastaPasta/dash/actions/runs/4703457936/jobs/8341980146
for expected pass and expected fail
## Breaking Changes
None, should be back ported to v19.x branch when we get the chance.
## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
https://github.com/dashpay/dash/issues/5294
## What was done?
Called `contrib/devtools/gen-manpages.sh` on top of new assume valid
values, see dash#5304
## How Has This Been Tested?
Reviewed only
## Breaking Changes
No breaking changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
https://github.com/dashpay/dash/issues/5294
## What was done?
Bumped defaultAssumeValid, nMinimumChainWork, chainTxData,
checkPointsData
## How Has This Been Tested?
Called rpcs `getblockhash N`, `getblock HASH`, `getchaintxstats 17280
HASH`
## Breaking Changes
No breaking changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
During reviewing TODO were found some TODOes that can be done now.
- fix: follow-up dash#3467 - replaced commented code to disabled code
- follow-up bitcoin#16394 - uncommented code related to `watchonly`
feature
- removed out-dated TODO in `rpc/masternode.cpp` (already done)
- fix: renamed name of clean up test_unittests: removed TODO and updated
name of variable TRAVIS
- rewritten todo inside `.travis.yml`
- fix: adds a missing description for result of rpc `mnsync`
Last commit (`mnsync`) is an only candidate for backport to v19, other
changes are non significant.
Run functional/unit tests
No breaking changes
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
This was reported/requested by @HashEngineering:
> Older versions of our App won't sync due to if (obj.nVersion ==
BASIC_BLS_VERSION) . Older versions don't know what version a SML Entry
is. As such, they will never read the type field. On the android client
this causes an offset problem when reading the mnlistdiff and it will
throw an exception that bans the peer that supplied it. Soon enough, no
peers will be left to connect to because they will all give the android
client bad data.
## What was done?
With this PR, SML will serialise the new v19 fields (`nType`,
`platformHTTPPort`, `platformNodeID`) if the client's version is at
least equal to `70227`.
Note: Serialisation for hashing skips the above rule.
Also, functional test mininode protocol version is set to `70227`.
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
1. we need to move time forward to let invs being relayed
2. nNextInvSend in SendMessages can be bumped up to 30+ seconds into the
future in rare cases
make sure timeouts in tests are high enough to relay tx inv/wait for
corresponding islock
## What was done?
tl;dr: bump mocktime while waiting, wait longer
extracted fixes from https://github.com/dashpay/dash/pull/5288 but I
expect this to fix other sporadic test failures too
## How Has This Been Tested?
tests are ok locally and in https://github.com/dashpay/dash/pull/5288
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [ ] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
should hopefully fix some sporadic ci test failures (like
https://gitlab.com/dashpay/dash/-/jobs/4052206622#L1962)
## What was done?
tweaked dynamically_add/update functions to make checks more consistent
and avoid some edge cases, pls see individual commits
## How Has This Been Tested?
`feature_llmq_hpmn.py` and `feature_dip3_v19.py` still work locally,
let's see if ci is now (constantly) happy about these too...
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
This was reported/requested by @HashEngineering:
> Older versions of our App won't sync due to if (obj.nVersion ==
BASIC_BLS_VERSION) . Older versions don't know what version a SML Entry
is. As such, they will never read the type field. On the android client
this causes an offset problem when reading the mnlistdiff and it will
throw an exception that bans the peer that supplied it. Soon enough, no
peers will be left to connect to because they will all give the android
client bad data.
## What was done?
With this PR, SML will serialise the new v19 fields (`nType`,
`platformHTTPPort`, `platformNodeID`) if the client's version is at
least equal to `70227`.
Note: Serialisation for hashing skips the above rule.
Also, functional test mininode protocol version is set to `70227`.
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
During reviewing TODO were found some TODOes that can be done now.
## What was done?
- fix: follow-up dash#3467 - replaced commented code to disabled code
- follow-up bitcoin#16394 - uncommented code related to `watchonly`
feature
- removed out-dated TODO in `rpc/masternode.cpp` (already done)
- fix: renamed name of clean up test_unittests: removed TODO and updated
name of variable TRAVIS
- rewritten todo inside `.travis.yml`
- fix: adds a missing description for result of rpc `mnsync`
Last commit (`mnsync`) is an only candidate for backport to v19, other
changes are non significant.
## How Has This Been Tested?
Run functional/unit tests
## Breaking Changes
No breaking changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
https://github.com/dashpay/dash/issues/5294
## What was done?
Bumped defaultAssumeValid, nMinimumChainWork, chainTxData,
checkPointsData
## How Has This Been Tested?
Called rpcs `getblockhash N`, `getblock HASH`, `getchaintxstats 17280
HASH`
## Breaking Changes
No breaking changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
should hopefully fix some sporadic ci test failures (like
https://gitlab.com/dashpay/dash/-/jobs/4052206622#L1962)
## What was done?
tweaked dynamically_add/update functions to make checks more consistent
and avoid some edge cases, pls see individual commits
## How Has This Been Tested?
`feature_llmq_hpmn.py` and `feature_dip3_v19.py` still work locally,
let's see if ci is now (constantly) happy about these too...
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
9cc8e30125df14fe47e21e55ab3bf26f4d416565 test: fix sign comparison warning in socket tests (fanquake)
Pull request description:
This fixes:
```bash
In file included from test/sock_tests.cpp:10:
In file included from /usr/local/include/boost/test/unit_test.hpp:18:
In file included from /usr/local/include/boost/test/test_tools.hpp:46:
/usr/local/include/boost/test/tools/old/impl.hpp:107:17: warning: comparison of integers of different signs: 'const long' and 'const unsigned long' [-Wsign-compare]
return left == right;
~~~~ ^ ~~~~~
```
which was introduced in #20788.
ACKs for top commit:
practicalswift:
cr ACK 9cc8e30125df14fe47e21e55ab3bf26f4d416565
vasild:
ACK 9cc8e30125df14fe47e21e55ab3bf26f4d416565
Tree-SHA512: 7069a4fde5cec01be03f8477fe396e53658f170efbf1d9ef3339d553bb90a2be9f4acd6b348127b14cd2f91426e0cd1fc35d2d3c9f201cf748c0cf50f47e46a5
fabeb5b9c7f678ab3bc24c1860f8514ac52bb56f fuzz: Disable shuffle when merge=1 (MarcoFalke)
Pull request description:
This should hopefully help make the deletion of fuzz inputs more deterministic.
My tests (N=1) revealed that without this patch 7000 files differ (https://github.com/bitcoin-core/qa-assets/pull/44#issuecomment-768841467). With this patch, "only" 2000 files differ.
ACKs for top commit:
practicalswift:
cr ACK fabeb5b9c7f678ab3bc24c1860f8514ac52bb56f: `-shuffle=0` and `-prefer_small=1` make sense
Tree-SHA512: 21a701f52450d402a91dd6e0b33d564c63a9c3b919738eb9a80c24d48fc5b964088e325470738f39af0d595612c844acc7bf0941590cc2dc8c6f6ee4cb69c861
b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb build: Add -Werror=mismatched-tags (Hennadii Stepanov)
1485124291368c4a2ca8ea09c18e813f1dbabf5c Fix -Wmismatched-tags warnings (Hennadii Stepanov)
Pull request description:
Warnings were introduced in #20749:
```
./validation.h:43:1: warning: class 'CCheckpointData' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]
class CCheckpointData;
^
./chainparams.h:24:8: note: previous use is here
struct CCheckpointData {
^
./validation.h:43:1: note: did you mean struct here?
class CCheckpointData;
^~~~~
struct
1 warning generated.
```
This change fixes AppVeyor build: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/37547435
ACKs for top commit:
glozow:
utACK b6aadcd5b4🚗
practicalswift:
cr ACK b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb: patch looks correct
Tree-SHA512: 3ac887ebdbf9a1ae33c1fd5381b3b8d83388ad557ddeb55013acd42bb9752a5bd009e3a0eed52644a023a7a0dda1c159277981af82f58fb0abfe60b84e01bf29
5b41d84b3469484ff6a1f4eb7c890b6444a16091 doc: add xorriso to macOS depends packages (fanquake)
Pull request description:
This was missed in #20470.
ACKs for top commit:
hebasto:
ACK 5b41d84b3469484ff6a1f4eb7c890b6444a16091, tested on Linux Mint 20.1 (x86_64).
Tree-SHA512: bcfd8468a099c69175f8a9d295c1466764ab25d6a61121b28675a09c3e96f45b6309e1523d341f4cb21d0ddee4945f00ba060ba02da835f2f0db7e694fd6c44b
ad57fb756b1c2df625790bd9c296ec28daa93740 wallet: Add BerkeleyDB version sanity check at init time (Wladimir J. van der Laan)
Pull request description:
Detect version conflicts between the run-time BerkeleyDB library and the one used during compilation.
This is very unsafe (can result in anything from crashes to corruption) so shut down when one is detected.
ACKs for top commit:
decryp2kanon:
utACK ad57fb7
achow101:
ACK ad57fb756b1c2df625790bd9c296ec28daa93740
theStack:
utACK ad57fb756b1c2df625790bd9c296ec28daa93740
meshcollider:
Code review ACK ad57fb756b1c2df625790bd9c296ec28daa93740
Tree-SHA512: 99cd7d836bffbdeb3d4e14053f7139cc85a6d42e631a3f9a3058a848042446b364faee127500f5acb374616e6a61ab2bedebfac1ba9bc993b4d6227114c2a6c2
faccf8b1e1af293dfe9158d732718e7798a2fd89 refactor: Enable -Wswitch for FeeEstimateHorizon (MarcoFalke)
Pull request description:
This enables the `-Wswitch` compiler warning for `FeeEstimateHorizon` by removing the `default` case in `switch` statements.
ACKs for top commit:
practicalswift:
cr ACK faccf8b1e1af293dfe9158d732718e7798a2fd89
jonatack:
ACK faccf8b1e1af293dfe9158d732718e7798a2fd89
hebasto:
ACK faccf8b1e1af293dfe9158d732718e7798a2fd89, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 63a8dff6e8dead149ec2fa8319e7ff41022c9534d423d3086fd8f22be073dc4915f74c7fe9139ee681a8204730cf58c80ef40c93fb33032d586e68b4f78f557d
