feat(crypto): add FN-DSA / Falcon-512 post-quantum signature support#23
Open
Federico2014 wants to merge 14 commits intorelease_pqc_basefrom
Open
feat(crypto): add FN-DSA / Falcon-512 post-quantum signature support#23Federico2014 wants to merge 14 commits intorelease_pqc_basefrom
Federico2014 wants to merge 14 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 |
Federico2014
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 46 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="framework/src/main/java/org/tron/core/consensus/ConsensusService.java">
<violation number="1" location="framework/src/main/java/org/tron/core/consensus/ConsensusService.java:143">
P3: The `requireSupportedPqScheme` check is redundant — `LocalWitnesses.setPqScheme()` already validates the scheme against `PQSchemeRegistry` at config-load time (throwing `TronError(WITNESS_INIT)` for unsupported schemes), and `PQSchemeRegistry.fromSeed()` would throw `IllegalArgumentException` regardless. Consider removing this defensive check and relying on the setter's validation as the single validation point.
(Based on your team's feedback about relying on LocalWitnesses.setPqScheme to validate the PQ signature scheme.) [FEEDBACK_USED]</violation>
</file>
<file name="protocol/src/main/protos/core/contract/account_contract.proto">
<violation number="1" location="protocol/src/main/protos/core/contract/account_contract.proto:30">
P2: Reserve the deleted field name as well as tag number to prevent accidental `pq_key` reuse in future schema changes.</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:1242">
P2: Remove the hard-coded PQ scheme whitelist in `Args` and rely on `localWitnesses.setPqScheme(scheme)` for validation.
(Based on your team's feedback about using PQ scheme registry/setter as the single source of truth.) [FEEDBACK_USED]</violation>
</file>
<file name="crypto/src/main/java/org/tron/common/crypto/pqc/PQAuthDigest.java">
<violation number="1" location="crypto/src/main/java/org/tron/common/crypto/pqc/PQAuthDigest.java:26">
P1: Make domain byte arrays private to prevent package-level mutation of digest domain separators.</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:933">
P3: Don’t swallow the other checked exceptions here; they can make this test pass on unrelated failures and hide a broken cap check.</violation>
</file>
<file name="chainbase/src/main/java/org/tron/core/db/BandwidthProcessor.java">
<violation number="1" location="chainbase/src/main/java/org/tron/core/db/BandwidthProcessor.java:150">
P1: When PQ witnesses are present, this overwrites instead of accumulates signature overhead; mixed legacy+PQ transactions will have create-account size miscomputed and can be rejected incorrectly.</violation>
</file>
<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 input is too short to exercise the `sig_len == 0` check. Increase it to at least 931 bytes so the test actually covers the zero-length-signature path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| public static final String TX_DOMAIN = "TRON_TX_AUTH_V1"; | ||
| public static final String BLOCK_DOMAIN = "TRON_BLOCK_AUTH_V1"; | ||
|
|
||
| static final byte[] TX_DOMAIN_BYTES = TX_DOMAIN.getBytes(StandardCharsets.UTF_8); |
There was a problem hiding this comment.
P1: Make domain byte arrays private to prevent package-level mutation of digest domain separators.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crypto/src/main/java/org/tron/common/crypto/pqc/PQAuthDigest.java, line 26:
<comment>Make domain byte arrays private to prevent package-level mutation of digest domain separators.</comment>
<file context>
@@ -0,0 +1,73 @@
+ public static final String TX_DOMAIN = "TRON_TX_AUTH_V1";
+ public static final String BLOCK_DOMAIN = "TRON_BLOCK_AUTH_V1";
+
+ static final byte[] TX_DOMAIN_BYTES = TX_DOMAIN.getBytes(StandardCharsets.UTF_8);
+ static final byte[] BLOCK_DOMAIN_BYTES = BLOCK_DOMAIN.getBytes(StandardCharsets.UTF_8);
+
</file context>
| : trx.getInstance().getPqWitnessList()) { | ||
| pqWitnessBytes += aw.getSerializedSize(); | ||
| } | ||
| sigOverhead = pqWitnessBytes; |
There was a problem hiding this comment.
P1: When PQ witnesses are present, this overwrites instead of accumulates signature overhead; mixed legacy+PQ transactions will have create-account size miscomputed and can be rejected incorrectly.
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/db/BandwidthProcessor.java, line 150:
<comment>When PQ witnesses are present, this overwrites instead of accumulates signature overhead; mixed legacy+PQ transactions will have create-account size miscomputed and can be rejected incorrectly.</comment>
<file context>
@@ -140,8 +140,17 @@ public void consume(TransactionCapsule trx, TransactionTrace trace)
+ : trx.getInstance().getPqWitnessList()) {
+ pqWitnessBytes += aw.getSerializedSize();
+ }
+ sigOverhead = pqWitnessBytes;
+ }
long createAccountBytesSize = trx.getInstance().toBuilder().clearRet()
</file context>
Suggested change
| sigOverhead = pqWitnessBytes; | |
| sigOverhead += pqWitnessBytes; |
| bytes owner_address = 1; | ||
| bytes account_address = 2; | ||
| AccountType type = 3; | ||
| reserved 4; // formerly PQPublicKey pq_key — Falcon-native account creation deferred per V2 scope |
There was a problem hiding this comment.
P2: Reserve the deleted field name as well as tag number to prevent accidental pq_key reuse in future schema changes.
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/contract/account_contract.proto, line 30:
<comment>Reserve the deleted field name as well as tag number to prevent accidental `pq_key` reuse in future schema changes.</comment>
<file context>
@@ -27,6 +27,7 @@ message AccountCreateContract {
bytes owner_address = 1;
bytes account_address = 2;
AccountType type = 3;
+ reserved 4; // formerly PQPublicKey pq_key — Falcon-native account creation deferred per V2 scope
}
</file context>
Suggested change
| reserved 4; // formerly PQPublicKey pq_key — Falcon-native account creation deferred per V2 scope | |
| reserved 4; | |
| reserved "pq_key"; // formerly PQPublicKey pq_key — Falcon-native account creation deferred per V2 scope |
Comment on lines
+1242
to
+1247
| if (!WITNESS_PQ_SEED_SCHEMES.contains(scheme)) { | ||
| throw new TronError("invalid " + ConfigKey.LOCAL_WITNESS_SEED_PQ_SCHEME | ||
| + ": " + schemeName + "; valid values: " + WITNESS_PQ_SEED_SCHEMES, | ||
| TronError.ErrCode.WITNESS_INIT); | ||
| } | ||
| localWitnesses.setPqScheme(scheme); |
There was a problem hiding this comment.
P2: Remove the hard-coded PQ scheme whitelist in Args and rely on localWitnesses.setPqScheme(scheme) for validation.
(Based on your team's feedback about using PQ scheme registry/setter as the single source of truth.)
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 1242:
<comment>Remove the hard-coded PQ scheme whitelist in `Args` and rely on `localWitnesses.setPqScheme(scheme)` for validation.
(Based on your team's feedback about using PQ scheme registry/setter as the single source of truth.) </comment>
<file context>
@@ -1220,6 +1229,36 @@ private static void initLocalWitnesses(Config config, CLIParameter cmd) {
+ String schemeName = config.getString(ConfigKey.LOCAL_WITNESS_SEED_PQ_SCHEME);
+ try {
+ PQScheme scheme = PQScheme.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>
Suggested change
| if (!WITNESS_PQ_SEED_SCHEMES.contains(scheme)) { | |
| throw new TronError("invalid " + ConfigKey.LOCAL_WITNESS_SEED_PQ_SCHEME | |
| + ": " + schemeName + "; valid values: " + WITNESS_PQ_SEED_SCHEMES, | |
| TronError.ErrCode.WITNESS_INIT); | |
| } | |
| localWitnesses.setPqScheme(scheme); | |
| localWitnesses.setPqScheme(scheme); |
| return miner; | ||
| } | ||
|
|
||
| private static void requireSupportedPqScheme(PQScheme scheme) { |
There was a problem hiding this comment.
P3: The requireSupportedPqScheme check is redundant — LocalWitnesses.setPqScheme() already validates the scheme against PQSchemeRegistry at config-load time (throwing TronError(WITNESS_INIT) for unsupported schemes), and PQSchemeRegistry.fromSeed() would throw IllegalArgumentException regardless. Consider removing this defensive check and relying on the setter's validation as the single validation point.
(Based on your team's feedback about relying on LocalWitnesses.setPqScheme to validate the PQ signature scheme.)
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/consensus/ConsensusService.java, line 143:
<comment>The `requireSupportedPqScheme` check is redundant — `LocalWitnesses.setPqScheme()` already validates the scheme against `PQSchemeRegistry` at config-load time (throwing `TronError(WITNESS_INIT)` for unsupported schemes), and `PQSchemeRegistry.fromSeed()` would throw `IllegalArgumentException` regardless. Consider removing this defensive check and relying on the setter's validation as the single validation point.
(Based on your team's feedback about relying on LocalWitnesses.setPqScheme to validate the PQ signature scheme.) </comment>
<file context>
@@ -85,6 +114,39 @@ public void start() {
+ return miner;
+ }
+
+ private static void requireSupportedPqScheme(PQScheme scheme) {
+ if (!PQSchemeRegistry.contains(scheme)) {
+ throw new TronError("unsupported PQ witness scheme: " + scheme,
</file context>
Comment on lines
+933
to
+938
| } catch (TooBigTransactionException e) { | ||
| Assert.fail("PQ pq_witness bytes should be deducted from create-account cap check"); | ||
| } catch (AccountResourceInsufficientException | ||
| | ContractValidateException | ||
| | TooBigTransactionResultException e) { | ||
| // acceptable: other code paths — we only care about the cap check |
There was a problem hiding this comment.
P3: Don’t swallow the other checked exceptions here; they can make this test pass on unrelated failures and hide a broken 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 933:
<comment>Don’t swallow the other checked exceptions here; they can make this test pass on unrelated failures and hide a broken cap check.</comment>
<file context>
@@ -881,4 +881,124 @@ public void testCalculateGlobalNetLimit() {
+ BandwidthProcessor processor = new BandwidthProcessor(chainBaseManager);
+ try {
+ processor.consume(trx, trace);
+ } catch (TooBigTransactionException e) {
+ Assert.fail("PQ pq_witness bytes should be deducted from create-account cap check");
+ } catch (AccountResourceInsufficientException
</file context>
Suggested change
| } catch (TooBigTransactionException e) { | |
| Assert.fail("PQ pq_witness bytes should be deducted from create-account cap check"); | |
| } catch (AccountResourceInsufficientException | |
| | ContractValidateException | |
| | TooBigTransactionResultException e) { | |
| // acceptable: other code paths — we only care about the cap check | |
| } catch (TooBigTransactionException e) { | |
| Assert.fail("PQ pq_witness bytes should be deducted from create-account cap check"); |
| 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 input is too short to exercise the sig_len == 0 check. Increase it to at least 931 bytes so the test actually covers the zero-length-signature path.
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 input is too short to exercise the `sig_len == 0` check. Increase it to at least 931 bytes so the test actually covers the zero-length-signature path.</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>
Suggested change
| byte[] input = new byte[32 + 2 + pk.length]; | |
| byte[] input = new byte[32 + 2 + pk.length + 1]; |
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.
What does this PR do?
Adds post-quantum (PQ) signature support to TRON, landing the V2 design with FN-DSA / Falcon-512 (FIPS-206 draft) as the launch scheme.
Highlights:
PQSchemeproto enum (FN_DSA_512 = 0as the proto3 default — never appears on the wire, zero overhead for the scheme tag).PQWitness { scheme, public_key, signature }carried alongside the existing ECDSA witness — multi-sig transactions may mix ECDSA and PQ witnesses, each contributing weight independently to the permission threshold.0x41 ‖ SHA-256(public_key)[0:20], distinct from the Keccak-256 ECDSA derivation so PQ and ECDSA address spaces cannot collide.PQSchemeRegistrystatic dispatch (publicKey/signature/seed lengths, fingerprint hash, sign / verify / fromSeed) — leaves room for additional PQ schemes without further protocol churn.0x16 verifyFnDsa512(EIP-8052 family), gated by theALLOW_FN_DSAproposal; legacy ECDSA path unchanged.CreateAccountActuatorandAccountPermissionUpdateActuator.BlockHeaderfor SR dual-sign during the transition;ManagerandConsensusServiceupdated to verify PQ witnesses when set.localwitness_seed_pq_scheme+localwitness_seed_pq(currentlyFN_DSA_512).FNDSAKatTest) pins seed → keypair derivation against BouncyCastle 1.79; benchmark test (SignatureSchemeBenchmarkTest) tracks keygen / sign / verify latency vs. ECKey.Why are these changes required?
NIST has standardised lattice-based signatures (FIPS-204 ML-DSA, FIPS-206 FN-DSA). TRON needs a wire-format and consensus path for PQ signatures so that key-rotation and dual-signing strategies can be enabled before quantum capability becomes a credible threat. V2 finalises the witness layout (embedded public key, address-as-fingerprint) so a PQ-native account requires no on-chain ECDSA preimage at any point in its lifecycle.
This PR has been tested by:
FNDSATest— sign / verify, malformed input, length boundsFNDSAKatTest— 5 seed vectors pinning pk / sk SHA-256 + 21-byte address (regression detection on BC 1.79 seeding)PQAuthDigestTest— domain separation for tx and block authenticationBlockCapsulePQTest— PQ block-header sign / verify pathTransactionCapsuleTest::transactionSizeComparisonByScheme— tx-size regression (175 B ECDSA → ~1670 B Falcon-512 V2 TRX)AccountPermissionUpdateActuatorTest,CreateAccountActuatorTest— PQ-native permission and account pathsSignatureSchemeBenchmarkTest— keygen 21.7 ms, sign 1.9 ms, verify 0.11 ms (Falcon-512) vs. 0.56 / 3.18 / 1.20 ms (ECKey) on macOS arm64 / JDK 17 / BC 1.79, 500 iterPQFullNode/PQWitnessNode/PQClientFollow up
ALLOW_FN_DSAproposal (Feature/change util capsule tronprotocol/java-tron#100); emergencyFORBID_ECDSA_SIGNproposal kept as a kill-switch.Extra details
PQSchemeRegistrylater without further protocol changes.