feat(crypto): add MLDSA / FN-DSA / SLH-DSA / Ephemeral secp256k1 post-quantum schemes#22
Open
Federico2014 wants to merge 18 commits intorelease_pqc_basefrom
Open
feat(crypto): add MLDSA / FN-DSA / SLH-DSA / Ephemeral secp256k1 post-quantum schemes#22Federico2014 wants to merge 18 commits intorelease_pqc_basefrom
Federico2014 wants to merge 18 commits intorelease_pqc_basefrom
Conversation
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.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found across 54 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:136">
P1: Seed length validation is hardcoded to `MLDSA65.SEED_LENGTH` (32 bytes) but FN-DSA and SLH-DSA require 48-byte seeds. A witness configured with `pqScheme = FN_DSA` or `SLH_DSA` will fail to start because its valid 96-hex-char seed gets rejected here. The validation should derive the expected length from the configured `pqScheme` (or accept a scheme parameter).</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:737">
P2: Redundant `computeRawHash(transaction)` call: the txid is already computed in `validateStructuredSignature` (line 737) and should be passed as a parameter to `ephemeralPreVerifyChecks` rather than recomputed. This adds an unnecessary SHA-256 hash per ephemeral witness on the transaction validation hot path.</violation>
</file>
<file name="common/src/main/java/org/tron/core/Constant.java">
<violation number="1" location="common/src/main/java/org/tron/core/Constant.java:64">
P2: The new PQ constants are currently unused and duplicate existing authoritative constants in crypto/capsule classes, creating a second source of truth.</violation>
</file>
<file name="actuator/src/main/java/org/tron/core/utils/ProposalUtil.java">
<violation number="1" location="actuator/src/main/java/org/tron/core/utils/ProposalUtil.java:889">
P1: Add fork-version gating for the new PQ proposal IDs; they currently bypass activation checks used by other recently added governance params.</violation>
</file>
<file name="framework/src/main/resources/config.conf">
<violation number="1" location="framework/src/main/resources/config.conf:778">
P2: The added committee config hint uses deprecated key `allowMlDsa`; update it to the current per-scheme keys to avoid operator misconfiguration.</violation>
</file>
<file name="framework/src/test/java/org/tron/core/BandwidthProcessorTest.java">
<violation number="1" location="framework/src/test/java/org/tron/core/BandwidthProcessorTest.java:934">
P2: Don't swallow unrelated exceptions here; the test can pass without exercising the create-account cap check.</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:312">
P2: `expectedPublicKeyLength` duplicates `PqSignatureRegistry.getPublicKeyLength`; use the registry as the single source of truth. If a new scheme is added to the registry but not to this switch, valid keys will be rejected. Replace the manual switch with a `PqSignatureRegistry.contains` + `getPublicKeyLength` call.
(Based on your team's feedback about treating the registry as the single source of truth for allowed schemes.) [FEEDBACK_USED]</violation>
</file>
<file name="framework/src/test/java/org/tron/core/capsule/BlockCapsulePqTest.java">
<violation number="1" location="framework/src/test/java/org/tron/core/capsule/BlockCapsulePqTest.java:26">
P2: This stateful test class should not extend `BaseTest`; use `BaseMethodTest` so each test gets an isolated context.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (StringUtils.startsWith(hex, "0x")) { | ||
| hex = hex.substring(2); | ||
| } | ||
| int expectedHexLen = MLDSA65.SEED_LENGTH * 2; |
There was a problem hiding this comment.
P1: Seed length validation is hardcoded to MLDSA65.SEED_LENGTH (32 bytes) but FN-DSA and SLH-DSA require 48-byte seeds. A witness configured with pqScheme = FN_DSA or SLH_DSA will fail to start because its valid 96-hex-char seed gets rejected here. The validation should derive the expected length from the configured pqScheme (or accept a scheme parameter).
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/utils/LocalWitnesses.java, line 136:
<comment>Seed length validation is hardcoded to `MLDSA65.SEED_LENGTH` (32 bytes) but FN-DSA and SLH-DSA require 48-byte seeds. A witness configured with `pqScheme = FN_DSA` or `SLH_DSA` will fail to start because its valid 96-hex-char seed gets rejected here. The validation should derive the expected length from the configured `pqScheme` (or accept a scheme parameter).</comment>
<file context>
@@ -95,6 +116,35 @@ public void addPrivateKeys(String privateKey) {
+ if (StringUtils.startsWith(hex, "0x")) {
+ hex = hex.substring(2);
+ }
+ int expectedHexLen = MLDSA65.SEED_LENGTH * 2;
+ if (StringUtils.isBlank(hex) || hex.length() != expectedHexLen) {
+ throw new TronError(String.format("ML-DSA seed must be %d hex chars, actual: %d",
</file context>
| } | ||
| break; | ||
| } | ||
| case ALLOW_ML_DSA_44: { |
There was a problem hiding this comment.
P1: Add fork-version gating for the new PQ proposal IDs; they currently bypass activation checks used by other recently added governance params.
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/utils/ProposalUtil.java, line 889:
<comment>Add fork-version gating for the new PQ proposal IDs; they currently bypass activation checks used by other recently added governance params.</comment>
<file context>
@@ -886,6 +886,41 @@ public static void validator(DynamicPropertiesStore dynamicPropertiesStore,
}
break;
}
+ case ALLOW_ML_DSA_44: {
+ if (value != 0 && value != 1) {
+ throw new ContractValidateException(
</file context>
| "permission uses legacy scheme, auth_witness is not allowed"); | ||
| } | ||
|
|
||
| byte[] txid = computeRawHash(transaction).getBytes(); |
There was a problem hiding this comment.
P2: Redundant computeRawHash(transaction) call: the txid is already computed in validateStructuredSignature (line 737) and should be passed as a parameter to ephemeralPreVerifyChecks rather than recomputed. This adds an unnecessary SHA-256 hash per ephemeral witness on the transaction validation hot path.
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/core/capsule/TransactionCapsule.java, line 737:
<comment>Redundant `computeRawHash(transaction)` call: the txid is already computed in `validateStructuredSignature` (line 737) and should be passed as a parameter to `ephemeralPreVerifyChecks` rather than recomputed. This adds an unnecessary SHA-256 hash per ephemeral witness on the transaction validation hot path.</comment>
<file context>
@@ -662,6 +704,183 @@ public boolean validatePubSignature(AccountStore accountStore,
+ "permission uses legacy scheme, auth_witness is not allowed");
+ }
+
+ byte[] txid = computeRawHash(transaction).getBytes();
+ List<AuthWitness> witnesses = transaction.getAuthWitnessList();
+ java.util.Set<ByteString> seen = new java.util.HashSet<>();
</file context>
| public static final String ECKey_ENGINE = "ECKey"; | ||
|
|
||
| // Post-quantum (ML-DSA / FIPS 204) signature constants | ||
| public static final int ML_DSA_44_PUBLIC_KEY_LENGTH = 1312; |
There was a problem hiding this comment.
P2: The new PQ constants are currently unused and duplicate existing authoritative constants in crypto/capsule classes, creating a second source of truth.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At common/src/main/java/org/tron/core/Constant.java, line 64:
<comment>The new PQ constants are currently unused and duplicate existing authoritative constants in crypto/capsule classes, creating a second source of truth.</comment>
<file context>
@@ -60,6 +60,25 @@ public class Constant {
public static final String ECKey_ENGINE = "ECKey";
+ // Post-quantum (ML-DSA / FIPS 204) signature constants
+ public static final int ML_DSA_44_PUBLIC_KEY_LENGTH = 1312;
+ public static final int ML_DSA_44_SIGNATURE_LENGTH = 2420;
+ public static final int ML_DSA_65_PUBLIC_KEY_LENGTH = 1952;
</file context>
| # consensusLogicOptimization = 0 | ||
| # allowOptimizedReturnValueOfChainId = 0 | ||
| # allowTvmOsaka = 0 | ||
| # allowMlDsa = 0 |
There was a problem hiding this comment.
P2: The added committee config hint uses deprecated key allowMlDsa; update it to the current per-scheme keys to avoid operator misconfiguration.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At framework/src/main/resources/config.conf, line 778:
<comment>The added committee config hint uses deprecated key `allowMlDsa`; update it to the current per-scheme keys to avoid operator misconfiguration.</comment>
<file context>
@@ -760,6 +775,7 @@ committee = {
# consensusLogicOptimization = 0
# allowOptimizedReturnValueOfChainId = 0
# allowTvmOsaka = 0
+ # allowMlDsa = 0
}
</file context>
| processor.consume(trx, trace); | ||
| } catch (TooBigTransactionException e) { | ||
| Assert.fail("PQ auth_witness bytes should be deducted from create-account cap check"); | ||
| } catch (AccountResourceInsufficientException |
There was a problem hiding this comment.
P2: Don't swallow unrelated exceptions here; the test can pass without exercising the create-account cap check.
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/core/BandwidthProcessorTest.java, line 934:
<comment>Don't swallow unrelated exceptions here; the test can pass without exercising the create-account cap check.</comment>
<file context>
@@ -881,4 +881,122 @@ public void testCalculateGlobalNetLimit() {
+ processor.consume(trx, trace);
+ } catch (TooBigTransactionException e) {
+ Assert.fail("PQ auth_witness bytes should be deducted from create-account cap check");
+ } catch (AccountResourceInsufficientException
+ | ContractValidateException
+ | TooBigTransactionResultException e) {
</file context>
| } | ||
| } | ||
|
|
||
| private static int expectedPublicKeyLength(SignatureScheme scheme) { |
There was a problem hiding this comment.
P2: expectedPublicKeyLength duplicates PqSignatureRegistry.getPublicKeyLength; use the registry as the single source of truth. If a new scheme is added to the registry but not to this switch, valid keys will be rejected. Replace the manual switch with a PqSignatureRegistry.contains + getPublicKeyLength call.
(Based on your team's feedback about treating the registry as the single source of truth for allowed schemes.)
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 312:
<comment>`expectedPublicKeyLength` duplicates `PqSignatureRegistry.getPublicKeyLength`; use the registry as the single source of truth. If a new scheme is added to the registry but not to this switch, valid keys will be rejected. Replace the manual switch with a `PqSignatureRegistry.contains` + `getPublicKeyLength` call.
(Based on your team's feedback about treating the registry as the single source of truth for allowed schemes.) </comment>
<file context>
@@ -237,4 +261,68 @@ public ByteString getOwnerAddress() throws InvalidProtocolBufferException {
+ }
+ }
+
+ private static int expectedPublicKeyLength(SignatureScheme scheme) {
+ switch (scheme) {
+ case ML_DSA_44:
</file context>
| @@ -0,0 +1,290 @@ | |||
| package org.tron.core.capsule; | |||
There was a problem hiding this comment.
P2: This stateful test class should not extend BaseTest; use BaseMethodTest so each test gets an isolated context.
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/core/capsule/BlockCapsulePqTest.java, line 26:
<comment>This stateful test class should not extend `BaseTest`; use `BaseMethodTest` so each test gets an isolated context.</comment>
<file context>
@@ -0,0 +1,290 @@
+import org.tron.protos.Protocol.Permission.PermissionType;
+import org.tron.protos.Protocol.SignatureScheme;
+
+public class BlockCapsulePqTest extends BaseTest {
+
+ private ECKey witnessKey;
</file context>
Federico2014
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Builds on the ML-DSA base in
release_pqc_baseand adds:ALLOW_ML_DSAintoALLOW_ML_DSA_44/ALLOW_ML_DSA_65, plus newALLOW_SLH_DSA,ALLOW_FN_DSA,ALLOW_EPHEMERAL_SECP256K1proposal flags. Witness-side allowlist tightened so only{ML_DSA_44, ML_DSA_65, FN_DSA, SLH_DSA}are validlocalwitness_seed_pq_schemevalues.PqSignatureRegistrywith the samefromSeed/verifyAPI as ML-DSA.Permission.Key.public_key,EphemeralWitnesscarried inAuthWitness.signature, per-account replay protection viaephemeral_used_bitmap(≤ 8 KiB) +last_ephemeral_nonce+Transaction.raw.nonce, with domain-separated digestTRON_EPHEMERAL_TX_AUTH_V1. IncludesMerkleTree(SHA-256, depth ≤ 20) andEphemeralSecp256k1(proof depth ≤ 16).0x12 VerifyMlDsa44(4500 energy) and0x14 VerifyMlDsa65(7000 energy), independently gated by their per-scheme flags.Test plan
:framework:checkstyleMainclean:actuator:compileJava :common:compileJava :framework:compileTestJavacleanMlDsaPrecompileTest,SignatureSchemeBenchmarkTest,EphemeralSecp256k1Test, ephemeral integration cases inTransactionCapsuleTest, replay-protection scenarios in chainbaseArgsTest/WitnessInitializerTeststill pass after the seed-scheme allowlist tightening:framework:testrun before mergeSummary by cubic
Adds new post‑quantum signature options (SLH‑DSA, FN‑DSA) and an Ephemeral secp256k1 auth path, plus ML‑DSA verify precompiles, all gated by per‑scheme governance flags. Introduces PQ auth witnesses for transactions/blocks and per‑account replay protection for the ephemeral scheme.
New Features
ALLOW_ML_DSA_44,ALLOW_ML_DSA_65,ALLOW_SLH_DSA,ALLOW_FN_DSA,ALLOW_EPHEMERAL_SECP256K1with strict proposal validation and runtime gating.PqSignatureRegistrywithfromSeed/verifyfor ML‑DSA‑44/65, SLH‑DSA‑SHA2‑128s, and FN‑DSA/Falcon‑512.Permission.Key.public_key,AuthWitnesscarriesEphemeralWitness, replay protection viaephemeral_used_bitmap/last_ephemeral_nonceandTransaction.raw.nonce(domainTRON_EPHEMERAL_TX_AUTH_V1).VerifyMlDsa44, 0x14VerifyMlDsa65, each gated by its flag.localwitness_seed_pq_schemeandlocalwitness_seed_pqwith allowlist enforcement; unsupported schemes fail fast at startup.Migration
ALLOW_*flags before using each scheme.committee.allowMlDsa44/committee.allowMlDsa65/committee.allowSlhDsa/committee.allowFnDsa/committee.allowEphemeralSecp256k1; setlocalwitness_seed_pq_schemeandlocalwitness_seed_pq(use the correct seed length per scheme). PQ‑only witnesses must setlocalWitnessAccountAddress.SignatureSchemeand expectedpublic_keyformat; ephemeral accounts must store the Merkle root and include nonces when signing. ML‑DSA verify precompiles expect input[msg32 | signature | publicKey].Written for commit d00d184. Summary will update on new commits. Review in cubic