new client-utils package #10407

turadg · 2024-11-06T00:18:31Z

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…

codegen RPC clients #9200
This will be where those client factories are kept.
make a functional testing package #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.

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

samsiegart · 2024-11-06T00:40:00Z

packages/agoric-cli/test/inter-cli.test.js

@@ -2,11 +2,11 @@
 /* eslint-env node */
 import '@endo/init';


Have you tried @endo/init/legacy.js? I'd expect axios to work with that, because @agoric/rpc itself already uses axios. It would still be great to get your patch into axios itself

Thank you! That will help so much with #9200.

samsiegart

LGTM for now. I am curious to know if we can make this in typescript, but I think most of this code so far is taken from existing js files so I'm not going to request it be rewritten.

turadg · 2024-11-06T01:30:32Z

I am curious to know if we can make this in typescript

It would require a build step. #9200 will require a build step for the codegen so it wouldn't be a huge change after that.

cloudflare-workers-and-pages · 2024-11-06T01:33:52Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`c3f5438`
Status:	✅ Deploy successful!
Preview URL:	https://a74c0988.agoric-sdk.pages.dev
Branch Preview URL:	https://10168-client-utils.agoric-sdk.pages.dev

View logs

dckc

Thanks for getting this moving.

After a bit of thinking-out-loud (mostly marked suggestion), I tried to focus my review on the package boundaries. A number of requests regard dependencies such as governance and inter-protocol that, as far as I can see, aren't used.

My most substantive request is probably around doing I/O at module-load-time for networkConfig.

packages/client-utils/src/wallet-utils.js

dckc · 2024-11-06T03:52:04Z

packages/client-utils/src/chain.js

+ * @returns {Promise<StargateClient>}
+ */
+export const makeStargateClient = config =>
+  StargateClient.connect(pickEndpoint(config));


Is it important to export makeStargateClient?

StargateClient.connect uses ambient access to the nest unless you pass in an endpoint object instead of a string.
(In particular, it uses axios, which seems a little silly now that fetch is mature.)

suggestion: Perhaps apply the pattern from #10038 ? I suppose we can do that later.

It's necessary for the pollBlocks feature of WalletUtils. Passing an Endpoint object doesn't help because that's just {url, headers}.

I think I've got it working with something from casting (so I added that dep back)

dckc · 2024-11-06T15:34:57Z

packages/client-utils/package.json

+    "@agoric/casting": "^0.4.2",
+    "@agoric/cosmic-proto": "*",
+    "@agoric/ertp": "^0.16.2",
+    "@agoric/governance": "^0.10.3",


I don't see any occurrences of governance in this new package.

request:

Suggested change

"@agoric/governance": "^0.10.3",

dckc · 2024-11-06T15:35:28Z

packages/client-utils/package.json

+    "@agoric/cosmic-proto": "*",
+    "@agoric/ertp": "^0.16.2",
+    "@agoric/governance": "^0.10.3",
+    "@agoric/inter-protocol": "^0.16.1",


nor inter-protocol.

request:

Suggested change

"@agoric/inter-protocol": "^0.16.1",

dckc · 2024-11-06T16:19:31Z

packages/client-utils/test/snapshots/exports.test.js.md

+      'offerStatusTuples',
+      'pickEndpoint',
+      'purseBalanceTuples',
+      'rpcUrl',


What rpcUrl does is reasonable, but the name suggests something more general purpose and there's no docstring.

suggestion: Rename it to include agoricNet or the like? Add a docstring?

dckc · 2024-11-06T16:26:43Z

packages/client-utils/README.md

+- GUI (such as dapps)
+- Tests (such as a3p-integration tests)
+
+As such the modules cannot assume they're running in Node. There are some ambient authorities in common in the above environments (e.g. `setTimeout`) but a further constraint is that these modules will not export ambient authority. Instead they will provide interfaces that are ergonomic for creating empowered objects in the client context.


👏 good to set this direction, even though we're not quite there yet.

dckc · 2024-11-06T16:35:53Z

packages/client-utils/src/rpc.js

@@ -48,6 +48,9 @@ export const getNetworkConfig = async env => {
  });
 };

+// TODO distribute load


suggestion: Make this comment part of the API docs?

Suggested change

// TODO distribute load

/**

* TODO distribute load

*/

consider @deprecated too.

why? it's to be used to pick an endpoint from a config

why: because without documentation, the caller would reasonably expect it to make use of >1 endpoint

dckc · 2024-11-06T16:38:10Z

packages/client-utils/test/exports.test.js

+import * as index from '@agoric/client-utils';
+
+test('index', t => {
+  t.snapshot(Object.keys(index).sort());


👏 This is very handy.

dckc · 2024-11-06T16:42:03Z

packages/cosmic-proto/scripts/codegen.cjs

@@ -1,3 +1,4 @@
+/* eslint-env node */


suggestion to follow convention:

Suggested change

/* eslint-env node */

#!/usr/bin/env node

/* eslint-env node */

dckc

Looks good.

I can live with not resolving our differences on dependency management.

incidental ## Description A couple days ago #10407 merged a new package, `@agoric/client-utils`. It wasn't marked private, but it didn't have `publishConfig` so the job that publishes to NPM was failing, ``` E402 You must sign up for private packages ``` This adds the necessary `publishConfig`. It also includes a couple other pending cleanups. ### Security Considerations Publishing another package. It's under the `@agoric` org so it can't be squatted. ### Scaling Considerations n/a ### Documentation Considerations n/a ### Testing Considerations We might want to have caught this problem in the PR but I don't think it's worth the effort. It was caught soon enough. ### Upgrade Considerations n/a

turadg requested review from samsiegart and 0xpatrickdev November 6, 2024 00:18

turadg requested a review from a team as a code owner November 6, 2024 00:18

samsiegart reviewed Nov 6, 2024

View reviewed changes

This was referenced Nov 6, 2024

9200 codegen client #10409

Draft

A3P commands without synthetic-chain #10396

Merged

samsiegart approved these changes Nov 6, 2024

View reviewed changes

turadg added automerge:rebase Automatically rebase updates, then merge and removed automerge:rebase Automatically rebase updates, then merge labels Nov 6, 2024

dckc requested changes Nov 6, 2024

View reviewed changes

turadg requested a review from dckc November 6, 2024 19:00

dckc approved these changes Nov 6, 2024

View reviewed changes

turadg added 16 commits November 6, 2024 12:06

lint: eslint-env node

5108ab1

build: smart-wallet doesn't need casting

e73afcb

chore(types): less ambient

769de1f

feat: client-utils package

50af71f

build: emit types

3026d97

refactor: format lib to client-utils

63c7850

chore: remove Natural

64bf826

chore: WalletUtils type

bd97f95

lint: cleanup imports

8582373

feat: makeWalletUtils wo/spawn

20083ae

refactor: pickEndpoint

5762418

refactor: network-config ocap module

55fd3d1

refactor: format lib back to agoric-cli

4ea3ec0

chore: deprecations

1563f0f

feat: one marshaller per WalletUtils

b141ce6

chore(types): more annotations

085db85

turadg added 3 commits November 6, 2024 12:12

feat: makeTendermintRpcClient

129516a

feat: ocap makeStargateClient

c8f7407

test: exports list

c3f5438

turadg force-pushed the 10168-client-utils branch from 6855062 to c3f5438 Compare November 6, 2024 20:15

turadg added the automerge:rebase Automatically rebase updates, then merge label Nov 6, 2024

mergify bot merged commit d4f2864 into master Nov 6, 2024
91 checks passed

mergify bot deleted the 10168-client-utils branch November 6, 2024 20:52

turadg mentioned this pull request Nov 9, 2024

fix publishing to NPM #10436

Merged

dckc mentioned this pull request Dec 9, 2024

identity-preserving marshallers for vstorage testing, clients etc. #10491

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new client-utils package #10407

new client-utils package #10407

turadg commented Nov 6, 2024 •

edited

Loading

samsiegart Nov 6, 2024

turadg Nov 6, 2024

samsiegart left a comment

turadg commented Nov 6, 2024

cloudflare-workers-and-pages bot commented Nov 6, 2024 •

edited

Loading

dckc left a comment

dckc Nov 6, 2024

turadg Nov 6, 2024

turadg Nov 6, 2024

dckc Nov 6, 2024

turadg Nov 6, 2024

dckc Nov 6, 2024

turadg Nov 6, 2024

dckc Nov 6, 2024

dckc Nov 6, 2024

dckc Nov 6, 2024

turadg Nov 6, 2024

dckc Nov 6, 2024

dckc Nov 6, 2024

dckc Nov 6, 2024

dckc left a comment

	/* eslint-env node */
	#!/usr/bin/env node
	/* eslint-env node */

new client-utils package #10407

new client-utils package #10407

Conversation

turadg commented Nov 6, 2024 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samsiegart left a comment

Choose a reason for hiding this comment

turadg commented Nov 6, 2024

cloudflare-workers-and-pages bot commented Nov 6, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

dckc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc left a comment

Choose a reason for hiding this comment

turadg commented Nov 6, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Nov 6, 2024 •

edited

Loading