neo-project / neo-modules

MIT License
60 stars 100 forks source link

rpc: add FindStorage #805

Closed ixje closed 1 year ago

ixje commented 1 year ago

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 commented 1 year ago

@superboyiii could you test it?

shargon commented 1 year ago

@ixje I vote for 1 option

ixje commented 1 year ago

@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 commented 1 year ago

Just FYI, a paging extension to getnepXXtransfers in NeoGo: https://github.com/nspcc-dev/neo-go/blob/master/docs/rpc.md#limits-and-paging-for-getnep11transfers-and-getnep17transfers

superboyiii commented 1 year ago

@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 commented 1 year ago

Tested, works as the expected.

ixje commented 1 year ago

@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

ixje commented 1 year ago

OK. Now everything is OK for me.

@shargon ☝️

cschuchardt88 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@shargon Review again please.