Repository navigation
refactor(proto): remove unused proto discover definition - #7014
Conversation
waynercheung
left a comment
There was a problem hiding this comment.
[SHOULD] Please record the pre-change serialization comparison required by #7013, including the commands, fixtures and results, or add fixed regression fixtures for Endpoint, HelloMessage and BackupMessage. The current PR does not document this comparison.
[DISCUSS] Please reconcile #7013's conditional deprecation requirement with the clarification in #6921 that these are unsupported legacy types. Record the agreed classification and describe the generated-type removal and expected descriptor change in this PR's compatibility notes.
The four removed legacy message types have been unused by java-tron for years and are not supported external APIs, as clarified in #6921. Their removal therefore does not require a deprecation cycle. The removal of the generated types and the corresponding descriptor changes are intentional and can be documented in the PR’s compatibility notes. |
|
Thanks. On the serialization comparison: agreed, no additional permanent regression tests are needed for this cleanup. For the record, I ran the comparison against the merge base 6d5adc4: [SHOULD] On the classification: agreed. Please add the compatibility note to the PR description before merge, and update the last paragraph of #7013 to match. Suggested text:
[NIT] Please correct "defintion" to "definition" in the PR title. |
|
@waynercheung I have update the PR description and PR title, thanks for your suggestion. |
Thanks, I have update the of last paragraph of issue #7013. |
* release_v4.8.3: feat(framework): remove --keystore-factory support from FullNode (tronprotocol#6978) refactor(proto): remove unused proto discover definition (tronprotocol#7014) fix(config): close resource streams (tronprotocol#7002) refactor(crypto): remove unused SM2 and SM3 (tronprotocol#6999) feat(plugins): backfill section bloom (tronprotocol#6973) refactor(api): remove dead WalletExtension gRPC service and config (tronprotocol#6975) refactor(framework): decouple Manager from TronJsonRpcImpl (tronprotocol#6990) refactor(api): replace response copy wrappers with Jetty content count (tronprotocol#6982) update a new version. version name:GreatVoyage-v4.8.2.2-1-gf3e81404fe,version code:18830 (tronprotocol#7008) feat: improve node stability and execution efficiency (tronprotocol#7007)
What does this PR do?
close #7013.
PingMessage,PongMessage,FindNeighbours, andNeighboursdefinitions fromprotocol/src/main/protos/core/Discover.proto.Endpoint, referenced byHelloMessage.from, andBackupMessage, used by backup keepalive messages, with their existing fields and options.p2p/src/main/proto/Discover.protoand align the UDP discovery examples and field descriptions with the implementation, including IPv6 addresses, network identifiers, and response timestamps.Compatibility:
The retained messages preserve their existing wire encoding. No runtime networking or consensus changes are introduced. The generated Java types org.tron.protos.Discover.PingMessage, PongMessage, FindNeighbours and Neighbours are removed, and the core/Discover.proto descriptor changes accordingly (BackupMessage moves from message index 5 to 1). Per #6921 these are unsupported legacy types, so no deprecation cycle is required. The active UDP discovery definitions are in p2p/src/main/proto/Discover.proto (org.tron.p2p.protos.Discover), which uses a different Java package and protobuf namespace, so they are not drop-in replacements.
Why are these changes required?
These definitions are redundant leftovers from the libp2p split and are no longer referenced by java-tron's runtime code. UDP discovery uses the separate p2p definitions, brought into this repository in #6992. Removing the legacy definitions reduces maintenance overhead and avoids confusion about the active discovery protocol.
Part of the cleanup tracked in #6921.