feat(crypto): add native secp256k1 support - #52
Open
Federico2014 wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughChangesThe PR adds a configurable JNA-backed secp256k1 implementation. It supports native signing, public-key recovery, serialization, validation, and address derivation. EC-key verification uses the native backend when configured and available. PBFT recovery now uses Native secp256k1 integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Args
participant SignUtils
participant NativeSecp256k1
participant PbftDataSyncHandler
Args->>SignUtils: Apply EC-key native verification setting
PbftDataSyncHandler->>SignUtils: Recover address from PBFT signature
SignUtils->>NativeSecp256k1: Recover address when native mode is enabled
NativeSecp256k1-->>SignUtils: Return recovered address
SignUtils-->>PbftDataSyncHandler: Return signature address
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
consensus/src/main/java/org/tron/consensus/pbft/message/PbftBaseMessage.java (1)
99-100: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the hardcoded engine flag.
The literal
trueselects the EC-key crypto engine. It preserves the previousECKey.signatureToAddressbehavior, so PBFT recovery stays on EC-key even when the node runs the SM2 engine. That intent is not visible at the call site.Add a short comment, or extract a named constant such as
PBFT_USES_ECKEY_ENGINE. The same literal appears inframework/src/main/java/org/tron/core/net/messagehandler/PbftDataSyncHandler.javaat lines 176 to 177.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@consensus/src/main/java/org/tron/consensus/pbft/message/PbftBaseMessage.java` around lines 99 - 100, The hardcoded crypto-engine flag in PbftBaseMessage signature recovery is undocumented. Add a short comment or reuse a clearly named constant such as PBFT_USES_ECKEY_ENGINE at the SignUtils.signatureToAddress call, and apply the same documentation or constant usage to the corresponding call in PbftDataSyncHandler so the intentional EC-key behavior remains explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crypto/build.gradle`:
- Line 16: Update the secp256k1 dependency declaration in the crypto Gradle
configuration: either replace
com.github.federico2014.besu-native:secp256k1:1.3.11 with the official upstream
coordinate, or document the fork’s necessity and build provenance, including who
reviewed it, while preserving the verification metadata alignment.
---
Nitpick comments:
In
`@consensus/src/main/java/org/tron/consensus/pbft/message/PbftBaseMessage.java`:
- Around line 99-100: The hardcoded crypto-engine flag in PbftBaseMessage
signature recovery is undocumented. Add a short comment or reuse a clearly named
constant such as PBFT_USES_ECKEY_ENGINE at the SignUtils.signatureToAddress
call, and apply the same documentation or constant usage to the corresponding
call in PbftDataSyncHandler so the intentional EC-key behavior remains explicit.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 26652c53-d3cf-4f18-b34a-210fb360adea
📒 Files selected for processing (16)
common/src/main/java/org/tron/common/parameter/CommonParameter.javacommon/src/main/java/org/tron/core/config/args/MiscConfig.javacommon/src/main/resources/reference.confcommon/src/test/java/org/tron/core/config/args/MiscConfigTest.javaconsensus/src/main/java/org/tron/consensus/pbft/message/PbftBaseMessage.javacrypto/build.gradlecrypto/src/main/java/org/tron/common/crypto/NativeSecp256k1.javacrypto/src/main/java/org/tron/common/crypto/SignUtils.javaframework/src/main/java/org/tron/core/config/args/Args.javaframework/src/main/java/org/tron/core/net/messagehandler/PbftDataSyncHandler.javaframework/src/main/resources/config.confframework/src/test/java/org/tron/common/ParameterTest.javaframework/src/test/java/org/tron/common/crypto/NativeSecp256k1BenchmarkTest.javaframework/src/test/java/org/tron/common/crypto/NativeSecp256k1Test.javaframework/src/test/java/org/tron/core/config/args/ArgsTest.javagradle/verification-metadata.xml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 16 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Federico2014
force-pushed
the
feature/configurable_secp256k1_jna
branch
from
August 7, 2026 08:31
780fcc6 to
f9bd0f3
Compare
Federico2014
force-pushed
the
feature/configurable_secp256k1_jna
branch
from
August 7, 2026 09:25
b2bfe99 to
174b6a0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Add an optional JNA-backed
NativeSecp256k1implementation for secp256k1 key construction, deterministic signing, public-key recovery, and address recovery.Add the
crypto.useNativeSecp256k1configuration option and route ECKey signature verification through the native implementation when explicitly enabled. PBFT signature recovery now uses the sharedSignUtilsrouting path.Add native/ECKey cross-compatibility tests, configuration tests, dependency verification metadata, and an opt-in signing and recovery benchmark.
Why are these changes required?
The existing pure-Java ECKey implementation has significantly higher signing and signature-recovery overhead. Native libsecp256k1 provides a faster implementation while preserving the existing signature format, recovery ID handling, canonical low-S behavior, and recovered TRON addresses.
The feature is disabled by default, so existing nodes retain the original Java implementation unless explicitly configured otherwise.
This PR has been tested by:
The native and Java implementations were cross-tested with deterministic signatures, bidirectional signature recovery, constructor compatibility, high-S signatures, malformed input, and configuration routing.
NativeSecp256k1Test, related configuration tests,checkstyleMain, andcheckstyleTestpassed.An opt-in local benchmark with 5,000 iterations measured approximately 32.08x faster signing and 16.57x faster signature-address recovery on an x86_64 Java 8 environment.
Follow up
None.
Extra details
The new configuration defaults to
falseand is effective only whencrypto.engine = "eckey". SM2 behavior is unchanged. If the native library is unavailable, verification remains on ECKey.This change adds JNA 5.12.1 and
com.github.federico2014.besu-native:secp256k1:1.3.11, including Gradle dependency verification metadata.Summary by cubic
Adds optional native secp256k1 via JNA for signing and public-key/address recovery, toggled by
crypto.useNativeSecp256k1whencrypto.engine = "eckey". PBFT signature recovery now goes throughSignUtils; startup throwsNATIVE_CRYPTO_INITif native is enabled but unavailable, and the flag is ignored forsm2.New Features
NativeSecp256k1(deterministic signing, pubkey/address recovery) with ECKey-compatible format and canonical low-S.SignUtils(setUseNativeSecp256k1), wired viaArgs, exposed asCommonParameter.useNativeSecp256k1; config keys added toreference.confandconfig.conf.Dependencies
net.java.dev.jna:jna:5.12.1andcom.github.federico2014.besu-native:secp256k1:1.3.11; updated Gradle verification metadata.Written for commit 174b6a0. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests