8605 a3p integration #8635

turadg · 2023-12-07T23:58:38Z

closes: #8605

Description

My goal is to have one CLI for these different use cases,

testing an upgrade handler in agoric-sdk
testing a proposal in a dapp repo
testing forthcoming proposals in agoric-3-proposals
rebuilding synthetic history as needed

This relies on the new,

synthetic-chain package and CLI agoric-3-proposals#46

Security Considerations

n/a

Scaling Considerations

n/a

Documentation Considerations

Once the UI settles, we should document this in a guide.

Testing Considerations

CI and also ran locally on M1.

Upgrade Considerations

n/a

Chris-Hibbert · 2023-12-15T01:34:57Z

a3p-integration/proposals/a:upgrade-next/use.sh

+# UNTIl this is upstream https://github.com/Agoric/agoric-3-proposals/issues/40
+# Set to zero so tests don't have to pay gas (we're not testing that)
+sed --in-place=.bak s/'minimum-gas-prices = ""'/'minimum-gas-prices = "0ubld,0uist"'/ ~/.agoric/config/app.toml


Suggested change

# UNTIl this is upstream https://github.com/Agoric/agoric-3-proposals/issues/40

# Set to zero so tests don't have to pay gas (we're not testing that)

sed --in-place=.bak s/'minimum-gas-prices = ""'/'minimum-gas-prices = "0ubld,0uist"'/ ~/.agoric/config/app.toml

# UNTIL this is upstream https://github.com/Agoric/agoric-3-proposals/issues/40

# Set to zero so tests don't have to pay gas (we're not testing that)

sed -i .bak -e 's/minimum-gas-prices = ""/minimum-gas-prices = "0ubld,0uist"/' ~/.agoric/config/app.toml

-e says the next arg is an expression. Single quotes can capture the entire substitution expression, which can then include spaces and doublequotes. --in-place=.bak didn't work on my version of sed. does -i work for you?

-e says the next arg is an expression. Single quotes can capture the entire substitution expression, which can then include spaces and doublequotes.

Sound like you think that would read better. I'll make those changes.

--in-place=.bak didn't work on my version of sed. does -i work for you?

Your version like on your Mac? IIRC it didn't work in Ubuntu, where this line runs.

Your version like on your Mac?

Yeah. nothing special, just what's here.

a3p-integration/proposals/b:wf2/README.md

mhofman

I just did a quick pass, far from being thorough. At this point I think I'm misaligned on the approach taken here. Let's discuss directly.

While I love the idea of a framework to run proposal layers on top of a3p, I don't understand the direction taken. I see why some shared tool is needed to ease the generation and execution of Docker image layers, but I don't understand why this tool should live in agoric-sdk. It is conceptually closer to a3p than agoric-sdk, and moving this logic back here seems like a regression.

I would recommend moving this tool to the a3p repo. In the future we could rename that repo if we want to detach it a little more from the agoric-3 chain (it could be a monorepo with the core tool, and different chain's images being built using the tooling).

I definitely want to find a solution to avoid the duplication of support libraries between the core tool/repo and consumer projects like agoric-sdk and dapps. My thinking is that the support library should be bundled in the base image, and proposals would just have it as a sort of "peer" dependency. I'm not sure of the specific hoops we'll have to jump through to make that work however.

packages/synthetic-chain/README.md

packages/synthetic-chain/src/cli/dockerfileGen.ts

a3p-integration/.gitignore

turadg · 2024-01-10T00:42:24Z

This deletes zoe-full-upgrade without replacing it. Is there a plan to duplicate it to the new a3p-integration context?

I thought it was dead code. The test is marked in master as failing: https://github.com/Agoric/agoric-sdk/blob/master/packages/deployment/upgrade-test/upgrade-test-scripts/unreleased-upgrade/post.test.js#L4

I could copy the files over but since it won't pass that seems like noise to me. I propose the best course of action is that when we want it to be passing, we pull up the code from master's history. E.g. https://github.com/Agoric/agoric-sdk/tree/82d35483fc88a1ec5c873ead92c215bba1372731/packages/deployment/upgrade-test/upgrade-test-scripts/unreleased-upgrade

mhofman

Nice to see the docker integration tests cleaned up. Mostly some nits.

I'm not sure the docker test will pass however, we'll see.

mhofman · 2024-01-08T18:58:00Z

a3p-integration/package.json

+    "private": true,
+    "scripts": {
+        "build": "echo Use synthetic-chain to build proposal images",
+        "test": "echo Use synthetic-chain to test proposal images"


can't this run synthetic-chain test ?

Yeah, I just thought it better to train people on the tool rather than creating another level of abstraction.

mhofman · 2024-01-10T01:30:07Z

a3p-integration/proposals/a:upgrade-next/post.test.js

+test('Ensure MaxBytes param was updated', async t => {
+  const { value: rawParams } = await agd.query(
+    'params',
+    'subspace',
+    'baseapp',
+    'BlockParams',
+  );
+  const blockParams = JSON.parse(rawParams);
+  t.is(blockParams.max_bytes, '5242880');
+});


Oh this probably should be removed since it should be upstream already (it's not testing this simulated upgrade anymore)

mhofman · 2024-01-10T01:32:30Z

a3p-integration/proposals/a:upgrade-next/test.sh

It feels weird to not be actually doing any test here, but I suppose we're not yet able to do any core proposal on master yet, and no chain software changes are happening either.

Could we at the very least maintain the tests checking the zoe and network vat incarnation number?

mhofman · 2024-01-10T01:35:48Z

.github/workflows/integration.yml

-      - name: docker run upgrade final stage
-        run: docker run --env "DEST=0" docker-upgrade-test:latest
+        run: make docker-build-sdk
+        working-directory: ./agoric-sdk/packages/deployment


AFAIK the repo is not checkout out in a agoric-sdk sub folder for these tests

which introduces 'parseArgs' in node:utils

8605 a3p integration

mhofman · 2024-01-15T14:41:24Z

packages/deployment/upgrade-test/Dockerfile

-# UPGRADE
-FROM ${BASE_IMAGE} as propose-unreleased-upgrade
-# TODO: Replace with actual Zoe core proposal for vat upgrades (MCS, Kread, Zoe, restart-contracts, etc)
-ARG UPGRADE_INFO='{"coreProposals":["@agoric/builders/scripts/vats/init-network.js"]}'


The new tests don't simulate a network core proposal anymore

8605 a3p integration

turadg force-pushed the 8605-local-a3p branch 6 times, most recently from 4f043e3 to a9adced Compare December 13, 2023 00:02

turadg requested review from mhofman and dckc December 13, 2023 00:08

turadg marked this pull request as ready for review December 13, 2023 00:08

turadg force-pushed the 8605-local-a3p branch 3 times, most recently from 55952c4 to 57d0a65 Compare December 13, 2023 19:20

turadg mentioned this pull request Dec 13, 2023

feat(deploy-script-support): Write out bundle file names in machine r… #8559

Merged

turadg force-pushed the 8605-local-a3p branch from 57d0a65 to fba3e3d Compare December 13, 2023 19:28

turadg requested a review from michaelfig December 13, 2023 19:32

turadg force-pushed the 8605-local-a3p branch from fba3e3d to 6501295 Compare December 13, 2023 19:48

This was referenced Dec 13, 2023

synthetic-chain package and CLI Agoric/agoric-3-proposals#46

Merged

Wallet factory upgrade 2 branch #8593

Merged

Chris-Hibbert reviewed Dec 15, 2023

View reviewed changes

mhofman reviewed Dec 15, 2023

View reviewed changes

packages/synthetic-chain/README.md Outdated Show resolved Hide resolved

packages/synthetic-chain/src/cli/dockerfileGen.ts Outdated Show resolved Hide resolved

a3p-integration/.gitignore Outdated Show resolved Hide resolved

turadg marked this pull request as draft December 15, 2023 17:09

turadg force-pushed the 8605-local-a3p branch from 9583ac6 to 7d232f7 Compare January 7, 2024 01:31

turadg changed the title ~~8605 local a3p~~ 8605 a3p integration Jan 8, 2024

turadg marked this pull request as ready for review January 8, 2024 16:28

turadg requested review from mhofman and Chris-Hibbert January 8, 2024 16:28

turadg mentioned this pull request Jan 8, 2024

implement appending Agoric/agoric-3-proposals#52

Merged

turadg force-pushed the 8605-local-a3p branch from 7d232f7 to 700a004 Compare January 8, 2024 21:33

turadg removed request for dckc and michaelfig January 8, 2024 21:34

Chris-Hibbert approved these changes Jan 10, 2024

View reviewed changes

turadg added the automerge:rebase Automatically rebase updates, then merge label Jan 10, 2024

turadg force-pushed the 8605-local-a3p branch from 2f05b47 to e061296 Compare January 10, 2024 01:26

mhofman reviewed Jan 10, 2024

View reviewed changes

turadg force-pushed the 8605-local-a3p branch 3 times, most recently from 27ef7dd to fadf0cd Compare January 11, 2024 18:59

aj-agoric assigned Chris-Hibbert Jan 11, 2024

turadg force-pushed the 8605-local-a3p branch from fadf0cd to 44747ef Compare January 11, 2024 22:42

turadg mentioned this pull request Jan 11, 2024

fix 'append' Agoric/agoric-3-proposals#56

Merged

turadg added 7 commits January 11, 2024 23:03

lint: disable jsdoc/require-param-type for .ts

eee7bfa

chore(types): import .js for .ts modules

845ba0d

chore(deps): bump commander to 11.1

edcf2f4

chore(deps): bump @types/node to 18

23ecf28

which introduces 'parseArgs' in node:utils

feat: a3p-integration package

d8b8410

ci: run a3p-integration instead of upgrade-tests

fea3198

chore: remove old upgrade-tests

d8690bf

turadg force-pushed the 8605-local-a3p branch from 44747ef to d8690bf Compare January 11, 2024 23:03

mergify bot merged commit df3d0a7 into master Jan 11, 2024
66 checks passed

mergify bot deleted the 8605-local-a3p branch January 11, 2024 23:36

mhofman pushed a commit that referenced this pull request Jan 12, 2024

Merge pull request #8635 from Agoric/8605-local-a3p

51ce59c

8605 a3p integration

mhofman pushed a commit that referenced this pull request Jan 12, 2024

Merge pull request #8635 from Agoric/8605-local-a3p

2acad14

8605 a3p integration

mhofman mentioned this pull request Jan 12, 2024

chore(ci): use synthetic-chain build config #8744

Merged

mhofman pushed a commit that referenced this pull request Jan 12, 2024

Merge pull request #8635 from Agoric/8605-local-a3p

d88e33e

8605 a3p integration

mhofman pushed a commit that referenced this pull request Jan 12, 2024

Merge pull request #8635 from Agoric/8605-local-a3p

8ab37f3

8605 a3p integration

mhofman reviewed Jan 15, 2024

View reviewed changes

mhofman mentioned this pull request Jan 15, 2024

ci: restore core proposal in upgrade test #8754

Merged

mhofman pushed a commit that referenced this pull request Jan 15, 2024

Merge pull request #8635 from Agoric/8605-local-a3p

897f51b

8605 a3p integration

mhofman pushed a commit that referenced this pull request Jan 15, 2024

Merge pull request #8635 from Agoric/8605-local-a3p

26d198b

8605 a3p integration

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8605 a3p integration #8635

8605 a3p integration #8635

turadg commented Dec 7, 2023 •

edited

Loading

Chris-Hibbert Dec 15, 2023

turadg Dec 15, 2023

Chris-Hibbert Dec 15, 2023 •

edited

Loading

mhofman left a comment

turadg commented Jan 10, 2024

mhofman left a comment

mhofman Jan 8, 2024

turadg Jan 10, 2024

mhofman Jan 10, 2024

mhofman Jan 10, 2024

mhofman Jan 10, 2024

mhofman Jan 15, 2024

8605 a3p integration #8635

8605 a3p integration #8635

Conversation

turadg commented Dec 7, 2023 • 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

Chris-Hibbert Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

mhofman left a comment

Choose a reason for hiding this comment

turadg commented Jan 10, 2024

mhofman 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

turadg commented Dec 7, 2023 •

edited

Loading

Chris-Hibbert Dec 15, 2023 •

edited

Loading