security: PQVM-01 CREATE nonce, C-2 batch gas pre-check, PQVM-02/03 intrinsic gas#2
Merged
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements several security hardenings across PQVM transaction execution, precompiles, and the interpreter, with accompanying regression tests. It primarily tightens CREATE collision rules, adds batch signature size/gas pre-checks, and enforces intrinsic gas + bounded signature verification within the transaction gas budget.
Changes:
- Add intrinsic gas calculation (incl. calldata costs), escrow
gas_limit * max_fee, refund unused gas fees, and cap signature verification gas to the post-intrinsic budget. - Add
MAX_BATCH_SIGNATURESand upfront batch-count parsing + gas charging forML_DSA_65_BATCH_VERIFY, rejecting oversized batches. - Reject CREATE collisions when the derived target has existing code or storage, and adjust nonce increment timing; add
has_storageto the DB trait/state plus regression coverage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/pqvm/tests/conformance_vectors.rs | Updates conformance checks for new gas constants and DB trait (has_storage) in test scaffolding. |
| crates/pqvm/src/lib.rs | Implements intrinsic gas + fee escrow/refund and limits signature verification gas; adds related tests. |
| crates/pqvm-state/src/lib.rs | Extends PqvmDatabase with has_storage and adds a PqvmState implementation. |
| crates/pqvm-precompiles/src/lib.rs | Adds batch-size limit + pre-charged gas behavior for batch signature verification; adds tests. |
| crates/pqvm-interpreter/src/lib.rs | Adds CREATE collision detection (code/storage) and defers creator nonce increment until successful deployment; adds tests. |
| crates/pqvm-gas/src/lib.rs | Introduces calldata byte gas constants used by intrinsic gas calculation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+171
to
+176
| let max_fee_charge = fee_for_gas(tx.max_fee, tx.gas_limit)?; | ||
| deduct_balance(state, sender, max_fee_charge)?; | ||
| state.increment_nonce(sender)?; | ||
|
|
||
| fn verify_transaction_signature(tx: &PQTx) -> Result<(), TxExecutionError> { | ||
| let public_key = tx | ||
| .public_key | ||
| .as_ref() | ||
| .ok_or(TxExecutionError::MissingPublicKey)?; | ||
| let precompile = match AlgoId::try_from(tx.sig_type) | ||
| .map_err(|_| TxExecutionError::UnknownSignatureAlgorithm(tx.sig_type))? | ||
| { | ||
| AlgoId::MlDsa65 => ML_DSA_65_VERIFY, | ||
| AlgoId::SlhDsaSha2256f => SLH_DSA_SHA2_256F_VERIFY, | ||
| }; | ||
| let mut input = Vec::with_capacity(public_key.len() + tx.signature.len() + 32); | ||
| input.extend_from_slice(public_key); | ||
| input.extend_from_slice(&tx.signature); | ||
| input.extend_from_slice(tx.signing_payload().as_slice()); | ||
| let output = precompiles::BasicPqPrecompiles | ||
| .execute(precompile, &input, u64::MAX) | ||
| .map_err(|err| TxExecutionError::SignaturePrecompile(err.to_string()))? | ||
| .ok_or_else(|| TxExecutionError::SignaturePrecompile("missing verifier".into()))?; | ||
| if output.output.first().copied() != Some(1) { | ||
| return Err(TxExecutionError::InvalidSignature); | ||
| if let Some(to) = tx.to { | ||
| state.transfer(sender, to, tx.value)?; |
| let account = state | ||
| .account_mut(address) | ||
| .ok_or(StateError::MissingAccount(address))?; | ||
| account.balance += amount; |
Comment on lines
+178
to
+179
| .keys() | ||
| .any(|(storage_address, _)| *storage_address == address) |
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.
Summary
crates/pqvm-interpreter/src/lib.rs:969-1019by rejecting CREATE collisions when the derived target already has code or storage, and by incrementing the creator nonce only after successful deployment state writes.crates/pqvm-precompiles/src/lib.rs:16,101-115,206-250by addingMAX_BATCH_SIGNATURES, parsing the batch count up front, chargingML_DSA_65_BATCH_VERIFYgas before verification work, and rejecting oversized batches.crates/pqvm/src/lib.rs:134-315by computing intrinsic gas (including calldata cost), escrowinggas_limit * max_feebefore execution, refunding unused gas, and limiting signature verification to the post-intrinsic gas budget instead ofu64::MAX.crates/pqvm-interpreter/src/lib.rs:1776-1874,crates/pqvm-precompiles/src/lib.rs:314-372, andcrates/pqvm/src/lib.rs:532-596.Validation
cargo fmt --all -- --checkcargo clippy --workspace -- -D warningscargo test --workspaceImpact-Deferred
shell-chain: Not run in this PQVM security-fix session; downstreamcargo test --workspace/make e2eremain to be executed against the updated executor.shell-sdk: Not run in this PQVM security-fix session; downstream SDK/API validation remains pending.shell-dex: Not run in this PQVM security-fix session; downstream frontend smoke/build checks remain pending.shell-explorer: Not run in this PQVM security-fix session; downstream decode/render smoke checks remain pending.shella-chrome-wallet: Not run in this PQVM security-fix session; downstream sign/send and keystore checks remain pending.shell-site: Not run in this PQVM security-fix session; downstream docs/build/link checks remain pending.shell-chain-white-paper: Not updated here; if these gas-accounting semantics are intended as spec changes, the illustrative text should be reviewed separately.