refactor(feeder): return values instead of pointers from client methods by Ehsan-saradar · Pull Request #3774 · NethermindEth/juno

Ehsan-saradar · 2026-06-30T10:00:53Z

Description

Closes #3686.

Makes the clients/feeder Client methods (and the shared doRequest helper) return values instead of pointers, and updates the Reader interface and all call sites accordingly.

What changed

doRequest[T, V] now returns (T, error) instead of (*T, error).
All Client methods that previously returned *starknet.X now return starknet.X; the Reader interface signatures match.
BlockHeader and FeeTokenAddresses, which previously dereferenced the pointer, are simplified to return the doRequest result directly.
CasmClassDefinition keeps its deprecated-class semantics: it still signals "no compiled class" via ErrDeprecatedCompiledClass, and consumers pass nil to the adapter on that path instead of relying on a nil pointer return.
Consumers updated to take the address locally where a pointer is still required: the starknetdata/feeder adapter, the rpc/v8|v9|v10 trace/transaction handlers, and the affected tests.
mocks/mock_feeder.go regenerated.

Notes

PublicKey now also returns felt.Felt by value (changed during review — it originally kept the pointer return, but there was no reason for the exception). PreConfirmedBlockWithIdentifier still returns the starknet.PreConfirmedUpdate interface, which is unchanged.

Testing

golangci-lint (diff + full): clean
make lint: pass
go test ./...: pass

Change doRequest and the feeder Client methods to return values rather than pointers, and update the Reader interface and all call sites accordingly. Callers that adapt the responses now take the address locally where a pointer is still required. Closes NethermindEth#3686

Copilot

Pull request overview

Refactors the clients/feeder API (and downstream adapters/callers) to return Starknet response structs by value instead of pointers, updating the Reader interface and all affected call sites/tests accordingly.

Changes:

Updated doRequest and clients/feeder Client/Reader methods to return value types (with local &value taken where pointers are still required).
Adjusted starknetdata/feeder adapter and RPC trace/transaction handlers to pass addresses to existing adapter functions.
Regenerated mocks/mock_feeder.go and updated tests to assert on zero/non-zero values instead of nil/non-nil pointers.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`starknetdata/feeder/feeder.go`	Adapts to feeder client value returns; takes addresses when calling `sn2core` adapters; preserves deprecated CASM semantics via `nil` CASM pointer.
`starknetdata/feeder/feeder_test.go`	Updates adapter test expectations to pass `&response` and handle optional CASM via `*starknet.CasmClass`.
`starknet/compiler/compiler_test.go`	Updates compiled-class adaptation call to pass `&compiledDef`.
`rpc/v9/transaction.go`	Passes `&txStatus` into status adapter after client returns a value.
`rpc/v9/trace.go`	Passes `&blockTrace` into trace adapter after client returns a value.
`rpc/v8/transaction.go`	Same as v9: uses `&txStatus` for adaptation.
`rpc/v8/trace.go`	Same as v9: uses `&blockTrace` for adaptation.
`rpc/v10/transaction.go`	Same as v9: uses `&txStatus` for adaptation.
`rpc/v10/trace.go`	Same as v9: uses `&blockTrace` for adaptation.
`mocks/mock_feeder.go`	Regenerated gomock for `Reader` interface now returning values.
`consensus/p2p/validator/fixtures_test.go`	Updates fixture builder to pass `&rawStateUpdate` to helpers expecting pointers.
`clients/feeder/request.go`	Changes `doRequest` to return `(T, error)` rather than `(*T, error)`.
`clients/feeder/feeder.go`	Updates `Reader` interface + `Client` methods to return values; simplifies methods that previously dereferenced `doRequest` pointers.
`clients/feeder/feeder_test.go`	Updates tests to use `assert.Zero`/`assert.NotZero` due to value returns.
`adapters/sn2core/sn2core_test.go`	Updates adapter tests to pass `&response` / `&compiled` as needed.

Files not reviewed (1)

mocks/mock_feeder.go: Generated file

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

Ehsan-saradar · 2026-07-01T06:01:45Z

 	if err = json.NewDecoder(body).Decode(&result); err != nil {
-		return nil, err
+		return result, err
 	}

 	err = V(&result).Validate()
 	if err != nil {
-		return nil, errors.Join(
+		return result, errors.Join(
 			ErrInvalidFeederResponse,
 			fmt.Errorf("querying %s: %w", fullURL, err),
 		)
 	}


Good catch, fixed. Now returning the zero value of T on both the decode and validation error paths so callers can never get partial data back.

Avoid returning partially decoded or validation-failed data alongside a non-nil error so callers cannot accidentally consume invalid responses.

rodrodros

LGTM, two small comments and good to merge

rodrodros · 2026-07-01T08:06:16Z

 	body, err := c.get(ctx, queryURL)
 	if err != nil {
-		return nil, err
+		return class, err


return default values by returning starknet.CasmClass{}. Please apply this to all the methods that return by value

Done — all error paths now return starknet.CasmClass{}. For the doRequest-based methods the helper itself returns the zero value of T on every error path, so this is consistent across all value-returning methods.

rodrodros · 2026-07-01T08:08:21Z

+	return doRequest[starknet.FeeTokenAddresses](ctx, c, queryURL)
 }

 func (c *Client) PublicKey(ctx context.Context) (*felt.Felt, error) {


This one should return by value too, is it possible?

Done — changed PublicKey to return felt.Felt by value (returns felt.Felt{} on the error path).

- CasmClassDefinition returns starknet.CasmClass{} literal on error paths - PublicKey returns felt.Felt by value instead of *felt.Felt

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

mocks/mock_feeder.go: Generated file

Ehsan-saradar · 2026-07-06T06:11:55Z

 	PreConfirmedBlockWithIdentifier(
 		ctx context.Context,
 		blockNumber string,
 		blockIdentifier string,
 		knownTransactionCount uint64,
 	) (starknet.PreConfirmedUpdate, error)
-	PublicKey(ctx context.Context) (*felt.Felt, error)
-	Signature(ctx context.Context, blockID string) (*starknet.Signature, error)
-	StateUpdate(ctx context.Context, blockID string) (*starknet.StateUpdate, error)
-	StateUpdateWithBlock(ctx context.Context, blockID string) (*starknet.StateUpdateWithBlock, error)
+	PublicKey(ctx context.Context) (felt.Felt, error)
+	Signature(ctx context.Context, blockID string) (starknet.Signature, error)
+	StateUpdate(ctx context.Context, blockID string) (starknet.StateUpdate, error)


Good catch — the description was stale. The change to PublicKey is intentional (requested during review), so I've updated the PR description to match the code.

Copilot

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.

Files not reviewed (1)

mocks/mock_feeder.go: Generated file

Ehsan-saradar marked this pull request as ready for review July 1, 2026 05:21

Copilot AI review requested due to automatic review settings July 1, 2026 05:21

Copilot started reviewing on behalf of Ehsan-saradar July 1, 2026 05:22 View session

Copilot AI reviewed Jul 1, 2026

View reviewed changes

refactor(feeder): return zero value of T on doRequest error paths

45b8b99

Avoid returning partially decoded or validation-failed data alongside a non-nil error so callers cannot accidentally consume invalid responses.

rodrodros reviewed Jul 1, 2026

View reviewed changes

refactor(feeder): address review comments on value returns

483bb4c

- CasmClassDefinition returns starknet.CasmClass{} literal on error paths - PublicKey returns felt.Felt by value instead of *felt.Felt

Ehsan-saradar requested review from Copilot and rodrodros July 4, 2026 20:29

Copilot started reviewing on behalf of Ehsan-saradar July 4, 2026 20:29 View session

Copilot AI reviewed Jul 4, 2026

View reviewed changes

Ehsan-saradar requested a review from Copilot July 6, 2026 08:12

Copilot started reviewing on behalf of Ehsan-saradar July 6, 2026 08:13 View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(feeder): return values instead of pointers from client methods#3774

refactor(feeder): return values instead of pointers from client methods#3774
Ehsan-saradar wants to merge 3 commits into
NethermindEth:mainfrom
Ehsan-saradar:fix/3686-feeder-return-by-value

Ehsan-saradar commented Jun 30, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Ehsan-saradar Jul 1, 2026

Uh oh!

rodrodros left a comment

Uh oh!

rodrodros Jul 1, 2026

Uh oh!

Ehsan-saradar Jul 1, 2026

Uh oh!

rodrodros Jul 1, 2026

Uh oh!

Ehsan-saradar Jul 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Ehsan-saradar Jul 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Ehsan-saradar commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What changed

Notes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Ehsan-saradar Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

rodrodros left a comment

Choose a reason for hiding this comment

Uh oh!

rodrodros Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

Ehsan-saradar Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

rodrodros Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

Ehsan-saradar Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Ehsan-saradar Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Ehsan-saradar commented Jun 30, 2026 •

edited

Loading