fix: contractGovernor can change params after contract is upgraded (#…

…10163)

closes: #9982
closes: #10172

## Description

When a governed contract is upgraded, its paramManager (which was ephemeral) is replaced, and the contractGovernor needs to get a fresh copy.

### Security Considerations

This bug caused us to need to cut RC2 when preparing the vaultFactory release. That time, the fix was to upgrade the contractGovernor when upgrading a governed contract.  

### Scaling Considerations

No scaling impacts.

### Testing Considerations

added a new test, which fails without the fix.

### Upgrade Considerations

This is staged on top of #10074, and the goal is to include it with the priceFeed coreEval. Recent commits have updated the coreEval to include upgrading and installing the contractGovernor code.

packages/boot/test/bootstrapTests/price-feed-replace.test.ts

@@ -81,6 +81,13 @@ test.serial('setupVaults; run updatePriceFeeds proposals', async t => {
   const SOME_GUI = 'someGUIHASH';
   await updateVaultDirectorParams(t, gd, SOME_GUI);
 
+  const { EV } = t.context.runUtils;
+  const agoricNames = await EV.vat('bootstrap').consumeItem('agoricNames');
+  const oldVaultInstallation = await EV(agoricNames).lookup(
+    'installation',
+    'VaultFactory',
+  );
+
   t.log('building all relevant CoreEvals');
   const coreEvals = await Promise.all([
     buildProposal(priceFeedBuilder, ['main']),
@@ -99,6 +106,16 @@ test.serial('setupVaults; run updatePriceFeeds proposals', async t => {
   t.not(instancePre, instancePost);
 
   await priceFeedDrivers[collateralBrandKey].refreshInvitations();
+
+  const newVaultInstallation = await EV(agoricNames).lookup(
+    'installation',
+    'VaultFactory',
+  );
+
+  t.notDeepEqual(
+    newVaultInstallation.getKref(),
+    oldVaultInstallation.getKref(),
+  );
 });
 
 test.serial('1. place bid', async t => {

packages/boot/test/bootstrapTests/updateUpgradedVaultParams.test.ts

@@ -0,0 +1,132 @@
+/**
+ * @file The goal of this test is to show that #9982 is fixed
+ * We change a parameter so that provideParamGovernance() is called once, and
+ * paramGoverance has been set. Then upgrade vaultFactory, so any ephemeral
+ * objects from the contract held by the governor are gone, then try to change
+ * param again, to show that the bug is fixedd.
+ */
+import { test as anyTest } from '@agoric/zoe/tools/prepare-test-env-ava.js';
+
+import type { TestFn } from 'ava';
+import { makeAgoricNamesRemotesFromFakeStorage } from '@agoric/vats/tools/board-utils';
+import { Fail } from '@endo/errors';
+
+import { makeSwingsetTestKit } from '../../tools/supports.js';
+import {
+  makeGovernanceDriver,
+  makeWalletFactoryDriver,
+} from '../../tools/drivers.js';
+import { updateVaultManagerParams } from '../tools/changeVaultParams.js';
+
+const makeDefaultTestContext = async t => {
+  console.time('DefaultTestContext');
+  const swingsetTestKit = await makeSwingsetTestKit(t.log);
+
+  const { runUtils, storage } = swingsetTestKit;
+  console.timeLog('DefaultTestContext', 'swingsetTestKit');
+  const { EV } = runUtils;
+
+  // Wait for ATOM to make it into agoricNames
+  await EV.vat('bootstrap').consumeItem('vaultFactoryKit');
+  console.timeLog('DefaultTestContext', 'vaultFactoryKit');
+
+  // has to be late enough for agoricNames data to have been published
+  const agoricNamesRemotes = makeAgoricNamesRemotesFromFakeStorage(
+    swingsetTestKit.storage,
+  );
+  agoricNamesRemotes.brand.ATOM || Fail`ATOM missing from agoricNames`;
+  console.timeLog('DefaultTestContext', 'agoricNamesRemotes');
+
+  const walletFactoryDriver = await makeWalletFactoryDriver(
+    runUtils,
+    storage,
+    agoricNamesRemotes,
+  );
+  console.timeLog('DefaultTestContext', 'walletFactoryDriver');
+
+  console.timeEnd('DefaultTestContext');
+
+  const gd = await makeGovernanceDriver(
+    swingsetTestKit,
+    agoricNamesRemotes,
+    walletFactoryDriver,
+    [
+      'agoric1ldmtatp24qlllgxmrsjzcpe20fvlkp448zcuce',
+      'agoric140dmkrz2e42ergjj7gyvejhzmjzurvqeq82ang',
+      'agoric1w8wktaur4zf8qmmtn3n7x3r0jhsjkjntcm3u6h',
+    ],
+  );
+
+  return { ...swingsetTestKit, agoricNamesRemotes, gd };
+};
+
+const test = anyTest as TestFn<
+  Awaited<ReturnType<typeof makeDefaultTestContext>>
+>;
+
+test.before(async t => {
+  t.context = await makeDefaultTestContext(t);
+});
+test.after.always(t => {
+  return t.context.shutdown && t.context.shutdown();
+});
+
+const outcome = {
+  bids: [{ payouts: { Bid: 0, Collateral: 1.800828 } }],
+};
+
+test('restart vaultFactory, change params', async t => {
+  const { runUtils, gd, agoricNamesRemotes } = t.context;
+  const { EV } = runUtils;
+  const vaultFactoryKit =
+    await EV.vat('bootstrap').consumeItem('vaultFactoryKit');
+
+  const { ATOM } = agoricNamesRemotes.brand;
+  ATOM || Fail`ATOM missing from agoricNames`;
+
+  const reserveKit = await EV.vat('bootstrap').consumeItem('reserveKit');
+  const bootstrapVat = EV.vat('bootstrap');
+  const electorateCreatorFacet = await bootstrapVat.consumeItem(
+    'economicCommitteeCreatorFacet',
+  );
+
+  const poserInvitation = await EV(electorateCreatorFacet).getPoserInvitation();
+  const creatorFacet1 = await EV.get(reserveKit).creatorFacet;
+  const shortfallInvitation =
+    await EV(creatorFacet1).makeShortfallReportingInvitation();
+
+  const zoe: ZoeService = await EV.vat('bootstrap').consumeItem('zoe');
+  const brands = await EV(zoe).getBrands(vaultFactoryKit.instance);
+  const getDebtLimitValue = async () => {
+    const params = await EV(vaultFactoryKit.publicFacet).getGovernedParams({
+      collateralBrand: brands.ATOM,
+    });
+
+    // @ts-expect-error getGovernedParams doesn't declare these fields
+    return params.DebtLimit.value.value;
+  };
+
+  // Change the value of a param before the upgrade so paramGovernance is set
+  t.is(await getDebtLimitValue(), 1_000_000_000n);
+  await updateVaultManagerParams(t, gd, ATOM, 50_000_000n);
+
+  t.is(await getDebtLimitValue(), 50_000_000n);
+
+  const privateArgs = {
+    // @ts-expect-error cast XXX missing from type
+    ...vaultFactoryKit.privateArgs,
+    initialPoserInvitation: poserInvitation,
+    initialShortfallInvitation: shortfallInvitation,
+  };
+
+  const vfAdminFacet = await EV(
+    vaultFactoryKit.governorCreatorFacet,
+  ).getAdminFacet();
+
+  t.log('awaiting VaultFactory restartContract');
+  const upgradeResult = await EV(vfAdminFacet).restartContract(privateArgs);
+  t.deepEqual(upgradeResult, { incarnationNumber: 1 });
+
+  await updateVaultManagerParams(t, gd, ATOM, 150_000_000n);
+  t.is(await getDebtLimitValue(), 150_000_000n);
+});

packages/boot/test/bootstrapTests/vaults-upgrade.test.ts

@@ -286,11 +286,16 @@ test.serial('restart vaultFactory', async t => {
   const vaultFactoryKit =
     await EV.vat('bootstrap').consumeItem('vaultFactoryKit');
 
-  const reserveKit = await EV.vat('bootstrap').consumeItem('reserveKit');
   const bootstrapVat = EV.vat('bootstrap');
+  const reserveKit = await bootstrapVat.consumeItem('reserveKit');
   const electorateCreatorFacet = await bootstrapVat.consumeItem(
     'economicCommitteeCreatorFacet',
   );
+  const agoricNames = await EV.vat('bootstrap').consumeItem('agoricNames');
+  const oldVaultInstallation = await EV(agoricNames).lookup(
+    'installation',
+    'VaultFactory',
+  );
 
   const poserInvitation = await EV(electorateCreatorFacet).getPoserInvitation();
   const creatorFacet1 = await EV.get(reserveKit).creatorFacet;
@@ -319,6 +324,13 @@ test.serial('restart vaultFactory', async t => {
   const upgradeResult = await EV(vfAdminFacet).restartContract(privateArgs);
   t.deepEqual(upgradeResult, { incarnationNumber: 1 });
   t.like(readCollateralMetrics(0), keyMetrics); // unchanged
+
+  const newVaultInstallation = await EV(agoricNames).lookup(
+    'installation',
+    'VaultFactory',
+  );
+
+  t.notDeepEqual(newVaultInstallation, oldVaultInstallation);
 });
 
 test.serial('restart contractGovernor', async t => {

packages/builders/scripts/vats/add-auction.js

@@ -7,12 +7,18 @@ export const defaultProposalBuilder = async ({ publishRef, install }) => {
     getManifestCall: [
       'getManifestForAddAuction',
       {
-        auctionsRef: publishRef(
+        auctioneerRef: publishRef(
           install(
             '@agoric/inter-protocol/src/auction/auctioneer.js',
             '../../inter-protocol/bundles/bundle-auctioneer.js',
           ),
         ),
+        contractGovernorRef: publishRef(
+          install(
+            '@agoric/governance/src/contractGovernor.js',
+            '../bundles/bundle-contractGovernor.js',
+          ),
+        ),
       },
     ],
   });

packages/builders/scripts/vats/upgradeVaults.js

@@ -7,12 +7,18 @@ export const defaultProposalBuilder = async ({ publishRef, install }) =>
     getManifestCall: [
       'getManifestForUpgradeVaults',
       {
-        vaultsRef: publishRef(
+        VaultFactoryRef: publishRef(
           install(
             '@agoric/inter-protocol/src/vaultFactory/vaultFactory.js',
             '../bundles/bundle-vaultFactory.js',
           ),
         ),
+        contractGovernorRef: publishRef(
+          install(
+            '@agoric/governance/src/contractGovernor.js',
+            '../bundles/bundle-contractGovernor.js',
+          ),
+        ),
       },
     ],
   });

packages/deploy-script-support/src/coreProposalBehavior.js

@@ -167,6 +167,7 @@ export const makeCoreProposalBehavior = ({
       const installAdmin = E(agoricNamesAdmin).lookupAdmin('installation');
       await Promise.all(
         installationEntries.map(([key, value]) => {
+          produceInstallations[key].reset();
           produceInstallations[key].resolve(value);
           return E(installAdmin).update(key, value);
         }),

packages/governance/src/contractGovernance/governParam.js

@@ -55,14 +55,14 @@ const assertBallotConcernsParam = (paramSpec, questionSpec) => {
 };
 
 /**
- * @param {ERef<ParamManagerRetriever>} paramManagerRetriever
+ * @param {() => ERef<ParamManagerRetriever>} paramManagerRetrieverAccessor
  * @param {Instance} contractInstance
  * @param {import('@agoric/time').TimerService} timer
  * @param {() => Promise<PoserFacet>} getUpdatedPoserFacet
  * @returns {ParamGovernor}
  */
 const setupParamGovernance = (
-  paramManagerRetriever,
+  paramManagerRetrieverAccessor,
   contractInstance,
   timer,
   getUpdatedPoserFacet,
@@ -76,6 +76,7 @@ const setupParamGovernance = (
     deadline,
     paramSpec,
   ) => {
+    const paramManagerRetriever = paramManagerRetrieverAccessor();
     const paramMgr = await E(paramManagerRetriever).get(paramSpec.paramPath);
     /** @type {import('@endo/marshal').Passable} */
     const changePs = {};

packages/governance/src/contractGovernorKit.js

@@ -160,7 +160,7 @@ export const prepareContractGovernorKit = (baggage, powers) => {
             const { timer } = powers;
             const { creatorFacet, instance } = this.state;
             paramGovernance = setupParamGovernance(
-              E(creatorFacet).getParamMgrRetriever(),
+              () => E(creatorFacet).getParamMgrRetriever(),
               instance,
               timer,
               () => this.facets.helper.getUpdatedPoserFacet(),