feat(derivation): SPEC-005 L2 state tiering and rollback#948
Open
feat(derivation): SPEC-005 L2 state tiering and rollback#948
Conversation
Replace the challenge mechanism with a batch-data-as-source-of-truth model: - When local L2 blocks don't match L1 batch data, rollback and re-derive from L1 instead of issuing a state challenge - Add L1 reorg detection for non-finalized confirmation modes (latest/safe) by tracking L1 block hashes and comparing on each derivation loop - L1 reorg only triggers DB cleanup and re-derivation; L2 rollback is only triggered when batch data comparison actually fails Key changes: - Remove validator/challenge dependency from derivation - Add verifyBlockContext() and verifyBatchRoots() for batch data comparison - Add detectReorg() with configurable check depth (default 64 blocks) - Add rollbackLocalChain() stub (TODO: geth SetHead API integration) - Add L1 block hash tracking in DB for reorg detection - Add metrics: l1_reorg_detected_total, l2_rollback_total, block_mismatch_total - Add --derivation.reorgCheckDepth CLI flag Made-with: Cursor
Bug fixes: - Fix detectReorg traversal direction: iterate oldest-to-newest to find the earliest divergence point, not the latest - Make rollbackLocalChain return error instead of nil to prevent silent fall-through to NewSafeL2Block on an already-existing block number - Handle edge case in handleL1Reorg when reorgAtL1Height <= startHeight Optimizations: - Skip recordL1Blocks in finalized mode (reorg detection is disabled, recording L1 hashes is unnecessary overhead) Cleanup: - Remove unused BatchIndex/L2EndBlock fields from DerivationL1Block - Add batch-internal tx count consistency check in verifyBlockContext - Use Info instead of Error for L1 reorg detection logs (expected in latest mode) - Update DERIVATION_REFACTOR.md with review feedback changes Made-with: Cursor
- Handle startHeight==0 edge case in handleL1Reorg by writing 0 instead of skipping the reset - Change recordL1Blocks to return on first failure instead of continue, preventing gaps in L1 block hash tracking that could cause missed reorgs - Return immediately after L1 reorg handling instead of continuing the same derivation loop, avoiding recording unstable L1 hashes during ongoing reorgs - Clarify verifyBlockContext tx count check is batch-internal consistency, not local-vs-L1 comparison (local-vs-L1 covered by state root in verifyBatchRoots) Made-with: Cursor
…cording fails recordL1Blocks now returns error. If any L1 header fetch fails mid-range, derivationBlock returns early without calling WriteLatestDerivationL1Height. This prevents permanent gaps in L1 block hash tracking that would make reorgs in the gap range undetectable. Made-with: Cursor
…reorg check, fix baseFee nil handling
1. Add `halted` flag: when rollback stub fails on batch mismatch, derivation
stops instead of infinitely retrying the same batch with wasted L1 RPCs.
2. Optimize detectReorg: check newest saved block first — if it matches,
skip the full scan (1 RPC instead of 64 in the common no-reorg case).
3. Fix verifyBlockContext BaseFee: explicitly error when one side is nil
and the other is not, instead of silently skipping the comparison.
4. Fix doc: DerivationL1Block field list now matches code ({Number, Hash}).
Made-with: Cursor
Expose morphnode_derivation_halted gauge (0/1) so operators can set up alerts when derivation halts due to unrecoverable batch mismatch. All three code paths that set d.halted=true now also call metrics.SetHalted(). Made-with: Cursor
…ix doc env var 1. Add nil check for lastHeader after derive() returns — if blockContexts is empty, skip the batch instead of panicking on lastHeader.Number. 2. Fix DERIVATION_REFACTOR.md: env var is MORPH_NODE_DERIVATION_REORG_CHECK_DEPTH (was missing NODE_ prefix). Made-with: Cursor
…ment halted metric Made-with: Cursor
Made-with: Cursor
Extract newly added functions into dedicated files for clarity: - verify.go: rollbackLocalChain, verifyBatchRoots, verifyBlockContext - reorg.go: detectReorg, handleL1Reorg, recordL1Blocks Existing batch parsing code stays in derivation.go to keep the diff scoped to this PR's changes only. No logic changes — pure file split. Made-with: Cursor
…org-detection Resolve derivation conflicts in favor of main's cleaner state: - Drop the L2Next/nextClient upgrade-switch plumbing (already removed on main): revert RetryableClient, executor, derivation/config, and flags to main; remove switchTime/useZktrie skip path in verifyBatchRoots. - Keep this branch's batch verification (verifyBatchRoots / verifyBlockContext), L1 reorg detection + handler, halted gauge, and rollback-then-rederive flow on root mismatch. - Reintegrate main's validator.ChallengeState path on root mismatch before attempting rollback; restore validator wiring in NewDerivationClient and node main.
- Delete node/validator package (config.go, validator.go, validator_test.go) - Drop validator wiring from node/cmd/node/main.go and derivation.NewDerivationClient signature - Remove validator.challengeEnable / validator.privateKey CLI flags - Drop ChallengeEnable/ChallengeState invocation in derivation rollback path - Remove MORPH_NODE_VALIDATOR_PRIVATE_KEY env from docker compose files Refs: morph-l2/morph-specs SPEC-005 §3.2.1 Co-authored-by: Cursor <cursoragent@cursor.com>
…nnel scaffolding
Introduces the data model and persistence schema for the SPEC-005 state
hierarchy (unsafe / safe / finalized / halted) without yet rewiring the
main derivation loop. The main loop continues to consume the existing
single confirmations cursor; switching to dual-channel drive is gated on
the SPEC-005 §8 blocking decisions.
What this commit adds:
- node/derivation/state.go:
L2HeadStage enum + HeadAnchor type per SPEC-005 §3.1.
- node/db/{keys,store}.go + node/derivation/{database,head_anchor}.go:
Persistent safe_head / finalized_head anchors (RLP DerivationHeadAnchor)
plus typed read/write helpers on Derivation. finalized_head is
documented as monotonic per SPEC-005 §3.1; atomicity caveats spelled
out in head_anchor.go for the future P3 rollback executor.
- node/derivation/dual_channel.go:
L1 safe / finalized tag fetchers per SPEC-005 §3.2. Not yet called
from the main loop on purpose — see file-level comment.
- node/derivation/verify.go:
Doc-only change: documents the SPEC-005 §3.4 invariant that
verifyBatchRoots is always executed and never depends on blob data
(both roots come from L1 calldata at parse time).
- node/derivation/metrics.go:
New gauges/counters for safe_head_l2_number, finalized_head_l2_number,
path_b_triggered_total, batch_root_mismatch_total.
No existing call sites are modified, so runtime behaviour is unchanged.
This commit is intended to be reviewable independently from P3 and the
go-ethereum SetHead-by-hash dependency.
Refs: morph-l2/morph-specs SPEC-005 §3.1 / §3.2 / §3.3 / §3.4 / §3.5
Co-authored-by: Cursor <cursoragent@cursor.com>
…C / mutex skeletons
Adds non-runtime skeletons for the four pieces of SPEC-005 work that are
gated on pending blocking decisions (tech-design §8). All four files are
self-contained and compile, but none are wired into the main derivation
loop yet — switching them on requires the corresponding §8 decisions to
land. Each TODO is annotated with the specific blocking item.
What this commit adds:
- node/derivation/verify_path_b.go (SPEC-005 §3.3 path B):
Eligibility check (last_block ≤ safe_head + locally present) and
trigger condition matching tech-design §3.2.2. Stub returns
errPathBUnavailable; the actual blob-rebuild encoder is left as a
TODO pending confirmation that we should reuse tx-submitter helpers
rather than duplicate them (open question §8 #3).
- node/derivation/verify.go::rollbackLocalChain (SPEC-005 §5.2):
Replaces the previous stub message with the formal 8-step atomic
ordering, plus a checkRollbackBoundary helper that enforces the
finalized_head boundary (SPEC-005 §3.6). The actual SetHead call is
still TODO and depends on §8 #4 (go-ethereum hash-matched SetHead).
- node/derivation/sequencer_mutex.go (SPEC-005 §3.6 / §4):
SequencerMutex primitive (RWMutex-based for now) with public
Acquire/Release Production / Rollback methods. Granularity (global
stop-the-world vs interval lock) intentionally hidden behind the
method API so §8 #5 can switch implementation without churning
callers.
- node/derivation/admin_rpc.go (SPEC-005 §5.1):
AdminAPI.SetL2Head(number, hash) skeleton. Hash-matched per
tech-design §3.3 and rejects targets below finalized_head before
delegating to the rollback executor. Authentication wiring blocked
on §8 #2.
No call sites are modified; runtime behaviour is unchanged. P3 unblocks
parallel work on §8 decisions: while operators decide on auth /
mutex granularity / SetHead semantics, downstream developers can build
tests against these signatures.
Refs: morph-l2/morph-specs SPEC-005 §3.3 / §3.6 / §4 / §5 / §8
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR performs a comprehensive refactor of the L2 derivation subsystem, removing the validator challenge mechanism and introducing L1 reorg detection with persistent head state tracking. The validator package is entirely removed, and batch verification now relies on L1 batch data as the source of truth, supported by block context verification and rollback recovery mechanisms. ChangesDerivation Refactor & Validator Removal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…pecs SPEC-005 The design rationale lived inline at node/derivation/DERIVATION_REFACTOR.md while we were iterating, but the authoritative spec now lives in morph-l2/morph-specs SPEC-005 (state tiering and rollback). Keeping a parallel copy here only invites drift. The corresponding SPEC-005 sections cover the same material: - §3.1 / §3.2 L2 state semantics + L1 dual-channel - §3.3 Path A / path B verification - §3.4 / §3.5 Root-independence + persistence - §3.6 Rollback boundaries - §5 Atomic rollback ordering Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
node/derivation/config.go (1)
117-119: ⚡ Quick winAdd validation for ReorgCheckDepth.
Unlike other numeric configuration fields (StartHeight, PollInterval, FetchBlockRange), ReorgCheckDepth is not validated for zero. While the default value (64) is reasonable, a user could explicitly set it to 0, which would make reorg detection ineffective.
🛡️ Suggested validation
if ctx.GlobalIsSet(flags.DerivationReorgCheckDepth.Name) { c.ReorgCheckDepth = ctx.GlobalUint64(flags.DerivationReorgCheckDepth.Name) + if c.ReorgCheckDepth == 0 { + return errors.New("invalid ReorgCheckDepth: must be > 0") + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@node/derivation/config.go` around lines 117 - 119, When reading ReorgCheckDepth into c.ReorgCheckDepth (using ctx.GlobalUint64(flags.DerivationReorgCheckDepth.Name)), add the same non-zero validation used for StartHeight/PollInterval/FetchBlockRange: if the user-set value is 0, return/raise a configuration error (or log and exit) rather than accepting zero. Update the branch that checks ctx.GlobalIsSet(flags.DerivationReorgCheckDepth.Name) to validate the uint64 result and handle invalid zero values consistently with the other numeric config fields.
🤖 Prompt for all review comments with AI agents
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 `@node/db/store.go`:
- Around line 256-271: DeleteDerivationL1BlocksFrom currently uses Has in a
contiguous-height loop which swallows DB errors and stops at the first missing
height, leaving stale records; instead, use a prefix iterator (s.db.NewIterator
with the derivation-L1-block prefix or an explicit upper bound from
ReadLatestDerivationL1Height) to walk all DerivationL1BlockKey entries starting
at the encoded height and call batch.Delete for each key, and treat any
iterator/Has error as fatal by panicking (consistent with the codebase) before
writing the batch; update references to DerivationL1BlockKey, s.db.NewIterator,
and ReadLatestDerivationL1Height accordingly.
In `@node/derivation/DERIVATION_REFACTOR.md`:
- Around line 120-132: The "Modified Files" table in DERIVATION_REFACTOR.md
currently reads as exhaustive but is partial; update the table to avoid
misleading readers by either (a) renaming its heading to indicate it's
non-exhaustive (e.g., "Selected modified files" or adding a clear "(not
exhaustive)" note) or (b) expanding it to include every touched file referenced
in the diff (ensure entries like node/derivation/derivation.go,
node/derivation/database.go, node/derivation/config.go,
node/derivation/metrics.go, node/db/keys.go, node/db/store.go,
node/flags/flags.go, and node/cmd/node/main.go are present); make the change in
DERIVATION_REFACTOR.md so readers auditing changes (e.g., looking for
DerivationReorgCheckDepth, DerivationL1Block struct, DerivationL1BlockKey, or
NewDerivationClient usage) won't be misled.
In `@node/derivation/derivation.go`:
- Around line 286-293: The rollback target underflows when computing
rollbackTarget := batchInfo.firstBlockNumber - 1 (in function that calls
rollbackLocalChain), so add an explicit guard to reject targets below genesis:
either (A) before computing rollbackTarget, check if batchInfo.firstBlockNumber
== 0 and handle by logging the error, setting d.halted = true,
metrics.SetHalted(), and returning; or (B) enhance checkRollbackBoundary (in
node/derivation/verify.go) to also reject targetBlockNumber == 0 or any value
below the genesis block (and return an error) so rollbackLocalChain is never
called with an underflowed uint64. Ensure you reference the symbols
batchInfo.firstBlockNumber, rollbackLocalChain, checkRollbackBoundary, d.halted
and metrics.SetHalted() when implementing the fix and include an explanatory log
message and error return.
In `@node/derivation/dual_channel.go`:
- Around line 25-49: Add a localized golangci-lint suppression to silence the
unused-lint for the currently-unwired helpers: annotate the function
declarations for fetchLatestSafeHeight, fetchLatestFinalizedHeight, and
fetchTaggedHeight with a trailing comment //nolint:unused so the file-level
scaffolding remains compiled while retaining the functions and their
documentation for future wiring.
---
Nitpick comments:
In `@node/derivation/config.go`:
- Around line 117-119: When reading ReorgCheckDepth into c.ReorgCheckDepth
(using ctx.GlobalUint64(flags.DerivationReorgCheckDepth.Name)), add the same
non-zero validation used for StartHeight/PollInterval/FetchBlockRange: if the
user-set value is 0, return/raise a configuration error (or log and exit) rather
than accepting zero. Update the branch that checks
ctx.GlobalIsSet(flags.DerivationReorgCheckDepth.Name) to validate the uint64
result and handle invalid zero values consistently with the other numeric config
fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 761a527c-1db8-468e-b51b-9ac5385bf985
📒 Files selected for processing (22)
node/cmd/node/main.gonode/db/keys.gonode/db/store.gonode/derivation/DERIVATION_REFACTOR.mdnode/derivation/admin_rpc.gonode/derivation/config.gonode/derivation/database.gonode/derivation/derivation.gonode/derivation/dual_channel.gonode/derivation/head_anchor.gonode/derivation/metrics.gonode/derivation/reorg.gonode/derivation/sequencer_mutex.gonode/derivation/state.gonode/derivation/verify.gonode/derivation/verify_path_b.gonode/flags/flags.gonode/ops-morph/docker-compose-validator.ymlnode/validator/config.gonode/validator/validator.gonode/validator/validator_test.goops/docker/docker-compose-4nodes.yml
💤 Files with no reviewable changes (5)
- node/validator/validator.go
- ops/docker/docker-compose-4nodes.yml
- node/ops-morph/docker-compose-validator.yml
- node/validator/validator_test.go
- node/validator/config.go
Comment on lines
+256
to
+271
| func (s *Store) DeleteDerivationL1BlocksFrom(height uint64) { | ||
| batch := s.db.NewBatch() | ||
| for h := height; ; h++ { | ||
| key := DerivationL1BlockKey(h) | ||
| has, err := s.db.Has(key) | ||
| if err != nil || !has { | ||
| break | ||
| } | ||
| if err := batch.Delete(key); err != nil { | ||
| panic(fmt.Sprintf("failed to delete DerivationL1Block at %d, err: %v", h, err)) | ||
| } | ||
| } | ||
| if err := batch.Write(); err != nil { | ||
| panic(fmt.Sprintf("failed to write batch delete for DerivationL1Blocks, err: %v", err)) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Iterate L1 blocks via prefix scan and panic on Has errors.
Two related concerns in DeleteDerivationL1BlocksFrom:
if err != nil || !has { break }swallows DB errors fromHasand treats them as "not found", silently terminating deletion. This breaks consistency with the rest ofStore(and the established codebase pattern), which panics on non-NotFounderrors.- The for-loop assumes records are perfectly contiguous from
heightupward. A previous interruptedrecordL1Blocks(e.g. mid-range header fetch failure) can leave a hole; deletion will stop at the first gap, leaving stale block records at higher heights. When a reorg rewindslatestDerivationL1Heightbelow those stale heights, they never get overwritten and can mislead futuredetectReorgcomparisons if the chain re-advances.
Prefer iterating the derivation-L1-block key prefix with db.NewIterator(derivationL1BlockPrefix, …) (or pass in an explicit upper bound from ReadLatestDerivationL1Height) and treat any Has/iterator error as fatal.
🛡️ Sketch of safer implementation
func (s *Store) DeleteDerivationL1BlocksFrom(height uint64) {
- batch := s.db.NewBatch()
- for h := height; ; h++ {
- key := DerivationL1BlockKey(h)
- has, err := s.db.Has(key)
- if err != nil || !has {
- break
- }
- if err := batch.Delete(key); err != nil {
- panic(fmt.Sprintf("failed to delete DerivationL1Block at %d, err: %v", h, err))
- }
- }
- if err := batch.Write(); err != nil {
- panic(fmt.Sprintf("failed to write batch delete for DerivationL1Blocks, err: %v", err))
- }
+ batch := s.db.NewBatch()
+ it := s.db.NewIterator(derivationL1BlockPrefix, encodeBlockNumber(height))
+ defer it.Release()
+ for it.Next() {
+ if err := batch.Delete(it.Key()); err != nil {
+ panic(fmt.Sprintf("failed to delete DerivationL1Block, err: %v", err))
+ }
+ }
+ if err := it.Error(); err != nil {
+ panic(fmt.Sprintf("iterator error deleting DerivationL1Blocks, err: %v", err))
+ }
+ if err := batch.Write(); err != nil {
+ panic(fmt.Sprintf("failed to write batch delete for DerivationL1Blocks, err: %v", err))
+ }
}(Adjust encodeBlockNumber / iterator helper to whatever exists in node/db/keys.go.)
Based on learnings: "DB write methods … use a panic-on-error pattern instead of returning errors. This panic behavior is intentional and consistent across the codebase".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node/db/store.go` around lines 256 - 271, DeleteDerivationL1BlocksFrom
currently uses Has in a contiguous-height loop which swallows DB errors and
stops at the first missing height, leaving stale records; instead, use a prefix
iterator (s.db.NewIterator with the derivation-L1-block prefix or an explicit
upper bound from ReadLatestDerivationL1Height) to walk all DerivationL1BlockKey
entries starting at the encoded height and call batch.Delete for each key, and
treat any iterator/Has error as fatal by panicking (consistent with the
codebase) before writing the batch; update references to DerivationL1BlockKey,
s.db.NewIterator, and ReadLatestDerivationL1Height accordingly.
Comment on lines
+286
to
293
| rollbackTarget := batchInfo.firstBlockNumber - 1 | ||
| if err := d.rollbackLocalChain(rollbackTarget); err != nil { | ||
| d.logger.Error("rollback failed, halting derivation to prevent infinite retry", | ||
| "target", rollbackTarget, "batchIndex", batchInfo.batchIndex, "error", err) | ||
| d.halted = true | ||
| d.metrics.SetHalted() | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect rollbackLocalChain and checkRollbackBoundary for underflow / boundary handling.
fd -t f 'verify\.go|verify_path_b\.go' node/derivation
ast-grep --pattern $'func (d *Derivation) rollbackLocalChain($_ $_) $_ {
$$$
}'
ast-grep --pattern $'func (d *Derivation) checkRollbackBoundary($_ $_) $_ {
$$$
}'
rg -nP -C3 '\bbaseHeight\b|\bfinalized_head\b|\bfinalizedHead\b' node/derivation/verify.go node/derivation/verify_path_b.go 2>/dev/nullRepository: morph-l2/morph
Length of output: 3729
Add explicit boundary check to reject rollback targets below genesis block.
rollbackTarget := batchInfo.firstBlockNumber - 1 is uint64. If firstBlockNumber is 0 (edge case on a fresh chain or genesis batch), this underflows to ^uint64(0) and silently passes to rollbackLocalChain. The same pattern exists at line 623.
checkRollbackBoundary in node/derivation/verify.go (lines 55–63) checks only target < finalized_head, but the code comment at lines 33–34 explicitly states "target < genesis → halted" must be enforced. Currently, if finalized is nil, the genesis boundary is never checked, and an overflowed target proceeds to rollbackLocalChain. While the system eventually halts (via the "rollback not implemented" error), the boundary violation is not caught explicitly.
Add a check in checkRollbackBoundary to reject targetBlockNumber == 0 or any value below genesis, or add a guard before computing rollbackTarget to prevent the underflow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node/derivation/derivation.go` around lines 286 - 293, The rollback target
underflows when computing rollbackTarget := batchInfo.firstBlockNumber - 1 (in
function that calls rollbackLocalChain), so add an explicit guard to reject
targets below genesis: either (A) before computing rollbackTarget, check if
batchInfo.firstBlockNumber == 0 and handle by logging the error, setting
d.halted = true, metrics.SetHalted(), and returning; or (B) enhance
checkRollbackBoundary (in node/derivation/verify.go) to also reject
targetBlockNumber == 0 or any value below the genesis block (and return an
error) so rollbackLocalChain is never called with an underflowed uint64. Ensure
you reference the symbols batchInfo.firstBlockNumber, rollbackLocalChain,
checkRollbackBoundary, d.halted and metrics.SetHalted() when implementing the
fix and include an explanatory log message and error return.
Comment on lines
+25
to
+49
| // fetchLatestSafeHeight returns the L1 block number of the latest "safe" head. | ||
| // | ||
| // "safe" here is the consensus-layer "safe" tag exposed via L1 RPC, not a | ||
| // confirmations-derived height. Use this to drive safe_head. | ||
| func (d *Derivation) fetchLatestSafeHeight(ctx context.Context) (uint64, error) { | ||
| return d.fetchTaggedHeight(ctx, rpc.SafeBlockNumber, "safe") | ||
| } | ||
|
|
||
| // fetchLatestFinalizedHeight returns the L1 block number of the latest | ||
| // "finalized" head. Use this to drive finalized_head; the result is | ||
| // expected to be monotonic across calls. | ||
| func (d *Derivation) fetchLatestFinalizedHeight(ctx context.Context) (uint64, error) { | ||
| return d.fetchTaggedHeight(ctx, rpc.FinalizedBlockNumber, "finalized") | ||
| } | ||
|
|
||
| func (d *Derivation) fetchTaggedHeight(ctx context.Context, tag rpc.BlockNumber, label string) (uint64, error) { | ||
| header, err := d.l1Client.HeaderByNumber(ctx, big.NewInt(int64(tag))) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("get L1 %s head: %w", label, err) | ||
| } | ||
| if header == nil || header.Number == nil { | ||
| return 0, fmt.Errorf("got nil header for L1 %s head", label) | ||
| } | ||
| return header.Number.Uint64(), nil | ||
| } |
Contributor
There was a problem hiding this comment.
Unblock CI: silence unused lint on the intentionally-unwired helpers.
golangci-lint fails on line 29 because fetchLatestSafeHeight has no callers yet (and fetchLatestFinalizedHeight/fetchTaggedHeight are reachable only through it, so they will likely be flagged once that one is silenced). Since the file-level comment already documents that these are exposed now and wired later, a localised //nolint:unused is the lowest-friction way to keep the scaffolding compiling without burying the intent.
🛠️ Proposed fix
// fetchLatestSafeHeight returns the L1 block number of the latest "safe" head.
//
// "safe" here is the consensus-layer "safe" tag exposed via L1 RPC, not a
// confirmations-derived height. Use this to drive safe_head.
+//
+//nolint:unused // SPEC-005 P2 scaffolding; wired in §8-gated follow-up.
func (d *Derivation) fetchLatestSafeHeight(ctx context.Context) (uint64, error) {
return d.fetchTaggedHeight(ctx, rpc.SafeBlockNumber, "safe")
}
// fetchLatestFinalizedHeight returns the L1 block number of the latest
// "finalized" head. Use this to drive finalized_head; the result is
// expected to be monotonic across calls.
+//
+//nolint:unused // SPEC-005 P2 scaffolding; wired in §8-gated follow-up.
func (d *Derivation) fetchLatestFinalizedHeight(ctx context.Context) (uint64, error) {
return d.fetchTaggedHeight(ctx, rpc.FinalizedBlockNumber, "finalized")
}
+//nolint:unused // SPEC-005 P2 scaffolding; wired in §8-gated follow-up.
func (d *Derivation) fetchTaggedHeight(ctx context.Context, tag rpc.BlockNumber, label string) (uint64, error) {🧰 Tools
🪛 GitHub Actions: Node / 0_check.txt
[error] 29-29: golangci-lint: func (*Derivation).fetchLatestSafeHeight is unused (unused)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@node/derivation/dual_channel.go` around lines 25 - 49, Add a localized
golangci-lint suppression to silence the unused-lint for the currently-unwired
helpers: annotate the function declarations for fetchLatestSafeHeight,
fetchLatestFinalizedHeight, and fetchTaggedHeight with a trailing comment
//nolint:unused so the file-level scaffolding remains compiled while retaining
the functions and their documentation for future wiring.
…ed head anchors Per morph-specs SPEC-005 §6.2 task table: #5 Drive derivation main loop on L1 safe; reorg detection unconditional #6 Persist safe_head anchor after each verified batch (+ metric) #6b finalized_head tracker advances at the tail of every iteration Changes: - node/derivation/config.go Default `Confirmations` flips from FinalizedBlockNumber to SafeBlockNumber, matching SPEC-005 §3.1 step 1. Operators may still override via `--derivation.confirmations`. - node/derivation/derivation.go - Step 1: drop the `confirmations != Finalized` guard around detectReorg — reorg detection now runs every loop on the safe segment, per SPEC-005 §3.2. - After verifyBatchRoots success, write a HeadAnchor (L2 last block + L1 commit block) as the new safe_head and update SafeHeadL2Number metric. - End of loop: invoke advanceFinalizedHead from the new finalized tracker. - node/derivation/finalized_tracker.go (new) advanceFinalizedHead implements SPEC-005 §3.1 step 4. Two paths: cheap path: safe_head.L1Number ≤ L1 finalized → ratchet to safe_head. steady-state: walk commit-batch logs in (currentFinalized, L1 finalized], decode-light the latest one, anchor finalized_head at its L2 last block. Calldata-only — never fetches blob data. Monotonicity is enforced; any regression attempt halts the node. - node/derivation/dual_channel.go Update header comment now that the main loop and finalized tracker are wired into both cursors. Out of scope (per spec §6.2 / §8 and per project decision to leave full rollback for the rollback-executor PR): - L1 anchor window depth tuning (#7 / §8 #1) - Path B implementation (#10) — skeleton stays - Rollback executor (#12) — skeleton stays in verify.go - Sequencer mutex wiring (#11) — skeleton stays - Admin RPC auth (#13 / §8 #2) — skeleton stays
…/finalized head anchors" This reverts commit 308dfcb. SPEC-005 scope was reset (morph-specs#18) to cover only: A. validator cleanup B. derivation Path B (local blob hash reconstruction fallback) The reverted commit changed Confirmations default from Finalized to Safe, added safe_head writes in the main loop, and introduced finalized_tracker.go. All of that is L2 state-tiering work — out of scope for SPEC-005 and deferred to a future independent PR. Skeleton files remain in node/derivation/ as placeholders to be reused by the future state-tiering PR: - state.go, head_anchor.go, dual_channel.go: state tiering primitives - admin_rpc.go, sequencer_mutex.go, verify.go::rollbackLocalChain: rollback executor primitives - verify_path_b.go: scope-A target (Path B implementation pending)
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
1 participant
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.
Summary
Refactors the derivation pipeline along the SPEC-005 "L2 state tiering and rollback" design (morph-l2/morph-specs#19). The branch combines the original batch-verification / L1-reorg-detection feature work with a recent layer of SPEC-005-aligned scaffolding (state model, head anchors, dual-channel fetcher, path B / rollback / admin RPC / mutex skeletons).
The runtime main loop is intentionally not yet rewired to the new state machine — switching it on is gated on the SPEC-005 §8 blocking decisions (anchor window depth, admin RPC auth, sequencer-mutex granularity, go-ethereum hash-matched
SetHead). Each TODO is annotated with the specific blocking item.Scope
verifyBlockContext(timestamp / gasLimit / baseFee / txCount)verifyBatchRoots(stateRoot + withdrawalRoot, blob-independent)unsafe / safe / finalized / halted) andHeadAnchorsafe/finalizedtags)SetL2Head(number, hash)Modified files (incremental on top of
main)Core derivation
node/derivation/derivation.go— main loop refactor: reorg detection, batch verification, rollback flow, halted state.node/derivation/verify.go—verifyBlockContext,verifyBatchRoots(with SPEC-005 §3.4 blob-independence invariant),rollbackLocalChain(8-step atomic ordering doc).node/derivation/reorg.go—detectReorg/handleL1Reorg/recordL1Blocks.SPEC-005 scaffolding (this branch's last 3 commits)
node/derivation/state.go—L2HeadStage/HeadAnchor.node/derivation/head_anchor.go— typed read/write helpers for head anchors.node/derivation/dual_channel.go— L1 safe / finalized tag fetchers.node/derivation/verify_path_b.go— path B eligibility + skeleton.node/derivation/admin_rpc.go—AdminAPI.SetL2Headskeleton (hash-matched, finalized-boundary-checked).node/derivation/sequencer_mutex.go—SequencerMutexprimitive.Persistence / config
node/db/{keys,store}.go—DerivationL1Block+DerivationHeadAnchorkeys / RLP CRUD.node/derivation/database.go— extendedReader/Writerinterfaces.node/derivation/config.go—ReorgCheckDepthconfig.node/derivation/metrics.go— reorg / rollback / halted / head-stage / path-B / batch-root-mismatch metrics.node/flags/flags.go—--derivation.reorgCheckDepth; removed--validator.challengeEnable/--validator.privateKey.node/cmd/node/main.go— drops the validator wiring fromNewDerivationClient.node/validator/(config.go / validator.go / validator_test.go).docker-compose-4nodes.ymlanddocker-compose-validator.yml).Design invariants (per SPEC-005)
verifyBatchRootsruns even when path A or path B is unavailable.finalized_headis fatal and enters halted state.WriteLatestDerivationL1Heightis skipped ifrecordL1Blocksfails mid-range.Follow-ups (gated on SPEC-005 §8 decisions)
SetHeadinterface.derivationBlockto consume the dual-channel cursor and write tosafe_head/finalized_head.Test plan
go build ./node/...(locally; sandbox missing tendermint sibling fails as expected).go test ./node/derivation/...(locally, derivation unit tests).--derivation.confirmations -2) reorg simulation: modify saved L1 hash → next loop detects and cleans up.Made with Cursor
Summary by CodeRabbit
Release Notes
New Features
Configuration Changes
derivation.reorgCheckDepthCLI flag (default: 64)validator.challengeEnableandvalidator.privateKeyflagsRefactor