From bd67a7925dea5f104142ae515d700c3bda223329 Mon Sep 17 00:00:00 2001 From: Chris Hibbert Date: Wed, 8 Jan 2025 14:55:54 -0800 Subject: [PATCH 1/3] refactor: support updating governedAPIs at upgrade --- .../src/contractGovernance/governApi.js | 18 +++++++++++------- packages/governance/src/contractGovernorKit.js | 13 ++++++------- packages/governance/src/contractHelper.js | 8 ++++++-- packages/governance/src/types.js | 2 +- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/packages/governance/src/contractGovernance/governApi.js b/packages/governance/src/contractGovernance/governApi.js index f700e6de740..b737825b615 100644 --- a/packages/governance/src/contractGovernance/governApi.js +++ b/packages/governance/src/contractGovernance/governApi.js @@ -12,7 +12,7 @@ import { /** * @import {Passable, RemotableObject} from '@endo/pass-style'; - * @import {Position, ApiGovernor, ApiInvocationIssue, PoserFacet, VoteOnApiInvocation} from '../types.js'; + * @import {Position, ApiGovernor, ApiInvocationIssue, PoserFacet, VoteOnApiInvocation, GovernedApis} from '../types.js'; */ /** @@ -32,15 +32,15 @@ const makeApiInvocationPositions = (apiMethodName, methodArgs) => { /** * manage contracts that allow governance to invoke functions. * - * @param {ERef<{ [methodName: string]: (...args: any) => Passable }>} governedApis - * @param {Array} governedNames names of the governed API methods + * @param {() => ERef} getGovernedApis + * @param {() => Promise>} getGovernedNames names of the governed API methods * @param {ERef} timer * @param {() => Promise} getUpdatedPoserFacet * @returns {ApiGovernor} */ const setupApiGovernance = ( - governedApis, - governedNames, + getGovernedApis, + getGovernedNames, timer, getUpdatedPoserFacet, ) => { @@ -54,6 +54,7 @@ const setupApiGovernance = ( voteCounterInstallation, deadline, ) => { + const governedNames = await getGovernedNames(); governedNames.includes(apiMethodName) || Fail`${apiMethodName} is not a governed API.`; @@ -93,8 +94,10 @@ const setupApiGovernance = ( const outcomeOfUpdate = E(counterPublicFacet) .getOutcome() .then( - /** @type {(outcome: Position) => ERef} */ - outcome => { + /** @type {(outcome: Position) => Promise} */ + async outcome => { + await null; + if (keyEQ(positive, outcome)) { keyEQ(outcome, harden({ apiMethodName, methodArgs })) || Fail`The question's method name (${q( @@ -102,6 +105,7 @@ const setupApiGovernance = ( )}) and args (${methodArgs}) didn't match the outcome ${outcome}`; // E(remote)[name](args) invokes the method named 'name' on remote. + const governedApis = await getGovernedApis(); return E(governedApis) [apiMethodName](...methodArgs) .then(() => { diff --git a/packages/governance/src/contractGovernorKit.js b/packages/governance/src/contractGovernorKit.js index 358c0d8fd3e..ad8aafa0336 100644 --- a/packages/governance/src/contractGovernorKit.js +++ b/packages/governance/src/contractGovernorKit.js @@ -123,15 +123,14 @@ export const prepareContractGovernorKit = (baggage, powers) => { await null; if (!apiGovernance) { trace('awaiting governed API dependencies'); - const [governedApis, governedNames] = await Promise.all([ - E(creatorFacet).getGovernedApis(), - E(creatorFacet).getGovernedApiNames(), - ]); + const governedNames = await E(creatorFacet).getGovernedApiNames(); trace('setupApiGovernance'); apiGovernance = governedNames.length - ? // @ts-expect-error FIXME - setupApiGovernance(governedApis, governedNames, timer, () => - this.facets.helper.getUpdatedPoserFacet(), + ? setupApiGovernance( + () => E(creatorFacet).getGovernedApis(), + () => E(creatorFacet).getGovernedApiNames(), + timer, + () => this.facets.helper.getUpdatedPoserFacet(), ) : { // if we aren't governing APIs, voteOnApiInvocation shouldn't be called diff --git a/packages/governance/src/contractHelper.js b/packages/governance/src/contractHelper.js index e47b0709f54..ca0f906d527 100644 --- a/packages/governance/src/contractHelper.js +++ b/packages/governance/src/contractHelper.js @@ -13,6 +13,7 @@ import { CONTRACT_ELECTORATE } from './contractGovernance/governParam.js'; /** * @import {VoteCounterCreatorFacet, VoteCounterPublicFacet, QuestionSpec, OutcomeRecord, AddQuestion, AddQuestionReturn, GovernanceSubscriptionState, GovernanceTerms, GovernedApis, GovernedCreatorFacet, GovernedPublicFacet} from './types.js'; + * @import {Baggage} from '@agoric/vat-data'; */ export const GOVERNANCE_STORAGE_KEY = 'governance'; @@ -181,7 +182,7 @@ const facetHelpers = (zcf, paramManager) => { * @see {makeVirtualGovernorFacet} * * @template CF - * @param {import('@agoric/vat-data').Baggage} baggage + * @param {Baggage} baggage * @param {CF} limitedCreatorFacet * @param {Record unknown>} [governedApis] */ @@ -190,6 +191,9 @@ const facetHelpers = (zcf, paramManager) => { limitedCreatorFacet, governedApis = {}, ) => { + // Far side-effects the object, and can only be applied once + const farGovernedApis = Far('governedAPIs', governedApis); + const governorFacet = prepareExo( baggage, 'governorFacet', @@ -202,7 +206,7 @@ const facetHelpers = (zcf, paramManager) => { // The contract provides a facet with the APIs that can be invoked by // governance /** @type {() => GovernedApis} */ - getGovernedApis: () => Far('governedAPIs', governedApis), + getGovernedApis: () => farGovernedApis, // The facet returned by getGovernedApis is Far, so we can't see what // methods it has. There's no clean way to have contracts specify the APIs // without also separately providing their names. diff --git a/packages/governance/src/types.js b/packages/governance/src/types.js index 6cd1b20aeba..10ee04bfde5 100644 --- a/packages/governance/src/types.js +++ b/packages/governance/src/types.js @@ -523,7 +523,7 @@ export {}; * @property {CreateQuestion} createQuestion */ -/** @typedef {{ [methodName: string]: (...args: any) => unknown }} GovernedApis */ +/** @typedef {Record unknown>} GovernedApis */ /** * @typedef {object} GovernorPublic From aff557204fc196b0257d75e0ef34c26e40660828 Mon Sep 17 00:00:00 2001 From: Chris Hibbert Date: Wed, 8 Jan 2025 14:56:51 -0800 Subject: [PATCH 2/3] chore: improve message when original contract facets weren't durable --- packages/zoe/src/contractFacet/zcfZygote.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/zoe/src/contractFacet/zcfZygote.js b/packages/zoe/src/contractFacet/zcfZygote.js index a63a25e9fbb..b38274f548f 100644 --- a/packages/zoe/src/contractFacet/zcfZygote.js +++ b/packages/zoe/src/contractFacet/zcfZygote.js @@ -488,9 +488,16 @@ export const makeZCFZygote = async ( publicFacet = undefined, creatorInvitation = undefined, }) => { - const priorCreatorFacet = zcfBaggage.get('creatorFacet'); - const priorPublicFacet = zcfBaggage.get('publicFacet'); - const priorCreatorInvitation = zcfBaggage.get('creatorInvitation'); + let priorCreatorFacet; + let priorPublicFacet; + let priorCreatorInvitation; + try { + priorCreatorFacet = zcfBaggage.get('creatorFacet'); + priorPublicFacet = zcfBaggage.get('publicFacet'); + priorCreatorInvitation = zcfBaggage.get('creatorInvitation'); + } catch (e) { + Fail`restartContract failed: original contract facets were not durable`; + } (priorCreatorFacet === creatorFacet && priorPublicFacet === publicFacet && From 1dc3e9a8a74356d6c873fb78a3b7d897026c8214 Mon Sep 17 00:00:00 2001 From: Chris Hibbert Date: Wed, 8 Jan 2025 14:58:41 -0800 Subject: [PATCH 3/3] test: test that APIs can change on upgrade --- packages/boot/package.json | 1 + .../test/bootstrapTests/governedContract.js | 85 ++++++++ .../test/bootstrapTests/governedContract2.js | 90 +++++++++ .../updateUpgradedVaultParams.test.ts | 2 +- .../test/bootstrapTests/upgradeAPI.test.ts | 183 ++++++++++++++++++ 5 files changed, 360 insertions(+), 1 deletion(-) create mode 100644 packages/boot/test/bootstrapTests/governedContract.js create mode 100644 packages/boot/test/bootstrapTests/governedContract2.js create mode 100644 packages/boot/test/bootstrapTests/upgradeAPI.test.ts diff --git a/packages/boot/package.json b/packages/boot/package.json index 1226c3b8065..cd69c6273a2 100644 --- a/packages/boot/package.json +++ b/packages/boot/package.json @@ -26,6 +26,7 @@ "@agoric/cosmic-swingset": "^0.41.3", "@agoric/ertp": "^0.16.2", "@agoric/fast-usdc": "0.1.0", + "@agoric/governance": "^0.10.3", "@agoric/inter-protocol": "^0.16.1", "@agoric/internal": "^0.3.2", "@agoric/kmarshal": "^0.1.0", diff --git a/packages/boot/test/bootstrapTests/governedContract.js b/packages/boot/test/bootstrapTests/governedContract.js new file mode 100644 index 00000000000..a0ef04b86fb --- /dev/null +++ b/packages/boot/test/bootstrapTests/governedContract.js @@ -0,0 +1,85 @@ +import { + CONTRACT_ELECTORATE, + ParamTypes, + handleParamGovernance, +} from '@agoric/governance'; +import { prepareExoClassKit, provide } from '@agoric/vat-data'; + +/** + * @import {GovernanceTerms} from '@agoric/governance/src/types.js'; + * @import {Baggage} from '@agoric/vat-data'; + */ + +const MALLEABLE_NUMBER = 'MalleableNumber'; + +const makeTerms = (number, invitationAmount) => { + return harden({ + governedParams: { + [MALLEABLE_NUMBER]: { type: ParamTypes.NAT, value: number }, + [CONTRACT_ELECTORATE]: { + type: ParamTypes.INVITATION, + value: invitationAmount, + }, + }, + }); +}; + +/** + * + * @param {ZCF< + * GovernanceTerms<{ + * MalleableNumber: 'nat', + * }>>} zcf + * @param {{initialPoserInvitation: Invitation}} privateArgs + * @param {Baggage} baggage + */ +const start = async (zcf, privateArgs, baggage) => { + const { makeDurableGovernorFacet, params } = await handleParamGovernance( + zcf, + privateArgs.initialPoserInvitation, + { + [MALLEABLE_NUMBER]: ParamTypes.NAT, + }, + ); + + const makeGoverned = prepareExoClassKit( + baggage, + 'governed Public', + undefined, + () => + harden({ + governanceAPICalled: 0, + }), + { + public: { + getNum() { + return params.getMalleableNumber(); + }, + getApiCalled() { + const { governanceAPICalled } = this.state; + return governanceAPICalled; + }, + }, + creator: {}, + governed: { + add1() { + const { state } = this; + state.governanceAPICalled += 1; + }, + }, + }, + ); + const facets = provide(baggage, 'theContract', () => makeGoverned()); + + const { governorFacet } = makeDurableGovernorFacet(baggage, facets.creator, { + add1: () => facets.governed.add1(), + }); + + return { publicFacet: facets.public, creatorFacet: governorFacet }; +}; + +harden(start); +harden(MALLEABLE_NUMBER); +harden(makeTerms); + +export { start, MALLEABLE_NUMBER, makeTerms }; diff --git a/packages/boot/test/bootstrapTests/governedContract2.js b/packages/boot/test/bootstrapTests/governedContract2.js new file mode 100644 index 00000000000..5d0013d3ad8 --- /dev/null +++ b/packages/boot/test/bootstrapTests/governedContract2.js @@ -0,0 +1,90 @@ +import { + CONTRACT_ELECTORATE, + ParamTypes, + handleParamGovernance, +} from '@agoric/governance'; +import { prepareExoClassKit, provide } from '@agoric/vat-data'; + +/** + * @import {GovernanceTerms} from '@agoric/governance/src/types.js'; + * @import {Baggage} from '@agoric/vat-data'; + */ + +const MALLEABLE_NUMBER = 'MalleableNumber'; + +const makeTerms = (number, invitationAmount) => { + return harden({ + governedParams: { + [MALLEABLE_NUMBER]: { type: ParamTypes.NAT, value: number }, + [CONTRACT_ELECTORATE]: { + type: ParamTypes.INVITATION, + value: invitationAmount, + }, + }, + }); +}; + +/** + * + * @param {ZCF< + * GovernanceTerms<{ + * MalleableNumber: 'nat', + * }>>} zcf + * @param {{initialPoserInvitation: Invitation}} privateArgs + * @param {Baggage} baggage + */ +const start = async (zcf, privateArgs, baggage) => { + const { makeDurableGovernorFacet, params } = await handleParamGovernance( + zcf, + privateArgs.initialPoserInvitation, + { + [MALLEABLE_NUMBER]: ParamTypes.NAT, + }, + ); + + const makeGoverned = prepareExoClassKit( + baggage, + 'governed Public', + undefined, + () => + harden({ + governanceAPICalled: 0, + }), + { + public: { + getNum() { + return params.getMalleableNumber(); + }, + getApiCalled() { + const { governanceAPICalled } = this.state; + return governanceAPICalled; + }, + }, + creator: {}, + governed: { + add1() { + const { state } = this; + state.governanceAPICalled += 1; + }, + add2() { + const { state } = this; + state.governanceAPICalled += 2; + }, + }, + }, + ); + const facets = provide(baggage, 'theContract', () => makeGoverned()); + + const { governorFacet } = makeDurableGovernorFacet(baggage, facets.creator, { + add1: () => facets.governed.add1(), + add2: () => facets.governed.add2(), + }); + + return { publicFacet: facets.public, creatorFacet: governorFacet }; +}; + +harden(start); +harden(MALLEABLE_NUMBER); +harden(makeTerms); + +export { start, MALLEABLE_NUMBER, makeTerms }; diff --git a/packages/boot/test/bootstrapTests/updateUpgradedVaultParams.test.ts b/packages/boot/test/bootstrapTests/updateUpgradedVaultParams.test.ts index 4f97e7ae909..37edf6fcb39 100644 --- a/packages/boot/test/bootstrapTests/updateUpgradedVaultParams.test.ts +++ b/packages/boot/test/bootstrapTests/updateUpgradedVaultParams.test.ts @@ -3,7 +3,7 @@ * 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. + * param again, to show that the bug is fixed. */ import { test as anyTest } from '@agoric/zoe/tools/prepare-test-env-ava.js'; diff --git a/packages/boot/test/bootstrapTests/upgradeAPI.test.ts b/packages/boot/test/bootstrapTests/upgradeAPI.test.ts new file mode 100644 index 00000000000..27ad2c73ab1 --- /dev/null +++ b/packages/boot/test/bootstrapTests/upgradeAPI.test.ts @@ -0,0 +1,183 @@ +import { test as anyTest } from '@agoric/swingset-vat/tools/prepare-test-env-ava.js'; +import type { TestFn } from 'ava'; +import path from 'path'; +import bundleSource from '@endo/bundle-source'; +import { CONTRACT_ELECTORATE, ParamTypes } from '@agoric/governance'; +import { MALLEABLE_NUMBER } from '@agoric/governance/test/swingsetTests/contractGovernor/governedContract.js'; +import { makeSwingsetTestKit } from '../../tools/supports.js'; + +const dirname = path.dirname(new URL(import.meta.url).pathname); + +const GOVERNED_CONTRACT_SRC = './governedContract.js'; +const GOVERNED_CONTRACT2_SRC = './governedContract2.js'; + +const setUpGovernedContract = async (zoe, timer, EV, controller) => { + const installBundle = contractBundle => EV(zoe).install(contractBundle); + const installBundleToVatAdmin = contractBundle => + controller.validateAndInstallBundle(contractBundle); + const source = `${dirname}/${GOVERNED_CONTRACT_SRC}`; + const source2 = `${dirname}/${GOVERNED_CONTRACT2_SRC}`; + const governedContractBundle = await bundleSource(source); + const governedContract2Bundle = await bundleSource(source2); + + const agoricNames = await EV.vat('bootstrap').consumeItem('agoricNames'); + const governorInstallation = await EV(agoricNames).lookup( + 'installation', + 'contractGovernor', + ); + const voteCounterInstallation = await EV(agoricNames).lookup( + 'installation', + 'binaryVoteCounter', + ); + + const electorateCreatorFacet = await EV.vat('bootstrap').consumeItem( + 'economicCommitteeCreatorFacet', + ); + const poserInvitation = await EV(electorateCreatorFacet).getPoserInvitation(); + const poserInvitation2 = await EV( + electorateCreatorFacet, + ).getPoserInvitation(); + + const invitationIssuer = await EV(zoe).getInvitationIssuer(); + const invitationAmount = + await EV(invitationIssuer).getAmountOf(poserInvitation); + + const governedTerms = { + governedParams: { + [MALLEABLE_NUMBER]: { + type: ParamTypes.NAT, + value: 602214090000000000000000n, + }, + [CONTRACT_ELECTORATE]: { + type: ParamTypes.INVITATION, + value: invitationAmount, + }, + }, + governedApis: ['governanceApi'], + }; + + const governedInstallation = await installBundle(governedContractBundle); + await installBundleToVatAdmin(governedContract2Bundle); + const governorTerms = { + timer, + governedContractInstallation: governedInstallation, + governed: { + terms: governedTerms, + issuerKeywordRecord: {}, + }, + }; + + const governorFacets = await EV(zoe).startInstance( + governorInstallation, + {}, + governorTerms, + { + governed: { + initialPoserInvitation: poserInvitation, + }, + }, + ); + + return { + governorFacets, + invitationAmount, + voteCounterInstallation, + contract2SHA: governedContract2Bundle.endoZipBase64Sha512, + poserInvitation2, + }; +}; + +// A more minimal set would be better. We need governance, but not econ vats. +const PLATFORM_CONFIG = '@agoric/vm-config/decentral-test-vaults-config.json'; + +const makeDefaultTestContext = async t => { + console.time('DefaultTestContext'); + const swingsetTestKit = await makeSwingsetTestKit(t.log, undefined, { + configSpecifier: PLATFORM_CONFIG, + }); + + const { runUtils, storage, controller } = swingsetTestKit; + console.timeLog('DefaultTestContext', 'swingsetTestKit'); + const { EV } = runUtils; + const zoe: ZoeService = await EV.vat('bootstrap').consumeItem('zoe'); + const timer = await EV.vat('bootstrap').consumeItem('chainTimerService'); + + const facets = await setUpGovernedContract(zoe, timer, EV, controller); + + return { ...swingsetTestKit, facets }; +}; + +const test = anyTest as TestFn< + Awaited> +>; + +test.before(async t => { + t.context = await makeDefaultTestContext(t); +}); + +test.after.always(t => { + return t.context.shutdown && t.context.shutdown(); +}); + +test(`start contract; verify`, async t => { + const { runUtils, facets } = t.context; + const { + governorFacets: { creatorFacet }, + } = facets; + const { EV } = runUtils; + const contractPublicFacet = await EV(creatorFacet).getPublicFacet(); + + const avogadro = await EV(contractPublicFacet).getNum(); + t.is(await EV(contractPublicFacet).getApiCalled(), 0); + t.is(avogadro, 602214090000000000000000n); +}); + +test(`verify API governance`, async t => { + const { runUtils, facets } = t.context; + const { + governorFacets: { creatorFacet }, + voteCounterInstallation: vci, + } = facets; + + const { EV } = runUtils; + + const question = await EV(creatorFacet).voteOnApiInvocation( + 'add1', + [], + vci, + 37n, + ); + t.truthy(question.instance); + + await t.throwsAsync( + () => EV(creatorFacet).voteOnApiInvocation('add2', [], vci, 37n), + { + message: /"add2" is not a governed API./, + }, + ); +}); + +test(`upgrade; verify enhanced API governance`, async t => { + const { runUtils, facets } = t.context; + const { + governorFacets: { creatorFacet }, + voteCounterInstallation: vci, + contract2SHA, + poserInvitation2, + } = facets; + + const { EV } = runUtils; + const af = await EV(creatorFacet).getAdminFacet(); + + await EV(af).upgradeContract(`b1-${contract2SHA}`, { + initialPoserInvitation: poserInvitation2, + }); + + const question2 = await EV(creatorFacet).voteOnApiInvocation( + 'add2', + [], + vci, + 37n, + ); + t.truthy(question2.instance); +});