chore: remove candidate transactions and deprecated preconfirmed api

[EgeCaner]

No description provided.

[github-actions]

Claude finished @EgeCaner's task in 4m 11s —— View job

---

PR Review: remove candidate transactions and deprecated preconfirmed API

• Read PR diff (non-test source changes)
• Review adapters / feeder / starknet removals
• Review rpc v9/v10 subscription & transaction changes
• Check for dangling references / build correctness
• Post inline comments + summary

This is a clean, well-scoped removal. The Go packages all build (go build ./rpc/... ./adapters/... ./core/... ./clients/... ./starknet/... ./builder/... ./sequencer/... ./sync/... passes), there are no dangling references to the removed symbols (CandidateTxs, IsCandidateTx, GetCandidateTransaction, DeprecatedPreConfirmedBlock), and the AdaptPreConfirmedBlock / TransactionStatus refactors are functionally equivalent minus the candidate paths. One robustness point and one minor question below.

Important

• adapters/sn2core/sn2core.go:538-544 — AdaptPreConfirmedBlock now indexes Receipts[i]/TransactionStateDiffs[i] unconditionally. The length check at L517-523 prevents index-out-of-range but not per-element nil, so a nil receipt or state diff (the old candidate signal) would now panic rather than error. This is safe under the new feeder API contract (no candidate slots), but since the input is external, consider an explicit guard — and note the test-side require.NotNilf contract check was also dropped. (inline comment posted)

Nit / question

• rpc/v{9,10}/subscription_transactions.go — TxnStatusCandidate / "CANDIDATE" is still accepted by the subscription UnmarshalJSON (subscription_types.go:102, subscriptions.go:309), but newTransactionsSubscriber no longer wires up any handler for it. A client subscribing to CANDIDATE alone will now silently receive nothing instead of an error. That's reasonable given candidates no longer exist in the protocol, but worth confirming whether the RPC should reject the value (or whether the enum should be removed in a follow-up) for clearer client feedback.
• TransactionStatus now relies entirely on the feeder for any CANDIDATE finality (still mapped via adapt_transaction.go:275), rather than the local pre-confirmed search. Intentional and consistent with the deprecation — just noting the behavior shift.

Nothing here blocks the PR. The candidate removal looks correct and the test reduction tracks the deleted code paths.
• chore/remove-candidate-txs-and-deprecated-pconf-api

[codecov]

Codecov Report

❌ Patch coverage is 89.74359% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.11%. Comparing base (82780a4) to head (9460a87).

Files with missing lines	Patch %	Lines
rpc/v10/transaction.go	75.00%	1 Missing and 1 partial ⚠️
rpc/v9/transaction.go	75.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3732      +/-   ##
==========================================
- Coverage   75.26%   75.11%   -0.16%     
==========================================
  Files         433      432       -1     
  Lines       38558    38446     -112     
==========================================
- Hits        29022    28879     -143     
- Misses       7536     7564      +28     
- Partials     2000     2003       +3     

☔ 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 @EgeCaner's task in 2m 50s —— View job

---

PR Review: remove candidate transactions and deprecated preconfirmed API

• Read updated PR diff (source changes)
• Review sn2core adapter (nil-deref concern)
• Review rpc v9/v10 subscription & transaction changes
• Check for dangling references / build correctness
• Post inline comments + summary

This is a clean, well-scoped removal. Affected Go packages build (go build ./adapters/... ./rpc/... ./core/... ./clients/... ./starknet/... ./builder/... ./sequencer/... ./sync/... passes), there are no dangling references to the removed symbols (CandidateTxs, IsCandidateTx, GetCandidateTransaction, DeprecatedPreConfirmedBlock, processCandidateTransactions), and the AdaptPreConfirmedBlock / TransactionStatus / subscription refactors are functionally equivalent minus the candidate paths.

Important

• adapters/sn2core/sn2core.go:538-544 — AdaptPreConfirmedBlock now indexes TransactionStateDiffs[i] / Receipts[i] unconditionally. The length check (L517-523) prevents index-out-of-range but not per-element nil, so a nil receipt or state diff (the old candidate signal) would panic instead of erroring. Since the input is an external feeder-gateway response, a malformed payload crashes the adapter. The test-side require.NotNilf contract assertion was also dropped. Inline comment with a suggested guard posted. This is a robustness nit under the new API contract (no candidate slots), not a confirmed bug.

Nit

• rpc/v{9,10}/subscription_transactions.go — TxnStatusCandidate / "CANDIDATE" is still accepted by the subscription UnmarshalJSON, but newTransactionsSubscriber no longer wires any handler for it. A client subscribing to CANDIDATE alone now silently receives nothing rather than an error. Reasonable given candidates no longer exist in the protocol — worth confirming whether the RPC should reject the value or drop the enum in a follow-up.
• rpc/v{9,10}/transaction.go — TransactionStatus now relies entirely on the feeder for any CANDIDATE finality (still mapped via adapt_transaction.go), rather than the local pre-confirmed search. Intentional and consistent with the deprecation; just noting the behavior shift. The codecov-flagged uncovered lines are the feederClient == nil break and the adapt-error branch — test gaps, not bugs.

Nothing here blocks the PR. The candidate removal looks correct and the test reduction tracks the deleted code paths.
• chore/remove-candidate-txs-and-deprecated-pconf-api

[github-actions]

Claude finished @EgeCaner's task in 2m 14s —— View job

---

PR Review: remove candidate transactions and deprecated preconfirmed API

• Read updated PR diff (source changes)
• Review sn2core adapter (nil-deref concern)
• Review rpc v9/v10 subscription & transaction changes
• Check for dangling references / build correctness
• Post inline comments + summary

This is a clean, well-scoped removal. Affected Go packages build (go build ./adapters/... ./rpc/... ./core/... ./clients/... ./starknet/... ./builder/... ./sequencer/... ./sync/... passes), there are no dangling references to the removed symbols (CandidateTxs, IsCandidateTx, GetCandidateTransaction, DeprecatedPreConfirmedBlock, processCandidateTransactions), and the AdaptPreConfirmedBlock / TransactionStatus / subscription refactors are functionally equivalent minus the candidate paths. The diff is unchanged since the previous review.

Important

• adapters/sn2core/sn2core.go:538-544 — AdaptPreConfirmedBlock indexes TransactionStateDiffs[i] / Receipts[i] unconditionally. The length check (L517-523) prevents index-out-of-range but not per-element nil, so a nil receipt or state diff (the old candidate signal) would panic instead of erroring. Since the input is an external feeder-gateway response, a malformed payload crashes the adapter. The test-side require.NotNilf contract assertion was also dropped. Robustness nit under the new no-candidate API contract, not a confirmed bug — inline comment with a suggested guard posted.

Nit

• rpc/v{9,10}/subscription_transactions.go — "CANDIDATE" is still accepted by the subscription UnmarshalJSON, but newTransactionsSubscriber (L91-102) no longer wires any handler for it. A client subscribing to CANDIDATE alone now silently receives nothing rather than an error. Reasonable given candidates no longer exist — worth confirming whether the RPC should reject the value or drop the enum in a follow-up.
• rpc/v{9,10}/transaction.go — TransactionStatus now relies entirely on the feeder for any CANDIDATE finality (still mapped via adapt_transaction.go) rather than the local pre-confirmed search. Intentional and consistent with the deprecation. The codecov-flagged uncovered lines are the feederClient == nil break and the adapt-error branch — test gaps, not bugs.

Nothing here blocks the PR. The candidate removal looks correct and the test reduction tracks the deleted code paths.

[rodrodros]

LGTM

[github-actions]

Claude finished @rodrodros's task in 1m 56s —— View job

---

PR Review: remove candidate transactions and deprecated preconfirmed API

• Read updated PR diff (incl. latest chore(adapters): remove unnecessary checks)
• Review sn2core adapter changes
• Review rpc v9/v10 subscription & transaction changes
• Check for dangling references / build correctness
• Post summary

The latest commit (9460a87) removes the status != "PRE_CONFIRMED" and payload-size checks from AdaptPreConfirmedBlock. I traced whether @rodrodros's "new validation layer" claim actually holds for the production path — it does, and this resolves the nil-deref/panic concern raised in the earlier review rounds.

Verification of the removed checks

The only production path producing a starknet.PreConfirmedBlock is the feeder client:

PreConfirmedBlockWithIdentifier (clients/feeder/feeder.go:379) → doRequest[…PreConfirmedUpdateEnvelope] → V(&result).Validate() (clients/feeder/request.go:38, runs on every decoded response) → PreConfirmedUpdateEnvelope.Validate() (starknet/pre_confirmed_update.go:178) → PreConfirmedBlock.validate() → validateTxsLength().

validateTxsLength (starknet/pre_confirmed_update.go:101) enforces exactly what the adapter used to defend against — and more:

• length equality across txs / receipts / state_diffs (replaces the removed size check → no index-out-of-range),
• per-element non-nil for receipts and state diffs (the original nil-panic concern → now caught here with a clear error),
• non-empty transactions,
• Status == "PRE_CONFIRMED" (replaces the removed status check, in validate()).

The PreConfirmedDeltaUpdate branch in ApplyUpdate runs the same validateTxsLength, so the delta path is covered too. Because validate() runs unconditionally inside doRequest, a malformed feeder payload now returns ErrInvalidFeederResponse before reaching the adapter rather than panicking inside it. The removed checks were genuinely redundant — this is a correct cleanup, and the doc comment on AdaptPreConfirmedBlock documents the precondition.

Result

Clean, well-scoped removal. go build ./adapters/... ./rpc/... ./clients/... ./starknet/... ./sync/... ./core/... passes, no dangling references to the removed symbols (CandidateTxs, IsCandidateTx, GetCandidateTransaction, DeprecatedPreConfirmedBlock, processCandidateTransactions), and the test reduction tracks the deleted code paths.

No blocking issues. The earlier panic-on-untrusted-input concern is fully addressed by the feeder-side validation layer — no inline comments to add this round.

Minor follow-up notes (non-blocking, unchanged from prior rounds):

• rpc/v{9,10}/subscription_transactions.go — "CANDIDATE" is still accepted by the subscription UnmarshalJSON but no longer wired to a handler; a client subscribing to CANDIDATE alone silently receives nothing. Reasonable given candidates no longer exist; consider rejecting the value or dropping the enum in a follow-up for clearer client feedback.
• TransactionStatus now relies entirely on the feeder for any CANDIDATE finality — intentional and consistent with the deprecation.
  • chore/remove-candidate-txs-and-deprecated-pconf-api