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

feat(dot/rpc): implement RPC method state_queryStorageAt #3185

Closed
wants to merge 7 commits into from

Conversation

edwardmack
Copy link
Contributor

Changes

  • Implement code for RPC method state_queryStorageAt which returns storage values for given keys at best block or given block hash if at parameter is used.
  • This fixes error rpc: can't find method "state.QueryStorageAt" that occurs when using polkadot.js/api methods in api.query package.

Tests

go test github.com/ChainSafe/gossamer/dot/rpc/modules -run ^TestStateModuleQueryStorageAt$ -v

Issues

closes: #3182

Primary Reviewer

@timwu20

@codecov
Copy link

codecov bot commented Mar 30, 2023

Codecov Report

Merging #3185 (20602d3) into development (7c08365) will increase coverage by 0.07%.
The diff coverage is 100.00%.

❗ Current head 20602d3 differs from pull request most recent head 49737fa. Consider uploading reports for the commit 49737fa to get more accurate results

Additional details and impacted files
@@               Coverage Diff               @@
##           development    #3185      +/-   ##
===============================================
+ Coverage        51.45%   51.52%   +0.07%     
===============================================
  Files              221      221              
  Lines            28281    28305      +24     
===============================================
+ Hits             14551    14584      +33     
+ Misses           12410    12404       -6     
+ Partials          1320     1317       -3     

Comment on lines +79 to +80
// StateStorageQueryAtRequest holds json fields
type StateStorageQueryAtRequest struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think StateStorageQueryAtRequest can be unexported

Copy link
Contributor

Choose a reason for hiding this comment

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

That would apply to its properties as well.

I don't see any variable of type StateStorageQueryAtRequest getting used by scale. Because scale requires structs and properties to be exported.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think they are exported for the encoding/json encoder to work right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, they are exported for encoding/json encoder to work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still, is there a reason we keep StateStorageQueryAtRequest exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we're using Gorilla web toolkit to expose functions through RPC, Gorilla exposes functions with a signature of containing exported structs for request and response. I tried changing StateStorageQueryAtRequest to un-exported and the RPC function was no longer accessible.

@edwardmack edwardmack force-pushed the ed/implement_state_queryStorageAt branch from 26cea02 to 99d0f68 Compare April 3, 2023 15:14
Copy link
Contributor

@jimjbrettj jimjbrettj left a comment

Choose a reason for hiding this comment

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

lgtm!

@edwardmack edwardmack requested a review from qdm12 April 4, 2023 20:01
Comment on lines +79 to +80
// StateStorageQueryAtRequest holds json fields
type StateStorageQueryAtRequest struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Still, is there a reason we keep StateStorageQueryAtRequest exported?

changes[i] = [2]*string{stringPtr(key), hexValue}
}

*response = []StorageChangeSetResponse{{
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL you don't need to initialize the slice pointer which is weird AF

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is because response is initialized since it is a function argument.

@edwardmack
Copy link
Contributor Author

This PR has been moved to PR #3191 which contains only code related to implementing state_queryStorageAt (not other merged code from other branches). @qdm12 please let me know if the remaining comments on this can be resolved, I believe I've addressed all comments in the new PR.

@edwardmack
Copy link
Contributor Author

closed see merged PR #3191

@edwardmack edwardmack closed this Apr 12, 2023
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.

Implement RPC method state_queryStorageAt
5 participants