test: a docker test for the zoe/zcf upgrade #8018

Chris-Hibbert · 2023-07-06T21:21:16Z

refs: #6678

Description

This is a Docker test to validate that the upgrade will work correctly on chain, and to build the necessary scripts.

Security Considerations

This is about deployment.

Scaling Considerations

N/A

Documentation Considerations

N/A

Testing Considerations

This is a test.

dckc

didn't finish my review today, but I'll share my thoughts so far

dckc · 2023-07-19T22:07:52Z

yarn.lock

@@ -2831,6 +2831,14 @@ better-sqlite3@^8.2.0:
    bindings "^1.5.0"
    prebuild-install "^7.1.0"

+better-sqlite3@^8.4.0:


isn't better-sqlite3 already in our yarn.lock? Are we introducing another version of it or something?

dckc · 2023-07-19T22:19:21Z

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/actions.sh

+
+
+echo XXXX install bundles XXXXXX
+for f in $here/zoe-full-upgrade/*bundle.json; do


I see this PR adds the bundles generated from the current git version. I can see why we might need to check in bundles from some older git version, but for the current version, I think we can do:

yarn bundle-source --cache-json /tmp packages/vats/src/vat-zoe.js Zoe-upgrade ZOE_HASH=`jq -r .endoZipBase64Sha512 /tmp/bundle-Zoe-upgrade.json` yarn bundle-source --cache-json /tmp packages/zoe/src/contractFacet/vatRoot.js Zcf-upgrade ZCF_HASH=`jq -r .endoZipBase64Sha512 /tmp/bundle-Zcf-upgrade.json`

and then assert that $ZOE_HASH matches the one from zcf-upgrade-script.js

These bundles were actually (and crucially) generated in #7966 (effectively, a newer version), which has the changes to Zoe and ZCF. This PR sits on top of a branch without those changes, so we can't build them here.

I guess you're right that I could rebase this branch on top of #7966, and build the way you describe. deployment/upgrade-test provides an immutable environment that isn't affected by the checked-out code under agoric-sdk.

What #7969 wants to do can only be done in a branch that doesn't contain the Zoe/ZCF changes, but this branch's goals don't require that isolation.

dckc · 2023-07-19T22:27:48Z

...t/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/zoe-full-upgrade/zcf-upgrade-script.js

+// import { E } from "@endo/far";
+
+const ZCF_BUNDLE_ID =
+  'b1-8674abc9a8de561c4a33fb475b87be75708cd901c37931fd5ac1f40d3ee99937a459a6ca7b4a8b7907512626caf98c125f22c15384826e37dfc899dc0bf2a63a';


I get c4b3d8bf11cfbcc90ced7e1a8a936d368098360bb01aa4c70a750a39f56dc9d87c10547e825585ce11ceeca5b24d7093bdef04843ac8ac6e90e661b1235bf69b

when I do:

yarn bundle-source --cache-json /tmp packages/zoe/src/contractFacet/vatRoot.js Zcf-upgrade export ZCF_HASH=`jq -r .endoZipBase64Sha512 /tmp/bundle-Zcf-upgrade.json`

It's not a surprise that they don't match what you get when you build here, since this branch doesn't have the Zoe/ZCF changes. But I'll rebase to a place that does as you suggested above, and we can make that work.

I was also presuming there might be changes to libraries, etc. that would effect the hash, and expected to do a final re-build before submitting the merge request.

packages/deployment/package.json

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/actions.sh

Chris-Hibbert

There are instructions in agoric-upgrade-11/actions.sh for building the bundles.

packages/deployment/package.json

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/actions.sh

Chris-Hibbert · 2023-07-19T23:33:57Z

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/actions.sh

+
+
+echo XXXX install bundles XXXXXX
+for f in $here/zoe-full-upgrade/*bundle.json; do


These bundles were actually (and crucially) generated in #7966 (effectively, a newer version), which has the changes to Zoe and ZCF. This PR sits on top of a branch without those changes, so we can't build them here.

I guess you're right that I could rebase this branch on top of #7966, and build the way you describe. deployment/upgrade-test provides an immutable environment that isn't affected by the checked-out code under agoric-sdk.

What #7969 wants to do can only be done in a branch that doesn't contain the Zoe/ZCF changes, but this branch's goals don't require that isolation.

Chris-Hibbert · 2023-07-19T23:36:55Z

...t/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/zoe-full-upgrade/zcf-upgrade-script.js

+// import { E } from "@endo/far";
+
+const ZCF_BUNDLE_ID =
+  'b1-8674abc9a8de561c4a33fb475b87be75708cd901c37931fd5ac1f40d3ee99937a459a6ca7b4a8b7907512626caf98c125f22c15384826e37dfc899dc0bf2a63a';


It's not a surprise that they don't match what you get when you build here, since this branch doesn't have the Zoe/ZCF changes. But I'll rebase to a place that does as you suggested above, and we can make that work.

I was also presuming there might be changes to libraries, etc. that would effect the hash, and expected to do a final re-build before submitting the merge request.

Chris-Hibbert · 2023-07-19T23:37:13Z

yarn.lock

@@ -2831,6 +2831,14 @@ better-sqlite3@^8.2.0:
    bindings "^1.5.0"
    prebuild-install "^7.1.0"

+better-sqlite3@^8.4.0:


turadg

Excellent. Thanks for the thorough comments

turadg · 2023-07-21T16:20:02Z

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/actions.sh

@@ -5,5 +5,55 @@
 # CWD is agoric-sdk
 here=./upgrade-test-scripts/agoric-upgrade-11

-# run zoe thru "null upgrade"
-$here/zoe-upgrade/zoe-upgrade-driver.sh
+# Pre-steps:


helpful docs

turadg · 2023-07-21T16:20:50Z

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/actions.sh

+# Finish
+#  * create instance of prober contract and run expecting to see atomicRearrange
+
+echo XXXX fill wallet XXXXXX


XXX elsewhere means tech debt. please user a different character. Or just upcase.

Suggested change

echo XXXX fill wallet XXXXXX

echo FILL WALLET

turadg · 2023-07-21T16:23:55Z

packages/deployment/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/actions.sh

@@ -5,5 +5,55 @@
 # CWD is agoric-sdk
 here=./upgrade-test-scripts/agoric-upgrade-11


this precedes this PR, but I want to say I think we can have a better name for this. eg. thisupgrade. or find a way to evaluate the scripts letting them use file path relative references. maybe if we adopt zx or execa.

turadg · 2023-07-21T16:24:50Z

...nt/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/zoe-full-upgrade/run-prober-script.js

@@ -0,0 +1,95 @@
+// to turn on ts-check:


??? did you mean to use @ts-check ?

bad copy-paste from prior test.

turadg · 2023-07-21T16:25:19Z

...nt/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/zoe-full-upgrade/run-prober-script.js

+
+console.info('running zcf prober');
+
+const sub = (a, v) => {


what happens if you import AmountMath?

AIUI, imports aren't supported in core-eval proposals.

We have a mechanism to support imports. You used it in #7969: packages/vats/src/proposals/zcf-proposal.js starts with import { E } from '@endo/far';.

But it's a complex mechanism, and E is in the environment of a core-eval script even without that import. I tend to do a certain amount of copy-and-paste before I go for the whole writeCoreProposal thing.

I tend to do a certain amount of copy-and-paste before I go for the whole writeCoreProposal thing.

@dckc I'm not positive how to interpret this. Should I change something here?

turadg · 2023-07-21T16:25:54Z

...nt/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/zoe-full-upgrade/run-prober-script.js

+};
+
+/*
+ * Test a full upgrade of Zzoe and ZCF.


Took me a couple of passes to see it. Thx.

turadg · 2023-07-21T16:27:18Z

...t/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/zoe-full-upgrade/zcf-upgrade-driver.sh

+set -euo pipefail
+
+# This shows how to upgrade Zoe and ZCF on a running chain. It presumes that
+# The bundles for Zoe and ZCF have been installed, and their hashes updated in


Suggested change

# The bundles for Zoe and ZCF have been installed, and their hashes updated in

# the bundles for Zoe and ZCF have been installed, and their hashes updated in

turadg · 2023-07-21T16:28:02Z

...t/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/zoe-full-upgrade/zcf-upgrade-driver.sh

+# zcf-upgrade-script.js. The bundles can be generated by running the test in
+# vats/test/bootstrapTests/test-zcf-upgrade.js, noting the logged ZOE BUNDLE ID
+# and ZCF BUNDLE ID, and finding the corresponding bundles in ~/.agoric/cache.
+#  !!   THERE HAS TO BE A BETTER WAY   !!


there will be! we just have to design and implement it :)

There are slightly clearer instructions now in actions.sh.

dckc

I haven't been able to reproduce the bundles.

I'd like to avoid checking them in at all.

turadg · 2023-07-24T15:24:31Z

...nt/upgrade-test/upgrade-test-scripts/agoric-upgrade-11/zoe-full-upgrade/run-prober-script.js

 /* global E */

-// import { E } from "@endo/far";
+// to turn on ts-check:


I don't get it. Why disable ts-check if this is all it takes to enable it? Oh, does the import fail at runtime? if that's it then please enable ts-check and do a type-only import.

/** @typedef {import('@endo/far').E} E */

@dckc I'm just cargo-culting here. What's your advice on best practices?

type-only import

TIL. Nifty.

But at runtime, the string import( cannot occur anywhere in the script, even in comments. See rejectImportExpressions

Conservatively reject the source text if it may contain a dynamic
import expression. To reject without parsing, rejectImportExpressions will
also reject some other text as well.

oh... this import( limitation is another thing that's addressed by the "cadillac method", using writeCoreProposal from @agoric/deploy-script-support:

agoric-sdk/packages/deploy-script-support/src/code-gen.js

Line 20 in 8bcf6af

export const defangEvaluableCode = code =>

by way of documentation, I started...

Why does import cause errors in a permissioned upgrade (CoreEval) script? Is there a better way? · Agoric/agoric-sdk · Discussion #8087

Chris-Hibbert · 2023-07-24T20:23:18Z

I haven't been able to reproduce the bundles. I'd like to avoid checking them in at all.

Let's pair to come up with a working alternative.

dckc

The blank check in zcf-upgrade-permit.json is worth filling in.

dckc · 2023-07-25T18:18:19Z

...upgrade-test/upgrade-test-scripts/agoric-upgrade-11/zoe-full-upgrade/zcf-upgrade-permit.json

@@ -0,0 +1 @@
+true


That's a blank check for zcf-upgrade-permit.json to use all powers in the bootstrap space. We neglected to go back and fill in the actual amount. Based on what upgradeZoeAndZcf uses from its argument, that seems to be:

Suggested change

true

{

"consume": { "vatStore": true, "vatAdminSvc": true }

}

dckc

chore: generate upgrade bundles and check hashes

test: a docker test for the zoe/zcf upgrade

Chris-Hibbert added Zoe package: Zoe test contract-upgrade labels Jul 6, 2023

Chris-Hibbert self-assigned this Jul 6, 2023

Chris-Hibbert force-pushed the 6678-upgradeZoeZcf-docker branch from bb381af to 5e1e4bd Compare July 6, 2023 21:49

Chris-Hibbert force-pushed the 6678-upgradeZoeZcf-docker branch 3 times, most recently from ee9a305 to 891ee25 Compare July 18, 2023 00:50

Chris-Hibbert force-pushed the 6678-upgradeZoeZcf-transplant branch from 606ad23 to 1e6d52e Compare July 19, 2023 20:54

Chris-Hibbert force-pushed the 6678-upgradeZoeZcf-docker branch from 891ee25 to 66cd691 Compare July 19, 2023 20:57

Chris-Hibbert requested review from turadg and dckc July 19, 2023 21:47

dckc reviewed Jul 19, 2023

View reviewed changes

Chris-Hibbert force-pushed the 6678-upgradeZoeZcf-docker branch from 66cd691 to 39150df Compare July 20, 2023 23:54

Chris-Hibbert changed the base branch from 6678-upgradeZoeZcf-transplant to 6678-test-upgradeZoeZcf July 20, 2023 23:55

Chris-Hibbert commented Jul 20, 2023

View reviewed changes

turadg approved these changes Jul 21, 2023

View reviewed changes

dckc requested changes Jul 21, 2023

View reviewed changes

Chris-Hibbert mentioned this pull request Jul 21, 2023

test: a test framework for verifying upgrade of Zoe and ZCF #7966

Merged

turadg reviewed Jul 24, 2023

View reviewed changes

dckc force-pushed the 6678-upgradeZoeZcf-docker branch from 1ce7f19 to bd043a9 Compare July 24, 2023 23:44

Chris-Hibbert force-pushed the 6678-upgradeZoeZcf-docker branch 2 times, most recently from 63009af to a5a339e Compare July 25, 2023 18:07

Chris-Hibbert requested a review from dckc July 25, 2023 18:07

dckc requested changes Jul 25, 2023

View reviewed changes

dckc approved these changes Jul 25, 2023

View reviewed changes

Chris-Hibbert force-pushed the 6678-upgradeZoeZcf-docker branch from 4b6a7cb to 18c0640 Compare July 26, 2023 00:06

Chris-Hibbert force-pushed the 6678-test-upgradeZoeZcf branch from 6bea479 to 5813535 Compare July 27, 2023 20:42

Base automatically changed from 6678-test-upgradeZoeZcf to master July 28, 2023 01:11

Chris-Hibbert force-pushed the 6678-upgradeZoeZcf-docker branch from 756c8e5 to 9eb583a Compare July 28, 2023 16:31

Chris-Hibbert enabled auto-merge July 28, 2023 16:31

Chris-Hibbert added this pull request to the merge queue Jul 28, 2023

Chris-Hibbert removed this pull request from the merge queue due to a manual request Jul 28, 2023

test: a docker test for the zoe/zcf upgrade

1261732

chore: generate upgrade bundles and check hashes

Chris-Hibbert force-pushed the 6678-upgradeZoeZcf-docker branch from 9eb583a to 1261732 Compare July 28, 2023 17:25

Chris-Hibbert enabled auto-merge July 28, 2023 17:25

Chris-Hibbert added this pull request to the merge queue Jul 28, 2023

Merged via the queue into master with commit 29ef60f Jul 28, 2023
62 of 64 checks passed

Chris-Hibbert deleted the 6678-upgradeZoeZcf-docker branch July 28, 2023 18:20

This was referenced Jul 28, 2023

test: a test framework for verifying upgrade of Zoe and ZCF #7969

Closed

Revert "test: a docker test for the zoe/zcf upgrade" #8110

Closed

mhofman mentioned this pull request Jul 29, 2023

fix(deployment): localize script to upgrade #8111

Merged

mhofman pushed a commit that referenced this pull request Aug 7, 2023

Merge pull request #8018 from Agoric/6678-upgradeZoeZcf-docker

29a25c3

test: a docker test for the zoe/zcf upgrade



		echo XXXX install bundles XXXXXX
		for f in $here/zoe-full-upgrade/*bundle.json; do

		@@ -5,5 +5,55 @@
		# CWD is agoric-sdk
		here=./upgrade-test-scripts/agoric-upgrade-11

	# The bundles for Zoe and ZCF have been installed, and their hashes updated in
	# the bundles for Zoe and ZCF have been installed, and their hashes updated in

-true
+{
+  "consume": { "vatStore": true, "vatAdminSvc": true }
+}

test: a docker test for the zoe/zcf upgrade #8018

test: a docker test for the zoe/zcf upgrade #8018

Conversation

Chris-Hibbert commented Jul 6, 2023

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

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

Chris-Hibbert 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

turadg 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

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

dckc Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chris-Hibbert commented Jul 24, 2023

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

dckc left a comment

Choose a reason for hiding this comment

dckc Jul 24, 2023 •

edited

Loading