tronprotocol / tronprotocol/java-tron

[Feature] Remove --keystore-factory support from FullNode

Open
#6,949 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

type:feature
Dominant language
Java
Stars
4.2k
Forks
1.7k
Avg merge
6d 20h
Merged PRs (30d)
14

Description

Summary

Keystore management moved from FullNode.jar to Toolkit.jar in GreatVoyage-v4.8.2 (#6637), leaving --keystore-factory behind as a deprecated legacy entry point that still runs the full prompt. Remove it, keeping the flag recognized only so it can name the replacement command.

Only the CLI is removed, along with the prompt helper left without a caller. The shared org.tron.keystore library stays.

Problem

Motivation

Removing the flag was part of the original plan (#6603), deferred to give operators a release of warning.

Current State

Passing --keystore-factory still works: FullNode.main hands off to KeystoreFactory, which prints a deprecation notice and then runs its GenKeystore / ImportPrivateKey prompt. The node itself never starts. That behavior rests on two pieces:

  1. The CLI shellKeystoreFactory in framework/.../program/, a 162-line interactive REPL, and the isKeystoreFactory() dispatch branch in FullNode.main.

  2. The parameter plumbing — the --keystore-factory declaration in CLIParameter, its assignment in Args.applyCLIParams, its entry in Args.getOptionGroup (what lists the flag in --help), and the keystoreFactory field with its Lombok accessors in CommonParameter.

Limitations or Risks

The legacy CLI duplicates functionality that Toolkit.jar keystore already provides. New features and fixes go to the Toolkit commands, so the two drift further apart with every release the flag survives.

Proposed Solution

Proposed Design

1. Delete the shell and its plumbing — the KeystoreFactory class, the dispatch branch in FullNode.main, the CommonParameter field and the Args wiring; only the CLIParameter declaration stays. The flag also stops appearing in --help.

2. Keep the option declared as a tombstone — passing it prints the Toolkit.jar keystore replacement and stops instead of running anything. The declaration has to stay: JCommander treats an argument as an option only when it matches a declared @Parameter, and anything else falls through to the positional seed-node list, so an undeclared --keystore-factory would be read as a seed-node address and fail with an unrelated error. The message carries the --sm2 note for SM2 nodes, since the Toolkit commands default to ECDSA while the legacy REPL followed the node config; plugins/README.md gets the same note.

3. Adjust the tests — the deprecation test class is removed along with the class it covers, and new cases cover the rejection.

Key Changes
  • framework: delete KeystoreFactory and the flag's wiring in FullNode and Args
  • common: remove the CommonParameter.keystoreFactory field and its accessors
  • crypto: remove the now-unreferenced WalletUtils.inputPassword2Twice() helper
  • CLI: reject --keystore-factory instead of executing it, and drop it from --help
  • Docs: update the plugins/README.md keystore section — "deprecated" becomes "removed", plus the --sm2 note
  • Messages: correct the WitnessInitializer tip for recovering keystores made by the legacy CLI — it names -p (--private-key) where it means --password. That tip and its counterpart in KeystoreUpdate stay, since they describe keystore files already on disk

Impact

  • Security: the opt-in key-management REPL leaves the node binary. The witness-keystore password prompt at node start (WitnessInitializer) is a separate path and keeps working as before.
  • Developer Experience: one keystore CLI to document and maintain instead of two.
  • Maintenance: --keystore-factory stays a reserved name in CLIParameter, and the declaration has to be carried until the positional seed-node parameter goes away.

Compatibility

Breaking Change: Yes. java -jar FullNode.jar --keystore-factory stops working, printing a message that names the Toolkit replacement.

Default Behavior Change: None. The flag is opt-in, and passing it never started a node in the first place.

Migration Required: Yes, for anyone still invoking the flag.

  • The command mapping is documented in the plugins/README.md keystore section. Existing keystore files are unaffected — the format is unchanged and Toolkit.jar keystore reads them.
  • Piped stdin does not carry over: the REPL took its answers that way, the Toolkit commands do not. In a non-interactive environment, pass --password-file, and --key-file as well for keystore import.

Additional Notes

  • Do you have ideas regarding implementation? Yes
  • Are you willing to implement this feature? Yes

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with KeystoreFactory in framework/.../program/ and the isKeystoreFactory() branch in FullNode.main, then trace the --keystore-factory declaration and wiring through CLIParameter, Args, and CommonParameter. Review the related deprecation tests and plugins/README.md; done means the legacy shell and plumbing are removed, the flag is rejected with the Toolkit replacement and --sm2 note, help no longer lists it, and the new rejection cases pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
cli, documentation
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.