Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@block/internal/syncing/da_follower.go`:
- Around line 128-156: The loop in daFollower.HandleCatchup currently pops
priority hints before fetching, which can permanently drop a hint on transient
fetchAndPipeHeight errors and causes double-fetch when priorityHeight ==
daHeight; change the logic to peek the hint instead of removing it (use the
inverse of popPriorityHeight or re-push it on error), attempt fetchAndPipeHeight
while only removing the hint after a successful fetch, on transient failures
(non-datypes.ErrHeightFromFuture) requeue the same priorityHeight (e.g.
f.pushPriorityHeight(priorityHeight)) and return the error so it will be retried
after backoff, and if priorityHeight == daHeight short-circuit the normal
sequential fetch path so you don't fetch the same height twice. Ensure you still
silently ignore datypes.ErrHeightFromFuture as before.

In `@block/internal/syncing/da_retriever.go`:
- Around line 291-298: The code flips r.strictMode as soon as isValidEnvelope is
true, before running full header checks (ValidateBasic and proposer validation),
so a signed-but-invalid envelope can incorrectly toggle strict mode; change the
logic in the DA retriever so r.strictMode is only set to true after the envelope
passes full header validation (call sites around the current block that
reference isValidEnvelope, header, ValidateBasic, and proposer validation),
mirroring the same guard for the other occurrence (lines ~305-313), and add a
regression test (e.g., "invalid envelope does not toggle strict mode") that
constructs a signed envelope which fails ValidateBasic/proposer checks and
asserts r.strictMode remains false after processing.

In `@block/internal/syncing/syncer_benchmark_test.go`:
- Line 80: The test is comparing the pointer fixt.s.daRetrieverHeight (an
*atomic.Uint64) to a uint64, which compares addresses rather than the stored
value; update the assertion in the test to read the atomic's value via
fixt.s.daRetrieverHeight.Load() and compare that to spec.heights+daHeightOffset
so the actual uint64 contents are asserted (i.e., replace the direct pointer
comparison in the assert.Equal call with a .Load() call).

In `@block/internal/syncing/syncer_test.go`:
- Around line 388-394: The test initializes
syncerInst1.daFollower/syncerInst2.daFollower but then starts and asserts
against local variables follower1/follower2 instead of the syncer's follower;
update the test to assign the created followers back to the syncer instances
(e.g., syncerInst1.daFollower = follower1 and syncerInst2.daFollower =
follower2) and check the Start() errors from both follower1.Start(ctx) and
follower2.Start(ctx) with require.NoError/require.Nil so both starts must
succeed; apply the same fix in the other affected blocks that create follower
instances (the sections around the other noted ranges) so the test uses the
syncer-owned lifecycle throughout.

In `@pkg/sequencers/based/sequencer.go`:
- Line 100: The forced-inclusion retriever is constructed with
block.NewForcedInclusionRetriever but never started; after creating
bs.fiRetriever call its Start(ctx) using a cancellable context (create ctx,
cancel := context.WithCancel(parentCtx) and store or wire cancel for shutdown)
so the retriever's background subscription/prefetch goroutines run; mirror the
pattern used in SingleSequencer by replacing context.Background() with the
cancellable context and invoking bs.fiRetriever.Start(ctx) immediately after
construction.

---

Outside diff comments:
In `@block/internal/syncing/da_follower.go`:
- Around line 53-72: The constructor currently always passes both cfg.Namespace
and dataNs (derived from cfg.DataNamespace) to da.NewSubscriber which can
duplicate the same namespace; modify the da_follower constructor (around
daFollower and f.subscriber creation) to build the Namespaces slice
conditionally: include cfg.Namespace, then only append dataNs if it is non-empty
and not equal to cfg.Namespace (or dedupe by value), and then pass that deduped
[][]byte to da.NewSubscriber via da.SubscriberConfig so the subscriber never
gets the same namespace twice.

---

Nitpick comments:
In `@block/internal/da/async_block_retriever.go`:
- Around line 174-182: The HandleEvent method on asyncBlockRetriever currently
returns an error when isInline is true to force the Subscriber to roll back and
let the catch-up loop handle inline events; add a brief clarifying comment above
the isInline check (near asyncBlockRetriever.HandleEvent and its call to
cacheBlock) explaining that returning an error is intentional to delegate inline
event processing to the catch-up state machine (Subscriber/catch-up loop) rather
than treating it as a real failure, so future readers understand the
control-flow hack.

In `@block/internal/da/tracing_test.go`:
- Around line 25-33: Add a unit test that verifies WithTracingClient.Subscribe
forwards the includeTimestamp flag correctly by exercising
mockFullClient.Subscribe via its subscribeFn: implement a mock subscribeFn that
captures the received ts parameter and returns a channel with a dummy
datypes.SubscriptionEvent, call WithTracingClient.Subscribe (or the wrapper
under test) with ts=true and ts=false, and assert the captured value equals the
input for both cases; ensure you use mockFullClient.Subscribe to forward to
subscribeFn so the test fails if Subscribe flips or drops includeTimestamp.

In `@block/internal/syncing/syncer_forced_inclusion_test.go`:
- Around line 77-82: The test never exercises the started ForcedInclusion
retriever subscription path; update the test to call
fiRetriever.Start(t.Context()) (after creating fiRetriever and before t.Cleanup)
and drive the subscription by sending a datypes.SubscriptionEvent into subCh to
verify the asynchronous handler path runs, keeping the existing
t.Cleanup(fiRetriever.Stop). Ensure the mock client expectation for Subscribe
(client.On("Subscribe", ...)) remains and that the event you send matches the
handler's expected format so the retriever processes the subscription case
rather than only the synchronous fallback.

In `@block/public.go`:
- Around line 81-82: The exported constructor NewForcedInclusionRetriever
currently takes a ctx but the retriever only honors cancellation when Start(ctx)
is called; update the public API so callers aren't misled by construction-time
cancellation — either remove the ctx parameter from NewForcedInclusionRetriever
and update all call sites to pass ctx into Start(ctx) instead, or add/expand the
exported doc comments on NewForcedInclusionRetriever and the relevant exported
retriever type to explicitly state this is a two-step lifecycle (construction
does not start background work or observe ctx; callers must call Start(ctx) to
begin and to enable cancellation). Reference NewForcedInclusionRetriever and
Start(ctx) in the doc comment so the intended usage is clear.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@block/internal/da/async_block_retriever.go`:
- Around line 224-245: The default branch in the switch inside
async_block_retriever.go's Retrieve (or the enclosing method handling DA fetch
results) currently only logs and returns nil, which lets Subscriber advance
localDAHeight on failures; change the default case to return a non-nil error so
the caller will back off and retry the same height. Specifically, in the switch
over result.Code (cases datypes.StatusHeightFromFuture, StatusNotFound,
StatusSuccess) update the default case to construct and return an error that
includes the height and result.Message (or wrap an existing sentinel like
datypes.ErrFetchFailed) instead of returning nil; leave the existing cacheBlock
calls unchanged for StatusNotFound/StatusSuccess and keep returning
datypes.ErrHeightFromFuture for StatusHeightFromFuture.

In `@block/internal/da/subscriber.go`:
- Around line 195-204: The code currently sets s.headReached true whenever an
inline event is successfully handled, which can be incorrect if a higher DA
height has already been observed; modify the success branch in
subscriber.HandleEvent (the block around updateHighest, localDAHeight, isInline,
err handling) to only set s.headReached.Store(true) when the processed ev.Height
is >= the latest highestSeenDAHeight (read via s.highestSeenDAHeight.Load()),
i.e. after err==nil && isInline do a compare like if ev.Height >=
s.highestSeenDAHeight.Load() { s.headReached.Store(true) } else { leave it false
}, and apply the same conditional change to the analogous success handling in
the other block around lines 281-290 so HasReachedHead() cannot become true
while highestSeenDAHeight is still ahead.

In `@block/internal/syncing/syncer.go`:
- Around line 184-185: Syncer starts the forced-inclusion retriever via
s.fiRetriever.Start(ctx) but never stops it; update Syncer.Stop to call
s.fiRetriever.Stop() (or the retriever's appropriate shutdown method) and ensure
it handles nil checks and any blocking waits/errors (e.g., call
s.fiRetriever.Stop(), then wait for completion if Stop returns a channel/error)
so the retriever started in NewForcedInclusionRetriever / fiRetriever.Start is
symmetrically shut down to avoid goroutine/resource leaks.

In `@pkg/sequencers/based/sequencer_test.go`:
- Around line 79-88: The DA mock returns datypes.StatusNotFound for Retrieve
which lets the real forced-inclusion retriever started by NewBasedSequencer spin
through empty heights; change the default mocked Retrieve return in the test
helper (where mockDAClient.MockClient.On("Subscribe", ...) and .On("Retrieve",
... ) are set) to return datypes.StatusHeightFromFuture instead of
datypes.StatusNotFound so the background retriever does not walk forward
forever—only override to StatusNotFound in tests that explicitly need that
behavior.

---

Duplicate comments:
In `@block/internal/syncing/da_follower.go`:
- Around line 125-150: The loop that drains P2P priority hints can fetch the
same DA height twice because a successful hint fetch (via fetchAndPipeHeight) is
followed by the unconditional normal fetch of daHeight; to fix, track whether
the loop successfully handled the current daHeight and skip the subsequent
normal fetch when it did. Concretely: in the draining loop around
popPriorityHeight and fetchAndPipeHeight set a boolean (e.g., priorityHandled)
when fetchAndPipeHeight returns nil for a priorityHeight equal to daHeight
(preserving the existing ErrHeightFromFuture handling and continue/return
behavior), then after the loop only call fetchAndPipeHeight(ctx, daHeight) if
priorityHandled is false; reference the functions/variables popPriorityHeight,
fetchAndPipeHeight, daHeight and the ErrHeightFromFuture check when making this
change.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/evm/go.mod`:
- Around line 5-8: The go.mod in apps/evm currently contains local replace
directives (the replace block referencing github.com/evstack/ev-node and
github.com/evstack/ev-node/execution/evm) which must be removed before rc.5;
delete that entire replace(...) section and update the module's require
statements to reference the published versions (follow release order: core →
root ev-node → execution/evm → apps/evm) so apps/evm's require entries point to
the released semantic versions instead of local paths; ensure there are no other
replace directives remaining in apps/evm/go.mod and run go mod tidy to verify
module graph consistency.

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@pkg/sequencers/based/sequencer_test.go`:
- Around line 79-82: The test's Retrieve mock on mockDAClient.MockClient returns
StatusNotFound which lets NewBasedSequencer's real forced-inclusion retriever
walk empty heights; change the mocked Retrieve return to use
datypes.StatusHeightFromFuture instead (i.e., return
datypes.ResultRetrieve{BaseResult: datypes.BaseResult{Code:
datypes.StatusHeightFromFuture}}) so the startup retriever is neutral; apply the
same change to the other similar mocks (including the mock for mockDAClient2
noted around lines 505-509) and keep the existing Subscribe mock unchanged.

---

Nitpick comments:
In `@pkg/sequencers/based/sequencer_test.go`:
- Line 79: The test stubs for mockDAClient.MockClient.On("Subscribe", ...)
return bare channels that never close, but FullDAClient.Subscribe promises the
returned stream closes when the provided context is cancelled; update each
Subscribe stub to create a channel and spawn a goroutine that waits on the
provided context (the mock call's first arg) and closes the channel on ctx.Done
so the test exercises the same teardown path as production (e.g., change the
Subscribe mock in sequencer_test.go to capture the ctx argument, create ch :=
make(chan datypes.SubscriptionEvent), start go func(){ <-ctx.Done(); close(ch)
}() and return ch, nil).

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@block/internal/syncing/syncer_forced_inclusion_test.go`:
- Around line 153-168: Test setup duplicates forced-inclusion wiring: extract a
small helper that creates the subscription mock, constructs
da.NewForcedInclusionRetriever and injects it into a NewSyncer instance to
reduce duplication. Create e.g. newForcedInclusionSyncer(t, client, cfg, gen,
st, mockExec, cm) that calls client.On("Subscribe", ...).Return(subCh,
nil).Maybe(), creates fiRetriever via da.NewForcedInclusionRetriever(...),
constructs the Syncer with NewSyncer(...), stops any existing s.fiRetriever,
assigns s.fiRetriever = fiRetriever, and returns the configured *Syncer and
subCh for use in tests; replace the duplicated block in the test with this
helper.

In `@pkg/sequencers/based/sequencer_test.go`:
- Around line 501-509: Add a missing Retrieve mock on mockDAClient2 to mirror
mockDAClient: on mockDAClient2.MockClient set an expectation for "Retrieve"
(with appropriate args e.g., mock.Anything, mock.Anything) and return the same
shape used for the first client (e.g., nil, nil) with .Maybe(), so that
mockDAClient2's Retrieve is stubbed before NewBasedSequencer or
replaceWithMockRetriever runs; target the MockFullDAClient instance
mockDAClient2 and the NewBasedSequencer test setup.
- Around line 79-82: The mock for Retrieve in createTestSequencer currently
returns datypes.StatusNotFound which is inconsistent with other tests and
creates a race before replaceWithMockRetriever swaps in the mock; update the
mock setup in the createTestSequencer helper (the
mockDAClient.MockClient.On("Retrieve", ...) call used during NewBasedSequencer
startup) to return datypes.ResultRetrieve with BaseResult.Code set to
datypes.StatusHeightFromFuture so it matches other tests and eliminates the race
window before replaceWithMockRetriever runs.