feat: plumb `icqConnection` through `orchestrate` facade #9927

0xpatrickdev · 2024-08-19T23:01:19Z

closes: #9890

Description

ensure CosmosOrchAccount is given an icqConnection if ChainConfig.icqEnabled is true
ensure RemoteChainFacade has .query() method available
~~[ ] ensure LocalChainFacade has .query() method available~~ see feat: plumb localchain.queryMany through orchestrate facade #9935

Security Considerations

n/a

Scaling Considerations

n/a

Documentation Considerations

n/a

Testing Considerations

Includes unit tests. Multichain tests would be a nice addition, but I wouldn't consider them a blocker for the PR.

Upgrade Considerations

n/a, code is currently unreleased

cloudflare-workers-and-pages · 2024-08-19T23:02:44Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`8c28e67`
Status:	🚫 Build failed.

View logs

packages/orchestration/src/exos/remote-chain-facade.js

0xpatrickdev · 2024-08-20T16:12:33Z

packages/orchestration/src/orchestration-api.ts

@@ -88,6 +89,8 @@ export interface Chain<CI extends ChainInfo> {
  makeAccount: () => Promise<OrchestrationAccount<CI>>;
  // FUTURE supply optional port object; also fetch port object

+  query: CI extends { icqEnabled: true } ? ICQQueryFunction : never;


never is maybe not correct here since all chain objects will have .query(). If it's called and icqEnabled = false, it will throw

This looks right to me. It's saying that if icqEnabled then .query should have type never and so never be used.

turadg

I did a review. I'll make the request changes too.

packages/orchestration/src/examples/basic-flows.flows.js

turadg · 2024-08-21T19:29:57Z

packages/orchestration/src/exos/icq-connection-kit.js

@@ -89,20 +85,17 @@ export const prepareICQConnectionKit = (zone, { watch, when }) =>
        },
        /**
         * @param {JsonSafe<RequestQuery>[]} msgs
-         * @returns {Promise<JsonSafe<ResponseQuery>[]>}
-         * @throws {Error} if packet fails to send or an error is returned


too badd there's no @rejects or whatever. Shouldn't this doc still inform the user of that?

turadg · 2024-08-21T19:31:08Z

packages/orchestration/src/exos/local-chain-facade.js

@@ -107,6 +108,9 @@ const prepareLocalChainFacadeKit = (
            this.facets.makeAccountWatcher,
          );
        },
+        query() {
+          return asVow(() => Fail`not yet implemented`);


Suggested change

return asVow(() => Fail`not yet implemented`);

// TODO https://github.com/Agoric/agoric-sdk/pull/9935

return asVow(() => Fail`not yet implemented`);

turadg · 2024-08-21T19:32:47Z

packages/orchestration/src/exos/remote-chain-facade.js

+            if (!icqEnabled) {
+              throw Fail`Queries not available for chain ${chainId}`;
+            }
+            // if none exists, make one + still send the query in the handler


this reads like "make (one + …)". it's only two chars:

Suggested change

// if none exists, make one + still send the query in the handler

// if none exists, make one and still send the query in the handler

turadg · 2024-08-21T19:34:00Z

packages/orchestration/src/exos/remote-chain-facade.js

+
+          return makeCosmosOrchestrationAccount(
+            chainAddress,
+            // @ts-expect-error stakingDenom already asserted


not in this scope. things can run in unexpected orders. better to assert again.

turadg · 2024-08-21T19:35:47Z

packages/vow/src/tools.js

@@ -18,7 +18,7 @@ import { makeWhen } from './when.js';
 * @param {object} [powers]
 * @param {IsRetryableReason} [powers.isRetryableReason]
 */
-export const prepareVowTools = (zone, powers = {}) => {
+export const prepareVowToolsForTesting = (zone, powers = {}) => {


needs rebasing on master after #9932

- with the refactor of remote-chain-facade.js, sometimes the icqConn resolves before the account. this change ensures the first account in each test context is cosmos1test and not cosmos1test1

- ensure CosmosOrchestrationAccountKit is given icqConnection if chainConfig has icqEnabled - ensure Chain.query() sends queries if chainConfig has icqEnabled - lazily provideICQConnection on first request and cache result in state - refs: #9890

- with #9927, the makeAccount flow might also involve making an ICQ connection. This change increases the timeout for makeAccount to account for the additional work/latency.

0xpatrickdev commented Aug 19, 2024

View reviewed changes

packages/orchestration/src/exos/remote-chain-facade.js Outdated Show resolved Hide resolved

0xpatrickdev commented Aug 19, 2024

View reviewed changes

packages/orchestration/src/exos/remote-chain-facade.js Outdated Show resolved Hide resolved

0xpatrickdev commented Aug 19, 2024

View reviewed changes

packages/orchestration/src/exos/remote-chain-facade.js Outdated Show resolved Hide resolved

0xpatrickdev commented Aug 20, 2024

View reviewed changes

0xpatrickdev force-pushed the 9890-queries-in-orchestrate branch 5 times, most recently from 2fa9230 to a3319b8 Compare August 21, 2024 15:23

0xpatrickdev changed the base branch from master to mfig-vows-default-to-swingset August 21, 2024 15:23

0xpatrickdev force-pushed the 9890-queries-in-orchestrate branch from a3319b8 to 1c7ec01 Compare August 21, 2024 16:00

0xpatrickdev marked this pull request as ready for review August 21, 2024 16:33

0xpatrickdev requested review from turadg and dtribble August 21, 2024 17:06

michaelfig force-pushed the mfig-vows-default-to-swingset branch from f2a2895 to a513b30 Compare August 21, 2024 17:51

turadg self-assigned this Aug 21, 2024

michaelfig force-pushed the mfig-vows-default-to-swingset branch from 05eb6f0 to 63f0294 Compare August 21, 2024 19:10

turadg reviewed Aug 21, 2024

View reviewed changes

turadg force-pushed the 9890-queries-in-orchestrate branch from 8bf62d9 to 85a8fbf Compare August 21, 2024 19:50

turadg added the automerge:rebase Automatically rebase updates, then merge label Aug 21, 2024

turadg approved these changes Aug 21, 2024

View reviewed changes

Base automatically changed from mfig-vows-default-to-swingset to master August 21, 2024 21:08

0xpatrickdev added 6 commits August 21, 2024 22:10

feat: icqConnection.query returns a vow

8a2f60e

test: track ica accounts separately from ibc channels

1cd5ca6

- with the refactor of remote-chain-facade.js, sometimes the icqConn resolves before the account. this change ensures the first account in each test context is cosmos1test and not cosmos1test1

chore: IBCMethod<'bindPort'> type

27aaf9e

types: Chain.query() type

7cbf3ca

test: icqConnection flow is restartable in async-flow

8c28e67

0xpatrickdev force-pushed the 9890-queries-in-orchestrate branch from 85a8fbf to 8c28e67 Compare August 22, 2024 02:10

mergify bot merged commit 6d97bc4 into master Aug 22, 2024
79 of 80 checks passed

mergify bot deleted the 9890-queries-in-orchestrate branch August 22, 2024 03:31

0xpatrickdev added a commit that referenced this pull request Aug 27, 2024

test: increase makeAccount timeout

483ce3d

- with #9927, the makeAccount flow might also involve making an ICQ connection. This change increases the timeout for makeAccount to account for the additional work/latency.

0xpatrickdev mentioned this pull request Aug 27, 2024

multichain-e2e: minimize ci flakes #9815

Closed

0xpatrickdev added a commit that referenced this pull request Aug 27, 2024

test: increase makeAccount timeout

7e65a8c

- with #9927, the makeAccount flow might also involve making an ICQ connection. This change increases the timeout for makeAccount to account for the additional work/latency.

0xpatrickdev mentioned this pull request Aug 27, 2024

test: increase attempts for rewards available condition #9974

Merged

0xpatrickdev added a commit that referenced this pull request Aug 27, 2024

test: increase makeAccount timeout

c3d2968

- with #9927, the makeAccount flow might also involve making an ICQ connection. This change increases the timeout for makeAccount to account for the additional work/latency.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: plumb `icqConnection` through `orchestrate` facade #9927

feat: plumb `icqConnection` through `orchestrate` facade #9927

0xpatrickdev commented Aug 19, 2024 •

edited by turadg

Loading

cloudflare-workers-and-pages bot commented Aug 19, 2024 •

edited

Loading

0xpatrickdev Aug 20, 2024

turadg Aug 21, 2024

turadg left a comment

turadg Aug 21, 2024

turadg Aug 21, 2024

turadg Aug 21, 2024

turadg Aug 21, 2024

turadg Aug 21, 2024

	return asVow(() => Fail`not yet implemented`);
	// TODO https://github.com/Agoric/agoric-sdk/pull/9935
	return asVow(() => Fail`not yet implemented`);

	// if none exists, make one + still send the query in the handler
	// if none exists, make one and still send the query in the handler

feat: plumb icqConnection through orchestrate facade #9927

feat: plumb icqConnection through orchestrate facade #9927

Conversation

0xpatrickdev commented Aug 19, 2024 • edited by turadg Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

cloudflare-workers-and-pages bot commented Aug 19, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

0xpatrickdev Aug 20, 2024

Choose a reason for hiding this comment

turadg Aug 21, 2024

Choose a reason for hiding this comment

turadg left a comment

Choose a reason for hiding this comment

turadg Aug 21, 2024

Choose a reason for hiding this comment

turadg Aug 21, 2024

Choose a reason for hiding this comment

turadg Aug 21, 2024

Choose a reason for hiding this comment

turadg Aug 21, 2024

Choose a reason for hiding this comment

turadg Aug 21, 2024

Choose a reason for hiding this comment

feat: plumb `icqConnection` through `orchestrate` facade #9927

feat: plumb `icqConnection` through `orchestrate` facade #9927

0xpatrickdev commented Aug 19, 2024 •

edited by turadg

Loading

cloudflare-workers-and-pages bot commented Aug 19, 2024 •

edited

Loading