Skip to content

[Feature] Elevate crypto.engine from local config to chain-identity constant #20

@Federico2014

Description

@Federico2014

Summary

crypto.engine in framework/src/main/resources/config.conf selects the signature algorithm (ECDSA secp256k1 vs sm2) for every signature produced and verified by a node — transactions, blocks, PBFT votes, relay handshakes, JSON-RPC tx hashes, keystores. It is currently a node-local setting (HOCON config), which means two operators on the same network can pick different values and their nodes will silently fail to verify each other's signatures. The comment above it already acknowledges the hazard:

# Specify the algorithm for generating a public key from private key.
# To avoid forks, please do not modify it
crypto {
  engine = "eckey"
}

This issue proposes elevating crypto.engine to a chain-identity constant — committed once at genesis to a non-proposal chain-state location, with the local conf value used only as a one-time seed and thereafter as a startup-time consistency check.

Problem

Motivation

A parameter that silently forks the network on mismatch does not belong in a per-node HOCON file. It is a chain-identity constant, same rank as chainId — set once at genesis, never mutable, never proposal-governed.

Current State

framework/src/main/java/org/tron/core/config/args/Args.java:198-199 reads crypto.engine from conf and stores it on the in-memory CommonParameter. Roughly 30 production call sites across the following modules read CommonParameter#isECKeyCryptoEngine() on hot paths:

  • framework: Wallet, Manager, RpcApiService, RelayService, ConsensusService, WitnessInitializer, JsonRpcApiUtil, BlockResult, Util (http), KeystoreFactory
  • chainbase: TransactionCapsule (signature verify / tx-id hashing), StorageRowCapsule
  • common: backup/Message
  • keystore: Wallet, WalletUtils, Credentials

crypto/src/main/java/org/tron/common/crypto/SignUtils.java:11-54 branches on this flag to dispatch to ECKey or SM2 — two different curves, different hashes, different signature recovery. A node that disagrees with the network cannot verify any of its peers' artifacts.

Limitations or Risks

  • Silent fork surface. A misconfigured conf file is enough to split a node off the network. No protocol-level check exists.
  • Weak signal to operators. The "do not modify" comment is the only safeguard. There is no runtime validation.
  • Parameter is misplaced conceptually. It is already a chain-wide invariant in practice; the code shape does not reflect that.

Proposed Solution

Proposed Design

Treat the engine as an immutable chain-identity constant, committed once at genesis. Concretely:

  1. Pick the correct storage location: CommonStore, not DynamicPropertiesStore.
    DynamicPropertiesStore is the home of proposal-governed / consensus-mutable parameters (ALLOW_CREATION_OF_CONTRACTS, ALLOW_TVM_*, fee / energy / bandwidth thresholds, etc.) — every value there is expected to change through SR proposals and is part of the revoking chain state. A parameter that must never change by any mechanism does not belong there; placing it alongside mutable proposal parameters is a category error.
    chainbase/src/main/java/org/tron/core/db/CommonStore.java is a non-revoking, non-proposal key-value store used for chain-level markers that sit outside proposal governance. That is the conceptually correct home for a chain-identity constant.
  2. Write once at genesis. In ChainBaseManager#initGenesis (ChainBaseManager.java:285), if the CRYPTO_ENGINE key is absent in CommonStore, seed it from the local CommonParameter value. After that, the store is the source of truth and no code path (actuator, proposal, or otherwise) is permitted to write to it again.
  3. Add a chain-aware facade. Introduce a small helper (e.g. ChainCrypto.isECKey()) backed by a volatile boolean cache loaded once from CommonStore. Every production caller of CommonParameter#isECKeyCryptoEngine() is moved to this facade. The volatile cache is mandatory because TransactionCapsule#validateSignature / tx-id hashing sit on the validation hot path — per-call LevelDB reads would regress TPS.
  4. Startup consistency check. At node startup (after DB init, before accepting network messages), compare the local conf value to the value in CommonStore. On mismatch, log a clear error and refuse to start. This converts what is currently a silent fork into a loud boot failure.
  5. Keep SignUtils and the wire format untouched. The two-engine dispatch in SignUtils.java stays as-is; only the source of the boolean changes.

Key Changes

  • Module(s): chainbase (CommonStore gains a CRYPTO_ENGINE key; new ChainCrypto facade; ChainBaseManager#initGenesis seeds the key), framework (startup validation in Manager / Args), and mechanical replacement of ~30 call sites across framework, chainbase, common, keystore.
  • Configuration: crypto.engine stays in config.conf and retains its current semantics for first boot; after that it is an assertion, not a setting. The # To avoid forks, please do not modify it comment can be tightened to reflect the new runtime check.
  • API: None. No protobuf change, no wire format change, no JSON-RPC / HTTP surface change.

What this proposal explicitly does NOT do

  • Does not add a proposal to change the engine on a live chain. That would be a hard fork with non-trivial historical-signature implications (existing blocks are signed with the old engine; the derivation from private key to address is different across ECDSA and SM2). This proposal keeps the engine immutable per chain.
  • Does not touch DynamicPropertiesStore. Nothing is added there; the proposal parameter surface is unchanged.
  • Does not change genesis block layout or any protobuf schema.

Impact

  • Security / Stability: Closes the silent-fork footgun. A misconfigured node now fails fast at startup with an actionable error instead of producing signatures that its peers reject.
  • Performance: Neutral. The volatile-cached facade resolves to a single field read on the hot path, identical cost to today's CommonParameter#isECKeyCryptoEngine().
  • Developer Experience: Slightly positive. One canonical accessor for "which engine does this chain use" replaces a conf-derived global, and the storage location correctly reflects the parameter's immutability.
  • Operational: Operators on an existing chain see no behavior change on upgrade — the first post-upgrade boot reads the local conf value (which already matches the chain in practice) and persists it to CommonStore.

Compatibility

  • Breaking Change: No (for correctly configured nodes).
  • Default Behavior Change: After upgrade, a node whose config.conf disagrees with the network will refuse to start instead of silently forking. This is the intended behavior change, and it only affects misconfigured deployments.
  • Migration Required: No action required for nodes whose conf matches the network (the vast majority). CommonStore is seeded on first boot after upgrade from the existing conf value.
  • Database: Additive only — a new key in CommonStore (not in DynamicPropertiesStore, and not part of the revoking / proposal state). No schema migration. Downgrading to a pre-change binary continues to work because the old binary simply ignores the new key.
  • Hard-fork impact: None. Protocol semantics are unchanged; this is an in-process refactor of where the flag is read from.

References

  • framework/src/main/resources/config.conf:128-131 — current config entry.
  • framework/src/main/java/org/tron/core/config/args/Args.java:198-199 — current read path.
  • crypto/src/main/java/org/tron/common/crypto/SignUtils.java:11-54 — engine dispatch.
  • chainbase/src/main/java/org/tron/core/db/CommonStore.java — target storage location (non-revoking, non-proposal).
  • chainbase/src/main/java/org/tron/core/ChainBaseManager.java:285initGenesis hook for one-time seeding.
  • chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java:246 — example of a hot-path caller that motivates the volatile cache.

Additional Notes

  • Do you have ideas regarding implementation? Yes — see "Proposed Design" above.
  • Are you willing to implement this feature? Yes.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions