CLI flags silently overridden by config file for 13 parameters

[vividctrlalt]

Software Versions

Version : develop branch (commit 13b68eac0)

Expected behaviour

CLI flags should take priority over config file values. This is the standard convention (CLI > config file > default) and is already correctly implemented for 9 other parameters (e.g. --storage-db-directory, --trust-node) using the Optional pattern.

java -jar FullNode.jar --rpc-thread 16
# Expected: rpcThreadNum = 16

Actual behaviour

For 13 parameters, Args.setParam(Config) unconditionally overwrites the CLI value with the config file value. The CLI flags are silently ignored.

# config.conf contains: node.rpc.thread = 8
java -jar FullNode.jar --rpc-thread 16
# Actual: rpcThreadNum = 8 (CLI value lost)

Root cause — unconditional assignment in Args.setParam():

PARAMETER.rpcThreadNum = config.hasPath(ConfigKey.NODE_RPC_THREAD)
    ? config.getInt(ConfigKey.NODE_RPC_THREAD)
    : Runtime.getRuntime().availableProcessors() / 2;
// JCommander-set value of 16 is overwritten to 8

Frequency

100% — every time a config file key exists for any of the 13 affected parameters.

Steps to reproduce the behaviour

1. Set node.rpc.thread = 8 in config.conf
2. Start node with java -jar FullNode.jar -c config.conf --rpc-thread 16
3. Observe that rpcThreadNum is 8, not 16

Any of the following 13 parameters can reproduce the same issue:

#	CLI Flag	Field	Config Key
1	--support-constant	supportConstant	vm.supportConstant
2	--max-energy-limit-for-constant	maxEnergyLimitForConstant	vm.maxEnergyLimitForConstant
3	--lru-cache-size	lruCacheSize	vm.lruCacheSize
4	--min-time-ratio	minTimeRatio	vm.minTimeRatio
5	--max-time-ratio	maxTimeRatio	vm.maxTimeRatio
6	--long-running-time	longRunningTime	vm.longRunningTime
7	--save-internaltx	saveInternalTx	vm.saveInternalTx
8	--save-featured-internaltx	saveFeaturedInternalTx	vm.saveFeaturedInternalTx
9	--max-connect-number	maxHttpConnectNumber	node.maxHttpConnectNumber
10	--rpc-thread	rpcThreadNum	node.rpc.thread
11	--solidity-thread	solidityThreads	node.solidity.threads
12	--validate-sign-thread	validateSignThreadNum	node.validateSignThreadNum
13	--history-balance-lookup	historyBalanceLookup	storage.balance.history.lookup

Backtrace

Not applicable — no crash or exception. The bug is silent incorrect behavior.

---

Below is the full findings of all 34 @Parameter CLI flags in CommonParameter and additional findings.

full findings: all 34 CLI parameters by category

Category 1: Startup parameters (6) — No issue

Only control startup flow, no config file overlap.

CLI Flag	Field	Description
-c, --config	shellConfFileName	Config file path
-d, --output-directory	outputDirectory	Data directory
--log-config	logbackPath	Log config file
-h, --help	help	Print help and exit
-v, --version	version	Print version and exit
--password	password	Witness password

Category 2: CLI-only parameters (10) — No issue

Only set by CLI, setParam(Config) does not override.

CLI Flag	Field	Description
-w, --witness	witness	Witness mode
-p, --private-key	privateKey	Witness private key
--witness-address	witnessAddress	Witness address
--debug	debug	TVM debug mode
--fast-forward	fastForward	Fast forward mode
--solidity	solidityNode	Solidity node mode
--keystore-factory	keystoreFactory	KeystoreFactory mode
--p2p-disable	p2pDisable	Disable P2P module
--es	eventSubscribe	Enable event subscription
--contract-parse-enable	contractParseEnable	Contract parse switch

Category 3: CLI + Config, CLI priority (9) — Correct

Uses Optional pattern, CLI value takes priority when present.

CLI Flag	Field	Config Key
--storage-db-directory	storageDbDirectory	storage.db.directory
--storage-db-engine	storageDbEngine	storage.db.engine
--storage-db-synchronous	storageDbSynchronous	storage.db.synchronous
--storage-index-directory	storageIndexDirectory	storage.index.directory
--storage-index-switch	storageIndexSwitch	storage.index.switch
--storage-transactionHistory-switch	storageTransactionHistorySwitch	storage.transactionHistory.switch
--trust-node	trustNodeAddr	node.trustNode
--save-cancel-all-unfreeze-v2-details	saveCancelAllUnfreezeV2Details	vm.saveCancelAllUnfreezeV2Details
--seed-nodes	seedNodes	seed.node.ip.list

Category 4: CLI + Config, config overwrites CLI (13) — BUG

See "Steps to reproduce" section above for full list.

Other issues found during investigation

clearParam() misses 9 fields

clearParam() is used for test state reset, but the following fields are not cleared, which may cause test pollution:

• maxEnergyLimitForConstant, lruCacheSize
• allowDelegateOptimization, unfreezeDelayDays
• allowDynamicEnergy, dynamicEnergyThreshold, dynamicEnergyIncreaseFactor, dynamicEnergyMaxFactor
• saveCancelAllUnfreezeV2Details

Inconsistent default value patterns

Two patterns coexist in Args.setParam():

• Pattern A (ternary): default value explicit — config.hasPath(key) ? config.get(key) : defaultValue
• Pattern B (if-block, no else): default value hidden in CommonParameter field declaration

Mixing both increases maintenance burden. Suggest unifying to Pattern A.

CommonParameter overloaded

780 lines, ~150 fields, mixing CLI parameters, config file values, and runtime objects. Long-term suggestion: split into domain-specific classes (RpcConfig, StorageConfig, VmConfig, etc.).

Suggested improvement roadmap

Industry comparison

Project	CLI/Config Overlap	Strategy	Priority Rule
Nginx	Nearly none	CLI for startup ops only; all business config in file	CLI > config
Hadoop	Full overlap	All items overridable via -D key=value; multi-layer XML merge	CLI > site XML > default XML
Kubernetes	Heavy → deprecating CLI	Migrating to config file; CLI flags marked deprecated	CLI > config file

java-tron's current situation is most similar to early Kubernetes.

Short-term (current release): Fix the priority bug

if (PARAMETER.rpcThreadNum <= 0) {
    PARAMETER.rpcThreadNum = config.hasPath(ConfigKey.NODE_RPC_THREAD)
        ? config.getInt(ConfigKey.NODE_RPC_THREAD)
        : Runtime.getRuntime().availableProcessors() / 2;
}

Mid-term (next release): Deprecate overlapping CLI flags

@Deprecated  // Use node.rpc.thread in config.conf instead
@Parameter(names = {"--rpc-thread"}, description = "[DEPRECATED] Num of gRPC thread")
public int rpcThreadNum;

Long-term (future release): Converge to Nginx model

Only keep ~8 startup-class CLI parameters (-c, -d, -w, -p, -h, -v, --password, --log-config). All other runtime parameters managed exclusively by config file.

[waynercheung]

@vividctrlalt
Thank you for this thorough investigation!
We've verified the root cause in the codebase — setParam(Config) does indeed unconditionally overwrite JCommander-parsed CLI values for all 13 parameters, while 9 others correctly use the guard-check pattern.

This is a confirmed bug. We can discuss the fix approach below and have two questions before opening a PR.

Fix approach

Group 1 — Numeric parameters with zero-initialized defaults
(rpcThreadNum, solidityThreads, validateSignThreadNum, minTimeRatio)

These fields carry no explicit initializer (or are initialized to 0/0.0), so a <= 0 sentinel in setParam() is sufficient:

  // Example: rpcThreadNum
  if (PARAMETER.rpcThreadNum <= 0) {
      PARAMETER.rpcThreadNum = config.hasPath(Constant.NODE_RPC_THREAD)
          ? config.getInt(Constant.NODE_RPC_THREAD)
          : (Runtime.getRuntime().availableProcessors() + 1) / 2;
  }

Group 2 — Numeric parameters with non-zero defaults
(maxHttpConnectNumber = 50, longRunningTime = 10, lruCacheSize = 500, maxEnergyLimitForConstant = 100_000_000L, maxTimeRatio = calcMaxTimeRatio())

For these fields, a bare <= 0 guard would always be false against the field's initial value, silently breaking config loading entirely — worse than the current bug. The correct fix requires three coordinated changes:

1. Change the field initializer to a sentinel value (0 or -1.0) so that "CLI not provided" is reliably detectable.
2. Move the original default value into the setParam() fallback branch.
3. Update clearParam() to reset these fields to their sentinel values (not the runtime defaults), or the fix breaks in test environments.

  // CommonParameter.java
  public int    lruCacheSize              = 0;    // sentinel; was 500
  public int    longRunningTime           = 0;    // sentinel; was 10
  public int    maxHttpConnectNumber      = 0;    // sentinel; was 50
  public long   maxEnergyLimitForConstant = 0L;   // sentinel; was 100_000_000L
  public double maxTimeRatio              = -1.0; // sentinel; was calcMaxTimeRatio()

  // Args.java — sentinel guard, original default preserved in fallback
  if (PARAMETER.lruCacheSize <= 0) {
      PARAMETER.lruCacheSize = config.hasPath(Constant.VM_LRU_CACHE_SIZE)
          ? config.getInt(Constant.VM_LRU_CACHE_SIZE)
          : 500;
  }

  if (PARAMETER.maxTimeRatio < 0) {
      PARAMETER.maxTimeRatio = config.hasPath(Constant.VM_MAX_TIME_RATIO)
          ? config.getDouble(Constant.VM_MAX_TIME_RATIO)
          : calcMaxTimeRatio();
  }

Group 3 — Boolean flag parameters
(supportConstant, saveInternalTx, saveFeaturedInternalTx, historyBalanceLookup)

All four are declared with JCommander's default arity = 0. Under this model, there is no CLI syntax to explicitly pass false — the flag is either present (true) or absent (field retains its default false). Therefore false unambiguously means "CLI not provided", and a if (!PARAMETER.field) guard is the correct and complete fix with no loss of expressiveness:

  if (!PARAMETER.saveInternalTx && config.hasPath(Constant.VM_SAVE_INTERNAL_TX)) {
      PARAMETER.saveInternalTx = config.getBoolean(Constant.VM_SAVE_INTERNAL_TX);
  }

Upgrading to Boolean (nullable) + arity = 1 to support explicit --save-internaltx false would be a separate enhancement, out of scope for this fix.

Questions

1. Scope of PR: Should we address all 13 parameters in a single PR, or split them
   (e.g., Group 1 first as a low-risk change, Group 2 + Group 3 separately)?
2. clearParam() gaps: The Group 2 fix already updates clearParam() for maxEnergyLimitForConstant and lruCacheSize (resetting them to sentinel values).
   Should we also address the remaining 7 fields you identified as missing (allowDelegateOptimization, unfreezeDelayDays, allowDynamicEnergy, dynamicEnergyThreshold, dynamicEnergyIncreaseFactor, dynamicEnergyMaxFactor, saveCancelAllUnfreezeV2Details) in the same PR, or track them as a separate issue?

[waynercheung]

Great point — this is a legitimate operational concern worth addressing explicitly.

On the backward-compatibility risk

The affected population is users who simultaneously:

1. Pass one of the 13 CLI flags in their startup script, and
2. Have the same key in config.conf with a different value, and
3. Have unknowingly been relying on the (buggy) config-wins behavior

In practice this is a narrow set, since operators who discovered these CLI flags had no effect would have stopped using them. That said, the risk is real and deserves a mitigation.

Agreed: add an INFO-level log when CLI overrides config

We can log whenever a CLI-supplied value takes precedence over a config file value, making the override observable without requiring any config or log-level changes:

// Example for rpcThreadNum
if (PARAMETER.rpcThreadNum > 0) {
    if (config.hasPath(Constant.NODE_RPC_THREAD)
            && config.getInt(Constant.NODE_RPC_THREAD) != PARAMETER.rpcThreadNum) {
        logger.info("CLI flag --rpc-thread ({}) overrides config file value ({})",
            PARAMETER.rpcThreadNum, config.getInt(Constant.NODE_RPC_THREAD));
    }
} else {
    PARAMETER.rpcThreadNum = config.hasPath(Constant.NODE_RPC_THREAD)
        ? config.getInt(Constant.NODE_RPC_THREAD)
        : (Runtime.getRuntime().availableProcessors() + 1) / 2;
}

This pattern also applies to the 9 parameters that already work correctly today — consistent logging across all CLI-overridable parameters improves long-term observability.

Agreed: document as a behavior change in release notes

We can add an explicit entry such as:

Behavior change: CLI flags for 13 parameters now correctly take precedence over config file values (fixing a bug where config silently overwrote CLI).
Affected flags:

  --rpc-thread, --solidity-thread, --validate-sign-thread,
  --max-connect-number, --lru-cache-size, --long-running-time,
  --max-energy-limit-for-constant, --min-time-ratio, --max-time-ratio,
  --support-constant, --save-internaltx, --save-featured-internaltx,
  --history-balance-lookup

Action required for operators: review your startup scripts and verify that any of the above CLI flags reflect your intended configuration before upgrading.

Does this approach address your concern?

[bladehan1]

Problem Statement

In command-line parsing, we often need to distinguish whether a user has explicitly specified a parameter, in order to decide whether to use the CLI value or fall back to a configuration file or other default source. Relying solely on the parameter's default value (e.g., 0 for int or false for boolean) is insufficient because it cannot differentiate between "user did not specify" and "user specified the same value as the default". This leads to ambiguity in logic.

---

Solutions and Evaluation

Solution 1: Setter + Boolean Flag

Add a corresponding boolean flag for each parameter that needs tracking, and set it to true inside the setter.

public class Args {
    private boolean rpcThreadNumSet = false;
    private int rpcThreadNum;

    @Parameter(names = "--rpc-thread")
    public void setRpcThreadNum(int num) {
        this.rpcThreadNum = num;
        this.rpcThreadNumSet = true;
    }
}

Pros:

• Accurately distinguishes whether the parameter was explicitly specified, unaffected by default values.
• Keeps fields as primitive types, avoiding wrapper classes.
• Clear and easy to understand.

Cons:

• Requires extra setter and flag for each parameter, adding some boilerplate.
• If fields were originally public, they need to be made private with getters/setters, potentially impacting existing code style.

---

Solution 2: Use Wrapper Types Initialized to null

Replace primitive types with their corresponding wrapper types (e.g., Integer, Boolean) and leave them as null by default. After parsing, if the value is null, it means the user did not specify it.

public class Args {
    @Parameter(names = "--rpc-thread")
    public Integer rpcThreadNum;  // defaults to null
}

Pros:

• Concise, no extra flags needed.
• null naturally represents "not specified", making it easy to merge with configuration values.

Cons:

• Forces the use of wrapper types, which may introduce null checks and potential NullPointerException risks.
• If null is a valid business value, additional handling is required (e.g., using a different sentinel value).
• For boolean, Boolean works similarly, but note that JCommander handles flag-style parameters differently (value-bearing parameters need arity=1).

---

Solution 3: Use JCommander's ParameterDescription.isAssigned()

After parsing, query the JCommander object to check whether a parameter was explicitly assigned.

JCommander jc = JCommander.newBuilder().addObject(args).build();
jc.parse(cmdArgs);
boolean assigned = jc.getParameters().stream()
        .anyMatch(pd -> pd.getLongestName().equals("--rpc-thread") && pd.isAssigned());

Pros:

• Does not modify the parameter class, keeping the POJO clean.
• Can centrally process the assignment status of all parameters.

Cons:

• Requires additional mapping code, handling aliases can be cumbersome.
• Tighter coupling with JCommander, higher maintenance overhead.
• If the parameter class is complex, it's error-prone and less controllable.

Conclusion: This approach is more complex and less controllable, so it's generally not recommended as a primary solution.

---

Solution 4: Hybrid Approach (Per-Field Strategy)

For parameters whose default value has no business meaning (e.g., thread count can never be 0), you can use the primitive default value (0) to indicate "not specified". For parameters where the default overlaps with a legitimate value, adopt Solution 1 or 2.

Pros:

• Tailored to specific cases, reducing unnecessary code.

Cons:

• Breaks consistency; different fields handled differently can lead to confusion.
• Future requirement changes may force re-evaluation of what default values mean, increasing maintenance costs.

---

Personal Recommendation

From the perspective of consistency and maintainability, I recommend adopting Solution 1 (Setter + Boolean Flag) uniformly. It accurately tracks explicit specification without introducing null-related risks, and integrates seamlessly with existing primitive-typed fields. While it adds a bit of boilerplate, the readability is high.

---

[vividctrlalt]

Root Cause Analysis

The current CommonParameter class has three responsibilities mixed together:

1. Default value definition (field initializers)
2. CLI parameter definition (JCommander @Parameter annotations)
3. Runtime parameter storage (getters/setters)

This leads to a critical problem: after JCommander parses CLI arguments, values are written directly into CommonParameter fields. Then Args.setParam(Config) loads values from the config file and unconditionally overwrites these 13 fields — the CLI values set by the user are silently lost.

Standard Practice

Parameter initialization should follow a three-layer override model, from lowest to highest priority:

Default (code defaults) → Config (config file) → CLI (command-line arguments)

Each layer only overrides values from the layer below it. Explicit values from a higher-priority layer should never be overwritten by a lower-priority one. This is the standard approach used by Hadoop (-D overrides site XML overrides default XML), Nginx, and other major projects.

Fix

PR #6569 restructures the parameter initialization flow, establishing a Default → Config → CLI three-layer override architecture that eliminates this issue at the root.

It also fixes the clearParam() problem mentioned in this issue, where 9 fields were not being reset. Default values are now defined uniformly via CommonParameter field initializers, and clearParam() resets state by simply creating a new CommonParameter(), eliminating the possibility of missed fields entirely.

[halibobo1205]

@vividctrlalt Please Note

--seed-nodes CLI flag has no effect

Cause

The @Parameter annotation uses description instead of names to define the flag:

// Current (broken)
@Parameter(description = "--seed-nodes")
public List<String> seedNodes = new ArrayList<>();

// Fixed
@Parameter(names = {"--seed-nodes"}, description = "Seed nodes list")
public List<String> seedNodes = new ArrayList<>();

In JCommander:

• names — defines the CLI flag (used for parsing)
• description — defines the help text (display only)

Behavior

Command	Result
--seed-nodes 127.0.0.1:18888	❌ Ignored — flag is not recognized
127.0.0.1:18888 (positional)	✅ Assigned to seedNodes

Without names, JCommander treats this field as a main parameter, which collects remaining positional arguments instead of a named flag.

[waynercheung]

PR #6569 restructures the parameter initialization flow, establishing a Default → Config → CLI three-layer override architecture that eliminates this issue at the root.

@vividctrlalt Missing regression test for #6567 (CLI should override config for the 13 affected params).

The new init flow (applyConfigParams then applyCLIParams with isAssigned) looks like it fixes the root cause.
However, I don’t see a direct regression test that reproduces the issue from #6567: config defines a key but CLI must still win (e.g. node.rpc.thread=8 + --rpc-thread 16).

Please add a minimal test that writes a temp config with a couple of those keys and asserts CLI takes precedence. This will prevent future refactors from reintroducing silent overrides.