Skip to content

feat: validate chunk cache content#16081

Open
viviveevee wants to merge 33 commits into
masterfrom
vivee/validated-chunk-cache-v2
Open

feat: validate chunk cache content#16081
viviveevee wants to merge 33 commits into
masterfrom
vivee/validated-chunk-cache-v2

Conversation

@viviveevee

@viviveevee viviveevee commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Verify a partial encoded chunk's header producer signature at arrival, without waiting for the parent block to be processed. Today, a chunk's signature can only be checked once its parent block is available (the parent is needed to resolve the chunk producer). Near the tip, parts routinely beat their block, so the pipeline caches and gossips chunks on a header whose authenticity is still unproven.

This PR carries a grandparent anchor (prev_prev_block_hash + epoch_id) in a new PartialEncodedChunkV3 wire message. The anchor is enough to resolve the producer for any chunk within head+2 even when the parent is not yet processed, so the header signature can be verified immediately. Gated behind ProtocolFeature::EarlyKickout.

Changes

  • core/primitives/src/sharding.rs — new PartialEncodedChunkV3 variant (= 2) carrying prev_prev_block_hash and epoch_id, all enum accessor arms, prev_prev_block_hash() / epoch_id() accessors, and the From<PartialEncodedChunkWithArcReceipts> impl that emits V3 when the anchor is present.
  • chain/chunks/src/shards_manager_actor.rs — producer emits V3 under EarlyKickout (anchor via grandparent_anchor); arrival path captures the anchor before the V2 collapse and threads it into validate_chunk_header_preliminary, which now resolves the producer from the anchor and verifies the signature. An anchorless V2 message on an EarlyKickout epoch is dropped.
  • chain/chain/src/signature_verification.rs — shared, carrier-agnostic anchored helpers (resolve_anchored_producer, verify_anchored_chunk_key, resolve_and_verify_anchored_producer), factored out and reused by both chunk-header and state-witness verification.
  • chain/client/src/stateless_validation/validate.rs — witness verification now calls the shared helpers instead of its own copies.
  • MetricsANCHORED_CHUNK_PRODUCER_LOOKUP_TOTAL moved from the client crate to chain/chain, now labeled by message_type (chunk / witness / contract_accesses / contract_deploys) and result.
  • Protocol schema — regenerated for the new message variant.

Testing

  • chain/chunks (shards_manager_actor.rs, test_utils.rs) — arrival-time unit tests: a valid V3 chunk beating its parent is verified and cached; a forged header (bad signature, or valid signature but height/epoch mismatched against the anchor) is rejected and not cached; an anchorless V2 message on an EarlyKickout epoch is dropped; V3 borsh round-trip.
  • chain/client/src/stateless_validation/validate_tests.rs — shared-helper edge cases (genesis / genesis+1 default anchor, skipped heights, parent-present vs parent-absent) run against the extracted helpers.
  • test-loop-teststest_early_kickout_version_upgrade (early_kickout_boundary.rs): a live chain upgrading into EarlyKickout reaches post-feature epochs with no skipped blocks and no missing chunks (a dropped valid V3 chunk would surface as a missing chunk).

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.90751% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.33%. Comparing base (b8a041c) to head (9e57a79).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
chain/chunks/src/shards_manager_actor.rs 89.56% 11 Missing and 8 partials ⚠️
chain/chain/src/signature_verification.rs 92.59% 4 Missing and 6 partials ⚠️
core/primitives/src/sharding.rs 84.48% 9 Missing ⚠️
.../client/src/stateless_validation/validate_tests.rs 94.36% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #16081      +/-   ##
==========================================
+ Coverage   73.19%   73.33%   +0.13%     
==========================================
  Files         856      857       +1     
  Lines      188394   189410    +1016     
  Branches   188394   189410    +1016     
==========================================
+ Hits       137904   138902     +998     
+ Misses      46070    46066       -4     
- Partials     4420     4442      +22     
Flag Coverage Δ
pytests-nightly 1.22% <0.00%> (-0.01%) ⬇️
unittests 69.98% <85.80%> (+0.12%) ⬆️
unittests-nightly 69.99% <92.05%> (+0.13%) ⬆️
unittests-spice 65.62% <89.53%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@viviveevee
viviveevee requested review from jancionear and stedfn July 17, 2026 05:59
@viviveevee
viviveevee marked this pull request as ready for review July 17, 2026 05:59
@viviveevee
viviveevee requested a review from a team as a code owner July 17, 2026 05:59
@github-actions

Copy link
Copy Markdown

Pull request overview

Adds a PartialEncodedChunkV3 wire message carrying a grandparent anchor (prev_prev_block_hash + epoch_id) so the header producer signature can be verified at arrival, without waiting for the parent block to land. Gated behind ProtocolFeature::EarlyKickout. On EarlyKickout epochs the shards-manager arrival path rejects anchorless V2 and forged V3 chunks before they enter the cache; the anchored-lookup helpers previously living in client/stateless_validation/validate.rs are moved into chain/chain/src/signature_verification.rs and reused by both the chunk-header and the witness/contract-distribution paths. Metric ANCHORED_CHUNK_PRODUCER_LOOKUP_TOTAL moves along with them and gains a message_type=chunk label.

Changes:

  • New PartialEncodedChunkV3 variant + accessors; From<PartialEncodedChunkWithArcReceipts> emits V3 when the anchor is present.
  • ShardsManagerActor::validate_chunk_header_preliminary verifies the anchored producer signature on EarlyKickout epochs; producer path threads grandparent_anchor into create_partial_chunk.
  • resolve_anchored_producer, verify_anchored_chunk_key, resolve_and_verify_anchored_producer extracted to signature_verification.rs; witness/deploys/accesses paths call the shared helpers.
  • Metric relocated to chain/chain::metrics with a chunk label added and label spaces normalized (contract accessescontract_accesses, contract deployscontract_deploys).
  • Arrival-time unit tests for good/forged/anchorless chunks; test-loop-tests::test_early_kickout_version_upgrade for the live upgrade boundary.

Reviewed changes

Per-file summary
File Description
chain/chain/src/metrics.rs Adds ANCHORED_CHUNK_PRODUCER_LOOKUP_TOTAL (moved from client) with chunk label added.
chain/chain/src/signature_verification.rs Adds shared resolve_anchored_producer, verify_anchored_chunk_key, resolve_and_verify_anchored_producer; adds parent-absent unit test.
chain/chunks/src/shards_manager_actor.rs Threads anchor through preliminary validation; rejects forged/anchorless chunks at arrival on EarlyKickout epochs; producer path attaches anchor; new/rewritten arrival tests.
chain/chunks/src/test_utils.rs Fixture takes protocol_version; new wrap_partial_encoded_chunk emits V3 under EarlyKickout.
chain/client/src/metrics.rs Removes ANCHORED_CHUNK_PRODUCER_LOOKUP_TOTAL (moved to chain).
chain/client/src/stateless_validation/validate.rs Deletes local anchor helpers; calls shared helpers; changes label strings.
chain/client/src/stateless_validation/validate_tests.rs Tests updated to loose lower-bound semantics and new label spelling.
chain/epoch-manager/src/test_utils.rs Adds epoch_config_at_version overload taking protocol_version.
core/primitives/src/sharding.rs Adds PartialEncodedChunkV3, enum accessors, From impls; adds prev_prev_block_hash/epoch_id on PartialEncodedChunkWithArcReceipts; #[allow(clippy::large_enum_variant)] on ShardChunk/EncodedShardChunk.
cspell.json Adds anchorless.
test-loop-tests/src/tests/early_kickout_boundary.rs New live-upgrade test asserting no missing chunks / skipped blocks across the boundary.
test-loop-tests/src/tests/mod.rs Wires the new test module (nightly-only).
tools/protocol-schema-check/res/protocol_schema.toml Regenerated schema hashes for PartialEncodedChunk*, routed-message, tiered-message.

Findings

Blocking (must fix before merge):

  • chain/client/src/stateless_validation/validate_tests.rs:35 — doc comment is broken. Currently reads:

    Lowest protocol version with EarlyKickout enabled — forces the V2 wire variant and turns
    variant regardless of the compile-time nightly feature.
    The "and turns / variant" fragment is a stale edit; sentence does not parse. Replace with the intended text, e.g.

    /// Lowest protocol version with `EarlyKickout` enabled — forces the V2 wire
    /// variant regardless of the compile-time `nightly` feature.
    

Non-blocking (nits, follow-ups, suggestions):

  • chain/chain/src/metrics.rs:10-24 — the metric label values change from contract accesses/contract deploys (with spaces) to contract_accesses/contract_deploys. This is a good change (Prometheus convention + parseability), but it is a silent breaking change for any dashboards/alerts filtering on those exact strings. Worth a heads-up in the PR body and to whoever owns the observability config.
  • chain/chain/src/signature_verification.rs:66,86,102parent_info.height() + 1 and anchor_height + CHUNK_GRANDPARENT_ANCHOR_HEIGHT_OFFSET use raw + on BlockHeight. Practically infallible, but docs/practices/style.md prefers checked_add(...).expect("...") for arithmetic on integers, with a note on why overflow is impossible. Cheap and self-documenting.
  • chain/chain/src/signature_verification.rs:82-107 — when the parent is absent, verify_anchored_chunk_key never cross-checks epoch_id against the anchor's epoch; it only bounds the height. Safety currently relies on the caller (shards-manager: possible_epochs_of_height_around_tip; witness path: validate_chunk_relevant) having already plausibility-checked epoch_id. A future caller that skips that step would open a hole. Consider either documenting this precondition in the function docstring or performing the cross-check inside the helper so it's self-contained.
  • chain/chunks/src/shards_manager_actor.rs:2222-2230grandparent_anchor(&prev_block_hash)?.unwrap_or_default() silently falls back to CryptoHash::default(). That's the intended genesis behavior, but any caller reaching this branch at height > genesis + 1 would forge a chunk that gets rejected on the peer side. A debug_assert!(chunk_header.height_created() <= genesis_height + 1) (or equivalent) would surface a producer bug locally instead of only at the receiver.
  • chain/chunks/src/shards_manager_actor.rs:2500-2504,3120,3842,3882,3915,3939 — the new arrival-time tests (test_mismatched_height_rejected_at_arrival, test_bad_signature_chunk_rejected_at_arrival, test_cached_bad_chunk_evicted_by_full_validation, test_v2_message_on_early_kickout_epoch_rejected, test_v3_borsh_round_trip) are not #[cfg(feature = "nightly")]-gated even though EarlyKickout sits under the nightly threshold in core/primitives-core/src/version.rs:580. ProtocolFeature::enabled is a plain numeric comparison so this appears to work today, but it means the nightly gate is now purely conventional — worth a sanity check that nothing else keys on the nightly cfg here and, if not, a one-line rationale next to early_kickout_fixture() would help future readers.
  • core/primitives/src/sharding.rs:945-948 — the epoch_id: EpochId field is Not trusted, but the docstring on the field just says "The chunk's own epoch id. Not trusted." Consider adding one line on how it's used (looked up to resolve the producer at the anchor, signature check catches forgeries) so a reader doesn't wonder why an untrusted field is on the wire at all.

⚠️ Issues found

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant