Contract Upgrade: V28 ProvisionPool and V14 Bank

[Chris-Hibbert]

What is the Problem Being Solved?

As mentioned in #8158, we need the ability to restart or replace all vats. This issue is focused on upgrading v28-ProvisionPool and v14-bank. They are related because of #8722 and #8724, which were addressed by #9481.

Description of the Design

These upgrades have been tested previously, and are in agoric-3-proposals/upgrade-17. The goal here is to reinstate those in a3p-integration, and stage them in upgrade.go for the next upgrade.

In addition, vat-bank is a client of feeDistributor, which will be separately be upgraded. Let's re-review the tests to be sure that vat-bank continues to send fees after an upgrade.

Security Considerations

vat-bank is a critical vat. provisionPool is involved in creating new smartWallets.

Scaling Considerations

not significant.

Test Plan

Move proposals/78:upgrade-17/provisionPool.test.js from agoric-3-proposals to a3p-integration. Verify that it checks everything relevant.

Upgrade Considerations

See above.

[anilhelvaci]

Thanks for creating the issue @Chris-Hibbert . A few questions here;

When I read proposals/78:upgrade-17/provisionPool.test.js code I can see that both vat-bank and vat-provisionPool start with incarnation = 0. However, as far as I can understand how upgrade.go is intended to work, staging vat-bank and vat-provisionPool in ugrade.go will result they will enter the testing phase with their incarnation being equal to 1. Which makes me think if logic in provisionPool.test.js will actually make sense when both vats are already upgraded.

• Do you think about a new scenario that covers both vat-provisionpool will not re-subscribe to vat-bank getCurrentAmountNotifier #8722 and test that restarting v28-provisionpool will re-subscribe to v14-bank asset subscriptions #8724? Like adding a new asset, depositing it into provisionPool address then observing its minted totalMintedConverted? And do the same for an existing asset like USDC_axl.
  • In this comment you mention that you couldn't find a reasonable way to mint new currencies after you add them into bankManager and @turadg's reply here talks about having a core-eval for this purpose. Why do you think using a core-eval that both adds a new asset then deposits it into provisionPool is not reasonable?
• Where should we local this provisionPool.test.js? z:acceptance ?

[Chris-Hibbert]

[in proposals/78:upgrade-17/provisionPool.test.js] both vat-bank and vat-provisionPool start with incarnation = 0. However, as far as I can understand how upgrade.go is intended to work, staging vat-bank and vat-provisionPool in ugrade.go will result they will enter the testing phase with their incarnation being equal to 1.

Yep. The point of adding them to upgrade.go is that they will actually upgrade, and their incarnations will increment. So the test will need to be updated.

I think it would be great to test both an existing and a new currency.

Why do you think using a core-eval that both adds a new asset then deposits it into provisionPool is not reasonable?

Turadg's suggestion is totally reasonable.

Where should we local this provisionPool.test.js? z:acceptance ?

I don't think this test needs to survive after the next upgrade. It would normally go in n:upgrade-next, but that's still tangled with U18 stuff. I'd make a new directory, like p:upgrade-19, with the intent that that moves to upgrade-next once that has been cleared out.

[anilhelvaci]

Yep. The point of adding them to upgrade.go is that they will actually upgrade, and their incarnations will increment. So the test will need to be updated.

I think it would be great to test both an existing and a new currency.

Alright I'll start working on updating tests with Turadg's suggestion.

I don't think this test needs to survive after the next upgrade. It would normally go in n:upgrade-next, but that's still tangled with U18 stuff. I'd make a new directory, like p:upgrade-19, with the intent that that moves to upgrade-next once that has been cleared out.

Thanks, I'll put the new tests under p:upgrade-19

[anilhelvaci]

Change requests to the PR indicates a dependency #10514