upgrade testing supports #9770

turadg · 2024-07-24T23:34:37Z

Description

A bunch of refactors and testing utilities to support #9303.

It also adds a couple upgrade tests of orchestration contracts, but neither of them cover restarting orchestration flows so I'll leave #9755 for that.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

per se

Upgrade Considerations

Helps to test upgrades, should not affect any.

cloudflare-workers-and-pages · 2024-07-25T00:10:20Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`816099a`
Status:	✅ Deploy successful!
Preview URL:	https://fd4e9c44.agoric-sdk.pages.dev
Branch Preview URL:	https://9303-upgrade-testing-support.agoric-sdk.pages.dev

View logs

codecov · 2024-07-25T00:13:05Z

Codecov Report

Attention: Patch coverage is 95.12195% with 6 lines in your changes missing coverage. Please review.

Project coverage is 56.40%. Comparing base (8a8fed3) to head (816099a).

Additional details and impacted files

Components	Coverage Δ
SwingSet/kernel	`73.36% <ø> (ø)`
ERTP	`92.45% <ø> (ø)`
Orchestration	`94.44% <25.00%> (+<0.01%)`	⬆️
swing-store	`95.95% <ø> (ø)`

Files	Coverage Δ
packages/boot/tools/drivers.ts	`100.00% <100.00%> (ø)`
packages/boot/tools/ibc/mocks.js	`100.00% <100.00%> (ø)`
packages/boot/tools/supports.ts	`99.84% <100.00%> (-0.16%)`	⬇️
packages/vats/src/core/lib-boot.js	`85.06% <100.00%> (+0.13%)`	⬆️
...rchestration/src/examples/sendAnywhere.contract.js	`49.05% <0.00%> (ø)`
packages/orchestration/src/utils/zoe-tools.js	`65.43% <50.00%> (+0.43%)`	⬆️
...s/orchestration/src/examples/sendAnywhere.flows.js	`44.06% <20.00%> (ø)`

... and 2 files with indirect coverage changes

dckc

very nice.

I'm not 100% confident that I grok "feat: defer inbound bridge messages" fully but I suppose the tests show that it works.

dckc · 2024-07-25T13:40:53Z

packages/boot/tools/supports.ts

+/**
+ * Elaboration of EVProxy with knowledge of bootstrap space in these tests.
+ */
+type BootstrapEV = EProxy & {


dckc · 2024-07-25T13:46:24Z

packages/zoe/src/contractFacet/types-ambient.d.ts

-  userSeat: ERef<UserSeat>;
+  userSeat: Promise<UserSeat>;


This excludes a remote presence for a UserSeat. That doesn't seem right. But I guess it could be a promise, so any consumers have to await it anyway. so close enough.

ERef is just T | Promise<T> and it's never just T. The remoteness is handled elsewhere.

dckc · 2024-07-25T13:53:44Z

packages/boot/tools/supports.ts

@@ -40,7 +41,7 @@ import type { MsgDelegateResponse } from '@agoric/cosmic-proto/cosmos/staking/v1
 import type { CoreEvalSDKType } from '@agoric/cosmic-proto/swingset/swingset.js';
 import type { EconomyBootstrapPowers } from '@agoric/inter-protocol/src/proposals/econ-behaviors.js';
 import type { SwingsetController } from '@agoric/swingset-vat/src/controller/controller.js';
-import type { BridgeHandler, IBCMethod } from '@agoric/vats';
+import type { BridgeHandler, IBCMethod, IBCPacket } from '@agoric/vats';


The vats package is really bursting at the seams, isn't it?

I wonder what it would cost to move IBCMethod, IBCPacket to network.

dckc · 2024-07-25T13:56:45Z

packages/boot/tools/supports.ts

+                // This is what it used to do but that's not valid because bridge responses
+                // aren't instantaneous.
+                // inbound(BridgeId.DIBC, icaMocks.channelOpenAck(obj));


we want to keep this?

just for code review. I'll drop it

dckc · 2024-07-25T13:59:24Z

packages/boot/test/orchestration/restart-contracts.test.ts

+
+  t.log('restart stakeAtom');
+  await evalProposal(
+    buildProposal('@agoric/builders/scripts/testing/restart-stakeAtom.js'),


dckc · 2024-07-25T14:00:33Z

packages/builders/scripts/testing/restart-sendAnywhere.js

+  // import dynamically so the module can work in CoreEval environment
+  const dspModule = await import('@agoric/deploy-script-support');


interesting... I wonder how this doesn't run afoul of the SES prohibition on dynamic import.

Maybe agoric run isn't locked down so much?

But this whole module is bundled for use as the actual core-eval.

I suppose the import() gets "de-fanged" by deploy-script-support. It would probably fail if we tried to run this part of the code on-chain, but we don't.

0xpatrickdev · 2024-07-25T13:29:06Z

packages/vats/src/core/types-ambient.d.ts

@@ -371,8 +371,10 @@ type ChainBootstrapSpaceT = {
  namesByAddress: import('../types.js').NameHub;
  namesByAddressAdmin: import('../types.js').NamesByAddressAdmin;
  networkVat: NetworkVat;
+  orchestration?: CosmosInterchainService;


Suggested change

orchestration?: CosmosInterchainService;

cosmosInterchainService?: CosmosInterchainService;

orchestrationVat?: OrchestrationVat;

In light of #9668

That's a bigger change than just the typedef

I thought we don't have orchestration in chain bootstrap space anymore? But, these two are there?

0xpatrickdev · 2024-07-25T13:41:45Z

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

 * @param {ZCFSeat} seat
 * @param {{ chainNames: string[] }} offerArgs
 */
 const makePortfolioAcctHandler = async (
  orch,
-  makePortfolioHolder,
+  { makePortfolioHolder },


Had this in mind with the original implementation: #9591 (comment)

But that is platform code and this is an example contract. So, don't see any harm here

0xpatrickdev · 2024-07-25T13:46:56Z

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

+  const cosmosAccount = await remoteChain.makeAccount();
+  return cosmosAccount.asContinuingOffer();


If you find yourself needing to make fixups, mind including this drive-by?

- * Create an account on a Cosmos chain and return a continuing offer with - * invitations makers for Delegate, WithdrawRewards, Transfer, etc. + * Create an OrchestrationAccount for a specific chain and return a continuing + * offer with invitations makers for Delegate, WithdrawRewards, Transfer, etc. * * @satisfies {OrchestrationFlow} * @param {Orchestrator} orch @@ -30,8 +30,8 @@ const makeOrchAccountHandler = async (orch, _ctx, seat, { chainName }) => { seat.exit(); // no funds exchanged mustMatch(chainName, M.string()); const remoteChain = await orch.getChain(chainName); - const cosmosAccount = await remoteChain.makeAccount(); - return cosmosAccount.asContinuingOffer(); + const orchAccount = await remoteChain.makeAccount(); + return orchAccount.asContinuingOffer(); };

0xpatrickdev · 2024-07-25T13:49:01Z

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

-    { makePortfolioHolder },
-    makePortfolioAcctHandler,
-  );
+  const orchFns = orchestrateAll(flows, { makePortfolioHolder });


These have different contexts - so it seems like 2x orchestrate is worth the cost here? Seems like it'll also be good to have examples using both orchestrateAll and orchestrate.

I vacillated on this but I don't see how providing makePortfolioHolder to both is a risk.

I think orchestrateAll is what we should steer people to in general because it gets the durable from a named export, which changing is known to break downstream, whereas the string argument was easily confused as a debugging name.

👍 the second argument is convincing

0xpatrickdev · 2024-07-25T13:57:20Z

packages/boot/tools/supports.ts

+    setTimeout(() => {
+      /**
+       * Mock when Agoric receives the ack from another chain over DIBC. Always
+       * happens after the packet is returned.
+       */
+      inbound(BridgeId.DIBC, msg);
+    });


👍 nice use of setTimeout here

ambient authority grumble...

ambient authority in a mock.

0xpatrickdev · 2024-07-25T13:57:57Z

packages/boot/tools/supports.ts

+    setTimeout(() => {
+      /**
+       * Mock when Agoric receives the ack from another chain over DIBC. Always
+       * happens after the packet is returned.


Should we also make a similar change in:

packages/orchestration/test/network-fakes.ts

packages/vats/tools/fake-bridge.js

I suppose so. WDYT @michaelfig ?

0xpatrickdev · 2024-07-25T14:00:22Z

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

@@ -190,6 +197,7 @@ test.serial('stakeAtom - smart wallet', async t => {
    encoding: 'bech32',
  };

+  // This will trigger the immediate ack of the mock bridge


~~Is "This" related to executeOffer?~~ It seems to be due to ackImmediately. Please consider making this comment more explicit

I tried to word it in a way that wouldn't be false if that code was refactored

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

packages/builders/scripts/testing/restart-stakeAtom.js

fixup! chore(types): userSeat is Remote

turadg requested review from dckc and 0xpatrickdev July 24, 2024 23:34

turadg force-pushed the 9303-upgrade-testing-supports branch from ef9165a to f44917f Compare July 25, 2024 00:10

Agoric deleted a comment from mergify bot Jul 25, 2024

turadg force-pushed the 9303-upgrade-testing-supports branch from f44917f to d374cd2 Compare July 25, 2024 00:16

dckc approved these changes Jul 25, 2024

View reviewed changes

turadg force-pushed the 9303-upgrade-testing-supports branch from d374cd2 to ab8f679 Compare July 25, 2024 14:15

turadg added the automerge:rebase Automatically rebase updates, then merge label Jul 25, 2024

0xpatrickdev approved these changes Jul 25, 2024

View reviewed changes

turadg added automerge:rebase Automatically rebase updates, then merge and removed automerge:rebase Automatically rebase updates, then merge labels Jul 25, 2024

turadg force-pushed the 9303-upgrade-testing-supports branch from ab8f679 to 07cf2e9 Compare July 25, 2024 14:35

turadg added 16 commits July 25, 2024 18:42

test: superfluous t.notThrowsAsync

2b6422e

fix(types):Zoe AdminFacet is far

4accd0a

chore(types): additions to bootstrap space

9a1926f

feat(types): BootstrapEV

d8caa34

chore(types): fixes for type satisfaction

0e4b4b7

chore(types): ZcfSeatKit.userSeat is Promise

683e70e

fixup! chore(types): userSeat is Remote

fix: userSeat is remote

633bee3

refactor: context as record for extensibility

4e5fe93

refactor: name localAccount

da14c1c

chore(types): fix MsgDelegateResponse cast

ae7b73f

refactor: extract basic-flows flows

48b578d

refactor: extract auto-stake-it flows

d2f2e49

test: defer inbound for fidelity

5b8afb7

refactor: ackPacketEvent

7699544

feat: defer inbound bridge messages

ef23bdd

test: queue startChannelOpenInit inbound

5f1f433

turadg added 2 commits July 25, 2024 18:42

test: restarting orchestration contracts

4ca6ee5

refactor: chain agnostic makeOrchAccount

816099a

turadg force-pushed the 9303-upgrade-testing-supports branch from 07cf2e9 to 816099a Compare July 25, 2024 18:42

mergify bot merged commit 0c96ea5 into master Jul 25, 2024
83 checks passed

mergify bot deleted the 9303-upgrade-testing-supports branch July 25, 2024 19:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upgrade testing supports #9770

upgrade testing supports #9770

turadg commented Jul 24, 2024

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

edited

Loading

codecov bot commented Jul 25, 2024 •

edited

Loading

dckc left a comment

dckc Jul 25, 2024

dckc Jul 25, 2024

turadg Jul 25, 2024

dckc Jul 25, 2024

dckc Jul 25, 2024

turadg Jul 25, 2024

dckc Jul 25, 2024

dckc Jul 25, 2024

turadg Jul 25, 2024

dckc Jul 25, 2024

0xpatrickdev Jul 25, 2024

turadg Jul 25, 2024

0xpatrickdev Jul 25, 2024

0xpatrickdev Jul 25, 2024

0xpatrickdev Jul 25, 2024

0xpatrickdev Jul 25, 2024

turadg Jul 25, 2024

0xpatrickdev Jul 25, 2024

0xpatrickdev Jul 25, 2024

dckc Jul 25, 2024

turadg Jul 25, 2024

0xpatrickdev Jul 25, 2024

turadg Jul 25, 2024

0xpatrickdev Jul 25, 2024

turadg Jul 25, 2024

		// import dynamically so the module can work in CoreEval environment
		const dspModule = await import('@agoric/deploy-script-support');

	orchestration?: CosmosInterchainService;
	cosmosInterchainService?: CosmosInterchainService;
	orchestrationVat?: OrchestrationVat;

		const cosmosAccount = await remoteChain.makeAccount();
		return cosmosAccount.asContinuingOffer();

upgrade testing supports #9770

upgrade testing supports #9770

Conversation

turadg commented Jul 24, 2024

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

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

Deploying agoric-sdk with Cloudflare Pages

codecov bot commented Jul 25, 2024 • edited Loading

Codecov Report

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

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

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

edited

Loading

codecov bot commented Jul 25, 2024 •

edited

Loading