Repository navigation
[Feature] Remove unused discovery message definitions #7013
Description
Background
protocol/src/main/protos/core/Discover.proto still contains four legacy message definitions that are no longer referenced by java-tron’s runtime code: PingMessage, PongMessage, FindNeighbours, and Neighbours. These proto definitions are redundant leftovers from the libp2p split.
UDP discovery now uses the separate proto definitions in p2p/src/main/proto/Discover.proto (introduced in PR #6992).
Rationale
Remove unused definitions to reduce maintenance overhead and avoid confusion with the UDP discovery.
Specification
- Remove the four legacy messages from
protocol/src/main/protos/core/Discover.proto. - Preserve
Endpoint, referenced byHelloMessage.from, andBackupMessage, used by backup keepalive messages, with their existing fields and options. - Update the Chinese and English protocol documents to point to the real p2p discovery definitions.
Test Specification
- Regenerate protobuf sources from a clean state and compile the project.
- Run the related handshake and backup keepalive regression tests.
- Compare serialization against pre-change fixtures for
Endpoint,HelloMessage, andBackupMessage.
Scope of Impact
The cleanup is intended to preserve node network behavior and the wire encoding of retained messages. It removes generated types and changes the file descriptor, so downstream code referencing the removed types must migrate or retain a compatible protocol version. Projects using their own copies are unaffected unless they adopt the removal.
As clarified in #6921, these four generated message types are unsupported legacy types and do not require a deprecation cycle. Their removal intentionally changes the generated API and file descriptor, while preserving node interoperability and the wire encoding of the retained messages.
Activity
xxo1shine commentedon Oct 9, 2026
@317787106 The cleanup makes sense if these message definitions are no longer used by the runtime. However, before removing them, we should verify whether the generated types are part of the public API and whether downstream consumers may depend on them. If they are covered by the compatibility policy in #6921, deprecation before removal would be safer. We should also ensure the retained messages' wire encoding remains unchanged.
waynercheung commentedon Oct 9, 2026
I support this cleanup. The runtime references I checked on
release_v4.8.3support the proposed scope: UDP discovery usesorg.tron.p2p.protos.Discover, whileprotocol.Endpointandprotocol.BackupMessageremain required by the framework handshake and backup paths. The runtime discovery implementation migrated to libp2p in v4.7.0 (27cc33babf); #6992 subsequently brought the existing p2p definitions into this repository.Building on @xxo1shine's compatibility point, the author already clarified in #6921 (#6921 (comment)) that these legacy generated types are not intended to be supported external APIs. Could we reconcile that position with the conditional deprecation language here, and record the classification explicitly? Direct removal seems reasonable if that boundary is confirmed. The release notes should still list the four removed generated types, identify the corresponding active UDP discovery definitions, and mention the descriptor change. Any migration guidance should note that the Java and protobuf namespaces differ, so these are not drop-in type replacements.
For regression coverage, fixed pre-change wire fixtures for
Endpoint,HelloMessage, andBackupMessage, including IPv6 and default-value cases, would provide a lightweight guard. A round trip using only the post-change implementation would not establish compatibility with the previous encoding. Since the retained definitions are untouched, this is a regression safeguard rather than a known encoding defect. #7014 currently adds no tests; could the PR include these fixtures, or document how the pre-change serialization comparison required by this issue was performed? The existing p2p discovery codec tests should also remain green.The file descriptor change should be treated as expected: removing the four declarations changes the message list and moves
BackupMessage's descriptor index from 5 to 1. We should verify that the retained message definitions, their fully qualified protobuf names, and their wire bytes remain unchanged.Please keep this scoped to the four legacy proto definitions and the corresponding documentation. Retiring the framework's legacy TCP Ping/Pong handling involves a separate peer-compatibility question. The documentation should also distinguish UDP discovery from the current p2p
Connect.KeepAliveMessageimplementation.The updated field descriptions in #7014 match the implementation:
version/echocarry the network ID, andNeighbours.timestampechoes the request timestamp. One documentation detail needs adjustment:../../../../p2p/src/main/proto/Discover.protoresolves within java-tron but breaks when these documents are distributed throughtronprotocol/protocolor copied into wallet-cli. Could we use an absolute GitHub URL pointing to the appropriate java-tron revision?lxcmyf commentedon Oct 14, 2026
#7014 removes the duplicate runtime definitions, but the two protocol documents still embed manually copied versions of the active p2p schemas; they only add a textual source path. This preserves a similar drift risk: future changes to
p2p/src/main/proto/Discover.protowill not update these copies automatically, and the active proto already also containsEndPointsandDnsRoot, which are absent from this section.Could we replace the copied schemas with a revision-pinned link to the authoritative proto, or add a generation/consistency check? Otherwise readers may treat an incomplete or stale Markdown copy as the protocol contract.