Skip to content

feat(crypto): ML-DSA + FN-DSA post-quantum signatures and TVM precompiles#21

Open
Federico2014 wants to merge 12 commits intorelease_pqc_basefrom
feat/add-pqc-signature
Open

feat(crypto): ML-DSA + FN-DSA post-quantum signatures and TVM precompiles#21
Federico2014 wants to merge 12 commits intorelease_pqc_basefrom
feat/add-pqc-signature

Conversation

@Federico2014
Copy link
Copy Markdown
Owner

@Federico2014 Federico2014 commented Apr 24, 2026

Summary

This PR adds post-quantum signature support to java-tron, covering ML-DSA (FIPS 204), FN-DSA / Falcon-512 (FIPS 206 draft), and TVM precompiled contracts for on-chain PQ verification. PQ signatures are gated behind on-chain governance proposals and are fully opt-in — existing ECDSA paths are unchanged.


ML-DSA (FIPS 204) — ML-DSA-44 and ML-DSA-65

  • PQSignature interface + MLDSA44 / MLDSA65 implementations
  • PQSignatureRegistry static dispatch table (sign / verify / fromSeed)
  • PQPublicKey sub-message in Permission.Key bundles scheme + public_key; absent on legacy ECDSA keys
  • PQAuthWitness proto message carries key_id (position index into the permission's key list) + signature; replaces address-based witness lookup
  • AccountPermissionUpdateActuator — validates PQ public-key lengths per scheme; supports mixed ECDSA+PQ key lists
  • TransactionCapsule.validateStructuredSignature — resolves key by key_id, verifies PQ auth witnesses
  • BlockCapsule.validateWitnessAuth — verifies PQ block-signing witnesses
  • ALLOW_ML_DSA governance proposal (id 97); DynamicPropertiesStore.allowMlDsa()
  • Configurable witness seed via localwitness_seed_pq / localwitness_seed_pq_scheme

FN-DSA / Falcon-512 (FIPS 206 draft)

  • FNDSA.java — BouncyCastle Falcon-512 keypair-bound signer/verifier
  • Variable-length signature handling: protocol-level upper bound 752 bytes; PQSignatureRegistry.isValidSignatureLength uses range check for FN-DSA, strict equality for ML-DSA
  • SignatureScheme.FN_DSA = 6 in Tron.proto
  • ALLOW_FN_DSA governance proposal (id 100); DynamicPropertiesStore.allowFnDsa()
  • Per-scheme governance helpers: isAnyPqSchemeAllowed(), isPqSchemeAllowed(SignatureScheme)
  • ML-DSA and FN-DSA each have independent activation flags; Manager P2P gate uses isAnyPqSchemeAllowed()
  • LocalWitnesses seed validation is scheme-aware via PQSignatureRegistry.getSeedLength(scheme) — accepts 32-byte seeds for ML-DSA and 48-byte seeds for FN-DSA

TVM Precompiled Contracts

Address Scheme Energy Input layout
0x12 ML-DSA-44 (EIP-8051) 4500 msg 32B | sig 2420B | pk 1312B
0x14 ML-DSA-65 (TRON ext.) 7000 msg 32B | sig 3309B | pk 1952B
0x16 FN-DSA / Falcon-512 (EIP-8052) 2500 msg 32B | sig_len 2B | sig <=752B | pk 896B

Each precompile is gated by its corresponding ALLOW_* governance flag.

Protocol refactoring — PQPublicKey + PQAuthWitness + key_id

  • PQPublicKey sub-message (scheme + public_key) nested in Key.pq_key; ECDSA keys leave pq_key absent
  • PQAuthWitness (renamed from AuthWitness): key_id uint32 replaces 21-byte signer_address; key_id=0 costs 0 wire bytes (proto3 default), key_id 1-127 costs 2 bytes — optimal for realistic key counts
  • pq_witness field name used consistently in both Transaction (repeated) and BlockHeader.raw (singular); replaces old auth_witness / witness_auth
  • PQAuthDigest: domain-separated hash now takes (txid, permissionId, keyId) instead of a signer address

Naming convention

Class names and proto message names use uppercase PQ (PQPublicKey, PQAuthWitness, PQSignature, PQSignatureRegistry, PQAuthDigest). Lowercase pq_* proto field names and Lombok-bound pqScheme / pqSeeds fields are kept as-is so generated accessors stay stable.

Key byte-length constants

Scheme Public key Signature
ML-DSA-44 1312 B 2420 B (fixed)
ML-DSA-65 1952 B 3309 B (fixed)
FN-DSA (Falcon-512) 896 B 1..752 B (variable)

Test coverage

  • FNDSATest — 13 unit tests: keygen, sign/verify, variable-length bound, cross-algo rejection, registry dispatch
  • AccountPermissionUpdateActuatorTest — PQ key validation: length, dedup, mixed ECDSA+PQ, FN-DSA cases
  • TransactionCapsuleTest — accept / tampered / not-activated / duplicate key_id / key_id out-of-range
  • BlockCapsulePQTest — PQ block witness sign/verify, legacy-sig rejection, key_id out-of-range
  • PQAuthDigestTest — domain-separation correctness for tx() and block() digest variants
  • PQSignatureRegistryTest — registry dispatch + per-scheme metadata
  • LocalWitnessesTest — per-scheme seed-length validation (ML-DSA 32B, FN-DSA 48B) and hex-format rejection
  • MlDsaPrecompileTest — 12 TVM precompile cases for 0x12 and 0x14
  • FnDsaPrecompileTest — 11 TVM precompile cases for 0x16

Test plan

  • ./gradlew :framework:test --tests "org.tron.common.crypto.pqc.FNDSATest"
  • ./gradlew :framework:test --tests "org.tron.common.crypto.pqc.PQSignatureRegistryTest"
  • ./gradlew :framework:test --tests "org.tron.common.crypto.pqc.PQAuthDigestTest"
  • ./gradlew :framework:test --tests "org.tron.common.utils.LocalWitnessesTest"
  • ./gradlew :framework:test --tests "org.tron.core.actuator.AccountPermissionUpdateActuatorTest"
  • ./gradlew :framework:test --tests "org.tron.core.capsule.TransactionCapsuleTest"
  • ./gradlew :framework:test --tests "org.tron.core.capsule.BlockCapsulePQTest"
  • ./gradlew :framework:test --tests "org.tron.common.runtime.vm.MlDsaPrecompileTest"
  • ./gradlew :framework:test --tests "org.tron.common.runtime.vm.FnDsaPrecompileTest"
  • ./gradlew :framework:checkstyleMain :framework:checkstyleTest
  • Verify activation via governance proposals: ALLOW_ML_DSA(97) and ALLOW_FN_DSA(100)
  • Confirm key_id=0 wire overhead is zero (default field omitted in proto3)
  • Confirm TVM precompile 0x16 returns 0 when ALLOW_FN_DSA is off
  • Confirm legacy ECDSA transactions unaffected when no PQ flag is active

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aac12772-be0d-4bdc-a210-6e7c1136d52c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/add-pqc-signature

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Federico2014 Federico2014 changed the title Feat/add pqc signature feat(crypto): add ML-DSA post-quantum signature scheme support Apr 24, 2026
@Federico2014 Federico2014 changed the title feat(crypto): add ML-DSA post-quantum signature scheme support feat(crypto): add post-quantum signature scheme support Apr 24, 2026
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

4 issues found across 37 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="chainbase/src/main/java/org/tron/common/utils/LocalWitnesses.java">

<violation number="1" location="chainbase/src/main/java/org/tron/common/utils/LocalWitnesses.java:118">
P2: `validatePqSeed` accepts `0X` prefixes that downstream hex decoding does not normalize, so configuration can pass validation but fail at runtime during seed decoding.</violation>
</file>

<file name="framework/src/main/java/org/tron/core/config/args/Args.java">

<violation number="1" location="framework/src/main/java/org/tron/core/config/args/Args.java:1231">
P1: PQ-seed witness initialization does not enforce `localWitnessAccountAddress`, allowing an invalid PQ-only witness configuration to pass.</violation>
</file>

<file name="chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java">

<violation number="1" location="chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java:761">
P1: `StrictMathWrapper.addExact` throws `ArithmeticException` on overflow, but the caller in `validatePubSignature` only catches `PermissionException`. A crafted transaction with extreme weight values would cause an unhandled exception instead of a proper `ValidateSignatureException`.</violation>
</file>

<file name="consensus/src/main/java/org/tron/consensus/base/Param.java">

<violation number="1" location="consensus/src/main/java/org/tron/consensus/base/Param.java:72">
P1: Avoid Lombok getter/setter exposure for `byte[]` key fields; it leaks mutable references and allows external mutation of internal key state.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread framework/src/main/java/org/tron/core/config/args/Args.java
Comment thread chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java Outdated
Comment thread consensus/src/main/java/org/tron/consensus/base/Param.java
Comment thread chainbase/src/main/java/org/tron/common/utils/LocalWitnesses.java Outdated
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="framework/src/test/java/org/tron/common/crypto/pqc/program/PqFullNode.java">

<violation number="1" location="framework/src/test/java/org/tron/common/crypto/pqc/program/PqFullNode.java:91">
P1: Use a mutable list for seed nodes; startup appends persisted peers to this list.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread framework/src/test/java/org/tron/common/crypto/pqc/program/PqFullNode.java Outdated
@Federico2014 Federico2014 changed the title feat(crypto): add post-quantum signature scheme support feat(crypto): integrate ML-DSA post-quantum signatures end-to-end Apr 26, 2026
@Federico2014 Federico2014 changed the base branch from feature/pqc_base to release_pqc_base April 26, 2026 13:23
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

4 issues found across 13 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="framework/src/main/java/org/tron/core/config/args/Args.java">

<violation number="1" location="framework/src/main/java/org/tron/core/config/args/Args.java:1236">
P2: `localwitness_seed_pq_scheme` accepts non-PQ enum values, which can pass config parsing and then fail later in PQ key derivation. Validate that the parsed scheme is registered in `PqSignatureRegistry` before setting it.</violation>
</file>

<file name="framework/src/main/java/org/tron/core/consensus/ConsensusService.java">

<violation number="1" location="framework/src/main/java/org/tron/core/consensus/ConsensusService.java:84">
P2: Validate `pqScheme` before calling `PqSignatureRegistry.fromSeed`; otherwise an unsupported enum value can abort witness startup with `IllegalArgumentException`.</violation>
</file>

<file name="actuator/src/main/java/org/tron/core/actuator/AccountPermissionUpdateActuator.java">

<violation number="1" location="actuator/src/main/java/org/tron/core/actuator/AccountPermissionUpdateActuator.java:298">
P1: Witness permission validation is now too permissive: it accepts any registered PQ scheme (including ML_DSA_44) instead of restricting witness keys to ML_DSA_65.</violation>
</file>

<file name="chainbase/src/main/java/org/tron/common/utils/LocalWitnesses.java">

<violation number="1" location="chainbase/src/main/java/org/tron/common/utils/LocalWitnesses.java:44">
P2: `pqScheme` is settable to unsupported signature schemes, which lets invalid config pass initialization and then crash at runtime during PQ key derivation. Restrict this setter to ML_DSA_44/ML_DSA_65 (and reject null).</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread framework/src/main/java/org/tron/core/config/args/Args.java Outdated
Comment thread chainbase/src/main/java/org/tron/common/utils/LocalWitnesses.java
@Federico2014 Federico2014 force-pushed the feat/add-pqc-signature branch from 51b9de1 to 51af6b6 Compare April 26, 2026 15:00
Micro-benchmark comparing keygen/sign/verify latency for secp256k1
ECKey, ML-DSA-44 and ML-DSA-65, for tracking PQ signature integration
performance characteristics across releases.
Introduce two TVM precompiles for post-quantum signature verification,
gated on the existing ALLOW_ML_DSA proposal:
- 0x12 VerifyMlDsa44: ML-DSA-44 (FIPS-204, SHAKE256), 4500 energy.
  Compatible with EIP-8051 0x12 at the algorithm level; uses raw
  1312-byte public keys (BouncyCastle-native form) rather than the
  20512-byte expanded form, so wire bytes differ.
- 0x14 VerifyMlDsa65: ML-DSA-65 (TRON extension), 7000 energy.

Input layout for both: [msg 32B | signature | publicKey], output is a
32-byte word (1 on success, 0 otherwise). Wires VMConfig.allowMlDsa()
through ConfigLoader so the flag is loaded from the store on startup.
@Federico2014 Federico2014 changed the title feat(crypto): integrate ML-DSA post-quantum signatures end-to-end feat(crypto): integrate post-quantum signatures end-to-end Apr 27, 2026
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java">

<violation number="1" location="actuator/src/main/java/org/tron/core/vm/PrecompiledContracts.java:2247">
P2: Javadoc claims short inputs are "right-padded with zeros" but the implementation rejects them (returns 0). Smart contract developers relying on the documented padding behavior will get silent verification failures. Either remove the padding claim from the doc or implement the padding.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.


/**
* Verifies an ML-DSA-44 signature (FIPS-204, SHAKE256). Input layout (right-padded with
* zeros if shorter): [msg 32B | signature 2420B | publicKey 1312B] = 3764B. Returns a
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 27, 2026

Choose a reason for hiding this comment

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

P2: Javadoc claims short inputs are "right-padded with zeros" but the implementation rejects them (returns 0). Smart contract developers relying on the documented padding behavior will get silent verification failures. Either remove the padding claim from the doc or implement the padding.

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/PrecompiledContracts.java, line 2247:

<comment>Javadoc claims short inputs are "right-padded with zeros" but the implementation rejects them (returns 0). Smart contract developers relying on the documented padding behavior will get silent verification failures. Either remove the padding claim from the doc or implement the padding.</comment>

<file context>
@@ -2221,4 +2242,72 @@ public Pair<Boolean, byte[]> execute(byte[] data) {
 
+  /**
+   * Verifies an ML-DSA-44 signature (FIPS-204, SHAKE256). Input layout (right-padded with
+   * zeros if shorter): [msg 32B | signature 2420B | publicKey 1312B] = 3764B. Returns a
+   * 32-byte word: 1 on success, 0 on failure or malformed input.
+   */
</file context>
Fix with Cubic

@Federico2014 Federico2014 changed the title feat(crypto): integrate post-quantum signatures end-to-end feat(crypto): integrate MLDSA post-quantum signatures Apr 27, 2026
@Federico2014 Federico2014 changed the title feat(crypto): integrate MLDSA post-quantum signatures feat(crypto): ML-DSA + FN-DSA post-quantum signature support Apr 27, 2026
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 21 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="framework/src/main/java/org/tron/core/config/args/Args.java">

<violation number="1" location="framework/src/main/java/org/tron/core/config/args/Args.java:1247">
P2: Remove the locally maintained WITNESS_PQ_SEED_SCHEMES gate and rely on LocalWitnesses.setPqScheme, which already validates via PqSignatureRegistry; hard-coding the list here can reject newly registered schemes and desynchronize configuration handling.

(Based on your team's feedback about relying on LocalWitnesses.setPqScheme for PQ scheme validation.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

String schemeName = config.getString(ConfigKey.LOCAL_WITNESS_SEED_PQ_SCHEME);
try {
SignatureScheme scheme = SignatureScheme.valueOf(schemeName);
if (!WITNESS_PQ_SEED_SCHEMES.contains(scheme)) {
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 27, 2026

Choose a reason for hiding this comment

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

P2: Remove the locally maintained WITNESS_PQ_SEED_SCHEMES gate and rely on LocalWitnesses.setPqScheme, which already validates via PqSignatureRegistry; hard-coding the list here can reject newly registered schemes and desynchronize configuration handling.

(Based on your team's feedback about relying on LocalWitnesses.setPqScheme for PQ scheme validation.)

View Feedback

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 1247:

<comment>Remove the locally maintained WITNESS_PQ_SEED_SCHEMES gate and rely on LocalWitnesses.setPqScheme, which already validates via PqSignatureRegistry; hard-coding the list here can reject newly registered schemes and desynchronize configuration handling.

(Based on your team's feedback about relying on LocalWitnesses.setPqScheme for PQ scheme validation.) </comment>

<file context>
@@ -1234,7 +1243,13 @@ private static void initLocalWitnesses(Config config, CLIParameter cmd) {
           try {
-            localWitnesses.setPqScheme(SignatureScheme.valueOf(schemeName));
+            SignatureScheme scheme = SignatureScheme.valueOf(schemeName);
+            if (!WITNESS_PQ_SEED_SCHEMES.contains(scheme)) {
+              throw new TronError("invalid " + ConfigKey.LOCAL_WITNESS_SEED_PQ_SCHEME
+                  + ": " + schemeName + "; valid values: " + WITNESS_PQ_SEED_SCHEMES,
</file context>
Fix with Cubic

@Federico2014 Federico2014 changed the title feat(crypto): ML-DSA + FN-DSA post-quantum signature support feat(crypto): ML-DSA + FN-DSA post-quantum signatures and TVM precompiles Apr 27, 2026
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="framework/src/test/java/org/tron/common/runtime/vm/FnDsaPrecompileTest.java">

<violation number="1" location="framework/src/test/java/org/tron/common/runtime/vm/FnDsaPrecompileTest.java:131">
P3: This test is too short to reach the `sig_len == 0` check, so it doesn't actually validate zero-length signatures.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

FNDSA key = new FNDSA();
byte[] pk = key.getPublicKey();
// sig_len = 0 is invalid (must be >= 1)
byte[] input = new byte[32 + 2 + pk.length];
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 27, 2026

Choose a reason for hiding this comment

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

P3: This test is too short to reach the sig_len == 0 check, so it doesn't actually validate zero-length signatures.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/test/java/org/tron/common/runtime/vm/FnDsaPrecompileTest.java, line 131:

<comment>This test is too short to reach the `sig_len == 0` check, so it doesn't actually validate zero-length signatures.</comment>

<file context>
@@ -0,0 +1,185 @@
+    FNDSA key = new FNDSA();
+    byte[] pk = key.getPublicKey();
+    // sig_len = 0 is invalid (must be >= 1)
+    byte[] input = new byte[32 + 2 + pk.length];
+    System.arraycopy(MESSAGE_HASH, 0, input, 0, 32);
+    // sig_len bytes = 0x00 0x00 → sigLen = 0
</file context>
Fix with Cubic

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 16 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="framework/src/main/java/org/tron/core/db/Manager.java">

<violation number="1" location="framework/src/main/java/org/tron/core/db/Manager.java:1798">
P2: `witnessAddress` and `witnessPermission` are now unused in this method after the removal of `signerAddress`. This performs an unnecessary database read and protobuf deserialization on every block signing. Remove the dead code.</violation>
</file>

<file name="protocol/src/main/protos/core/Tron.proto">

<violation number="1" location="protocol/src/main/protos/core/Tron.proto:20">
P3: Comment references non-existent `PqWitness` — the message is actually `PqAuthWitness`. Also, `pq_witness / pq_witness` is redundant; consider `Transaction.pq_witness / BlockHeader.pq_witness` for clarity.</violation>
</file>

<file name="actuator/src/main/java/org/tron/core/actuator/AccountPermissionUpdateActuator.java">

<violation number="1" location="actuator/src/main/java/org/tron/core/actuator/AccountPermissionUpdateActuator.java:133">
P2: Empty addresses should not be allowed for `UNKNOWN_SIG_SCHEME` (legacy ECDSA) keys since the address is the identity used for signature verification. A key with empty address and no PQ key passes all validation but can never sign, potentially creating an unsatisfiable permission. Consider requiring a valid address when the key's scheme is `UNKNOWN_SIG_SCHEME`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

"witness permission requires " + scheme
+ " but local PQ private key is not configured");
}
byte[] digest = PqAuthDigest.block(blockCapsule.getRawHashBytes(), 0);
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 28, 2026

Choose a reason for hiding this comment

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

P2: witnessAddress and witnessPermission are now unused in this method after the removal of signerAddress. This performs an unnecessary database read and protobuf deserialization on every block signing. Remove the dead code.

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/db/Manager.java, line 1798:

<comment>`witnessAddress` and `witnessPermission` are now unused in this method after the removal of `signerAddress`. This performs an unnecessary database read and protobuf deserialization on every block signing. Remove the dead code.</comment>

<file context>
@@ -1793,14 +1795,12 @@ private void signWitnessAuth(BlockCapsule blockCapsule, Miner miner, SignatureSc
     }
-    byte[] signerAddress = witnessPermission.getKeys(0).getAddress().toByteArray();
-    byte[] digest = PqAuthDigest.block(blockCapsule.getRawHashBytes(), signerAddress);
+    byte[] digest = PqAuthDigest.block(blockCapsule.getRawHashBytes(), 0);
     byte[] signature = PqSignatureRegistry.sign(scheme, pqPrivateKey, digest);
-    AuthWitness witnessAuth = AuthWitness.newBuilder()
</file context>
Fix with Cubic

Comment on lines +133 to +134
if (!key.getAddress().isEmpty()
&& !DecodeUtil.addressValid(key.getAddress().toByteArray())) {
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 28, 2026

Choose a reason for hiding this comment

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

P2: Empty addresses should not be allowed for UNKNOWN_SIG_SCHEME (legacy ECDSA) keys since the address is the identity used for signature verification. A key with empty address and no PQ key passes all validation but can never sign, potentially creating an unsatisfiable permission. Consider requiring a valid address when the key's scheme is UNKNOWN_SIG_SCHEME.

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/actuator/AccountPermissionUpdateActuator.java, line 133:

<comment>Empty addresses should not be allowed for `UNKNOWN_SIG_SCHEME` (legacy ECDSA) keys since the address is the identity used for signature verification. A key with empty address and no PQ key passes all validation but can never sign, potentially creating an unsatisfiable permission. Consider requiring a valid address when the key's scheme is `UNKNOWN_SIG_SCHEME`.</comment>

<file context>
@@ -125,7 +130,8 @@ private boolean checkPermission(Permission permission) throws ContractValidateEx
 
     for (Key key : permission.getKeysList()) {
-      if (!DecodeUtil.addressValid(key.getAddress().toByteArray())) {
+      if (!key.getAddress().isEmpty()
+          && !DecodeUtil.addressValid(key.getAddress().toByteArray())) {
         throw new ContractValidateException("key is not a validate address");
</file context>
Suggested change
if (!key.getAddress().isEmpty()
&& !DecodeUtil.addressValid(key.getAddress().toByteArray())) {
SignatureScheme scheme = keyScheme(key);
if (scheme == SignatureScheme.UNKNOWN_SIG_SCHEME
&& !DecodeUtil.addressValid(key.getAddress().toByteArray())) {
throw new ContractValidateException("key is not a validate address");
}
if (scheme != SignatureScheme.UNKNOWN_SIG_SCHEME
&& !key.getAddress().isEmpty()
&& !DecodeUtil.addressValid(key.getAddress().toByteArray())) {
Fix with Cubic

Comment thread protocol/src/main/protos/core/Tron.proto Outdated
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 30 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="protocol/src/main/protos/core/Tron.proto">

<violation number="1" location="protocol/src/main/protos/core/Tron.proto:271">
P2: Keep protobuf field 4 reserved in `Key` to preserve forward/backward compatibility guarantees and prevent accidental tag reuse.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

bytes address = 1; // empty for PQ-only keys
int64 weight = 2;
// Post-quantum key. Absent for legacy keys.
PQPublicKey pq_key = 3;
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot Apr 28, 2026

Choose a reason for hiding this comment

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

P2: Keep protobuf field 4 reserved in Key to preserve forward/backward compatibility guarantees and prevent accidental tag reuse.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At protocol/src/main/protos/core/Tron.proto, line 271:

<comment>Keep protobuf field 4 reserved in `Key` to preserve forward/backward compatibility guarantees and prevent accidental tag reuse.</comment>

<file context>
@@ -268,15 +268,14 @@ message Key {
   // Post-quantum key. Absent for legacy keys.
-  PqPublicKey pq_key = 3;
-  reserved 4;              // was public_key; merged into pq_key
+  PQPublicKey pq_key = 3;
 }
 
</file context>
Suggested change
PQPublicKey pq_key = 3;
PQPublicKey pq_key = 3;
reserved 4; // was public_key; merged into pq_key
Fix with Cubic

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant