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.

Sign up for GitHub

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

Jump to bottom

backport: merge bitcoin#22648, #22497, #25355, #26065, #26837, #25993, #26838, #27937, partial bitcoin#22589, #22229 (i2p support: part 2) #6035

Merged
merged 10 commits into from
May 29, 2024
Merged

backport: merge bitcoin#22648, #22497, #25355, #26065, #26837, #25993, #26838, #27937, partial bitcoin#22589, #22229 (i2p support: part 2) #6035

merged 10 commits into from
May 29, 2024

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented May 27, 2024 •
edited
Loading

Additional Information

Breaking Changes

  • With I2P connections, a new, transient address is used for each outbound connection if -i2pacceptincoming=0.

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 27, 2024
This was referenced May 27, 2024
Merged
Merged
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please rebase.

PastaPastaPasta added a commit that referenced this pull request May 28, 2024
@PastaPastaPasta
Merge #6034: backport: merge bitcoin#21261, bitcoin#20877, bitcoin#21832
3b3b1b8 
, 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
@kwvg kwvg marked this pull request as ready for review May 29, 2024 06:30
@kwvg kwvg requested review from knst, UdjinM6 and PastaPastaPasta May 29, 2024 06:30
knst
knst approved these changes May 29, 2024
View reviewed changes
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.

utACK eea4810

UdjinM6
UdjinM6 approved these changes May 29, 2024
View reviewed changes
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

src/net.cpp
: nTimeConnected(GetTimeSeconds()),
addr(addrIn),
addrBind(addrBindIn),
CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion, std::unique_ptr<i2p::sam::Session>&& i2p_sam_session)
Copy link

Choose a reason for hiding this comment

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

25355: nit: this part was refactored to put one param at a line in the original PR

src/net.h
@@ -622,7 +622,7 @@ class CNode

bool IsBlockRelayOnly() const;

CNode(NodeId id, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress &addrBindIn, const std::string &addrNameIn, ConnectionType conn_type_in, bool inbound_onion);
CNode(NodeId id, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress &addrBindIn, const std::string &addrNameIn, ConnectionType conn_type_in, bool inbound_onion, std::unique_ptr<i2p::sam::Session>&& i2p_sam_session = nullptr);
Copy link

Choose a reason for hiding this comment

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

25355: nit: same

PastaPastaPasta
PastaPastaPasta approved these changes May 29, 2024
View reviewed changes
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 utACK eea4810

doc/i2p.md
and there may be fewer I2P peers than Tor or IP ones. Therefore, using
`onlynet=i2p` alone (without other `onlynet=`) may make a node more susceptible
to [Sybil attacks](https://en.dash.it/wiki/Weaknesses#Sybil_attack). Use
`dash-cli -addrinfo` to see the number of I2P addresses known to your node.

## I2P related information in Dash Core
Copy link
Member

Choose a reason for hiding this comment

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

22589: I would consider this complete; we don't have a need for the other commit.

kwvg added 10 commits May 29, 2024 11:48
@kwvg @PastaPastaPasta
merge bitcoin#22589: update I2P hardcoded seeds and docs for 22.0
68ea6cc 
Arguably partial; however the commit it excludes is not relevant

includes:
- d2dffd5
@kwvg @PastaPastaPasta
partial bitcoin#22229: consolidate to f-strings (part 1)
f04ce8b 
includes:
- 86d9582
@kwvg @PastaPastaPasta
merge bitcoin#22648: improve i2p/tor docs and i2p reachable unit tests
63d5853
@kwvg @PastaPastaPasta
merge bitcoin#22497: remove ResetI2PPorts()
4977073
@kwvg @PastaPastaPasta
merge bitcoin#25355: add support for transient addresses for outbound…
9bf3829 
… connections
@kwvg @PastaPastaPasta
merge bitcoin#26065: use the same destination type for transient and …
9f80734 
…persistent addresses
@kwvg @PastaPastaPasta
merge bitcoin#26837: I2P network optimizations
859f59b
@kwvg @PastaPastaPasta
merge bitcoin#25993: Add I2P guidance related to bandwidth and i2pd s…
5058519 
…oftware version
@kwvg @PastaPastaPasta
merge bitcoin#26838: I2P documentation updates
1dc50d0
@kwvg @PastaPastaPasta
merge bitcoin#27937: i2p documentation updates
0a23820
PastaPastaPasta
PastaPastaPasta approved these changes May 29, 2024
View reviewed changes
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.

re-utACK 0a23820; only changed commit title / text

@PastaPastaPasta PastaPastaPasta merged commit 76b8ffd into dashpay:develop May 29, 2024
4 of 7 checks passed
PastaPastaPasta added a commit that referenced this pull request Jun 10, 2024
@PastaPastaPasta
Merge #6031: backport: merge bitcoin#23077, bitcoin#22834, bitcoin#24165
825bba1 
, bitcoin#24555, bitcoin#24663, bitcoin#24205, bitcoin#24687, bitcoin#25173, bitcoin#24991, partial bitcoin#24468 (cjdns support)

32f8fda merge bitcoin#24991: allow startup with -onlynet=onion -listenonion=1 (Kittywhiskers Van Gogh)
e67ed92 merge bitcoin#25173: add coverage for unknown network in -onlynet (Kittywhiskers Van Gogh)
77efd36 merge bitcoin#24687: Check an invalid -i2psam will raise an init error (Kittywhiskers Van Gogh)
fb1416f merge bitcoin#24205: improve network reachability test coverage and safety (Kittywhiskers Van Gogh)
7cb7479 merge bitcoin#24663: add links to doc/cjdns.md (Kittywhiskers Van Gogh)
c736ebf merge bitcoin#24555: create initial doc/cjdns.md for CJDNS how-to documentation (Kittywhiskers Van Gogh)
554bd24 partial bitcoin#24468: improve -onlynet help and related tor/i2p documentation (Kittywhiskers Van Gogh)
5436b6a merge bitcoin#24165: extend inbound eviction protection by network to CJDNS peers (Kittywhiskers Van Gogh)
d52724d merge bitcoin#22834: respect -onlynet= when making outbound connections (Kittywhiskers Van Gogh)
f9d1a9a merge bitcoin#23077: Full CJDNS support (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Depends on #6034

  * Depends on #6035

  * If `-proxy=` is given together with `-noonion` then the provided proxy will not be set as a proxy for reaching the Tor network. So it will not be possible to open manual connections to the Tor network for example with the `addnode` RPC. To mimic the old behavior use `-proxy=` together with `-onlynet=` listing all relevant networks except `onion`.

  * [bitcoin#24165](bitcoin#24165) has been backported _before_ [bitcoin#23758](bitcoin#23758) and to account for this, minor changes were made in `src/test/net_peer_eviction_tests.cpp` (using `nTimeConnected` instead of `m_connected`). When backporting  [bitcoin#23758](bitcoin#23758), these changes will have to be reversed as they won't be covered by the cherry-pick diff.

  * CJDNS support has been labelled as being introduced in Dash Core 21.0, in line with the milestone designation of the PR. Should `develop` be used for a new minor/patch release, `doc/cjdns.md` will have to be modified to reflect the correct version number.

  ## Breaking changes

  No expected protocol or consensus 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 **(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:
  PastaPastaPasta:
    utACK 32f8fda

Tree-SHA512: e23b22ca5edbe4c4abeab0bc07780303e68e7c4cc46b7697300b0837c5acd3a98649b6b03bd07a23c827bd85f64210173027b0b0eea31872c031fa4ed04eeb0c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knst knst knst approved these changes

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
21
Development

Successfully merging this pull request may close these issues.
4 participants
@kwvg @knst @UdjinM6 @PastaPastaPasta