chore: responses to reviews

fixed typos, etc.
added detail in README on Future Enhancements
Integrated with fees for zoe execution
integrated with ENDO changes to import formatting
Added checking of Installations now that Zoe has getInstallationForInstance

packages/governance/README.md

@@ -9,7 +9,7 @@ that voters can verify that their votes mean what they say and will be tabulated
 as expected.
 
 Any occasion of governance starts with the creation of a Registrar. Two kinds
-exist currently (Stakeholder support will be added shortly] that represent
+exist currently (Stakeholder support will be added shortly) that represent
 committees and stakeholders. The electorate may deal with many questions
 governing many things, so the electorate has to exist before any questions can
 be posed.
@@ -69,7 +69,7 @@ contract instance. The registrar creates new questions, and makes a new instance
 of a BallotCounter so everyone can see how ballots will be counted.
 
 Registrars have a public method to get from the ballot handle to a ballot.
-ballots include the ballotSpec, the BallotCounter instance and closingRule. For
+Ballots include the ballotSpec, the BallotCounter instance and closingRule. For
 contract governance, the question specifies the governed contract instance, the
 parameter to be changed, and the proposed new value.
 
@@ -94,10 +94,9 @@ private facet that contains only the method voteOnParamChange().
 
 When a prospective user of a contract receives a link to an instance of a
 contract, they can check the terms to see if the contract names a governor.  The
-governor's public facet will also refer to the contract it governs. With the
-instance links you'll eventually be able to retrieve the installation from Zoe
-(https://github.com/Agoric/agoric-sdk/issues/3449), which allows you to examine
-the souce.
+governor's public facet will also refer to the contract it governs. Once you
+have the instance you can retrieve the installation from Zoe which allows you to
+examine the source.
 
 The governedContract will provide the registrar, which allows you to check the
 electorate, and retrieve a list of open questions. (We should add closed
@@ -119,3 +118,51 @@ we'll come up with more types.) When it is `PARAM_CHANGE`, the ballotDetails
 will also identify the contract instance, the partcular parameter to be changed,
 and the proposed new value. At present, all parameter change elections are by
 majority vote, and if a majority doesn't vote in favor, then no change is made.
+
+## Future Extensions
+
+The architecture is intended to support several scenarios that haven't been
+filled in yet.
+
+### Registrars
+
+We currently have a committeeRegistrar, which has an opaque group of voters. The
+contract makes no attempt to make the voters legible to others. This might be
+useful for a private group making a decision, or a case where a dictator has the
+ability to appoint a committee that will make decisions.
+
+The ClaimsRegistrar (coming soon!) is a Registrar that gives the ability to vote
+to anyone who has an Attestation payment from the Attestation contract.
+Observers can't tell who the voters are, but they can validate the
+qualifications to vote.
+
+Another plausible registrar would use the result of a public vote to give voting
+facets to the election winners. There would have to be some kind of public
+registration of the identities of the candidates to make them visible.
+
+### BallotCounters
+
+The only ballot counter currently is the BinaryBallotCounter, which presumes
+there are two positions on the ballot and assigns every vote to one or the other
+or to 'spoiled'. At the end, it looks for a majority winner and announces that.
+It can be configured to have one of the possible outcomes as the default
+outcome. If there's a tie and no default, the winner is `undefined`.
+
+ContractGovernance uses this to make 'no change' be the default when voting on
+parameter changes.
+
+We should have ballotCounters for multiple candidate questions. I hope we'll
+eventually have IRV (instant runnoff) and various forms of proportional
+representation.
+
+### ElectionManager
+
+The election manager has a role in governance, but not a specific API. The
+manager's role is to make the setup of particular elections legible to voters
+and other observers. The current example is the ContractGovernor, which manages
+changes to contract parameters. There should also be managers that
+
+* take some action (i.e. add a new collateral type to the AMM) when a vote
+  passes.
+* manage a plebiscite amount stake holders to allow participants to express
+  opinions about the future of the chain.

packages/governance/docs/contracts.puml

@@ -14,8 +14,7 @@ class GovernedContract {
     +terms: { electionManager, governedParams }
     +getState()
     +getContractGovernor()
-    -getContractGovernor
-    -getParamManagerAccessor
+    -getParamManagerAccessor()
 }
 note  left : calls buildParamManager(paramDesc);\nmakes paramMgr state public\nreturns paramMgr in creatorFacet
 

packages/governance/src/ballotBuilder.js

@@ -3,9 +3,9 @@
 import { assert, details as X } from '@agoric/assert';
 import { Far, passStyleOf } from '@agoric/marshal';
 import { sameStructure } from '@agoric/same-structure';
-import { makeHandle } from '@agoric/zoe/src/makeHandle';
+import { makeHandle } from '@agoric/zoe/src/makeHandle.js';
 
-import { assertType, ParamType } from './paramManager';
+import { assertType, ParamType } from './paramManager.js';
 
 /**
  * @type {{

packages/governance/src/committeeRegistrar.js

@@ -5,9 +5,9 @@ import { Far } from '@agoric/marshal';
 import { makeNotifierKit } from '@agoric/notifier';
 import { allComparable } from '@agoric/same-structure';
 import { makeStore } from '@agoric/store';
-import { natSafeMath } from '@agoric/zoe/src/contractSupport';
+import { natSafeMath } from '@agoric/zoe/src/contractSupport/index.js';
 
-import { QuorumRule } from './ballotBuilder';
+import { QuorumRule } from './ballotBuilder.js';
 
 const { ceilDivide } = natSafeMath;
 

packages/governance/src/contractGovernor.js

@@ -5,8 +5,8 @@ import { E } from '@agoric/eventual-send';
 import { Far } from '@agoric/marshal';
 import { makePromiseKit } from '@agoric/promise-kit';
 
-import { setupGovernance, validateParamChangeBallot } from './governParam';
-import { assertContractGovernance } from './validators';
+import { setupGovernance, validateParamChangeBallot } from './governParam.js';
+import { assertContractGovernance } from './validators.js';
 
 /** @type {ValidateBallotDetails} */
 const validateBallotDetails = async (zoe, registrar, details) => {

packages/governance/src/governParam.js

@@ -11,8 +11,8 @@ import {
   QuorumRule,
   ElectionType,
   makeBallotSpec,
-} from './ballotBuilder';
-import { assertType } from './paramManager';
+} from './ballotBuilder.js';
+import { assertType } from './paramManager.js';
 
 /** @type {MakeParamChangePositions} */
 const makeParamChangePositions = (paramSpec, proposedValue) => {

packages/governance/src/paramManager.js

@@ -50,7 +50,8 @@ const assertType = (type, value, name) => {
     case ParamType.INSTALLATION:
       // TODO(3344): add a better assertion once Zoe validates installations
       assert(
-        typeof value === 'object' && !Object.getOwnPropertyNames(value).length,
+        typeof value === 'object' &&
+          Object.getOwnPropertyNames(value).length === 1,
         X`value for ${name} must be an Installation, was ${value}`,
       );
       break;

packages/governance/src/types.js

@@ -25,7 +25,7 @@
 
 /**
  * @typedef {Object} SimpleQuestion
- * @property {string} question
+ * @property {string} text
  */
 
 /**

packages/governance/test/swingsetTests/committeeBinary/bootstrap.js

@@ -10,7 +10,7 @@ import {
   makeBallotSpec,
   QuorumRule,
   ElectionType,
-} from '../../../src/ballotBuilder';
+} from '../../../src/ballotBuilder.js';
 
 const makeVoterVat = async (log, vats, zoe) => {
   const voterCreator = E(vats.voter).build(zoe);

packages/governance/test/swingsetTests/committeeBinary/vat-zoe.js

@@ -14,4 +14,5 @@ export function buildRootObject(vatPowers) {
       const zoe = E(zoeService).bindDefaultFeePurse(feePurse);
       return zoe;
     },
-  })}
+  });
+}

packages/governance/test/swingsetTests/contractGovernor/bootstrap.js

@@ -2,9 +2,9 @@
 
 import { E } from '@agoric/eventual-send';
 import { Far } from '@agoric/marshal';
-import buildManualTimer from '@agoric/zoe/tools/manualTimer';
+import buildManualTimer from '@agoric/zoe/tools/manualTimer.js';
 import { q } from '@agoric/assert';
-import { governedParameterTerms } from './governedContract';
+import { governedParameterTerms } from './governedContract.js';
 
 /**
  * @param {ERef<ZoeService>} zoe
@@ -50,7 +50,6 @@ const installContracts = async (zoe, cb) => {
     committeeRegistrar,
     binaryBallotCounter,
     contractGovernor,
-
     governedContract,
   };
   return installations;
@@ -97,6 +96,7 @@ const oneVoterValidate = async (
   governedInstanceP,
   registrarInstance,
   governorInstanceP,
+  installations,
 ) => {
   const [
     voters,
@@ -116,6 +116,7 @@ const oneVoterValidate = async (
     governedInstance,
     registrarInstance,
     governorInstance,
+    installations,
   );
 };
 
@@ -174,6 +175,7 @@ const makeBootstrap = (argv, cb, vatPowers) => async (vats, devices) => {
         governedInstance,
         registrarInstance,
         governorInstance,
+        installations,
       );
 
       await E(timer).tick();

packages/governance/test/swingsetTests/contractGovernor/governedContract.js

@@ -4,7 +4,7 @@ import { assert, details as X } from '@agoric/assert';
 import { Far } from '@agoric/marshal';
 import { sameStructure } from '@agoric/same-structure';
 
-import { buildParamManager, ParamType } from '../../../src/paramManager';
+import { buildParamManager, ParamType } from '../../../src/paramManager.js';
 
 const MALLEABLE_NUMBER = 'MalleableNumber';
 

packages/governance/test/swingsetTests/contractGovernor/test-governor.js

@@ -1,5 +1,3 @@
-/* global __dirname */
-
 // @ts-check
 
 // TODO Remove babel-standalone preinitialization
@@ -10,6 +8,7 @@ import '@agoric/babel-standalone';
 import '@agoric/install-ses';
 // eslint-disable-next-line import/no-extraneous-dependencies
 import test from 'ava';
+import path from 'path';
 
 import { buildVatController, buildKernelBundles } from '@agoric/swingset-vat';
 import bundleSource from '@agoric/bundle-source';
@@ -24,6 +23,9 @@ const CONTRACT_FILES = [
   },
 ];
 
+const filename = new URL(import.meta.url).pathname;
+const dirname = path.dirname(filename);
+
 test.before(async t => {
   const start = Date.now();
   const kernelBundles = await buildKernelBundles();
@@ -39,7 +41,7 @@ test.before(async t => {
       } else {
         ({ bundleName, contractPath } = settings);
       }
-      const source = `${__dirname}${contractPath}`;
+      const source = `${dirname}${contractPath}`;
       const bundle = await bundleSource(source);
       contractBundles[bundleName] = bundle;
     }),
@@ -49,12 +51,12 @@ test.before(async t => {
   const vats = {};
   await Promise.all(
     ['voter', 'zoe'].map(async name => {
-      const source = `${__dirname}/vat-${name}.js`;
+      const source = `${dirname}/vat-${name}.js`;
       const bundle = await bundleSource(source);
       vats[name] = { bundle };
     }),
   );
-  const bootstrapSource = `${__dirname}/bootstrap.js`;
+  const bootstrapSource = `${dirname}/bootstrap.js`;
   vats.bootstrap = {
     bundle: await bundleSource(bootstrapSource),
     parameters: { contractBundles }, // argv will be added to this

packages/governance/test/swingsetTests/contractGovernor/vat-voter.js

@@ -4,9 +4,9 @@ import { q } from '@agoric/assert';
 import { E } from '@agoric/eventual-send';
 import { Far } from '@agoric/marshal';
 
-import { assertContractRegistrar } from '../../../src/validators';
-import { validateBallotFromCounter } from '../../../src/contractGovernor';
-import { assertBallotConcernsQuestion } from '../../../src/governParam';
+import { assertContractRegistrar } from '../../../src/validators.js';
+import { validateBallotFromCounter } from '../../../src/contractGovernor.js';
+import { assertBallotConcernsQuestion } from '../../../src/governParam.js';
 
 const build = async (log, zoe) => {
   return Far('voter', {
@@ -24,29 +24,38 @@ const build = async (log, zoe) => {
           governedInstance,
           registrarInstance,
           governorInstance,
+          installations,
         ) => {
-          // I'd like to validate Installations, but there doesn't seem to be a
-          // way to get it from an Instance. I'd verify the Registrar,
-          // ballotCounter, and contractGovernor.
-
           await validateBallotFromCounter(
             zoe,
             registrarInstance,
             counterInstance,
           );
 
-          const governedTermsP = E(zoe).getTerms(governedInstance);
-          const governedParamP = E.get(governedTermsP).governedParams;
-          const counterPublicP = E(zoe).getPublicFacet(counterInstance);
-          const ballotDetailsP = E(counterPublicP).getDetails();
-
-          const [governedParam, ballotDetails] = await Promise.all([
-            governedParamP,
-            ballotDetailsP,
+          const [
+            governedParam,
+            ballotDetails,
+            registrarInstallation,
+            ballotCounterInstallation,
+            governedInstallation,
+            governorInstallation,
+          ] = await Promise.all([
+            E.get(E(zoe).getTerms(governedInstance)).governedParams,
+            E(E(zoe).getPublicFacet(counterInstance)).getDetails(),
+            E(zoe).getInstallationForInstance(registrarInstance),
+            E(zoe).getInstallationForInstance(counterInstance),
+            E(zoe).getInstallationForInstance(governedInstance),
+            E(zoe).getInstallationForInstance(governorInstance),
           ]);
 
           const contractParam = governedParam.contractParams;
           assertBallotConcernsQuestion(contractParam[0], ballotDetails);
+          assert(
+            installations.binaryBallotCounter === ballotCounterInstallation,
+          );
+          assert(installations.governedContract === governedInstallation);
+          assert(installations.contractGovernor === governorInstallation);
+          assert(installations.committeeRegistrar === registrarInstallation);
           await assertContractRegistrar(
             zoe,
             governorInstance,

packages/governance/test/swingsetTests/contractGovernor/vat-zoe.js

@@ -2,9 +2,17 @@
 
 import { Far } from '@agoric/marshal';
 
-import { makeZoe } from '@agoric/zoe';
+import { makeZoeKit } from '@agoric/zoe';
+import { E } from '@agoric/eventual-send';
 
-export const buildRootObject = _vatPowers =>
-  Far('root', {
-    buildZoe: vatAdminSvc => makeZoe(vatAdminSvc),
+export function buildRootObject(vatPowers) {
+  return Far('root', {
+    buildZoe: vatAdminSvc => {
+      const shutdownZoeVat = vatPowers.exitVatWithFailure;
+      const { zoeService } = makeZoeKit(vatAdminSvc, shutdownZoeVat);
+      const feePurse = E(zoeService).makeFeePurse();
+      const zoe = E(zoeService).bindDefaultFeePurse(feePurse);
+      return zoe;
+    },
   });
+}

packages/governance/test/unitTests/test-ballotCount.js

@@ -3,18 +3,18 @@
 import { test } from '@agoric/zoe/tools/prepare-test-env-ava.js';
 import '@agoric/zoe/exported.js';
 import { E } from '@agoric/eventual-send';
-import buildManualTimer from '@agoric/zoe/tools/manualTimer';
+import buildManualTimer from '@agoric/zoe/tools/manualTimer.js';
 import { makeHandle } from '@agoric/zoe/src/makeHandle.js';
+import { q } from '@agoric/assert';
 
 import { makeBinaryBallotCounter } from '../../src/binaryBallotCounter.js';
-import { q } from '@agoric/assert';
 import {
   makeBallotSpec,
   ChoiceMethod,
   ElectionType,
   QuorumRule,
-} from '../../src/ballotBuilder';
-import { makeParamChangePositions } from '../../src/governParam';
+} from '../../src/ballotBuilder.js';
+import { makeParamChangePositions } from '../../src/governParam.js';
 
 const QUESTION = harden({ text: 'Fish or cut bait?' });
 const FISH = harden({ text: 'Fish' });