Skip to content

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

Closed
Federico2014 wants to merge 1 commit into
release_v4.8.3from
refactor/remove-sm2-sm3-v4.8.3
Closed

Federico2014 wants to merge 1 commit into
release_v4.8.3from
refactor/remove-sm2-sm3-v4.8.3

Conversation

@Federico2014

@Federico2014 Federico2014 commented Sep 22, 2026 •

Copy link
Copy Markdown
Owner

What does this PR do?

Remove SM2/SM3 implementations, runtime engine selection, and Toolkit --sm2 options. Simplify signing and hashing APIs to ECKey/secp256k1 and SHA-256, and update their callers and tests across the dependent modules.

Retain a startup-only compatibility check before storage or keystore initialization: an absent crypto.engine uses the default suite; explicit eckey is accepted with a deprecation warning; all other values, including sm2, null, and non-string values, fail with migration guidance. Add configuration and FullNode startup regression tests and document the migration requirements.

Why are these changes required?

The unused engine switch spans consensus-critical hashes, signatures, addresses, and Merkle roots. Removing it simplifies the cryptographic path, while the startup check prevents an existing SM2 deployment from silently switching algorithms against incompatible chain data. Implements the compatibility requirements discussed in tronprotocol#6959.

This PR has been tested by:

  • Focused configuration and FullNode startup tests passed, covering default startup, explicit eckey, unsupported values, and rejection before database-directory creation or keystore initialization.
  • ./gradlew test :framework:checkstyleMain :framework:checkstyleTest --continue --console=plain passed: 3,437 tests passed, 27 skipped, zero failures; both framework Checkstyle tasks passed.
  • Manual live-node testing was not performed.

Follow up

Existing SM2/SM3 deployments must remain on a compatible release until a separate migration is completed. Changing the configuration does not convert chain data or keys.

Extra details

Targets release_v4.8.3. The cross-module changes are required by the removal of shared engine-selecting APIs. This breaks those Java APIs and removes SM2/SM3 support; the default ECKey/SHA-256 path is retained.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 3b2fe5dd-4531-4d7c-948b-26d3204ebc1b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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 97 files

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

Re-trigger cubic

Comment thread framework/src/test/java/org/tron/program/FullNodeCryptoEngineTest.java Outdated
@Federico2014
Federico2014 force-pushed the refactor/remove-sm2-sm3-v4.8.3 branch from 65d9f19 to c8fd19f Compare September 23, 2026 16:32
Remove SM2/SM3 implementations and runtime engine selection, simplify cryptographic APIs and update their callers and tests. Reject unsupported legacy crypto.engine settings before storage or keystore initialization, with migration guidance and startup regression coverage.
@Federico2014
Federico2014 force-pushed the refactor/remove-sm2-sm3-v4.8.3 branch from c8fd19f to 93ec564 Compare September 23, 2026 16:34
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.

1 participant