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: add FindStorage #805

Merged
merged 4 commits into from
Aug 21, 2023
Merged

rpc: add FindStorage #805

merged 4 commits into from
Aug 21, 2023

Conversation

ixje
Copy link
Contributor

@ixje ixje commented Jul 17, 2023

close #758

Arguments:

  1. Similar to GetStorage() the first argument is the contract hash or contract id
  2. search prefix, base 64 encoded. Can be set to "" to return all storage.
  3. start location
    The pageSize is currently fixed to 50. If the find results exceed 50 results it shall return the truncated key set to true and the next key set to the start location of the next page. This way a consumer can directly use the result of json["next"] as 3rd parameter to continue where left off.

Up for discussion:

  1. Should the pageSize be configurable in the RpcServer config?
  2. Should the pageSize be configurable as parameter by the invoker?

@shargon
Copy link
Member

shargon commented Jul 27, 2023

@superboyiii could you test it?

@shargon
Copy link
Member

shargon commented Jul 27, 2023

@ixje I vote for 1 option

@ixje
Copy link
Contributor Author

ixje commented Jul 27, 2023

@ixje I vote for 1 option

They're not mutually exclusive. The server could set a maximum and the user could consume it at the maximum size or smaller. The question was whether we should support that or leave it hardcoded as is?

@roman-khimov
Copy link
Contributor

@superboyiii
Copy link
Member

@ixje I vote for 1 option

They're not mutually exclusive. The server could set a maximum and the user could consume it at the maximum size or smaller. The question was whether we should support that or leave it hardcoded as is?

I think it should be set in config.json so can be more flexible.

@superboyiii
Copy link
Member

Tested, works as the expected.

@ixje
Copy link
Contributor Author

ixje commented Aug 2, 2023

@ixje I vote for 1 option

They're not mutually exclusive. The server could set a maximum and the user could consume it at the maximum size or smaller. The question was whether we should support that or leave it hardcoded as is?

I think it should be set in config.json so can be more flexible.

@superboyiii I changed it so the value is configurable in the config only. Let me know if we want to add user control up to the configured maximum as well

superboyiii
superboyiii previously approved these changes Aug 4, 2023
Copy link
Member

@superboyiii superboyiii left a comment

Choose a reason for hiding this comment

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

OK. Now everything is OK for me.

@ixje
Copy link
Contributor Author

ixje commented Aug 4, 2023

OK. Now everything is OK for me.

@shargon ☝️

@cschuchardt88
Copy link
Member

@ixje I think pageSize should configurable in config.json. Or event have this feature be able to be disabled. Reason being is that with tons of Users requesting to find storage it could slow down the server or timeout requests.

@ixje
Copy link
Contributor Author

ixje commented Aug 15, 2023

@ixje I think pageSize should configurable in config.json. Or event have this feature be able to be disabled. Reason being is that with tons of Users requesting to find storage it could slow down the server or timeout requests.

It is now configurable in the config. Completely disabling can be done through the config by adding the method to the DisabledMethods list.

@superboyiii
Copy link
Member

@shargon Review again please.

@ixje ixje requested a review from shargon August 21, 2023 06:13
Jim8y
Jim8y previously approved these changes Aug 21, 2023
@shargon shargon dismissed stale reviews from Jim8y and superboyiii via 62225ef August 21, 2023 09:28
@shargon shargon merged commit 4610b64 into neo-project:master Aug 21, 2023
Jim8y added a commit to Jim8y/neo-modules that referenced this pull request Sep 3, 2023
* 'wss' of github.com:Liaojinghui/neo-modules:
  RpcServer:  added GetContractState by contract id support (neo-project#813)
  rpc: add FindStorage (neo-project#805)
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.

Simplify getting latest state with GetState() or FindStates()
6 participants