Skip to content

[Feature] Remove unused SM2 algorithm and related configuration #6588

Closed

Description

@Federico2014

Background

The java-tron codebase contains a complete SM2 implementation in the crypto module. The SignUtils.java provides a configuration flag isECKeyCryptoEngine to switch between ECKey (SECP256K1) and SM2. Additionally, SM3 hash implementation exists alongside SM2.

The crypto.engine configuration option allows switching between cryptographic engines, but in practice only ECKey is ever used in production.

Problem Statement

  1. SM2/SM3 is never used in production: All production deployments use ECKey (SECP256K1). The isECKeyCryptoEngine flag is always true.

  2. Dead code maintenance burden: Maintaining ~1,100+ lines of unused cryptographic code (SM2 + SM3) increases complexity, testing surface, and audit scope.

  3. Incompatible with TRON network: TRON uses SECP256K1 for all transactions. SM2 would not work with existing accounts.

  4. Bug Bounty false positives: The unused SM2 code frequently triggers false positive reports in bug bounty programs. Security researchers often flag SM2-related code as potential vulnerabilities, requiring time-consuming investigation and response.

  5. Unnecessary configuration complexity: The crypto.engine config and isECKeyCryptoEngine checks exist throughout the codebase but only serve unused code paths.

Rationale

Why should this feature exist?

Removing unused code improves code quality and reduces maintenance overhead. Eliminating the crypto.engine configuration simplifies the codebase by removing conditional logic that never changes in production.

What are the use cases?

  1. Developers will have cleaner codebase to work with
  2. Security audits will focus on actually-used cryptography
  3. Fewer false positive reports in bug bounty programs
  4. Simplified code with no unnecessary conditional branches

Scope of Impact

  • Core protocol
  • API/RPC
  • Database
  • Network layer
  • Smart contracts
  • Documentation
  • Other: crypto module, keystore, config

Breaking Changes:

Yes - but only for non-production code paths:

  • crypto.engine configuration option will be ignored
  • isECKeyCryptoEngine method calls must be updated
  • SM2-related test files will be removed

Backward Compatibility:

Fully backward compatible for production:

  • Default ECKey behavior remains unchanged
  • No production config uses SM2
  • Config crypto.engine can remain in old configs (will be ignored with warning)

Implementation

Are you willing to implement this feature?

  • Yes, I can implement this feature

Activity

  1. SeriousCoding789 commented on Mar 19, 2026

    @SeriousCoding789
    Contributor

    This is an excellent proposal that will significantly improve code quality and reduce maintenance overhead. However, I have some concerns about potential third-party integration impacts that should be addressed through a careful migration strategy.

    While SM2 is unused in production, some third-party integrations might still reference SM2-related APIs:

    1. Direct API Usage: Libraries or tools that call SignUtils.fromPrivate(bytes, isECKeyCryptoEngine) with hardcoded false values
    2. Configuration Dependencies: External systems that set crypto.engine = "sm2" in their configs
    3. Test Frameworks: Third-party testing tools that might use SM2 for isolated testing scenarios

    I strongly recommend implementing a phased deprecation approach rather than immediate removal - first add deprecation warnings while maintaining API compatibility, then gradually remove the code over 2-3 releases. This would give third-party developers adequate time to update their integrations and minimize ecosystem disruption.

    What are your thoughts on this approach?

  2. changed the title [-][FEATURE] Remove unused SM2 algorithm and related configuration[/-] [+][Feature] Remove unused SM2 algorithm and related configuration[/+] on Mar 19, 2026
  3. Federico2014 commented on Mar 19, 2026

    @Federico2014
    CollaboratorAuthor

    @Sunny6889 Thanks for the feedback! I understand your concerns about third-party integrations, but after careful consideration:

    • SM2 is truly unused in production: TRON network uses SECP256K1 for all transactions. SM2 would not work with existing accounts.
    • Third-party impact should be minimal: Any third-party tools using SM2 can continue using an older version of java-tron. This is similar to how other breaking changes are handled in the ecosystem.

    Given the low impact and the benefits of cleaner codebase, I believe immediate removal is the right approach. If anyone has strong objections with concrete use cases, we can reconsider.

  4. warku123 commented on Mar 19, 2026

    @warku123
    Contributor

    I really support this cleanup.
    Removing 1100+ lines of unused SM2/SM3 code will reduce maintenance overhead and eliminate recurring false positive security reports. Since TRON exclusively uses SECP256K1 in production, this is a safe change with no backward compatibility concerns.

  5. Federico2014 commented on Mar 20, 2026

    @Federico2014
    CollaboratorAuthor

    There are many Args.getInstance().isECKeyCryptoEngine() checks across the code base. If we remove them all, it affects 70+ files, but it's a clean approach. Alternatively, we could only remove the SM2 algorithm and keep
    Args.getInstance().isECKeyCryptoEngine() always returning true, which involves fewer code changes. I prefer cleaning up all Args.getInstance().isECKeyCryptoEngine() checks. Which approach do you think is more reasonable?

  6. halibobo1205 commented on Mar 26, 2026

    @halibobo1205
    Collaborator

    Agree with removing it directly — SM2 was never part of the public API, and no third party depends on it, so a clean removal seems like the better option. I don't see much value in keeping isECKeyCryptoEngine around either; what do others think?

  7. zhaog100 commented on Mar 26, 2026

    @zhaog100

    /attempt

  8. zhaog100 commented on Mar 26, 2026

    @zhaog100

    /attempt

  9. moved this to In Progress in java-tronon Mar 31, 2026
  10. moved this from In Progress to In Review in java-tronon Mar 31, 2026
  11. moved this from In Review to In Progress in java-tronon Mar 31, 2026
  12. Federico2014 commented on Apr 13, 2026

    @Federico2014
    CollaboratorAuthor

    After further discussion in PR #6627 (see #6627 (comment)), I've decided to close this issue.

    The reviewer pointed out that the benefit of this change is minimal — isECKeyCryptoEngine is always true and has no impact on runtime behavior. However, the cost is significant: 75 files changed across core modules. The SM2/SM3 support was designed as a crypto extension point and may still have value as a reserved abstraction.

    Weighing the risk-to-benefit ratio, we'll leave the current code as-is.

  13. moved this from In Progress to Done in java-tronon Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions