Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RPC] getStateUpdate #365

Merged
merged 16 commits into from
Sep 1, 2022
Merged

[RPC] getStateUpdate #365

merged 16 commits into from
Sep 1, 2022

Conversation

adriantpaez
Copy link
Contributor

@adriantpaez adriantpaez commented Aug 22, 2022

Resolves #364
Resolves #353

Description

The starknet_getStateUpdate RPC endpoint returns Felt values as decimal representation when we need it to return them as hexadecimal. Also, I introduce some improvements to the StateUpdate flow (get-save-serve).

Changes:

  • Add sync.proto file to manage all database marshal and unmarshal processes for StateUpdate
  • Rename StateDiff to StateUpdate
  • Use Felt as a key in StorageDiff map instead of string
  • Parse BlockHash in stateUpdateResponseToStateDiff function
  • Add the Value() method to Felt to get the value referenced by a pointer to Felt

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Testing

Requires testing: Yes

Did you write tests??: No. I test the output of starkent_getStateUpdate by hand and all the errors are fixed. In the future, we need to add unit tests to this.

Documentation

If this requires a documentation update, did you add one? No. The documentation is already on main

Update the database manager with the marshal and unmarshal functions
from protobuf definitions.

Use Felt instead its hex representation for storage diffs maps.
@adriantpaez adriantpaez added the RPC JSON RPC API label Aug 22, 2022
@adriantpaez adriantpaez added this to the 0.2 milestone Aug 22, 2022
@adriantpaez adriantpaez self-assigned this Aug 22, 2022
@codecov
Copy link

codecov bot commented Aug 26, 2022

Codecov Report

Merging #365 (aca0b2c) into main (517971b) will decrease coverage by 0.29%.
The diff coverage is 74.05%.

@@            Coverage Diff             @@
##             main     #365      +/-   ##
==========================================
- Coverage   92.12%   91.82%   -0.30%     
==========================================
  Files          66       67       +1     
  Lines        7083     7228     +145     
==========================================
+ Hits         6525     6637     +112     
- Misses        554      585      +31     
- Partials        4        6       +2     
Flag Coverage Δ
unittests 91.82% <74.05%> (-0.30%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
internal/rpc/starknet/starknet.go 0.00% <0.00%> (ø)
internal/rpc/starknet/types.go 0.00% <0.00%> (ø)
internal/sync/sync.go 100.00% <ø> (ø)
internal/sync/utils.go 100.00% <ø> (ø)
internal/db/sync/manager.go 95.34% <93.05%> (-4.66%) ⬇️
internal/db/sync/sync.pb.go 100.00% <100.00%> (ø)
internal/sync/apiCollection.go 100.00% <100.00%> (ø)
internal/sync/l1Collector.go 100.00% <100.00%> (ø)
pkg/felt/helpers.go 100.00% <100.00%> (ø)
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@adriantpaez adriantpaez marked this pull request as ready for review August 26, 2022 14:38
Copy link
Contributor

@joshklop joshklop left a comment

Choose a reason for hiding this comment

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

LGTM

internal/rpc/starknet/types.go Show resolved Hide resolved
Use github.com/stretchr/testify/assert instead of gotest.tools/assert
Use gotest.tools/assert instead of github.com/stretchr/testify/assert
pkg/felt/helpers.go Outdated Show resolved Hide resolved
@adriantpaez adriantpaez merged commit c58249e into main Sep 1, 2022
@adriantpaez adriantpaez deleted the adrian/starknet_getStateUpdate branch September 1, 2022 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC JSON RPC API
Projects
None yet
5 participants