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

Remove legacy queries before cosmos-sdk 0.47 upgrade #9096

Open
JimLarson opened this issue Mar 15, 2024 · 7 comments
Open

Remove legacy queries before cosmos-sdk 0.47 upgrade #9096

JimLarson opened this issue Mar 15, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@JimLarson
Copy link
Contributor

JimLarson commented Mar 15, 2024

What is the Problem Being Solved?

The cosmos-sdk 0.47 upgrade planned for agoric-upgrade-16 (#8225) removes support for "legacy queries". These are recognized by use of the RPC endpoint (e.g. main.rpc.agoric.net or port :26657) for the abci_query method with a "path" parmeter that starts with /custom.

Queries should transition to using gRPC directly via the gRPC endpoint, or the REST-gRPC gateway via the API endpoint. It is possible to access gRPC via the RPC endpoint by using abci_query with a gRCP path (the same path used for the REST gateway), but this is not recommended due to the higher overhead and greater locking required.

Description of the Design

An access via a legacy query looks like:

$ curl \
    --header "Content-Type: application/json" \
    --request POST \
    --data '{
                    "method": "abci_query", 
                    "params": {
                        "path": "/custom/vstorage/children/published.committees", 
                        "data": ""
                     }, 
                     "id": 1}' \
  https://emerynet.rpc.agoric.net/

which returns an encoded result like

{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "response": {
      "code": 0,
      "log": "",
      "info": "",
      "index": "0",
      "key": null,
      "value": "ewogICJjaGlsZHJlbiI6IFsKICAgICJFY29ub21pY19Db21taXR0ZWUiLAogICAgImtyZWFkLWdvdiIKICBdCn0=",
      "proofOps": null,
      "height": "4282742",
      "codespace": ""
    }
  }
}

Changing this to a gRPC-encoded path would require encoding of the argument in addition to decoding the response:

$ curl \
  --header "Content-Type: application/json" \
  --request POST \
  --data '{
                  "method": "abci_query", 
                  "params": {
                      "path": "/agoric.vstorage.Query/Children", 
                      "data": "0a147075626c69736865642e636f6d6d697474656573"}, 
                      "id": 1, 
                      "height": 4282839}' \
  https://emerynet.rpc.agoric.net/

which again returns encoded results

{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "response": {
      "code": 0,
      "log": "",
      "info": "",
      "index": "0",
      "key": null,
      "value": "ChJFY29ub21pY19Db21taXR0ZWUKCWtyZWFkLWdvdg==",
      "proofOps": null,
      "height": "4282839",
      "codespace": ""
    }
  }
}

So instead we recommend a REST query against the API endpoint:

$ curl \
    -H "x-cosmos-block-height: 14575536" \
    https://main.api.agoric.net/agoric/vstorage/children/published.committees

which returns the easily-parseable response

{
  "children": [
    "Economic_Committee",
    "kread-gov"
  ],
  "pagination": null
}

Security Considerations

N/A

Scaling Considerations

gRPC queries are preferred due to the low overhead. REST queries (via the API endpoint) are preferred over RPC queries due to the lower overhead and use of the proper HTTP operation (GET vs POST), though not as good as gRPC.

Test Plan

Each specific call site should test its modified query.

Upgrade Considerations

Queriers can and should upgrade before agoric-upgrade-16.

@JimLarson JimLarson added the enhancement New feature or request label Mar 15, 2024
@JimLarson
Copy link
Contributor Author

Attn @toliaqat @sufyaankhan

@JimLarson
Copy link
Contributor Author

Notes on vstorage queries in particular: https://github.com/Agoric/agoric-sdk/tree/master/golang/cosmos/x/vstorage

@JimLarson
Copy link
Contributor Author

JimLarson commented Mar 16, 2024

@JimLarson
Copy link
Contributor Author

JimLarson commented Mar 18, 2024

alexesDev added a commit to p2p-org/p2p-agoric-vstorage-viewer that referenced this issue Mar 19, 2024
@JimLarson
Copy link
Contributor Author

Quick note on querying at a historical block height. Example from cosmos-sdk docs:

curl \
    -X GET \
    -H "Content-Type: application/json" \
    -H "x-cosmos-block-height: 123" \
    http://localhost:1317/cosmos/bank/v1beta1/balances/$MY_VALIDATOR_ADDRESS

@JimLarson
Copy link
Contributor Author

Since agoric-labs/KREAd seems to be an unmodifed fork of Kryha's version, and Kryha-develop branch tip doesn't seem to contain a legacy query, I don't think we need to update our stale fork - it should be synced instead. But someone might want to contact Kryha in case their active version still has it.

@JimLarson
Copy link
Contributor Author

Filed #9145 for ensuring the KREAd frontend gets upgraded - if necessary.

Confirmed that @warner created our fork in agoric-labs for performance testing and that it will be upgraded by syncing when needed. Modifying the agoric-labs/KREAd code is not a task blocking upgrade-16.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants