From fb4e312532ebfa679c61c4214cc2fe20b3161aab Mon Sep 17 00:00:00 2001 From: Chris Hibbert Date: Mon, 13 Sep 2021 15:35:08 -0700 Subject: [PATCH] refactor: add a helper for governedContracts --- packages/governance/src/contractHelper.js | 62 +++++++++++++++++++ packages/governance/src/governParam.js | 3 +- .../contractGovernor/bootstrap.js | 13 ++-- .../contractGovernor/governedContract.js | 60 ++++++------------ .../contractGovernor/test-governor.js | 12 ++-- .../contractGovernor/vat-voter.js | 5 +- .../test/unitTests/test-paramGovernance.js | 44 +++++++------ 7 files changed, 122 insertions(+), 77 deletions(-) create mode 100644 packages/governance/src/contractHelper.js diff --git a/packages/governance/src/contractHelper.js b/packages/governance/src/contractHelper.js new file mode 100644 index 000000000000..925a672fa4f3 --- /dev/null +++ b/packages/governance/src/contractHelper.js @@ -0,0 +1,62 @@ +// @ts-check + +import { Far } from '@agoric/marshal'; +import { assert, details as X, q } from '@agoric/assert'; +import { sameStructure } from '@agoric/same-structure'; + +import { buildParamManager } from './paramManager.js'; + +/* + * Helper for the 90% of contracts that will have only a single set of + * parameters. In order to support managed parameters, a contract only has to + * * define the parameter template, which includes name, type and value + * * call handleParamGovernance() to get makePublicFacet and makeCreatorFacet + * * add any methods needed in the public and creator facets. + */ +const handleParamGovernance = (zcf, governedParamsTemplate) => { + const terms = zcf.getTerms(); + /** @type {ParamDescriptions} */ + const governedParams = terms.main; + const { electionManager } = terms; + + assert( + sameStructure(governedParams, governedParamsTemplate), + X`Terms must include ${q(governedParamsTemplate)}, but were ${q( + governedParams, + )}`, + ); + const paramManager = buildParamManager(governedParams); + + const makePublicFacet = (originalPublicFacet = {}) => { + return Far('publicFacet', { + ...originalPublicFacet, + getSubscription: () => paramManager.getSubscription(), + getContractGovernor: () => electionManager, + getGovernedParamsValues: () => { + return { main: paramManager.getParams() }; + }, + }); + }; + + /** @type {LimitedCreatorFacet} */ + const limitedCreatorFacet = Far('governedContract creator facet', { + getContractGovernor: () => electionManager, + }); + + const makeCreatorFacet = (originalCreatorFacet = Far('creatorFacet', {})) => { + return Far('creatorFacet', { + getParamMgrRetriever: () => { + return Far('paramRetriever', { get: () => paramManager }); + }, + getInternalCreatorFacet: () => originalCreatorFacet, + getLimitedCreatorFacet: () => limitedCreatorFacet, + }); + }; + + return harden({ + makePublicFacet, + makeCreatorFacet, + }); +}; +harden(handleParamGovernance); +export { handleParamGovernance }; diff --git a/packages/governance/src/governParam.js b/packages/governance/src/governParam.js index bca49c9958da..55666e56e324 100644 --- a/packages/governance/src/governParam.js +++ b/packages/governance/src/governParam.js @@ -5,6 +5,7 @@ import { Far } from '@agoric/marshal'; import { makePromiseKit } from '@agoric/promise-kit'; import { sameStructure } from '@agoric/same-structure'; +import { q } from '@agoric/assert'; import { ChoiceMethod, QuorumRule, @@ -52,7 +53,7 @@ const assertBallotConcernsQuestion = (paramName, questionDetails) => { // @ts-ignore typescript isn't sure the question is a paramChangeIssue // if it isn't, the assertion will fail. questionDetails.issue.paramSpec.parameterName === paramName, - X`expected ${paramName} to be included`, + X`expected ${q(paramName)} to be included`, ); }; diff --git a/packages/governance/test/swingsetTests/contractGovernor/bootstrap.js b/packages/governance/test/swingsetTests/contractGovernor/bootstrap.js index 71318e8ebce2..f0e6a0d1f7ea 100644 --- a/packages/governance/test/swingsetTests/contractGovernor/bootstrap.js +++ b/packages/governance/test/swingsetTests/contractGovernor/bootstrap.js @@ -4,7 +4,7 @@ import { E } from '@agoric/eventual-send'; import { Far } from '@agoric/marshal'; import buildManualTimer from '@agoric/zoe/tools/manualTimer.js'; import { observeIteration } from '@agoric/notifier'; -import { governedParameterTerms } from './governedContract.js'; +import { governedParameterInitialValues } from './governedContract.js'; const { quote: q } = assert; @@ -24,7 +24,7 @@ const contractGovernorStart = async ( const { details, instance, outcomeOfUpdate } = await E( contractCreatorFacet, ).voteOnParamChange( - { key: 'contractParams', parameterName: 'MalleableNumber' }, + { key: 'main', parameterName: 'MalleableNumber' }, 299792458n, installations.binaryVoteCounter, 3n, @@ -132,7 +132,7 @@ const oneVoterValidate = async ( const checkContractState = async (zoe, contractInstanceP, log) => { const contractInstance = await contractInstanceP; const contractPublic = E(zoe).getPublicFacet(contractInstance); - let state = await E(contractPublic).getState(); + let paramValues = await E(contractPublic).getGovernedParamsValues(); const subscription = await E(contractPublic).getSubscription(); const paramChangeObserver = Far('param observer', { updateState: update => { @@ -142,8 +142,9 @@ const checkContractState = async (zoe, contractInstanceP, log) => { observeIteration(subscription, paramChangeObserver); // it takes a while for the update to propagate. The second time it seems good - state = await E(contractPublic).getState(); - const malleableNumber = state.MalleableNumber; + paramValues = await E(contractPublic).getGovernedParamsValues(); + const malleableNumber = paramValues.main.MalleableNumber; + log(`current value of ${malleableNumber.name} is ${malleableNumber.value}`); }; @@ -170,7 +171,7 @@ const makeBootstrap = (argv, cb, vatPowers) => async (vats, devices) => { governedContractInstallation: installations.governedContract, governed: { issuerKeywordRecord: {}, - terms: { governedParams: governedParameterTerms }, + terms: { main: governedParameterInitialValues }, }, }; const privateArgs = { registrarCreatorFacet }; diff --git a/packages/governance/test/swingsetTests/contractGovernor/governedContract.js b/packages/governance/test/swingsetTests/contractGovernor/governedContract.js index d4abece3b1b9..1df92f35638e 100644 --- a/packages/governance/test/swingsetTests/contractGovernor/governedContract.js +++ b/packages/governance/test/swingsetTests/contractGovernor/governedContract.js @@ -1,17 +1,13 @@ // @ts-check -import { Far } from '@agoric/marshal'; -import { sameStructure } from '@agoric/same-structure'; - -import { buildParamManager, ParamType } from '../../../src/paramManager.js'; - -const { details: X, quote: q } = assert; +import { handleParamGovernance } from '../../../src/contractHelper.js'; +import { ParamType } from '../../../src/paramManager.js'; const MALLEABLE_NUMBER = 'MalleableNumber'; /** @type {ParameterNameList} */ const governedParameterTerms = { - contractParams: [MALLEABLE_NUMBER], + main: [MALLEABLE_NUMBER], }; /** @type {ParamDescriptions} */ @@ -22,48 +18,28 @@ const governedParameterInitialValues = [ type: ParamType.NAT, }, ]; -harden(governedParameterTerms); +harden(governedParameterInitialValues); /** @type {ContractStartFn} */ const start = async zcf => { - const { electionManager, governedParams } = zcf.getTerms(); - - assert( - sameStructure(governedParams, governedParameterTerms), - X`Terms must include ${MALLEABLE_NUMBER}: ${q(governedParams)}`, + const { makePublicFacet, makeCreatorFacet } = handleParamGovernance( + zcf, + governedParameterInitialValues, ); - const paramManager = buildParamManager(governedParameterInitialValues); - - const getParamMgrRetriever = () => - Far('paramManagerRetriever', { - get: ({ key }) => { - assert( - key === `contractParams`, - X`"contractParams" is a required key: ${key}`, - ); - return paramManager; - }, - }); - const publicFacet = Far('public face of governed contract', { - getState: () => paramManager.getParams(), - getSubscription: () => paramManager.getSubscription(), - getContractGovernor: () => electionManager, - }); - - /** @type {LimitedCreatorFacet} */ - const limitedCreatorFacet = Far('governedContract creator facet', { - getContractGovernor: () => electionManager, - }); - - const creatorFacet = Far('governedContract powerful creator facet', { - getParamMgrRetriever, - getLimitedCreatorFacet: () => limitedCreatorFacet, - }); - return { publicFacet, creatorFacet }; + return { + publicFacet: makePublicFacet({}), + creatorFacet: makeCreatorFacet({}), + }; }; + harden(start); harden(MALLEABLE_NUMBER); harden(governedParameterTerms); -export { start, governedParameterTerms, MALLEABLE_NUMBER }; +export { + start, + governedParameterTerms, + MALLEABLE_NUMBER, + governedParameterInitialValues, +}; diff --git a/packages/governance/test/swingsetTests/contractGovernor/test-governor.js b/packages/governance/test/swingsetTests/contractGovernor/test-governor.js index c3f77201427c..1e48a78bb55e 100644 --- a/packages/governance/test/swingsetTests/contractGovernor/test-governor.js +++ b/packages/governance/test/swingsetTests/contractGovernor/test-governor.js @@ -85,16 +85,16 @@ const main = async (t, argv) => { const expectedcontractGovernorStartLog = [ '=> voter and registrar vats are set up', '@@ schedule task for:3, currently: 0 @@', - 'Voter Alice voted for {"changeParam":{"key":"contractParams","parameterName":"MalleableNumber"},"proposedValue":"[299792458n]"}', - 'Voter Bob voted for {"noChange":{"key":"contractParams","parameterName":"MalleableNumber"}}', - 'Voter Carol voted for {"noChange":{"key":"contractParams","parameterName":"MalleableNumber"}}', - 'Voter Dave voted for {"changeParam":{"key":"contractParams","parameterName":"MalleableNumber"},"proposedValue":"[299792458n]"}', - 'Voter Emma voted for {"changeParam":{"key":"contractParams","parameterName":"MalleableNumber"},"proposedValue":"[299792458n]"}', + 'Voter Alice voted for {"changeParam":{"key":"main","parameterName":"MalleableNumber"},"proposedValue":"[299792458n]"}', + 'Voter Bob voted for {"noChange":{"key":"main","parameterName":"MalleableNumber"}}', + 'Voter Carol voted for {"noChange":{"key":"main","parameterName":"MalleableNumber"}}', + 'Voter Dave voted for {"changeParam":{"key":"main","parameterName":"MalleableNumber"},"proposedValue":"[299792458n]"}', + 'Voter Emma voted for {"changeParam":{"key":"main","parameterName":"MalleableNumber"},"proposedValue":"[299792458n]"}', '@@ tick:1 @@', '@@ tick:2 @@', '@@ tick:3 @@', '&& running a task scheduled for 3. &&', - 'vote outcome: {"changeParam":{"key":"contractParams","parameterName":"MalleableNumber"},"proposedValue":"[299792458n]"}', + 'vote outcome: {"changeParam":{"key":"main","parameterName":"MalleableNumber"},"proposedValue":"[299792458n]"}', 'updated to "[299792458n]"', 'MalleableNumber was changed to "[602214090000000000000000n]"', 'current value of MalleableNumber is 299792458', diff --git a/packages/governance/test/swingsetTests/contractGovernor/vat-voter.js b/packages/governance/test/swingsetTests/contractGovernor/vat-voter.js index 2dfdff75cd54..a3981c5ff9da 100644 --- a/packages/governance/test/swingsetTests/contractGovernor/vat-voter.js +++ b/packages/governance/test/swingsetTests/contractGovernor/vat-voter.js @@ -45,7 +45,7 @@ const build = async (log, zoe) => { governorInstallation, validatedQuestion, ] = await Promise.all([ - E.get(E(zoe).getTerms(governedInstance)).governedParams, + E.get(E(zoe).getTerms(governedInstance)).main, E(E(zoe).getPublicFacet(counterInstance)).getDetails(), E(zoe).getInstallationForInstance(registrarInstance), E(zoe).getInstallationForInstance(counterInstance), @@ -54,8 +54,7 @@ const build = async (log, zoe) => { validateQuestionFromCounterP, ]); - const contractParam = governedParam.contractParams; - assertBallotConcernsQuestion(contractParam[0], questionDetails); + assertBallotConcernsQuestion(governedParam[0].name, questionDetails); assert(installations.binaryVoteCounter === voteCounterInstallation); assert(installations.governedContract === governedInstallation); assert(installations.contractGovernor === governorInstallation); diff --git a/packages/governance/test/unitTests/test-paramGovernance.js b/packages/governance/test/unitTests/test-paramGovernance.js index dafccd6f1846..4f11cfe14a8c 100644 --- a/packages/governance/test/unitTests/test-paramGovernance.js +++ b/packages/governance/test/unitTests/test-paramGovernance.js @@ -19,7 +19,7 @@ import { makeParamChangePositions, } from '../../src/governParam.js'; import { - governedParameterTerms, + governedParameterInitialValues, MALLEABLE_NUMBER, } from '../swingsetTests/contractGovernor/governedContract.js'; @@ -47,7 +47,7 @@ test('governParam happy path with fakes', async t => { const governedFacets = await E(zoe).startInstance( governedInstall, {}, - { governedParams: governedParameterTerms }, + { main: governedParameterInitialValues }, ); const Retriever = governedFacets.creatorFacet.getParamMgrRetriever(); @@ -86,11 +86,13 @@ test('governParam happy path with fakes', async t => { t.fail(e), ); - t.deepEqual(governedFacets.publicFacet.getState(), { - MalleableNumber: { - name: MALLEABLE_NUMBER, - type: 'nat', - value: 25n, + t.deepEqual(governedFacets.publicFacet.getGovernedParamsValues(), { + main: { + MalleableNumber: { + name: MALLEABLE_NUMBER, + type: 'nat', + value: 25n, + }, }, }); }); @@ -107,7 +109,7 @@ test('governParam no votes', async t => { const governedFacets = await E(zoe).startInstance( governedInstall, {}, - { governedParams: governedParameterTerms }, + { main: governedParameterInitialValues }, ); const Retriever = governedFacets.creatorFacet.getParamMgrRetriever(); @@ -152,11 +154,13 @@ test('governParam no votes', async t => { t.is(e, 'no quorum'), ); - t.deepEqual(governedFacets.publicFacet.getState(), { - MalleableNumber: { - name: MALLEABLE_NUMBER, - type: 'nat', - value: 602214090000000000000000n, + t.deepEqual(governedFacets.publicFacet.getGovernedParamsValues(), { + main: { + MalleableNumber: { + name: MALLEABLE_NUMBER, + type: 'nat', + value: 602214090000000000000000n, + }, }, }); }); @@ -173,7 +177,7 @@ test('governParam bad update', async t => { const governedFacets = await E(zoe).startInstance( governedInstall, {}, - { governedParams: governedParameterTerms }, + { main: governedParameterInitialValues }, ); const brokenParamMgr = Far('broken ParamMgr', { getParam: () => { @@ -225,11 +229,13 @@ test('governParam bad update', async t => { 'Expected a throw', ); - t.deepEqual(governedFacets.publicFacet.getState(), { - MalleableNumber: { - name: MALLEABLE_NUMBER, - type: 'nat', - value: 602214090000000000000000n, + t.deepEqual(governedFacets.publicFacet.getGovernedParamsValues(), { + main: { + MalleableNumber: { + name: MALLEABLE_NUMBER, + type: 'nat', + value: 602214090000000000000000n, + }, }, }); });