Skip to content

refactor(toolkit): move keystore-factory to plugins module#4

Open
Federico2014 wants to merge 1 commit intodevelopfrom
feature/keystore-factory-to-plugins
Open

refactor(toolkit): move keystore-factory to plugins module#4
Federico2014 wants to merge 1 commit intodevelopfrom
feature/keystore-factory-to-plugins

Conversation

@Federico2014
Copy link
Copy Markdown
Owner

@Federico2014 Federico2014 commented Apr 1, 2026

What does this PR do?

Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details


Summary by cubic

Moved the keystore factory into the plugins module as a Toolkit subcommand and removed the node flag-based launcher. Also repackaged keystore classes under org.tron.common.crypto.keystore and updated references.

  • Refactors

    • Added keystore-factory as a Toolkit subcommand in plugins; updated plugins/README.md with usage.
    • Removed --keystore-factory flag and CommonParameter.keystoreFactory; FullNode no longer starts it.
    • Moved Credentials, Wallet, WalletFile, WalletUtils to org.tron.common.crypto.keystore and updated imports.
    • Simplified key generation/decryption to use EC key engine by default (removed Args.isECKeyCryptoEngine() dependency).
  • Migration

    • Run the tool with: java -jar Toolkit.jar keystore-factory, then use GenKeystore or ImportPrivateKey.
    • Remove --keystore-factory from any node startup scripts or configs.
    • Update any external imports from org.tron.keystore.* to org.tron.common.crypto.keystore.*.

Written for commit 6533628. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Removed keystoreFactory startup option from main node initialization.
  • Documentation

    • Added keystore factory interactive mode guide with command examples and file output specifications.
  • Refactor

    • Reorganized cryptographic keystore utilities to a unified package structure.
    • Simplified keystore factory into a standalone plugin command with improved architecture.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 1, 2026

📝 Walkthrough

Walkthrough

Reorganizes keystore classes from org.tron.keystore to org.tron.common.crypto.keystore, removes the --keystore-factory CLI flag and related configuration, relocates KeystoreFactory to plugins as a Picocli subcommand, and eliminates conditional keystore factory startup logic from FullNode.

Changes

Cohort / File(s) Summary
Keystore Package Migration
crypto/src/main/java/org/tron/common/crypto/keystore/Credentials, Wallet, WalletFile, WalletUtils java
Moved keystore classes from org.tron.keystore to org.tron.common.crypto.keystore package. Replaced Args.getInstance().isECKeyCryptoEngine() with hardcoded true in Wallet and WalletUtils. Reduced exception declarations in WalletUtils generateNewWalletFile() method.
Keystore Import Updates (Framework & Core)
framework/src/main/java/org/tron/core/Wallet, DiversifierT, WitnessInitializer java
Updated imports to reference keystore classes from new org.tron.common.crypto.keystore package. Changed method invocation source for generateRandomBytes() in Wallet and DiversifierT.
CLI Configuration Removal
common/src/main/java/org/tron/common/parameter/CommonParameter, framework/src/main/java/org/tron/core/config/args/CLIParameter, Args java
Removed keystoreFactory boolean field from CommonParameter, removed --keystore-factory CLI parameter from CLIParameter, and removed CLI wiring from Args including help option grouping metadata.
FullNode Startup Logic Removal
framework/src/main/java/org/tron/program/FullNode java
Removed early-exit branch that invoked KeystoreFactory.start() when keystore factory mode was enabled, allowing standard full-node startup path to execute unconditionally.
KeystoreFactory Plugin Integration
plugins/src/main/java/common/org/tron/plugins/KeystoreFactory, Toolkit java
Relocated KeystoreFactory from org.tron.program to org.tron.plugins, converted to Picocli command implementing Runnable with metadata, replaced start() with run() override, and registered as subcommand in Toolkit.
Test Import Updates
framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsVerifyProofTest, org/tron/core/config/args/ArgsTest, org/tron/core/.../WitnessInitializerTest, org/tron/core/db/TransactionExpireTest, org/tron/core/db/TxCacheDBInitTest, org/tron/core/db/TxCacheDBTest, org/tron/core/jsonrpc/ApiUtilTest, org/tron/core/zksnark/SendCoinShieldTest, org/tron/core/zksnark/ShieldedReceiveTest, org/tron/keystroe/CredentialsTest, org/tron/keystore/CredentialsTest, org/tron/keystore/WalletFileTest, org/tron/program/SupplementTest java
Updated imports from org.tron.keystore.* to org.tron.common.crypto.keystore.* across all test files. Removed test assertion for keystoreFactory flag in ArgsTest.
Plugin Build Configuration
plugins/build.gradle
Added dependency on :crypto module with transitive exclusions. Updated binaryRelease task to include project(':crypto').jar and project(':common').jar.
Documentation
plugins/README.md
Added "Keystore Factory" documentation section describing interactive mode, available commands (GenKeystore, ImportPrivateKey, Exit/Quit), usage examples, input prompts, and output file location/naming convention.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

  • Federico2014/tron-skills#1: This PR implements the proposed relocation of KeystoreFactory from org.tron.program to org.tron.plugins as a proper Picocli subcommand and removal of the --keystore-factory CLI flag, directly addressing the feature request objectives.

Poem

🐰 The keystore now finds a new home so grand,
Reorganized packages across the land!
CLI flags fade, like morning dew,
While Picocli commands spring bright and new,
Hopping through plugins with factored delight! 🏃‍♂️✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.68% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and concisely describes the primary refactoring: moving the keystore-factory functionality from the framework to the plugins module. It accurately summarizes the main change across the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/keystore-factory-to-plugins

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.42.0)
framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsVerifyProofTest.java
framework/src/main/java/org/tron/core/Wallet.java

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
plugins/src/main/java/common/org/tron/plugins/KeystoreFactory.java (1)

105-148: Consider using try-with-resources for Scanner.

The Scanner is created but only closed on explicit exit/quit commands. While acceptable for a CLI tool (JVM exit handles cleanup), using try-with-resources would be more defensive.

♻️ Optional: Use try-with-resources for Scanner
 `@Override`
 public void run() {
-  Scanner in = new Scanner(System.in);
-  help();
-  while (in.hasNextLine()) {
-    try {
-      // ... existing code ...
-      case "exit":
-      case "quit": {
-        System.out.println("Exit !!!");
-        in.close();
-        return;
-      }
-      // ...
-    } catch (Exception e) {
-      logger.error(e.getMessage());
+  try (Scanner in = new Scanner(System.in)) {
+    help();
+    while (in.hasNextLine()) {
+      try {
+        // ... existing code ...
+        case "exit":
+        case "quit": {
+          System.out.println("Exit !!!");
+          return;
+        }
+        // ...
+      } catch (Exception e) {
+        logger.error(e.getMessage());
+      }
     }
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/src/main/java/common/org/tron/plugins/KeystoreFactory.java` around
lines 105 - 148, The run() method in KeystoreFactory creates a Scanner (Scanner
in = new Scanner(System.in)) and only closes it on explicit "exit"/"quit" paths;
change run() to use try-with-resources for the Scanner so it is always closed
(even on exceptions/early returns) by wrapping the Scanner creation in a
try(...) block and moving the loop inside that block, removing the explicit
in.close() in the exit/quit branch; this ensures deterministic resource cleanup
while keeping the existing command parsing and methods genKeystore(),
importPrivateKey(), and help() intact.
crypto/src/main/java/org/tron/common/crypto/keystore/WalletUtils.java (1)

34-46: Inconsistent exception declarations with the underlying implementation.

generateFullNewWalletFile and generateLightNewWalletFile still declare NoSuchAlgorithmException, NoSuchProviderException, and InvalidAlgorithmParameterException, but the underlying generateNewWalletFile they call no longer throws these. These declarations are now misleading since SignUtils.getGeneratedRandomSign() doesn't throw checked exceptions.

♻️ Proposed fix to align exception declarations
 public static String generateFullNewWalletFile(String password, File destinationDirectory)
-    throws NoSuchAlgorithmException, NoSuchProviderException,
-    InvalidAlgorithmParameterException, CipherException, IOException {
+    throws CipherException, IOException {

   return generateNewWalletFile(password, destinationDirectory, true);
 }

 public static String generateLightNewWalletFile(String password, File destinationDirectory)
-    throws NoSuchAlgorithmException, NoSuchProviderException,
-    InvalidAlgorithmParameterException, CipherException, IOException {
+    throws CipherException, IOException {

   return generateNewWalletFile(password, destinationDirectory, false);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crypto/src/main/java/org/tron/common/crypto/keystore/WalletUtils.java` around
lines 34 - 46, The two wrapper methods generateFullNewWalletFile and
generateLightNewWalletFile declare checked exceptions (NoSuchAlgorithmException,
NoSuchProviderException, InvalidAlgorithmParameterException) that the called
generateNewWalletFile no longer throws (SignUtils.getGeneratedRandomSign is not
throwing checked exceptions); update the method signatures by removing those
three exceptions and only declare the actual checked exceptions thrown (e.g.,
CipherException, IOException) so the throws clauses on generateFullNewWalletFile
and generateLightNewWalletFile match generateNewWalletFile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crypto/src/main/java/org/tron/common/crypto/keystore/Wallet.java`:
- Line 214: The decrypt path currently hardcodes EC reconstruction via
SignUtils.fromPrivate(privateKey, true) which breaks SM2; change Wallet.decrypt
to accept an explicit crypto-engine flag/enum (e.g., boolean isEc or
CryptoEngine engine) and call SignUtils.fromPrivate(privateKey, engineChoice)
instead of the hardcoded true, then update all callers that invoke
Wallet.decrypt(...) (credential loading paths and any direct calls) to pass the
active engine selection from runtime/config so SM2 deployments preserve their
engine choice.

In `@crypto/src/main/java/org/tron/common/crypto/keystore/WalletUtils.java`:
- Around line 50-52: The generateNewWalletFile() call currently hardcodes
SignUtils.getGeneratedRandomSign(..., true) which forces ECKey wallets; change
it to consult the node crypto engine setting by replacing the literal true with
CommonParameter.getInstance().isECKeyCryptoEngine(), and add the required import
for org.tron.common.parameter.CommonParameter; ensure
SignUtils.getGeneratedRandomSign(...) now receives the dynamic boolean so
SM2-configured systems are respected.

In `@plugins/README.md`:
- Around line 184-193: The README's fenced code blocks showing keystore naming
(the block containing "UTC--yyyy-MM-ddTHH-mm-ss.nVV--<address>.json" and the
example block
"UTC--2024-01-15T10-30-45.123Z--TJRyHRFpmjG6mZ95mZaMvHx5fU7fGvRQoN.json") are
missing language specifiers; update both fenced blocks to include a language
(e.g., add ```text after the opening backticks) so static analysis and renderers
apply correct syntax highlighting and the examples display properly.

---

Nitpick comments:
In `@crypto/src/main/java/org/tron/common/crypto/keystore/WalletUtils.java`:
- Around line 34-46: The two wrapper methods generateFullNewWalletFile and
generateLightNewWalletFile declare checked exceptions (NoSuchAlgorithmException,
NoSuchProviderException, InvalidAlgorithmParameterException) that the called
generateNewWalletFile no longer throws (SignUtils.getGeneratedRandomSign is not
throwing checked exceptions); update the method signatures by removing those
three exceptions and only declare the actual checked exceptions thrown (e.g.,
CipherException, IOException) so the throws clauses on generateFullNewWalletFile
and generateLightNewWalletFile match generateNewWalletFile.

In `@plugins/src/main/java/common/org/tron/plugins/KeystoreFactory.java`:
- Around line 105-148: The run() method in KeystoreFactory creates a Scanner
(Scanner in = new Scanner(System.in)) and only closes it on explicit
"exit"/"quit" paths; change run() to use try-with-resources for the Scanner so
it is always closed (even on exceptions/early returns) by wrapping the Scanner
creation in a try(...) block and moving the loop inside that block, removing the
explicit in.close() in the exit/quit branch; this ensures deterministic resource
cleanup while keeping the existing command parsing and methods genKeystore(),
importPrivateKey(), and help() intact.
🪄 Autofix (Beta)

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

Run ID: ed2652be-5726-4b5b-af67-f8c0cba2f3a4

📥 Commits

Reviewing files that changed from the base of the PR and between bb8b4be and 6533628.

📒 Files selected for processing (28)
  • common/src/main/java/org/tron/common/parameter/CommonParameter.java
  • crypto/src/main/java/org/tron/common/crypto/keystore/Credentials.java
  • crypto/src/main/java/org/tron/common/crypto/keystore/Wallet.java
  • crypto/src/main/java/org/tron/common/crypto/keystore/WalletFile.java
  • crypto/src/main/java/org/tron/common/crypto/keystore/WalletUtils.java
  • framework/src/main/java/org/tron/core/Wallet.java
  • framework/src/main/java/org/tron/core/config/args/Args.java
  • framework/src/main/java/org/tron/core/config/args/CLIParameter.java
  • framework/src/main/java/org/tron/core/config/args/WitnessInitializer.java
  • framework/src/main/java/org/tron/core/zen/address/DiversifierT.java
  • framework/src/main/java/org/tron/program/FullNode.java
  • framework/src/test/java/org/tron/common/runtime/vm/PrecompiledContractsVerifyProofTest.java
  • framework/src/test/java/org/tron/core/config/args/ArgsTest.java
  • framework/src/test/java/org/tron/core/config/args/WitnessInitializerTest.java
  • framework/src/test/java/org/tron/core/db/TransactionExpireTest.java
  • framework/src/test/java/org/tron/core/db/TxCacheDBInitTest.java
  • framework/src/test/java/org/tron/core/db/TxCacheDBTest.java
  • framework/src/test/java/org/tron/core/jsonrpc/ApiUtilTest.java
  • framework/src/test/java/org/tron/core/zksnark/SendCoinShieldTest.java
  • framework/src/test/java/org/tron/core/zksnark/ShieldedReceiveTest.java
  • framework/src/test/java/org/tron/keystore/CredentialsTest.java
  • framework/src/test/java/org/tron/keystore/WalletFileTest.java
  • framework/src/test/java/org/tron/keystroe/CredentialsTest.java
  • framework/src/test/java/org/tron/program/SupplementTest.java
  • plugins/README.md
  • plugins/build.gradle
  • plugins/src/main/java/common/org/tron/plugins/KeystoreFactory.java
  • plugins/src/main/java/common/org/tron/plugins/Toolkit.java
💤 Files with no reviewable changes (4)
  • common/src/main/java/org/tron/common/parameter/CommonParameter.java
  • framework/src/test/java/org/tron/core/config/args/ArgsTest.java
  • framework/src/main/java/org/tron/program/FullNode.java
  • framework/src/main/java/org/tron/core/config/args/CLIParameter.java

byte[] privateKey = performCipherOperation(Cipher.DECRYPT_MODE, iv, encryptKey, cipherText);

return SignUtils.fromPrivate(privateKey, Args.getInstance().isECKeyCryptoEngine());
return SignUtils.fromPrivate(privateKey, true);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Preserve crypto-engine selection in decrypt path.

At Line 214, forcing SignUtils.fromPrivate(privateKey, true) always reconstructs EC keys and drops SM2 compatibility. This propagates to credential loading paths that call Wallet.decrypt(...) directly.

💡 Proposed fix (make engine explicit instead of hardcoded)
-  public static SignInterface decrypt(String password, WalletFile walletFile)
+  public static SignInterface decrypt(String password, WalletFile walletFile,
+      boolean isECKeyCryptoEngine)
       throws CipherException {
@@
-    return SignUtils.fromPrivate(privateKey, true);
+    return SignUtils.fromPrivate(privateKey, isECKeyCryptoEngine);
   }

Please also update callers to pass the active engine selection explicitly (for example from runtime config) so SM2 deployments keep working.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crypto/src/main/java/org/tron/common/crypto/keystore/Wallet.java` at line
214, The decrypt path currently hardcodes EC reconstruction via
SignUtils.fromPrivate(privateKey, true) which breaks SM2; change Wallet.decrypt
to accept an explicit crypto-engine flag/enum (e.g., boolean isEc or
CryptoEngine engine) and call SignUtils.fromPrivate(privateKey, engineChoice)
instead of the hardcoded true, then update all callers that invoke
Wallet.decrypt(...) (credential loading paths and any direct calls) to pass the
active engine selection from runtime/config so SM2 deployments preserve their
engine choice.

Comment on lines +50 to +52
throws CipherException, IOException {

SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(),
Args.getInstance().isECKeyCryptoEngine());
SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if generateNewWalletFile is called from production code paths
rg -n "generateNewWalletFile|generateFullNewWalletFile|generateLightNewWalletFile" --type java -g '!**/test/**'

Repository: Federico2014/java-tron

Length of output: 808


🏁 Script executed:

# Find all callers of the three methods across the entire codebase
rg -n "generateNewWalletFile|generateFullNewWalletFile|generateLightNewWalletFile" --type java

Repository: Federico2014/java-tron

Length of output: 1079


🏁 Script executed:

# Examine KeystoreFactory.java to verify the reference
find . -name "KeystoreFactory.java" -type f | head -5

Repository: Federico2014/java-tron

Length of output: 134


🏁 Script executed:

# Check the signature and usage of Args vs CommonParameter for crypto engine config
rg -n "isECKeyCryptoEngine" --type java -B 2 -A 2 | head -50

Repository: Federico2014/java-tron

Length of output: 5666


🏁 Script executed:

# Check KeystoreFactory.java around lines 62-63
sed -n '55,70p' ./plugins/src/main/java/common/org/tron/plugins/KeystoreFactory.java

Repository: Federico2014/java-tron

Length of output: 710


🏁 Script executed:

# Check WalletUtils.java imports and full context
head -60 crypto/src/main/java/org/tron/common/crypto/keystore/WalletUtils.java

Repository: Federico2014/java-tron

Length of output: 2357


🏁 Script executed:

# Check if Args class exists and is used for crypto engine config
rg -n "class Args|isECKeyCryptoEngine" --type java crypto/src/main/java | head -20

Repository: Federico2014/java-tron

Length of output: 1238


Hardcoded true breaks SM2 crypto engine support.

The hardcoded true in generateNewWalletFile() means this method always generates ECKey-based wallets, ignoring the node's configured crypto engine. This is inconsistent with the production pattern shown in KeystoreFactory.java (line 62), which correctly uses CommonParameter.getInstance().isECKeyCryptoEngine().

Although currently only used in tests, this method should respect the configured crypto engine for consistency and to support SM2-configured systems.

🔧 Proposed fix
 public static String generateNewWalletFile(
     String password, File destinationDirectory, boolean useFullScrypt)
     throws CipherException, IOException {
 
-  SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true);
+  SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(),
+      CommonParameter.getInstance().isECKeyCryptoEngine());
   return generateWalletFile(password, ecKeyPair, destinationDirectory, useFullScrypt);
 }

Requires adding: import org.tron.common.parameter.CommonParameter;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
throws CipherException, IOException {
SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(),
Args.getInstance().isECKeyCryptoEngine());
SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true);
import org.tron.common.parameter.CommonParameter;
// ... other code ...
throws CipherException, IOException {
SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(),
CommonParameter.getInstance().isECKeyCryptoEngine());
return generateWalletFile(password, ecKeyPair, destinationDirectory, useFullScrypt);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crypto/src/main/java/org/tron/common/crypto/keystore/WalletUtils.java` around
lines 50 - 52, The generateNewWalletFile() call currently hardcodes
SignUtils.getGeneratedRandomSign(..., true) which forces ECKey wallets; change
it to consult the node crypto engine setting by replacing the literal true with
CommonParameter.getInstance().isECKeyCryptoEngine(), and add the required import
for org.tron.common.parameter.CommonParameter; ensure
SignUtils.getGeneratedRandomSign(...) now receives the dynamic boolean so
SM2-configured systems are respected.

Comment on lines +184 to +193

Keystore files are saved in the `Wallet` directory with the following naming convention:
```
UTC--yyyy-MM-ddTHH-mm-ss.nVV--<address>.json
```

Example:
```
UTC--2024-01-15T10-30-45.123Z--TJRyHRFpmjG6mZ95mZaMvHx5fU7fGvRQoN.json
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add language specifiers to fenced code blocks.

The static analysis tool flagged these code blocks as missing language specifiers. Adding them improves rendering and syntax highlighting in documentation viewers.

📝 Proposed fix
 ### Output:

 Keystore files are saved in the `Wallet` directory with the following naming convention:
-```
+```text
 UTC--yyyy-MM-ddTHH-mm-ss.nVV--<address>.json

Example:
- +text
UTC--2024-01-15T10-30-45.123Z--TJRyHRFpmjG6mZ95mZaMvHx5fU7fGvRQoN.json

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Keystore files are saved in the `Wallet` directory with the following naming convention:
```
UTC--yyyy-MM-ddTHH-mm-ss.nVV--<address>.json
```
Example:
```
UTC--2024-01-15T10-30-45.123Z--TJRyHRFpmjG6mZ95mZaMvHx5fU7fGvRQoN.json
```
Keystore files are saved in the `Wallet` directory with the following naming convention:
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 186-186: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


[warning] 191-191: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/README.md` around lines 184 - 193, The README's fenced code blocks
showing keystore naming (the block containing
"UTC--yyyy-MM-ddTHH-mm-ss.nVV--<address>.json" and the example block
"UTC--2024-01-15T10-30-45.123Z--TJRyHRFpmjG6mZ95mZaMvHx5fU7fGvRQoN.json") are
missing language specifiers; update both fenced blocks to include a language
(e.g., add ```text after the opening backticks) so static analysis and renderers
apply correct syntax highlighting and the examples display properly.

Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

2 issues found across 28 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crypto/src/main/java/org/tron/common/crypto/keystore/WalletUtils.java">

<violation number="1" location="crypto/src/main/java/org/tron/common/crypto/keystore/WalletUtils.java:52">
P1: Do not hardcode the crypto engine to ECKey here; it ignores runtime configuration and can generate keystores incompatible with SM2-configured environments.</violation>
</file>

<file name="crypto/src/main/java/org/tron/common/crypto/keystore/Wallet.java">

<violation number="1" location="crypto/src/main/java/org/tron/common/crypto/keystore/Wallet.java:214">
P2: Hardcoding `true` forces the EC key crypto engine and ignores the runtime configuration, which can cause decrypt/sign behavior to differ from configured expectations.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.


SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(),
Args.getInstance().isECKeyCryptoEngine());
SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true);
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 1, 2026

Choose a reason for hiding this comment

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

P1: Do not hardcode the crypto engine to ECKey here; it ignores runtime configuration and can generate keystores incompatible with SM2-configured environments.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crypto/src/main/java/org/tron/common/crypto/keystore/WalletUtils.java, line 52:

<comment>Do not hardcode the crypto engine to ECKey here; it ignores runtime configuration and can generate keystores incompatible with SM2-configured environments.</comment>

<file context>
@@ -48,11 +47,9 @@ public static String generateLightNewWalletFile(String password, File destinatio
 
-    SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(),
-        Args.getInstance().isECKeyCryptoEngine());
+    SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true);
     return generateWalletFile(password, ecKeyPair, destinationDirectory, useFullScrypt);
   }
</file context>
Suggested change
SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), true);
SignInterface ecKeyPair = SignUtils.getGeneratedRandomSign(Utils.getRandom(), org.tron.common.parameter.CommonParameter.getInstance().isECKeyCryptoEngine());
Fix with Cubic

byte[] privateKey = performCipherOperation(Cipher.DECRYPT_MODE, iv, encryptKey, cipherText);

return SignUtils.fromPrivate(privateKey, Args.getInstance().isECKeyCryptoEngine());
return SignUtils.fromPrivate(privateKey, true);
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 1, 2026

Choose a reason for hiding this comment

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

P2: Hardcoding true forces the EC key crypto engine and ignores the runtime configuration, which can cause decrypt/sign behavior to differ from configured expectations.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crypto/src/main/java/org/tron/common/crypto/keystore/Wallet.java, line 214:

<comment>Hardcoding `true` forces the EC key crypto engine and ignores the runtime configuration, which can cause decrypt/sign behavior to differ from configured expectations.</comment>

<file context>
@@ -212,7 +211,7 @@ public static SignInterface decrypt(String password, WalletFile walletFile)
     byte[] privateKey = performCipherOperation(Cipher.DECRYPT_MODE, iv, encryptKey, cipherText);
 
-    return SignUtils.fromPrivate(privateKey, Args.getInstance().isECKeyCryptoEngine());
+    return SignUtils.fromPrivate(privateKey, true);
   }
 
</file context>
Fix with Cubic

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