-
Notifications
You must be signed in to change notification settings - Fork 130
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/modules) implement state_queryStorage
rpc method
#1707
Changes from 8 commits
dae9318
1fa9a96
49a92d5
8d02937
5062c5d
729fb22
86b1d6f
3ba79fe
50a8fba
59189ea
6cb2465
91b371c
08bfddc
6e7d83b
ade00e4
0845fd0
04fdfbe
ea91284
5f3f69a
a709595
0364347
5002961
65d034c
0754903
59cbcc2
d8047ba
d46afba
4f0ac85
27b96d3
d4264b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -17,6 +17,7 @@ package core | |||||
|
||||||
import ( | ||||||
"context" | ||||||
"errors" | ||||||
"math/big" | ||||||
"os" | ||||||
"sync" | ||||||
|
@@ -550,3 +551,66 @@ func (s *Service) GetMetadata(bhash *common.Hash) ([]byte, error) { | |||||
rt.SetContextStorage(ts) | ||||||
return rt.Metadata() | ||||||
} | ||||||
|
||||||
type ( | ||||||
// Changes represents the key-value data inside a block storage | ||||||
Changes map[string]string | ||||||
) | ||||||
|
||||||
// QueryStorage returns the key-value data by block based on `keys` params | ||||||
// on every block starting `from` until `to` block, if `to` is not nil | ||||||
func (s *Service) QueryStorage(from *common.Hash, to *common.Hash, keys []string) (map[common.Hash]Changes, error) { | ||||||
if from == nil { | ||||||
return nil, errors.New("cannot query data without a starting block hash") | ||||||
} | ||||||
|
||||||
var err error | ||||||
blocksToQuery := []common.Hash{*from} | ||||||
if to != nil { | ||||||
if blocksToQuery, err = s.blockState.SubChain(*from, *to); err != nil { | ||||||
noot marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return nil, err | ||||||
} | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think if there is no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok! I will take a look at substrate node implementation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update to use the |
||||||
|
||||||
queries := make(map[common.Hash]Changes) | ||||||
for _, hash := range blocksToQuery { | ||||||
changes, err := s.tryQueryStorage(hash, keys) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
|
||||||
queries[hash] = changes | ||||||
} | ||||||
|
||||||
return queries, nil | ||||||
} | ||||||
|
||||||
// tryQueryStorage will try to get all the `keys` inside the block's current state | ||||||
func (s *Service) tryQueryStorage(block common.Hash, keys []string) (Changes, error) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
It's assumed you are making a best effort to retrieve the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why did we need to change the method's name? |
||||||
stateRootHash, err := s.storageState.GetStateRootFromBlock(&block) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
|
||||||
changes := make(map[string]string) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't it be better if we make this slice initially instead of a map? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as a map, this function can return the data as |
||||||
|
||||||
for _, k := range keys { | ||||||
keyBytes, err := common.HexToBytes(k) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
|
||||||
storedData, err := s.storageState.GetStorage(stateRootHash, keyBytes) | ||||||
if err != nil { | ||||||
return nil, err | ||||||
} | ||||||
|
||||||
if storedData == nil { | ||||||
continue | ||||||
} | ||||||
|
||||||
changes[k] = common.BytesToHex(storedData) | ||||||
} | ||||||
|
||||||
return changes, nil | ||||||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,9 +85,9 @@ type StateStorageRequest struct { | |
|
||
// StateStorageQueryRangeRequest holds json fields | ||
type StateStorageQueryRangeRequest struct { | ||
Keys []*common.Hash `json:"keys" validate:"required"` | ||
StartBlock *common.Hash `json:"startBlock" validate:"required"` | ||
Block *common.Hash `json:"block"` | ||
Keys []string `json:"keys" validate:"required"` | ||
StartBlock *common.Hash `json:"startBlock" validate:"required"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since this is required, can you change it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||
EndBlock *common.Hash `json:"block"` | ||
} | ||
|
||
// StateStorageKeysQuery field to store storage keys | ||
|
@@ -129,8 +129,8 @@ type StateMetadataResponse string | |
|
||
// StorageChangeSetResponse is the struct that holds the block and changes | ||
type StorageChangeSetResponse struct { | ||
Block *common.Hash | ||
Changes []KeyValueOption | ||
Block *common.Hash `json:"block"` | ||
Changes [][]string `json:"changes"` | ||
} | ||
|
||
// KeyValueOption struct holds json fields | ||
|
@@ -387,8 +387,28 @@ func (sm *StateModule) GetStorageSize(r *http.Request, req *StateStorageSizeRequ | |
} | ||
|
||
// QueryStorage isn't implemented properly yet. | ||
func (sm *StateModule) QueryStorage(r *http.Request, req *StateStorageQueryRangeRequest, res *StorageChangeSetResponse) error { | ||
// TODO implement change storage trie so that block hash parameter works (See issue #834) | ||
func (sm *StateModule) QueryStorage(r *http.Request, req *StateStorageQueryRangeRequest, res *[]StorageChangeSetResponse) error { | ||
changesByBlock, err := sm.coreAPI.QueryStorage(req.StartBlock, req.EndBlock, req.Keys) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
var response []StorageChangeSetResponse | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pre-allocate this slice.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||
|
||
for block, c := range changesByBlock { | ||
var changes [][]string | ||
|
||
for key, value := range c { | ||
changes = append(changes, []string{key, value}) | ||
} | ||
|
||
response = append(response, StorageChangeSetResponse{ | ||
Block: &block, | ||
Changes: changes, | ||
}) | ||
} | ||
|
||
*res = response | ||
return nil | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we declare this at the top or somewhere in
lib/common
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I move to the top