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
fa0d8359b351fd179a0a2f458671a4d7828c9a80 log: Clarify that failure to read fee_estimates.dat is non-fatal (MarcoFalke)
faefa5db5f1d95b772873f4429e8a8fbb4e71cf3 log: Clarify that failure to write fee_estimates.dat is non-fatal (MarcoFalke)
Pull request description:
two minor logging fixups
ACKs for top commit:
practicalswift:
ACK fa0d8359b351fd179a0a2f458671a4d7828c9a80: patch looks correct
laanwj:
Code review ACK fa0d8359b351fd179a0a2f458671a4d7828c9a80
Tree-SHA512: d1e7e595d3b4a5e497ee7ab70f3be5783dafec2726ef8e012db836c15e8e622022859a4472d6b516fe19d327737b25fdfb509cd9aeb022ca847b13c54e55800a
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
3ebde2143aa98af213872b98b474d904e55056f7 [test] Fix wait condition in disconnect_p2ps (Amiti Uttarwar)
Pull request description:
#19315 currently has a [test failure](https://cirrus-ci.com/task/4545582645641216) because of a race. `disconnect_p2ps` is intended to have a `wait_until` clause that prevents this race, but the conditional doesn't match since its comparing two different object types. `MY_SUBVERSION` is defined in messages.py as a byte string, but is compared to the value returned by the RPC. This PR simply converts types to ensure they match, which should prevent the race from occurring.
HUGE PROPS TO jnewbery for discovering the issue 🔎
ACKs for top commit:
jnewbery:
ACK 3ebde2143aa98af213872b98b474d904e55056f7
glozow:
Code review ACK 3ebde2143a
Tree-SHA512: ca096b80a3e4d757a645f38846d6dc89d6a3d35c3435513a72d278e305faddd4aff9e75a767941b51b2abbf59c82679bac1e9a0140d6f285efe3053e51bcc2a8
defe48a51f4315f8cc607875a099981593c8cc39 doc: Update wallet files in files.md (Hennadii Stepanov)
Pull request description:
This PR is a #19077 follow up, and it addresses the [comment](https://github.com/bitcoin/bitcoin/pull/19077#discussion_r504805234):
> If need to update, there are two corrections that could be made:
>
> * Line 69 "Wallets are Berkeley DB (BDB) databases" is no longer true
>
> * Line 76 "Wallet lock file" should say "BDB wallet lock file"
ACKs for top commit:
RiccardoMasutti:
ACK defe48a
meshcollider:
ACK defe48a51f4315f8cc607875a099981593c8cc39
Tree-SHA512: 39939f86a9c7842bf06913998305dcbd6209585f1da0fe9c274bac0572eb8464e59176884dd9e2b91312f34efad40cdeb4085ec72c2a2c1b33d16b6ab505140c
675e55e01392971aa56bda56cb09498b466d0902 Ignore unknown messages before VERACK (Suhas Daftuar)
Pull request description:
This allows for feature negotiation to take place with messages between VERSION and VERACK in the future, without requiring additional software changes to specifically ignore messages for features that are unimplemented by our software.
ACKs for top commit:
sipa:
utACK 675e55e01392971aa56bda56cb09498b466d0902
practicalswift:
ACK 675e55e01392971aa56bda56cb09498b466d0902: patch looks correct
MarcoFalke:
ACK 675e55e01392971aa56bda56cb09498b466d0902
hebasto:
ACK 675e55e01392971aa56bda56cb09498b466d0902, the offender peer will be eventually disconnected due to the timeout.
Tree-SHA512: 8d2b1d8b9843f2ee26b2c30f7c5ff0bfcfbe3f46b32cd0369c48ece26624151091237e83ce3f18c6da004099026602cfab1642ac916db777f047d170b365c007
638441928a446726ce3a7fb20433a5478e7585bb test: add parameterized constructor for msg_sendcmpct() (Sebastian Falbesoner)
Pull request description:
While working on the test for #19776 I noticed that creating a `sendcmpct` message is quite cumbersome -- due to the lack of a parameterized constructor, one needs to create an empty (that is, initialized with default values) object and then set the two fields one by one. This PR replaces the default constructor with a parameterized constructor and uses it in the test `p2p_compactblocks.py`, reducing LOC. No need to pollute the namespace with temporary throw-away message objects anymore.
ACKs for top commit:
guggero:
Code review ACK 638441928a446726ce3a7fb20433a5478e7585bb.
epson121:
Code review ACK 638441928a446726ce3a7fb20433a5478e7585bb
Tree-SHA512: 3b58d276d714b73abc6cc98d1d52dec5f6026b33f03faaeb7dcbc5d83ac377555179f98b159b2b9ecc8957999c35a1dc082e3c69299c5fde4e35f1bd0587ce9d
637d8bce741213295bd9b9d1982cae663c701ba1 Change FILE_CHAR_BLOCKLIST to FILE_CHARS_DISALLOWED (Benoit Verret)
Pull request description:
Blocklist is ambiguous. It could mean a list of blocks.
Example: "blocknotify" in the same file refers to Bitcoin blocks.
ACKs for top commit:
MarcoFalke:
ACK 637d8bce741213295bd9b9d1982cae663c701ba1
laanwj:
ACK 637d8bce741213295bd9b9d1982cae663c701ba1 — this is a clear variable name improvement
theStack:
ACK 637d8bce741213295bd9b9d1982cae663c701ba1
jonatack:
ACK 637d8bce741213295bd9b9d1982cae663c701ba1
eriknylund:
ACK 637d8bce741213295bd9b9d1982cae663c701ba1
promag:
ACK 637d8bce741213295bd9b9d1982cae663c701ba1.
Tree-SHA512: 028e7102eeaf61105736c55c119a7f5c05411f2b6715a7939c41cb9e8f13afb757bbb6e7a302b3aae21722e69dab91f6eff8099e5884d248299905b4c7687c02
