From 4839907f705a663f243aab70857aa2cc1747faa5 Mon Sep 17 00:00:00 2001 From: Chris Hibbert Date: Thu, 1 Aug 2024 15:42:27 -0700 Subject: [PATCH 1/2] feat: add the ability to override param values When upgrading a contract, some parameter values originally defined based on terms must be overridden by values supplied via privateArgs. This provides a path to supply those overrides when creating the new paramManager. The vaultFactory uses this new feature to pass values through. The proposal to upgrade vaultFactory gets the current values as updated by governance and passes them in when upgrading. --- .../contractGovernance/typedParamManager.js | 23 ++++++++++------ .../test/unitTests/typedParamManager.test.js | 24 +++++++++++++++++ .../src/proposals/upgrade-vaults.js | 27 +++++++++++++++++++ .../src/vaultFactory/vaultFactory.js | 4 ++- 4 files changed, 69 insertions(+), 9 deletions(-) diff --git a/packages/governance/src/contractGovernance/typedParamManager.js b/packages/governance/src/contractGovernance/typedParamManager.js index cc6f82e732c..ecd80d844f4 100644 --- a/packages/governance/src/contractGovernance/typedParamManager.js +++ b/packages/governance/src/contractGovernance/typedParamManager.js @@ -141,6 +141,7 @@ harden(makeParamManagerSync); * @param {ZCF>} zcf * @param {I} invitations invitation objects, which must come from privateArgs * @param {M} paramTypesMap + * @param {object} [overrides] * @returns {TypedParamManager} */ export const makeParamManagerFromTerms = ( @@ -148,17 +149,23 @@ export const makeParamManagerFromTerms = ( zcf, invitations, paramTypesMap, + overrides, ) => { + if (overrides) { + console.log('TPM ', { overrides }); + } + const { governedParams } = zcf.getTerms(); - /** @type {Array<[Keyword, SyncSpecTuple | AsyncSpecTuple]>} */ + /** @type {Array<[Keyword, (SyncSpecTuple | AsyncSpecTuple)]>} */ const makerSpecEntries = Object.entries(paramTypesMap).map( - ([paramKey, paramType]) => [ - paramKey, - /** @type {SyncSpecTuple} */ ([ - paramType, - governedParams[paramKey].value, - ]), - ], + ([paramKey, paramType]) => { + const value = + overrides && overrides[paramKey] + ? overrides[paramKey] + : governedParams[paramKey].value; + + return [paramKey, /** @type {SyncSpecTuple} */ ([paramType, value])]; + }, ); // Every governed contract has an Electorate param that starts as `initialPoserInvitation` private arg for (const [name, invitation] of Object.entries(invitations)) { diff --git a/packages/governance/test/unitTests/typedParamManager.test.js b/packages/governance/test/unitTests/typedParamManager.test.js index a4ef53487db..fcc4255bdbb 100644 --- a/packages/governance/test/unitTests/typedParamManager.test.js +++ b/packages/governance/test/unitTests/typedParamManager.test.js @@ -361,3 +361,27 @@ test('Unknown', async t => { await paramManager.updateParams({ Surprise: ['gift', 'party'] }); t.deepEqual(paramManager.getSurprise(), ['gift', 'party']); }); + +test('makeParamManagerFromTerms overrides', async t => { + const terms = harden({ + governedParams: { + Mmr: { type: 'nat', value: makeRatio(150n, drachmaKit.brand) }, + }, + }); + const issuerKeywordRecord = harden({ + Ignore: drachmaKit.issuer, + }); + const { zcf } = await setupZCFTest(issuerKeywordRecord, terms); + + const paramManager = await makeParamManagerFromTerms( + makeStoredPublisherKit(), + // @ts-expect-error missing governance terms + zcf, + { Electorate: zcf.makeInvitation(() => null, 'mock poser invitation') }, + { + Mmr: 'ratio', + }, + { Mmr: makeRatio(100n, drachmaKit.brand) }, + ); + t.deepEqual(paramManager.getMmr().numerator.value, 100n); +}); diff --git a/packages/inter-protocol/src/proposals/upgrade-vaults.js b/packages/inter-protocol/src/proposals/upgrade-vaults.js index 7c8547bf2c6..b6ff0d9cc79 100644 --- a/packages/inter-protocol/src/proposals/upgrade-vaults.js +++ b/packages/inter-protocol/src/proposals/upgrade-vaults.js @@ -85,6 +85,32 @@ export const upgradeVaults = async (powers, { options }) => { } } + const restoreDirectorParams = async () => { + const { publicFacet: directorPF } = kit; + + await null; + + const subscription = E(directorPF).getElectorateSubscription(); + const notifier = makeNotifierFromAsyncIterable(subscription); + let { value, updateCount } = await notifier.getUpdateSince(0n); + // @ts-expect-error It's an amount. + while (AmountMath.isEmpty(value.current.MinInitialDebt.value)) { + ({ value, updateCount } = await notifier.getUpdateSince(updateCount)); + trace( + `minInitialDebt was empty, retried`, + value.current.MinInitialDebt.value, + ); + } + + return harden({ + MinInitialDebt: value.current.MinInitialDebt.value, + ReferencedUI: value.current.ReferencedUI.value, + RecordingPeriod: value.current.RecordingPeriod.value, + ChargingPeriod: value.current.ChargingPeriod.value, + }); + }; + const directorParamOverrides = await restoreDirectorParams(); + const readManagerParams = async () => { const { publicFacet: directorPF } = kit; @@ -138,6 +164,7 @@ export const upgradeVaults = async (powers, { options }) => { initialPoserInvitation: poserInvitation, initialShortfallInvitation: shortfallInvitation, managerParams: managerParamValues, + directorParamOverrides, }); const upgradeResult = await E(kit.adminFacet).upgradeContract( diff --git a/packages/inter-protocol/src/vaultFactory/vaultFactory.js b/packages/inter-protocol/src/vaultFactory/vaultFactory.js index 595fdc84e20..b2d27dca2f6 100644 --- a/packages/inter-protocol/src/vaultFactory/vaultFactory.js +++ b/packages/inter-protocol/src/vaultFactory/vaultFactory.js @@ -74,6 +74,7 @@ harden(meta); * string, * import('./params.js').VaultManagerParamOverrides * >; + * directorParamOverrides: [object]; * }} privateArgs * @param {import('@agoric/swingset-liveslots').Baggage} baggage */ @@ -86,6 +87,7 @@ export const start = async (zcf, privateArgs, baggage) => { storageNode, auctioneerInstance, managerParams, + directorParamOverrides, } = privateArgs; trace('awaiting debtMint'); @@ -117,7 +119,6 @@ export const start = async (zcf, privateArgs, baggage) => { marshaller, ); /** a powerful object; can modify the invitation */ - trace('awaiting makeParamManagerFromTerms'); const vaultDirectorParamManager = await makeParamManagerFromTerms( { publisher: governanceSubscriptionKit.publication, @@ -129,6 +130,7 @@ export const start = async (zcf, privateArgs, baggage) => { [SHORTFALL_INVITATION_KEY]: initialShortfallInvitation, }, vaultDirectorParamTypes, + directorParamOverrides, ); const director = provideDirector( From ef2963a33eb1e6d9b2806fb910149072c2f32f95 Mon Sep 17 00:00:00 2001 From: Chris Hibbert Date: Mon, 5 Aug 2024 14:19:43 -0700 Subject: [PATCH 2/2] chore: improved variable naming and error handling --- .../src/proposals/upgrade-vaults.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/inter-protocol/src/proposals/upgrade-vaults.js b/packages/inter-protocol/src/proposals/upgrade-vaults.js index b6ff0d9cc79..2a92ebd6515 100644 --- a/packages/inter-protocol/src/proposals/upgrade-vaults.js +++ b/packages/inter-protocol/src/proposals/upgrade-vaults.js @@ -85,7 +85,7 @@ export const upgradeVaults = async (powers, { options }) => { } } - const restoreDirectorParams = async () => { + const readCurrentDirectorParams = async () => { const { publicFacet: directorPF } = kit; await null; @@ -109,7 +109,7 @@ export const upgradeVaults = async (powers, { options }) => { ChargingPeriod: value.current.ChargingPeriod.value, }); }; - const directorParamOverrides = await restoreDirectorParams(); + const directorParamOverrides = await readCurrentDirectorParams(); const readManagerParams = async () => { const { publicFacet: directorPF } = kit; @@ -125,9 +125,17 @@ export const upgradeVaults = async (powers, { options }) => { const notifier = makeNotifierFromAsyncIterable(subscription); let { value, updateCount } = await notifier.getUpdateSince(0n); // @ts-expect-error It's an amount. - while (AmountMath.isEmpty(value.current.DebtLimit.value)) { + if (AmountMath.isEmpty(value.current.DebtLimit.value)) { + // The parameters might have been empty at start, and the notifier might + // give the first state before the current state. + trace(`debtLimit was empty, retrying`, value.current.DebtLimit.value); ({ value, updateCount } = await notifier.getUpdateSince(updateCount)); - trace(`debtLimit was empty, retried`, value.current.DebtLimit.value); + + // @ts-expect-error It's an amount. + if (AmountMath.isEmpty(value.current.DebtLimit.value)) { + trace('debtLimit was empty after retrying'); + throw Error('🚨Governed parameters empty after retry, Giving up'); + } } trace(kwd, 'params at', updateCount, 'are', value.current); params[kwd] = harden({