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

KREAd frontend should remove legacy queries #4

Open
otoole-brendan opened this issue Mar 28, 2024 · 1 comment
Open

KREAd frontend should remove legacy queries #4

otoole-brendan opened this issue Mar 28, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@otoole-brendan
Copy link

Description

Creating this issue in Kread repo to match Agoric/agoric-sdk#9145

In the upcoming agoric-upgrade-16, we plan to upgrade our cosmos-sdk major version to 0.47. This version of cosmos-sdk removes support for "legacy queries", which had been deprecated since version 0.45. Our codebase originally demonstrated use of legacy queries as the mechanism for reading "vstorage", and this pattern had been copied by some versions of the KREAd frontend code.

Proposed Solution

Proposed Solution
The Kryha/KREAd frontend appear to have eliminated legacy queries on at least recent version of the development branch. However, we are uncertain if that is the version currently running in production.
This ticket is to ensure that the public-facing KREAd frontend does not use legacy queries, whether that involves development, deployment, or if it's already done this ticket can be closed.
See Agoric/agoric-sdk#9096 for more information on legacy queries and how to switch to alternative query methods.

Acceptance criteria

Confirmation legacy queries not used

Additional info

No response

@otoole-brendan otoole-brendan added the enhancement New feature or request label Mar 28, 2024
@Jorge-Lopes
Copy link
Owner

Jorge-Lopes commented Apr 2, 2024

Conclusion

The current state of KREAd running in production, commit 1f281a8 , the agoric/rpc pkg being imported has the version "^0.6.0", package.json.
This confirms that the KREAd version running in production still uses legacy queries.

Note: the same is true for the develop branch, package.json.

Context

At the PR #40 , were the rpc pkg is upgraded to version 0.6.0 , the ChainStorageWatcher executes a method used to query the vstorage called batchVstorageQuery, where we can see that the structure passed as option to the fetch method is the same as described as legacy queries on the issue #9096

  const options = {
    method: 'POST',
    body: JSON.stringify(
      paths.map((path, index) => ({
        jsonrpc: '2.0',
        id: index,
        method: 'abci_query',
        params: { path: `/custom/vstorage/${path[0]}/${path[1]}` },
      })),
    ),
  };

Source

On the PR #55 the batchVstorageQuery is update to use the JSON API instead of the deprecated RPC method.

On the PR #65, the batchQuery file is replaced with vstorageQuery , which still use the expected JSON API to execute the queries.
This feature is present from the @agoric/rpc version: 0.7.2 and forward.

The latest version is 0.9.0

WietzeSlagman pushed a commit to Kryha/KREAd that referenced this issue Apr 8, 2024
## Description

The purpose of this PR is to update the KREAd frontend code to use the
JSON API instead of the deprecated RPC method to query the Vstorage.

With this in mind, it was necessary to update the `@agoric/rpc` and
`@agoric/web-components` packages, and the respective imported functions
being used by KREAd.

Changes made to original code:
- At commit 5c6bdaf the packages
mentioned above are updated to the latest version.
- At commit 42f886f 
- the `fetchChainInfo` function is updated to return the `api` along
with the `rpc` and `chainName`
- the `connectAgoric` function is updated to pass the `rpc` as argument
to `makeAgoricWalletConnection`
- the `startWatching` function is updated to pass the `api` as argument
to `makeAgoricChainStorageWatcher`
- At commit fbeeedb on all calls of the
`watchLatest` method of `chainStorageWatcher`, the `onError` argument
was removed, since it is no longer expected.

## Related Issues

Fixes the following issues:
- [#4](Jorge-Lopes#4)
- [#9145](Agoric/agoric-sdk#9145)

## Checklist

Make sure all items are checked before submitting the pull request.
Remove any items that are not applicable.

- [x] The PR title is clear and concise.
- [x] Are there changes in the /fronted folder? Make sure `cd frontend
&& yarn build` runs successfully.;
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

2 participants