Repository navigation
Land verified tools and docs stack - #1702
Merged
Merged
Conversation
The contract cited a false P2P-05 commit authority, dead transitions_* proof paths, and closed trackers as owners, and it lacked the required ownership map. Readers followed stale evidence for SYNC-01 through SYNC-07. Point every clause at its live owner and smallest real proof, state the unproven seed_live_from_utxo hold and the missing process-level settlement scenario explicitly, and add the seven-row ownership table plus the settlement-closure row.
ApplyScratch repackaged facts apply_block_admitted already held and serialized raw transactions before the Propose return and coinbase verification, so the wire bytes existed on paths that never committed. Hold txids, the same-block spent set, and the two UTXO capacity locals directly, and serialize raw transactions only in the commit tail when capture_rawtx asks for them. A new regression applies an external spend plus a same-block spend with the flag off and on and requires identical UTXO membership, txids, tip, and commit identity, with undo restoring the same state.
The first form of the test proved undo only through post-disconnect membership. Compare the persisted undo records byte for byte so a capture-flag-dependent durable record cannot pass silently.
ordered checked package structure and submit_package_with_times repeated the same check, so local submission paid two ordered passes over the same transactions. Snapshot restore calls only submit_package_with_times, so that site stays the single authoritative structural gate. No result is cached in another facts type.
block_by_hash only delegated to record_for_hash, two names for one lookup. block_hash_at_height repeated the in-view branches, next_applied_block_hash wrapped one checked successor computation that two other surfaces inlined (one with saturating_add, which made a u32::MAX record its own successor), and handlers repeated confirmation arithmetic the render owner already had. Fold the lookup into block_by_hash, route the height read through the in-view form, give every successor computation the same checked form, and delegate confirmation depth to render::confirmations. REST, Core RPC, and Esplora keep their distinct serializers and error precedence.
NodeState::blocks handed out the raw RwLock<BlockLog> and NodeState::shutdown duplicated the chainstate shutdown handle. Both had canonical owners already: BlockLogReader for reads and Chainstate::shutdown_handle for the atomic. Migrate every caller, preserving each load and store ordering, and keep the pointer-identity assertion on the test-only raw_handle. The generic composition visitors stay: they preserve concrete KvStore types across backend erasure and have live Fjall, redb, and RocksDB consumers.
Pending.fallback kept terminal reconstruction state alive after failure, and compact follow-ups published fetch ownership through two separate marks that the enqueue could race. A failed send could leave an orphan mark suppressing recovery from another peer. Absence of the pending entry now means terminal, releasing filled transactions and retained bytes at once. One enqueue_owned_body_fetch holds the exact-source peer-table guard across the scheduler lock, the nonblocking send, and ownership publication under the 16-entry deferred cap; on failure it publishes nothing. The header-carried ownership flag and its second publisher are deleted.
Keep replayable side headers in the authoritative checkpoint so restart can seed stale-tip relay from the restored block tree. Preserve the active tip and reject corrupt or heavier side sections. Write header artifact version 2 and retain the explicitly approved version 1 reader. Reuse matching restored nodes during journal replay. Checkpoint and journal tests pass on disposable stores; no operator data was opened, converted, or reset.
A peer's block notfound reply left the requested body assigned to that peer until expiry. Release only its exact owned request and let the existing scheduler select another source without a busy retry loop. Keep unavailable-body evidence bounded and expiring. Retain ownership while resolving deferred requests so stale snapshots cannot reinstall a declined or disconnected owner.
Use the captured applied ancestry for locator-based header replies. Keep header admission, best-header selection, and direct hash lookups unchanged. This explicitly approved policy delays early header relay. The same corrected process fixture fails Core convergence with the old serving policy and passes all 99 checks with this change. The 102-test correction union and strict lint also pass.
LND received blocks but could not observe unconfirmed incoming payments. Capture admitted transaction ownership within the commit writer only when rawtx is enabled, then publish exact bytes through the ordered observer before its admission sequence event. Retained payloads survive same-batch expiry without deep copies. Rejected and duplicate submissions publish no admission event.
LND sends the legacy allowhighfees argument when it broadcasts a wallet transaction. Core accepts that shape, but the amount-only parser rejected it before admission. Keep legacy sendrawtransaction semantics separate from strict testmempoolaccept amount parsing.
Reject rename path escape and open checkpoint sources nonblocking with regular-file validation. Preserve operator data and exercise the live storage helpers.
Use explicit error matches and remove an unused test import without changing checkpoint assertions.
- deny.toml: replace dead rustls-mbedtls-provider ban (no such crate) with rustls-mbedcrypto-provider + rustls-mbedpki-provider bans - dependency-range.md: align DEP-01/DEP-02 lanes, KEEP_LOCK restore, skip-anchor invariant, license/source rules with deny.toml + script - corpus.py: verify/document frame offsets; 3 offset assertions - offline-full-validation.md: literal custody paths, transient-copy accounting for pins and per-arm binaries - validation-tooling.md: explicit Core comparator path (core.binary, install-bitcoind.sh --print-path default), narrowed diagnostics - db-migration.md: checkpoint v1 MUST remain readable via read_headers - external-api.md + rpc README: batch HTTP semantics from handle_decoded_json (order, 204, 16 MiB bound, no count limit) - architecture.md: name pool ledger (Mempool Behind Arc RwLock) - contracts README: chain_feed proof via chain_feed.rs tests - embedding.md: name typed Node reads incl. chain-feed set - observability.md: qualify nonzero seed observations (genesis seed emits none until connect/apply)
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.
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is a clean-up and docs stack split from #1687. The Rust and Python changes consist of:
- Line-length reformatting across
corpus.pyandtest_corpus.pyto comply with 88-char PEP 8 limits; no logic changed. crates/rpc/src/handlers/chain.rs: Theconfirmations()function was refactored to delegate tocrate::render::confirmations(view.height(), height, on_active). The old implementation computed depth directly fromtip.height; both paths produce the same value becausetipis derived fromview.tip(). Thenext_applied_block_hashhelper was inlined intoblock_verbose_typed— semantically identical.test_corpus.py:_ShortReads,_RecordingReads, and_FlakyWriteswere refactored to subclassio.BytesIOand typed Protocol bases; the_isolated_productslambda was replaced with a named function; new test methods forwrite_frameand archive-size/trailing-bytes rejection were added.
No blocking defects were found. The PR is safe to merge.
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}
added 2 commits
October 28, 2026 15:29
Replace the observer body-interest callback with registration-time BodyInterest, retention-bound unavailable-body records, and add a three-peer starvation regression.
Register the ZMQ sequence observer with BodyInterest::Bodies only when the publisher has a rawtx subscriber. Metadata-only notification setups no longer retain accepted transaction bodies.
metaphoric-dev
marked this pull request as ready for review
October 28, 2026 09:12
added 3 commits
October 28, 2026 21:35
Merges origin/main at 46de452 (Tor/I2P #1684 over #1685 public-listener split) into stack/11-tools-docs (shared observer/retention ports, ZMQ body interest, corpus and validation-tooling docs). Listener conflicts (main design wins; no ListenerRole or rpc-public-bind restored): crates/p2p/src/listener.rs keeps the branch sync-owner handle_compact_outcome returning Option<(Hash256, Message)> via enqueue_owned_body_fetch, because the merged tree unanimously uses that API (enqueue/defer/resolve_owned_body_fetch, 3-arg send_headers; record_owned_body_fetch and body_fetch_owned exist nowhere else in the merge). Adopts main's #1684 additive change on the same function: peer_addr is impl Into<PeerRemote> with let peer_addr = peer_addr.into(). crates/p2p/src/listener/aggregate_budget_tests.rs keeps the branch .is_none() assertion for the Option return while adopting main's PeerEndpoint::from address construction. All other paths auto-merged (mempool admission/gateway/mutation/orphan, node config/options, rpc core_compat, sync, docs); Cargo.lock merged without conflict so no metadata regeneration was needed.
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.
Collaborator
Author
|
@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. |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
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.
Draft subsystem stack split from PR #1687.
Base: f024fc3
Tip: 796fce6
Full repository gate: native-gate-20261028T000017Z-2220212 (scripts/ci-pr.sh all, exit 0).
Scope: tools and docs.
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
Sync impact: YES. Touches authoritative paths: checkpoint restore writes side branches into the block tree in
crates/chainstate/src/checkpoint/headers.rsand its v2 artifact, and sync request recovery routes unavailable-body retries through the sync owner. Affected clauses: SYNC-04 (recovery closure), SYNC-05 (durable progress), and SYNC-06 (work drivers). Smallest proofs:crates/p2p/src/sync/body-rejection regressions,crates/chainstate/src/checkpoint/tests/side_branches.rs, andcrates/node/tests/overhaul_durable_head.rs.
The PR appears safe to merge; no new blocking issue was established.Summary
This stack adds checkpoint side-branch retention, routes unavailable block bodies back through sync scheduling, and adds accepted-transaction bodies to ordered mempool notifications.
rawtx.sendrawtransactionaccepts the legacyfalsefee argument.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Peer answers notfound] --> B{Exact current owner?} B -->|No| C[Leave request unchanged] B -->|Yes| D[Release owned request] D --> E[Remember peer cannot serve this hash] E --> F[Wake sync] F --> G[Plan request for another peer] E --> H[Retry interval expires] H --> I[Original peer becomes eligible again]Reviews (4) · Last reviewed commit: "Adapt merged sync tests to branch owners..." · Reviewed by Greptile