Skip to content

feat(framework): remove --keystore-factory support from FullNode - #6978

Open
0xbigapple wants to merge 6 commits into
tronprotocol:release_v4.8.3from
0xbigapple:feature/remove-keystore-factory
Open

0xbigapple wants to merge 6 commits into
tronprotocol:release_v4.8.3from
0xbigapple:feature/remove-keystore-factory

Conversation

@0xbigapple

@0xbigapple 0xbigapple commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

close #6949
Removes the deprecated --keystore-factory CLI from FullNode.jar. Keystore management moved to Toolkit.jar keystore <new|import|list|update> in GreatVoyage-v4.8.2 (#6637); this PR deletes the legacy shim left behind:

  • KeystoreFactory and its dispatch branch in FullNode.main
  • the CommonParameter.keystoreFactory field and the Args parameter plumbing
  • KeystoreFactoryDeprecationTest (its subject is gone)

The option stays declared in CLIParameter as a tombstone: passing it now exits with status 1 and prints the Toolkit replacement, including an --sm2 note for SM2 nodes — the legacy mode followed crypto.engine, while the Toolkit commands default to ECDSA. Undeclaring the option instead would let JCommander parse the flag as a positional seed-node address and fail with a misleading error.

Why are these changes required?

FullNode.jar is the node binary; it should not ship an interactive key-management REPL that reads private keys and passwords from stdin now that Toolkit.jar keystore is the supported implementation of the same operations. Two implementations drift. Deprecation shipped in GreatVoyage-v4.8.2, giving one full release of warning before removal.

This PR has been tested by:

  • Unit Tests: ArgsTest updated to stop passing the flag; keystore library tests (org.tron.keystore, 60 tests) unchanged and passing
  • Manual Testing:
    • FullNode.jar --keystore-factory → exit 1, stderr names the Toolkit replacement
    • FullNode.jar --keystore-factory --help → exit 0, prints normal help; --help no longer lists the flag
    • Private chain: a witness configured via localwitnesskeystore + --password loads the keystore and produces blocks — the witness keystore startup path is unaffected

Follow up

Extra details

Breaking change: java -jar FullNode.jar --keystore-factory stops working. Keystore file format and existing keystores are unaffected. The public KeystoreFactory class and the CommonParameter.keystoreFactory Lombok accessors are removed with it — internal API, source-level breakage only for out-of-tree code compiling against these modules.

WalletUtils.inputPassword2Twice() is removed as well: its last callers were the REPL's GenKeystore/ImportPrivateKey commands, and the Toolkit keystore commands implement their own password confirmation.

Behavior change worth a release note: the legacy REPL read from plain stdin, so a private key could be piped in (printf ... | java -jar FullNode.jar --keystore-factory) without ever being written to disk. Toolkit keystore import does not read stdin, and --key-file accepts only a regular file (symlinks and FIFOs are rejected by design), so non-interactive imports now require the plaintext key in a file on disk. Create it with restrictive permissions and delete it immediately after the import:

(umask 077; printf '%s\n' "$PRIVATE_KEY" > key.txt)
java -jar Toolkit.jar keystore import --key-file key.txt --password-file pass.txt
shred -u key.txt 2>/dev/null || rm -f key.txt

- delete KeystoreFactory and its parameter plumbing; keep the flag as a
  tombstone that exits 1 with a Toolkit migration hint
- update plugins/README.md keystore migration notes
- detect the flag via isAssigned so a repeated flag cannot bypass the exit
- replace raw exit(1) with TronError(PARAMETER_INIT) so the shutdown is logged
- reword the SM2 hint: --sm2 applies to commands that create or modify a keystore
- note on the tombstone parameter why the declaration must stay
- ArgsTest: assert TronError, add repeated-flag case, class-level @after clearParam
Its last callers were deleted with the --keystore-factory REPL;
Toolkit implements its own password confirmation.
- drop the flag name from the witness keystore recovery tip
- test the tombstone against an invalid config path
@0xbigapple
0xbigapple requested review from lxcmyf and waynercheung and removed request for xxo1shine September 17, 2026 07:23
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