Skip to content

[Feature] Improve startup error classification and remove inactive assertions #43

Description

@bladehan1

Summary

Make two small, low-risk maintenance improvements in java-tron:

  1. Replace IllegalArgumentException with TronError(..., PARAMETER_INIT) for three startup configuration errors that are already explicitly identified by the code.
  2. Remove selected historical Java assert statements that are inactive under the default production JVM configuration, while preserving assertions inherited from upstream implementations.

Problem

Motivation

Some validation failures that are already known to be parameter initialization errors still throw the generic IllegalArgumentException and therefore do not carry java-tron's structured error code. In addition, a few historical Java assert statements remain in production code even though assertions are disabled unless the JVM is started with -ea. These statements can give maintainers the false impression that production runtime validation is present.

This proposal improves error classification and code clarity only at the confirmed locations. It does not introduce a general startup exception-handling framework.

Current State

  • Reward configuration prerequisites, the DNS server type, and required DNS publishing fields throw IllegalArgumentException when invalid.
  • These exceptions already provide understandable messages and stack traces. They prevent startup and the process exits with code 1.
  • Several historical assert statements exist in production code but normally do not execute because production JVMs do not enable -ea by default.
  • The core TrieImpl implementation is derived from EthereumJ, where its assertions express internal branch/KV node-type invariants.
  • Blake2bfMessageDigest is derived from the Besu implementation.

Limitations or Risks

  • This change does not fix a failure to terminate, swallowed exceptions, or missing diagnostic messages; those problems do not exist on the selected paths.
  • External tooling that depends on the concrete Java exception type or exact log format may observe a change.
  • Not every exception raised during startup should be classified as PARAMETER_INIT. Parser, environment, database, network, third-party, and programming errors require separate evaluation.
  • Assertion removal is limited to confirmed historical java-tron code where the assertions are inactive by default.
  • Passing tests without -ea proves that legal execution paths do not depend on assertion failures; it does not establish that upstream-derived invariant checks have no maintenance value.

Proposed Solution

Proposed Design

1. Classify selected startup configuration errors

Use TronError(..., PARAMETER_INIT) for these existing validations:

  • Reward configuration conflict: committee.allowOldRewardOpt = 1 while the required reward-related switches are not enabled.
  • Invalid DNS server type: DNS publishing is enabled, but node.dns.serverType is neither aws nor aliyun.
  • Missing fields already required by the DNS publishing configuration.

2. Remove selected runtime-inactive historical assertions

  • Remove confirmed historical assertions from RepositoryImpl, MerklePath, EnergyProcessor, and ResourceProcessor.
  • Do not introduce replacement checks or alter existing control flow in those classes.
  • Preserve the node-type assertions in TrieImpl because its core implementation is derived from EthereumJ and the checks document internal Trie invariants.
  • Preserve the assertion in Blake2bfMessageDigest, which was copied from the Besu implementation.
  • Evaluate any conversion of the retained assertions into explicit production runtime checks separately.

Key Changes

  • CommitteeConfig: use TronError(PARAMETER_INIT) for the reward configuration prerequisite failure.
  • Args: use TronError(PARAMETER_INIT) for the DNS server type and required-field failures.
  • CommitteeConfigTest: update the expected exception type.
  • Remove the confirmed inactive historical assertions from the four java-tron classes listed above.
  • Keep assertions in the EthereumJ-derived Trie implementation and the Besu-derived Blake2bf implementation.

Impact

  • Stability: Valid configurations and normal execution paths are unchanged. The three invalid configurations fail startup both before and after this change.
  • Operations: The selected configuration failures receive the PARAMETER_INIT classification. Their messages and process exit code 1 remain unchanged.
  • Maintainability: Remove selected inactive assertions without unnecessarily diverging from the EthereumJ and Besu source implementations.
  • Performance: No runtime performance impact.

Compatibility

  • Breaking Change: No.
  • Default Behavior Change: No. Only the exception type and categorized logging on invalid startup configurations are observable changes.
  • Migration Required: No.
  • Protocol / Database / Network: No impact.
  • Dynamic Configuration Reload: Not on the modified paths and remains unchanged.

Testing Strategy

  • Create a conflicting allowOldRewardOpt configuration and verify that it throws TronError with PARAMETER_INIT.
  • Verify that an invalid DNS server type and missing DNS fields preserve their existing messages and use PARAMETER_INIT.
  • Run the relevant configuration tests to confirm that valid configurations remain unchanged.
  • Run the existing tests for the affected modules to confirm that removing selected inactive assertions does not change behavior.
  • Run TrieTest after retaining the TrieImpl assertions to verify existing Trie behavior.

References

  • Baseline: GreatVoyage-v4.8.2.1 (f8b05d40abc949fa588ab64d8fd8fd82845ebeed).
  • Affected modules: common, framework, chainbase, and actuator.
  • Preserved upstream-derived implementations: EthereumJ TrieImpl and Besu Blake2bfMessageDigest.

Additional Notes

  • Do you have ideas regarding implementation? Yes. Use the limited replacements and cleanup described above.
  • Are you willing to implement this feature? Yes.
  • Estimated Complexity: Low.

Non-goals

  • Do not standardize every startup configuration exception.
  • Do not change exception handling for JCommander, Typesafe Config, network addresses, storage, genesis configuration, databases, or third-party libraries.
  • Do not change ExitManager, dynamic reload, or subsystem initialization behavior.
  • Do not attempt to eliminate every Uncaught exception log.
  • Do not replace the retained upstream-derived assertions with explicit runtime checks in this change.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions