refactor(pq): remove redundant PQ guards and add pending pool pre-checks

[bladehan1]

What does this PR do?

1. Fix PQ signature bypass in TransactionCapsule.validatePubSignature: Remove && isAnyPqSchemeAllowed() from line 499. The old code skipped PQ validation entirely when no PQ scheme was activated, so any transaction carrying arbitrary forged pq_auth_sig entries would silently pass using ECDSA weight alone. After this change, every transaction with pq_auth_sig entries is always validated — when the scheme is disabled, the inner isPqSchemeAllowed(scheme) check correctly rejects it. This is a behavior change (accept → reject), not a refactor.
2. Remove equivalent guards in BlockCapsule.validateSignature and TransactionUtil.checkPermission: In these two paths the outer isAnyPqSchemeAllowed() guard was truly redundant — both old and new code reject when PQ is disabled (only the exception message changes from generic to scheme-specific).
3. Stream-ify the Key lookup in validatePQSignatureGetWeight: replace the manual for-loop with stream().filter().findFirst().orElseThrow(...), preserving identical semantics.
4. Add logger.warn in Manager.pushTransaction for the PQ pending pool full branch (txId, current count, limit).
5. Add Manager.isPqPendingFull and wire it as an early pre-check in Wallet.broadcastTransaction (after the existing isTooManyPending() check), returning SERVER_BUSY with "PQ pending pool is full." before pushTransaction is called. This fixes the silent SUCCESS previously returned when the PQ pending pool was full.

Why are these changes required?

• Security (TransactionCapsule:499): The old outer guard isAnyPqSchemeAllowed() short-circuited before validating PQ signatures when PQ was globally disabled. An attacker could append arbitrary pq_auth_sig entries to any transaction that already carried a valid ECDSA signature; those entries were silently ignored, and the transaction passed weight validation with ECDSA alone. Removing the outer guard ensures every pq_auth_sig entry is always validated, regardless of activation state.
• Cleanup (BlockCapsule, TransactionUtil): In both locations, the outer guard threw an exception identical in effect to what the inner per-scheme check would throw anyway. Removing them simplifies the rejection path and makes error messages scheme-specific.
• Observability / correctness (pending pool): A PQ transaction broadcast to a full PQ pending pool previously received a silent SUCCESS; the fix adds an explicit early SERVER_BUSY response and an actionable warn log.

This PR has been tested by:

• Unit Tests
  • TransactionCapsuleTest.pqKeyNotInPermissionThrows — stream orElseThrow path: signer not in permission → PermissionException
  • TransactionCapsuleTest.pqKeyFoundReturnsWeight — stream happy path: key found → correct weight returned
  • TransactionCapsuleTest.pqWrongKeyLengthThrows — pk/sig length mismatch → SignatureFormatException
  • TransactionCapsuleTest.pqWeightOverflowThrows — two Long.MAX_VALUE weights → PermissionException("weight overflow")
  • ManagerMockTest.testPushTransactionPqPendingFull — PQ pool full → pushTransaction returns false + warn log
  • WalletMockTest.testBroadcastTransactionPqPendingFull — PQ pool full → SERVER_BUSY with "PQ" in message, pushTransaction never called
  • BlockCapsulePQTest, TransactionCapsuleTest, TransactionUtilTest — existing tests pass

Follow up

• Exception message wording change for the equivalent guard removals (BlockCapsule, TransactionUtil): the old generic string "pq_auth_sig not allowed: no post-quantum scheme is activated" is replaced by scheme-specific text. Downstream monitoring rules matching the old string will need updating.
• isVerified cache interaction with proposal "enable-then-disable" is a known scope-out; tracked separately.

Extra details

• No .proto changes, no new response_code values, no config changes.
• Each changed file can be independently reverted without affecting the others.

[coderabbitai]

📝 Walkthrough

Walkthrough

PQ signature validation now runs without a global activation pre-check in transaction and block code. Wallet and Manager now reject shielded and PQ transactions when pending pools are full, with tests covering the new SERVER_BUSY responses.

Changes

PQ signature validation

Layer / File(s)	Summary
PQ verification flow actuator/src/main/java/org/tron/core/utils/TransactionUtil.java, chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java, chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java	validateSignature and getTransactionSignWeight now enter PQ handling without the global activation pre-check, and PQ permission-key matching uses filter(...).findFirst() with the same missing-key exception.

Pending queue capacity

Layer / File(s)	Summary
Manager pending-full checks framework/src/main/java/org/tron/core/db/Manager.java, framework/src/test/java/org/tron/core/db/ManagerMockTest.java	Manager adds pending-full predicates for shielded and PQ transactions, logs rejected transactions with current/max counts, and tests cover pushTransaction(...) returning false when counters reach Integer.MAX_VALUE.
Wallet pending-full rejection framework/src/main/java/org/tron/core/Wallet.java, framework/src/test/java/org/tron/core/WalletMockTest.java	broadcastTransaction returns SERVER_BUSY for full shielded and PQ pending pools, and tests assert the error messages and that submission stops before pushTransaction(...).

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Wallet
  participant Manager
  Client->>Wallet: broadcastTransaction(transaction)
  alt shielded pending pool is full
    Wallet->>Manager: isShieldedPendingFull(transaction)
    Manager-->>Wallet: true
    Wallet-->>Client: response_code.SERVER_BUSY "Shielded pending pool is full"
  else PQ pending pool is full
    Wallet->>Manager: isPqPendingFull(transaction)
    Manager-->>Wallet: true
    Wallet-->>Client: response_code.SERVER_BUSY "PQ pending pool is full"
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

🐰 I hopped through signatures, neat and bright,
Then peered at pools of shielded light.
If burrows filled, I cried “Busy!” fast,
Yet PQ carrots still held fast.
Hoppity hop, the checks are done.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main refactor: removing redundant PQ guards and adding pending-pool pre-checks.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/pq_opt

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[bladehan1]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java (1)

499-503: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Remove the remaining global PQ activation pre-check.

This relaxed condition is still shadowed by validatePubSignature Lines 669-673, so normal trx.validateSignature(...) callers can still fail on isAnyPqSchemeAllowed() before reaching the per-scheme checks here.

Proposed fix

-      if (this.transaction.getPqAuthSigCount() > 0 &&
-          !dynamicPropertiesStore.isAnyPqSchemeAllowed()) {
-        throw new ValidateSignatureException(
-            "pq_auth_sig not allowed: no post-quantum scheme is activated");
-      }
       int signatureCount = getTotalSignatureCount();

🤖 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 `@chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java` around
lines 499 - 503, The remaining global PQ activation pre-check in
TransactionCapsule still blocks normal trx.validateSignature(...) flows before
the per-scheme logic runs. Remove the isAnyPqSchemeAllowed() gating from the PQ
path in TransactionCapsule.validateSignature/validatePubSignature and rely on
the specific PQ scheme checks inside validatePQSignatureGetWeight and the
existing per-signature validation branches so callers can reach the intended
scheme-specific handling.

🧹 Nitpick comments (1)

framework/src/main/java/org/tron/core/db/Manager.java (1)

931-943: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider reusing the new predicates inside pushTransaction.

The inline shielded/PQ pending-full conditions here duplicate isShieldedPendingFull(trx.getInstance()) / isPqPendingFull(trx.getInstance()). Reusing them keeps the “full” definition in one place (the warn logging can stay inline). Optional given the inline logging needs the counter values.

🤖 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 `@framework/src/main/java/org/tron/core/db/Manager.java` around lines 931 -
943, The pending-pool full checks inside pushTransaction duplicate the logic
already centralized in isShieldedPendingFull and isPqPendingFull. Update the
guard conditions in Manager.pushTransaction to call those predicates instead of
rechecking the counters inline, while keeping the existing warn logs inline so
the counter values remain available.

🤖 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 `@framework/src/main/java/org/tron/core/db/Manager.java`:
- Around line 2184-2192: `isShieldedTransaction` should guard against empty
contract lists before reading the first contract, since `isShieldedPendingFull`
calls it directly and can trigger an `IndexOutOfBoundsException` on transactions
with no contracts. Update `Manager.isShieldedTransaction` to check
`transaction.getRawData().getContractList().isEmpty()` and return false before
accessing `getContract(0)`, keeping the existing shielded transaction logic
unchanged for non-empty transactions.

In `@framework/src/main/java/org/tron/core/Wallet.java`:
- Around line 581-591: The shielded/PQ pending-full checks in
Wallet.broadcastTransaction are running before the contract-count validation,
which can trigger an IndexOutOfBoundsException via
dbManager.isShieldedPendingFull or dbManager.isPqPendingFull on empty-contract
transactions. Move both checks to after the contract-size guard in
broadcastTransaction, but still before pushTransaction, so the transaction is
known to have at least one contract and the existing CONTRACT_VALIDATE_ERROR
path is preserved.

---

Outside diff comments:
In `@chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java`:
- Around line 499-503: The remaining global PQ activation pre-check in
TransactionCapsule still blocks normal trx.validateSignature(...) flows before
the per-scheme logic runs. Remove the isAnyPqSchemeAllowed() gating from the PQ
path in TransactionCapsule.validateSignature/validatePubSignature and rely on
the specific PQ scheme checks inside validatePQSignatureGetWeight and the
existing per-signature validation branches so callers can reach the intended
scheme-specific handling.

---

Nitpick comments:
In `@framework/src/main/java/org/tron/core/db/Manager.java`:
- Around line 931-943: The pending-pool full checks inside pushTransaction
duplicate the logic already centralized in isShieldedPendingFull and
isPqPendingFull. Update the guard conditions in Manager.pushTransaction to call
those predicates instead of rechecking the counters inline, while keeping the
existing warn logs inline so the counter values remain available.

🪄 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: 387b5842-4db1-4713-85b8-b53f0a31a8fe

📥 Commits

Reviewing files that changed from the base of the PR and between be7280e and 7432b4a.

📒 Files selected for processing (7)

• actuator/src/main/java/org/tron/core/utils/TransactionUtil.java
• chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java
• chainbase/src/main/java/org/tron/core/capsule/TransactionCapsule.java
• framework/src/main/java/org/tron/core/Wallet.java
• framework/src/main/java/org/tron/core/db/Manager.java
• framework/src/test/java/org/tron/core/WalletMockTest.java
• framework/src/test/java/org/tron/core/db/ManagerMockTest.java

💤 Files with no reviewable changes (2)

• chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java
• actuator/src/main/java/org/tron/core/utils/TransactionUtil.java

[cubic-dev-ai]

1 issue found across 7 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>