Dash already introduced support for storage of address-port pairs (referred to as "port discrimination") and allowed the usage of non-default ports in P2P with dash#2168.
Albeit this was only permitted for networks with fAllowMultiplePorts enabled (which at the time was devnet and as it stands currently, on every network except mainnet).
Keeping in line with the above policy (discussion on lifting such restrictions on mainnet is outside the scope of this PR), when backporting bitcoin#23306, changes have been made to retain the effects of discriminate_ports.
This involves appending placing a !m_discriminate_ports condition to behaviour that otherwise would be removed entirely.
Additionally, in line with upstream backports, changes have been made that render port distinguishment enabled as the new default in addrman_tests (the old default was to keep it disabled, to mirror mainnet and pre-change upstream behaviour).
To ensure distinguishment disabled works as expected, affected pre-backport tests were reintroduced with the _nondiscriminate suffix.
I would propose at some point to rename the flag to ignore_port/suppress_port (if not remove it altogether should the mainnet restriction be lifted) as discriminate (or distinguish) isn't immediately clear with if address entries will be discriminated/distinguished using ports (i.e. considered) or ports will be discriminated against (i.e. ignored).
Breaking Changes
It's unclear if these backports result in serialization issues for older versions, as Dash Core technically supported address-port pairs since 0.12 and suppressed it on mainnet by setting zero-ing out the port (source), meaning even with port discrimination disabled, the serialization format should remain the same.
Regardless, following upstream backports, a new version has been introduced (v4) that marks the AddrMan format incompatible with older versions of Dash Core.
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 (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)
Additional Information
fAllowMultiplePorts
enabled (which at the time wasdevnet
and as it stands currently, on every network exceptmainnet
).mainnet
is outside the scope of this PR), when backporting bitcoin#23306, changes have been made to retain the effects ofdiscriminate_ports
.!m_discriminate_ports
condition to behaviour that otherwise would be removed entirely.addrman_tests
(the old default was to keep it disabled, to mirrormainnet
and pre-change upstream behaviour)._nondiscriminate
suffix.I would propose at some point to rename the flag to
ignore_port
/suppress_port
(if not remove it altogether should themainnet
restriction be lifted) as discriminate (or distinguish) isn't immediately clear with if address entries will be discriminated/distinguished using ports (i.e. considered) or ports will be discriminated against (i.e. ignored).Breaking Changes
It's unclear if these backports result in serialization issues for older versions, as Dash Core technically supported address-port pairs since 0.12 and suppressed it on
mainnet
by setting zero-ing out the port (source), meaning even with port discrimination disabled, the serialization format should remain the same.Regardless, following upstream backports, a new version has been introduced (v4) that marks the AddrMan format incompatible with older versions of Dash Core.
Checklist:
Go over all the following points, and put an
x
in all the boxes that apply.