Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/actors/src/block_validation.rs`:
- Around line 1421-1441: The code currently falls through to get_block_bounds
when latest.ledgers has no entry for ledger, causing missing-ledger to be
treated as an internal BlockBoundsLookupError; change the early disambiguation
in the block_index_guard read (the block where latest.ledgers.iter().find(...)
is used) to explicitly handle the None case for the ledger: if the find(...)
returns None, return a consensus-invalid PreValidationError (either reuse
PreValidationError::PoAChunkOffsetOutOfBlockBounds or add a new variant like
PreValidationError::PoALedgerNotPresent) instead of continuing to
get_block_bounds; keep the existing check that returns
PoAChunkOffsetOutOfBlockBounds when ledger_chunk_offset >= chain_max for
Some(chain_max).

In `@crates/actors/src/validation_service/block_validation_task.rs`:
- Around line 57-62: The validate_block path still re-reads the parent's epoch
snapshot from the block tree which reintroduces the eviction race; modify
validate_block to use the captured self.parent_epoch_snapshot (the
Arc<EpochSnapshot> field) everywhere it's needed (not only in
data_txs_are_valid) — specifically replace calls to
block_tree.get_epoch_snapshot()/similar lookups before PoA checks, shadow-tx
validation, and when computing cascade_active; derive cascade_active from the
captured snapshot via
config.consensus.hardforks.is_cascade_active_for_epoch(&self.parent_epoch_snapshot)
so the validator uses the same activation rule as the producer and avoids
ParentEpochSnapshotMissing races.

In `@crates/chain-tests/src/block_production/block_validation.rs`:
- Around line 436-450: The test currently computes cascade_active from
tree.canonical_epoch_snapshot(), causing divergence at epoch boundaries;
instead, derive cascade_active from the parent epoch snapshot you already
fetched: call
config_override.consensus.hardforks.is_cascade_active_for_epoch(&parent_epoch)
(or &parent_epoch_snapshot) after obtaining parent_epoch via
tree.get_epoch_snapshot(&parent_hash) and replace the use of
tree.canonical_epoch_snapshot() with this parent-based check so cascade_active
aligns with the parent_epoch_snapshot used elsewhere in the block validation
test.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/actors/src/block_tree_service.rs`:
- Around line 415-457: The InternalFailure branch in
on_block_validation_finished leaves the block marked as scheduled/validating and
never re-enqueues it, which can permanently stall the block and its descendants;
update this branch to clear the "validation scheduled" flag and re-schedule
validation for transient internal failures: after computing height/state (using
cache.get_block and cache.get_block_and_status as shown) call the routine that
clears the scheduled validation marker (e.g.,
clear_scheduled_validation(block_hash) or unset the scheduled flag in the cache
entry) and then enqueue the block for validation again (e.g.,
schedule_validation(block_hash) or push into the validation queue) so that
on_block_prevalidated will not short-circuit future retries; keep the existing
BlockStateUpdated event and logging, and ensure any cache writes use the same
locking/error handling pattern as elsewhere to avoid deadlocks.

---

Outside diff comments:
In `@crates/actors/src/validation_service/block_validation_task.rs`:
- Around line 683-689: The current match on shadow_tx_result treats any
eyre::Result error as ShadowTransactionInvalid; instead inspect the underlying
error returned by shadow_transactions_are_valid() (use err.downcast_ref::<...>()
or pattern-match known error enums from the payload cache / DB / parent lookup)
and only map genuine transaction-invalid errors to
ValidationError::ShadowTransactionInvalid; map cache-miss / DB/view failures /
missing-parent cases to a transient/internal validation result (e.g.,
ValidationError::InternalError or a transient variant you have) so local
failures are not classified as consensus-invalid. Reference shadow_tx_result,
shadow_transactions_are_valid(), and ValidationError::ShadowTransactionInvalid
when updating the match arm.

---

Duplicate comments:
In `@crates/actors/src/block_validation.rs`:
- Around line 1480-1495: The check currently only rejects when latest.ledgers
contains the ledger and the offset is out-of-bounds, but if latest.ledgers has
no entry for ledger we should also reject as consensus-invalid instead of
falling through to get_block_bounds; modify the block starting with let index =
block_index_guard.read(); to explicitly treat the case where latest exists but
latest.ledgers.iter().find(|l| l.ledger == ledger) is None as
Err(PreValidationError::PoAChunkOffsetOutOfBlockBounds) (i.e. return that error
before calling index.get_block_bounds), keeping the existing early-return when
ledger_chunk_offset >= chain_max and preserving the existing
get_block_bounds(...) call for the remaining path. Use the symbols index,
latest, latest.ledgers.iter().find(...), ledger, ledger_chunk_offset, and
PreValidationError::PoAChunkOffsetOutOfBlockBounds to locate and change the
logic.

In `@crates/actors/src/validation_service/block_validation_task.rs`:
- Around line 384-400: The code currently re-reads the parent epoch snapshot
multiple times in validate_block() (the get_epoch_snapshot call that assigns
parent_epoch_snapshot) and then later before PoA and before shadow/cascade
activation, which can race if the parent is evicted; change validate_block() to
capture the parent_epoch_snapshot once (the value returned by
block_tree_guard.read().get_epoch_snapshot(&block.previous_block_hash)) and
reuse that same snapshot for all subsequent checks and for computing
cascade_active using
config.consensus.hardforks.is_cascade_active_for_epoch(&parent_epoch_snapshot);
also pass this captured parent_epoch_snapshot (and derived EMA/activation flag)
into data_txs_are_valid() instead of re-reading the snapshot; apply the same
single-capture-and-reuse fix to the other occurrences referenced (the similar
get_epoch_snapshot usages around the 477-499 and 633-641 regions).

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/actors/src/validation_service/block_validation_task.rs`:
- Around line 684-688: The current match on shadow_tx_result collapses all
failures into ValidationError::ShadowTransactionInvalid; instead inspect the
error returned by shadow_transactions_are_valid (shadow_tx_result) and match its
specific variants so that transient/local/runtime failures (e.g. missing
execution payloads, DB/block-tree lookup failures, parent-snapshot eviction) are
returned as an InternalFailure variant while only true consensus-rejection
errors map to ValidationError::ShadowTransactionInvalid; update the match arm
handling Err(err) to distinguish error kinds (using the error enum/variants from
shadow_transactions_are_valid or helper methods on the error) and return
ValidationError::InternalFailure(...) for those local/runtime cases and
ValidationError::ShadowTransactionInvalid(err.to_string()) only for genuine
shadow-tx validation rejections.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/actors/src/validation_service/block_validation_task.rs`:
- Around line 530-539: The shadow-tx validation code currently maps every eyre
failure into ValidationResult::Invalid(ShadowTransactionInvalid) (e.g., the
block_tree_guard.read().get_commitment_snapshot(...) call and similar calls
later), which wrongly treats infra/runtime errors as consensus violations;
change the shadow-tx stage to return a typed Result<ValidationResult,
ShadowTxError> (or a ValidationResult enum that distinguishes Invalid(reason) vs
InternalFailure(error)), update callers of validate_shadow_transactions / the
shadow-tx stage to propagate InternalFailure for non-rule errors (including
errors from get_commitment_snapshot, execution-payload provider calls, and other
runtime failures) and only produce
ValidationResult::Invalid(ShadowTransactionInvalid) for explicit consensus rule
violations; ensure error construction preserves the original eyre error for
InternalFailure so logs/metrics can classify transient infra failures
separately.

---

Duplicate comments:
In `@crates/actors/src/validation_service/block_validation_task.rs`:
- Around line 407-423: Capture the parent epoch snapshot once at the start of
validate_block() by calling
block_tree_guard.read().get_epoch_snapshot(&block.previous_block_hash) and
storing the returned Arc as parent_epoch_snapshot (reuse the Some(snapshot)
branch value) instead of re-reading it later; then pass that same
parent_epoch_snapshot Arc into the PoA validation, shadow-tx/cascade validation
tasks and derive cascade_active from it via
config.consensus.hardforks.is_cascade_active_for_epoch(&parent_epoch_snapshot)
so all stages use the identical snapshot and activation rule (replace subsequent
calls that re-query the block tree with the stored parent_epoch_snapshot).

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/actors/src/block_validation.rs`:
- Around line 1424-1442: Convert the table-driven tests to rstest parameterized
cases: change the test function validation_cancel_reason_classifier_dispatch to
use #[rstest] with a #[case] for each ValidationCancelReason variant
(HeightDifference, ParentMissing, ChannelClosed), accept a parameter named
reason, and run the two assertions inside the body
(assert!(!reason.is_internal()) and
assert!(!ValidationError::ValidationCancelled { reason
}.is_internal_failure())). Do the same refactor for the other table-driven test
referenced (lines 1448-1501) so each variant is a separate #[case], producing
per-case test failures and following the repo convention.
- Around line 1507-1510: Add a regression test that constructs a "latest index"
entry that omits the target PoA ledger and calls poa_is_valid() on that
constructed entry, asserting it returns
Err(PreValidationError::PoALedgerInactive { ledger_id: <expected> });
specifically, instead of only instantiating
PreValidationError::PoALedgerInactive directly, build the latest index entry
(using the same builders/helpers used elsewhere in this module) without adding
the target ledger so the validation path that previously produced
BlockBoundsLookupError is exercised, then call poa_is_valid() and assert it
yields the PoALedgerInactive variant.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/actors/src/block_validation.rs`:
- Around line 699-725: The is_internal_failure() method currently treats all
unlisted ValidationError variants as consensus-invalid via the `_ => false`
fallback; change that fallback to conservative `_ => true` and/or add explicit
match arms returning true for the variants produced by runtime/local-dependent
helpers (the errors constructed by recall_recall_range_is_valid(),
shadow_transactions_are_valid(), submit_payload_to_reth(), etc.) so those
runtime/transport/VDF-related failure cases are classified as internal; edit the
match in is_internal_failure to return true for those specific variants (or flip
the default) rather than defaulting to false.

---

Duplicate comments:
In `@crates/actors/src/block_tree_service.rs`:
- Around line 435-485: The InternalFailure branch in
on_block_validation_finished currently leaves the cache entry as-is which
preserves the "validation-scheduled" state and prevents on_block_prevalidated
from ever retrying; change this path to clear the scheduled/validating flag and
enqueue the block for retry (or at least mark it as not-scheduled) instead of
just returning. Specifically, in the ValidationResult::InternalFailure arm in
on_block_validation_finished, update the cached block status (via
cache.get_block_and_status / cache mutation helpers) to remove the
validation-scheduled marker (or set state to a retryable state), update
chain_sync_state.record_block_validation_error as needed, and emit the
BlockStateUpdated event with the new state so service_senders.block_state_events
consumers can retry; ensure this complements on_block_prevalidated's
short-circuit logic so transient internal failures are retried rather than
permanently stranded.

In `@crates/actors/src/block_validation.rs`:
- Around line 1487-1505: Replace the manual loop/table-style assertions in the
test function validation_cancel_reason_classifier_dispatch with an rstest
parameterized test: add #[rstest] and #[case(...)] entries for each
ValidationCancelReason variant (e.g., HeightDifference, ParentMissing,
ChannelClosed), accept a reason: ValidationCancelReason parameter, and run the
two assertions against reason.is_internal() and
ValidationError::ValidationCancelled { reason }.is_internal_failure(); do the
same conversion for the other similar test blocks that check classifier tables
so each case shows per-case failures under rstest.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@crates/actors/src/block_tree_service.rs`:
- Around line 415-485: The InternalFailure branch in
on_block_validation_finished leaves the cache entry in ValidationScheduled,
which prevents on_block_prevalidated from retrying the block; update this branch
to clear the ValidationScheduled marker (or change the block's status to a
non-scheduled state) and/or re-enqueue the block for prevalidation so transient
internal failures don't stall validation permanently: locate the
ValidationResult::InternalFailure handling in on_block_validation_finished,
after recording the error with chain_sync_state.record_block_validation_error
and before returning, mutate the cache entry that matches block_hash to remove
ValidationScheduled (or set state to NotOnchain/Unknown) and/or push the block
back onto the prevalidation queue so on_block_prevalidated can attempt
validation again; ensure the BlockStateUpdated event (BlockStateUpdated { ...
validation_result }) reflects the new status and that
service_senders.block_state_events still broadcasts the update.

In `@crates/actors/src/block_validation.rs`:
- Around line 703-739: The is_internal_failure() matcher currently treats
RecallRangeInvalid and ExecutionLayerFailed as consensus-invalid (false) even
though their inner causes can be local/runtime outages; update
is_internal_failure() to inspect those variant payloads and return true when
their contained error indicates a local/internal failure (e.g., errors produced
by recall_recall_range_is_valid()’s steps_guard read/get_steps or
engine/transport errors from submit_payload_to_reth()). Concretely, add arms for
Self::RecallRangeInvalid { source, .. } and Self::ExecutionLayerFailed { source,
.. } (or the exact field names in ValidationError) and call a helper on the
inner error (or match its concrete error kinds) to return true for
IO/transport/lock/read-related failures and false for genuine consensus-invalid
reasons so that local outages are classified as InternalFailure instead of
Invalid.

In `@crates/actors/src/validation_service.rs`:
- Around line 436-462: The code treats both cancelled and panicked concurrent
validation tasks as ValidationError::TaskPanicked; change the branch so when
e.is_cancelled() is true you send the cancellation validation variant instead of
TaskPanicked (i.e., use the existing cancellation enum variant — e.g.
ValidationError::Cancelled or ValidationError::TaskCancelled — with the same
task/details fields) in the call to self.send_validation_result, leaving the
panic branch to continue sending ValidationError::TaskPanicked; update the error
message/logging if needed so the sent result matches e.is_cancelled().

In `@crates/actors/src/validation_service/block_validation_task.rs`:
- Around line 424-440: The code currently re-reads the parent epoch snapshot
(via block_tree_guard.read().get_epoch_snapshot(&block.previous_block_hash))
before spawning shadow/data stages which can fail if the snapshot is evicted;
instead, clone the already obtained parent_epoch_snapshot Arc and pass that
clone into the later tasks, remove the second get_epoch_snapshot call so only
the EMA snapshot is fetched there, and compute cascade_active by calling
config.consensus.hardforks.is_cascade_active_for_epoch(&parent_epoch_snapshot)
(apply the same change to the analogous block around the 516-553 range). Ensure
all references to the parent snapshot in spawn/shadow/data-stage code use the
cloned parent_epoch_snapshot variable rather than re-reading from
block_tree_guard.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/actors/src/shadow_tx_generator.rs`:
- Around line 237-255: The check that detects a promoted publish transaction
also scheduled for a perm_fee refund should return a peer-structure error, not a
snapshot invariant; change the error constructor used in the loop that iterates
publish_ledger.txs and ledger_expiry_balance_delta.user_perm_fee_refunds from
ShadowTxGenError::SnapshotInvariant(...) to ShadowTxGenError::Structural(...) so
the condition is classified as a malformed block shape (consistent with the
surrounding comment and generate_expected_shadow_transactions validation
behavior).

---

Outside diff comments:
In `@crates/domain/src/models/block_index.rs`:
- Around line 253-293: The anchored lookup wrongly assumes the anchor block
contains the ledger and treats missing ledgers during probes as total_chunks=0;
instead, when computing anchor_max you must walk backwards from anchor_height to
find the most recent block <= anchor_height that contains ledger and use its
total_chunks (use block_index_item_by_height in a loop until found or zero), and
likewise when probing in the binary search replace unwrap_or(0) behavior with a
carry-forward lookup that finds the latest prior block <= mid that has the
ledger (not just treating absent ledgers as 0); adjust uses of chunk_offset,
anchor_height and ledger accordingly so anchors and mid probes use
carried-forward totals consistent with how push_block writes only present
ledgers.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/actors/src/validation_service/block_validation_task.rs`:
- Around line 856-895: The cancel path currently uses tokio::join! (stages_join)
which only becomes Ready when all six stage futures finish, so a
partially-completed stage (e.g., a NodeFault) is missed when cancellation fires;
fix by replacing the tokio::join!-based composite (stages_join) with a drainable
collection (e.g., futures::stream::FuturesUnordered or individually polled
futures) that yields per-stage completions, wire that into the select with
exit_if_block_is_too_old and, in the cancel branch after aborting poa via
poa_abort_slot, repeatedly poll/drain the collection to collect any Ready stage
results and then call merge_stage_results_with_cancel (using the same
StageOutcome variants: Joined, JoinedAfterCancel, Cancelled) so
already-completed NodeFaults are observed and merged before returning.
- Around line 561-584: The Err branch in the match that maps `res` to
`ValidationResult` currently treats all `Err(err)` as
`ValidationError::TaskPanicked`; update that branch to first check
`err.is_cancelled()` (the `JoinError` from the PoA `JoinHandle`) and if true
return `ValidationError::ValidationCancelled` (preserving a helpful `details`
string), otherwise preserve the existing panic handling and return
`ValidationError::TaskPanicked` (as currently done); refer to the match around
`let result: ValidationResult = match res { ... }`, the `AbortHandle::abort()`
usage that can produce cancelled `JoinError`, and the
`ValidationError::TaskPanicked` / `ValidationError::ValidationCancelled`
variants to locate where to implement the `is_cancelled()` check and mapping.

In `@crates/chain-tests/src/validation/data_tx_pricing.rs`:
- Around line 149-155: The test currently treats any
ValidationError::ShadowTransactionInvalid(_) as acceptable which allows
unrelated failures to pass; update both match arms that now use
ValidationError::ShadowTransactionInvalid(_) so they only accept the
shadow-validation variant(s) that indicate a perm-fee problem (i.e., replace the
wildcard with the concrete inner variant(s) that represent perm-fee errors from
the ShadowTransactionInvalid payload—for example
InsufficientPermFee/MissingPermFee-style variants) and apply the same narrowed
pattern to the other identical assertion so only perm-fee-related shadow
failures satisfy the test.

In `@crates/domain/src/models/execution_payload_cache.rs`:
- Around line 330-359: Wrap both the network request and the receiver await
under the same tokio::time::timeout so the configured wait_timeout applies to
the full request_payload_from_the_network(...) + receiver wait path (start the
Instant before calling request_payload_from_the_network and use
timeout(self.wait_timeout, async { request...; receiver.await })). On the
timeout branch clear the in-flight marker in
payloads_currently_requested_from_the_network for the given evm_block_hash and
also remove the orphaned sender from payload_senders (call pop(evm_block_hash))
so is_waiting_for_payload() becomes false; preserve returning
ExecutionPayloadWaitError::WaitTimeout with elapsed_ms.
- Around line 356-359: The numeric narrowing of started.elapsed().as_millis()
(u128) to u64 should use checked conversion instead of an as cast; replace
occurrences like the elapsed_ms: started.elapsed().as_millis() as u64 in the
ExecutionPayloadWaitError::WaitTimeout construction with a checked conversion
using u64::try_from(started.elapsed().as_millis()) and handle the Result (for
example with unwrap_or(u64::MAX) or propagating an error) and apply the same
change to all similar instances in this file.

In `@crates/p2p/src/block_status_provider.rs`:
- Around line 504-526: The test block_status_returns_in_tree_pending_validation
is missing the ChainState::NotOnchain(BlockState::ValidationScheduled) case —
add a new #[case(ChainState::NotOnchain(BlockState::ValidationScheduled))] to
the rstest parameterization so the test covers all four ChainState variants that
should map to InTreePendingValidation (refer to the function
block_status_returns_in_tree_pending_validation and the BlockState/ChainState
enums when adding the case).

In `@crates/types/src/config/node.rs`:
- Around line 731-740: The tests need to assert the new serialized default for
the execution payload timeout so refactors don't silently change behavior:
update the config serialization/legacy/testing tests that exercise defaults
(where other defaults are asserted) to include an assertion for the public field
execution_payload_wait_timeout_millis and/or its provider
default_execution_payload_wait_timeout_millis (expected value 60_000 or test
override 5_000 depending on the test case), and add equivalent checks in the
legacy/roundtrip config tests so both wire and runtime defaults are pinned.

---

Duplicate comments:
In `@crates/actors/src/shadow_tx_generator.rs`:
- Around line 267-271: The error returned when a transaction is both in the
publish ledger and has a perm_fee refund should be classified as structural, not
a snapshot invariant: update the return in shadow_tx_generator (where it
currently returns ShadowTxGenError::SnapshotInvariant(...)) to return
ShadowTxGenError::Structural(...) with the same descriptive message so malformed
peer block shape is routed as consensus/structural rejection rather than an
internal snapshot invariant failure.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/actors/src/cache_service.rs`:
- Around line 545-573: spawn_prune_block_hash_task currently spawns an unbounded
OS thread per prune request, leading to resource pressure; change it to enqueue
work on a serialized/bounded worker (reuse the existing
PruneTxidsFromCachedDataRoots queue/worker pattern) instead of calling
std::thread::spawn: create or use the component that processes a
single-threaded/prioritized queue, push a (data_root, block_hash) task into that
queue, and have the worker call the same db.update_eyre closure with
cached_data_root_by_data_root and db_tx.put::<CachedDataRoots> to perform the
prune and logging; ensure the queue is bounded/backpressured and that errors are
logged the same way as the current warn block.

In `@crates/actors/src/validation_service.rs`:
- Around line 424-429: The call to vdf_terminal_finalize_via(...) ignores its
bool return, so when the block-tree channel is closed the Invalid/ParentMissing
terminal verdicts get dropped without executing the existing fallback logic
(record_validation_finished / record_block_validation_error); update the
ParentMissing and other terminal arms (locations using vdf_terminal_finalize_via
in validation_service.rs, including the blocks around lines shown) to check the
returned bool and if false run the same fallback handling (call
record_validation_finished or record_block_validation_error as appropriate), or
refactor vdf_terminal_finalize_via to return/propagate its success and perform
the fallback internally so callers cannot ignore the failure.

---

Duplicate comments:
In `@crates/actors/src/block_tree_service.rs`:
- Around line 1320-1365: The code currently logs errors from
cache.remove_block(&block_hash) but continues to broadcast BlockStateUpdated {
discarded: true } even when the removal failed; change the control flow in the
function containing cache.remove_block so that on Err(err) you fail fast: after
logging the error (tracing::error!(...)), abort processing for this block (e.g.,
return an Err or early-return/continue from the caller) and do not construct or
broadcast the BlockStateUpdated with discarded: true; ensure this affects the
cache_write_succeeded check and the subsequent soft-internal LRU/metric logic so
they only run when remove_block returned Ok.