Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 207 forks source link

Remove legacy queries before cosmos-sdk 0.47 upgrade #9096

Open JimLarson opened 8 months ago

JimLarson commented 8 months ago

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 commented 8 months ago

Attn @toliaqat @sufyaankhan

JimLarson commented 8 months ago

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

JimLarson commented 8 months ago

Some internal references from code search:

JimLarson commented 8 months ago

More internal code search results from agoric-labs:

JimLarson commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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.