Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport: merge bitcoin#23077, #22834, #24165, #24555, #24663, #24205, #24687, #25173, #24991, partial bitcoin#24468 (cjdns support) #6031

Merged
merged 10 commits into from
Jun 10, 2024

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented May 21, 2024

Additional Information

Breaking changes

No expected protocol or consensus changes.

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
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@kwvg kwvg added this to the 21 milestone May 21, 2024
@kwvg kwvg marked this pull request as ready for review May 21, 2024 10:01
@kwvg kwvg requested review from knst, UdjinM6 and PastaPastaPasta May 21, 2024 10:01
Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but kvb in strings should be dashified IMO

test/functional/interface_bitcoin_cli.py Outdated Show resolved Hide resolved
src/bitcoin-cli.cpp Outdated Show resolved Hide resolved
src/bitcoin-cli.cpp Outdated Show resolved Hide resolved
doc/cjdns.md Show resolved Hide resolved
knst
knst previously approved these changes May 21, 2024
Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kwvg kwvg changed the title backport: merge bitcoin#21261, #21832, #22648, #22959, #23077, #22834, #24165, #24555, #24663, partial bitcoin#22589, #22229, #24468 (cjdns support) backport: merge bitcoin#21261, #21832, #22648, #22959, #23077, #22834, #24165, #24555, #24663, partial bitcoin#22589, #22229, #24468 (cjdns support: part 1) May 21, 2024
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM; partial CI failure not great in p2p_node_network_limited.py tsan. Build restarted

doc/tor.md Show resolved Hide resolved
UdjinM6
UdjinM6 previously approved these changes May 21, 2024
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@kwvg kwvg dismissed stale reviews from UdjinM6 and knst via b0ae0bb May 27, 2024 11:21
@kwvg kwvg changed the title backport: merge bitcoin#21261, #21832, #22648, #22959, #23077, #22834, #24165, #24555, #24663, partial bitcoin#22589, #22229, #24468 (cjdns support: part 1) backport: merge bitcoin#23077, #22834, #24165, #24555, #24663, #24205, #24687, #25173, #24991, partial bitcoin#24468 (cjdns support) May 27, 2024
@kwvg kwvg marked this pull request as draft May 27, 2024 11:22
Copy link

This pull request has conflicts, please rebase.

PastaPastaPasta added a commit that referenced this pull request May 28, 2024
, bitcoin#22547, bitcoin#22544, bitcoin#22959, bitcoin#23324, partial bitcoin#20764 (cli backports: part 2)

bde72a4 merge bitcoin#23324: print peer counts for all reachable networks in -netinfo (Kittywhiskers Van Gogh)
4b24544 merge bitcoin#22959: Display all proxies in -getinfo (Kittywhiskers Van Gogh)
30b0fcf merge bitcoin#22544: drop torv2; torv3 becomes onion per GetNetworkName() (Kittywhiskers Van Gogh)
b6ca36e merge bitcoin#22547: Add progress bar for -getinfo (Kittywhiskers Van Gogh)
1f89bfd merge bitcoin#21832: Implement human readable -getinfo (Kittywhiskers Van Gogh)
2200b78 merge bitcoin#20877: user help and argument parsing improvements (Kittywhiskers Van Gogh)
bd934c7 partial bitcoin#20764: cli -netinfo peer connections dashboard updates (Kittywhiskers Van Gogh)
b2d8656 merge bitcoin#21261: update inbound eviction protection for multiple networks, add I2P peers (Kittywhiskers Van Gogh)
0b16b50 cli: fix loop counter comparison in `ProcessReply` (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependency for #6035

  * Dependency for #6031

  * In [dash#5904](#5904) ([bitcoin#21595](bitcoin#21595)), one of the loops in `ProcessReply` is supposed to iterate `rows.size()` times (which at the time was hardcoded to `3`), the backport erroneously set the value to `m_networks.size()` (which also evaluated to `3`) as part of increasing `m_networks.size()` usage.

    As this pull request includes [bitcoin#23324](bitcoin#23324), which changes it over to  `rows.size()`, the above has been corrected in a separate commit for documentation purposes.

  * `-addrinfo` output

    ![dash-cli addrinfo output](https://github.com/dashpay/dash/assets/63189531/24db46be-729e-4fa8-a268-87f2497cff9a)

  * `-getinfo` output (diamonds are due to rendering limitations of my terminal and are not indicative of the symbols used)

    ![dash-cli getinfo output](https://github.com/dashpay/dash/assets/63189531/626fe67f-f505-4a04-931a-76e75146e5a0)

  * `-netinfo` output

    ![dash-cli netinfo output](https://github.com/dashpay/dash/assets/63189531/afbff3d0-7127-44e2-bfe7-81b08c0e214e)

  ## Breaking Changes

  * CLI `-addrinfo` now returns a single field for the number of `onion` addresses known to the node instead of separate `torv2` and `torv3` fields, as support for TorV2 addresses was removed from Dash Core in 18.0.

  * `-getinfo` has been updated to return data in a user-friendly format that also reduces vertical space.

  ## 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

ACKs for top commit:
  PastaPastaPasta:
    utACK bde72a4

Tree-SHA512: 921cb45b7e243a321a32c835eb23d5ba8df610ff234a548a9051436a2c21845ce70097fb9a9bb812b77b04373f9f0a9f90264168d97b08da1890be06bfd9f99c
Copy link

This pull request has conflicts, please rebase.

@kwvg kwvg force-pushed the cjdns branch 2 times, most recently from 6af25da to 32f8fda Compare May 29, 2024 17:14
@kwvg
Copy link
Collaborator Author

kwvg commented May 31, 2024

CJDNS has been confirmed to work on Dash Core 32f8fda using CJDNS v22 (f344983) on two nodes following instructions from Bitcoin's cjdns.md.

  • A VPS running Ubuntu 20.04 LTS using IPv6 without onlynet restrictions
  • A Docker container running Ubuntu 20.04 LTS using IPv4 with onlynet=cjdns

Marking PR as ready for review.

@kwvg kwvg marked this pull request as ready for review May 31, 2024 20:13
@kwvg kwvg requested review from knst, UdjinM6 and PastaPastaPasta May 31, 2024 20:14
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 32f8fda

@PastaPastaPasta PastaPastaPasta merged commit 825bba1 into dashpay:develop Jun 10, 2024
6 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants