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

Re-design datastore key queries ( JSON RPC API / GRPC) #4721

Open
3 tasks
Leo-Besancon opened this issue Jul 8, 2024 · 7 comments · May be fixed by #4809
Open
3 tasks

Re-design datastore key queries ( JSON RPC API / GRPC) #4721

Leo-Besancon opened this issue Jul 8, 2024 · 7 comments · May be fixed by #4809
Assignees
Labels

Comments

@Leo-Besancon
Copy link
Collaborator

Leo-Besancon commented Jul 8, 2024

The JsonRPC API endpoint get_addresses lists, in the response, all the datastore keys of the given addresses.
One issue with this is that there is no limits in the number of datastore entries for a given address. So if the address has a lot of keys, the response may be too large (e.g. larger than the default 50Mo response limit of the API) which causes issues.

For example, the following explorer page errors out because of it: https://explorer.massa.net/mainnet/address/AS1Ba1T2mMpHvLEhTvsNkSDqm4djno2ezVojGrtLf1wRd8ThfZD3
A request / response to the mainnet API showing the issue:

{
    "jsonrpc": "2.0",
    "id": 1,
    "method": "get_addresses",
    "params": [["AS1Ba1T2mMpHvLEhTvsNkSDqm4djno2ezVojGrtLf1wRd8ThfZD3"]]
}
{
    "jsonrpc": "2.0",
    "error": {
        "code": -32008,
        "message": "Response is too big",
        "data": "Exceeded max limit of 52428800"
    },
    "id": 1
}

TODO:

  • List all the current ways we list datastore key (json rpc API, GRPC)
  • Design a unified way to list the keys
    • Use (Bound, Bound) to define the range of keys to query. This is a way to generalize the optional prefix in some of the listing cases, with Bounds being Included, Excluded or Unbounded.
    • Limit the number of keys returned (max num would be a constant, or maybe a config parameter instead when applicable)
    • Best if non-breaking
  • Implement and test all the key listing cases
@SlnPons
Copy link
Collaborator

SlnPons commented Nov 21, 2024

Note priority N°1 is to address API JSONRPC+GRPC.

@SlnPons SlnPons added the p2 label Nov 21, 2024
@damip
Copy link
Member

damip commented Dec 5, 2024

How it would work:

  • optional key_start parameter that can be either
    • absent (no lower bound)
    • INCLUDED(key)
    • EXCLUDED(key)
  • same for optional key_end parameter:
    • absent (no upper bound)
    • INCLUDED(key)
    • EXCLUDED(key)
  • optional LIMIT parameter (default 100) and error if the user tries to set it higher than 500

@peterjah
Copy link
Collaborator

peterjah commented Dec 13, 2024

  • It would be nice to have the ability to provide a filter when quering keys
  • maybe the get_addresses endpoint is not the good place to get storage keys? maybe it would be cleaner to create a new one "get_datastore_keys"
  • the "get_datastore_entries" return should also be modified to return a key/value list. because today you provide a list of key in input and receive a list of key in output, but you have to pray that the ordering is kept unchanged and there is no missing values. having key/value pairs as result would be safer and cleaner

@SlnPons SlnPons added p1 and removed p2 labels Jan 3, 2025
@modship
Copy link
Member

modship commented Jan 3, 2025

  • QueryState
    • AddressDatastoreKeysCandidate -> Execution.get_final_and_candidate_datastore_keys()
    • AddressDatastoreKeysFinal -> Execution.get_final_and_candidate_datastore_keys()
  • massa-api :
    • get_addresses() -> Execution.get_addresses_infos()
  • interfaceImpl :
    • get_keys() -> Context.get_keys()
    • get_keys_for() -> Context.get_keys()
    • get_ds_keys_wasmv1() -> Context.get_keys()

  • Context:
    • get_keys() -> SpeculativeLedger.get_key()

  • SpeculativeLedger
    • get_keys() -> Ledger.get_datastore_keys()
  • Execution
    • get_addresses_infos() -> Execution.get_final_and_candidate_datastore_keys()
    • get_final_and_candidate_datastore_keys() -> Ledger.get_datastore_keys()

@damip
Copy link
Member

damip commented Jan 4, 2025

@modship for now we focus only on the jsonrpc and grpc APIs. Let's not do the ABI part for now

@modship
Copy link
Member

modship commented Jan 6, 2025

@modship for now we focus only on the jsonrpc and grpc APIs. Let's not do the ABI part for now

So, why not just re-using https://github.com/massalabs/massa/blob/main/massa-api-exports/src/page.rs ?
Like

async fn get_stakers(

@damip
Copy link
Member

damip commented Jan 6, 2025

@modship we cannot know the total count nor list all matching keys internally for performance reasons

@modship modship linked a pull request Jan 6, 2025 that will close this issue
8 tasks
@modship modship self-assigned this Jan 6, 2025
@modship modship linked a pull request Jan 15, 2025 that will close this issue
1 task
@damip damip changed the title Re-design datastore key queries (unify between JSON RPC API / GRPC / ABI AS / ABI WASMV1) Re-design datastore key queries ( JSON RPC API / GRPC) Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants