Skip to content

refactor(crypto): remove unused SM2 and SM3 support - #54

Open
Federico2014 wants to merge 1 commit into
developfrom
refactor/remove-sm2-sm3
Open

refactor(crypto): remove unused SM2 and SM3 support#54
Federico2014 wants to merge 1 commit into
developfrom
refactor/remove-sm2-sm3

Conversation

@Federico2014

@Federico2014 Federico2014 commented Aug 6, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

This PR removes the unused SM2 signing and SM3 hashing implementations, eliminates the runtime crypto engine switch, and simplifies the signing and hashing APIs to use ECKey and SHA-256 directly.

It updates the dependent actuator, chainbase, common, consensus, framework, crypto, and plugin call sites. It also removes the crypto.engine configuration and Toolkit --sm2 option, updates documentation, and adjusts the related tests.

Why are these changes required?

SM2 and SM3 are not used by the supported default node configuration. Maintaining the alternate crypto path adds duplicated logic, configuration ambiguity, and unnecessary maintenance overhead.

The cross-module changes are required because the removed algorithm-selection parameters were propagated through hashing, signing, transaction, block, consensus, Keystore, and Toolkit APIs.

This PR has been tested by:

  • Unit Tests: focused common configuration, framework crypto and Keystore, and plugin Keystore tests
  • Build Verification: framework and plugin production/test compilation
  • Manual Testing: Not performed

Follow up

None.

Extra details

The supported ECKey and SHA-256 consensus behavior remains unchanged. This PR does not change database formats, does not require a hard fork, and preserves compatibility with standard ECKey Keystore files.

The removed crypto.engine configuration, SM2/SM3 APIs, SM2 Keystore handling, and Toolkit --sm2 option are no longer supported.

This implementation follows the direction of tronprotocol#6627 and closes #51.

Summary by CodeRabbit

  • Breaking Changes

    • Removed SM2 cryptography support; signing, hashing, address generation, and keystore operations now use EC cryptography.
    • Removed the --sm2 keystore option.
    • Removed configurable crypto-engine settings and related configuration entries.
    • Simplified wallet and keystore interfaces by removing algorithm-selection options.
  • Bug Fixes

    • Standardized transaction, block, message, address, and Merkle hashing on SHA-256.
    • Keystore import now handles whitespace and uppercase 0X key prefixes correctly.
    • Legacy crypto settings are validated and rejected when unsupported.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 819eb8b9-1030-4a65-b610-123b64ea7251

📥 Commits

Reviewing files that changed from the base of the PR and between 3eb3793 and 94f83d9.

📒 Files selected for processing (5)
  • framework/src/main/java/org/tron/core/consensus/ConsensusService.java
  • framework/src/main/java/org/tron/core/net/service/relay/RelayService.java
  • framework/src/main/java/org/tron/core/services/http/Util.java
  • framework/src/test/java/org/tron/core/zksnark/ShieldedReceiveTest.java
  • plugins/src/test/java/org/tron/plugins/KeystoreUpdateTest.java
🚧 Files skipped from review as they are similar to previous changes (4)
  • framework/src/test/java/org/tron/core/zksnark/ShieldedReceiveTest.java
  • framework/src/main/java/org/tron/core/services/http/Util.java
  • framework/src/main/java/org/tron/core/consensus/ConsensusService.java
  • framework/src/main/java/org/tron/core/net/service/relay/RelayService.java

📝 Walkthrough

Walkthrough

The PR removes SM2/SM3 support and the crypto.engine switch. Cryptographic APIs now use ECKey and SHA-256 directly. Production callers, configuration, keystore tools, plugins, documentation, and tests are updated accordingly.

Changes

Cryptography cleanup

Layer / File(s) Summary
Simplified cryptographic APIs
common/..., crypto/..., plugins/...
Sha256Hash and SignUtils no longer select algorithms through boolean parameters. SM2-specific APIs and implementations are removed.
Production caller migration
actuator/..., chainbase/..., consensus/..., framework/..., plugins/...
Transaction, block, message, VM, consensus, wallet, RPC, Merkle, and database hashing and signing use ECKey and SHA-256 defaults.
Configuration and keystore updates
common/..., framework/..., crypto/..., plugins/..., docs/...
Crypto-engine configuration, keystore algorithm flags, and --sm2 command options are removed.
Test updates
framework/src/test/..., plugins/src/test/..., common/src/test/...
Tests use the revised APIs, remove SM2/SM3 coverage, validate legacy configuration handling, and verify rejection of the removed --sm2 option.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.13% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: removal of unused SM2 and SM3 cryptographic support.
Linked Issues check ✅ Passed The changes remove SM2/SM3, the crypto.engine switch, engine-dependent APIs, keystore options, and related tests while retaining ECKey/SHA-256 paths [#51].
Out of Scope Changes check ✅ Passed The production, configuration, documentation, and test changes directly support removing SM2/SM3 and the crypto.engine switch [#51].
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/remove-sm2-sm3

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 95 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

@Federico2014
Federico2014 force-pushed the refactor/remove-sm2-sm3 branch from d1d1111 to 6540280 Compare August 7, 2026 07:40
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@Federico2014
Federico2014 force-pushed the refactor/remove-sm2-sm3 branch from 3eb3793 to 94f83d9 Compare August 10, 2026 08:54
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.

[Feature] Remove unused SM2/SM3 crypto engine

1 participant