Skip to content

fix(config): deprecate CLI params and add guardrails#6580

Open
vividctrlalt wants to merge 2 commits intotronprotocol:developfrom
vividctrlalt:fix/pr6569-review-followup
Open

fix(config): deprecate CLI params and add guardrails#6580
vividctrlalt wants to merge 2 commits intotronprotocol:developfrom
vividctrlalt:fix/pr6569-review-followup

Conversation

@vividctrlalt
Copy link
Contributor

Summary

Follow-up to #6569 review feedback:

  • Deprecate 23 CLI parameters that can be replaced by config-file options, marking them with @Deprecated in CLIParameter and logging runtime warnings via reflection when used
  • Add max(3_000_000L, ...) guardrail for --max-energy-limit-for-constant CLI path, consistent with config-file validation (addresses @waynercheung's review in refactor(config): extract CLIParameter and restructure Args init flow #6569)
  • Document seedNodes positional parameter behavior with comment explaining why isEmpty() is used instead of isAssigned(), plus deprecation warning

Retained CLI params (11)

-c, -d, --log-config, -h, -v, -w, -p, --witness-address, --password, --keystore-factory, --debug

Deprecated CLI params (23)

All storage, VM, network/thread, and misc runtime parameters — these should be configured via config file.

Test plan

Related

…rotocol#6569 review follow-up

- Mark 23 config-file-replaceable CLI parameters as @deprecated in CLIParameter
- Add runtime deprecation warning via reflection when deprecated CLI options are used
- Add max(3_000_000L, ...) guardrail for --max-energy-limit-for-constant CLI path,
  consistent with config-file validation (addresses waynercheung's review feedback)
- Document seedNodes positional parameter behavior and add deprecation warning
- Retain 11 essential CLI params: -c, -d, --log-config, -h, -v, -w, -p,
  --witness-address, --password, --keystore-factory, --debug
@vividctrlalt vividctrlalt changed the title fix(config): deprecate CLI params and add safety guardrails (#6569 follow-up) fix(config): deprecate CLI params and add guardrails Mar 16, 2026
// isAssigned(). An empty-check is used instead — this is safe because the default
// is an empty list, so non-empty means the user explicitly passed values on CLI.
if (!cmd.seedNodes.isEmpty()) {
logger.warn("Positional seed-node arguments are deprecated. "
Copy link

Choose a reason for hiding this comment

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

Since seedNodes is deprecated, the corresponding help info should also be removed, such as: helpStr.append(String.format("%nUsage: java -jar %s [options] [seedNode ...]%n", programName));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback! Since seedNodes is deprecated but not yet removed, users may still want to use it, so I kept the Usage line but added a deprecation note right below it. Also noticed that --es and --solidity in the help output are deprecated too (marked in this PR), so I added (deprecated) hints to all deprecated options in the help text. See 84027da.

Mark deprecated options with "(deprecated)" in help text and add a
note below the Usage line that positional seedNode args are deprecated.
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.

2 participants