Skip to content

Commit 434d917

Browse files
committed
fix(config): honor legacy fullNodeAllowShieldedTransaction key
Address PR review from lvs0075 (2026-04-22, reference.conf:218). reference.conf ships `allowShieldedTransactionApi = true`, so after the HOCON `withFallback` merge, `section.hasPath("allowShieldedTransactionApi")` is always true regardless of what the user wrote. That made the legacy-key compatibility branch in NodeConfig.fromConfig() dead code: a user who wrote only `node.fullNodeAllowShieldedTransaction = false` in their config.conf silently got `true`, unintentionally enabling the shielded transaction API. Fix: replace the unreachable else-if chain with a direct override. The legacy key is intentionally not defaulted in reference.conf, so `hasPath` on it reliably means "user supplied it". Same pattern as maxActiveNodesWithSameIp. Also restores the deprecation warning develop's Args.java emits when a user still uses the legacy key; this PR dropped it along with the rewrite. Regression tests: - testShieldedApiDefaultsToTrueWhenNeitherKeySet - testShieldedApiModernKeyRespected - testShieldedApiLegacyKeyRespected (guards the reported bug) - testShieldedApiLegacyKeyTakesPriorityOverModern (mirrors testLegacyAliasTakesPriorityOverModernKey)
1 parent a4cfe65 commit 434d917

2 files changed

Lines changed: 40 additions & 4 deletions

File tree

common/src/main/java/org/tron/core/config/args/NodeConfig.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -391,11 +391,13 @@ public static NodeConfig fromConfig(Config config) {
391391
nc.maxConnectionsWithSameIp = section.getInt("maxActiveNodesWithSameIp");
392392
}
393393

394-
// Legacy key fallback: node.fullNodeAllowShieldedTransaction -> allowShieldedTransactionApi
395-
if (section.hasPath("allowShieldedTransactionApi")) {
396-
nc.allowShieldedTransactionApi = section.getBoolean("allowShieldedTransactionApi");
397-
} else if (section.hasPath("fullNodeAllowShieldedTransaction")) {
394+
// Legacy key fallback: node.fullNodeAllowShieldedTransaction -> allowShieldedTransactionApi.
395+
// reference.conf does not ship the legacy key, so hasPath here reliably means the user
396+
// set it in their config. When present, it overrides the modern key.
397+
if (section.hasPath("fullNodeAllowShieldedTransaction")) {
398398
nc.allowShieldedTransactionApi = section.getBoolean("fullNodeAllowShieldedTransaction");
399+
logger.warn("Configuring [node.fullNodeAllowShieldedTransaction] will be deprecated. "
400+
+ "Please use [node.allowShieldedTransactionApi] instead.");
399401
}
400402
// node.shutdown.* — PascalCase keys (BlockTime, BlockHeight), cannot auto-bind
401403
nc.shutdownBlockTime = config.hasPath("node.shutdown.BlockTime")

common/src/test/java/org/tron/core/config/args/NodeConfigTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,4 +282,38 @@ public void testLegacyAliasTakesPriorityOverModernKey() {
282282
withRef("node { maxActiveNodesWithSameIp = 5, maxConnectionsWithSameIp = 10 }"));
283283
assertEquals(5, nc.getMaxConnectionsWithSameIp());
284284
}
285+
286+
@Test
287+
public void testShieldedApiDefaultsToTrueWhenNeitherKeySet() {
288+
NodeConfig nc = NodeConfig.fromConfig(withRef());
289+
assertTrue(nc.isAllowShieldedTransactionApi());
290+
}
291+
292+
@Test
293+
public void testShieldedApiModernKeyRespected() {
294+
NodeConfig nc = NodeConfig.fromConfig(
295+
withRef("node.allowShieldedTransactionApi = false"));
296+
assertFalse(nc.isAllowShieldedTransactionApi());
297+
}
298+
299+
@Test
300+
public void testShieldedApiLegacyKeyRespected() {
301+
// Regression guard: reference.conf ships `allowShieldedTransactionApi = true`, which
302+
// used to make the legacy-key fallback dead code. A user who only set the legacy key
303+
// must still have their value honored.
304+
NodeConfig nc = NodeConfig.fromConfig(
305+
withRef("node.fullNodeAllowShieldedTransaction = false"));
306+
assertFalse(nc.isAllowShieldedTransactionApi());
307+
}
308+
309+
@Test
310+
public void testShieldedApiLegacyKeyTakesPriorityOverModern() {
311+
// Consistent with maxActiveNodesWithSameIp: legacy key presence wins over modern.
312+
NodeConfig nc = NodeConfig.fromConfig(
313+
withRef("node {\n"
314+
+ " allowShieldedTransactionApi = false\n"
315+
+ " fullNodeAllowShieldedTransaction = true\n"
316+
+ "}"));
317+
assertTrue(nc.isAllowShieldedTransactionApi());
318+
}
285319
}

0 commit comments

Comments
 (0)