9ad26f1664 fix: follow-up bitcoin#20773 - coinjoin loader can be nullptr too (Konstantin Akimov)
4125485401 fix: follow-up bitcoin#20773 - for collateral lock/unlock coins (Konstantin Akimov)
06ea87040b Merge bitcoin/bitcoin#20773: refactor: split CWallet::Create (W. J. van der Laan)
d89847f43e Merge bitcoin/bitcoin#22004: fuzz: Speed up transaction fuzz target (MarcoFalke)
82a6aa5907 Merge bitcoin/bitcoin#21810: fuzz: Various RPC fuzzer follow-ups (MarcoFalke)
3bfefde53b Merge bitcoin/bitcoin#21931: ci: Bump cirrus fuzz CPUs to avoid timeout (MarcoFalke)
2398283ff6 Merge bitcoin/bitcoin#21922: fuzz: Avoid timeout in EncodeBase58 (fanquake)
813993d44a Merge bitcoin/bitcoin#21892: fuzz: Avoid excessively large min fee rate in tx_pool (MarcoFalke)
4dd36f603a Merge bitcoin/bitcoin#21798: fuzz: Create a block template in tx_pool targets (MarcoFalke)
c2bd834f3b Merge bitcoin/bitcoin#21169: fuzz: Add RPC interface fuzzing. Increase fuzzing coverage from 65% to 70%. (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
Regular backports from bitcoin v22
## What was done?
see commits
## How Has This Been Tested?
Run unit/functional tests
## 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
- [x] I have assigned this pull request to a milestone
Top commit has no ACKs.
Tree-SHA512: 5d2cca761bb7971b3d83f1b54a5c26d62c9495becdf6d897e22bfd975837605bb5326dc37793f838389c9145db34ef45992be95e789af8c12af9b6ade79a0038
489ebb7b34c403a3ce78ff6fb271f8e6ecb47304 wallet: make chain optional for CWallet::Create (Ivan Metlushko)
d73ae939649f3b30e52b5a2cccd7fafd1ab36766 CWallet::Create move chain init message up into calling code (Ivan Metlushko)
44c430ffac940e1d1dd7f5939a495470bc694489 refactor: Add CWallet:::AttachChain method (Russell Yanofsky)
e2a47ce08528dfb39c0340145c6977f6afd587f6 refactor: move first run detection to client code (Ivan Metlushko)
Pull request description:
This is a followup for https://github.com/bitcoin/bitcoin/pull/20365#discussion_r522265003
First part of a refactoring with overall goal to simplify `CWallet` and de-duplicate code with `wallettool`
**Rationale**: split `CWallet::Create` and create `CWallet::AttachChain`.
`CWallet::AttachChain` takes chain as first parameter on purpose. In future I suggest we can remove `chain` from `CWallet` constructor.
The second commit is based on be164f9cf89b123f03b926aa980996919924ee64 from #15719 (thanks ryanofsky)
cc ryanofsky achow101
ACKs for top commit:
ryanofsky:
Code review ACK 489ebb7b34c403a3ce78ff6fb271f8e6ecb47304. Only changes since last review were adding a const variable declaration, and implementing suggestion not to move feerate option checks to AttachChain. Thanks for updates and fast responses!
Tree-SHA512: 00235abfe1b00874c56c449adcab8a36582424abb9ba27440bf750af8f3f217b68c11ca74eb30f78a2109ad1d9009315480effc78345e16a3074a1b5d8128721
bbbb51877a96c3473aeea5914f751eec7835b5c4 fuzz: Speed up transaction fuzz target (MarcoFalke)
Pull request description:
`hashBlock` and `include_addresses` are orthogonal, so no need to do an exhaustive "search".
Might fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34491
ACKs for top commit:
practicalswift:
cr ACK bbbb51877a96c3473aeea5914f751eec7835b5c4: patch looks correct, and `TxToUniv` surprisingly wide in the `transaction_fuzz_target` flame graph! Putting it on a diet makes sense.
Tree-SHA512: 1e7c30c7fecf96364a9a1597c0a22139389fdeb67db59f3c2c6fc088196e3332877b2865991a957980d542f99a2f48cc066dd7cc16c695a5113190fe06205089
5252f86eb616a1112e427c268c8e8825f2a63d67 fuzz: Reduce maintenance requirements by allowing RPC annotations also for conditionally available RPC commands (such as wallet commands) without the fragility of #ifdef forests (practicalswift)
54549dda310e2becee9cb4997d1408a90e91934f fuzz: RPC fuzzer post-merge follow-ups. Remove unused includes. Update list of fuzzed RPC commands. (practicalswift)
Pull request description:
Various RPC fuzzer follow-ups:
* Remove unused includes.
* Update list of fuzzed RPC commands.
* Reduce maintenance requirements by allowing RPC annotations also for conditionally available RPC commands (such as wallet commands) without the fragility of `#ifdef` forests.
Context: https://github.com/bitcoin/bitcoin/pull/21169#pullrequestreview-646723483
ACKs for top commit:
MarcoFalke:
Concept ACK 5252f86eb616a1112e427c268c8e8825f2a63d67
Tree-SHA512: 286d70798131706ffb157758e1c73f7f00ed96ce120c7d9dc849e672b283f1362df47b206cfec9da44d5debb5869225e721761dcd5c38a7d5d1019dc6c912ab2
99993f066405863c66ccaec0a8427129f4515768 fuzz: Avoid excessively large min fee rate in tx_pool (MarcoFalke)
Pull request description:
Any fee rate above 1 BTC / kvB is clearly nonsense, so no need to fuzz this.
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34078
ACKs for top commit:
practicalswift:
cr ACK 99993f066405863c66ccaec0a8427129f4515768: patch looks correct despite no `fa` prefix in commit hash
Tree-SHA512: bd3651d354b13d889ad1708d2b385ad0479de036de74a237346eefad5dbfb1df76ec02b55ec00487ec598657ef6102f992302b14c4e47f913a9962f81f4157e6
fa03d0acd6bd8bb6d3d5227512f042ff537ad993 fuzz: Create a block template in tx_pool targets (MarcoFalke)
fa61ce5cf5c1d73d352173806571bcd7799ed2ee fuzz: Limit mocktime to MTP in tx_pool targets (MarcoFalke)
fab646b8ea293bb2b03707c6ef6790982625e492 fuzz: Use correct variant of ConsumeRandomLengthString instead of hardcoding a maximum size (MarcoFalke)
fae2c8bc54e6c0fe69a82bd1b232c52edd1acd34 fuzz: Allow to pass min/max to ConsumeTime (MarcoFalke)
Pull request description:
Relatively simple check to ensure a block can always be created from the mempool
ACKs for top commit:
practicalswift:
Tested ACK fa03d0acd6bd8bb6d3d5227512f042ff537ad993
Tree-SHA512: e613376ccc88591cbe594db14ea21ebc9b2b191f6325b3aa4ee0cd379695352ad3b480e286134ef6ee30f043d486cf9792a1bc7e44445c41045ac8c3b931c7ff
545404e7e1c72985557ccffe865cea269143e5dd fuzz: Add RPC interface fuzzing. Increase fuzzing coverage from 65% to 70%. (practicalswift)
Pull request description:
Add RPC interface fuzzing.
This PR increases overall fuzzing line coverage from [~65%](https://marcofalke.github.io/btc_cov/fuzz.coverage/) to ~70% 🎉
To test this PR:
```
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make -C src/ test/fuzz/fuzz
$ FUZZ=rpc src/test/fuzz/fuzz
```
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for more information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
ACKs for top commit:
MarcoFalke:
Concept ACK 545404e7e1c72985557ccffe865cea269143e5dd
Tree-SHA512: 35fc1b508af42bf480ee3762326b09ff2eecdb7960a1917ad16345fadd5c0c21d666dafe736176e5a848ff6492483c782e4ea914cd9000faf50190df051950fd
2777d30caa fix(ci): only run some GH actions on PR approval, not on every comment (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/events-that-trigger-workflows#running-a-workflow-when-a-pull-request-is-approved
## What was done?
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [ ] 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK 2777d30caa
Tree-SHA512: 183deae1d1b32408bd81962721141e976912b2614a046ab23565537db62fcff7d367331db8ced7cf176dc67adbc18b96e7678b37068f807aaafb1718d55dea70
540f6871d3 fix: lock `::cs_main` before accessing `ChainstateManager::m_best_header` (Kittywhiskers Van Gogh)
aafded67d9 fix: compilation error due to rebase error between bitcoin#22937 and ipc/process (Kittywhiskers Van Gogh)
Pull request description:
## Issue being fixed or feature implemented
CI failure for multiprocess
## What was done?
It resolve compilation failure on develop (apply changes from 22937 to ipc/process)
See alternate solutions:
- https://github.com/dashpay/dash/pull/6192
- https://github.com/dashpay/dash/pull/6195
## How Has This Been Tested?
Run build locally:
```
make MULTIPROCESS=1 -j10
./configure --prefix=$(pwd)/depends/x86_64-pc-linux-gnu --enable-suppress-external-warnings --enable-werror --enable-debug --enable-crash-hooks --enable-maintainer-mode --enable-stacktraces --enable-multi-process
```
## 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
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK 540f6871d3
Tree-SHA512: 75b675e93763e8e53086a10b93516b8ec94418902f9be2de738517176cc835c0dad25c286426089a5327a9c13d1787b89debda2c6025cb598489204d7d5af2cf
5394d63e18 feat: improve merge-check action to leave a comment and label to PRs (pasta)
Pull request description:
## Issue being fixed or feature implemented
this is untested but I think it'll work, simply add comment and label to PRs which fail this CI
## What was done?
## How Has This Been Tested?
hasn't but if this action breaks that is not critical
## Breaking Changes
None
## Checklist:
- [ ] 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
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK 5394d63e18
Tree-SHA512: 78ae7c05446a0625143fb7e04b7e63ae0830ca45cde4f924eb4e04dfb12ac3cc9e697efbd914a71a9b5ba98400597e7559258b5d93419ed7eb7cc14a9b876fe7
770651aa15 set hosts in guix-check (pasta)
580bbe6d1c feat: improve guix building; run always, save artifacts (pasta)
101a31555f refactor: simplify caching setup, add a restore key to actually cache besides 1 run (pasta)
1b139e4837 feat: automatically run guix-build on all tags pushed (pasta)
Pull request description:
## Issue being fixed or feature implemented
Previously, we only ran guix on 1 machine for all hosts; this slowed it down a lot. Let's move to GitHub action runners, but run them all separately. Then upload the artifacts.
In the future there is significant caching I can add that should help a lot more. But currently, takes about 1 hour
## What was done?
## How Has This Been Tested?
see: https://github.com/PastaPastaPasta/dash/actions/runs/10345024600
## Breaking Changes
None
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [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
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK 770651aa15
Tree-SHA512: 639b95c3b6a26f205ed00c138a9189f915cfc36a815516035e59ceda82675414b1bd31a361b33449b5e4c58a7655f3a7d616b362c23f7fa75e72b1284be06b9e
92f82f6fc6 fix: group the ENV in quotations (pasta)
2237b4266b refactor: use `=` with ENV in dockerfiles (pasta)
Pull request description:
## Issue being fixed or feature implemented
see: https://docs.docker.com/go/dockerfile/rule/legacy-key-value-format/
Basically avoid old format of ENV in docker files
## What was done?
see commit
## How Has This Been Tested?
Building the containers locally, seems all fine
## Breaking Changes
_Please describe any breaking changes your code introduces_
## 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
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 92f82f6fc6
UdjinM6:
utACK 92f82f6fc6
Tree-SHA512: 0fb5028b9fd8e8f2cac6d7488c3254f58a9ee96f2c236a1d0b90ea48f57b793be3caab6fc764b4ac1127eada9fae85468a13d2eb2a1c3e0a6940a6c8bc601681
acf1315270 Merge bitcoin/bitcoin#25091: test: Remove extended lint (cppcheck) (MacroFake)
4dbdecdd1e refactor: rename builder-image -> build-image and builder as image name to dashcore-ci-runner (pasta)
ed8ffa7fb4 feat: have cppcheck linter respect CACHE_DIR env variable (pasta)
d1addb27aa fix: change fallback download path to be an s3 link which includes a few packages (pasta)
35c76705d1 feat: implement basic Github Actions based CI, which reuses underlying logic from GitLab CI (pasta)
Pull request description:
## Issue being fixed or feature implemented
We currently rely on GitLab for running CI, and while it has worked quite well, I am worried about having all of our eggs in one basket! As such, I've long wanted to explore implemeenting Github Actions based CI, but was too lazy! Well I finally spent some 60 commits trying to figure everything out, and this PR is the result of it.
As a result, we will now have two semi-redunant CI systems, the primary one on GitLab, and this one on Github Actions. Currently the GA based CI will only do one host, and does linting. Be aware this GA based CI does not actually run the tests, it does build depends, src and run linters on 1 host.
In the future, we should expand it from simply arm32 builds to having feature parity to GitLab.
While it appears the GA default runners are a bit slower than what we have on GitLab, there's a big difference, the GA runners are free :D If we decide to make the GA based CI primary, we'll probably want to setup some custom runners to have improved build speeds. Even still, a heavily cached build doing all linters took around 5 minutes if I recall correctly. Without caches I think it took maybe an hour, so defenitely not bad.
## What was done?
See the individual commits, they're pretty self explanatory
## How Has This Been Tested?
Lots of CI runs on my prior branch :) CI should run on this PR, and we should see how long it'll take w/o cache :D
## Breaking Changes
N/A - CI only
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
Top commit has no ACKs.
Tree-SHA512: fef41d1b73fdeac29e5096ffa7fa26660efc44ac274376d5880b5fabf6b0e760aeda4a4ae9c24656ee0e3adb760459f0d45b955cefffc9d0eeb5384afbc9d473
BACKPORT NOTICE:
we keep and maintain cppcheck linter as lint-cppcheck-dash.sh
efae252f3072da598160670691757a0d60b9beb4 test: Remove extended lint (cppcheck) (laanwj)
Pull request description:
These are unreferenced in the CI and documentation, and have been since 2019 (see #17549).
I'm not sure the cppcheck is worthwhile. It takes a long time to run (I think this is why it isn't in the normal lints), and right
now it only appears to find implicit constructors. The list of exceptions is out of date. But if anyone wants to bring it back at any
time in the future they can do so from git history (and port it to Python).
ACKs for top commit:
fanquake:
ACK efae252f3072da598160670691757a0d60b9beb4
Tree-SHA512: 1a770b5d20ff1199d0d6bc471ae3d2c3438f0f0b169ce8d2fe73480daf8d3a7146c066b799afc90aa7898982c5fee79c1daca10e16e2bff0a7b38850aedd55b2
Reasonably enough I guess, it appears that a number of our dependencies block github runner IP addresses? the blobs stored at the s3 link were downloadable from the normal location when accessed locally, but returned a 404 from github runners. These blobs were also not present in the bitcoincore depends-sources anyhow.
f4cb0fbfe1 fix: no need to relay quorum commitment in case of block undo (Konstantin Akimov)
0431a33919 fix: follow-up changes for bitcoin#14193. (Konstantin Akimov)
86b76d19b6 Merge bitcoin/bitcoin#21812: ci: Enable D_GLIBCXX_DEBUG for multiprocess task (fanquake)
334496ea7e Merge bitcoin/bitcoin#21775: p2p: Limit m_block_inv_mutex (MarcoFalke)
23b83109ea Merge bitcoin/bitcoin#21750: net: remove unnecessary check of CNode::cs_vSend (MarcoFalke)
b34514191f Merge bitcoin/bitcoin#21738: test: Use clang-12 for ASAN, Add missing suppression (fanquake)
3411577473 Merge bitcoin/bitcoin#19160: multiprocess: Add basic spawn and IPC support (W. J. van der Laan)
970048d917 fix: missing changes from bitcoin#19267 - run multiprocess on CI (Konstantin Akimov)
f2b7ee73db fix: follow-up bitcoin#15402 - removed dead code (Konstantin Akimov)
274068cdbc fix: follow-up bitcoin/bitcoin#21732 - minor missing typo (MarcoFalke)
e9450a8b36 Merge #21669: test: Remove spurious double lock tsan suppressions by bumping to clang-12 (MarcoFalke)
ef92c3065c Merge #21663: ci: Fix macOS brew install command (W. J. van der Laan)
Pull request description:
## Issue being fixed or feature implemented
Just regular backports from v22
## What was done?
See commits for backports.
Also there're 2 bugs are fixed which became visible after backporting bitcoin#21775 - both are related to possible deadlocks in net_processing
## How Has This Been Tested?
Run unit and functional tests. Enabled multiprocess builds on CI
## 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
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
utACK f4cb0fbfe1
PastaPastaPasta:
utACK f4cb0fbfe1
Tree-SHA512: 3204c2aa243fa4834ccf4ff4672d183cf9b35f87b857df8543572cd685729e15fca39f85b27194233e57cbc1746e36b556efab95ce20d0aa0a7d4476a9f3c6c0
92834d1ef2 fix: help p2p_timeouts to succeed on the my localhost (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Locally on my environment the functional tests `p2p_timeouts.py` fails in 80% runs.
Output:
```
stdout:
2024-08-08T05:02:32.216000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_∋_🏃_20240808_120217/p2p_timeouts_0
2024-08-08T05:02:35.079000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
def test_function():
if check_connected:
assert self.is_connected
return test_function_in()
'''
2024-08-08T05:02:35.080000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "DASH/test/functional/test_framework/test_framework.py", line 159, in main
self.run_test()
File "DASH/test/functional/p2p_timeouts.py", line 93, in run_test
no_verack_node.wait_for_disconnect(timeout=1)
File "DASH/test/functional/test_framework/p2p.py", line 495, in wait_for_disconnect
self.wait_until(test_function, timeout=timeout, check_connected=False)
File "DASH/test/functional/test_framework/p2p.py", line 487, in wait_until
wait_until_helper(test_function, timeout=timeout, lock=p2p_lock, timeout_factor=self.timeout_factor)
File "DASH/test/functional/test_framework/util.py", line 267, in wait_until_helper
raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
AssertionError: Predicate ''''
def test_function():
if check_connected:
assert self.is_connected
return test_function_in()
''' not true after 1.0 seconds
2024-08-08T05:02:35.581000Z TestFramework (INFO): Stopping nodes
2024-08-08T05:02:36.582000Z TestFramework (WARNING): Not cleaning up dir /tmp/test_runner_∋_🏃_20240808_120217/p2p_timeouts_0
2024-08-08T05:02:36.582000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/test_runner_∋_🏃_20240808_120217/p2p_timeouts_0/test_framework.log
2024-08-08T05:02:36.582000Z TestFramework (ERROR):
2024-08-08T05:02:36.582000Z TestFramework (ERROR): Hint: Call DASH/test/functional/combine_logs.py '/tmp/test_runner_∋_🏃_20240808_120217/p2p_timeouts_0' to consolidate all logs
2024-08-08T05:02:36.582000Z TestFramework (ERROR):
2024-08-08T05:02:36.582000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-08-08T05:02:36.582000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
2024-08-08T05:02:36.582000Z TestFramework (ERROR):
stderr:
TEST | STATUS | DURATION
p2p_timeouts.py | ✓ Passed | 4 s
p2p_timeouts.py | ✖ Failed | 4 s
p2p_timeouts.py | ✖ Failed | 5 s
p2p_timeouts.py | ✖ Failed | 5 s
p2p_timeouts.py | ✖ Failed | 6 s
p2p_timeouts.py | ✖ Failed | 6 s
p2p_timeouts.py | ✖ Failed | 7 s
ALL | ✖ Failed | 37 s (accumulated)
Runtime: 7 s
```
## What was done?
Increased a timeout to see for first disconnect event +1 second.
## How Has This Been Tested?
100% succeed:
```
$ test/functional/test_runner.py -j20 p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py p2p_timeouts.py
p2p_timeouts.py | ✓ Passed | 4 s
p2p_timeouts.py | ✓ Passed | 5 s
p2p_timeouts.py | ✓ Passed | 5 s
p2p_timeouts.py | ✓ Passed | 6 s
p2p_timeouts.py | ✓ Passed | 6 s
p2p_timeouts.py | ✓ Passed | 7 s
p2p_timeouts.py | ✓ Passed | 7 s
p2p_timeouts.py | ✓ Passed | 8 s
p2p_timeouts.py | ✓ Passed | 8 s
p2p_timeouts.py | ✓ Passed | 9 s
p2p_timeouts.py | ✓ Passed | 9 s
p2p_timeouts.py | ✓ Passed | 10 s
p2p_timeouts.py | ✓ Passed | 10 s
p2p_timeouts.py | ✓ Passed | 11 s
p2p_timeouts.py | ✓ Passed | 11 s
p2p_timeouts.py | ✓ Passed | 12 s
```
## 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
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
ACK 92834d1ef2
PastaPastaPasta:
utACK 92834d1ef2
Tree-SHA512: 598178fd97e82def16b32cbaf1f476d3416768456a7f92fb4faadc041b73147cc7be3e6760287bde22d1a3e5a5a9190124ede6da81a1722feba1e80fcc3ae4e3
c92b0f57da merge bitcoin#25720: Reduce bandwidth during initial headers sync when a block is found (Kittywhiskers Van Gogh)
0f9ece0ed9 merge bitcoin#25514: Move CNode::nServices and CNode::nLocalServices to Peer (Kittywhiskers Van Gogh)
c9923ca36b partial bitcoin#25454: Avoid multiple getheaders messages in flight to the same peer (Kittywhiskers Van Gogh)
26d477b6ae revert: Fix duplicate initial headers sync (Kittywhiskers Van Gogh)
abccb2dd03 test: drop genesis block from `blockheader_testnet3` (Kittywhiskers Van Gogh)
0574a7d19e merge bitcoin#25404: Use MAX_BLOCKS_TO_ANNOUNCE consistently (Kittywhiskers Van Gogh)
ed871d2a07 merge bitcoin#24171: Sync chain more readily from inbound peers during IBD (Kittywhiskers Van Gogh)
a04290fc5c merge bitcoin#24178: Respond to getheaders if we have sufficient chainwork (Kittywhiskers Van Gogh)
bcafa282a3 merge bitcoin#24909: Move and rename pindexBestHeader, fHavePruned (Kittywhiskers Van Gogh)
70485cb2f5 partial bitcoin#24169: Add --enable-c++20 option (Kittywhiskers Van Gogh)
27e885de5f merge bitcoin#23880: Serialize cmpctblock at most once in NewPoWValidBlock (Kittywhiskers Van Gogh)
9f7ac69a7e merge bitcoin#24024: Remove cs_main lock annotation from ChainstateManager.m_blockman (Kittywhiskers Van Gogh)
9399f90a13 partial bitcoin#23832: Changes time variables from int to chrono (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Dependent on https://github.com/dashpay/dash/pull/6085
* Dependency for https://github.com/dashpay/dash/pull/6098
* ~~[bitcoin#25514](https://github.com/bitcoin/bitcoin/pull/25514) removes `peers.spvNodeConnections` and `peers.fullNodeConnections` reporting from `CalculateNumConnectionsChangedStats` as the services flags used to distingush between the two have been moved to the `Peer` struct, accessable only through `PeerManager`.~~
~~As `PeerManager` isn't accessable to `CConnman`, even if a new public function was exposed through `PeerManger` (as we have for `IsInvInFilter` and others or we try to access the value through `GetNodeStateStats`), `CConnman` would be unable to leverage it.~~ Resolved with patch by UdjinM6, thanks!
* [bitcoin#23880](https://github.com/bitcoin/bitcoin/pull/23880) introduces code that is not valid C++20 (but valid C++17) and therefore, required a partial backport of [bitcoin#24169](https://github.com/bitcoin/bitcoin/pull/24169) (fae679065e4ef0c6383bbdd1876aaed6c1e40104) to make the code C++20 legal.
* [bitcoin#25454](https://github.com/bitcoin/bitcoin/pull/25454) introduces a 10-point penalty for remitting more than `MAX_BLOCKS_TO_ANNOUNCE` unconnected block headers. `blockheader_testnet3.hex` (introduced in [bitcoin#16551](https://github.com/bitcoin/bitcoin/pull/16551), part of [dash#5963](https://github.com/dashpay/dash/pull/5963)), unlike in Bitcoin, includes the genesis block.
By definition of a genesis block, there is no block before it that connects to, which causes the 10-point penalty to trip and `p2p_dos_header_tree.py` to fail (see below). This has been remedied by removing the genesis block from the test data to match upstream and also because no node has a good reason to ever broadcast the genesis block as-is over P2P.
<details>
<summary>Test Failure</summary>
```
dash@6a2649cc721f:/src/dash$ ./test/functional/p2p_dos_header_tree.py
2024-06-17T17:59:35.874000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_h5hfluy1
2024-06-17T17:59:36.892000Z TestFramework (INFO): Read headers data
2024-06-17T17:59:36.895000Z TestFramework (INFO): Feed all non-fork headers, including and up to the first checkpoint
2024-06-17T17:59:38.411000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/src/dash/test/functional/test_framework/test_framework.py", line 158, in main
self.run_test()
File "./test/functional/p2p_dos_header_tree.py", line 53, in run_test
assert {
AssertionError
2024-06-17T17:59:38.913000Z TestFramework (INFO): Stopping nodes
2024-06-17T17:59:39.917000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_h5hfluy1
2024-06-17T17:59:39.917000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_h5hfluy1/test_framework.log
2024-06-17T17:59:39.917000Z TestFramework (ERROR):
2024-06-17T17:59:39.917000Z TestFramework (ERROR): Hint: Call /src/dash/test/functional/combine_logs.py '/tmp/dash_func_test_h5hfluy1' to consolidate all logs
2024-06-17T17:59:39.917000Z TestFramework (ERROR):
2024-06-17T17:59:39.917000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-06-17T17:59:39.917000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
2024-06-17T17:59:39.917000Z TestFramework (ERROR):
```
</details>
* [bitcoin#25454](https://github.com/bitcoin/bitcoin/pull/25454) has a goal similar to [dash#2032](https://github.com/dashpay/dash/pull/2032) (and its predecessor, [dash#1589](https://github.com/dashpay/dash/pull/1589)), namely, avoiding `getheaders`(`2`) duplication to the same peer. Unfortunately, Dash's mitigation seems to conflict with Bitcoin's mitigation and this results in `feature_minchainwork.py` failing (see below). This has been remedied by partially reverting [dash#2032](https://github.com/dashpay/dash/pull/2032).
<details>
<summary>Test Failure</summary>
```
dash@1bebec413839:/src/dash$ ./test/functional/feature_minchainwork.py
2024-08-01T17:29:41.116000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_co8xkx43
2024-08-01T17:29:42.145000Z TestFramework (INFO): Testing relay across node 1 (minChainWork = 101)
2024-08-01T17:29:42.145000Z TestFramework (INFO): Generating 49 blocks on node0
[...]
2024-08-01T17:29:51.707000Z TestFramework (INFO): Generating one more block
2024-08-01T17:29:51.709000Z TestFramework (INFO): Verifying nodes are all synced
2024-08-01T17:30:51.989000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/src/dash/test/functional/test_framework/test_framework.py", line 159, in main
self.run_test()
File "./test/functional/feature_minchainwork.py", line 101, in run_test
self.sync_all()
File "/src/dash/test/functional/test_framework/test_framework.py", line 811, in sync_all
self.sync_blocks(nodes)
File "/src/dash/test/functional/test_framework/test_framework.py", line 777, in sync_blocks
raise AssertionError("Block sync timed out after {}s:{}".format(
AssertionError: Block sync timed out after 60s:
'00ab061e30aebd2f97153d26cd72f921af896f05c6469ad73c7de4fc283d9590'
'00ab061e30aebd2f97153d26cd72f921af896f05c6469ad73c7de4fc283d9590'
'000008ca1832a4baf228eb1553c03d3a2c8e02399550dd6ea8d65cec3ef23d2e'
2024-08-01T17:30:52.490000Z TestFramework (INFO): Stopping nodes
2024-08-01T17:30:53.495000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_co8xkx43
2024-08-01T17:30:53.495000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_co8xkx43/test_framework.log
2024-08-01T17:30:53.495000Z TestFramework (ERROR):
2024-08-01T17:30:53.495000Z TestFramework (ERROR): Hint: Call /src/dash/test/functional/combine_logs.py '/tmp/dash_func_test_co8xkx43' to consolidate all logs
2024-08-01T17:30:53.495000Z TestFramework (ERROR):
2024-08-01T17:30:53.495000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-08-01T17:30:53.495000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
2024-08-01T17:30:53.495000Z TestFramework (ERROR):
```
</details>
* [bitcoin#25720](https://github.com/bitcoin/bitcoin/pull/25720) introduces a new test, `p2p_initial_headers_sync.py`, to validate that when a client has a stale tip, it will only engage in headers sync with one peer (emit a `getheaders2`\* message).
Unmodified, this test fails (see below) as while the backport deals with one source of `getheaders2` messages, the test setup unwittingly triggers another ([source](2379462294/src/net_processing.cpp (L5446-L5448))), specifically, allowing the `pindexBestHeader->GetBlockTime() > GetAdjustedTime() - nMaxTipAge` condition to evaluate `true`.
This is because, unlike in Bitcoin test suite's `setup_chain()` ([source](22d96d76ab/test/functional/test_framework/test_framework.py (L379-L385))), Dash sets the mocktime to match the mock chain ([source](2379462294/test/functional/test_framework/test_framework.py (L408-L416))) during setup, while the test assumes that the mock chain is stale enough to not trigger this source of `getheaders2` messages.
As the tip is barely stale, it emits the `getheaders2` message, which is detected, causing the test to fail. This has been remedied by overriding `setup_chain()` to behave more like Bitcoin's test suite.
_\* - `getheaders2` is a Dash-specific message that is courtesy of compressed headers, Bitcoin would be checking for `getheaders`_
<details>
<summary>Test Failure</summary>
```
dash@6a2649cc721f:/src/dash$ ./test/functional/p2p_initial_headers_sync.py
2024-06-17T21:24:09.921000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_3gypo3ab
2024-06-17T21:24:10.681000Z TestFramework (INFO): Adding a peer to node0
2024-06-17T21:24:11.684000Z TestFramework (INFO): Connecting two more peers to node0
2024-06-17T21:24:13.689000Z TestFramework (INFO): Verify that peer2 and peer3 don't receive a getheaders after connecting
2024-06-17T21:24:15.193000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/src/dash/test/functional/test_framework/test_framework.py", line 158, in main
self.run_test()
File "./test/functional/p2p_initial_headers_sync.py", line 60, in run_test
assert "getheaders2" not in peer2.last_message
AssertionError
2024-06-17T21:24:15.695000Z TestFramework (INFO): Stopping nodes
2024-06-17T21:24:16.698000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_3gypo3ab
2024-06-17T21:24:16.698000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_3gypo3ab/test_framework.log
2024-06-17T21:24:16.699000Z TestFramework (ERROR):
2024-06-17T21:24:16.699000Z TestFramework (ERROR): Hint: Call /src/dash/test/functional/combine_logs.py '/tmp/dash_func_test_3gypo3ab' to consolidate all logs
2024-06-17T21:24:16.699000Z TestFramework (ERROR):
2024-06-17T21:24:16.699000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-06-17T21:24:16.699000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
2024-06-17T21:24:16.699000Z TestFramework (ERROR):
```
</details>
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
Top commit has no ACKs.
Tree-SHA512: 16b7c42195e197e8b6800c3425b4ff15a124b0e3e0da5c98ca6e22486b52c4ea82f2f05b83e28e838860b1ce76daa1e435c5eae4fb7591a161f26a5fff6189cc
bitcoin#25454 introduces a 10 point penalty for remitting more than
MAX_BLOCKS_TO_ANNOUNCE unconnected block headers. Whether they are
connected or not is determined by taking the first entry and running
its hashPrevBlock through LookupBlockIndex. This new behaviour causes a
test failure in p2p_dos_header_tree.py in Dash.
Bitcoin doesn't face a test failure with this new behaviour as the first
non-fork block in its test data is the dump for block 1 (00000000b873e7
9784647a6c82962c70d228557d24a747ea4d1b8bbe878e1206) but Dash uses block
0 (00000bafbc94add76cb75e2ec92894837288a481e5c005f6563d91623bf8bc2c),
the genesis block.
By definition of a genesis block, it has a hashPrevBlock of 0, which
cannot be looked up. This trips the penalty. This doesn't cause any
problems in the field as nobody is expected to ever broadcast the
genesis block but it does cause a test failure for us.
We need to correct that by getting rid of the genesis block from the
test data.
this commit will not work with `--enable-c++20` as c++20 does away with
aggregate initialization when constructors are declared. a partial
backport of bitcoin#24169 will sort that out.
stdout:
2024-08-08T05:02:32.216000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_∋_🏃_20240808_120217/p2p_timeouts_0
2024-08-08T05:02:35.079000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
def test_function():
if check_connected:
assert self.is_connected
return test_function_in()
'''
2024-08-08T05:02:35.080000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "DASH/test/functional/test_framework/test_framework.py", line 159, in main
self.run_test()
File "DASH/test/functional/p2p_timeouts.py", line 93, in run_test
no_verack_node.wait_for_disconnect(timeout=1)
File "DASH/test/functional/test_framework/p2p.py", line 495, in wait_for_disconnect
self.wait_until(test_function, timeout=timeout, check_connected=False)
File "DASH/test/functional/test_framework/p2p.py", line 487, in wait_until
wait_until_helper(test_function, timeout=timeout, lock=p2p_lock, timeout_factor=self.timeout_factor)
File "DASH/test/functional/test_framework/util.py", line 267, in wait_until_helper
raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
AssertionError: Predicate ''''
def test_function():
if check_connected:
assert self.is_connected
return test_function_in()
''' not true after 1.0 seconds
2024-08-08T05:02:35.581000Z TestFramework (INFO): Stopping nodes
2024-08-08T05:02:36.582000Z TestFramework (WARNING): Not cleaning up dir /tmp/test_runner_∋_🏃_20240808_120217/p2p_timeouts_0
2024-08-08T05:02:36.582000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/test_runner_∋_🏃_20240808_120217/p2p_timeouts_0/test_framework.log
2024-08-08T05:02:36.582000Z TestFramework (ERROR):
2024-08-08T05:02:36.582000Z TestFramework (ERROR): Hint: Call DASH/test/functional/combine_logs.py '/tmp/test_runner_∋_🏃_20240808_120217/p2p_timeouts_0' to consolidate all logs
2024-08-08T05:02:36.582000Z TestFramework (ERROR):
2024-08-08T05:02:36.582000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-08-08T05:02:36.582000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
2024-08-08T05:02:36.582000Z TestFramework (ERROR):
stderr:
TEST | STATUS | DURATION
p2p_timeouts.py | ✓ Passed | 4 s
p2p_timeouts.py | ✖ Failed | 4 s
p2p_timeouts.py | ✖ Failed | 5 s
p2p_timeouts.py | ✖ Failed | 5 s
p2p_timeouts.py | ✖ Failed | 6 s
p2p_timeouts.py | ✖ Failed | 6 s
p2p_timeouts.py | ✖ Failed | 7 s
ALL | ✖ Failed | 37 s (accumulated)
Runtime: 7 s
