Skip to content

refactor(config): improve startup errors and remove inactive assertions #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bladehan1 wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

refactor(config): improve startup errors and remove inactive assertions #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48d883f
refactor(config): standardize startup errors
bladehan1 Aug 10, 2026
cc91a11
chore(chainbase): remove empty placeholder classes
bladehan1 Aug 31, 2026
f5628ea
refactor(crypto): remove test-only Blake2bfDigest constructor
bladehan1 Sep 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions actuator/src/main/java/org/tron/core/vm/repository/RepositoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,6 @@ private long increase(long lastUsage, long usage, long lastTime, long now, long
}

if (lastTime != now) {
assert now > lastTime;
if (lastTime + windowSize > now) {
long delta = now - lastTime;
double decay = (windowSize - delta) / (double) windowSize;
Expand Down Expand Up @@ -973,8 +972,6 @@ public long calculateGlobalEnergyLimit(AccountCapsule accountCapsule) {
long totalEnergyLimit = getDynamicPropertiesStore().getTotalEnergyCurrentLimit();
long totalEnergyWeight = getDynamicPropertiesStore().getTotalEnergyWeight();

assert totalEnergyWeight > 0;

@cubic-dev-ai cubic-dev-ai Bot Aug 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: This assertion is not inactive/historical code — it guards division by totalEnergyWeight on the very next lines. If totalEnergyWeight is 0, the hardened path throws a bare ArithmeticException and the non-hardened path silently yields an unbounded value from (long)(... / 0.0) (Long.MAX_VALUE). The PR limits removal to dead asserts and retains critical ones; this one is live protection in a core resource method, so keeping it (or an explicit zero check) preserves the safety net rather than deleting it.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At actuator/src/main/java/org/tron/core/vm/repository/RepositoryImpl.java, line 976:

<comment>This assertion is not inactive/historical code — it guards division by totalEnergyWeight on the very next lines. If totalEnergyWeight is 0, the hardened path throws a bare ArithmeticException and the non-hardened path silently yields an unbounded value from `(long)(... / 0.0)` (Long.MAX_VALUE). The PR limits removal to dead asserts and retains critical ones; this one is live protection in a core resource method, so keeping it (or an explicit zero check) preserves the safety net rather than deleting it.</comment>

<file context>
@@ -973,8 +972,6 @@ public long calculateGlobalEnergyLimit(AccountCapsule accountCapsule) {
-    assert totalEnergyWeight > 0;
-
     if (hardenResourceCalculation()) {
       return BigInteger.valueOf(energyWeight)
           .multiply(BigInteger.valueOf(totalEnergyLimit))
</file context>

@bladehan1 bladehan1 Sep 1, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

该断言并非测试，一般不会运行到


if (hardenResourceCalculation()) {
return BigInteger.valueOf(energyWeight)
.multiply(BigInteger.valueOf(totalEnergyLimit))
Expand Down
Empty file removed chainbase/src/main/java/org/tron/common/utils/ForkUtils.java
View file
Open in desktop
Empty file.
1 change: 0 additions & 1 deletion chainbase/src/main/java/org/tron/common/zksnark/MerklePath.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ private static long convertVectorToLong(List<Boolean> v) throws ZksnarkException
}

public byte[] encode() throws ZksnarkException {
assert (authenticationPath.size() == index.size());

@cubic-dev-ai cubic-dev-ai Bot Sep 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The deleted assertion was the only enforcement of the invariant that authenticationPath.size() == index.size() (path depth == index bit-length). With it removed outright instead of converted to an explicit runtime check, encode() now silently produces a malformed encoding when a MerklePath is constructed with mismatched sizes, because the public constructor validates nothing and convertVectorToLong only bounds-checks index.size() <= 64 without relating it to authenticationPath.size(). An explicit size check costs one if and keeps a clear diagnostic for what was previously an always-inactive guard.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At chainbase/src/main/java/org/tron/common/zksnark/MerklePath.java, line 78:

<comment>The deleted assertion was the only enforcement of the invariant that `authenticationPath.size() == index.size()` (path depth == index bit-length). With it removed outright instead of converted to an explicit runtime check, `encode()` now silently produces a malformed encoding when a `MerklePath` is constructed with mismatched sizes, because the public constructor validates nothing and `convertVectorToLong` only bounds-checks `index.size() <= 64` without relating it to `authenticationPath.size()`. An explicit size check costs one `if` and keeps a clear diagnostic for what was previously an always-inactive guard.</comment>

<file context>
@@ -75,7 +75,6 @@ private static long convertVectorToLong(List<Boolean> v) throws ZksnarkException
 
   public byte[] encode() throws ZksnarkException {
-    assert (authenticationPath.size() == index.size());
     List<List<Byte>> pathByteList = Lists.newArrayList();
     long indexLong; // 64
     for (int i = 0; i < authenticationPath.size(); i++) {
</file context>

List<List<Byte>> pathByteList = Lists.newArrayList();
long indexLong; // 64
for (int i = 0; i < authenticationPath.size(); i++) {
Expand Down
Empty file removed chainbase/src/main/java/org/tron/core/config/args/Parameter.java
View file
Open in desktop
Empty file.
3 changes: 0 additions & 3 deletions chainbase/src/main/java/org/tron/core/db/EnergyProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,6 @@ public long calculateGlobalEnergyLimit(AccountCapsule accountCapsule) {
long totalEnergyWeight = dynamicPropertiesStore.getTotalEnergyWeight();
if (dynamicPropertiesStore.allowNewReward() && totalEnergyWeight <= 0) {
return 0;
} else {
assert totalEnergyWeight > 0;
}
if (hardenCalculation()) {
return calculateGlobalLimitV1(frozeBalance, totalEnergyLimit, totalEnergyWeight);
Expand Down Expand Up @@ -205,4 +203,3 @@ private long scaleByRate(long value, long numerator, long denominator) {
}
}


1 change: 0 additions & 1 deletion chainbase/src/main/java/org/tron/core/db/ResourceProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ protected long increase(long lastUsage, long usage, long lastTime, long now, lon
}

if (lastTime != now) {
assert now > lastTime;
if (lastTime + windowSize > now) {
long delta = now - lastTime;
double decay = (windowSize - delta) / (double) windowSize;
Expand Down
7 changes: 5 additions & 2 deletions common/src/main/java/org/tron/core/config/args/CommitteeConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package org.tron.core.config.args;

import static org.tron.core.exception.TronError.ErrCode.PARAMETER_INIT;

import com.typesafe.config.Config;
import com.typesafe.config.ConfigBeanFactory;
import com.typesafe.config.ConfigValue;
import lombok.Getter;
import lombok.Setter;
import lombok.extern.slf4j.Slf4j;
import org.tron.core.exception.TronError;

/**
* Committee (governance) configuration bean.
Expand Down Expand Up @@ -160,11 +163,11 @@ private void postProcess() {
// cross-field: allowOldRewardOpt requires at least one reward/vote flag
if (allowOldRewardOpt == 1 && allowNewRewardAlgorithm != 1
&& allowNewReward != 1 && allowTvmVote != 1) {
throw new IllegalArgumentException(
throw new TronError(
"At least one of the following proposals is required to be opened first: "
+ "committee.allowNewRewardAlgorithm = 1"
+ " or committee.allowNewReward = 1"
+ " or committee.allowTvmVote = 1.");
+ " or committee.allowTvmVote = 1.", PARAMETER_INIT);
}
}
}
13 changes: 11 additions & 2 deletions common/src/test/java/org/tron/core/config/args/CommitteeConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package org.tron.core.config.args;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;

import com.typesafe.config.Config;
import com.typesafe.config.ConfigFactory;
import org.junit.Test;
import org.tron.core.exception.TronError;

public class CommitteeConfigTest {

Expand Down Expand Up @@ -57,9 +59,16 @@ public void testDynamicEnergyThresholdClamped() {
.getDynamicEnergyThreshold());
}

@Test(expected = IllegalArgumentException.class)
@Test
public void testAllowOldRewardOptWithoutPrerequisites() {
CommitteeConfig.fromConfig(withRef("committee { allowOldRewardOpt = 1 }"));
TronError error = assertThrows(TronError.class,
() -> CommitteeConfig.fromConfig(withRef("committee { allowOldRewardOpt = 1 }")));

assertEquals(TronError.ErrCode.PARAMETER_INIT, error.getErrCode());
assertEquals("At least one of the following proposals is required to be opened first: "
+ "committee.allowNewRewardAlgorithm = 1"
+ " or committee.allowNewReward = 1"
+ " or committee.allowTvmVote = 1.", error.getMessage());
}

@Test
Expand Down
16 changes: 0 additions & 16 deletions crypto/src/main/java/org/tron/common/crypto/Blake2bfMessageDigest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,22 +76,6 @@ public static class Blake2bfDigest implements Digest {
v = new long[16];
}

// for tests
Blake2bfDigest(
final long[] h, final long[] m, final long[] t, final boolean f, final long rounds) {
assert rounds <= 4294967295L; // uint max value
buffer = new byte[MESSAGE_LENGTH_BYTES];
bufferPos = 0;

this.h = h;
this.m = m;
this.t = t;
this.f = f;
this.rounds = rounds;

v = new long[16];
}

@Override
public String getAlgorithmName() {
return "BLAKE2f";
Expand Down
9 changes: 5 additions & 4 deletions framework/src/main/java/org/tron/core/config/args/Args.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1045,8 +1045,9 @@ private static void loadDnsPublishParameters(NodeConfig.DnsConfig dns,
String serverType = dns.getServerType();
if (StringUtils.isNotEmpty(serverType)) {
if (!"aws".equalsIgnoreCase(serverType) && !"aliyun".equalsIgnoreCase(serverType)) {
throw new IllegalArgumentException(
"Check node.dns.serverType, must be aws or aliyun");
throw new TronError(

@cubic-dev-ai cubic-dev-ai Bot Sep 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: TronError extends java.lang.Error, not Exception, so these three throw sites (serverType check, logEmptyError, and the CommitteeConfig postProcess change) change the failure type from a catchable RuntimeException (IllegalArgumentException) to an Error that only the ExitManager uncaught-handler can terminate on. For FullNode startup this is the intended design and exit code 1 is preserved. However, Args.loadDnsPublishConfig/loadDnsPublishParameters are public and documented as callable by tests and external code; any caller that previously wrapped DNS/config loading in catch (Exception) or catch (RuntimeException) for graceful fallback will silently lose that edge and let an Error escape (process exit). Verify no current or future caller relies on catching these failures as Exception.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/java/org/tron/core/config/args/Args.java, line 1048:

<comment>TronError extends java.lang.Error, not Exception, so these three throw sites (serverType check, logEmptyError, and the CommitteeConfig postProcess change) change the failure type from a catchable RuntimeException (IllegalArgumentException) to an Error that only the ExitManager uncaught-handler can terminate on. For FullNode startup this is the intended design and exit code 1 is preserved. However, Args.loadDnsPublishConfig/loadDnsPublishParameters are public and documented as callable by tests and external code; any caller that previously wrapped DNS/config loading in catch (Exception) or catch (RuntimeException) for graceful fallback will silently lose that edge and let an Error escape (process exit). Verify no current or future caller relies on catching these failures as Exception.</comment>

<file context>
@@ -1045,8 +1045,9 @@ private static void loadDnsPublishParameters(NodeConfig.DnsConfig dns,
         if (!"aws".equalsIgnoreCase(serverType) && !"aliyun".equalsIgnoreCase(serverType)) {
-          throw new IllegalArgumentException(
-              "Check node.dns.serverType, must be aws or aliyun");
+          throw new TronError(
+              "Check node.dns.serverType, must be aws or aliyun",
+              TronError.ErrCode.PARAMETER_INIT);
</file context>

"Check node.dns.serverType, must be aws or aliyun",
TronError.ErrCode.PARAMETER_INIT);
}
if ("aws".equalsIgnoreCase(serverType)) {
publishConfig.setDnsType(DnsType.AwsRoute53);
Expand Down Expand Up @@ -1088,7 +1089,8 @@ private static void loadDnsPublishParameters(NodeConfig.DnsConfig dns,
}

private static void logEmptyError(String arg) {
throw new IllegalArgumentException(String.format("Check %s, must not be null or empty", arg));
throw new TronError(String.format("Check %s, must not be null or empty", arg),
TronError.ErrCode.PARAMETER_INIT);
}

// createTriggerConfig removed — logic moved to applyEventConfig()
Expand Down Expand Up @@ -1315,4 +1317,3 @@ private static Map<String, String[]> getOptionGroup() {
return optionGroupMap;
}
}

58 changes: 58 additions & 0 deletions framework/src/test/java/org/tron/core/config/args/ArgsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,64 @@ public void testMaxMessageSizeNegativeValueRejected() {
}
}

@Test
public void testDnsPublishRejectsInvalidServerTypeWithParameterInitError() {
Config config = dnsPublishConfig(
"node.dns.serverType", "unsupported");

TronError error = Assert.assertThrows(TronError.class,
() -> Args.loadDnsPublishConfig(NodeConfig.fromConfig(config)));

Assert.assertEquals(TronError.ErrCode.PARAMETER_INIT, error.getErrCode());
Assert.assertEquals("Check node.dns.serverType, must be aws or aliyun",
error.getMessage());
}

@Test
public void testDnsPublishRejectsEmptyRequiredParameterWithParameterInitError() {
Config config = dnsPublishConfig("node.dns.dnsDomain", "");

TronError error = Assert.assertThrows(TronError.class,
() -> Args.loadDnsPublishConfig(NodeConfig.fromConfig(config)));

Assert.assertEquals(TronError.ErrCode.PARAMETER_INIT, error.getErrCode());
Assert.assertEquals("Check node.dns.dnsDomain, must not be null or empty",
error.getMessage());
}

@Test
public void testCommitteeConfigRejectsOldRewardOptimizationWithoutPrerequisite() {
Map<String, Object> configMap = new HashMap<>();
configMap.put("storage.db.directory", "database");
configMap.put("committee.allowOldRewardOpt", 1);
Config config = ConfigFactory.parseMap(configMap)
.withFallback(ConfigFactory.defaultReference());

try {
TronError error = Assert.assertThrows(TronError.class,
() -> Args.applyConfigParams(config));

Assert.assertEquals(TronError.ErrCode.PARAMETER_INIT, error.getErrCode());
} finally {
Args.clearParam();
}
}

private Config dnsPublishConfig(String key, String value) {
Map<String, Object> configMap = new HashMap<>();
configMap.put("node.dns.publish", true);
configMap.put("node.dns.dnsDomain", "nodes.example.org");
configMap.put("node.dns.dnsPrivate",
"1234567890123456789012345678901234567890123456789012345678901234");
configMap.put("node.dns.serverType", "aliyun");
configMap.put("node.dns.accessKeyId", "access-key-id");
configMap.put("node.dns.accessKeySecret", "access-key-secret");
configMap.put("node.dns.aliyunDnsEndpoint", "dns.aliyuncs.com");
configMap.put(key, value);
return ConfigFactory.parseMap(configMap)
.withFallback(ConfigFactory.defaultReference());
}

@Test
public void testRpcMaxMessageSizeExceedsIntMax() {
// HOCON's Config.getInt() throws when a numeric value exceeds int range.
Expand Down
Loading