feat(l1): introduce SettlementLayer and EthMessagingClient abstractions

[brbrr]

This is the first part of #3727 (deprecated) that adds an abstraction layer between the Geth L1 client and Juno, separating the settlement layer and the actual RPC client.

[codecov]

Codecov Report

❌ Patch coverage is 77.66497% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.50%. Comparing base (81d48e3) to head (b66a9c5).

Files with missing lines	Patch %	Lines
node/node.go	9.09%	30 Missing ⚠️
l1/geth_l1_state_provider.go	87.73%	11 Missing and 2 partials ⚠️
node/migration.go	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3738      +/-   ##
==========================================
+ Coverage   75.40%   75.50%   +0.09%     
==========================================
  Files         438      437       -1     
  Lines       39501    39549      +48     
==========================================
+ Hits        29786    29861      +75     
+ Misses       7646     7618      -28     
- Partials     2069     2070       +1     

☔ 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.

[github-actions]

Claude finished @brbrr's task in 5m 10s —— View job

---

PR Review: L1 SettlementLayer abstraction

I reviewed the diff against origin/main (and the branch base, since the branch predates current main). The packages build cleanly, and the refactor in l1.go preserves the existing sync/catch-up logic 1:1 — only the types change (Subscriber→SettlementLayer, contract.StarknetLogStateUpdate→StateUpdate). Nicely documented throughout.

Reviewed:

• l1/geth_settlement.go (new adapter + forwarder)
• l1/settlement.go + l1/l1.go
• node/node.go, node/migration.go, node/metrics.go wiring
• rpc/rpccore + RPC GetMessageStatus receipt consumption
• tests + stale-reference / dead-code sweep

This is a clean, well-structured refactor. Logic is correct; I found no blockers — one test-coverage gap worth addressing and two nits.

Important

• Forwarder is untested — gethStateUpdateForwarder / WatchStateUpdate is the only concurrency-sensitive code in the PR (goroutine lifecycle, closeOnce shutdown, non-blocking Err() delivery, sink back-pressure) and has no direct coverage. The HTTP test server can't exercise WS subscriptions, and the l1_*_test.go suites mock WatchStateUpdate at the interface boundary, so the real forwarder never runs. Codecov's 54 missing lines in this file are largely this block. Details + a suggested 4-case test left inline. The logic itself looks correct on inspection — shutdown is once-guarded, no send-after-close, no double-writer on sink, and Unsubscribe unblocks run via <-w.closed — it just isn't pinned down by a test.

Nits

• rpc/rpccore/rpccore.go:38 — EthMessagingClient alias is added but unused in this PR (it's for the next commit per the description). Fine as a forward-declaration, but it's currently dead code; consider landing it together with its first consumer so it doesn't linger unused.
• node/node.go newGethSettlement dials with a fresh context.Background()+1m timeout, so in node/migration.go:fetchL1HeadIfMissing the migration's ctx cancellation won't cancel the dial. Harmless given the bounded dial timeout, but threading the caller's ctx would be more consistent.

Verified (no action needed)

• Partial gethReceiptToEth copy (only Logs) is safe — GetMessageStatus (rpc/v10/l1.go) reads only Logs[].Topics/Data, all of which are converted.
• No stale references to the removed Subscriber / EthSubscriber / EthReceiptAdapter / Client.L1() remain.
• Concurrent settlement RPC calls (prometheus gauge scrapes vs. the sync loop) are safe — listener is set once before hand-off and go-ethereum's rpc.Client is concurrency-safe.

[github-actions]

Claude finished @brbrr's task in 3m 41s —— View job

---

PR Review: L1 SettlementLayer abstraction — follow-up

• Read PR context & prior review
• Review new l1/geth_forwarder_test.go
• Re-review l1/geth_settlement.go forwarder logic
• Verify the two prior nits were addressed
• Build affected packages

Re-reviewed after the address feedback commit (fcd716d9). All three items from the prior review are resolved, and ./l1/..., ./node/..., ./rpc/rpccore/... build cleanly.

Resolved

• ✅ Forwarder test gap (the one "Important") — l1/geth_forwarder_test.go now drives gethStateUpdateForwarder directly through a fake event.Subscription, covering exactly the four paths flagged: normal forward + decode, inner-error Err() delivery-then-close, Unsubscribe() goroutine exit (asserts inner.Unsubscribe() ran), and sink-stall + Unsubscribe() unblocking via <-w.closed. The fake's closeOnce-equivalent (CompareAndSwap) and the buffered errCh/raw match the production wiring, so the assertions exercise the real logic rather than a re-implementation. Good coverage of the concurrency-sensitive block.
• ✅ Unused EthMessagingClient alias — removed from rpc/rpccore/rpccore.go (deferred to the commit that introduces its first consumer).
• ✅ ctx threading into newGethSettlement — node/migration.go now passes the migration's ctx, and node/node.go layers the 1-minute dial timeout on the caller's context. node.New still passes context.Background(), which is correct — there's no node-lifetime context at construction time, and that's documented.

Nits (non-blocking)

• l1/geth_forwarder_test.go:144 TestGethStateUpdateForwarder_SinkStalledThenUnsubscribe — because raw is buffered (cap 1), after Unsubscribe() closes w.closed the run loop's top-level select may pick the <-w.closed arm before ever dequeuing from raw, so the intended "blocked on w.sink <- su" path isn't guaranteed to execute on every run. The test still passes either way (it asserts only shutdown + unsubscribe), so this is a determinism nit, not a correctness issue — the stall arm is already covered deterministically by inspection. If you want to pin it, have the test wait for the goroutine to reach the send (e.g. an unbuffered raw so the w.raw <- send only completes once run has dequeued) before calling Unsubscribe().

Verdict

No blockers. The previously-flagged test gap is closed and both nits are addressed — this is good to merge from my side (note: I cannot approve PRs). I was unable to run go test ./l1/ -race in this environment (command requires approval), but the forwarder logic and test assertions check out on inspection and the packages compile.
• branch feat/l1-settlement-geth

[rodrodros]

This PR looks more complicated than it should, let's discuss it more on Monday

[github-actions]

Claude finished @brbrr's task in 4m 21s —— View job

---

PR Review: SettlementLayer & EthMessagingClient abstractions

• Read new abstractions (settlement.go, geth_settlement.go)
• Review forwarder concurrency / resource leaks
• Review l1.go rewiring & node wiring
• Review tests for meaningful assertions
• Build affected packages (./l1/... ./node/... ./rpc/... — OK)

I reviewed this against the branch base (ada24e91e, since the branch predates current origin/main). This is a clean, well-structured refactor and I found no correctness, concurrency, or resource-leak blockers. The l1.Client sync/catch-up logic is preserved 1:1 — only types change (Subscriber→SettlementLayer, contract.StarknetLogStateUpdate→StateUpdate).

Verified (no action needed)

• Forwarder concurrency (geth_settlement.go:276-328) — shutdown is closeOnce-guarded; no send-after-close (errCh send is non-blocking then closed exactly once); run unblocks a stalled sink <- su via <-w.closed; Unsubscribe exits the goroutine and forwards to inner. The new geth_forwarder_test.go drives all four paths (normal forward, inner-error delivery+close, unsubscribe exit, sink-stall+unsubscribe) directly against a fake event.Subscription — the previously-flagged test gap is closed.
• SetListener data race — the field is read unlocked from every RPC method, but node.go calls SetListener once before any goroutine starts and the contract is documented (geth_settlement.go:111-122). Single-shot wiring is respected.
• Resource lifecycle — settlement.Close() is deferred in Client.Run/CatchUpL1Head; the migration path (node/migration.go) builds a listener-less settlement and closes it the same way. No leak.
• FinalisedHeight not-found maps ethereum.NotFound→eth.ErrNotFound, distinguishing "no finality yet" from transport failure — covered by test.
• gethReceiptToEth copies only Logs, which is all starknet_getMessageStatus reads (rpc/v10/l1.go) — safe, and documented as such.
• No stale references to the removed Subscriber / EthSubscriber / EthReceiptAdapter / Client.L1() remain.
• Tests: meaningful assertions, no flaky time.Sleep (the two time.Sleep matches are only failure-message strings), race-sensitive forwarder tests included.

Nits (non-blocking)

• gethReceiptToEth is a latent footgun — it silently drops every receipt field except Logs. Correct today, but a future L1Client consumer reading e.g. Status or BlockNumber off the receipt would get a zero value with no compile-time signal. The doc comment mitigates this; consider it a known trade-off.
• PR title scope — the title says "and EthMessagingClient abstractions", but per the description that type is deferred to a later commit and is not present in this diff. Title is slightly ahead of the contents (purely cosmetic).
• SinkStalledThenUnsubscribe determinism (geth_forwarder_test.go:144) — because raw is buffered (cap 1), run may take the <-w.closed arm before dequeuing, so the intended "blocked on send" path isn't guaranteed every run. Test still passes (it only asserts shutdown + unsubscribe); an unbuffered raw would pin the stall path deterministically. Already noted in a prior review.

Note: I could not run go test ./l1/ -race in this environment (command requires approval), but the package builds cleanly and the forwarder logic + assertions check out on inspection.

Re @rodrodros's "more complicated than it should be" — that's a design-altitude question (worth the Monday discussion) rather than a correctness one; from a bug/race/leak standpoint the implementation is sound.
• branch feat/l1-settlement-geth

[github-actions]

Claude finished @brbrr's task in 3m 47s —— View job

---

PR Review: SettlementLayer & EthMessagingClient abstractions

• Read new abstractions (settlement.go, geth_settlement.go)
• Review forwarder concurrency / resource leaks
• Review l1.go rewiring & node wiring
• Review tests
• Build affected packages (./l1/... ./node/... ./rpc/... — OK)

Re-reviewed after the latest Simplify StateUpdate production commit (4de2d8887), against the branch base ada24e91e (the branch predates current origin/main, so there's no merge base with it). No correctness, concurrency, or resource-leak blockers.

The simplification is a real improvement

The hand-rolled gethStateUpdateForwarder (manual closeOnce / closed channel / shutdown(cause) plumbing) is gone, replaced by forwardStateUpdates built on go-ethereum's event.NewSubscription (geth_settlement.go:273-295). This is the right call — event.NewSubscription already owns once-only close, idempotent Unsubscribe, error delivery, and goroutine teardown, so the adapter is now just type translation + the stall-vs-quit select. Net −34 lines and the concurrency-sensitive surface shrank to something the runtime library guarantees. This directly addresses @rodrodros's "more complicated than it should be" note. Lifecycle re-verified:

• defer inner.Unsubscribe() runs on every producer exit (error, quit, or stall+quit) → no upstream subscription leak.
• sink <- ... is guarded by <-quit, so a stalled sink can't wedge the goroutine after Unsubscribe.
• Unsubscribe() blocks until the producer returns → deterministic teardown, no send-after-close.

Verified (no action needed)

• l1.Client logic preserved 1:1 (l1.go) — only types change (Subscriber→SettlementLayer, contract.StarknetLogStateUpdate→StateUpdate); applyStateUpdate, reorg-clear, setL1Head finality gating all unchanged.
• SetListener race contract — read unlocked from RPC methods, but set once in node.go before hand-off and documented (geth_settlement.go:110-121). Single-shot wiring respected.
• Resource lifecycle — settlement.Close() deferred in Run/CatchUpL1Head; migration path builds a listener-less settlement and closes it the same way.
• ctx threading — newGethSettlement layers a 1-min dial timeout on the caller's ctx (node.go:648); migration passes its ctx; node.New correctly uses context.Background() (no node-lifetime ctx at construction).
• gethReceiptToEth copies only Logs, which is all starknet_getMessageStatus reads — safe and documented.
• rpccore.L1Client is now documented and the previously-unused EthMessagingClient alias was removed (deferred to its first consumer).
• Forwarder tests rewired to drive forwardStateUpdates directly through a fake event.Subscription, covering all four paths (normal forward, inner-error delivery+close, unsubscribe exit, sink-stall+unsubscribe).

Nits (non-blocking)

• SinkStalledThenUnsubscribe determinism (geth_forwarder_test.go:143) — raw is buffered (cap 1), so the producer's outer select may take <-quit before dequeuing, meaning the intended "blocked on sink <- ..." path isn't guaranteed each run. Test still passes (it only asserts shutdown + unsubscribe). An unbuffered raw would pin the stall path. Carried over from prior reviews.
• gethReceiptToEth latent footgun — silently drops every receipt field except Logs; a future L1Client consumer reading e.g. Status/BlockNumber would get a zero value with no compile-time signal. Mitigated by the doc comment; known trade-off.
• PR title scope — EthMessagingClient is deferred to a later commit and isn't in this diff; title is slightly ahead of contents (cosmetic).

Note: build is clean, but I couldn't run go test ./l1/ -race in this environment (command requires approval). The forwarder logic and test assertions check out on inspection.
• branch feat/l1-settlement-geth

[Copilot]

Pull request overview

This PR introduces a new L1 abstraction boundary by splitting the “settlement layer” (watch/filter state update events, heights, chain ID, receipts) from the higher-level l1.Client sync loop, and wires the new adapter into node startup/migrations and RPC.

Changes:

• Add l1.SettlementLayer + l1.StateUpdate as chain-neutral types consumed by l1.Client, and implement a go-ethereum-backed adapter (l1.GethSettlement).
• Remove the old geth-specific EthSubscriber and the RPC EthReceiptAdapter, wiring GethSettlement directly into RPC handlers for starknet_getMessageStatus.
• Refactor L1 metrics so the listener is shared across l1.Client and the settlement adapter, and split settlement polling gauges into a separate registrar.

Reviewed changes

Copilot reviewed 18 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
rpc/rpccore/rpccore.go	Adds/clarifies the L1Client interface documentation used by RPC for message status.
rpc/rpccore/eth_receipt_adapter.go	Removes the receipt adapter now that the settlement client returns Juno eth types directly.
rpc/rpccore/eth_receipt_adapter_test.go	Removes adapter tests (adapter deleted).
node/node.go	Constructs GethSettlement, attaches metrics listener/options, passes settlement to both l1.Client and RPC.
node/migration.go	Uses GethSettlement + l1.Client to fetch L1 head during migration without metrics listener.
node/metrics.go	Splits L1 metrics into listener vs settlement polling gauges; hooks into SettlementLayer.
node/metrics_test.go	Updates tests to use MockSettlementLayer and the split metric registration flow.
mocks/mock_subscriber.go	Deletes mock for the removed Subscriber interface.
mocks/mock_settlement_layer.go	Adds mock for the new SettlementLayer interface.
l1/settlement.go	Introduces SettlementLayer and chain-neutral StateUpdate.
l1/l1.go	Refactors l1.Client to depend on SettlementLayer and StateUpdate instead of geth types.
l1/l1_test.go	Updates internal tests to use MockSettlementLayer and StateUpdate.
l1/l1_pkg_test.go	Moves tests to l1_test and uses exported test helpers to avoid import cycles.
l1/export_test.go	Exposes package-private hooks to external tests (SetSettlement, cache accessors, retry loops).
l1/geth/contract/starknet.go	Adds generated contract binding used by GethSettlement.
l1/geth/contract/starknet_filter.go	Adds helper to fully drain filtered logs from abigen’s FilterLogs.
l1/geth_settlement.go	Adds go-ethereum-backed GethSettlement implementing SettlementLayer (+ receipt lookup).
l1/geth_settlement_test.go	Adds unit tests for GethSettlement via a minimal JSON-RPC test server.
l1/geth_forwarder_test.go	Adds tests for forwarding/teardown behavior of the state update forwarder.
l1/eth/subscription.go	Removes the old eth.Subscription interface (now expected to live at the l1 boundary).
l1/eth_subscriber.go	Deletes the old go-ethereum-backed subscriber implementation (replaced by GethSettlement).

Files not reviewed (1)

• mocks/mock_settlement_layer.go: Generated file

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Claude finished @brbrr's task in 3m 48s —— View job

---

PR Review: SettlementLayer abstraction

• Gather context (diff vs origin/main, prior reviews)
• Review new abstractions (settlement.go, subscription.go, geth_settlement.go)
• Review forwarder concurrency / resource leaks
• Review l1.go rewiring + node/migration/metrics wiring
• Review tests
• Build affected packages (./l1/... ./node/... ./rpc/... — OK)

Re-reviewed the current tip (1c464ae0c) against origin/main. This remains a clean refactor with no correctness, concurrency, or resource-leak blockers. The l1.Client sync/catch-up logic is preserved 1:1 — only the types change (Subscriber→SettlementLayer, contract.StarknetLogStateUpdate→StateUpdate).

Verified (no action needed)

• Forwarder lifecycle (geth_settlement.go:249-271) — the hand-rolled closeOnce/shutdown plumbing is gone, replaced by event.NewSubscription. defer inner.Unsubscribe() runs on every producer exit (error / quit / stall+quit) → no upstream leak; the sink <- … send is guarded by <-quit so a stalled sink can't wedge the goroutine; Unsubscribe() blocks until the producer returns → deterministic teardown, no send-after-close.
• Resubscribe path (l1.go:243-267) — because event.NewSubscription.Unsubscribe() blocks until the producer returns, the old forwarder has fully stopped before the new one is created, so there's never a second concurrent writer on the shared updateChan. Safe.
• Subscription interface now lives in l1/subscription.go (addresses @RafaelGranza's "shouldn't live in eth" and Copilot's "undefined identifier" note — both resolved).
• SetListener/listener race — the settlement listener is read unlocked from RPC methods but set once at construction (WithSettlementListener) before hand-off, and documented. Single-shot wiring respected.
• Resource lifecycle — settlement.Close() is deferred in Run/CatchUpL1Head; the migration path builds a listener-less settlement and closes it the same way. No leak. Metrics gauges (registerL1SettlementMetrics) that outlive Close() just return 0 on error — no panic.
• ctx threading — newGethSettlement now layers a 1-min dial timeout on the caller's ctx (node.go:683); migration passes its own ctx; node.New correctly uses context.Background().
• gethReceiptToEth copies only Logs, which is all starknet_getMessageStatus reads (rpc/v10/l1.go) — safe and documented.
• Error wraps now consistently start with a verb-ing form (dialing, binding, getting, filtering, subscribing) — addresses @RafaelGranza / @rodrodros's format note. The unused EthMessagingClient alias and the log-field/"transport warnings" comment flagged in earlier reviews are gone from the current diff.
• Forwarder tests (geth_forwarder_test.go) drive forwardStateUpdates directly through a fake event.Subscription, covering all four paths (normal forward, inner-error delivery+close, unsubscribe exit, sink-stall+unsubscribe).

Nits (non-blocking, all cosmetic — no inline comments posted)

• Constructor doc vs. behavior (geth_settlement.go:51-54) — NewGethSettlement's comment says "ws/wss is required" / "transport is selected by URL scheme", but the constructor doesn't enforce a scheme (only WatchStateUpdate needs ws; unary calls work over HTTP). Enforcement actually lives in node.go:newGethSettlement. Consider softening the constructor comment to "ws/wss required for WatchStateUpdate" so it doesn't over-claim. (Raised by Copilot.)
• Error-wrap wording — node/node.go:639 still wraps as "create L1 client" and node/migration.go:113 as "creating a new L1 client", but the failing op is building the settlement adapter (node.go:690 already uses the clearer "set up L1 settlement client"). Aligning the other two would make startup/migration failures clearer. (Raised by Copilot.)
• SinkStalledThenUnsubscribe determinism (geth_forwarder_test.go:142) — raw is buffered (cap 1), so the producer's outer select may take <-quit before dequeuing; the intended "blocked on sink <- …" path isn't guaranteed each run (test still passes — it only asserts shutdown + unsubscribe). An unbuffered raw would pin the stall path. Carried over from prior reviews.
• gethReceiptToEth latent footgun — silently drops every receipt field except Logs; a future L1Client consumer reading Status/BlockNumber would get a zero value with no compile-time signal. Mitigated by the doc comment; known trade-off.

Re @rodrodros's "more complicated than it should be" — the event.NewSubscription rewrite (net −34 lines vs. the hand-rolled forwarder) directly shrinks the concurrency surface; from a bug/race/leak standpoint the implementation is sound. Whether the abstraction boundary itself is worth it is a design-altitude call, not a correctness one.

Verdict: No blockers. Good to merge from a correctness standpoint (note: I can't approve PRs). I couldn't run go test ./l1/ -race here (requires approval), but the package builds cleanly and the forwarder logic + assertions check out on inspection.
• branch feat/l1-settlement-geth

[rodrodros]

Why are these pointers?

[brbrr]

It was a mechanical change, driven by the old usage, and the fact that L1Head uses pointers. But also it seems consistent with Felt usage across the codebase 🤷. Do you think they should be values?

[rodrodros]

First review, overall looks good and makes sense, will perform additional rounds

[RafaelGranza]

LGTM. Just the one nit on the unused fields.

[github-actions]

Claude finished @brbrr's task in 3m 43s —— View job

---

PR Review: L1StateProvider abstraction

• Review new abstractions (l1_state_provider.go, subscription.go, geth_l1_state_provider.go)
• Review forwarder concurrency / resource leaks
• Review l1.go rewiring + node/migration/metrics wiring
• Review tests
• Build affected packages (go build ./l1/... — FAILS)

Re-reviewed the current tip on feat/l1-settlement-geth. The abstraction and the event.NewSubscription-based forwarder are still sound (all points from prior rounds hold), but the latest state has a compile break — the package no longer builds.

🔴 Blocker

• l1/l1.go does not compile. receiveL1StateUpdates (lines 249–253) still has the pre-rename signature:

  func (c *Client) receiveL1StateUpdates(
      ctx context.Context,
      sub event.Subscription,                          // undefined: event
      updateCh chan *contract.StarknetLogStateUpdate,  // undefined: contract
  ) error {

  Neither event nor contract is imported in this file anymore, and the caller watchL1StateUpdates passes an l1.Subscription and a chan *StateUpdate. Result:

  l1/l1.go:251:6: undefined: event
  l1/l1.go:252:17: undefined: contract
  

  This breaks go build ./l1/... and everything downstream (node, rpc) — CI will be red. It's a leftover from the Settlement → L1StateProvider rename. Fix the signature to the new types:

  func (c *Client) receiveL1StateUpdates(
      ctx context.Context,
      sub Subscription,
      updateCh chan *StateUpdate,
  ) error {

  Fix this →

Nit

• l1/l1.go:267 — the c.applyStateUpdate(stateUpdate) line is indented with spaces instead of a tab, so gofmt -l flags the file and the lint job fails. (Inline comment posted.)

Verified (no action needed)

• Forwarder lifecycle (geth_l1_state_provider.go:234-256) — event.NewSubscription owns once-only close / idempotent Unsubscribe; defer inner.Unsubscribe() runs on every exit; the sink <- … send is guarded by <-quit so a stalled sink can't wedge the goroutine. No leak, no send-after-close.
• FinalisedHeight maps ethereum.NotFound → eth.ErrNotFound (distinguishes "no finality yet" from transport failure).
• gethReceiptToEth copies only Logs, which is all starknet_getMessageStatus reads — safe and documented.
• Subscription now lives in l1/subscription.go and StateUpdate/L1StateProvider in l1/l1_state_provider.go — resolves the earlier naming/placement feedback.
• Node/migration/metrics wiring, ctx threading, and listener-once construction are unchanged from the last approved round.

Once the signature is fixed and the file is gofmt'd, the package compiles and this is good from a correctness standpoint. (I can't run go test ./l1/ -race here — it requires approval — but the build error is deterministic and reproduced locally.)
• branch feat/l1-settlement-geth

[github-actions]

Nit: this line is indented with spaces instead of a tab, so gofmt -l flags the file (CI lint will fail). Re-indent with a tab.

Suggested change

	c.applyStateUpdate(stateUpdate)
	c.applyStateUpdate(stateUpdate)

[github-actions]

Claude finished @brbrr's task in 3m 34s —— View job

---

PR Review: L1StateProvider abstraction — re-review after latest push

• Verify the previously-flagged l1/l1.go compile break is resolved
• Re-check forwarder concurrency / resource lifecycle
• Review l1.go rewiring + node/migration/metrics wiring
• Build affected packages (go build ./l1/... ./node/... ./rpc/... — OK)
• Post review summary

Re-reviewed the current tip on feat/l1-settlement-geth. The two blockers from my previous round are both fixed and I found no new blockers.

✅ Resolved since last review

• Compile break fixed — receiveL1StateUpdates (l1/l1.go:249-253) now uses the post-rename types (sub Subscription, updateCh chan *StateUpdate); event/contract are no longer referenced. go build ./l1/... ./node/... ./rpc/... succeeds.
• gofmt clean — l1/l1.go:267 (c.applyStateUpdate(stateUpdate)) is now tab-indented; gofmt -l l1/ node/ rpc/rpccore/ reports nothing.
• Naming/wording feedback landed — Settlement→L1StateProvider rename is consistent (newGethL1StateProvider, l1_state_provider.go); error wraps use the ...ing verb form and clearer messages (creating L1 state provider, initializing L1 client); the unused EthMessagingClient alias and unused fields flagged by @RafaelGranza are gone; export_test.go carries the // Deprecated: tags and the "stay as is" NOTE per @rodrodros.

Verified (no action needed)

• Forwarder lifecycle (geth_l1_state_provider.go:234-256) — built on event.NewSubscription, so once-only close / idempotent Unsubscribe / error delivery are library-owned. defer inner.Unsubscribe() runs on every exit (event / quit / stall+quit) → no upstream leak; the sink <- … send is guarded by <-quit, so a stalled sink can't wedge the goroutine; Unsubscribe() blocks until the producer returns → no send-after-close.
• Resubscribe path (l1.go:262-281) — Unsubscribe() blocks until the old producer stops before a new one is created, so there's never a second concurrent writer on the shared updateCh.
• FinalisedHeight maps ethereum.NotFound → eth.ErrNotFound (distinguishes "no finality yet" from transport failure).
• listener race — read unlocked from RPC methods but set once at construction (WithL1StateProviderListener) before hand-off, and documented. Single-shot wiring respected. Metrics gauges that outlive Close() return 0 on error, no panic.
• ctx threading — newGethL1StateProvider layers a 1-min dial timeout on the caller's ctx; migration passes its own ctx; node.New uses context.Background() (no node-lifetime ctx at construction).
• gethReceiptToEth copies only Logs, which is all starknet_getMessageStatus reads — safe and documented.

Nits (non-blocking, cosmetic — carried over, no inline comments)

• SinkStalledThenUnsubscribe determinism (geth_forwarder_test.go:42) — raw is still buffered (cap 1), so the producer's outer select may take <-quit before dequeuing; the intended "blocked on sink <- …" path isn't guaranteed each run (test still passes — it only asserts shutdown + unsubscribe). An unbuffered raw would pin the stall path.
• gethReceiptToEth latent footgun — silently drops every receipt field except Logs; a future L1Client consumer reading Status/BlockNumber would get a zero value with no compile-time signal. Mitigated by the doc comment; known trade-off.

Verdict: No blockers — the previous compile/format breakage is fixed, the package builds and is gofmt-clean, and all cross-round feedback is addressed. Good to merge from a correctness standpoint (note: I can't approve PRs). I couldn't run go test ./l1/ -race here (requires approval), but the packages build cleanly and the forwarder logic + assertions check out on inspection.
• branch feat/l1-settlement-geth