Skip to content

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

Description

@0xbigapple

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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    • Status
      No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions