tronprotocol / tronprotocol/java-tron
[Feature] Remove --keystore-factory support from FullNode
Nobody has claimed this yet.
- 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:
-
The CLI shell —
KeystoreFactoryinframework/.../program/, a 162-line interactive REPL, and theisKeystoreFactory()dispatch branch inFullNode.main. -
The parameter plumbing — the
--keystore-factorydeclaration inCLIParameter, its assignment inArgs.applyCLIParams, its entry inArgs.getOptionGroup(what lists the flag in--help), and thekeystoreFactoryfield with its Lombok accessors inCommonParameter.
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: deleteKeystoreFactoryand the flag's wiring inFullNodeandArgscommon: remove theCommonParameter.keystoreFactoryfield and its accessorscrypto: remove the now-unreferencedWalletUtils.inputPassword2Twice()helper- CLI: reject
--keystore-factoryinstead of executing it, and drop it from--help - Docs: update the
plugins/README.mdkeystore section — "deprecated" becomes "removed", plus the--sm2note - Messages: correct the
WitnessInitializertip for recovering keystores made by the legacy CLI — it names-p(--private-key) where it means--password. That tip and its counterpart inKeystoreUpdatestay, 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-factorystays a reserved name inCLIParameter, 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.mdkeystore section. Existing keystore files are unaffected — the format is unchanged andToolkit.jar keystorereads 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-fileas well forkeystore import.
Additional Notes
- Do you have ideas regarding implementation? Yes
- Are you willing to implement this feature? Yes
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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