test(a3p-integration): update to agoric-upgrade-18 #10839

mujahidkay · 2025-01-13T13:29:36Z

Agoric/agoric-3-proposals#211

Description

This PR updates a3p to use upgrade-18 image as well as remove u18 core proposals from upgrade.go.

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

None

Testing Considerations

All testing as it updates the base image for a3p

Upgrade Considerations

Removes u18 core proposals from upgrade.go

cloudflare-workers-and-pages · 2025-01-13T13:33:04Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`24d0ce5`
Status:	✅ Deploy successful!
Preview URL:	https://884b6ae2.agoric-sdk.pages.dev
Branch Preview URL:	https://mk-revise-a3p-post-u18.agoric-sdk.pages.dev

View logs

Chris-Hibbert · 2025-01-13T23:19:06Z

Please retain verifyPushPrice, which is called in priceFeedUpdate.

mujahidkay · 2025-01-27T14:00:30Z

a3p-integration/proposals/f:fast-usdc/test/test-cli.sh

 cat accept.json
 yarn agoric wallet send --offer accept.json --from agoric1ee9hr0jyrxhy999y755mp862ljgycmwyp4pl7q --keyring-backend test
-ACCEPT_OFFER_ID=$(agoric wallet extract-id --offer accept.json)
+# UNTIL https://github.com/Agoric/agoric-sdk/issues/10891
+# ACCEPT_OFFER_ID=$(agoric wallet extract-id --offer accept.json)


Even though the fix is merged, reverting this to its original state still fails in CI. So, keeping the workaround for now.

multichain-testing/yarn.lock

turadg

Safe to merge. I think we need @Chris-Hibbert to confirm the test removals are appropriate.

mhofman

I don't think the u19 proposals are being applied in the next chain software upgrade, upgrade.go needs some staged but commented changes to be uncommented. I'm confused as to how test would pass without this.

I'm also surprised to see so many pre-built submission checked-in for upgrade-next, but that seemed to have been there before and just carried from the previous upgrade-19, so out of scope for this PR.

mhofman · 2025-01-27T16:59:50Z

golang/cosmos/app/upgrade.go

-			CoreProposalSteps = append(CoreProposalSteps,
-				vm.CoreProposalStepForModules(
-					"@agoric/builders/scripts/vats/upgrade-orchestration.js",
-				),
 			)

 			// CoreProposals for Upgrade 19. These should not be introduced


I believe the following needs to be uncommented

mhofman · 2025-01-27T17:00:49Z

a3p-integration/proposals/p:upgrade-19/package.json

-    "sdk-generate": [
-      "testing/replace-feeDistributor-short.js replaceFeeDistributor",
-      "testing/add-USD-LEMONS.js addUsdLemons",
-      "vats/upgrade-provisionPool.js upgradeProvisionPool",
-      "vats/upgrade-asset-reserve.js upgradeAssetReserve",
-      "vats/upgrade-psm.js upgradePSM",
-      "vats/upgrade-paRegistry.js",
-      "vats/upgrade-agoricNames.js agoricNamesCoreEvals/upgradeAgoricNames",
-      "testing/add-USD-OLIVES.js agoricNamesCoreEvals/addUsdOlives",
-      "testing/publish-test-info.js agoricNamesCoreEvals/publishTestInfo",
-      "vats/upgrade-mintHolder.js upgrade-mintHolder A3P_INTEGRATION",
-      "vats/terminate-governor-instance.js terminate-governor board02963:ATOM-USD_price_feed"
-    ]


I think some of these need to be moved to upgrade-next as they're used in test. Not sure how it might be passing without it.

They are present in n:upgrade-next/package.json

Oh I missed that. Ok we need to remove the ones now applies in upgrade.go from that list, and also probably any code that triggered the core eval submissions.

mhofman · 2025-01-27T17:03:34Z

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

-yarn ava initial.test.js
-
-# test more, in ways that change system state
-yarn ava ./*.test.js


I'm a little uncomfortable carrying the non-wildcard test approach of the previous upgrade-19. @Chris-Hibbert are there any interdependency of tests?

I don't believe so. I'd also prefer switching to a wildcard invocation.

multichain-testing/yarn.lock

mhofman · 2025-01-27T18:11:35Z

a3p-integration/proposals/n:upgrade-next/package.json

@@ -8,16 +8,9 @@
    },
    "type": "Software Upgrade Proposal",
    "sdk-generate": [
-      "testing/replace-feeDistributor-short.js replaceFeeDistributor",


I think this one needs to remain

But it is a part of core proposals? I would love to know why this should remain

I assumed that any testing proposal is solely used for testing. @Chris-Hibbert can you clarify?

mhofman · 2025-01-27T18:11:55Z

golang/cosmos/app/upgrade.go

-// 	)
-// }
+func upgradeMintHolderCoreProposal(upgradeName string) (vm.CoreProposalStep, error) {
+	variant := getVariantFromUpgradeName(upgradeName)


Yikes, another variant type upgrade

mhofman · 2025-01-27T18:15:29Z

a3p-integration/proposals/n:upgrade-next/eval.sh

An eval.sh wouldn't be executed in chain software upgrade proposal. It should be deleted, and any core evals submitted should be tests related ones and moved to test.sh

Actually it looks like that based on sdk-generate some of these core evals were never actually submitted !? @Chris-Hibbert ?

Looks like you're right. I don't see tests for ZCF, or PriceAdminRegistry.

mujahidkay · 2025-01-27T18:41:19Z

a3p-integration/proposals/n:upgrade-next/package.json

      "testing/add-USD-LEMONS.js addUsdLemons",
-      "vats/upgrade-provisionPool.js upgradeProvisionPool",
-      "vats/upgrade-asset-reserve.js upgradeAssetReserve",
-      "vats/upgrade-psm.js upgradePSM",


Removing this results in the following error:

✔ verify trading after upgrade (12.7s) ─ upgrade PSMs Rejected promise returned by test. Reason: Error { code: 'ENOENT', errno: -2, path: 'upgradePSM', syscall: 'scandir', message: 'ENOENT: no such file or directory, scandir \'upgradePSM\'', }

Probably because the test expects it to be a core eval (or am i doing something wrong)

test.serial('upgrade PSMs', async t => { // @ts-expect-error casting const { vstorageKit } = t.context; await evalBundles(UPGRADE_PSM_DIR);

I think you're right. Some core proposals were submitted in the test layer, which is incorrect because acceptance tests would have run against an non upgraded psm in this case. We need to remove this core eval from the test as it should now be done in the upgrade.go

Chris-Hibbert

The test removals look correct.

Chris-Hibbert · 2025-01-27T18:43:47Z

golang/cosmos/app/upgrade.go

-					// Upgrade to include a cleanup from https://github.com/Agoric/agoric-sdk/pull/10319
-					"@agoric/builders/scripts/smart-wallet/build-wallet-factory2-upgrade.js",


Please reinstate. #10794 wants us to do a smartWallet upgrade on every release until it's fixed.

Will add this back. I'm about to force push to a previous state prior to e75914c and resolve conflicts.

Chris-Hibbert · 2025-01-27T19:06:29Z

a3p-integration/proposals/n:upgrade-next/eval.sh

Looks like you're right. I don't see tests for ZCF, or PriceAdminRegistry.

Chris-Hibbert · 2025-01-27T19:37:24Z

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

-yarn ava initial.test.js
-
-# test more, in ways that change system state
-yarn ava ./*.test.js


I don't believe so. I'd also prefer switching to a wildcard invocation.

mhofman · 2025-01-27T21:08:04Z

a3p-integration/proposals/n:upgrade-next/initial.test.js

We probably need to adapt this file to the expected vat upgrades of u19

I worked on this today. And this only seems valid if the upgrades are triggered via core proposals, right? The current state of this PR keeps the vat upgrades as core evals, apart from a couple, all others have corresponding tests that verify incarnation number after core eval is applied.

P.S. vat names I used (locally. apart from psmVats which didn't match for some reason) picked from here

const vats = { walletFactory: { incarnation: 6 }, 'zcf-b1-92c07-reserve': { incarnation: 1 }, priceAuthority: { incarnation: 1 }, agoricNames: { incarnation: 1 }, '-provisionPool-governor': { incarnation: 1 }, '-provisionPool': { incarnation: 1 }, // ...psmVats, ...mintHolderVats, };

BTW, I did uncomment the core proposals and got this test to pass with updated incarnation numbers but that resulted in failure of other tests (since I commented out evalBundles from tests).

We probably need to adapt this file to the expected vat upgrades of u19

I am not sure if this is required for this PR to merge, or should this be done in #10901. Please confirm @mhofman

this only seems valid if the upgrades are triggered via core proposals, right?

should this be done in #10901.

The content will change when upgrades are going through as core proposals but this PR should keep the file. If this PR doesn't run core proposals and postpones that to #10901, then yes that PR is in charge of updating the file with the new incarnation numbers.

mhofman

I think this aligns with the "do nothing" upgrade we aligned on to get this landed.

@Chris-Hibbert will rework upgrade.go and the upgrade-next structure to move u19 core proposals as part of chain software and away from the test layer.

mergify · 2025-01-31T08:38:18Z

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

❌ integration-test-result
☑️ linear-history

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

mergify · 2025-01-31T09:19:12Z

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

❌ integration-test-result
☑️ linear-history

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

mujahidkay mentioned this pull request Jan 13, 2025

feat: add agoric-upgrade-18 (proposal 83) Agoric/agoric-3-proposals#211

Merged

3 tasks

mujahidkay force-pushed the mk/revise-a3p-post-u18 branch from b436c99 to 12e62c8 Compare January 22, 2025 09:57

mujahidkay self-assigned this Jan 22, 2025

mujahidkay added the force:integration Force integration tests to run on PR label Jan 22, 2025

mujahidkay force-pushed the mk/revise-a3p-post-u18 branch 2 times, most recently from 84f9b5c to 71a0889 Compare January 23, 2025 17:54

turadg force-pushed the mk/revise-a3p-post-u18 branch 3 times, most recently from c3f4254 to 1d15d42 Compare January 24, 2025 23:06

mujahidkay force-pushed the mk/revise-a3p-post-u18 branch 2 times, most recently from d03f2cc to 8260dd4 Compare January 27, 2025 12:00

mujahidkay marked this pull request as ready for review January 27, 2025 13:51

mujahidkay requested a review from a team as a code owner January 27, 2025 13:51

mujahidkay requested a review from AgoricTriage January 27, 2025 13:51

mujahidkay commented Jan 27, 2025

View reviewed changes

multichain-testing/yarn.lock Outdated Show resolved Hide resolved

mujahidkay changed the title ~~WIP draft to adopt u18 a3p changes~~ test(a3p-integration): update to agoric-upgrade-18 Jan 27, 2025

mujahidkay requested review from turadg and dckc January 27, 2025 15:52

turadg approved these changes Jan 27, 2025

View reviewed changes

mujahidkay requested a review from Chris-Hibbert January 27, 2025 16:39

mhofman reviewed Jan 27, 2025

View reviewed changes

mujahidkay commented Jan 27, 2025

View reviewed changes

Chris-Hibbert reviewed Jan 27, 2025

View reviewed changes

mujahidkay force-pushed the mk/revise-a3p-post-u18 branch 2 times, most recently from 1a188be to 69dc2a7 Compare January 27, 2025 20:01

mhofman reviewed Jan 27, 2025

View reviewed changes

mhofman mentioned this pull request Jan 27, 2025

u19 is not acceptance tested #10901

Closed

mujahidkay force-pushed the mk/revise-a3p-post-u18 branch from f25b968 to d7a0a0e Compare January 30, 2025 08:43

mujahidkay requested a review from mhofman January 30, 2025 12:03

mhofman approved these changes Jan 31, 2025

View reviewed changes

mujahidkay added the automerge:rebase Automatically rebase updates, then merge label Jan 31, 2025

mujahidkay and others added 7 commits January 31, 2025 07:51

test(a3p-integration): update to agoric-upgrade-18

216bfa0

chore(deps): yarn up --recursive agoric '@agoric/*'

7bf47db

test: rm for old priceAuthority

e11787c

test: remove terminateGovernor logic

7b3aaee

chore: README for n:upgrade-next

cf0b2cd

test: wallet cli workaround

5dd377c

test(a3p): retain initial.test.js

24d0ce5

mujahidkay force-pushed the mk/revise-a3p-post-u18 branch from 8b6e056 to 24d0ce5 Compare January 31, 2025 07:51

mergify bot merged commit 3d64bee into master Jan 31, 2025
83 checks passed

mergify bot deleted the mk/revise-a3p-post-u18 branch January 31, 2025 09:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(a3p-integration): update to agoric-upgrade-18 #10839

test(a3p-integration): update to agoric-upgrade-18 #10839

mujahidkay commented Jan 13, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Jan 13, 2025 •

edited

Loading

Chris-Hibbert commented Jan 13, 2025

mujahidkay Jan 27, 2025

turadg left a comment

mhofman left a comment

mhofman Jan 27, 2025

mhofman Jan 27, 2025

mujahidkay Jan 27, 2025

mhofman Jan 27, 2025

mhofman Jan 27, 2025

Chris-Hibbert Jan 27, 2025

mhofman Jan 27, 2025

mujahidkay Jan 27, 2025 •

edited

Loading

mhofman Jan 27, 2025

mhofman Jan 27, 2025

mujahidkay Jan 27, 2025

mhofman Jan 27, 2025

mhofman Jan 27, 2025

Chris-Hibbert Jan 27, 2025

mujahidkay Jan 27, 2025

mhofman Jan 27, 2025

Chris-Hibbert left a comment

Chris-Hibbert Jan 27, 2025

mujahidkay Jan 27, 2025

Chris-Hibbert Jan 27, 2025

Chris-Hibbert Jan 27, 2025

mhofman Jan 27, 2025

mujahidkay Jan 28, 2025 •

edited

Loading

mhofman Jan 28, 2025

mhofman left a comment

mergify bot commented Jan 31, 2025

mergify bot commented Jan 31, 2025

		// Upgrade to include a cleanup from https://github.com/Agoric/agoric-sdk/pull/10319
		"@agoric/builders/scripts/smart-wallet/build-wallet-factory2-upgrade.js",

test(a3p-integration): update to agoric-upgrade-18 #10839

test(a3p-integration): update to agoric-upgrade-18 #10839

Conversation

mujahidkay commented Jan 13, 2025 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

cloudflare-workers-and-pages bot commented Jan 13, 2025 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

Chris-Hibbert commented Jan 13, 2025

Choose a reason for hiding this comment

turadg left a comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

mujahidkay Jan 27, 2025 • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mujahidkay Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhofman left a comment

Choose a reason for hiding this comment

mergify bot commented Jan 31, 2025

mergify bot commented Jan 31, 2025

mujahidkay commented Jan 13, 2025 •

edited

Loading

cloudflare-workers-and-pages bot commented Jan 13, 2025 •

edited

Loading

mujahidkay Jan 27, 2025 •

edited

Loading

mujahidkay Jan 28, 2025 •

edited

Loading