Repository navigation
Tracking: code refactor and cleanup #6921
Description
Summary
Over years of development, java-tron has accumulated a considerable amount of legacy code, including dead code, unreachable branches, deprecated APIs, duplicated implementations, and obsolete configuration options. Some feature flags contain branches that are never reachable on public networks, some services are registered but always return UNIMPLEMENTED, while others duplicate the same logic across multiple implementations. There are also configuration options that are no longer functional but still require documentation, testing, and maintenance.
This issue serves as a tracking (epic) issue to consolidate these cleanup efforts. It provides an overview of completed work, tracks planned cleanup tasks, and defines the common principles that should guide future cleanup work.
Each cleanup effort will continue to be implemented through its own dedicated Issue and Pull Request. This tracking issue serves only as an index and roadmap.
Motivation
Legacy code provides little or no functional value while continuously increasing the maintenance burden of the project.
Dead code and permanently unreachable branches make it difficult for contributors to determine whether a piece of code is actually executed or whether a configuration option still has any effect. This increases the cognitive cost of understanding the codebase and slows down development. Likewise, maintaining multiple copies of the same logic—such as separate servlet implementations for FullNode, Solidity, and PBFT—requires every functional change to be replicated in several places, making behavioral inconsistencies more likely over time.
Reducing unused code also reduces the security audit surface. Static analysis, security reviews, and bug bounty programs become more focused, generate fewer false positives, and require less effort to evaluate obsolete implementations. For example, the existing partial SM2 implementation repeatedly raises compliance questions despite being unreachable on public networks.
Some obsolete configuration options are also misleading for node operators. Options such as node.walletExtensionApi appear to enable functionality but have no practical effect, leading to unnecessary operational confusion.
Certain historical feature flags are effectively "footguns" that can introduce consensus risks if misconfigured, such as actuator.whitelist and crypto.engine. Removing these unreachable code paths eliminates unnecessary risk without changing observable network behavior.
Finally, removing dead code, obsolete test utilities, and duplicated implementations reduces long-term maintenance costs, simplifies regression testing, and makes future refactoring significantly easier.
Cleanup Principles
All cleanup tasks should follow a consistent set of principles:
-
The highest priority is to preserve observable behavior by default; every intentional delta must be explicitly listed, independently tested, and follow the applicable deprecation/migration process. For any modifications involving consensus, hashing, serialization, or protocol encoding, byte-level regression verification should be performed to ensure that block hashes, transaction hashes, address derivation, and protobuf encoding remain unchanged.
-
Public APIs and protobuf services should follow a deprecation process. They should first be marked as deprecated and remain available for at least one release cycle before being removed in a future major release. This avoids unexpected compatibility issues for downstream SDKs and applications. The WalletExtension cleanup follows this approach.
-
Each cleanup should be implemented through an independent Issue and Pull Request to simplify review and rollback. Large refactorings may be further divided into logical commits to make review easier.
Whenever a configuration option is removed, the corresponding documentation, reference.conf, and configuration validation should be updated accordingly.
Completed
| Issue | Description |
|---|---|
| #6666 | Remove actuator.whitelist and related logic to eliminate potential fork risks. |
| #6665 | Remove InfluxDB metrics backend support. |
| #6595 | Remove scheduled database backup in favor of dual-node failover. |
| #6548 | Remove HTTP REST mappings in gRPC protobuf definitions. |
Planned
The following cleanup proposals will each be tracked as independent issues.
1. Consolidate Solidity/PBFT HTTP APIs
The servlet implementations under interfaceOnSolidity.http and interfaceOnPBFT.http contain almost no unique business logic. They simply switch the read cursor before delegating to the corresponding FullNode servlet.
The proposal is to remove these wrapper servlets and reuse the existing FullNode implementations, while switching read cursors through a request-path-based filter for /walletsolidity/* and /walletpbft/*. This eliminates duplicated implementations and prevents long-term behavioral drift.
issue: #6922
PR:#6976
author: @SeriousCoding789
2. Consolidate Solidity/PBFT gRPC APIs
RpcApiServiceOnSolidity and RpcApiServiceOnPBFT consist almost entirely of boilerplate delegation to the primary RpcApiService.
The proposal is to reuse the existing implementation and perform cursor switching through a parameterized ServerInterceptor, removing nearly one thousand lines of duplicated code.
issue: #6927
PR:#6977
author: @SeriousCoding789
3. Remove the SM2/SM3 cryptographic engine
The crypto.engine configuration is effectively fixed to ECKey on public networks, making all SM2/SM3 execution paths unreachable. In addition, the current SM2 implementation does not fully comply with the GM/T 0003 specification.
The proposal is to remove the unused SM2 implementation together with all unreachable isECKeyCryptoEngine branches. Besides reducing maintenance costs, this cleanup also prepares a cleaner cryptographic abstraction for future post-quantum cryptography work.
issue: #6959
PR: #6999
author: @Federico2014
4. Remove WalletExtension dead code and deprecate its protobuf service
The WalletExtension gRPC service has never implemented any functionality. Every RPC method currently returns UNIMPLEMENTED, while node.walletExtensionApi is an ineffective configuration option.
The cleanup will be performed in two stages. The first stage removes the dead Java implementation, obsolete configuration, and test utilities while marking the protobuf service as deprecated. The second stage removes the protobuf service entirely in a future major release.
issue: #6931
author: @0xbigapple
5. Remove --keystore-factory support from FullNode
Since v4.8.2, keystore generation has been fully migrated to Toolkit.jar. Continuing to support --keystore-factory inside FullNode.jar unnecessarily duplicates functionality and blurs component responsibilities.
Removing this option allows FullNode to focus exclusively on node operation while providing a single entry point for keystore management.
issue:#6949
author: @0xbigapple
6. Remove unused RLP utilities
The RLP-related utilities under framework/src/main/java/org/tron/core/capsule/utils are no longer part of TRON's core execution path.
Removing these unused utilities will simplify maintenance, reduce testing overhead, and make module boundaries clearer.
issue:No issue needed
author: @SeriousCoding789
PR:#6991
7. Remove non-Prometheus metrics implementations
Prometheus has become the standard monitoring solution for java-tron. Maintaining additional metrics implementations and MonitorApi introduces unnecessary duplication and operational confusion.
The proposal is to remove all non-Prometheus metrics implementations together with MonitorApi, leaving Prometheus as the single supported monitoring backend.
issue: #6923
author: @warku123
Future Work
Additional dead code, unreachable functionality, and obsolete components are welcome.
New cleanup candidates can be proposed under this tracking issue before being promoted into dedicated issues and pull requests.
1. Remove the VM trace compression path
VMConfig.vmTraceCompressed is hardcoded to false with no setter, and there is no corresponding key in the VmConfig bean or reference.conf — unlike vm.vmTrace, which is fully wired through Args → CommonParameter → ConfigLoader. The flag is therefore permanently off.
author: @halibobo1205
PR: #6997
2. Remove selected historical Java assert statements
Remove selected historical Java assert statements that are inactive under the default production JVM configuration and deletes two tracked 0-byte Java placeholder files.
PR: #6960
author: @bladehan1
3. Clear up dead code in getAssetIssuesPaginated and useAssetAccountNet
author: @halibobo1205
4. Remove four unused defination in protocol/src/main/protos/core/Discover.proto
Remove PingMessage, PongMessage, FindNeighbours, and Neighbours from protocol/src/main/protos/core/Discover.proto.
These four definitions are no longer used by java-tron. UDP discovery uses libp2p’s separate org.tron.p2p.protos.Discover definitions, while java-tron’s TCP ping/pong messages use a fixed 0xC0 payload.
Keep Endpoint, which is referenced by HelloMessage.from, and BackupMessage, which is used by backup keepalive messages.
issue: #7013
PR:
author: @317787106
This tracking issue will continue to be updated as cleanup work progresses.
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsNo status
Activity
GuipaiQigong111 commentedon Aug 19, 2026
Thanks for putting this tracking issue together. The overall direction makes sense, and I agree that each cleanup item should be handled in a separate issue and PR so the changes remain easy to review, verify, and roll back.
A few requirements should be applied to the follow-up work:
Please link every child issue and PR back to this tracking issue and keep the Completed and Planned sections updated. I suggest starting with the lower-risk utility and implementation cleanup items, while handling the cryptographic engine and public API or protobuf changes as separate, higher-scrutiny efforts.
[-]Tracking Issue: Code refactor and cleanup[/-][+]Tracking: Code refactor and cleanup[/+][-]Tracking: Code refactor and cleanup[/-][+]Tracking: code refactor and cleanup[/+]SeriousCoding789 commentedon Aug 20, 2026
Strongly support this cleanup effort — the duplicated implementations and the silent feature drift they cause are a real maintenance burden, and consolidating them is well worth it.
#6922 is an important subtask under this track, on the HTTP layer: the same API surface is maintained four times (FullNode / Solidity / PBFT / standalone SolidityNode), and the Solidity and PBFT stacks alone are ~96 wrapper servlets whose only job is switching the read cursor, with each service also keeping its own hand-written registration list. It collapses those per-endpoint wrappers into one servlet Filter per cursor service and drives all four services from a single declarative endpoint registry — removing ~2,900 lines with zero external behavior change.
317787106 commentedon Aug 20, 2026
@Sunny6889 Thanks for the support and for opening #6922. This proposal is clearly aligned with the HTTP API consolidation item and is appropriate to track as a child issue.
One clarification: the current proposal should not be described as having “zero external behavior change.” The four services do not expose exactly the same API surface today, and #6922 includes several intentional observable changes, such as adding two endpoints to the PBFT surface, removing five PBFT endpoints, aligning SolidityNode response behavior, and changing IOException handling.
Please continue the detailed design and scope discussion in #6922. Once the scope and verification plan are agreed, we can update this tracking issue accordingly.
yanghang8612 commentedon Aug 20, 2026
Strongly support this cleanup roadmap. Legacy code is not merely cosmetic debt: duplicated implementations can drift over time, ineffective configuration options mislead node operators, and unreachable consensus or cryptographic branches unnecessarily increase maintenance costs and the security-audit surface.
The proposed safeguards are appropriate: preserve observable network behavior, handle each cleanup through a separate Issue and PR, deprecate public APIs before removal, and perform byte-level regression verification for consensus, hashing, serialization, and protocol-encoding changes.
With these safeguards, this cleanup should significantly improve maintainability and security without affecting MainNet, Nile, or Shasta behavior. +1.
SeriousCoding789 commentedon Aug 21, 2026
Thanks for your clarification suggestion, will update the issue description.
warku123 commentedon Aug 24, 2026
Planned item 7 (remove non-Prometheus metrics implementations) now has a dedicated issue: #6923. It covers the removal of the legacy Dropwizard stack together with MonitorApi, the migration of the one functional consumer (fetch-block peer selection), and a tron:node_info{version, chain_id} metric to close the observability gap.
halibobo1205 commentedon Aug 26, 2026
Proposing one more cleanup candidate for the Future Work list: the VM trace compression path.
Evidence of unreachability
VMConfig.vmTraceCompressedis hardcoded tofalsewith no setter, and there is no corresponding key in theVmConfigbean orreference.conf— unlikevm.vmTrace, which is fully wired throughArgs→CommonParameter→ConfigLoader. The flag is therefore permanently off, and everything behind it is unreachable:common/src/main/java/org/tron/core/vm/config/VMConfig.java:11false, no settercommon/src/main/java/org/tron/core/vm/config/VMConfig.java:92actuator/src/main/java/org/tron/core/actuator/VMActuator.java:306actuator/src/main/java/org/tron/core/vm/VMUtils.java:127zipAndEncode(), only caller is the dead branch aboveactuator/src/main/java/org/tron/core/vm/VMUtils.java:112,123compress()overloads, only callers arezipAndEncode()/ each otheractuator/src/main/java/org/tron/core/vm/VMUtils.java:100write(InputStream, OutputStream, int)andBUF_SIZE, used only bycompress()No test references either the flag or these methods. Removal drops roughly 50 lines plus the
java.util.zipand Base64 imports from the actuator VM path.Why remove rather than wire the flag into config
saveProgramTraceFile()already writes to./vm_trace; there is no need for an in-processDeflateron the actuator path.Deflaterhanded toDeflaterOutputStreamincompress()is neverend()-ed —DeflaterOutputStream.close()callsdef.end()only when it created theDeflateritself — which is inconsistent withByteUtil.compress()/decompress()incommon, where release happens in afinallyblock. That is harmless today precisely because the path is unreachable, but it is the kind of finding that keeps resurfacing in static analysis and review, and hardening dead code is not worth a PR.Compatibility impact
None. The option was never exposed in
reference.conf, so no configuration migration, documentation, or release-note changes are required, andvm.vmTraceitself is unaffected. No consensus, hashing, serialization, or protocol-encoding surface is touched — the removed branch only changed the on-disk format of a debug trace file that is off by default.Happy to open a dedicated issue and PR under this tracking issue if the direction looks right. cc @yanghang8612
lxcmyf commentedon Aug 26, 2026
I think this epic would benefit from classifying each cleanup by compatibility risk, because the current absolute wording of Principle 1 (“changes must not alter observable behavior”) already conflicts with the existing child proposals:
I suggest changing Principle 1 to: preserve observable behavior by default; every intentional delta must be explicitly listed, independently tested, and follow the applicable deprecation/migration process. The tracking table could then include, for every child: issue/PR, cleanup class, observable delta, evidence of unreachability, deprecation required, network-validation requirement, target release, and status. This would give reviewers a concrete release gate instead of applying the same standard to dead-code deletion, public API removal, and consensus-adjacent refactoring.
For the VM trace compression candidate specifically, I verified the current
upstream/developimplementation:vmTraceCompressedis hardcoded tofalsewith no setter; its only read is inVMActuator; and thezipAndEncode→compresschain has no other callers. It looks suitable for a dedicated direct-removal issue. The regression test should enablevmTraceand assert that the existing plain trace output remains unchanged, rather than adding coverage for the unreachable branch.Please also link #6922 and #6923 directly from the Planned section and track their status there, so this epic remains the source of truth rather than relying on comments for child-issue discovery.
317787106 commentedon Aug 26, 2026
@halibobo1205 @lxcmyf Thanks, I have add it to Future work.
16 remaining items
halibobo1205 commentedon Sep 17, 2026
Proposing two more dead-branch cleanup candidates for the Future Work list. Both are never-taken branches caused by comparing protobuf
ByteStringvalues with==/!=(reference equality). Strictly speaking, they are not structurally dead: protobuf returnsthe shared
ByteString.EMPTYsingleton for an emptybytesfield, so reference equality can hold when both sides are empty; the branches are dead because empty values are rejected by validators, so the removal rationale is a validator invariant.1. Dead
ordertie-break inAssetIssueStore.getAssetIssuesPaginatedAssetIssueStore.java:50-53:Long.compare(o1.getOrder(), o2.getOrder())runs only when the two names are reference-equal, and empty names are rejected byTransactionUtil.validReadableBytes, so the branch is unreachable. Theorderfield is declared// uselessin the proto and is never written or validated by node code. Proposal: delete the tie-break so the comparator reverts to the plain name comparison.2. Dead issuer branch in
BandwidthProcessor.useAssetAccountNetBandwidthProcessor.java:313-315: the rule "an issuer transferring its own token pays from its own bandwidth" has never fired:getOwnerAddress() == getAddress()can only hold when both sides are empty, and an empty owner address is rejected byDecodeUtil.addressValid. Proposal: delete these three lines; behavior-preserving, no external change.317787106 commentedon Sep 21, 2026
Proposing another cleanup item for the Future Work list: remove
PingMessage,PongMessage,FindNeighbours, andNeighboursfrom protocol/src/main/protos/core/Discover.proto.These four definitions are no longer used by java-tron. UDP discovery uses libp2p’s separate org.tron.p2p.protos.Discover definitions, while java-tron’s TCP ping/pong messages use a fixed 0xC0 payload.
Keep
Endpoint, which is referenced by HelloMessage.from, andBackupMessage, which is used by backup keepalive messages.The cleanup should also update the corresponding protocol documentation and verify that handshake and backup message encoding remains unchanged. Any downstream compatibility impact from removing the generated message classes should be
checked before removal, following this issue’s cleanup principles.
317787106 commentedon Sep 21, 2026
@halibobo1205 Thanks for identifying these candidates. Small cleanup suggestions are welcome here too. We can use the comments to collect them while keeping the main roadmap focused on larger cleanup efforts.
For these localized changes, a focused PR linked to this issue should be sufficient; no separate issue is needed. Please include evidence that the branches are unreachable and regression tests confirming unchanged behavior, particularly for the bandwidth accounting change.
lxcmyf commentedon Sep 22, 2026
Two points should be explicit before treating these as behavior-preserving cleanup:
Discover.protomessages does not affect java-tron call sites, but it removes generated public classes from the protocol artifact. Please either follow a deprecation cycle or provide evidence that no supported downstream dependency uses them.BandwidthProcessorbranch is part of consensus-critical resource accounting. Deleting the broken==condition preserves the current de facto behavior, but also permanently abandons the apparent intended rule. The PR should include a regression test for an issuer transferring its own asset and verify account bandwidth, asset bandwidth counters, and persisted account state.If restoring the intended issuer behavior is desired, it should be handled separately as an activated consensus change. The
AssetIssueStoretie-break looks suitable for direct cleanup once equal-name pagination behavior is covered.halibobo1205 commentedon Sep 22, 2026
@317787106
Unreachability evidence for the two candidates.
Common lemma. Neither branch is structurally dead, so the evidence below is about data invariants. protobuf-java returns the shared
ByteString.EMPTYsingleton for an absent or zero-lengthbytesfield, and allocates a distinctByteStringper parsed message for non-empty ones (verified against the runtime jar:javap -cofCodedInputStream$ArrayDecoder.readBytesshowsifne / getstatic ByteString.EMPTY / areturn). So for two independently parsed messages,==/!=on abytesfield reduces to "both sides empty", and each site is unreachable iff the empty case cannot occur there.Site A —
AssetIssueStore.java:50-53(order tie-break)getAllAssetIssues()→TronStoreWithRevoking.iterator()(TronStoreWithRevoking.java:185-193, fresh capsule per entry); the snapshot layers and store cache only carry raw bytes (SnapshotImpl.java:115-126,SnapshotRoot.java:186-188). No sharedByteStringinstance can enter the list.o1.getName() != o2.getName()is always true and the comparator always returns the name comparison at line 51; the tie-break at line 53 never runs.AssetIssueActuator.validate(AssetIssueActuator.java:165-167) →TransactionUtil.validAssetName/validReadableBytes(TransactionUtil.java:89-91, 104-110) rejects empty names and any byte outside0x21–0x7E. The empty-name rejection has been present in every revision of this check since0392b3d2a9(2018-05-10); it is not fork-gated.Wallet.getAssetIssueList(offset, limit)(Wallet.java:1546), behind the read-onlygetPaginatedAssetIssueListendpoints. The result is never persisted, hashed, or emitted to the event plugin.Site B —
BandwidthProcessor.java:313-315(issuer self-transfer branch)accountCapsulefromAccountStore.get(BandwidthProcessor.java:133),assetIssueCapsulefromgetAssetIssueStoreFinal(...).get(BandwidthProcessor.java:303-306). No construction path shares an instance between them.==is always false and the branch never executes.owner_addressis rejected when empty / not 21 bytes / wrong prefix byDecodeUtil.addressValid(DecodeUtil.java:15-33), enforced atAssetIssueActuator.java:160-163; everyAccountCapsuleconstruction path setsaddress, andAccountStore.getreturns null for an empty stored value (AccountStore.java:62-65).useAssetAccountNethas a single call site (BandwidthProcessor.java:155), reached only throughManager.consumeBandwidth(Manager.java:1023) from pushTransaction / generateBlock / processBlock. There is no estimation or dry-run caller, so removal leaves the consensus write set byte-identical.317787106 commentedon Sep 22, 2026
@lxcmyf The compatibility scope for this cleanup is compatibility across java-tron versions: preserving node interoperability, wire formats, and persisted-data compatibility.
Third-party ecosystem projects generally operate at the application layer and integrate through java-tron’s HTTP, gRPC, or JSON-RPC APIs. They do not normally depend on these obsolete discovery-protocol definitions. The presence of public generated classes in the protocol artifact does not, by itself, make every class a supported external API. Compatibility requirements for supported APIs and their protobuf messages still apply.
These four definitions no longer participate in java-tron’s runtime communication. UDP discovery uses libp2p’s separate definitions, while TCP ping/pong uses a fixed 0xC0 payload. Removing them therefore does not change the messages exchanged between java-tron versions.
We should continue the cleanup, retain Endpoint and BackupMessage, update the relevant documentation, and verify that handshake and backup message encoding remains unchanged. Possible third-party references to these unused generated classes should not require a deprecation cycle or block their removal. The tracking issue’s earlier wording about downstream compatibility should be clarified accordingly.
halibobo1205 commentedon Sep 23, 2026
The VM trace compression path will be removed by:
317787106 commentedon Oct 8, 2026
I add the issue #7013 to this track.
317787106 commentedon Oct 8, 2026
Propose retire legacy framework TCP ping/pong compatibility code:
TCP keepalive is now handled by the p2p module. The framework's active ping scheduling and timeout handling were removed in e236398, while passive handling of legacy
P2P_PING/P2P_PONGmessages remained for backward compatibility.Propose a dedicated issue and PR, to remove:
PingMessageandPongMessageunderframework/src/main/java/org/tron/core/net/message/keepalive.KeepAliveService.The active p2p keepalive implementation and UDP discovery messages should remain unchanged.
lxcmyf commentedon Oct 9, 2026
The compatibility boundary should be clarified before removing this path.
P2P_PINGis still dispatched toKeepAliveService, which responds withP2P_PONG; removing that handling could affect connections with older peers that still send these messages. Please identify the oldest supported peer version and add a mixed-version interoperability test. If backward compatibility is no longer required, documenting the version cutoff and verifying that the p2p keepalive handles supported peers would make the removal easier to assess.0xbigapple commentedon Oct 10, 2026
@317787106 Reminder: Principle 2 and Planned item 4 still describe the two-stage WalletExtension plan. #6975 removes the Java implementation, the
node.walletExtensionApikey and the protobuf definitions together, targeting v4.8.3. Could you update both?A correction to my earlier comment: on a solidity node that listed these methods in
node.disabledApi, the status changes fromUNAVAILABLEtoUNIMPLEMENTED. Details are in #6931 and #6975.