Repository navigation
Land verified corpus and offline-import stack - #1699
Conversation
Type the existing stream doubles and JSON fixtures precisely, and use scoped loader patching. Preserve packed and indexed corpus behavior and mutation coverage.
Accept verified indexed Core files beside packed archives while preserving exact logical bytes, product identities, custody checks, and result schemas. Cover dual-format fixtures and source mutation refusal without claiming a live campaign.
Import Core-framed corpora through the production settlement owner with exact digest checks, durable bodies and undo, clean checkpoint publication, and idempotent reopen. Wire indexed custody into comparator arms without changing product identities or claiming a live campaign.
Normalize the authoritative C150 manifest beside the framed comparator schema without creating a converted copy. Match the configured network identity and resume nonzero-height ranges by manifest-relative index. Cover both readers with schema, network, range, and archive-validation tests.
Exercise direct frame writing, digest-consistent archive size mismatch, and trailing bytes after a complete entry list.
Align test doubles, JSON helpers, dynamic loader patching, and context managers while preserving all 84 corpus tests. Apply canonical Python formatting.
Verify every archive entry against its imported header, force full offline validation, and refuse stale persisted indexes. Remove an unused test helper.
Implement atomic index publication with exact archive binding and source-mutation refusal. Import the producer manifest identity instead of duplicating it.
Require index entries to match real Core frames, execute indexed parity without a second packed copy, and retain read-only source mutation checks.
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR introduces a corpus and offline-import subsystem stack. The code is well-structured with thorough boundary checks, archive binding, and per-record manifest verification. The overall security and correctness posture is strong.
Blocking Issue
crates/node/src/offline_import.rs — Unbounded memory allocation in read_manifest: The std::fs::read(path) call at line 350–351 reads the entire manifest file into memory before the 512 MiB size check at line 352. This means a 512 MiB (or larger) file on disk results in a full heap allocation before the guard triggers. Replacing with a bounded take(limit + 1) stream prevents the allocation entirely. I've left a comment with a code suggestion for this.
Non-Blocking Observations (noted inline but not merge-blocking)
- The
RawManifest#[serde(untagged)]deserialization silently ignores unknown fields; the design deliberately accepts this and the actual security binding is the archive digest chain. Not a defect. - The
stream_importheader-emission loop inrestore.rsis bounded bytip.heightvalidation incheck_identity, preventing iteration overu32::MAXheights. Not a defect. spool_scratchincorpus.pyuses_unique_scratch(archive_path)as the base — same parent asarchive_scratch. UUID collision is negligibly unlikely in practice. Not a defect.
The inline comments I added that concluded "not a defect" are informational only and should not block merge. Only the read_manifest allocation issue requires a fix.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Port the bounded index page and restore paths and buffer UTXO seed reads. Give the corpus spool a manifest-independent scratch namespace so concurrent writers cannot share a scratch path. The remote branch already bounds manifest reads.
Merges origin/main (Tor/I2P overlay connectivity, split public listener) into the corpus-offline branch, preserving the branch's offline corpus index/spool and BlockPayload plumbing while adopting main's listener addressing and overlay behavior. - crates/node/src/config.rs: keep both validators; branch validate_index_bundle plus main p2p.overlay.validate. - crates/p2p/src/listener.rs send_block: main's PeerRemote addressing (impl Into<PeerRemote>) with branch's BlockPayload body; main's block+serialized form cannot apply because merged wire/inbound/sync all speak BlockPayload (branch-owned refactor, auto-merged). - crates/p2p/src/listener.rs handle_compact_outcome: keep branch Option follow-up return consumed by the merged enqueue_owned_body_fetch single-enqueue site; adopt main's PeerRemote parameter. Main's bool direct-send body rejected: it calls a 4-argument send_block that no longer exists in the merged tree. - aggregate_budget_tests: PeerEndpoint address (main) with is_none assertion matching the kept Option return. - crates/rpc/tests/core_compat.rs: adopt main's ipv4/ipv6/onion/i2p network rows (superset of the branch-deleted len==3 check); keep branch's BlockPayload submitblock cases and truncation test. - No ListenerRole or rpc-public-bind restored; neither exists on either side of this merge.
Main compact-sync tests called record_owned_body_fetch, which the branch replaced with enqueue_owned_body_fetch. The NotFoundFixture fields also kept SocketAddr after current_source and test_addr moved to PeerRemote. - Add the missing SocketAddr import. - Store owner and alternate as PeerRemote. - Mark the owned fetch through enqueue_owned_body_fetch.
|
@codex review |
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fae5a35a53
ℹ️ 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".
…scation iter_frames treats an 8-byte all-zero header followed only by zero bytes as Core blk preallocation padding (clean EOF). Non-zero bytes after padding fail with a trim diagnostic naming the offset. A magic mismatch at offset 0 fails closed with an explicit diagnostic: raw blk files are never XOR-obfuscated, so no de-obfuscation path is attempted. PR #1699 findings 1-2.
Open every blk source with O_NOFOLLOW so a symlink swapped in after validation fails closed instead of being followed or hashed; pre-hash now goes through _source_path instead of bypassing it. build_index re-hashes all sources after the scan/re-read and refuses to publish when a file changed mid-build, binding the three opens to one generation. PR #1699 findings 3-4.
Add build_archive_from_index plus the build-archive command: the packed archive streams from fully verified index payloads and the published manifest digest/size must equal the index logical digest/size, otherwise both outputs are unpublished and the build fails. _run_writer now returns its summary for the assertion. PR #1699 finding 5.
OfflineScanHardening pins zero-padded-tail acceptance, the trim diagnostic, the offset-0 obfuscation fail-closed message, symlink refusal in hash/build/verify, mid-scan mutation rejection without publishing, and archive-from-index byte parity with manifest digest binding (function-level and build-archive CLI). 103 tests green. PR #1699 findings 1-5.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
# Conflicts: # crates/index/src/portable/restore.rs # crates/utxo/src/snapshot.rs # docs/benchmarks/offline-full-validation.md # tools/campaign-corpus/corpus.py # tools/campaign-corpus/test_corpus.py
Foreground import runs no maintenance worker, so a RetentionLimit refusal aborted the whole import even though the refused block committed nothing. connect_batch now matches NotCommitted(JournalBackpressure) carrying RetentionLimit, publishes one synchronous clean checkpoint (compacting every superseded journal segment), and retries the uncommitted block exactly once; any other failure, a skipped publication, or a second refusal propagates. Neither the refused attempt nor the checkpoint moves the applied tip, so reruns stay idempotent by tip comparison. Adds a regression test that proves pressure refuses a child and that the relief path commits it at height 1.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e4a10d46b
ℹ️ 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".
TipSnapshot/DurableHead record height, hash, work, and tx counts but not the ValidationMode that committed each block, so a prefix connected under AssumeValid/Fast is indistinguishable from a Full one. Skipping it would certify unexecuted scripts as fully validated. Root fix: when the import completes full validation and certifies the tip, it atomically records a full-validation provenance sidecar in the data dir bound to the applied tip hash and height. A rerun skips its prefix only while that record still binds the current tip; an absent, unreadable, or mismatched record refuses, because a normal live run leaves no such record. The record is written after checkpoint publication and before the certified state, so a state-write failure stays resumable. Adds end-to-end regression tests: fresh import validates and records provenance, a certified directory resumes idempotently, and a tip without provenance is refused.
Draft subsystem stack split from PR #1687.
Base: f024fc3
Tip: 2785fd8
Full repository gate: native-gate-20261027T223049Z-1567775 (scripts/ci-pr.sh all, exit 0).
Scope: corpus and offline import.
This is a verified stack tip. Do not rebase or reparent it
without owner approval.
No better-than-Core claim. No mainnet qualification.
See also: #1687
The PR appears safe to merge, with the remaining restore performance issue non-blocking.Findings
Summary
This stack adds verified corpus tooling, foreground offline import, portable transaction-index bundles, and snapshot-backed index seeding.
Since the previous review:
No new actionable issues were found.
Reviews (3) · Last reviewed commit: "Stream corpus index pages and separate s..." · Reviewed by Greptile