Skip to content

refactor(feeder): use the new includeSignature flag in the get_state_update feeder endpoint#3601

Draft
thiagodeev wants to merge 16 commits into
mainfrom
thiagodeev/feeder-optimizations
Draft

refactor(feeder): use the new includeSignature flag in the get_state_update feeder endpoint#3601
thiagodeev wants to merge 16 commits into
mainfrom
thiagodeev/feeder-optimizations

Conversation

@thiagodeev
Copy link
Copy Markdown
Contributor

@thiagodeev thiagodeev commented May 4, 2026

Combine get_state_update + get_signature into a single feeder call by passing the new includeSignature=true parameter to get_state_update. Removes the redundant f.client.Signature(ctx, blockID) call in starknetdata/feeder.stateUpdateWithBlock.

  • Split clients/feeder.Client.StateUpdateWithBlock into two methods:
    • StateUpdateWithBlock(ctx, blockID) — unchanged behaviour, no signature.
    • StateUpdateWithBlockAndSignature(ctx, blockID) — single request, returns state update + block + signature.
  • Add starknet.StateUpdateWithBlockAndSignature type carrying Signature []*felt.Felt.
  • adapters/sn2core.AdaptBlock now takes []*felt.Felt instead of *starknet.Signature.
  • Update clients/feeder/testdata/**/state_update_with_block/*.json fixtures to include the signature field (values pulled from the matching signature/*.json)

Reference: https://demerzelsolutions.slack.com/archives/C03090TS3TK/p1777375140952279

Extra:

  • In clients/feeder/feeder.go, per-method c.get + json.NewDecoder(body).Decode(&x) boilerplate was replaced by a single generic doRequest[T any](ctx, client, queryURL) (*T, error) helper. Methods in the file are now sorted alphabetically. No behavioural change, just style.

Note:

  • this PR can only be merged after the feeder implements this new feature

TODO

  • add tests for the new MigrationFeeder feat
  • update this PR description wiht the chagnes

@thiagodeev thiagodeev added the disable-deploy-test We don't want to run deploy tests with this PR because it might affect our development environment. label May 4, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

❌ Patch coverage is 51.67785% with 72 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.20%. Comparing base (6fc5100) to head (4457b3c).

Files with missing lines Patch % Lines
starknetdata/feeder/migrationFeeder.go 4.76% 60 Missing ⚠️
clients/feeder/feeder.go 89.28% 5 Missing and 1 partial ⚠️
starknetdata/feeder/feeder.go 75.00% 2 Missing and 4 partials ⚠️

❌ Your patch check has failed because the patch coverage (51.67%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3601      +/-   ##
==========================================
+ Coverage   76.16%   76.20%   +0.03%     
==========================================
  Files         396      397       +1     
  Lines       36585    36541      -44     
==========================================
- Hits        27865    27846      -19     
+ Misses       6743     6726      -17     
+ Partials     1977     1969       -8     

☔ View full report in Codecov by Sentry.
📢 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.

@thiagodeev thiagodeev force-pushed the thiagodeev/feeder-optimizations branch from 77f8e8a to f5fbdf0 Compare May 5, 2026 12:37
@thiagodeev thiagodeev changed the title Thiagodeev/feeder-optimizations refactor(feeder): use the new includeSignature flag in the get_state_update feeder endpoint May 5, 2026
@thiagodeev thiagodeev force-pushed the thiagodeev/feeder-optimizations branch 2 times, most recently from b9faf76 to b730843 Compare May 8, 2026 13:21
@thiagodeev thiagodeev force-pushed the thiagodeev/feeder-optimizations branch 2 times, most recently from d71d865 to 9401d3e Compare May 19, 2026 10:23
@RafaelGranza RafaelGranza self-requested a review May 20, 2026 12:13
@thiagodeev thiagodeev force-pushed the thiagodeev/feeder-optimizations branch 2 times, most recently from a5252de to 4501be1 Compare May 21, 2026 02:21
Comment thread starknetdata/feeder/feeder_test.go
Comment thread clients/feeder/feeder.go Outdated
@RafaelGranza
Copy link
Copy Markdown
Contributor

Not approving yet, only because I'd like to run this against my own Juno node first. Code-wise looks fine.

Copy link
Copy Markdown
Contributor

@RafaelGranza RafaelGranza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After testing locally, sepolia-integration works as expected, and I would have
approved based on the approach we'd originally agreed on.

But since you'll be adding the capability-detection on top of this so it stays
compatible with all Starknet versions, I'd rather wait and re-test once that
lands before approving.

@thiagodeev thiagodeev force-pushed the thiagodeev/feeder-optimizations branch 2 times, most recently from 1afc808 to d6ebb1b Compare May 21, 2026 20:21
@RafaelGranza
Copy link
Copy Markdown
Contributor

Holding off on approval.

I know the capability detection is coming in a follow-up, but I'd rather see the optimization and the detection land as a single PR. Otherwise the node sits in a half-broken state in between, working on sepolia-integration but dropping Block.Signatures on mainnet/sepolia/integration.

Happy to approve as soon as the two are combined.

@thiagodeev thiagodeev removed the disable-deploy-test We don't want to run deploy tests with this PR because it might affect our development environment. label May 22, 2026
@thiagodeev thiagodeev force-pushed the thiagodeev/feeder-optimizations branch from 34e9a6e to 4457b3c Compare May 22, 2026 20:49
@thiagodeev thiagodeev deployed to Development May 22, 2026 20:54 — with GitHub Actions Active
Copy link
Copy Markdown
Contributor

@rodrodros rodrodros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the change but I am unsure if this is the best way of doing it 🤔. Let's talk on Monday

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants