fix(config): deprecate CLI params and add guardrails #6580
+144
−5
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| import static org.tron.core.Constant.DEFAULT_PROPOSAL_EXPIRE_TIME; | ||
| import static org.tron.core.Constant.DYNAMIC_ENERGY_INCREASE_FACTOR_RANGE; | ||
| import static org.tron.core.Constant.DYNAMIC_ENERGY_MAX_FACTOR_RANGE; | ||
| import static org.tron.core.Constant.ENERGY_LIMIT_IN_CONSTANT_TX; | ||
| import static org.tron.core.Constant.MAX_PROPOSAL_EXPIRE_TIME; | ||
| import static org.tron.core.Constant.MIN_PROPOSAL_EXPIRE_TIME; | ||
| import static org.tron.core.config.Parameter.ChainConstant.BLOCK_PRODUCE_TIMEOUT_PERCENT; | ||
|
|
@@ -83,6 +84,40 @@ | |
| @Component | ||
| public class Args extends CommonParameter { | ||
|
|
||
| /** | ||
| * Maps deprecated CLI option names to their config-file equivalents. | ||
| * Options not in this map have no config equivalent and are being removed entirely. | ||
| */ | ||
| private static final Map<String, String> DEPRECATED_CLI_TO_CONFIG; | ||
|
|
||
| static { | ||
| Map<String, String> m = new HashMap<>(); | ||
| m.put("--storage-db-directory", "storage.db.directory"); | ||
| m.put("--storage-db-engine", "storage.db.engine"); | ||
| m.put("--storage-db-synchronous", "storage.db.sync"); | ||
| m.put("--storage-index-directory", "storage.index.directory"); | ||
| m.put("--storage-index-switch", "storage.index.switch"); | ||
| m.put("--storage-transactionHistory-switch", "storage.transHistory.switch"); | ||
| m.put("--contract-parse-enable", "event.subscribe.contractParse"); | ||
| m.put("--support-constant", "vm.supportConstant"); | ||
| m.put("--max-energy-limit-for-constant", "vm.maxEnergyLimitForConstant"); | ||
| m.put("--lru-cache-size", "vm.lruCacheSize"); | ||
| m.put("--min-time-ratio", "vm.minTimeRatio"); | ||
| m.put("--max-time-ratio", "vm.maxTimeRatio"); | ||
| m.put("--save-internaltx", "vm.saveInternalTx"); | ||
| m.put("--save-featured-internaltx", "vm.saveFeaturedInternalTx"); | ||
| m.put("--save-cancel-all-unfreeze-v2-details", "vm.saveCancelAllUnfreezeV2Details"); | ||
| m.put("--long-running-time", "vm.longRunningTime"); | ||
| m.put("--max-connect-number", "node.maxHttpConnectNumber"); | ||
| m.put("--rpc-thread", "node.rpc.thread"); | ||
| m.put("--solidity-thread", "node.solidity.threads"); | ||
| m.put("--validate-sign-thread", "node.validateSignThreadNum"); | ||
| m.put("--trust-node", "node.trustNode"); | ||
| m.put("--history-balance-lookup", "storage.balance.history.lookup"); | ||
| m.put("--es", "event.subscribe.enable"); | ||
| DEPRECATED_CLI_TO_CONFIG = Collections.unmodifiableMap(m); | ||
| } | ||
|
|
||
| @Getter | ||
| private static String configFilePath = ""; | ||
|
|
||
|
|
@@ -152,7 +187,7 @@ public static void applyConfigParams( | |
|
|
||
| if (config.hasPath(ConfigKey.VM_MAX_ENERGY_LIMIT_FOR_CONSTANT)) { | ||
| long configLimit = config.getLong(ConfigKey.VM_MAX_ENERGY_LIMIT_FOR_CONSTANT); | ||
| PARAMETER.maxEnergyLimitForConstant = max(ENERGY_LIMIT_IN_CONSTANT_TX, configLimit, true); | ||
| } | ||
|
|
||
| if (config.hasPath(ConfigKey.VM_LRU_CACHE_SIZE)) { | ||
|
|
@@ -661,6 +696,9 @@ public static void applyConfigParams( | |
| PARAMETER.eventFilter = | ||
| config.hasPath(ConfigKey.EVENT_SUBSCRIBE_FILTER) ? getEventFilter(config) : null; | ||
|
|
||
| PARAMETER.eventSubscribe = config.hasPath(ConfigKey.EVENT_SUBSCRIBE_ENABLE) | ||
| && config.getBoolean(ConfigKey.EVENT_SUBSCRIBE_ENABLE); | ||
|
There was a problem hiding this comment. It would be helpful to add test cases for the newly introduced
Contributor
Author
There was a problem hiding this comment. Added two test cases in
See 5bfa482. |
||
|
|
||
| if (config.hasPath(ConfigKey.ALLOW_SHIELDED_TRANSACTION_API)) { | ||
| PARAMETER.allowShieldedTransactionApi = | ||
| config.getBoolean(ConfigKey.ALLOW_SHIELDED_TRANSACTION_API); | ||
|
|
@@ -998,6 +1036,28 @@ private static void applyCLIParams(CLIParameter cmd, JCommander jc) { | |
| .map(ParameterDescription::getLongestName) | ||
| .collect(Collectors.toSet()); | ||
|
|
||
| jc.getParameters().stream() | ||
| .filter(ParameterDescription::isAssigned) | ||
| .filter(pd -> { | ||
| try { | ||
| return CLIParameter.class.getDeclaredField(pd.getParameterized().getName()) | ||
| .isAnnotationPresent(Deprecated.class); | ||
| } catch (NoSuchFieldException e) { | ||
| return false; | ||
| } | ||
| }) | ||
| .forEach(pd -> { | ||
| String cliOption = pd.getLongestName(); | ||
| String configKey = DEPRECATED_CLI_TO_CONFIG.get(cliOption); | ||
| if (configKey != null) { | ||
| logger.warn("CLI option '{}' is deprecated and will be removed in a future release." | ||
| + " Please use config key '{}' instead.", cliOption, configKey); | ||
| } else { | ||
| logger.warn("CLI option '{}' is deprecated and will be removed in a future release.", | ||
| cliOption); | ||
| } | ||
| }); | ||
|
|
||
| if (assigned.contains("--output-directory")) { | ||
| PARAMETER.outputDirectory = cmd.outputDirectory; | ||
| } | ||
|
|
@@ -1008,7 +1068,8 @@ private static void applyCLIParams(CLIParameter cmd, JCommander jc) { | |
| PARAMETER.supportConstant = cmd.supportConstant; | ||
| } | ||
| if (assigned.contains("--max-energy-limit-for-constant")) { | ||
| PARAMETER.maxEnergyLimitForConstant = max(ENERGY_LIMIT_IN_CONSTANT_TX, | ||
| cmd.maxEnergyLimitForConstant, true); | ||
| } | ||
| if (assigned.contains("--lru-cache-size")) { | ||
| PARAMETER.lruCacheSize = cmd.lruCacheSize; | ||
|
|
@@ -1091,7 +1152,12 @@ private static void applyCLIParams(CLIParameter cmd, JCommander jc) { | |
| if (assigned.contains("--log-config")) { | ||
| PARAMETER.logbackPath = cmd.logbackPath; | ||
| } | ||
| // seedNodes is a JCommander positional (main) parameter, which does not support | ||
| // 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. " | ||
|
There was a problem hiding this comment. Since
Contributor
Author
There was a problem hiding this comment. Thanks for the feedback! Since |
||
| + "Please use seed.node.ip.list in the config file instead."); | ||
| List<InetSocketAddress> seeds = new ArrayList<>(); | ||
| for (String s : cmd.seedNodes) { | ||
| seeds.add(NetUtil.parseInetSocketAddress(s)); | ||
|
|
@@ -1677,6 +1743,9 @@ public static void printHelp(JCommander jCommander) { | |
| jCommander.getProgramName(); | ||
| helpStr.append(String.format("%nUsage: java -jar %s [options] [seedNode <seedNode> ...]%n", | ||
| programName)); | ||
| helpStr.append(String.format( | ||
| "%nNote: Positional seedNode arguments are deprecated." | ||
| + " Use seed.node.ip.list in the config file instead.%n")); | ||
| helpStr.append(String.format("%nVERSION: %n%s-%s%n", Version.getVersion(), | ||
| getCommitIdAbbrev())); | ||
|
|
||
|
|
@@ -1698,9 +1767,21 @@ public static void printHelp(JCommander jCommander) { | |
| logger.warn("Miss option:{}", option); | ||
| continue; | ||
| } | ||
| boolean isDeprecated; | ||
| try { | ||
| isDeprecated = CLIParameter.class.getDeclaredField( | ||
| parameterDescription.getParameterized().getName()) | ||
| .isAnnotationPresent(Deprecated.class); | ||
| } catch (NoSuchFieldException e) { | ||
| isDeprecated = false; | ||
| } | ||
| String desc = upperFirst(parameterDescription.getDescription()); | ||
| if (isDeprecated) { | ||
| desc += " (deprecated)"; | ||
| } | ||
| String tmpOptionDesc = String.format("%s\t\t\t%s%n", | ||
| Strings.padEnd(parameterDescription.getNames(), optionMaxLength, ' '), | ||
| desc); | ||
| helpStr.append(tmpOptionDesc); | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable
private static final String DB_DIRECTORY_CONFIG_KEY = "storage.db.directory";has already been defined and does not need to be redefined; otherwise, it will be difficult to maintain. It should be set to public.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review! These string literals in the map are user-facing hint text shown in deprecation warnings, not config-parsing keys used in code logic. While
Storage.javadoes define similar constants (e.g.DB_DIRECTORY_CONFIG_KEY), they are intentionallyprivate— and I'd prefer not to change their visibility just to serve a deprecated-params map that will eventually be removed along with the CLI params themselves.Additionally, the config key constants should be further modularized by domain in the future (storage, vm, node, etc.), so coupling them across classes now would work against that direction.