Repository navigation
Land verified AssumeUTXO stack - #1698
Conversation
Keep canonical coins, raw bodies, and one mutation owner. Distinguish implemented historical-prefix reads from the snapshot-seed and exact block-input targets. Retain early metadata until its readers and recovery dependencies have a verified replacement.
Require bounded prefix continuation for stores and snapshots. Use native backend ranges and migrate test fixtures to the same contract. Attribute planned index row bytes by family without confusing them with physical backend writes. See also: #1616
Validate portable index manifests and artifacts before mutation. Restore through the existing writer and startup path. Require txindex and reject conflicting import and export directives. See also: #1514
Let embedders query confirmed transactions, script history, and exact outpoint spends without requiring every index to reach the live tip. Each answer carries its proven floor, height, and block hash. Use one immutable index snapshot, bounded ancestry, and a post-read active-chain check. Preserve strict live queries and existing endpoints.
…nimal prerequisite from 174ec73, no staged emission) Extract Tx::retained_heap_bytes plus focused test and script retained methods needed by 0ab0d Block::retained_heap_bytes to build (real prerequisite, not invention). Drop block.rs (payload-owner pick carries Block retained itself), block_stager/telemetry/metrics/observability (staged-gauge scope excluded; docs stay family-only). Exact-serialization preserved; no consensus change. Consistency: snapshot reads need snapshot isolation; tip linearizability; seams causal; consumers idempotent.
Keep decoded blocks and their exact consensus bytes together. Reuse the borrowed varint buffers in sighash encoding.
Expose the existing read-only witness-fill decision so mining can preserve exact input bytes before mutating a candidate.
Bind accepted snapshot bytes to a witness and stream them through a bounded worker-owned seed cell with integrity checks.
Use one exact BlockPayload across chainstate, P2P, mining, and RPC. Seed ScriptLive from verified immutable artifacts or retained history replay, with scoped recovery and lease cleanup. The seed path is not an authoritative sync path, and the former seed_live_from_utxo hold gap is now closed. Emit the window pipeline execution diagnostics the observability tests require: `transition_held_us` on the transition guard's drop and error path, and `flush_wait_us` around the durable group flush, both on the apply caller thread. Recover an uncounted bundle base from the receiver's nearest counted ancestor path and retained block metadata, withhold when the required bodies are absent, and export the counted tip count directly, so restore works below a restarted receiver's tip.
Mark T1 and T3 implemented and remove the obsolete independent-input target. Align the contract index with verified snapshot seeding and exact BlockPayload ownership.
Cover malformed-seed witness refusal, archive-finalization seed completion, activation-only offers, and scoped ScriptLive query unavailability with exact public-seam tests.
Apply rustfmt to the existing payload compatibility fixture without changing its behavior.
Keep concrete storage construction in test-only code out of the runtime ownership scan without weakening the one-owner guard. Preserve every query and ScriptLive acceptance test under its existing name.
Replace per-window Vec<BlockPayload> deep clones in p2p/sync/commit.rs with Vec<&BlockPayload> borrowed in drained order; no per-window block or body reconstruction. Change commit_window (p2p/sync/chain.rs, node/sync.rs), Chainstate::apply_window/connect_window (chainstate/lib.rs), and window batch paths (chainstate/window.rs) to &[&BlockPayload] borrowed slices, preserving exact-serialization (payload.bytes() is canonical, never re-encoded) with no consensus change and no Arc<Block> migration. Convert Chainstate::validate_block to borrowed &BlockPayload (no from_local clone); update template validation (node/mining.rs) to move into payload, validate borrowed, recover via into_block for solving, and reuse payload bytes for submit hex; update proposal to &BlockPayload. Update tests to reuse one payload borrowed for validation/application (chainstate/connect/tests, observability) and borrowed slices for windows (cache, invalidation, tx_count, sync/historical fixtures; test SyncChain impls to &[&]); keep setup clones only for borrowed fixtures, not hot path. Consistency: tip publication needs linearizability (single applied tip), snapshot reads need snapshot isolation (immutable DB snapshot + health checks), cross-crate event seams need causal consistency; consumers idempotent via outbox/sagas pattern (no exactly-once).
…cs per Main buildability order, contrary to original exact list) Observability: keep index.family_planned_write_bytes prose and RPC isolation; drop node.package_charge_overrun and four staged-body gauges plus staged tests (no code emits them; staged emission excluded via minimal Tx/script, package code absent). Indexing/README: KEEP IDX-11, portable owners, bundle gaps, portable README scope (code has portable module via full 03f1c55 prerequisite for seed tests; original sequence designed on top); drop pool row only (unrelated mining/pool, no pool/stratum code; portable_bundle test kept as bundle test). Previous docs-cleanup dropped portable/bundle; reworked to match bundle-inclusive reality. README: IDX-01-IDX-12 with historical prefix queries (indexing.md is at 1.6 with IDX-12). Bundle scope included per Main buildability order (seed tests need PortableImportProduce + import_tx_lookup_stream; unified commit needs bytes_are_block helper), contrary to original exact eleven + bundle exclusion. Portable module, PortableImportRow (excluded? Actually index.rs PortableImportRow? Full portable includes PortableImportRow? Yes, but query-only? Hmm. Keep as is: code has portable, docs mention portable). No pool/stratum (excluded unrelated). No staged/package phantom (no code). Consistency: tip linearizability; snapshot reads snapshot isolation; seams causal; consumers idempotent; outbox/sagas.
Keep the input buffer owned so its ignored suffix is released after the exact-prefix copy.
Drop the production decoded-byte and high-water accessors and use DrainedBlock::payload directly for borrowed commit slices; the accessors remain only behind #[cfg(test)] for the staging metric assertions.
Apply canonical Rust formatting to the remaining payload consumers.
Move chainstate journal validation into one helper without changing error order, and remove the unused Hash256 import from the snapshot module. The staging byte metrics are already test-local in the removal commit, so nothing is restored here.
A failed final directory sync could leave a manifest after its artifact was removed. Retain ownership of both publication entries until the directory commit completes, and preserve the artifact when durable manifest removal cannot be established.
Recover an uncounted bundle base from the receiver's nearest counted ancestor path and retained block metadata. Preserve exact identity checks and withhold when the required bodies are absent. Real worker regression, counterfactual failures, and public process restore, refusal, and reorg scenarios verify the recovery path. No stored format or public API changes.
Poll shutdown and revocation during both chain walks, including before work starts. Preserve the existing cancellation outcome and keep old bundles verifiable without an arbitrary rewind cap.
Replace codec-only and orphan-manifest gap claims with the implemented publication cleanup and real Fjall lifecycle evidence. Keep missing process scenarios and performance qualification explicit.
Require locators to match real top-level transaction boundaries and complete chain coverage, refuse incomplete restores, preserve cleanup errors, and flush bundle directories on Windows. Cover embedded transactions, omitted records, and cleanup failure.
… post-reset watermarks, seed paused proof on source set covered_anchor retries when the resolved node height disagrees with the watermark instead of joining a prefix the watermark never covered. grant_leg_history drops a live higher seed pin once the lower fresh grant lands (it pins a superset) and restores it on any refusal, so catch-up never holds two live leases and never loses the seed on transient errors; retained pending rows keep their pin past a moved tip. finish_reconcile_pass reloads fence/watermarks immediately after the scoped ScriptLive reset so leg selection never treats the erased watermark as synced. SeedHarness runs over the minted source set, so the paused-seed concurrency proof commits into the set the seed streams from; focused regressions cover the anchor check, the single-holder drain, and the post-reset replay.
Simplify portable restore matching, remove a redundant semicolon, use character patterns, and keep test failures contextual without changing assertions.
Park the fresh pin on lower-leg stalls and clear stale ScriptLive captures after reset fences reload watermarks.
Name the existing tests for ScriptLive seeding, historical-prefix queries, payload ownership, checkpoint side branches, and paused-seed apply and lease behavior.
Keep one live pin through buffered commits while draining subsumed or fully empty bundles.
Expose dumptxoutset through the chain control and node lifecycle. Publish Core v2 snapshot bytes without replacing an existing file. Document the RPC deviation and exercise the export consumers. See also: #1514
Carry the authenticated coin set and statistics through activation as one consumed proof. Write the existing archive format with its sealed trailer instead of folding statistics again. Preserve proofless inspection and fail-closed recovery behavior. Archive bytes, mutation sensitivity, snapshot process behavior and scoped strict lint are verified. No performance gain is claimed.
Open committed UTXO artifacts through the shared nonblocking regular-file guard, remove the unused no-replace wrapper, and keep snapshot output borrowing direct.
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
AssumeUTXO Stack — Review Summary
This PR introduces the AssumeUTXO subsystem: snapshot activation, background historical validation, witnessed archive staging (write_coins_sealed/SealedTrailer), historical checkpoint acceleration, and the full lifecycle fail-close/finalization machinery. The overall architecture is sound and the security-critical commitment verification paths are well-structured.
Critical Issues Found
1. TOCTOU anchor lookup in activate_verified_snapshot (crates/chainstate/src/assumeutxo.rs, line 577–588): The pinned anchor is resolved from state.height() before acquiring the lifecycle lock. The actual guard (require_unactivated) runs after the lock, but uses a different check (disk status) than anchor identity. While install_verified re-verifies state.height() != pinned.height, the pinned was resolved in a window where the lifecycle state had not yet been serialized. The fix is to move the anchor lookup inside the lifecycle lock. A one-click suggestion is attached.
2. Missing else branch for outcome.height > base_height in step_historical (crates/chainstate/src/assumeutxo.rs, line 808–873): When the historical chainstate's connected block height exceeds base_height, neither the < base_height nor == base_height branch executes. persist_status is silently skipped, historical_undo.remove_archived is still called, and Ok(outcome) is returned — bypassing all lifecycle finalization and durability commits. This path should explicitly fail-close since the Historical role enforces a stop at base_height.
3. VerifiedSnapshotState consumed before directory creation in write_coins_sealed (crates/chainstate/src/assumeutxo_snapshot.rs, line 288–306): state.into_parts() is called first, destructuring the proof into set and stats. If the subsequent archive_directory() call fails, the proven VerifiedSnapshotState is irrecoverably consumed and dropped — the caller cannot retry with the same proven state. The directory creation should precede the proof consumption.
Minor Issues (not blocking, flagged for awareness)
- The
height + 1arithmetic inbegin_historicalandnext_historical_blockuses bare addition without overflow protection, unlikerecover_pendingwhich usessaturating_add(1). Whileu32::MAXheights are practically impossible on any real chain, this inconsistency is worth noting.
The crash-recovery tests (process_death_recovers_each_snapshot_phase_from_the_committed_root), the commitment-mismatch fail-close tests, and the e2e core_snapshot_import_syncs_both_roles_and_recovers test provide excellent coverage of the lifecycle state machine. The sealed_archive_matches_recomputed_archive_byte_for_byte test is a strong guard for the new write_coins_sealed/SealedTrailer path.
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}
Resolve the pinned anchor inside the lifecycle lock, create the archive directory before consuming state, clear interrupted seed rows before replay, stream bounded index pages, verify bundle locators by height, and buffer UTXO seed reads. The remote branch already corrects the indexing contract.
Charge each grouped locator position against a 256 MiB ceiling and refuse a larger bundle with a typed error before building an unbounded map. Release the charge as each height verifies.
| let added = record.positions.len().saturating_mul(LOCATOR_POSITION_BYTES); | ||
| if self.pending_bytes.saturating_add(added) > MAX_LOCATOR_PENDING_BYTES { | ||
| return Err(IndexBundleError::RestoreTooLarge { | ||
| limit_bytes: MAX_LOCATOR_PENDING_BYTES, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Restore now rejects every valid bundle with more than 4,194,304 transaction positions. validate_artifact collects all positions before checking any height, so the 256 MiB counter limits the whole bundle, not one batch.
Export has no matching limit and can successfully produce these bundles. Import then logs a refusal and rebuilds the index from block history instead of restoring it. Keep memory bounded by spilling grouped positions to disk or using bounded verification passes, rather than refusing otherwise valid exports.
Share one 4,194,304-position ceiling between export and restore. Export refuses to publish a bundle restore would reject, and restore refuses an oversized bundle before building the grouped map.
Main brings #1684 AddrMan overlay support and #1685 public-listener addressing (PeerRemote/PeerEndpoint, overlay validate, expanded getnetworkinfo compat). Branch preserves AssumeUTXO index-bundle config plus the BlockPayload/linearizable-compact-outcome listener lineage from its parent stacks. Resolutions: - crates/node/src/config.rs: union; keep validate_index_bundle() (branch) and add p2p.overlay.validate() (main). - crates/p2p/src/listener.rs send_block: main's impl Into<PeerRemote> peer with branch's BlockPayload body; every merged caller (message loop, compact outcome, unit tests) passes a payload against wire Message::Block(BlockPayload), so the (Block, serialized) signature has no live caller in this tree. - crates/p2p/src/listener.rs handle_compact_outcome: keep the Option-follow-up/enqueue_owned_body_fetch design all merged callers use, with main's PeerRemote addressing at the boundary. - aggregate_budget_tests.rs: main's PeerEndpoint addr with the Option-style .is_none() assertion matching the kept signature. - crates/rpc/tests/core_compat.rs: take main's expanded networks assertion (branch had dropped the stale len==3 check); all other auto-merged cases from both sides kept. - No ListenerRole or rpc-public-bind restored; Cargo.lock regenerated via cargo metadata.
Second main merge (fa187f9: #1676 ranged/multipath descriptor scan, #1677 compact-blocks-near-tip). Prior merge resolution (a74ef07) preserved untouched: union config validators, main PeerRemote addressing with payload/Option listener lineage, main compat assertions, branch-canonical AddrMan v1. This merge auto-resolved with no conflicts: scantxoutset handler additions, sync compact-near-tip gating, registry/docs updates.
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.
There was a problem hiding this comment.
Greptile has paused reviews on this repository — it used its 300 free open-source review credits for this billing period. Reviews resume automatically on November 13. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.
|
@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: 297f73fb53
ℹ️ 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".
| for tx in parsed.transactions() { | ||
| let span = tx.span(); | ||
| if is_top_level_span(position, span.start(), span.len()) { | ||
| return tx.materialize().txid().0.to_le_bytes() == expected; |
There was a problem hiding this comment.
When restoring a bundle containing transaction-heavy blocks, verify_height_positions calls this function for every transaction position, but each call restarts its search at the beginning of the block. A complete block with N transactions therefore requires N(N+1)/2 span comparisons, making verification quadratic per block even though the body is parsed only once. Across the allowed 4,194,304 positions, this adds substantial CPU work to the index worker before reconciliation can resume. Since parsed.transactions() is already in block-offset order, binary-search the requested offset, or sort the collected positions and verify them with a single merge walk, while retaining the exact span and txid checks.
Useful? React with 👍 / 👎.
Writer, decoder, and hash_serialized_3 feed sort vouts by bytewise subtract-one MSB VARINT order (the coins-DB cursor order), not numeric vout. Numeric order diverges at the 16,512-output width transition (16511 encodes [FF,7F] > 16512 [80,80,00]), which broke commitment agreement with Core. Duplicate detection is preserved. Adds a focused test around vout 16,511/16,512 covering decoder staging, writer emission, and stable-view agreement.
Staged temp names embed pid plus a wall-clock and process-wide counter suffix with create_new, so a SIGKILL orphan from a reused pid can never collide with a later export. Retries generate a fresh suffix on AlreadyExists (bounded). Nothing unlinks another process's staged file; StagedFile now owns its temp name. Adds a test that a stale same-pid-looking temp file does not block a new export.
|
@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". |
Draft subsystem stack split from PR #1687.
Base: f024fc3
Tip: 1b18364
Full repository gate: native-gate-20261027T231422Z-1260404 (scripts/ci-pr.sh all, exit 0).
Scope: AssumeUTXO.
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 sync and commit paths:
SyncChain::commit_windowandconnect_historical, as a borrow-only refactor from ownedBlockplusBytesto borrowedBlockPayloadslices; outcomes, follower order, and settlement are unchanged. Affected clauses: SYNC-02, SYNC-05, SYNC-06, SYNC-04, and ARCH-07b. Smallest proofs:crates/node/tests/sync_smoke.rs,crates/node/tests/overhaul_durable_head.rs,chain_effects.rs::committed_connect_settlement_failure, andcrates/node/tests/crash_recovery.rs.The large-bundle restore finding remains blocking; restricting export does not remove restore's whole-bundle limit.
Findings
Summary
The AssumeUTXO stack adds verified snapshot state, snapshot-backed index seeding, portable index bundles, and borrowed
BlockPayloadpaths.The latest changes:
Reviews (5) · Last reviewed commit: "Match export and restore bundle position..." · Reviewed by Greptile