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

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

closes: #10395
closes: #10425
closes: #10564

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:	`164151e`
Status:	✅ Deploy successful!
Preview URL:	https://37bef8fc.agoric-sdk.pages.dev
Branch Preview URL:	https://anil-10395-pp-vbank.agoric-sdk.pages.dev

View logs

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.

golang/cosmos/app/upgrade.go

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

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

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.

a3p-integration/proposals/p:upgrade-19/.gitignore

a3p-integration/proposals/p:upgrade-19/nullUpgradePP/null-upgrade-pp-permit.json

a3p-integration/proposals/p:upgrade-19/depositUSD-LEMONS/deposit-usd-lemons.js

golang/cosmos/app/upgrade.go

packages/inter-protocol/src/provisionPoolKit.js

Chris-Hibbert

I requested a few clean-ups, but this looks good. Thanks.

packages/inter-protocol/src/provisionPoolKit.js

Chris-Hibbert · 2024-11-27T17:30:28Z

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

+ * @param {string} wanted
+ * @param {string} [from]
+ */
+export const bankSend = (addr, wanted, from = VALIDATORADDR) => {


please pull bankSend, provision, introduceAndProvision, getProvisionedAddress, and checkUserProvisioned out to a separate library file that can be shared. We shouldn't have to define these more than once.

Maybe getProvisionedAddress is simple enough that it doesn't have to go in the library.

please pull bankSend, provision, introduceAndProvision, getProvisionedAddress, and checkUserProvisioned out to a separate library file that can be shared. We shouldn't have to define these more than once.

#10514 already tracks this one. I can put those methods into a helper module then move them to the shared library as part of #10514. I think we shouldn't wait for the shared library to land this PR.

I think we shouldn't wait for the shared library to land this PR

That's fine.

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

anilhelvaci · 2024-11-27T21:46:24Z

We talked about commenting about upgrade-19 stuff in upgrade.go however the CI seems to be failing because it expects upgrade-paRegistry.js core eval to be executed:

registry › priceAuthorityRegistry upgrade

  registry.test.js:16

   15: test('priceAuthorityRegistry upgrade', async t => {
   16:   t.is(await getIncarnation('priceAuthority'), 1); 
   17:                                                    

  Difference (- actual, + expected):

  - 0
  + 1

I think the issue is related to this PR that recently landed #10418 @Chris-Hibbert

Chris-Hibbert · 2024-12-02T19:45:52Z

however the CI seems to be failing because it expects upgrade-paRegistry.js core eval to be executed:

I ran into a similar issue. My conclusion was that the upgrades that are not in U18 should be cleanly moved to u19 and tested there. If that means you have to move some tests from U18 or correct some expected output in U18 in a3p-integration, then that's the right approach. Leaving those tests anachronistically in U18 in a3p-integration will be more confusing than cleaning it up.

anilhelvaci · 2024-12-02T20:29:15Z

I ran into a similar issue. My conclusion was that the upgrades that are not in U18 should be cleanly moved to u19 and tested there. If that means you have to move some tests from U18 or correct some expected output in U18 in a3p-integration, then that's the right approach. Leaving those tests anachronistically in U18 in a3p-integration will be more confusing than cleaning it up.

Yep, I think it's resolved in master.

I rebased my branch on top of the master. Could you please approve the PR if it looks good to you? @Chris-Hibbert

Chris-Hibbert

LGTM

Chris-Hibbert · 2024-12-02T20:55:48Z

packages/vats/src/proposals/upgrade-provisionPool-proposal.js

@@ -1,5 +1,8 @@
 import { E } from '@endo/far';
 import { deeplyFulfilled } from '@endo/marshal';
+import { makeTracer } from '@agoric/internal';
+
+const tracer = makeTracer('UpgradeProvisionPool');


not a big deal,but we usually call this trace.

Refs: #10395

…grade Refs: #10395 fix: format fixes

Make sure the handler returned from E(creatorFacet).makeHandler() is durable and provisionPool is upgradable multiple times. Refs: #10425 Refs: #10564 fix: remove unnecessary comment chore: get rid of ephemera, address change requests Refs: #10395 Refs: #10425 fix: drop trace chore: address change requests fix: rebase fixes fix: yarn.lock fix fix: bring back missing upgrade-paRegistry proposal

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

Closed

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

anilhelvaci force-pushed the anil/10395-pp-vbank branch from 51a631d to 6fdcf87 Compare November 21, 2024 11:31

Chris-Hibbert mentioned this pull request Nov 26, 2024

bootstrap upgrade complicated by 5 non-durable exports #8849

Open

anilhelvaci force-pushed the anil/10395-pp-vbank branch from 1e8f8d0 to bc5a94e Compare November 26, 2024 15:52

anilhelvaci requested a review from Chris-Hibbert November 26, 2024 15:56

anilhelvaci mentioned this pull request Nov 26, 2024

reduce complexity by dropping setReferences and start using privateArgs instead in provisionPool #10564

Closed

anilhelvaci force-pushed the anil/10395-pp-vbank branch from bc5a94e to a8b124a Compare November 26, 2024 17:53

Chris-Hibbert requested changes Nov 26, 2024

View reviewed changes

anilhelvaci force-pushed the anil/10395-pp-vbank branch 2 times, most recently from d5f827a to 58e7583 Compare November 27, 2024 17:50

Chris-Hibbert requested changes Nov 27, 2024

View reviewed changes

anilhelvaci force-pushed the anil/10395-pp-vbank branch 3 times, most recently from 6e7cda9 to 9c75709 Compare November 27, 2024 20:17

anilhelvaci requested a review from Chris-Hibbert November 27, 2024 20:19

anilhelvaci force-pushed the anil/10395-pp-vbank branch 2 times, most recently from 6ae5628 to 7346df1 Compare November 27, 2024 21:43

Jorge-Lopes mentioned this pull request Dec 2, 2024

feat(provisionPool): allow overriding PerAccountInitialAmount during contract upgrade #10594

Draft

anilhelvaci force-pushed the anil/10395-pp-vbank branch from 7346df1 to ce1e94f Compare December 2, 2024 20:25

Chris-Hibbert approved these changes Dec 3, 2024

View reviewed changes

anilhelvaci added the automerge:rebase Automatically rebase updates, then merge label Dec 3, 2024

anilhelvaci added 4 commits December 3, 2024 17:53

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

0767f72

Refs: #10395

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

c8916b2

Refs: #10395

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

bf6c1ff

…grade Refs: #10395 fix: format fixes

anilhelvaci force-pushed the anil/10395-pp-vbank branch from ce1e94f to 164151e Compare December 3, 2024 17:53

mergify bot merged commit dcf2772 into master Dec 3, 2024
79 checks passed

mergify bot deleted the anil/10395-pp-vbank branch December 3, 2024 18:40

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 •

edited

Loading

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

edited

Loading

Chris-Hibbert left a comment

Chris-Hibbert commented Nov 7, 2024

anilhelvaci commented Nov 8, 2024

Chris-Hibbert commented Nov 13, 2024

Chris-Hibbert left a comment

Chris-Hibbert Nov 27, 2024

anilhelvaci Nov 27, 2024

Chris-Hibbert Dec 2, 2024 •

edited

Loading

anilhelvaci commented Nov 27, 2024

Chris-Hibbert commented Dec 2, 2024

anilhelvaci commented Dec 2, 2024

Chris-Hibbert left a comment

Chris-Hibbert Dec 2, 2024

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

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

Conversation

anilhelvaci commented Nov 7, 2024 • edited Loading

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 commented Nov 7, 2024

anilhelvaci commented Nov 8, 2024

Chris-Hibbert commented Nov 13, 2024

Chris-Hibbert left a comment

Choose a reason for hiding this comment

Chris-Hibbert Nov 27, 2024

Choose a reason for hiding this comment

anilhelvaci Nov 27, 2024

Choose a reason for hiding this comment

Chris-Hibbert Dec 2, 2024 • edited Loading

Choose a reason for hiding this comment

anilhelvaci commented Nov 27, 2024

Chris-Hibbert commented Dec 2, 2024

anilhelvaci commented Dec 2, 2024

Chris-Hibbert left a comment

Choose a reason for hiding this comment

Chris-Hibbert Dec 2, 2024

Choose a reason for hiding this comment

anilhelvaci commented Nov 7, 2024 •

edited

Loading

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

edited

Loading

Chris-Hibbert Dec 2, 2024 •

edited

Loading