fuzz: seed qbit targets and verify replay and mutation - #164
Merged
Merged
Conversation
Inputs that start with "QBFX", a target tag and format version 1 select either a cached-fixture body (selector low nibble other than 0xF, 15/16) or the existing key generation and signing body (low nibble 0xF, 1/16). The fixture bodies verify and mutate real SLH-DSA signatures and signed P2MR spends that are built once per process on first use. An unmodified fixture must verify, and in pqc any changed signature, public key or message must be rejected. Tagged inputs with another version are ignored. Untagged inputs keep the previous layout and behavior.
Add deterministic seeds for asert_chain_transition, asert_edge_cases, asert_math, auxpow, p2mr_script and pqc. generate_seeds.py builds each seed from a documented case by encoding the values the harness reads from FuzzedDataProvider, and MANIFEST.json records the size, SHA-256 and meaning of every seed. overlay.py checks the tree against the manifest and size limits, then copies the seeds into a fuzz corpus directory without replacing existing inputs.
test_runner.py --require_qbit_corpus fails unless the six qbit targets are compiled, selected, have regular input files and are reported as replayed by the fuzz executable. It prints the input files present and replayed for each target. --mutate_min_time runs a libFuzzer mutation phase per target, reading the corpus and writing new inputs to a temporary directory, and fails on builds without libFuzzer. CI overlays the qbit seeds after qa-assets is cloned or reused, including on Windows, and replays with --require_qbit_corpus. When QBIT_FUZZ_MUTATE_MIN_TIME is set, ci/test/03_test_script.sh also runs the mutation phase for the six targets. The native fuzz setup exports the variable so a workflow value reaches the CI container. Add tests for the runner options and seed corpora, and a source coverage test for key generation, signing and P2MR signature verification.
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. |
9 tasks
The application test times all synchronous GUI teardown, including work that happens after the pending send worker has observed cancellation and joined. Slow teardown can therefore fail the five-second assertion even when the cancellation contract succeeds. Use the existing watchdog to enforce the cancellation deadline, anchored before shutdown starts. Keep total teardown time as diagnostic output, assert worker completion before teardown returns, and preserve the wallet lifetime and cancellation checks. Validation: reproduce the old failure with a post-cancellation delay, verify the fixed test accepts it, and verify missed cancellation still fails the watchdog.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43bb00cb5c
ℹ️ 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".
Replace only the exact destination filenames listed in the current seed manifest. Leave unrelated qbit-prefixed inputs intact, including names removed from earlier manifests and names owned by another target. Check destination types before writing seeds and retain exclusive file creation. Extend regression coverage for repeated overlays, updated and missing managed seeds, preserved inputs, and unsafe destinations. Document the filename ownership rule and manual cleanup of historical seed names.
A post-return read of create_finished can observe a worker that completed after sendEvent returned. It cannot establish the required lifetime order. Record worker completion and the synchronous dialog destruction callback with sequence numbers under the synthetic wallet mutex. Require completion to precede destruction inside teardown, independently of scheduling after sendEvent returns. Report each event's timing and remove obsolete elapsed state while preserving the five-second cancellation watchdog.
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
Fixes #154. Give all six qbit fuzz targets repository-owned seed inputs and make CI fail when a target or its replay evidence is missing.
Add 53 deterministic seeds (37,274 bytes), a size/hash/semantics manifest, generator, and validated overlay that preserves upstream inputs.
Add versioned fixture inputs to
pqcandp2mr_script. Cache real keys and signatures per process while retaining real verification, untouched-input assertions, legacy input semantics, and named seeds that exercise key generation and signing.Add
--require_qbit_corpusfor Linux and Windows replay. Add a separate, bounded libFuzzer mutation mode for the six targets, with temporary output storage, timeout enforcement, and initialization/progress counts. MissingINITEDevidence or a final count no greater than initialization fails the run.Forward
QBIT_FUZZ_MUTATE_MIN_TIMEthrough native fuzz CI and document the behavior. The companion Run scheduled validation against the maintained release line and report the tested commit #152 change enables 120 seconds per target in the nightly workflow; this PR should land first.Stabilize the shared Qt shutdown regression test: enforce the five-second send-worker cancellation deadline, preserve wallet lifetime checks, and report total GUI teardown time separately. Slow teardown after successful cancellation no longer causes a false failure.
Resolve review findings: replace only current manifest-owned corpus filenames and preserve other inputs; record worker completion before synchronous dialog destruction to verify Qt lifetime ordering without a post-return observation race.
Testing
Review follow-up: all 16 runner tests passed, including corpus preservation across repeated overlays and unsafe-destination checks. The new cases fail against the old cleanup. A controlled late-completion record passes the old Qt assertion and fails the revised one; clean Qt CTest, slow teardown, and cancellation-fault checks behaved as expected. Full Docker lint passed on the updated signed head.
CI follow-up: the shared Qt test passed a Linux Debug build, all 12 QtTest suites through CTest, and three clean repeat runs. A 5.5-second delay after worker cancellation reproduces the old failure and passes with the fix; deliberately disabled cancellation still fails the five-second watchdog, with and without the extra delay. Full repository Docker lint passed on the updated signed commit. Hosted CentOS validation is rerunning; local verification used GCC 13.3 / Qt 6.4.2.
python3 test/fuzz/test_qbit_fuzz_runner.py: 16 tests passed again after merging current1.x.x. New regression cases fail against the old runner, including the captured 18-initialization/18-total false success; the fix accepts the minimal positive-progress boundary.Linux LLVM 21 sanitizer build: 18 runner/integration tests passed, including all 53 seeds and 120-second mutation for all six targets. Fresh post-initialization executions: PQC 21, P2MR 24; all other targets also positive. macOS native integration: 2 tests passed, including replay and rejection of mutation mode on a non-libFuzzer executable.
The actual updated validator rejected both recorded initialization-only failures and accepted all 12 recorded 120-second paired runs. Three alternating baseline/candidate pairs used identical starting inputs and matching mutation RNG seeds; this does not establish a portable throughput guarantee.
Coverage contract: all 17 named-seed cases passed. Seven deliberately broken harness variants were rejected, followed by exact source restoration, a clean build, and a passing rerun. The 10 shared legacy cases matched baseline anchor counts.
Full repository lint passed in Docker on the publication head. Prior real-binary evidence covers the unchanged fuzz implementation; the subsequent base merge only adds unrelated wallet/Qt changes.
Built locally.
Ran focused unit or functional tests for the changed area.
Ran lint or formatting checks relevant to this change.
Not run. Reason: hosted Windows MSVC/platform jobs and mutation on the self-hosted nightly runner await publication and workflow delivery.
Target Branch
mainor a maintainer-requested release branch such as0.1.x. Target:1.x.x.Risk / Review Notes
Notes: changes affect test harnesses, assets, runner, and CI integration. Production consensus, signer, PQC backend, and wallet code are unchanged by this PR. The time budget includes corpus initialization and may overrun while an input finishes; the progress guard prevents initialization-only success. New runner/coverage contract scripts are local checks and are not themselves wired into a CI job. The coverage script needs an instrumented build.
Docs / Process Impact
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.