feat(crypto): ML-DSA + FN-DSA post-quantum signatures and TVM precompiles#21
feat(crypto): ML-DSA + FN-DSA post-quantum signatures and TVM precompiles#21Federico2014 wants to merge 12 commits intorelease_pqc_basefrom
Conversation
|
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
51b9de1 to
51af6b6
Compare
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
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)) { |
There was a problem hiding this comment.
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.)
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>
There was a problem hiding this comment.
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]; |
There was a problem hiding this comment.
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>
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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>
| if (!key.getAddress().isEmpty() | ||
| && !DecodeUtil.addressValid(key.getAddress().toByteArray())) { |
There was a problem hiding this comment.
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>
| 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())) { |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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>
| PQPublicKey pq_key = 3; | |
| PQPublicKey pq_key = 3; | |
| reserved 4; // was public_key; merged into pq_key |
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
PQSignatureinterface +MLDSA44/MLDSA65implementationsPQSignatureRegistrystatic dispatch table (sign / verify / fromSeed)PQPublicKeysub-message inPermission.Keybundlesscheme+public_key; absent on legacy ECDSA keysPQAuthWitnessproto message carrieskey_id(position index into the permission's key list) +signature; replaces address-based witness lookupAccountPermissionUpdateActuator— validates PQ public-key lengths per scheme; supports mixed ECDSA+PQ key listsTransactionCapsule.validateStructuredSignature— resolves key bykey_id, verifies PQ auth witnessesBlockCapsule.validateWitnessAuth— verifies PQ block-signing witnessesALLOW_ML_DSAgovernance proposal (id 97);DynamicPropertiesStore.allowMlDsa()localwitness_seed_pq/localwitness_seed_pq_schemeFN-DSA / Falcon-512 (FIPS 206 draft)
FNDSA.java— BouncyCastle Falcon-512 keypair-bound signer/verifierPQSignatureRegistry.isValidSignatureLengthuses range check for FN-DSA, strict equality for ML-DSASignatureScheme.FN_DSA = 6in Tron.protoALLOW_FN_DSAgovernance proposal (id 100);DynamicPropertiesStore.allowFnDsa()isAnyPqSchemeAllowed(),isPqSchemeAllowed(SignatureScheme)isAnyPqSchemeAllowed()LocalWitnessesseed validation is scheme-aware viaPQSignatureRegistry.getSeedLength(scheme)— accepts 32-byte seeds for ML-DSA and 48-byte seeds for FN-DSATVM Precompiled Contracts
0x12msg 32B | sig 2420B | pk 1312B0x14msg 32B | sig 3309B | pk 1952B0x16msg 32B | sig_len 2B | sig <=752B | pk 896BEach precompile is gated by its corresponding
ALLOW_*governance flag.Protocol refactoring —
PQPublicKey+PQAuthWitness+key_idPQPublicKeysub-message (scheme+public_key) nested inKey.pq_key; ECDSA keys leavepq_keyabsentPQAuthWitness(renamed fromAuthWitness):key_id uint32replaces 21-bytesigner_address;key_id=0costs 0 wire bytes (proto3 default),key_id 1-127costs 2 bytes — optimal for realistic key countspq_witnessfield name used consistently in bothTransaction(repeated) andBlockHeader.raw(singular); replaces oldauth_witness/witness_authPQAuthDigest: domain-separated hash now takes(txid, permissionId, keyId)instead of a signer addressNaming convention
Class names and proto message names use uppercase PQ (
PQPublicKey,PQAuthWitness,PQSignature,PQSignatureRegistry,PQAuthDigest). Lowercasepq_*proto field names and Lombok-boundpqScheme/pqSeedsfields are kept as-is so generated accessors stay stable.Key byte-length constants
Test coverage
FNDSATest— 13 unit tests: keygen, sign/verify, variable-length bound, cross-algo rejection, registry dispatchAccountPermissionUpdateActuatorTest— PQ key validation: length, dedup, mixed ECDSA+PQ, FN-DSA casesTransactionCapsuleTest— accept / tampered / not-activated / duplicate key_id / key_id out-of-rangeBlockCapsulePQTest— PQ block witness sign/verify, legacy-sig rejection, key_id out-of-rangePQAuthDigestTest— domain-separation correctness fortx()andblock()digest variantsPQSignatureRegistryTest— registry dispatch + per-scheme metadataLocalWitnessesTest— per-scheme seed-length validation (ML-DSA 32B, FN-DSA 48B) and hex-format rejectionMlDsaPrecompileTest— 12 TVM precompile cases for 0x12 and 0x14FnDsaPrecompileTest— 11 TVM precompile cases for 0x16Test 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:checkstyleTestALLOW_ML_DSA(97)andALLOW_FN_DSA(100)key_id=0wire overhead is zero (default field omitted in proto3)ALLOW_FN_DSAis off