chore(pp-upgrade): test functionality after provisionPool and bank upgrade #10419

anilhelvaci · 2024-11-07T08:19:15Z

Description

As mentioned in #8158, we need the ability to restart or replace all vats. This PR focuses on the vats v28-provisionPool and v14-bank. The goal is to make sure after upgrading both of those vats, v28-provisionPool functionality keeps working. We pay special attention to #8722 and #8724 during the tests as those issues were identified as potential problems against v28-provisionPool upgrade.

Security Considerations

v28-provisionPool and v14-bank are critical vats for the system when it comes to onboarding new users and keeping track of ERTP representations of user assets. Reviewers, please highlight the slightest risk you see.

Scaling Considerations

v28-provisionPool vat has a linear relationship with the number of users entering the Agoric system. So it is pretty important it keeps working. Reviewers, please highlight the slightest risk you see.

Documentation Considerations

None.

Testing Considerations

During the testing, we focused on provisionPoolAddress' cosmos level balances as our source of truth. The reasoning behind this is; if the IST balance of the provisionPoolAddress it can only mean

it has received the anchor asset we've sent
v28-provisionPool is notified of this balance change
executed a swap against corresponding PSM
deposited the payout to IST purse
v14-bank updated the balances correctly

In our test we send two anchor assets;

USDC_axl to make sure v28-provisionPool recovered existing purses
USD_LEMONS to make sure v28-provisionPool is notified of new assets

Upgrade Considerations

Both v28-provisionPool and v14-bank are staged upgrades in upgrade.go

cloudflare-workers-and-pages · 2024-11-07T08:32:20Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`51a631d`
Status:	✅ Deploy successful!
Preview URL:	https://4bdcafc1.agoric-sdk.pages.dev
Branch Preview URL:	https://anil-10395-pp-vbank.agoric-sdk.pages.dev

View logs

Refs: #10395

…grade Refs: #10395 fix: format fixes

Chris-Hibbert

I suspect that it would be a good idea to convert makeBridgeProvisionTool from a Far object to an Exo, but I'm not positive. I've asked for advice and will let you know what I hear.

Chris-Hibbert · 2024-11-07T19:22:04Z

golang/cosmos/app/upgrade.go

+				vm.CoreProposalStepForModules(
+					"@agoric/builders/scripts/vats/upgrade-provisionPool.js",
+				),
+				vm.CoreProposalStepForModules(
+					"@agoric/builders/scripts/vats/upgrade-bank.js",
+				),


Please put this in a separate section with a comment saying it's for upgrade 19 as is done in #10418.

Chris-Hibbert · 2024-11-07T21:22:55Z

a3p-integration/proposals/p:upgrade-19/test-lib/sync-tools.js

Duplicating these files is the wrong solution. AIUI, the problem is that when docker runs the proposals, it only mounts a single proposal's directory at a time. I've talked this over with @turadg, and I'd like to try using symbolic links to manage this.

Please add a lib directory in a3p-integration next to proposals, and put a canonical copy of the file there. Then make each of the proposals that want to import all or some of sync-tools include a symbolic link to that file, so the tests can import ./sync-tools.js. Do the same with errors.js.

I haven't verified how this will work with github, but I'd like to try the approach out here.

Chris-Hibbert · 2024-11-07T21:42:04Z

a3p-integration/proposals/p:upgrade-19/provisionPool.test.js

+    PROVISIONING_POOL_ADDR,
+    ambientAuthority,
+    { denom: 'uist', value: istBalanceBeforeLemonsSent + 500000 },
+    { errorMessage: 'Provision pool not bale swap USDC_axl for IST.' },


Suggested change

{ errorMessage: 'Provision pool not bale swap USDC_axl for IST.' },

{ errorMessage: 'Provision pool not able to swap USDC_axl for IST.' },

Chris-Hibbert · 2024-11-07T21:54:26Z

a3p-integration/proposals/p:upgrade-19/provisionPool.test.js

+    { maxRetries: 5, retryIntervalMs: 1000, log: console.log, setTimeout },
+  );
+
+test(`upgrade provision pool`, async t => {


Please check the incarnation #s of both vats. I think the tests would pass if the upgrade didn't take place since there's no change in behavior to detect.

Chris-Hibbert · 2024-11-07T23:45:27Z

I suspect that it would be a good idea to convert makeBridgeProvisionTool from a Far object to an Exo, but I'm not positive. I've asked for advice and will let you know what I hear.

The response I got was "yes, convert it to an Exo", but it appears that this object may be one of those discussed in #8849, which may mean we need a clearer story on upgrading the bootstrap vat (V1) before we can proceed to upgrade provisionPool.

You can work on converting it to an Exo

I need to have more design discussions with the kernel team about the impact. The concern seems to be about whether auto-provisioning will continue to work. The links mentioned were

https://community.agoric.com/t/proposal-65-agoric-upgrade-13-smart-wallet-auto-provisioning-interchain-stack/560
do a PLEASE_PROVISION, see agd tx swingset provision-one docs
for auto-provision, try making a vault from an account that has never done a PLEASE_PROVISION

anilhelvaci · 2024-11-08T12:35:34Z

The response I got was "yes, convert it to an Exo", but it appears that this object may be one of those discussed in #8849, which may mean we need a clearer story on upgrading the bootstrap vat (V1) before we can proceed to upgrade provisionPool.

You can work on converting it to an Exo

I need to have more design discussions with the kernel team about the impact. The concern seems to be about whether auto-provisioning will continue to work. The links mentioned were

https://community.agoric.com/t/proposal-65-agoric-upgrade-13-smart-wallet-auto-provisioning-interchain-stack/560

do a PLEASE_PROVISION, see agd tx swingset provision-one docs

for auto-provision, try making a vault from an account that has never done a PLEASE_PROVISION

#10425 tracks this now. From your comment I understand the path for this PR to land;

Close 10425
wait the plan for upgrading vat-bootstrap is clear
address change requests

Could you please confirm? @Chris-Hibbert

Chris-Hibbert · 2024-11-13T19:06:42Z

#10425 tracks this now. From your comment I understand the path for this PR to land;

Close 10425

wait the plan for upgrading vat-bootstrap is clear

address change requests

Could you please confirm?

Let's pursue 10425. If we can write tests that show the bootstrap vat doesn't break, and we can continue to provision new wallets, I don't think we have to be able to upgrade vat-bootstrap before upgrading provisionPool.

anilhelvaci requested review from Chris-Hibbert and iomekam November 7, 2024 08:32

anilhelvaci marked this pull request as ready for review November 7, 2024 08:33

anilhelvaci requested a review from a team as a code owner November 7, 2024 08:33

anilhelvaci added 3 commits November 7, 2024 16:30

chore(pp-upgrade): create and set a new bridgeHandler in the proposal

7e03007

Refs: #10395

chore(pp-upgrade): create helper core-evals

50f3e23

Refs: #10395

chore(pp-upgrade): test functionality after provisionPool and bank up…

51a631d

…grade Refs: #10395 fix: format fixes

anilhelvaci force-pushed the anil/10395-pp-vbank branch from e375cde to 51a631d Compare November 7, 2024 13:31

anilhelvaci changed the title ~~chore(pp-upgrade): create and set a new bridgeHandler in the proposal~~ chore(pp-upgrade): chore(pp-upgrade): test functionality after provisionPool and bank upgrade Nov 7, 2024

Chris-Hibbert added the force:integration Force integration tests to run on PR label Nov 7, 2024

Chris-Hibbert requested changes Nov 7, 2024

View reviewed changes

anilhelvaci mentioned this pull request Nov 8, 2024

convert makeBridgeProvisionTool from a Far to an Exo #10425

Open

anilhelvaci changed the title ~~chore(pp-upgrade): chore(pp-upgrade): test functionality after provisionPool and bank upgrade~~ chore(pp-upgrade): test functionality after provisionPool and bank upgrade Nov 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(pp-upgrade): test functionality after provisionPool and bank upgrade #10419

chore(pp-upgrade): test functionality after provisionPool and bank upgrade #10419

anilhelvaci commented Nov 7, 2024

cloudflare-workers-and-pages bot commented Nov 7, 2024 •

edited

Loading

Chris-Hibbert left a comment

Chris-Hibbert Nov 7, 2024

Chris-Hibbert Nov 7, 2024

Chris-Hibbert Nov 7, 2024

Chris-Hibbert Nov 7, 2024

Chris-Hibbert commented Nov 7, 2024

anilhelvaci commented Nov 8, 2024

Chris-Hibbert commented Nov 13, 2024

	{ errorMessage: 'Provision pool not bale swap USDC_axl for IST.' },
	{ errorMessage: 'Provision pool not able to swap USDC_axl for IST.' },

chore(pp-upgrade): test functionality after provisionPool and bank upgrade #10419

Are you sure you want to change the base?

chore(pp-upgrade): test functionality after provisionPool and bank upgrade #10419

Conversation

anilhelvaci commented Nov 7, 2024

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

cloudflare-workers-and-pages bot commented Nov 7, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

Chris-Hibbert left a comment

Choose a reason for hiding this comment

Chris-Hibbert Nov 7, 2024

Choose a reason for hiding this comment

Chris-Hibbert Nov 7, 2024

Choose a reason for hiding this comment

Chris-Hibbert Nov 7, 2024

Choose a reason for hiding this comment

Chris-Hibbert Nov 7, 2024

Choose a reason for hiding this comment

Chris-Hibbert commented Nov 7, 2024

anilhelvaci commented Nov 8, 2024

Chris-Hibbert commented Nov 13, 2024

cloudflare-workers-and-pages bot commented Nov 7, 2024 •

edited

Loading