feat(pqc): add post-quantum signature support#1
Open
Federico2014 wants to merge 7 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds PQ signature primitives (FN-DSA-512 / ML-DSA-44), a scheme registry, WalletFile scheme and seed fields, keystore create/decrypt logic, Transaction PQ signing/validation, provider/hash wiring, CredentialsFalcon adapter, WalletApi/CLI integration, proto additions, docs, and unit tests. ChangesPost-Quantum Falcon & ML Signature Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/org/tron/keystore/WalletUtils.java (1)
198-204:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not silently ignore non-null scheme tags in credential dispatch.
Line 198 only handles
FN_DSA_512; any other non-null scheme currently falls through toisEckeyand may pick the wrong decryptor. Fail fast on unknown tags and honor explicitSM2/ECKEYtags.Proposed fix
public static Credentials loadCredentials(byte[] password, WalletFile walletFile) throws CipherException { - if ("FN_DSA_512".equals(walletFile.getScheme())) { + String scheme = walletFile.getScheme(); + if ("FN_DSA_512".equals(scheme)) { return CredentialsFalcon.create(Wallet.decryptPQ(password, walletFile)); } + if ("SM2".equals(scheme)) { + return CredentialsSM2.create(Wallet.decryptSM2(password, walletFile)); + } + if ("ECKEY".equals(scheme)) { + return CredentialsEckey.create(Wallet.decrypt(password, walletFile)); + } + if (scheme != null) { + throw new CipherException("Unsupported wallet scheme: " + scheme); + } if (isEckey) { return CredentialsEckey.create(Wallet.decrypt(password, walletFile)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/tron/keystore/WalletUtils.java` around lines 198 - 204, The dispatch currently only checks for "FN_DSA_512" and otherwise falls through to isEckey, which misroutes when walletFile.getScheme() is non-null but not "FN_DSA_512"; update WalletUtils to inspect walletFile.getScheme() and, when non-null, explicitly branch on known tags ("FN_DSA_512" -> call CredentialsFalcon.create with Wallet.decryptPQ, "SM2" -> call CredentialsSM2.create with Wallet.decryptSM2, "ECKEY" -> call CredentialsEckey.create with Wallet.decrypt); if the scheme is non-null and not one of these known values throw an IllegalArgumentException (fail fast); retain the original isEckey-based behavior only when scheme is null.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/tron/common/utils/TransactionUtils.java`:
- Around line 357-373: Update validation and signing checks for PQ auth: adjust
the transaction validator (validTransaction) so that transactions containing at
least one PQAuthSig (PQAuthSig entries on the transaction proto) are accepted
even when signatureCount == 0 (i.e., treat presence of pq_auth_sig as satisfying
signature requirements) and ensure you still validate PQAuthSig fields
(publicKey, signature non-empty) as before; and harden signPQ(Transaction,
FNDSA512, PQScheme) to reject unsupported/unknown scheme values (e.g., if scheme
== PQScheme.UNKNOWN_PQ_SCHEME or not one of the explicitly supported enums for
FNDSA512) by throwing an IllegalArgumentException (or similar) before signing,
referencing the signPQ overloads, PQAuthSig, and PQScheme symbols so callers
cannot stamp unsupported schemes onto Falcon signatures.
In `@src/main/java/org/tron/keystore/Wallet.java`:
- Around line 349-355: The code in createPQ builds sensitive byte arrays
(derivedKey, encryptKey, iv, cipherText) and returns without wiping them; update
the createPQ flow to use a try/finally (or try-with-resources equivalent) around
the derivation/encryption steps so that in the finally block you explicitly
overwrite and zero out derivedKey, encryptKey (Arrays.fill(..., (byte)0)), iv,
and any other temporary sensitive buffers (e.g., cipherText or
extendedPrivateKey) after createPQWalletFile is called; ensure you still return
the wallet file but perform the zeroing in finally to guarantee clearing on all
exit paths and reference the methods/vars generateDerivedScryptKey,
performCipherOperation, generateRandomBytes, generateMac, and createPQWalletFile
when locating the code to change.
- Around line 403-406: In decryptPQ in class Wallet, guard the PQScheme parsing
so an invalid or tampered scheme string doesn't throw an uncaught
IllegalArgumentException: wrap the PQScheme.valueOf(walletFile.getScheme()) call
in a try/catch that catches IllegalArgumentException (and NullPointerException
if needed) and rethrow a CipherException with a clear message like "Unsupported
or invalid PQ scheme: <scheme>" (include the original scheme string) so all
failures are surfaced as CipherException; update the existing check that
compares to PQScheme.FN_DSA_512 to run only after successful parsing.
In `@src/main/java/org/tron/walletcli/cli/commands/WalletCommands.java`:
- Around line 136-172: extendedPriv (the decoded extended private key) isn’t
wiped and can remain on the heap; ensure it is zeroed after use by adding a
finally block that calls Arrays.fill(extendedPriv, (byte)0) (guarded with a null
check) around the block that calls wrapper.importWalletPQForCli(...) and uses
result, so the byte[] is cleared on both success and failure; add the necessary
import for java.util.Arrays if missing.
In `@src/main/java/org/tron/walletcli/Client.java`:
- Around line 3238-3276: The password and extPriv buffers in registerWalletPQ
and importWalletPQ can remain in memory if walletApiWrapper.registerWalletPQ or
walletApiWrapper.importWalletPQ throws; move the sensitive-data clearing into
finally blocks so they always run. Concretely, wrap the wrapper calls
(walletApiWrapper.registerWalletPQ in registerWalletPQ;
walletApiWrapper.importWalletPQ in importWalletPQ) in try/finally and call
StringUtils.clear(password) in the finally of both methods, and in
importWalletPQ also call java.util.Arrays.fill(extPriv, (byte)0) (guarding for
null) in the finally so extPriv is zeroed even on exceptions. Ensure you don’t
clear password or extPriv before they’re used and handle the case where password
or extPriv may be null.
- Around line 3285-3305: The input loop in inputPQExtendedPrivateKey currently
accepts any read with len >= hexLen and silently ignores extra bytes; change the
validation so inputs longer than hexLen are rejected instead of truncated: after
reading into temp in inputPQExtendedPrivateKey, verify that the meaningful input
length equals hexLen (or, alternatively, trim trailing newline/whitespace and
then require exact length) and only call StringUtils.hexs2Bytes when the length
matches exactly; if len > hexLen (or trimmed length > hexLen) treat it as
invalid, clear temp via StringUtils.clear(temp), increment nTime and continue
retrying (using the existing retryTime), leaving the rest of the logic
unchanged.
In `@src/main/java/org/tron/walletcli/WalletApiWrapper.java`:
- Around line 252-297: Both registerWalletPQ and importWalletPQ create a
WalletFile but never set a friendly name; call nameWallet(walletFile, false)
after creating the WalletFile (after CreatePQWalletFile) and before calling
WalletApi.store2Keystore so the REPL shows a non-null friendly name. Apply this
to both registerWalletPQ (before keystoreName = WalletApi.store2Keystore(...))
and importWalletPQ (also before store2Keystore; keep the existing
isUnifiedExist()/wallet.getWalletList().add(...) and logout() logic unchanged).
In `@src/main/java/org/tron/walletserver/WalletApi.java`:
- Around line 546-548: The current checks (e.g., in isPQWallet and the branches
that call TransactionUtils.signPQ(...)) treat FN_DSA_512 as a hardcoded gate
instead of resolving the actual keystore scheme from walletFile.getScheme();
change the logic to parse/resolve the scheme enum once from
walletFile.getScheme() (e.g., map to PQScheme or equivalent) and pass that
resolved scheme into TransactionUtils.signPQ(...) so the signing path matches
the wallet metadata; if walletFile.getScheme() is null or cannot be resolved to
a known PQ scheme, fail closed (do not attempt PQ signing) and fall back to the
legacy EC/SM2 path only when appropriate (using the existing isEckey boolean).
In `@src/main/protos/core/Tron.proto`:
- Around line 543-547: The schema currently allows both or neither of the
mutually exclusive auth fields (witness_signature and pq_auth_sig), so change
each paired field group to a protobuf oneof (e.g., oneof auth { bytes
witness_signature = X; PQAuthSig pq_auth_sig = Y; }) to enforce “exactly one” at
the schema level; apply the same oneof refactor to the other pair referenced
(lines 661-663) and update any references to the fields to use the chosen oneof
name (e.g., auth) where necessary.
---
Outside diff comments:
In `@src/main/java/org/tron/keystore/WalletUtils.java`:
- Around line 198-204: The dispatch currently only checks for "FN_DSA_512" and
otherwise falls through to isEckey, which misroutes when walletFile.getScheme()
is non-null but not "FN_DSA_512"; update WalletUtils to inspect
walletFile.getScheme() and, when non-null, explicitly branch on known tags
("FN_DSA_512" -> call CredentialsFalcon.create with Wallet.decryptPQ, "SM2" ->
call CredentialsSM2.create with Wallet.decryptSM2, "ECKEY" -> call
CredentialsEckey.create with Wallet.decrypt); if the scheme is non-null and not
one of these known values throw an IllegalArgumentException (fail fast); retain
the original isEckey-based behavior only when scheme is null.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3cc89e0e-8854-4066-98d3-f60cdfbf3def
📒 Files selected for processing (21)
CLAUDE.mdREADME.mdbuild.gradlesrc/main/java/org/tron/common/crypto/pqc/FNDSA512.javasrc/main/java/org/tron/common/crypto/pqc/PQSchemeRegistry.javasrc/main/java/org/tron/common/crypto/pqc/PQSignature.javasrc/main/java/org/tron/common/utils/TransactionUtils.javasrc/main/java/org/tron/keystore/CredentialsFalcon.javasrc/main/java/org/tron/keystore/Wallet.javasrc/main/java/org/tron/keystore/WalletFile.javasrc/main/java/org/tron/keystore/WalletUtils.javasrc/main/java/org/tron/walletcli/Client.javasrc/main/java/org/tron/walletcli/WalletApiWrapper.javasrc/main/java/org/tron/walletcli/cli/commands/WalletCommands.javasrc/main/java/org/tron/walletserver/WalletApi.javasrc/main/protos/core/Tron.protosrc/test/java/org/tron/common/crypto/pqc/FNDSA512Test.javasrc/test/java/org/tron/common/crypto/pqc/PQSchemeRegistryTest.javasrc/test/java/org/tron/common/crypto/pqc/PQSignatureDefaultsTest.javasrc/test/java/org/tron/keystore/WalletPQAddressTest.javasrc/test/java/org/tron/keystore/WalletPQKeystoreTest.java
![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 21 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="src/main/java/org/tron/walletcli/WalletApiWrapper.java">
<violation number="1" location="src/main/java/org/tron/walletcli/WalletApiWrapper.java:261">
P1: Normalize the PQ scheme before persisting the wallet; storing `UNKNOWN_PQ_SCHEME` creates wallets that later fail PQ detection/decryption.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:1852">
P2: The README incorrectly says `ImportWalletPQ` can take the extended private key as a second argument, but the command only accepts optional scheme args and always prompts for key input from stdin.</violation>
</file>
<file name="src/main/protos/core/Tron.proto">
<violation number="1" location="src/main/protos/core/Tron.proto:547">
P2: `BlockHeader` documents signature-field exclusivity but does not enforce it; both signature fields can be set simultaneously.</violation>
<violation number="2" location="src/main/protos/core/Tron.proto:663">
P2: `HelloMessage` claims `signature` and `pq_auth_sig` are mutually exclusive, but the proto schema does not enforce that.</violation>
</file>
<file name="src/main/java/org/tron/walletcli/Client.java">
<violation number="1" location="src/main/java/org/tron/walletcli/Client.java:3209">
P2: Normalize the parsed PQ scheme before returning it; returning `UNKNOWN_PQ_SCHEME` unchanged can create keystores that are later treated as non-PQ wallets.</violation>
</file>
<file name="src/main/java/org/tron/walletcli/cli/commands/WalletCommands.java">
<violation number="1" location="src/main/java/org/tron/walletcli/cli/commands/WalletCommands.java:51">
P2: Validate `--scheme` against supported registry entries, not only `UNKNOWN_PQ_SCHEME`; otherwise unsupported enum values can pass parsing and lead to inconsistent command behavior.</violation>
</file>
<file name="src/main/java/org/tron/keystore/CredentialsFalcon.java">
<violation number="1" location="src/main/java/org/tron/keystore/CredentialsFalcon.java:65">
P1: `getPair()` throwing here breaks existing `Credentials` call sites for FN_DSA_512 wallets, causing runtime failures where code still assumes all `Credentials` provide a `SignInterface` pair.</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.
Actionable comments posted: 7
♻️ Duplicate comments (1)
src/main/java/org/tron/walletcli/WalletApiWrapper.java (1)
252-316:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winREPL PQ wallets register with a
nullfriendly name.Both
registerWalletPQ(252-271) andimportWalletPQ(278-316) skip thenameWallet(walletFile, false)call that legacyregisterWallet(line 213) andimportWallet(line 234) perform beforestore2Keystore. As a result, the PQ wallet entries show asnullinselectWalletFileByListuntilloginAllfalls back to the filesystem name at line 674. AddnameWallet(walletFile, false)right afterCreatePQWalletFile(...)and beforestore2Keystore(...)in both REPL methods to match the legacy flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/tron/walletcli/WalletApiWrapper.java` around lines 252 - 316, registerWalletPQ and importWalletPQ create PQ WalletFile via WalletApi.CreatePQWalletFile but never assign a friendly name, causing null entries; after the WalletFile is created (the WalletFile variable from CreatePQWalletFile) call nameWallet(walletFile, false) before invoking WalletApi.store2Keystore(...) in both registerWalletPQ and importWalletPQ so the keystore entry gets the same friendly-name initialization as legacy registerWallet/importWallet.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 1822-1901: The new PQ docs block in README.md violates
markdownlint rules MD046 and MD028; fix by normalizing all code fences to the
repository style (use triple-backtick fenced blocks with a language tag like
```console consistently for the examples around the "Register a PQ wallet" and
"Import a PQ wallet" sections and the example command blocks) and remove any
blank lines inside blockquotes (ensure lines beginning with '>' such as the CLI
description and the "Why no interactive prompt?" block have no empty lines
between '>' lines). Also ensure blockquote lines are consistently prefixed with
'> ' and that code blocks are separated from surrounding text as per the repo's
linting convention so MD046/MD028 no longer trigger.
In `@src/main/java/org/tron/keystore/Wallet.java`:
- Around line 352-369: createPQ currently only checks key lengths and then uses
the caller-supplied publicKey without verifying it matches the secret material;
derive the public key from the provided secret material (use extendedPrivateKey
when present, otherwise derive from seed) via the PQ scheme's public-key
derivation routine (e.g., the PQSchemeRegistry/PQ scheme implementation
associated with scheme) and compare the derived public key byte-for-byte to the
supplied publicKey; if they differ, throw a CipherException indicating a
mismatched public key. Place this validation in createPQ (after the
extendedPrivateKey/seed length checks and before any address
derivation/serialization) and reference publicKey, extendedPrivateKey, seed,
createPQ, PQSchemeRegistry and decryptPQ in the message for clarity.
- Around line 616-630: The PQ decryption path must only accept scrypt KDFs:
update deriveScryptKey so it only handles WalletFile.ScryptKdfParams
(generateDerivedScryptKey) and remove the branch that accepts
WalletFile.Aes128CtrKdfParams; if the kdfParams is not an instance of
ScryptKdfParams throw a CipherException (do not fall back to PBKDF2/AES-CTR).
Ensure decryptPQ(...) calls this updated deriveScryptKey so FN_DSA_512 and other
PQ keystores cannot be decrypted under PBKDF2 by mistake.
In `@src/main/java/org/tron/walletcli/Client.java`:
- Around line 3269-3286: The method importWalletPQ treats the single-argument
form as a scheme and exits early because parsePQScheme(parameters) returns null;
fix by handling the single-argument (hex|path) case before parsing the scheme:
if parameters.length == 1 (or extractPQHexArg(parameters) != null) treat
parameters[0] as hexOrPathArg and set a default or required scheme accordingly,
otherwise call parsePQScheme(parameters) to read an explicit scheme; update the
logic around parsePQScheme, extractPQHexArg, and the subsequent
expected/key-length checks so the documented one-argument usage reaches the
hex/path handling instead of returning early.
In `@src/main/java/org/tron/walletcli/WalletApiWrapper.java`:
- Around line 278-316: The importWalletPQ implementation must verify that when
both seed and extendedPrivateKey are provided they represent the same keypair:
instantiate two FNDSA512 objects (one via
FNDSA512.fromPrivateKeyWithPublicKey(extendedPrivateKey) and one via new
FNDSA512(seed)), compare their public keys (signer.getPublicKey()) and/or
privateWithPublic (getPrivateKeyWithPublicKey()) for equality, and if they
differ return/throw an error instead of silently persisting inconsistent data
(update the logic around the current branch that picks signer and the call to
WalletApi.CreatePQWalletFile); apply the same check to importWalletPQForCli as
well.
In `@src/main/resources/help_summary.txt`:
- Line 153: The summary for ImportWalletPQ is misleadingly specific to "extended
private key"; update the one-line description for the ImportWalletPQ command to
reflect both supported input formats (seed and extended private key / xprv),
e.g., change the text around the ImportWalletPQ entry so it mentions "seed or
extended private key" (or "seed/xprv") to match the actual accepted formats
implemented by the ImportWalletPQ command.
In `@src/test/java/org/tron/keystore/WalletPQKeystoreTest.java`:
- Around line 117-118: The test currently mutates the MAC with data-dependent
replaces; instead, deterministically flip a single known nibble so the MAC
always changes: read the MAC from walletFile.getCrypto().getMac(), compute a
deterministic mutation of one hex character (e.g. take the last hex char, map it
to a different valid hex digit or xor its nibble value with 0x1), then call
walletFile.getCrypto().setMac(...) with the modified string; apply the same
deterministic-nibble-flip approach to the other instance referenced around
WalletPQKeystoreTest (the second getMac()/setMac() pair).
---
Duplicate comments:
In `@src/main/java/org/tron/walletcli/WalletApiWrapper.java`:
- Around line 252-316: registerWalletPQ and importWalletPQ create PQ WalletFile
via WalletApi.CreatePQWalletFile but never assign a friendly name, causing null
entries; after the WalletFile is created (the WalletFile variable from
CreatePQWalletFile) call nameWallet(walletFile, false) before invoking
WalletApi.store2Keystore(...) in both registerWalletPQ and importWalletPQ so the
keystore entry gets the same friendly-name initialization as legacy
registerWallet/importWallet.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da6a818e-3679-45d3-94ad-c655a0b9af71
📒 Files selected for processing (10)
README.mdsrc/main/java/org/tron/keystore/Wallet.javasrc/main/java/org/tron/keystore/WalletFile.javasrc/main/java/org/tron/walletcli/Client.javasrc/main/java/org/tron/walletcli/WalletApiWrapper.javasrc/main/java/org/tron/walletcli/cli/commands/WalletCommands.javasrc/main/java/org/tron/walletserver/WalletApi.javasrc/main/resources/commands.txtsrc/main/resources/help_summary.txtsrc/test/java/org/tron/keystore/WalletPQKeystoreTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/tron/walletserver/WalletApi.java
- src/main/java/org/tron/walletcli/cli/commands/WalletCommands.java
Federico2014
changed the title
There was a problem hiding this comment.
3 issues found across 23 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="src/main/java/org/tron/walletcli/cli/commands/WalletCommands.java">
<violation number="1" location="src/main/java/org/tron/walletcli/cli/commands/WalletCommands.java:98">
P2: When `MASTER_PASSWORD` is missing, the handler returns before wiping decoded PQ key bytes. Clear the decoded buffer before this early return to avoid leaving sensitive key material in memory.</violation>
</file>
<file name="src/main/java/org/tron/walletcli/Client.java">
<violation number="1" location="src/main/java/org/tron/walletcli/Client.java:3216">
P2: `ImportWalletPQ <hex|path>` is currently broken: non-scheme single arguments are rejected as "Unsupported PQ scheme" before key-material extraction runs.</violation>
</file>
<file name="src/main/java/org/tron/keystore/WalletUtils.java">
<violation number="1" location="src/main/java/org/tron/keystore/WalletUtils.java:199">
P1: Returning `CredentialsFalcon` from `loadCredentials` breaks existing call sites that immediately use `credentials.getPair()`, causing runtime `UnsupportedOperationException` for FN_DSA_512 wallets.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Federico2014
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
4 issues found across 14 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="src/main/java/org/tron/walletserver/WalletApi.java">
<violation number="1" location="src/main/java/org/tron/walletserver/WalletApi.java:551">
P2: Parsing `walletFile.scheme` as a PQ enum and throwing on mismatch can break signing for non-PQ wallets that carry legacy scheme tags (e.g., `SM2`/`ECKEY`). Treat non-PQ/unknown values as non-PQ so EC/SM2 fallback still works.</violation>
</file>
<file name="src/main/java/org/tron/common/crypto/jce/TronCastleProvider.java">
<violation number="1" location="src/main/java/org/tron/common/crypto/jce/TronCastleProvider.java:42">
P2: Re-registering an existing BC provider (`removeProvider` + `addProvider`) unintentionally changes global provider precedence. This can alter algorithm/provider selection in unrelated crypto calls.</violation>
</file>
<file name="src/main/java/org/tron/common/utils/TransactionUtils.java">
<violation number="1" location="src/main/java/org/tron/common/utils/TransactionUtils.java:344">
P0: PQ transaction validation is only checking field presence, not signature validity/owner binding, so forged `pqAuthSig` entries can be accepted as valid.</violation>
</file>
<file name="src/main/java/org/tron/keystore/Wallet.java">
<violation number="1" location="src/main/java/org/tron/keystore/Wallet.java:370">
P2: Handle `IllegalArgumentException` from extended-key parsing and rethrow `CipherException` so `createPQ` fails through its declared error contract.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Federico2014
force-pushed
the
feature/pq-fn-dsa-512
branch
from
a81b8db to
a19f806
Compare
Federico2014
changed the title
Federico2014
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
5 issues found across 19 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Federico2014
force-pushed
the
feature/pq-fn-dsa-512
branch
from
dcf663f to
a54feda
Compare
Mirrors the protocol added by java-tron's ALLOW_FN_DSA_512 proposal so wallet-cli can generate, import, store, and transact with PQ keypairs. Highlights: - Proto sync: PQScheme, PQAuthSig, and pq_auth_sig fields on Transaction / BlockHeader / HelloMessage (additive, backwards compatible). - BouncyCastle 1.78.1 -> 1.84 for org.bouncycastle.pqc.crypto.falcon.*. - Crypto layer (org.tron.common.crypto.pqc): FNDSA512, PQSchemeRegistry, PQSignature. Address is 0x41 || Keccak-256(pubkey)[12..32], reusing Hash.sha3omit12. - Keystore: WalletFile.scheme discriminator (null/ECKEY for legacy, FN_DSA_512 for Falcon). Wallet.createStandardPQ / decryptPQ reuse the existing scrypt+AES-CTR plumbing to encrypt the 2176-byte extended private key (f||g||F||h). New CredentialsFalcon wraps the FNDSA512 signer; deliberately not part of the SignInterface union. - Signing path: TransactionUtils.signPQ routes signatures into Transaction.pq_auth_sig instead of Transaction.signature. WalletApi dispatches per-wallet via WalletFile.scheme; the process-wide isEckey selector is untouched for ECKey/SM2. - REPL commands: generatePQKey, registerWalletPQ, importWalletPQ. - Standard CLI commands: generate-pq-key, register-wallet-pq, import-wallet-pq (MASTER_PASSWORD env-var auth, JSON output). - Tests: FNDSA512Test, PQSchemeRegistryTest, PQSignatureDefaultsTest, WalletPQAddressTest, WalletPQKeystoreTest. - Docs: README.md gains a Post-quantum signatures section; CLAUDE.md documents per-wallet dispatch and the pqc package surface.
Federico2014
force-pushed
the
feature/pq-fn-dsa-512
branch
2 times, most recently
from
2b54cfa to
d6d60e8
Compare
Federico2014
force-pushed
the
feature/pq-fn-dsa-512
branch
from
d6d60e8 to
1d04b6a
Compare
…ts in deployContract
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
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 signature support to wallet-cli, mirroring the protocol changes introduced by the
ALLOW_FN_DSA_512andALLOW_ML_DSA_44proposals in java-tron. Users can generate, store, import, and transact with PQ keypairs in either scheme.Two PQ schemes are now supported:
f‖g‖F‖h), ≤752-byte signature.rho‖K‖tr‖s1‖s2‖t0), fixed 2420-byte signature.Key changes:
PQScheme(withFN_DSA_512,ML_DSA_44),PQAuthSig, andpq_auth_sigfields onTransaction,BlockHeader, andHelloMessage(additive, backward-compatible).FNDSA512,MLDSA44,PQSchemeRegistry, and thePQSignatureabstraction. Dispatch is per-wallet viaWalletFile.scheme, not process-wide. Address derivation uses0x41 || Keccak-256(pubkey)[12..32]for both schemes.WalletFile.schemeacts as a discriminator (FN_DSA_512,ML_DSA_44).Wallet.createPQuses a single scrypt KDF run to generateDK, whereDK[0..16]keys AES-CTR for both segments using independent IVs, andDK[16..32]generates independent Keccak-256 MACs.Wallet.decryptPQvalidates all present MACs before decryption and performs consistency checks.PQSchemeRegistryis the single dispatch point; per-scheme seed/persisted-key lengths come from the registry and thegetPersistedPrivateKey()default abstracts the on-disk encoding asymmetry between Falcon (must appendh) and ML-DSA (private encoding is self-contained).generatePQKey,registerWalletPQ, andimportWalletPQto the REPL, andgenerate-pq-key,register-wallet-pq, andimport-wallet-pqto the standard CLI. All three commands accept a--scheme/ positionalschemeargument (defaultFN_DSA_512).Why are these changes required?
To provide a reference user-facing tool for interacting with the upcoming post-quantum signature features in TRON. Supporting both Falcon-512 and ML-DSA-44 from day one mirrors the dual-scheme rollout on the node side and lets users compare the two schemes for their own use cases. The dual-ciphertext keystore design ensures the wallet can securely store the (longer) persisted private key alongside the original seed, allowing full key recovery while preventing keystream reuse.
This PR has been tested by:
FNDSA512Test,MLDSA44Test,PQSchemeRegistryTest(covers dispatch for both schemes),PQSignatureDefaultsTest,WalletPQAddressTest,WalletPQKeystoreTest,WalletPQMlDsa44KeystoreTest. Coverage includes dual-ciphertext roundtrips, ext-only and seed-only paths, missing material, IV independence, segment-specific MAC tamper rejection, and signature-length validation.Follow up
Extra details
org.bouncycastle.pqc.crypto.falcon.*andorg.bouncycastle.pqc.crypto.crystals.dilithium.*.oneof auth) in the proto, hardensignPQagainst unsupported scheme tags, verify imported seed/private key generate the same keypair, and explicitly check scheme values against the registry.Summary by CodeRabbit
New Features
Documentation
Chores