feat: validate feeder responses on the client layer

[thiagodeev]

This PR makes the feeder client validate gateway responses before handing them off, so malformed or incomplete data from the gateway is caught at the boundary and reported as an error instead of slipping through and causing failures later.

Validation currently covers the pre_confirmed block and delta-update responses, checking that the fields the rest of the system relies on are present and consistent; other response types are left unvalidated for now and will be addressed in follow-up PRs. Because these guarantees are now enforced at the boundary, duplicate checks downstream in the sn2core adapter are no longer needed and have been removed.

[codecov]

Codecov Report

❌ Patch coverage is 93.08176% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.22%. Comparing base (942d552) to head (db31c27).

Files with missing lines	Patch %	Lines
starknet/pre_confirmed_update.go	94.82%	3 Missing ⚠️
sync/pending_polling.go	40.00%	2 Missing and 1 partial ⚠️
node/node.go	50.00%	1 Missing and 1 partial ⚠️
starknet/fee_token_addresses.go	0.00%	2 Missing ⚠️
clients/feeder/feeder.go	95.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3749      +/-   ##
==========================================
+ Coverage   75.16%   75.22%   +0.05%     
==========================================
  Files         427      433       +6     
  Lines       38470    38558      +88     
==========================================
+ Hits        28915    29004      +89     
  Misses       7547     7547              
+ Partials     2008     2007       -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 @thiagodeev's task in 2m 54s —— View job

---

PR Review: validate feeder responses on the client layer

Nice direction — validating gateway responses at the client boundary and dropping the duplicated checks in sn2core is a clean improvement, and the Validatable[T] constraint wired into doRequest is a tidy way to make it generic. I found one blocker around candidate transactions, plus a few smaller items. Posted inline; summary below.

🔴 Blocker

• Validation rejects candidate transactions. validateTxsLength (starknet/pre_confirmed_update.go:62) errors whenever any receipts[i]/stateDiffs[i] is nil — but that is exactly how a candidate tx is encoded (IsCandidateTx, adapters/sn2core/sn2core.go:512). AdaptPreConfirmedBlock still handles these and feeds them into pending.PreConfirmed.CandidateTxs, which is consumed downstream (pending.go:165, builder/state.go:52). So any real pre_confirmed block/delta containing a candidate tx will now fail at the feeder boundary and never reach the adapter. It's also inconsistent: the PR removes the candidate-tx workaround from AdaptPreConfirmedWithDelta but keeps the equivalent in AdaptPreConfirmedBlock. Validation should accept the nil receipt and nil state-diff case as a valid candidate (only rejecting the inconsistent one-nil-but-not-the-other case). Fix this →

🟡 Important

• Wrong error message (pre_confirmed_update.go:116): the L1DataGasPrice == nil branch reports "l2_gas_price is required". Suggestion posted inline.

⚪ Nits

• txs[i] == (Transaction{}) (:63) only matches a fully zero-valued tx; a tx with any single field set passes. Consider checking a field the system relies on (e.g. Hash/Version). Also note it relies on Transaction staying comparable.
• clients/feeder/feeder_test.go:1311 — the "valid response is returned" subtest has a copy-pasted comment describing the invalid case.
• Re @rodrodros's formatting comment: doRequest's signature already appears reformatted to the multi-line style in the latest commit — looks addressed.

Notes (non-blocking)

• The many placeholder Validate() methods returning (self, nil) are fine for an incremental rollout, but only Block.Validate carries the TODO — worth a brief note on the others (or a tracking issue) so they aren't mistaken for "validated".
• Patch coverage is low (Codecov flags pre_confirmed_update.go at ~45%); the new validate() branches (missing gas prices, nil sequencer, length mismatch) aren't exercised. Worth a small table-driven test once the candidate-tx behavior is settled.
  · branch thiagodeev/validate-feeder-responses

[github-actions]

Claude finished @thiagodeev's task in 1m 31s —— View job

---

PR Review: validate feeder responses on the client layer

Re-reviewed after commit 2a6342d ("fix: PR comments from Claude").

What got addressed since the last review ✅

• Wrong error message on the L1DataGasPrice == nil branch → now correctly reports "l1_data_gas_price is required" (pre_confirmed_update.go:116).
• Copy-pasted comment in the "valid response is returned" subtest → removed (clients/feeder/feeder_test.go).

🔴 Blocker — still open

• Validation rejects candidate transactions. validateTxsLength (starknet/pre_confirmed_update.go:62-72) still errors whenever any receipts[i] or stateDiffs[i] is nil — but that is exactly how a candidate tx is encoded (IsCandidateTx, adapters/sn2core/sn2core.go:512). AdaptPreConfirmedBlock still feeds these into pending.PreConfirmed.CandidateTxs, consumed at core/pending/pending.go:165 and builder/state.go:52. So any real pre_confirmed block/delta with a candidate tx now fails at the feeder boundary and never reaches the adapter — the candidate-tx path becomes dead code and pre_confirmed sync breaks whenever the gateway emits one. It's also inconsistent with the new "already sanitized, no nil values" comments now placed on both adapters (validation rejects rather than sanitizes), and with this PR removing the candidate-tx workaround from AdaptPreConfirmedWithDelta while keeping it in AdaptPreConfirmedBlock. Fix: accept the both-nil case as a valid candidate; only reject the inconsistent one-nil case. Detail + suggested code posted inline.

⚪ Nits (carried over, non-blocking)

• txs[i] == (Transaction{}) (:63) only catches a fully zero-valued tx — a tx with any single field set passes. Consider checking a field the system relies on (e.g. Hash/Version non-nil). Also relies on Transaction staying comparable.
• Patch coverage stays low — the new validate() branches (missing gas prices, nil sequencer, length mismatch, and especially the candidate-tx path) aren't exercised. Worth a small table-driven test once the candidate-tx behavior is settled.
• The placeholder Validate() methods returning (self, nil) are fine for incremental rollout, but only Block.Validate carries the TODO — a tracking issue/note for the others would avoid them being mistaken for "validated".

The boundary-validation direction and the Validatable[T] wiring into doRequest are a clean improvement — the one blocker is the candidate-tx rejection above; once that's resolved this looks good.
· branch thiagodeev/validate-feeder-responses

[rodrodros]

PR is still looks half baked..., I wouldn't consider it ready for review

[github-actions]

Claude finished @thiagodeev's task in 3m 16s —— View job

---

PR Review: validate feeder responses on the client layer

Re-reviewed after the latest commits (8d2083ef9 → 71fbb24dd). The affected packages (starknet, clients/feeder, adapters/sn2core) build cleanly. Overall this is in good shape now — the structural feedback and the earlier test-coverage gap have been addressed.

✅ Resolved since earlier reviews

• Candidate-tx "blocker" is no longer applicable. Per @rodrodros, candidate transactions are no longer part of the response (to be removed in a follow-up), so validateTxsLength rejecting nil receipts/state-diffs is now correct and internally consistent — both the delta and full-block paths reject nil and neither adapter carries candidate-tx handling anymore. No action needed.
• Test coverage gap closed. pre_confirmed_update_test.go now has table-driven tests exercising every validate() failure branch for both PreConfirmedBlock and PreConfirmedDeltaUpdate (missing gas prices, nil sequencer, length mismatch, empty tx, nil receipt/state-diff), plus the envelope-level NoChange/default-branch paths.
• Structural feedback addressed: PublicKey extracted to starknet/public_key.go; Validatable + doRequest moved to clients/feeder/request.go; validateTxsLength now sits below both callers; placeholder Validate() methods relocated next to their types, each carrying the TODO note.
• Wrong l1_data_gas_price error message and the copy-pasted test comment are fixed.

⚪ Nits (non-blocking, carried over)

• txs[i] == (Transaction{}) (starknet/pre_confirmed_update.go:116) only catches a fully zero-valued tx — a tx with even one field set passes. It also silently depends on Transaction staying comparable (adding a slice/map field later breaks compilation here). Consider asserting a field the system relies on, e.g. Hash != nil. Fix this →

Note on the PreConfirmedNoChange return discussion

Re @thiagodeev's question on :189: your reading is right — the trailing return e, nil is required because the PreConfirmedNoChange case (and the success paths of the other cases) fall through to it. The current shape (empty NoChange case + single trailing return e, nil) is clean and idiomatic; moving the return into each case would only add three duplicate return e, nil lines. I'd leave it as is unless @rodrodros prefers otherwise.

No blockers from my side — once the nit is settled this looks ready.
· branch thiagodeev/validate-feeder-responses

[github-actions]

Claude finished @thiagodeev's task in 2m 37s —— View job

---

PR Review: validate feeder responses on the client layer

Re-reviewed after the latest commits (eb17df3ed → 31fb3f4c0). Affected packages (clients/feeder, starknet, sync) build cleanly. This latest batch is a solid improvement — the Validate() signature is simpler and the error now carries the failing query URL and a typed sentinel.

✅ What the new commits do well

• Validate() error signature — dropping the (*T, error) self-return is the right call; the old returned value was always the receiver and unused. All placeholder methods and the real PreConfirmedUpdateEnvelope.Validate are consistently updated, and no stale Validate() (*…) signatures remain.
• Typed, contextual error — doRequest now wraps with errors.Join(ErrInvalidFeederResponse, …query URL…). The sentinel survives the full call chain (doRequest → PreConfirmedBlockWithIdentifier → starknetdata/feeder → sync/data_source), so errors.Is works downstream. Verified, and the new feeder_test.go assertions cover both the sentinel and the URL in the message.
• Severity-aware polling (sync/pending_polling.go:216) — surfacing a malformed gateway response at Error while keeping transient/network failures at Debug is a sensible distinction, and both branches still continue so the poll loop is unaffected.

⚪ Nits (non-blocking)

• Stale doc comment on Validatable (clients/feeder/request.go:19-26) — still says "returns itself with no changes in case of success", which no longer matches the error-only signature. Suggestion posted inline.
• Error-level log cadence (pending_polling.go:217) — if the gateway persistently emits an invalid pre_confirmed response, this logs at Error every poll interval. That's defensible (you want visibility), just flagging the potential for repeated error lines under a sustained gateway issue.
• Carried over and already addressed by the author: txs[i] == (Transaction{}) (pre_confirmed_update.go:116) — per your note, value-struct txs can't be nil-checked so the empty-struct comparison is intentional. Just be aware it silently depends on Transaction staying comparable (a future slice/map field would break compilation here).

No blockers. The earlier candidate-tx concern is resolved (candidate txs are no longer part of the response, per @rodrodros) and the test-coverage gap is closed. Once the doc-comment nit is settled this looks ready.
· branch thiagodeev/validate-feeder-responses

[github-actions]

Claude finished @thiagodeev's task in 1m 54s —— View job

---

PR Review: validate feeder responses on the client layer

Re-reviewed after the latest commit (0fca2124e). Affected packages (clients/feeder, starknet, sync, adapters/sn2core) build cleanly.

✅ Resolved this round

• Stale Validatable doc comment fixed (clients/feeder/request.go:19-26) — the comment no longer claims the method "returns itself with no changes in case of success"; it now matches the Validate() error contract. This was the only open nit from the previous review.

Status of earlier feedback

• Candidate-tx blocker — resolved per @rodrodros (candidate txs are no longer part of the response; removal is a follow-up), so validateTxsLength rejecting nil receipts/state-diffs is correct and consistent across both the delta and full-block paths.
• Wrong l1_data_gas_price error message, copy-pasted test comment — fixed.
• Structural feedback (PublicKey → its own file, doRequest/Validatable → request.go, validateTxsLength placement, grouped placeholder Validate() methods with TODOs) — addressed.
• Test coverage gap — closed; pre_confirmed_update_test.go has table-driven tests for every validate() failure branch plus the envelope paths.
• txs[i] == (Transaction{}) (pre_confirmed_update.go:116) — author confirmed this is intentional: Transaction is a value struct (not a pointer), so an empty-struct comparison is the only way to detect a null JSON tx. Just be aware it silently depends on Transaction staying comparable — adding a slice/map field later would break compilation here. Non-blocking.

⚪ Minor (non-blocking)

• Error-level log cadence (sync/pending_polling.go:217) — a persistently malformed gateway response will log at Error every poll interval. That's defensible (you want the visibility), just flagging the potential for repeated error lines under a sustained gateway issue. Carried over from the prior review; no change needed.

No blockers. All previously raised items are addressed or accepted; this looks ready to merge.
· branch thiagodeev/validate-feeder-responses

[github-actions]

Claude finished @thiagodeev's task in 3m 26s —— View job

---

PR Review — re-review after latest commits (0fca2124e → 060bbf858)

• Reviewed changes since last review
• Checked *url.URL refactor in request.go / feeder.go
• Verified error-message / validation changes
• Built affected packages (clients/feeder, starknet, sync, adapters/sn2core) — compiles cleanly
• Posted inline comment + summary

---

This batch cleanly addresses @rodrodros's latest round. No blockers.

✅ Resolved this round

• Client.url now stored pre-parsed as *url.URL and buildQueryString is a free function taking the parsed base — matches the "parse the base once, not per request" request. Verified safe: url.URL.JoinPath copies the receiver, so concurrent buildQueryString(c.url, …) calls don't mutate the shared base. It's also slightly more robust than the old base.Path += endpoint (handles a base URL with no trailing slash), and all configured FeederURLs end in /feeder_gateway/ so there's no path regression.
• get moved back into feeder.go next to the Client type, as requested.
• Error wrapping — envelope Validate() now wraps with invalid pre_confirmed delta update: / invalid pre_confirmed full block:, so the failing variant is identifiable; doRequest message trimmed to querying %s: %w.
• Polling log — msg is now a const with the core text "polling pre-confirmed block", and the Debug branch carries block_number. (The ErrInvalidFeederResponse Error branch carries the block number via the query URL in the joined error chain — confirmed.)
• Tests — felt.NewFromUint64 constructor used, ErrorContains assertions added for the envelope default-branch case.

⚪ Nits (non-blocking)

• NewClient panic drops the parse error (feeder.go:151) — including the underlying err + URL would make a misconfigured --cn-feeder-url easier to diagnose. Suggestion posted inline. (No regression — the old code panicked too, just lazily and without the error.)
• Pre-existing (not introduced here, mentioning only for awareness): get writes c.client.Timeout on a shared *http.Client, which is a data race if the client is queried from multiple goroutines. Out of scope for this PR.
• txs[i] == (Transaction{}) (pre_confirmed_update.go:116) — accepted as intentional (value-struct tx can't be nil-checked); silently depends on Transaction staying comparable. Carried over, no change needed.

The boundary-validation direction and Validatable[T]/doRequest wiring are a solid improvement. All previously raised items are addressed or accepted — this looks ready to merge once the panic nit is settled (optional).
· branch thiagodeev/validate-feeder-responses

[github-actions]

Claude finished @thiagodeev's task in 7m 12s —— View job

---

PR Review: validate feeder responses on the client layer

Re-reviewed after the latest commits (060bbf858 → 030c4cfe6). go vet ./clients/feeder/... ./starknet/... ./sync/... passes. This batch cleanly addresses @rodrodros's 06-23 round.

✅ Resolved this round

• No more panic on user-supplied URL — NewClient now takes a pre-parsed *url.URL; parsing + error handling moved to the callers (node.go returns invalid feeder URL …: %w, and cmd/juno validates --cn-feeder-url/--cn-gateway-url up front via the new validateHTTPURL, with table-driven coverage). This is the right fix for the "bad UX to panic" comment.
• Length info in the mismatch error — validateTxsLength now reports txs: %d, receipts: %d, stateDiffs: %d, so you can see which slice differs.
• Block number logged on both branches of the pending-poll error path (pending_polling.go:217,220).
• Multi-line if is gone; the remaining length check is a single-line condition.

Answers to the open questions

• Client.url as *url.URL vs value (feeder.go:33) — pointer is fine here: buildQueryString calls baseURL.JoinPath(...), which copies the receiver and returns a fresh *url.URL, so the shared base is never mutated and concurrent requests are safe. A value field would work too; no correctness difference.
• doRequest returning *T vs T (request.go:26) — *T matches every existing call site (most Client methods return *starknet.X), and T is needed as a type param for the Validatable[T] constraint regardless. Keeping *T avoids a copy and an extra deref at each caller. Reasonable as-is.
• get(ctx, queryURL *url.URL) taking the full URL (feeder.go:165) — fair point that get only uses queryURL.String(). You could narrow it to a string, but doRequest still needs the *url.URL for its querying %s: %w error, so the type has to exist at that layer anyway. Narrowing get to string is a pure style call — leave or change, no impact.

⚪ Nits (non-blocking)

• validateHTTPURL (cmd/juno/juno.go): the two fmt.Errorf("URL must …") calls have no format args — errors.New(...) is marginally cleaner. Not linted here, purely cosmetic.
• node.New still calls url.Parse(cfg.Network.FeederURL), which is lenient (won't reject a scheme-less string) — the real guard is validateHTTPURL in cmd/juno. Fine for built-in networks (valid URLs) and custom networks (validated in PreRunE); just noting the parse there is a safety net, not the validation.
• txs[i] == (Transaction{}) (pre_confirmed_update.go:119) — accepted as intentional (value-struct tx can't be nil-checked); silently depends on Transaction staying comparable. Carried over, no change needed.

No blockers — the boundary-validation direction and Validatable[T]/doRequest wiring are a clean improvement, and all previously raised items are addressed or accepted. Looks ready to merge.
· branch thiagodeev/validate-feeder-responses

[github-actions]

Claude finished @thiagodeev's task in 3m 11s —— View job

---

PR Review: validate feeder responses on the client layer

Re-reviewed after the latest commits (030c4cfe6 → db31c271d). go vet ./cmd/juno/... ./clients/feeder/... ./starknet/... ./sync/... passes. This batch is small and cleanly addresses @rodrodros's 06-23 error-message round.

✅ Resolved this round

• validateHTTPURL error messages (cmd/juno/juno.go:687-692) — the scheme branch now reports the offending value ("URL must use http or https scheme, got %s") and the no-arg fmt.Errorf is now errors.New("URL must have a host"). Both match the two review comments exactly.
• Config-holds-strings-vs-URLs (juno.go:449) and re-parse in node.New (node/node.go:366) — acknowledged and tracked as a follow-up in Make Network type store URLs as a *url.URL type instead of a string #3761, which is the right call to avoid scope creep here.

Verification of the wiring

• validateHTTPURL is invoked in the custom-network PreRunE branch (juno.go:438-443) and wraps with invalid feeder URL %s: %w / invalid gateway URL %s: %w, so a bad --cn-feeder-url/--cn-gateway-url now fails up front with a clear message instead of panicking later. Built-in networks ship valid URLs, so there's no regression there.
• The boundary-validation path (doRequest → errors.Join(ErrInvalidFeederResponse, querying %s: %w)) and the Validatable[T] constraint remain intact; errors.Is(err, ErrInvalidFeederResponse) still works downstream and the failing query URL (incl. block number) is carried in the joined chain.

⚪ Carried over (accepted, no change needed)

• txs[i] == (Transaction{}) (starknet/pre_confirmed_update.go:119) — intentional, since Transaction is a value struct and can't be nil-checked. Silently depends on Transaction staying comparable; a future slice/map field would break compilation here. Noted only for awareness.
• Error-level log cadence on a persistently malformed gateway response (sync/pending_polling.go) — defensible; you want the visibility.

No blockers. All previously raised items are addressed or tracked as follow-ups (#3761) — this looks ready to merge.
· branch thiagodeev/validate-feeder-responses

[rodrodros]

LGTM