Skip to content

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 PastaPastaPasta, UdjinM6 and knst 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

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

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
@github-actions
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
@github-actions
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 PastaPastaPasta, UdjinM6 and knst 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
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