test: continuing offers in orchestrate async-flows #9679

0xpatrickdev · 2024-07-10T05:36:19Z

Description

Adds basic-flow.contract.js, proposal, and builder for testing orchestrate (async-flow) continuing invitations to facilitate testing. These are also the first bootstrap + multichain tests with orchestrate / async-flow.
Updates asContinuingInvitation and getPublicTopics methods to return Vows to satisfy membrane constraints
- Ensures storagePath, part of ContinuingOfferResult is a string instead of a Promise/Vow so smart-wallet can easily handle this and the async-flow membrane is appeased (no promises)
Updates chain facade implementations, ensuring each account has it's own storage node (towards orchestration - ICA Controller - Account - vstorage logging #9066)

Testing Considerations

Includes unit, bootstrap, and multichain (e2e) tests for an async-flow contract. E2E ensure accounts can be created on agoric, cosmos, and osmosis and return continuing invitations.

Upgrade Considerations

This did not require any changes to smart-wallet. There is a change to PublicTopicShape to accept a promise or a string (previously just a promise), but this is only in contracts relaying on smart-wallet, not smart-wallet itself.

cloudflare-workers-and-pages · 2024-07-10T05:37:20Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`b7a9ed3`
Status:	✅ Deploy successful!
Preview URL:	https://e82ffca3.agoric-sdk.pages.dev
Branch Preview URL:	https://pc-9673-sync-continuing-offe.agoric-sdk.pages.dev

View logs

0xpatrickdev · 2024-07-10T16:08:57Z

packages/orchestration/src/examples/kitchen-sink.contract.js

+ * @file Primarily a testing fixture, but also serves as an example of how to
+ *   leverage basic functionality of the Orchestration API with async-flow.
+ */


Feedback welcome if anyone can think of a better name than kitchen-sink

A kitchen sink will be some effort to maintain. Whenever we add some functionality we'll test it in a targeted fashion and have to also include it here.

If the scope is to test async flow, maybe "stress-flows" or "basic-flows"

Thanks, I will go with basic-flows. Will make this change in the final rebase after ✅ if that's ok

0xpatrickdev · 2024-07-10T16:10:30Z

multichain-testing/yarn.lock

@@ -1938,26 +1931,6 @@ __metadata:
  languageName: node
  linkType: hard

-"eslint-plugin-prettier@npm:^5.1.3":


How / when can we make the Multichain E2E Testing job part of force-integration?

@toliaqat, happen to know how we can add Multichain E2E Testing job to force-integration?

Based on history, successful runs are all under 17 mins and failed runs take several minutes.

Thanks @LuqiPan @toliaqat. I am sending this PR to the merge queue, but interested to see where we land on this.

turadg · 2024-07-10T17:00:39Z

packages/orchestration/src/examples/kitchen-sink.contract.js

+ * @file Primarily a testing fixture, but also serves as an example of how to
+ *   leverage basic functionality of the Orchestration API with async-flow.
+ */


A kitchen sink will be some effort to maintain. Whenever we add some functionality we'll test it in a targeted fashion and have to also include it here.

If the scope is to test async flow, maybe "stress-flows" or "basic-flows"

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

packages/boot/test/bootstrapTests/orchestration.test.ts

multichain-testing/test/kitchen-sink.test.ts

turadg · 2024-07-10T17:16:14Z

packages/orchestration/src/examples/kitchen-sink.contract.js

+  mustMatch(chainName, M.string());
+  const remoteChain = await orch.getChain(chainName);
+  const cosmosAccount = await remoteChain.makeAccount();
+  // @ts-expect-error asContinuingOffer does not exist on OrchestrationAccountI


why not? you did add it to orchestrationAccountMethods. Those lists must be the same:

agoric-sdk/packages/orchestration/src/utils/orchestrationAccount.js

Lines 11 to 22 in 21742bd

/** @see {OrchestrationAccountI} */

export const orchestrationAccountMethods = {

getAddress: M.call().returns(ChainAddressShape),

getBalance: M.call(M.any()).returns(Vow$(CoinShape)),

getBalances: M.call().returns(Vow$(M.arrayOf(CoinShape))),

send: M.call(ChainAddressShape, AmountArgShape).returns(VowShape),

transfer: M.call(AmountArgShape, ChainAddressShape)

.optional(M.record())

.returns(VowShape),

transferSteps: M.call(AmountArgShape, M.any()).returns(VowShape),

asContinuingOffer: M.call().returns(

Vow$({

I meant to come back to this. Addressed in the batch of fixups via orchestration-api.ts.

Feedback on the JS Doc comments is appreciated. Our docs are a bit light on these subjects but this seemed like a good place to over-explain a little.

packages/orchestration/src/exos/cosmos-orchestration-account.js

turadg · 2024-07-10T17:19:24Z

packages/orchestration/src/exos/cosmos-orchestration-account.js

+              account: {
+                description: PUBLIC_TOPICS.account[0],
+                subscriber: topicKit.subscriber,
+                // eslint-disable-next-line @jessie.js/safe-await-separator


please include justification

Perhaps too naively, we don't care if this crosses a turn boundary. We don't expect the answer to storagePath to change.

I was a little intimidated by the phrasing in the eslint config and have reverted:

agoric-sdk/.eslintrc.cjs

Lines 111 to 112 in df05b07

// that are used by multiple clients. So we enable it throughout the repo

// and aim for no exceptions.

packages/orchestration/src/exos/local-orchestration-account.js

turadg · 2024-07-10T20:15:46Z

multichain-testing/test/kitchen-sink.test.ts

+    contractName: 'kitchenSink',
+    wallet: accounts[0],
+  };
+  test.serial(`Create account on ${chainName}`, makeAccountScenario, scenario);


consider using the test.macro's title maker and just passing the literals needed to lookup the rest. Then you don't need a loop. e.g.

test.serial(makeAccountScenario, 'agoric'); test.serial(makeAccountScenario, 'cosmoshub'); test.serial(makeAccountScenario, 'osmosis');

Thanks for the push to refactor the test. It led to simplifying the contract!

turadg · 2024-07-10T20:17:21Z

packages/orchestration/src/orchestration-api.ts

+  asContinuingOffer: () => Promise<ContinuingOfferResult>;
+  /**
+   * Public topics are a map to different vstorage paths and subscribers that
+   * can be shared with on or offchain clients.
+   * When returned as part of a continuing invitation, it will appear
+   * in the {@link CurrentWalletRecord} in vstorage.
+   */
+  getPublicTopics: () => Promise<TopicsRecord>;


these are top level API changes that should have @dtribble approval. I approve and will leave to you how/when to sync with Dean

packages/zoe/src/contractSupport/topics.js

- previously, each orch account was given the contracts storage node. this creates a child node per account - refs #9066, helping ensure orch account addresses are available for smart-wallet clients. left some suggestions in code comments for further improvements

- update PublicTopic.storagePath on async-flow endowments to return a string instead of a promise. Uses asVow with async function since it's reasonable to assume the remote call will resolve promptly (in the same run) - update asContinuingOffer to return a vow and unwrap the result of getPublicTopic since this is also a vow. same promptness considerations mentioned above apply - closes: #9673

- seem like it was missed in #9664, but not caught since this job runs after merges to master

- tests a contract that uses orchestrate/async-flow in the multichain-testing environment

- fixes changed parameter from #9662 - improves test to print delegate offer result error on failure

mhofman · 2024-07-11T01:58:34Z

packages/zoe/src/contractSupport/topics.js

@@ -7,7 +7,7 @@ export { SubscriberShape };
 export const PublicTopicShape = M.splitRecord(
  {
    subscriber: SubscriberShape,
-    storagePath: M.promise(/* string */),
+    storagePath: M.or(M.promise(/* string */), M.string()),


As we discussed, changing this shape is safe against upgrade as it couldn't have been used as a durable state of collection shape before because durably storing promises is not possible.

0xpatrickdev force-pushed the pc/9673-sync-continuing-offer branch 2 times, most recently from 10264b8 to a404135 Compare July 10, 2024 14:40

0xpatrickdev changed the title ~~chore: continuing offers in orchestrate async-flows~~ test: continuing offers in orchestrate async-flows Jul 10, 2024

0xpatrickdev commented Jul 10, 2024

View reviewed changes

0xpatrickdev marked this pull request as ready for review July 10, 2024 16:42

0xpatrickdev requested review from turadg and mhofman July 10, 2024 16:42

0xpatrickdev mentioned this pull request Jul 10, 2024

feat: auto-stake-it example contract #9666

Merged

turadg reviewed Jul 10, 2024

View reviewed changes

0xpatrickdev requested a review from turadg July 10, 2024 20:09

turadg approved these changes Jul 10, 2024

View reviewed changes

0xpatrickdev force-pushed the pc/9673-sync-continuing-offer branch from 7376204 to 4087da7 Compare July 10, 2024 22:59

0xpatrickdev added 6 commits July 10, 2024 18:59

chore: add .asContinuingOffer to LOA holder

bc12cbd

test: basic-flows.contract.js for testing async-flow continuing offers

8330fba

feat(contractSupport): storagePath pattern accepts string or promise

f425141

chore: update yarn.lock

033a2f8

- seem like it was missed in #9664, but not caught since this job runs after merges to master

0xpatrickdev force-pushed the pc/9673-sync-continuing-offer branch from 4087da7 to 13a66e0 Compare July 10, 2024 23:03

0xpatrickdev added 2 commits July 10, 2024 20:12

test: e2e test of basic-flows.contract.js

8d8820b

- tests a contract that uses orchestrate/async-flow in the multichain-testing environment

chore: ChainAddress.value

b7a9ed3

- fixes changed parameter from #9662 - improves test to print delegate offer result error on failure

0xpatrickdev force-pushed the pc/9673-sync-continuing-offer branch from 13a66e0 to b7a9ed3 Compare July 11, 2024 00:12

0xpatrickdev added the automerge:rebase Automatically rebase updates, then merge label Jul 11, 2024

mergify bot merged commit 12ce494 into master Jul 11, 2024
87 checks passed

mergify bot deleted the pc/9673-sync-continuing-offer branch July 11, 2024 01:18

mhofman reviewed Jul 11, 2024

View reviewed changes

0xpatrickdev mentioned this pull request Jul 12, 2024

bump multichain helm chart to latest #9697

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: continuing offers in orchestrate async-flows #9679

test: continuing offers in orchestrate async-flows #9679

0xpatrickdev commented Jul 10, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Jul 10, 2024 •

edited

Loading

0xpatrickdev Jul 10, 2024

turadg Jul 10, 2024

0xpatrickdev Jul 10, 2024

0xpatrickdev Jul 10, 2024

LuqiPan Jul 10, 2024

0xpatrickdev Jul 11, 2024

turadg Jul 10, 2024

turadg Jul 10, 2024

0xpatrickdev Jul 10, 2024

turadg Jul 10, 2024

0xpatrickdev Jul 10, 2024

turadg Jul 10, 2024

0xpatrickdev Jul 10, 2024

turadg Jul 10, 2024

mhofman Jul 11, 2024

	/** @see {OrchestrationAccountI} */
	export const orchestrationAccountMethods = {
	getAddress: M.call().returns(ChainAddressShape),
	getBalance: M.call(M.any()).returns(Vow$(CoinShape)),
	getBalances: M.call().returns(Vow$(M.arrayOf(CoinShape))),
	send: M.call(ChainAddressShape, AmountArgShape).returns(VowShape),
	transfer: M.call(AmountArgShape, ChainAddressShape)
	.optional(M.record())
	.returns(VowShape),
	transferSteps: M.call(AmountArgShape, M.any()).returns(VowShape),
	asContinuingOffer: M.call().returns(
	Vow$({

	// that are used by multiple clients. So we enable it throughout the repo
	// and aim for no exceptions.

test: continuing offers in orchestrate async-flows #9679

test: continuing offers in orchestrate async-flows #9679

Conversation

0xpatrickdev commented Jul 10, 2024 • edited Loading

Description

Testing Considerations

Upgrade Considerations

cloudflare-workers-and-pages bot commented Jul 10, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

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

0xpatrickdev commented Jul 10, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Jul 10, 2024 •

edited

Loading