chore: cleanup types, and comments, etc.

add an index.js
consistently type looksLikeFoo()
an assertion for the governor's installation in assertContractGovernance()

packages/governance/exported.js

@@ -0,0 +1 @@
+import './src/types.js';

packages/governance/src/committeeRegistrar.js

@@ -12,15 +12,17 @@ import { QuorumRule } from './question.js';
 
 const { ceilDivide } = natSafeMath;
 
-// Each CommitteeRegistrar represents a particular set of voters. The number of
-// voters is visible in the terms.
-//
-// This contract creates an electorate that is not visible to observers. There
-// may be uses for such a structure, but it is not appropriate for elections
-// where the set of voters needs to be known, unless the contract is used in a
-// way that makes the distribution of voter facets visible.
-
-/** @type {ContractStartFn} */
+/**
+ * Each CommitteeRegistrar represents a particular set of voters. The number of
+ * voters is visible in the terms.
+ *
+ * This contract creates an electorate that is not visible to observers. There
+ * may be uses for such a structure, but it is not appropriate for elections
+ * where the set of voters needs to be known, unless the contract is used in a
+ * way that makes the distribution of voter facets visible.
+ *
+ *  @type {ContractStartFn}
+ */
 const start = zcf => {
   /**
    * @typedef {Object} QuestionRecord
@@ -31,7 +33,6 @@ const start = zcf => {
   /** @type {Store<Handle<'Question'>, QuestionRecord>} */
   const allQuestions = makeStore('Question');
   const { subscription, publication } = makeSubscriptionKit();
-  const invitations = [];
 
   const getOpenQuestions = async () => {
     const isOpenPQuestions = allQuestions.keys().map(key => {
@@ -69,9 +70,10 @@ const start = zcf => {
   };
 
   const { committeeName, committeeSize } = zcf.getTerms();
-  for (let i = 0; i < committeeSize; i += 1) {
-    invitations[i] = makeCommitteeVoterInvitation(i);
-  }
+
+  const invitations = harden(
+    [...Array(committeeSize).keys()].map(makeCommitteeVoterInvitation),
+  );
 
   /** @type {AddQuestion} */
   const addQuestion = async (voteCounter, questionSpec) => {

packages/governance/src/contractGovernor.js

@@ -4,7 +4,6 @@ import { E } from '@agoric/eventual-send';
 import { Far } from '@agoric/marshal';
 
 import { setupGovernance, validateParamChangeQuestion } from './governParam.js';
-import { assertContractGovernance } from './validators.js';
 
 const { details: X } = assert;
 
@@ -20,8 +19,6 @@ const validateQuestionDetails = async (zoe, registrar, details) => {
     .electionManager;
   const governorPublic = E(zoe).getPublicFacet(governorInstance);
 
-  await assertContractGovernance(zoe, governedInstance, governorInstance);
-
   return Promise.all([
     E(governorPublic).validateVoteCounter(counterInstance),
     E(governorPublic).validateRegistrar(registrar),

packages/governance/src/index.js

@@ -0,0 +1,31 @@
+// @ts-check
+
+export {
+  ChoiceMethod,
+  ElectionType,
+  QuorumRule,
+  looksLikeQuestionSpec,
+  positionIncluded,
+  looksLikeIssueForType,
+  buildUnrankedQuestion,
+} from './question.js';
+
+export {
+  validateQuestionDetails,
+  validateQuestionFromCounter,
+} from './contractGovernor.js';
+
+export { handleParamGovernance } from './contractHelper.js';
+
+export {
+  makeParamChangePositions,
+  validateParamChangeQuestion,
+  assertBallotConcernsQuestion,
+} from './governParam.js';
+
+export { ParamType, assertType } from './paramManager.js';
+
+export {
+  assertContractGovernance,
+  assertContractRegistrar,
+} from './validators.js';

packages/governance/src/question.js

@@ -56,23 +56,26 @@ const QuorumRule = {
   ALL: 'all',
 };
 
-/** @param {SimpleIssue} issue */
+/** @type {LooksLikeSimpleIssue} */
 const looksLikeSimpleIssue = issue => {
-  assert.typeof(
-    issue.text,
-    'string',
+  assert.typeof(issue, 'object', X`Issue ("${issue}") must be a record`);
+  assert(
+    issue && typeof issue.text === 'string',
     X`Issue ("${issue}") must be a record with text: aString`,
   );
+  return undefined;
 };
 
-/** @param {ParamChangeIssue} issue */
+/** @type {LooksLikeParamChangeIssue} */
 const looksLikeParamChangeIssue = issue => {
+  assert(issue, X`argument to looksLikeParamChangeIssue cannot be null`);
+  assert.typeof(issue, 'object', X`Issue ("${issue}") must be a record`);
   assert.typeof(
-    issue.paramSpec,
+    issue && issue.paramSpec,
     'object',
     X`Issue ("${issue}") must be a record with paramSpec: anObject`,
   );
-  assert(issue.proposedValue);
+  assert(issue && issue.proposedValue);
   assertType(ParamType.INSTANCE, issue.contract, 'contract');
 };
 
@@ -101,9 +104,16 @@ const positionIncluded = (positions, p) =>
   positions.some(e => sameStructure(e, p));
 
 // QuestionSpec contains the subset of QuestionDetails that can be specified before
+/** @type {LooksLikeClosingRule} */
 function looksLikeClosingRule(closingRule) {
-  const { timer, deadline } = closingRule;
-  Nat(deadline);
+  assert(closingRule, X`argument to looksLikeClosingRule cannot be null`);
+  assert.typeof(
+    closingRule,
+    'object',
+    X`ClosingRule ("${closingRule}") must be a record`,
+  );
+  Nat(closingRule && closingRule.deadline);
+  const timer = closingRule && closingRule.timer;
   assert(passStyleOf(timer) === 'remotable', X`Timer must be a timer ${timer}`);
 }
 

packages/governance/src/types.js

@@ -168,7 +168,6 @@
  *   available to the Registrar, which should wrap and attenuate it so each
  *   voter gets only the ability to cast their own vote at a weight specified by
  *   the registrar.
- *   attenuated wrapper that doesn't make that available!
  * @property {SubmitVote} submitVote
  */
 
@@ -205,14 +204,33 @@
 
 /**
  * @callback LooksLikeQuestionSpec
- * @param {QuestionSpec} allegedQuestionSpec
+ * @param {unknown} allegedQuestionSpec
  * @returns {QuestionSpec}
  */
 
+/**
+ * @callback LooksLikeParamChangeIssue
+ * @param {unknown} issue
+ * @returns { asserts issue is ParamChangeIssue }
+ */
+
 /**
  * @callback LooksLikeIssueForType
  * @param {ElectionType} electionType
- * @param {Issue} issue
+ * @param {unknown} issue
+ * @returns { asserts issue is Issue }
+ */
+
+/**
+ * @callback LooksLikeSimpleIssue
+ * @param {unknown} issue
+ * @returns { asserts issue is SimpleIssue }
+ */
+
+/**
+ * @callback LooksLikeClosingRule
+ * @param {unknown} closingRule
+ * @returns { asserts closingRule is ClosingRule }
  */
 
 /**
@@ -225,10 +243,10 @@
 /**
  * @typedef {Object} RegistrarPublic
  * @property {() => Subscription<QuestionDetails>} getQuestionSubscription
- * @property {() => ERef<Handle<'Question'>[]>} getOpenQuestions,
+ * @property {() => Promise<Handle<'Question'>[]>} getOpenQuestions,
  * @property {() => string} getName
  * @property {() => Instance} getInstance
- * @property {(h: Handle<'Question'>) => ERef<Question>} getQuestion
+ * @property {(h: Handle<'Question'>) => Promise<Question>} getQuestion
  */
 
 /**
@@ -241,16 +259,16 @@
  *  addQuestion() can be used directly when the creator doesn't need any
  *  reassurance. When someone needs to connect addQuestion to the Registrar
  *  instance, getPoserInvitation() lets them get addQuestion with assurance.
- * @property {() => ERef<Invitation>} getPoserInvitation
+ * @property {() => Promise<Invitation>} getPoserInvitation
  * @property {AddQuestion} addQuestion
- * @property {() => Invitation[]} getVoterInvitations
+ * @property {() => Promise<Invitation>[]} getVoterInvitations
  * @property {() => Subscription<QuestionDetails>} getQuestionSubscription
  * @property {() => RegistrarPublic} getPublicFacet
  */
 
 /**
  * @typedef {Object} ClosingRule
- * @property {Timer} timer
+ * @property {ERef<Timer>} timer
  * @property {Timestamp} deadline
  */
 
@@ -391,8 +409,8 @@
 
 /**
  * @typedef {Object} GovernorPublic
- * @property {() => ERef<Instance>} getRegistrar
- * @property {() => ERef<Instance>} getGovernedContract
+ * @property {() => Promise<Instance>} getRegistrar
+ * @property {() => Promise<Instance>} getGovernedContract
  * @property {(voteCounter: Instance) => Promise<boolean>} validateVoteCounter
  * @property {(regP: ERef<Instance>) => Promise<boolean>} validateRegistrar
  * @property {(details: QuestionDetails) => boolean} validateTimer
@@ -427,18 +445,18 @@
  *   A powerful facet that carries access to both the creatorFacet to be passed
  *   to the caller and the paramManager, which will be used exclusively by the
  *   ContractGovenor.
- * @property {() => ERef<LimitedCreatorFacet>} getLimitedCreatorFacet
+ * @property {() => Promise<LimitedCreatorFacet>} getLimitedCreatorFacet
  * @property {() => ParamManagerRetriever} getParamMgrRetriever
  */
 
 /**
  * @typedef {Object} GovernedContractCreatorFacet
  * @property {VoteOnParamChange} voteOnParamChange
- * @property {() => ERef<LimitedCreatorFacet>} getCreatorFacet - creator
+ * @property {() => Promise<LimitedCreatorFacet>} getCreatorFacet - creator
  *   facet of the governed contract, without the tightly held ability to change
  *   param values.
  * @property {() => any} getPublicFacet - public facet of the governed contract
- * @property {() => ERef<Instance>} getInstance - instance of the governed
+ * @property {() => Promise<Instance>} getInstance - instance of the governed
  *   contract
  */
 
@@ -506,7 +524,8 @@
  * @param {ERef<ZoeService>} zoe
  * @param {Instance} allegedGoverned
  * @param {Instance} allegedGovernor
- * @returns {GovernancePair}
+ * @param {Installation} contractGovernorInstallation
+ * @returns {Promise<GovernancePair>}
  */
 
 /**
@@ -523,7 +542,7 @@
  *
  * Validate that the question details correspond to a parameter change question
  * that the registrar hosts, and that the voteCounter and other details are
- * consistent.
+ * consistent with it.
  *
  * @param {ERef<ZoeService>} zoe
  * @param {Instance} registrar

packages/governance/src/validators.js

@@ -16,10 +16,12 @@ const assertContractGovernance = async (
   zoe,
   allegedGoverned,
   allegedGovernor,
+  contractGovernorInstallation,
 ) => {
   const allegedGovernorPF = E(zoe).getPublicFacet(allegedGovernor);
   const realGovernedP = E(allegedGovernorPF).getGovernedContract();
   const allegedGovernedTermsP = E(zoe).getTerms(allegedGoverned);
+
   const [
     { electionManager: realGovernorInstance },
     realGovernedInstance,
@@ -35,7 +37,15 @@ const assertContractGovernance = async (
     X`The alleged governed did not match the governed contract retrieved from the governor`,
   );
 
-  // TODO(3344): assert the installation once Zoe validates installations
+  const governorInstallationFromGoverned = await E(
+    zoe,
+  ).getInstallationForInstance(realGovernorInstance);
+
+  assert(
+    governorInstallationFromGoverned === contractGovernorInstallation,
+    X`The governed contract is not governed by an instance of the provided installation.`,
+  );
+
   return { governor: realGovernorInstance, governed: realGovernedInstance };
 };
 

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

@@ -9,7 +9,7 @@ import {
   QuorumRule,
   ElectionType,
   looksLikeQuestionSpec,
-} from '../../../src/question.js';
+} from '../../../src/index.js';
 
 const { quote: q } = assert;
 
@@ -24,7 +24,7 @@ const createQuestion = async (qDetails, closingTime, tools, quorumRule) => {
   const { issue, positions, electionType } = qDetails;
   const closingRule = {
     timer: tools.timer,
-    deadline: 3n,
+    deadline: closingTime,
   };
 
   const questionSpec = looksLikeQuestionSpec(
@@ -66,8 +66,7 @@ const committeeBinaryStart = async (
     positions: [harden({ text: 'Eeny' }), harden({ text: 'Meeny' })],
     electionType,
   };
-  const eeny = details.positions[0];
-  const meeny = details.positions[1];
+  const [eeny, meeny] = details.positions;
   const tools = { registrarFacet, installations, timer };
   const { counterInstance } = await createQuestion(
     details,

packages/governance/test/swingsetTests/committeeBinary/test-committee.js

@@ -101,7 +101,7 @@ const expectedCommitteeBinaryTwoQuestionsLog = [
   'Carol voted on {"text":"Choose"} for {"text":"Two Potato"}',
   'Dave voted on {"text":"Choose"} for {"text":"One Potato"}',
   'Emma voted on {"text":"Choose"} for {"text":"One Potato"}',
-  '@@ schedule task for:3, currently: 0 @@',
+  '@@ schedule task for:4, currently: 0 @@',
   'Alice voted on {"text":"How high?"} for {"text":"1 foot"}',
   'Bob voted on {"text":"How high?"} for {"text":"2 feet"}',
   'Carol voted on {"text":"How high?"} for {"text":"1 foot"}',
@@ -117,8 +117,8 @@ const expectedCommitteeBinaryTwoQuestionsLog = [
   '@@ tick:2 @@',
   '@@ tick:3 @@',
   '&& running a task scheduled for 3. &&',
-  '&& running a task scheduled for 3. &&',
   '@@ tick:4 @@',
+  '&& running a task scheduled for 4. &&',
   'vote outcome: {"text":"One Potato"}',
   'vote outcome: {"text":"1 foot"}',
 ];