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

codegen RPC clients #9200

Open
Tracked by #9877
turadg opened this issue Apr 5, 2024 · 4 comments · May be fixed by #10409
Open
Tracked by #9877

codegen RPC clients #9200

turadg opened this issue Apr 5, 2024 · 4 comments · May be fixed by #10409
Assignees
Labels
enhancement New feature or request

Comments

@turadg
Copy link
Member

turadg commented Apr 5, 2024

What is the Problem Being Solved?

For a few weeks we had RPC query clients using Telescope:

But the implementation broke using the library in vats, which is more urgent. So this ticket is for restoring that functionality.

Description of the Design

Some options:

  1. publish them as a separate package, versioned by agoric-upgrade-NN
  2. rework Telescope to output two hierarchies with one that is safe for vats
  3. rework Telescope to inject the dependencies instead of importing them (e.g. provide a generic query util, one of which works over HTTP and another that uses the local chain host)
  4. wait for https://github.com/cosmology-tech/interchainjs, probably ready by November 2024

Security Considerations

Scaling Considerations

Test Plan

Upgrade Considerations

@turadg turadg added the enhancement New feature or request label Apr 5, 2024
@michaelfig
Copy link
Member

Can you clarify "broke using the library in vats"? I believe there's at least an option 4:

  1. give instructions as to which @agoric/cosmic-proto modules can be imported within a contract/vat. This is also useful to minimize the amount of dead code bundled with your contract since @endo/bundle-source doesn't yet do tree shaking.

@turadg
Copy link
Member Author

turadg commented Apr 5, 2024

Can you clarify "broke using the library in vats"?

Getting the registry requires enabling amino types in codegen, which entails tendermint-rpc which entails axios and http which is not available in a vat.

E.g. if you revert the last three commits of #9112 and run build:submissions in a3p-integration you'll get,

TypeError#1: Failed to load module "./src/proposals/localchain-test.js" in package "file:///opt/agoric/agoric-sdk/packages/vats/" (11 underlying failures: Cannot find external module "http" in package file:///opt/agoric/agoric-sdk/node_modules/axios/, Cannot find external module "https" in package file:///opt/agoric/agoric-sdk/node_modules/axios/, Cannot find external module "util" in package file:///opt/agoric/agoric-sdk/node_modules/axios/, Cannot find external module "zlib" in package file:///opt/agoric/agoric-sdk/node_modules/axios/, Cannot find external module "stream" in package file:///opt/agoric/agoric-sdk/node_modules/axios/, Cannot find external module "events" in package file:///opt/agoric/agoric-sdk/node_modules/axios/, Cannot find external module "stream" in package file:///opt/agoric/agoric-sdk/node_modules/axios/, Cannot find external module "util" in package file:///opt/agoric/agoric-sdk/node_modules/axios/, Cannot find external module "stream" in package file:///opt/agoric/agoric-sdk/node_modules/axios/, Cannot find external module "stream" in package file:///opt/agoric/agoric-sdk/node_modules/axios/, Cannot find external module "url" in package file:///opt/agoric/agoric-sdk/node_modules/axios/

I've pushed that as #9202 so you can see it in CI.

Even a targeted import of a module has the same problem,

import { MsgDelegate } from '@agoric/cosmic-proto/cosmos/staking/v1beta1/tx';

This is why I enumerated option 3 to unencumber the codec modules with the client dependencies. I don't see how we could get much use out of detailed instructions while the modules mix dependencies in this way.

@turadg turadg self-assigned this Apr 8, 2024
@michaelfig
Copy link
Member

Thanks for the clear answer. I agree: my option 4 won't get us anywhere.

0xpatrickdev added a commit that referenced this issue Jul 17, 2024
- helper for building an ibc transfer message with @cosmjs/stargate signing client
- hardcodes timeoutTimestamp until #9200, as we experienced weird flakes in CI
0xpatrickdev added a commit that referenced this issue Jul 17, 2024
- helper for building an ibc transfer message with @cosmjs/stargate signing client
- hardcodes timeoutTimestamp until #9200, as we experienced weird flakes in CI
mergify bot added a commit that referenced this issue Jul 22, 2024
closes: #9578

## Description

The reason for #9578 was an import from `@agoric/cosmic-proto` in a vat instead of `@agoric/cosmic-proto/vatsafe`. That's quite an easy error to make (a footgun) so this makes the top level module export be the "vat safe" one.

When consumers want the convenience of a barrel export or rpc clients that can resolve anything (#9200), they can use a different subpath.

### Security Considerations
none

### Scaling Considerations
can reduce bundle size

### Documentation Considerations
reduces need to document

### Testing Considerations
existing coverage

### Upgrade Considerations
affects only new bundles
This was referenced Nov 6, 2024
mergify bot added a commit that referenced this issue Nov 6, 2024
closes: #10168

## Description
Provide a new package, `client-utils`, as a home for utilities that are useful to clients of an Agoric chain. This doesn't currently use `@agoric/rpc` but over time some of it may be pushed down into that package. 

Related work…

- #9200 
This will be where those client factories are kept.

- #10369

This will solve most of what we need for functional testing. Some aspects are specific to the A3P synthetic chain (like account addresses and references to history) but most of what the tests are doing with the chain are operations that any client might do.

- #9109
- #8963

This will contribute to those goals.


### Security Considerations

Reduces authority needed to query chain (from child_process to fetch)

### Scaling Considerations
This is a big package, but it's not to be run on chain. Most client apps use some form of code shaking so they'll only take what they need.

### Documentation Considerations
Once this settles down it ought to be a part of docs.agoric.com

### Testing Considerations
The only test yet is a live one to make sure a query to Emerynet for Swingset params succeeds as expected, even under SES.

No package CI yet. Mostly it's refactoring of existing code so those uses serve as coverage. I do think this would benefit from some additional testing.

### Upgrade Considerations
Will never be on chain
@turadg
Copy link
Member Author

turadg commented Nov 7, 2024

The scope of this is codegen for read-only queries. Transaction signing support can build on this and I'll track that in #5912

mergify bot added a commit that referenced this issue Nov 8, 2024
closes: #10369

## Description

Many of the imports from `@agoric/synthetic-chain` are not about synthetics per se. They should be possible with the code in agoric-sdk.

This brings the new `@agoric/client-utils` into functional testing in a3p-integration. It makes the canonical source for `sync-tool.js` that was duplicated in different proposal tests.

This also demonstrates a style of smart-wallet offer execution without the `@agoric/synthetic-chain` package. See `OpenVault`. It still goes through `execSwingsetTransaction` spawning `agd` but that could also be optimized with an RPC client lib for SwingSet that does signing (see #9200). That's out of scope for this effort to simply stop reliable in another repo for testing utilities.

Review by commit is recommended

### Security Considerations
n/a, tests

### Scaling Considerations
n/a, tests

### Documentation Considerations
When these patterns stabilize more we may want to write them down. For now I think let's keep it flexible.

### Testing Considerations
per se

### Upgrade Considerations
One impact to the upgrade process is that when moving a proposal from a3p-integration to agoric-3-proposals, the dependencies may have to be updated. Though it's likely they'll work on the `/usr/src/agoric-sdk` in the image
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

Successfully merging a pull request may close this issue.

2 participants