backport: merge bitcoin#22817, #23042, #22777, #23774, #25443, #26138, #26854, #27128, #27761, #27863, #28287, #30118, partial bitcoin#22778 (auxiliary backports: part 16) by kwvg · Pull Request #6276 · dashpay/dash

kwvg · 2024-09-17T10:53:25Z

Additional Information

Depends on feat(rpc): introduce and use setmnthreadactive (regtest-only) #6286
Depends on test: fixes and improvements for feature_llmq_simplepose.py #6287
Depends on test: avoid node (dis)connections in functional tests that are invalid or likely to fail #6289
When backporting bitcoin#28287, p2p_net_deadlock.py relies on the function, random_bytes(), that is introduced in bitcoin#25625. Backporting bitcoin#25625 would attract changes outside the scope of this PR.

In the interest of brevity, the changes that introduce random_bytes() have been included in bitcoin#28287 instead.

Breaking Changes

None expected.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas (note: N/A)
I have added or updated relevant unit/integration/functional/e2e tests
I have made corresponding changes to the documentation (note: N/A)
I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

knst

should you just skip 25443 because

test: soften connect_nodes assertions
....
Effectively reverts https://github.com/bitcoin/bitcoin/pull/25443

?

kwvg · 2024-09-17T11:27:21Z

There are changes introduced to connect_nodes() after bitcoin#25443 and for ease of review, bitcoin#25443 was retained to keep the diffs intact.

Making a separate "effective revert" commit after connect_nodes() changes have been made, allows for one commit to be reverted when a potential future PR resolves the underlying cause instead of modifying every commit that changes connect_nodes() to account for the absence of bitcoin#25443.

knst · 2024-09-17T11:33:11Z

we don't have signet as well:

# Setup the three signets, which are incompatible with each other

allows for one commit to be reverted when a potential future PR

Do 20445 "partial" at least... No one going to remember, that we need to revert here something, maybe add TODO here?

kwvg · 2024-09-17T11:40:53Z

we don't have signet as well

I've kept the signet comments in as it's unclear if signet will be implemented soon. If it will be soon, I think it's fine to keep them in. If it will be in the foreseeable future, then I can remove them in a separate commit so it is easier to revert when signet is implemented. If it won't be, then I can remove them from the commit and also remove the signet references I kept around in prior backports.

Do 20445 "partial" at least... No one going to remember, that we need to revert here something, maybe add TODO here?

Can do that. Will wait for GitLab CI result and will mark as partial and add TODO to connect_blocks().

github-actions · 2024-09-24T13:49:16Z

This pull request has conflicts, please rebase.

github-actions · 2024-09-25T15:11:17Z

This pull request has conflicts, please rebase.

UdjinM6 · 2024-09-25T17:33:26Z

let's extract fd82bdc and 927dd6f into their own PR maybe?

…t-only) 1e17b74 test: no longer connect nodes in parallel in `start_masternodes` (UdjinM6) be72ef5 test: use `setmnthreadactive` to get controlable `connect_nodes` behaviour (UdjinM6) e2ed82a feat(rpc): introduce `setmnthreadactive` (regtest-only) (UdjinM6) Pull request description: ## Issue being fixed or feature implemented This adds a new rpc command to enable/disable automatic masternode connections creation. We need this for #6276. 1e17b74 is extracted from ede1833 to avoid multiple jobs calling `setmnthreadactive` on the same node in parallel. ## What was done? Add `setmnthreadactive` rpc and use it ## How Has This Been Tested? run 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 ACKs for top commit: kwvg: LGTM, ACK 1e17b74 PastaPastaPasta: utACK 1e17b74 Tree-SHA512: 83c1c07d0066e26202fd21942a09e41c3560c4d32229b44390946c4acb22319b32aa61a13b9106d20fc8cc197dd2a8ab5fdfcfdeaf3da76af062fc0fd7646972

…e.py` a656d2f feat: more logging (UdjinM6) cedd3d5 refactor: make expected_connections optional (UdjinM6) fd2fbe0 fix: check mn state after each mined quorum (UdjinM6) cce87a6 fix: should have at least 2 connections when testing isolate_mn (UdjinM6) 793f4b7 fix: connect repaired mns only (UdjinM6) 8597acd fix: remember mns that don't listen and avoid them (UdjinM6) 2069625 fix: calculate expected_complaints correctly (UdjinM6) Pull request description: ## Issue being fixed or feature implemented Fix some general mistakes and also `connect_nodes` related issues discovered while debugging #6276. Add some logging to make debugging a bit easier. ~NOTE: builds on top of #6278 to avoid conflicts, will rebase~ done ## What was done? pls see individual commits ## How Has This Been Tested? run tests ## Breaking Changes n/a ## 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: kwvg: LGTM, ACK a656d2f knst: ACK a656d2f PastaPastaPasta: utACK a656d2f Tree-SHA512: 30f657218ce0338f9a5a09d9a839cca9c4605740265d2182a1e143ec6ece739fecf748f7b60ccec065c17d9f6d893c0c47893be05c44bb8d34624fb4bf3c2a58

…at are invalid or likely to fail 40f2ab9 test: don't attempt to reconnect already connected nodes (Kittywhiskers Van Gogh) 4a0fc8b test: don't attempt to (dis)connect nodes to/from themselves (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependency for #6276 ## 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 **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 40f2ab9 Tree-SHA512: aaaeedabeb6b8ef77187fc14db1888c39863daf66afda93b8c8bc1dbbdf3ff6734445fd296d5b1034da6104e2d7cfcacf26b97b7be0a697b7a99f3671b6cb9a2

github-actions · 2024-10-01T14:48:02Z

This pull request has conflicts, please rebase.

Due to stricter checks, we can no longer start masternodes in parallel, as entities used to process `to_connection` checks are reused before the previous check is completed, resulting in an exception. Since we're now validating the establishment of a two-way connection, we have to do it one at a time.

…ctions.py

…only connections includes: - 290a8da

…twork group

The fix introduced in bitcoin#27981 (8c986d6, dash#6067) is not validated to work until bitcoin#28287, an upcoming backport. As the latter has identified the former backport didn't pass validation, let's fix it so that the latter tests succeed when they're backported in an upcoming commit.

… deadlock situation `random_bytes()` is introduced in bitcoin#25625 but the function def alone doesn't warrant a full backport, so we'll only implement the section relevant to this PR.

UdjinM6

p2p_node_network_limited.py fails. Backporting 30118 (2c825b4) should help.

UdjinM6

utACK e458adb

PastaPastaPasta

utACK e458adb

…ollow-up dash#6276) 9b0c506 test: call `self.generate()` in `p2p_net_deadlock.py` (Kittywhiskers Van Gogh) Pull request description: ## Additional Information `develop` is currently borked ([build](https://gitlab.com/dashpay/dash/-/jobs/7999893074#L266)) because [dash#6288](#6288) changed the expected syntax for `generate*` calls and [dash#6276](#6276) introduced a new test (`p2p_net_deadlock.py`) that was based on a version of `develop` that used the older syntax. No conflicts were reported because it was a new file introduced in the latter PR that the former PR did not have knowledge of. ## Breaking changes None expected. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 9b0c506 PastaPastaPasta: utACK 9b0c506 Tree-SHA512: 3b49f0ff4fc5beea8bd48019c581577ebb47cf50c5526e60367740e5deea7fc92451a480abd49b2b3b2a7be7fb9ca2a4aca99c565ef7aaee1ba8cce43e0be776

, bitcoin#28849, bitcoin#28805, bitcoin#28951, bitcoin#29058, bitcoin#29239, partial bitcoin#28331, bitcoin#28452 (BIP324 backports: part 2) 5dd60c4 merge bitcoin#29239: Make v2transport default for addnode RPC when enabled (Kittywhiskers Van Gogh) b2ac426 merge bitcoin#29058: use v2transport for manual/addrfetch connections, add to -netinfo (Kittywhiskers Van Gogh) 92e862a merge bitcoin#28951: damage ciphertext/aad in full byte range (Kittywhiskers Van Gogh) 4e96e26 merge bitcoin#28805: Make existing functional tests compatible with --v2transport (Kittywhiskers Van Gogh) 9371e2e merge bitcoin#28849: fix node index bug when comparing peerinfo (Kittywhiskers Van Gogh) 400c9dd merge bitcoin#28634: add check for missing garbage terminator detection (Kittywhiskers Van Gogh) 65eb194 merge bitcoin#28588: add checks for v1 prefix matching / wrong network magic detection (Kittywhiskers Van Gogh) 6074974 merge bitcoin#28577: raise V1_PREFIX_LEN from 12 to 16 (Kittywhiskers Van Gogh) ff92d1a partial bitcoin#28331: BIP324 integration (Kittywhiskers Van Gogh) f9f8805 fix: drop `CConnman::mapNodesWithDataToSend`, use transport data (UdjinM6) d39d8a4 partial bitcoin#28452: Do not use std::vector = {} to release memory (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependent on #6280 * Dependent on #6276 * Dependency for #6329 * [bitcoin#28452](bitcoin#28452) was backported as the behavior it introduces is required for backports like [bitcoin#28331](bitcoin#28331). As it pre-dates earlier BIP324 backports, the `ClearShrink` usage from those backports have also been incorporated into this backport. * When backporting [bitcoin#28331](bitcoin#28331), in TestFramework's `add_nodes()`, `extra_args[i]` is not converted to a `list` like it is done upstream and avoiding duplication of `-v2transport=1` is instead done by an additional check. This is done to prevent test failures in `feature_mnehf.py`. * The local variable `args` is removed in [bitcoin#28805](bitcoin#28805) as it does nothing (the argument appending logic is removed as part of the backport) and upstream removes it anyways. Special thanks to UdjinM6 for significant contributions to this and dependent PRs! 🎉 ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [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)_ ACKs for top commit: UdjinM6: light ACK 5dd60c4 PastaPastaPasta: utACK 5dd60c4 Tree-SHA512: 7f3d0e54e1c96fc99b2d6b1e64485110aa73aeec0e4e4245ed4e6dc0529a7608e9c39103af636d5945d289775c40d3d15d7d4a75bea82462194dbf355fca48dc

knst reviewed Sep 17, 2024

View reviewed changes

kwvg force-pushed the auxports16 branch from b302fb2 to 23acac9 Compare September 22, 2024 18:28

kwvg added this to the 21.2 milestone Sep 22, 2024

kwvg requested a review from UdjinM6 September 22, 2024 18:40

github-actions bot added the needs rebase label Sep 24, 2024

kwvg force-pushed the auxports16 branch from 23acac9 to 6f7d5bc Compare September 24, 2024 13:56

github-actions bot removed the needs rebase label Sep 24, 2024

This was referenced Sep 25, 2024

feat(rpc): introduce and use setmnthreadactive (regtest-only) #6286

Merged

test: fixes and improvements for feature_llmq_simplepose.py #6287

Merged

github-actions bot added the needs rebase label Sep 25, 2024

thephez added the RPC Some notable changes to RPC params/behaviour/descriptions label Sep 25, 2024

kwvg mentioned this pull request Sep 26, 2024

test: avoid node (dis)connections in functional tests that are invalid or likely to fail #6289

Merged

5 tasks

kwvg changed the title ~~backport: merge bitcoin#22741, #22817, #25443, #26138, #26854, #27128, #28287, partial bitcoin#22550 (auxiliary backports: part 16)~~ backport: merge bitcoin#22817, #25443, #26138, #26854, #27128, #28287 (auxiliary backports: part 16) Sep 26, 2024

kwvg force-pushed the auxports16 branch from 6f7d5bc to e64d254 Compare September 26, 2024 20:49

github-actions bot removed the needs rebase label Sep 26, 2024

kwvg changed the title ~~backport: merge bitcoin#22817, #25443, #26138, #26854, #27128, #28287 (auxiliary backports: part 16)~~ backport: merge bitcoin#22817, #23042, #22777, #25443, #26138, #26854, #27128, #27761, #27863, #28287, partial bitcoin#22778 (auxiliary backports: part 16) Sep 30, 2024

kwvg force-pushed the auxports16 branch from e64d254 to d8ce4b7 Compare September 30, 2024 12:48

github-actions bot added the needs rebase label Oct 1, 2024

github-actions bot removed the needs rebase label Oct 1, 2024

kwvg added 11 commits October 2, 2024 08:31

merge bitcoin#22777: don't request tx relay on feeler connections

0354417

merge bitcoin#23774: Add missing assert_equal import to p2p_add_conne…

85c4aef

…ctions.py

partial bitcoin#22778: Reduce resource usage for inbound block-relay-…

60b5392

…only connections includes: - 290a8da

merge bitcoin#25443: Fail if connect_nodes fails

d6ce037

merge bitcoin#26138: Avoid race in disconnect_nodes helper

892e329

merge bitcoin#26854: Fix intermittent timeout in p2p_permissions.py

d4b0fae

merge bitcoin#27128: fix intermittent issue in p2p_disconnect_ban

2854a6a

merge bitcoin#27761: Log addresses of stalling peers

1adb9a2

merge bitcoin#27863: do not break when addr is not from a distinct ne…

19e7bf6

…twork group

merge bitcoin#28287: add sendmsgtopeer rpc and a test for net-level…

ac94de2

… deadlock situation `random_bytes()` is introduced in bitcoin#25625 but the function def alone doesn't warrant a full backport, so we'll only implement the section relevant to this PR.

kwvg force-pushed the auxports16 branch from d632295 to ac94de2 Compare October 2, 2024 08:34

kwvg marked this pull request as ready for review October 2, 2024 11:23

kwvg requested review from PastaPastaPasta and knst October 2, 2024 11:23

UdjinM6 requested changes Oct 3, 2024

View reviewed changes

merge bitcoin#30118: improve robustness of connect_nodes()

e458adb

kwvg requested a review from UdjinM6 October 3, 2024 21:41

UdjinM6 approved these changes Oct 4, 2024

View reviewed changes

PastaPastaPasta approved these changes Oct 4, 2024

View reviewed changes

PastaPastaPasta merged commit 8cef87d into dashpay:develop Oct 4, 2024

kwvg mentioned this pull request Oct 5, 2024

test: call self.generate() in p2p_net_deadlock.py (follow-up dash#6276) #6315

Merged

5 tasks

kwvg mentioned this pull request Oct 9, 2024

backport: merge bitcoin#28577, #28588, #28634, #28849, #28805, #28951, #29058, #29239, partial bitcoin#28331, #28452 (BIP324 backports: part 2) #6324

Merged

5 tasks

UdjinM6 modified the milestones: 21.2, 22 Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

backport: merge bitcoin#22817, #23042, #22777, #23774, #25443, #26138, #26854, #27128, #27761, #27863, #28287, #30118, partial bitcoin#22778 (auxiliary backports: part 16)#6276

backport: merge bitcoin#22817, #23042, #22777, #23774, #25443, #26138, #26854, #27128, #27761, #27863, #28287, #30118, partial bitcoin#22778 (auxiliary backports: part 16)#6276
PastaPastaPasta merged 14 commits intodashpay:developfrom
kwvg:auxports16

kwvg commented Sep 17, 2024 •

edited

Loading

Uh oh!

knst left a comment

Uh oh!

kwvg commented Sep 17, 2024

Uh oh!

knst commented Sep 17, 2024 •

edited

Loading

Uh oh!

kwvg commented Sep 17, 2024

Uh oh!

github-actions bot commented Sep 24, 2024

Uh oh!

github-actions bot commented Sep 25, 2024

Uh oh!

UdjinM6 commented Sep 25, 2024

Uh oh!

github-actions bot commented Oct 1, 2024

Uh oh!

UdjinM6 left a comment

Uh oh!

UdjinM6 left a comment

Uh oh!

PastaPastaPasta left a comment •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

kwvg commented Sep 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Additional Information

Breaking Changes

Checklist:

Uh oh!

knst left a comment

Choose a reason for hiding this comment

Uh oh!

kwvg commented Sep 17, 2024

Uh oh!

knst commented Sep 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kwvg commented Sep 17, 2024

Uh oh!

github-actions bot commented Sep 24, 2024

Uh oh!

github-actions bot commented Sep 25, 2024

Uh oh!

UdjinM6 commented Sep 25, 2024

Uh oh!

github-actions bot commented Oct 1, 2024

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kwvg commented Sep 17, 2024 •

edited

Loading

knst commented Sep 17, 2024 •

edited

Loading

PastaPastaPasta left a comment •

edited

Loading