Skip to content

multi: Misc cleanup of recently added addrv2 code.#3613

Merged
davecgh merged 8 commits intodecred:masterfrom
davecgh:multi_misc_addrv2_cleanup
Feb 14, 2026
Merged

multi: Misc cleanup of recently added addrv2 code.#3613
davecgh merged 8 commits intodecred:masterfrom
davecgh:multi_misc_addrv2_cleanup

Conversation

@davecgh
Copy link
Member

@davecgh davecgh commented Feb 9, 2026

Requires #2627.

This builds on the work in #2627 to finalize a few remaining review items that we decided to do separately.

The following is an overview of the changes:

  • Consolidate address version selection logic in the server
  • Remove old style methods from new addrv2 message
  • Rename IP -> EncodedAddr to more accurately reflect the field's purpose
  • Misc cleanup to avoid some additional allocs, use standard test formatting, and remove some unnecessary nil checks
  • Keep TorV3 support as part of the new addrv2 protocol version (12) instead doing two simultaneous version bumps
  • Reduce allocations in new addv2 writes
  • Rename all "TOR" references to the more canonical "Tor"

See each commit for a more detailed description.

@davecgh davecgh added this to the 2.2.0 milestone Feb 9, 2026
@davecgh davecgh force-pushed the multi_misc_addrv2_cleanup branch 2 times, most recently from dabbfe9 to c5955cd Compare February 9, 2026 20:39
@davecgh davecgh force-pushed the multi_misc_addrv2_cleanup branch from c5955cd to ca39014 Compare February 10, 2026 02:19
@davecgh davecgh force-pushed the multi_misc_addrv2_cleanup branch from ca39014 to f8b8383 Compare February 10, 2026 04:11
@davecgh davecgh force-pushed the multi_misc_addrv2_cleanup branch from f8b8383 to eea28cd Compare February 11, 2026 03:09
@davecgh
Copy link
Member Author

davecgh commented Feb 11, 2026

Updated to add a commit to rename all "TOR" references to the more canonical "Tor". All references to Tor on the main project site, and basically everywhere else, refer to the network as "Tor", not "TOR".

Copy link
Contributor

@matthawkins90 matthawkins90 left a comment

Choose a reason for hiding this comment

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

Very excited to finally have sef's work and TorV3!

Copy link
Member

@alexlyp alexlyp left a comment

Choose a reason for hiding this comment

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

ACK

This consolidates the logic for choosing which version address message
to send to the remote peers based on the protocol version to a single
function and updates the callers to make use of it.
The vast majority of the code in the wire package was written nearly a
decade and a half ago when Go was still quite new and many of the
patterns are less than ideal as a result.

For example, methods that add items one at a time along with a
conditional check are inefficient and realistically unnecessary because
the caller is already expected to make use of the exported constants to
avoid errors to begin with and the messages will fail to encode and
decode if they exceed the max allowed anyway.

Not only that, in order to allow those types of methods and avoid a
bunch of additional allocations due to multiple appends, the
constructors have to create a slice with a large capacity with no
information as to what a good guess would be which is rather wasteful.

So, in practice, callers are much better off just creating the list with
the exact size they know they need and set it at message creation time.

Unfortunately, since the wire package is used as the base of virtually
every single other package, changing the API in a breaking way that
would cause a major module version bump would be a massive undertaking
and thus care has been taken to avoid requiring one.

However, the new MsgAddrV2 type was just added and a new version has not
been tagged yet, so it is safe to modify the API for the message in a
breaking way.

With the aforementioned in mind, this removes the AddAddress,
AddAddresses, and ClearAddresses methods from the newly introduced
MsgAddrV2 type.
This renames the IP field of the new wire.NetAddressV2 struct to
EncodedAddr to more accurately reflect its use.

This is a breaking API change and thus ordinarily would not be allowed,
but since the struct was recently introduced and a new version of the
wire modules has not been tagged yet, it is safe to modify the API in a
breaking way.
This contains some basic housekeeping and does not make any functional
changes:

- Use nil instead of empty instances to avoid some allocs
- Use standard formatting in some of the tests
- Remove unnecessary checks for nil slices on len checks since taking
  the length of a nil slice is already 0
- Correct a wire comment regarding network addresses
This removes the additional TORv3Version protocol version bump in favor
of keeping the TorV3 additions as part of the AddrV2Version protool
bump.

This is a breaking API change and thus ordinarily would not be allowed,
but since the struct was recently introduced and a new version of the
wire modules has not been tagged yet, it is safe to modify the API in a
breaking way.
This converts the encoding logic for the new MsgAddrV2 to avoid
allocations by using pointers to the data to be written and also
introduces fast paths in the writeElement function for the new types.
This modifies the logic for serializing version 2 network addresses to
avoid additional copies into local arrays for the various address types
by directly writing the encoded address after the switch that verifies
the encoded address is the right size for the type.
This renames all mentions of TORv3 to the more canonical TorV3.  All
references to Tor on the main project site, and basically everywhere
else, refer to the network as "Tor", not "TOR".
@davecgh davecgh force-pushed the multi_misc_addrv2_cleanup branch from eea28cd to fd39805 Compare February 14, 2026 07:31
@davecgh davecgh merged commit fd39805 into decred:master Feb 14, 2026
34 checks passed
@davecgh davecgh deleted the multi_misc_addrv2_cleanup branch February 14, 2026 07:34
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