Conversation
Add a PQC domain to SignatureCache and a VerifyPQCSignature override in CachingTransactionSignatureChecker, following the Schnorr pattern. Entries are SHA256(nonce || 'P' || 31 zero bytes || sighash || pubkey || signature) in the existing shared cuckoo cache. Only a verified success is inserted, and only when the checker stores; non-store lookups keep the lazy erase semantics. Interpreter checks, validation-weight charges, script flags, the full-script cache and data signatures are unchanged. Add a null-by-default, test-only PQCSignatureCacheObserver so tests can prove that a fresh mempool admission verifies each PQC signature once in the policy pass and reuses it in the consensus pass. Tests: pqc_sigcache_tests covers cache-key components and domain separation, failures never populating the cache (including cold/warm equivalence over the witness-vector corpus), unchanged P2MR errors and weight budgets on warm caches, store/consume and reclamation, and the shared budget. txvalidationcache_tests/pqc_policy_consensus_reuse checks the observer event log for real ProcessTransaction admissions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Time the mempool admission script-check pattern (standard flags without full-script storing, then consensus flags with storing) for distinct, pre-signed P2MR CHECKSIGPQC spends against one long-lived validation cache with default sizes. Signing and pool construction happen before timing, and epochs and iterations are fixed so no transaction is checked twice. A low-priority variant attaches a counting observer to bound the test-only observer cost. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79c2bd51b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep the verifier and its scoped counter header-defined so consensus builds do not depend on bitcoin_common. Exercise middle and final bytes of all cache-key inputs to detect prefix truncation.
Pre-allocate capacity for the per-tuple cache entries so the reclamation test does not grow the vector inside its fill loop.
Pick up the Qt wallet shutdown test rework so CI no longer evaluates the removed wall-clock bound.
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
Cache successful P2MR transaction-signature (SLH-DSA) checks in the existing signature cache so mempool admission's policy pass and consensus pass verify each PQC signature once instead of twice. Closes #153.
CachingTransactionSignatureCheckernow overridesVerifyPQCSignaturewith the same shape as the ECDSA and Schnorr overrides: look up a salted entry ('P'domain, keyed by sighash, public key and raw signature bytes), return true on a hit, otherwise call the primitive verifier and insert only a verified success when the checker is in store mode. The cache is the same bounded cuckoo cache with the same budget, lazy-discard consumption and minimum allocation; no separate store is added.SignatureCache(null by default, never installed by production code; one relaxed atomic load beside a SHA256 and an SLH-DSA verify) lets tests observe each cache lookup and the verified result after a miss. A separate scoped counter atCPQCPubKey::Verifyrecords actual backend invocations, so extra verification on cache hits cannot hide behind unchanged cache events. The backend and validity checks remain unchanged. The newtxvalidationcache_tests/pqc_policy_consensus_reuseadmits fresh P2MR spends through the realProcessTransactionand asserts the ordered event log: one miss and one verified insert in the policy pass, then a hit in the consensus pass (two verifications instead of four for a two-input spend). It also documents the non-evidence cases: a repeatedtest_acceptshows a lone policy hit because the consensus pass is a full-script cache hit, and a duplicate submission is rejected before any script check.pqc_sigcache_testscover key composition and domain separation, failures never populating the cache, cold/warm/uncached equivalence over the P2MR witness-vector corpus including error codes, unchanged weight budgets at the legacy and v2 boundaries, store/consume semantics with seeded reclamation, and the shared bounded budget.checkinputs_p2mr_pqc.cpptimes the two-pass admission pattern per fresh transaction (pre-signed pool, fixed epochs so no transaction is ever rechecked).Testing
79c2bd51b2, 186 cases pass acrosspqc_tests,txvalidationcache_tests,pqc_sigcache_tests,cuckoocache_tests,script_p2mr_tests,script_tests, andtransaction_tests.79c2bd51b2.337bfa668f; a redundant-verification-on-hit mutation is also caught by the final primitive counter. Each restored named test passes. The full unit suite passed before the counter follow-up.Measurements
These measurements were collected at
337bfa668f, before the primitive-counter follow-up; its overhead has not been measured.Three interleaved baseline/candidate pairs on an Apple M5 Max (18 cores, macOS 26.4, Apple clang 17, Release
-O2, default 16 MiB signature cache). The baseline compiles out the override from the candidate tree. The benchmark times two script-check passes on distinct pre-signed transactions; it does not time full mempool admission. The observer was detached.All five-signature pairs were noisy: other builds were active and load rose during baseline arms. Observer overhead was not isolated above noise. No production or block-connection speedup is claimed.
Target Branch
mainor a maintainer-requested release branch such as0.1.x. (Targets1.x.xas requested by the maintainer.)Risk / Review Notes
Notes: consensus-adjacent script code. The change reuses a verified primitive result only when sighash, public key and signature bytes are identical, within a distinct salted domain; validity outcomes and script errors are shown equal for uncached, cold and warm checks across the witness-vector corpus, and weight budgets are equal at both weight-rule boundaries. Block connection consumes entries lazily exactly as for ECDSA and Schnorr.
Docs / Process Impact
Choose exactly one:
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Review follow-up
bitcoin_common.