fix(pqc): harden PQ auth sig checks and wallet UX#938
Open
Federico2014 wants to merge 4 commits into
Open
Conversation
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
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?
Hardens PQ auth signature handling and tightens wallet/CLI UX, and lays the groundwork for adapting
getCanDelegatedMaxSizeto the newpq_schemefield introduced in nile-testnet#53.Why are these changes required?
Transaction.pq_auth_sig) do not bumpgetSignatureCount(), so existing guards that bail ongetSignatureCount() != 0silently invalidate a signed PQ transaction by rewriting raw_data.Chain.Transactionlacks thepq_auth_sigfield (field 6), so PQ sigs are lost when rebuilding transactions (e.g. to inject fee limits) unless unknown fields are explicitly preserved.Wallet.validPasswordbecause the ECDSA ciphertext/mac path does not apply.--extended-private-key-hexis exposed in shell history andps;--key-file/ stdin is safer.getCanDelegatedMaxSizewas hardcoded to an ECDSA-sized bandwidth estimate; for PQ accounts, the delegate transaction carries a much largerPQAuthSig(e.g. ML-DSA-44 ≈ 3.7 KB vs ~65 bytes ECDSA), so the returned max is too large — after delegating that amount, the account has no bandwidth left to send the delegate tx itself. This PR does not fix that yet; it adds the proto field and the pass-through API surface so the fix can land without a second wire-format change once the Trident SDK accepts the parameter. Until then the scheme is accepted but ignored.Key Changes
TransactionUtilssetExpirationTime/setPermissionIdbail when a PQ auth sig is attached;PQ_AUTH_SIG_FIELD_NUMBERexposed and shared withWalletApiWalletApigetCanDelegatedMaxSizeoverload withPQSchemeparameter (pass-through only)WalletvalidatePasswordPQinvalidPassword(no key material decrypted, no signer constructed)PQSchemeRegistryDecodeUtil.addressPreFixByte+ length guard incomputeAddressMLDSA44ApiClientPQSchemeis currently ignored (no-op) pending Trident SDK supportapi.protoCanDelegatedMaxSizeRequestMessagegainsPQScheme pq_scheme = 3Client/WalletCommands/QueryCommands--key-file/ stdin import;--schemeonget-can-delegated-max-size(documented as a no-op for now); REPL 3rd param for scheme with a no-op noteREADME--key-fileimport flowThis PR has been tested by:
./gradlew compileJava)./gradlew test)Follow up
PQSchemein itsgetCanDelegatedMaxSizeAPI method, then wirepqSchemethroughApiClient, pass the active wallet's scheme fromWalletApi.delegateResource, remove the TODO markers inApiClient.java/WalletApi.java, and drop the "no-op" notes from the CLI/REPL help.