chore: improved types and type-checking, rename .puml file

rename ballot validation to looksLike
improve checking for ballotDetails

packages/governance/src/ballotBuilder.js

@@ -4,6 +4,7 @@ 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.js';
+import { Nat } from '@agoric/nat';
 
 import { assertType, ParamType } from './paramManager.js';
 
@@ -47,36 +48,71 @@ const QuorumRule = {
   ALL: 'all',
 };
 
-const verifyQuestionFormat = (electionType, question) => {
+/** @param {SimpleQuestion} question */
+const looksLikeSimpleQuestion = question => {
+  assert.typeof(
+    question.text,
+    'string',
+    X`Question ("${question}") must be a record with text: aString`,
+  );
+};
+
+/** @param {ParamChangeQuestion} question */
+const looksLikeParamChangeQuestion = question => {
+  assert.typeof(
+    question.paramSpec,
+    'object',
+    X`Question ("${question}") must be a record with paramSpec: anObject`,
+  );
+  assert(question.proposedValue);
+  assertType(ParamType.INSTANCE, question.contract, 'contract');
+};
+
+/** @type {LooksLikeQuestionForType} */
+const looksLikeQuestionForType = (electionType, question) => {
+  assert(
+    passStyleOf(question) === 'copyRecord',
+    X`A question can only be a pass-by-copy record: ${question}`,
+  );
+
   switch (electionType) {
     case ElectionType.SURVEY:
     case ElectionType.ELECTION:
-      assert.typeof(
-        question.text,
-        'string',
-        X`Question ("${question}") must be a record with text: aString`,
-      );
+      looksLikeSimpleQuestion(/** @type {SimpleQuestion} */ (question));
       break;
     case ElectionType.PARAM_CHANGE:
-      assert.typeof(
-        question.paramSpec,
-        'object',
-        X`Question ("${question}") must be a record with paramSpec: anObject`,
+      looksLikeParamChangeQuestion(
+        /** @type {ParamChangeQuestion} */ (question),
       );
-      assert(question.proposedValue);
-      assertType(ParamType.INSTANCE, question.contract, electionType);
       break;
     default:
       throw Error(`Election type unrecognized`);
   }
 };
 
+/** @type {PositionIncluded} */
 const positionIncluded = (positions, p) =>
   positions.some(e => sameStructure(e, p));
 
 // BallotSpec contains the subset of BallotDetails that can be specified before
+function looksLikeClosingRule(closingRule) {
+  const { timer, deadline } = closingRule;
+  Nat(deadline);
+  assert(passStyleOf(timer) === 'remotable', X`Timer must be a timer ${timer}`);
+}
+
+const assertEnumIncludes = (enumeration, value, name) => {
+  assert(
+    Object.getOwnPropertyNames(enumeration)
+      .map(k => enumeration[k])
+      .includes(value),
+    X`Illegal ${name}: ${value}`,
+  );
+};
+
 // the ballot is created.
-const makeBallotSpec = (
+/** @type {LooksLikeBallotSpec} */
+const looksLikeBallotSpec = ({
   method,
   question,
   positions,
@@ -85,33 +121,36 @@ const makeBallotSpec = (
   closingRule,
   quorumRule,
   tieOutcome,
-) => {
-  verifyQuestionFormat(electionType, question);
+}) => {
+  looksLikeQuestionForType(electionType, question);
 
   assert(
     positions.every(
       p => passStyleOf(p) === 'copyRecord',
       X`positions must be records`,
     ),
   );
-
   assert(
-    [QuorumRule.MAJORITY, QuorumRule.ALL, QuorumRule.NO_QUORUM].includes(
-      quorumRule,
-    ),
-    X`Illegal QuorumRule ${quorumRule}`,
+    positions.includes(tieOutcome),
+    X`tieOutcome must be a legal position: ${tieOutcome}`,
   );
+  assertEnumIncludes(QuorumRule, quorumRule, 'QuorumRule');
+  assertEnumIncludes(ElectionType, electionType, 'ElectionType');
+  assertEnumIncludes(ChoiceMethod, method, 'ChoiceMethod');
+  assert(maxChoices > 0, X`maxChoices must be positive: ${maxChoices}`);
+
+  looksLikeClosingRule(closingRule);
 
-  return {
+  return harden({
     method,
     question,
     positions,
-    maxChoices,
+    maxChoices: Number(maxChoices),
     electionType,
     closingRule,
     quorumRule,
     tieOutcome,
-  };
+  });
 };
 
 const buildEqualWeightBallot = (ballotSpec, counterInstance) => {
@@ -147,7 +186,7 @@ const buildEqualWeightBallot = (ballotSpec, counterInstance) => {
 
 /** @type {BuildBallot} */
 const buildBallot = (ballotSpec, counterInstance) => {
-  verifyQuestionFormat(ballotSpec.electionType, ballotSpec.question);
+  looksLikeQuestionForType(ballotSpec.electionType, ballotSpec.question);
 
   switch (ballotSpec.method) {
     case ChoiceMethod.CHOOSE_N:
@@ -164,16 +203,15 @@ harden(buildBallot);
 harden(ChoiceMethod);
 harden(QuorumRule);
 harden(ElectionType);
-harden(makeBallotSpec);
-harden(verifyQuestionFormat);
+harden(looksLikeQuestionForType);
 harden(positionIncluded);
 
 export {
   ChoiceMethod,
   ElectionType,
   QuorumRule,
   buildBallot,
-  makeBallotSpec,
+  looksLikeBallotSpec,
   positionIncluded,
-  verifyQuestionFormat,
+  looksLikeQuestionForType,
 };

packages/governance/src/binaryBallotCounter.js

@@ -10,7 +10,7 @@ import { makeStore } from '@agoric/store';
 import {
   ChoiceMethod,
   buildBallot,
-  verifyQuestionFormat,
+  looksLikeQuestionForType,
   positionIncluded,
 } from './ballotBuilder.js';
 import { scheduleClose } from './closingRule.js';
@@ -46,7 +46,7 @@ const validateBinaryBallotSpec = ballotSpec => {
     ),
   );
 
-  verifyQuestionFormat(ballotSpec.electionType, ballotSpec.question);
+  looksLikeQuestionForType(ballotSpec.electionType, ballotSpec.question);
 
   assert(
     positions.every(

packages/governance/src/governParam.js

@@ -10,7 +10,7 @@ import {
   ChoiceMethod,
   QuorumRule,
   ElectionType,
-  makeBallotSpec,
+  looksLikeBallotSpec,
 } from './ballotBuilder.js';
 import { assertType } from './paramManager.js';
 
@@ -79,21 +79,21 @@ const setupGovernance = async (
       paramSpec,
       proposedValue,
     );
-    const question = {
+    const question = harden({
       paramSpec,
       contract: contractInstance,
       proposedValue,
-    };
-    const ballotSpec = makeBallotSpec(
-      ChoiceMethod.CHOOSE_N,
+    });
+    const ballotSpec = looksLikeBallotSpec({
+      method: ChoiceMethod.CHOOSE_N,
       question,
-      [positive, negative],
-      ElectionType.PARAM_CHANGE,
-      1,
-      { timer, deadline },
-      QuorumRule.MAJORITY,
-      negative,
-    );
+      positions: [positive, negative],
+      electionType: ElectionType.PARAM_CHANGE,
+      maxChoices: 1,
+      closingRule: { timer, deadline },
+      quorumRule: QuorumRule.MAJORITY,
+      tieOutcome: negative,
+    });
 
     const {
       publicFacet: counterPublicFacet,

packages/governance/src/paramManager.js

@@ -33,7 +33,6 @@ const ParamType = {
   STRING: 'string',
   UNKNOWN: 'unknown',
 };
-harden(ParamType);
 
 const assertType = (type, value, name) => {
   switch (type) {
@@ -128,5 +127,7 @@ const buildParamManager = paramDescriptions => {
   });
 };
 
+harden(ParamType);
 harden(buildParamManager);
+harden(assertType);
 export { ParamType, buildParamManager, assertType };

packages/governance/src/types.js

@@ -198,6 +198,18 @@
  * } CompletedBallot
  */
 
+/**
+ * @callback LooksLikeBallotSpec
+ * @param {BallotSpec} allegedBallotSpec
+ * @returns {BallotSpec}
+ */
+
+/**
+ * @callback LooksLikeQuestionForType
+ * @param {ElectionType} electionType
+ * @param {Question} question
+ */
+
 /**
  * @callback SubmitVote
  * @param {Handle<'Voter'>} seat
@@ -452,7 +464,7 @@
  * @callback SetupGovernance
  * @param {ERef<ParamManagerAccessor>} accessor
  * @param {ERef<PoserFacet>} poserFacet
- * @param {ERef<Instance>} contractInstance
+ * @param {Instance} contractInstance
  * @param {Timer} timer
  * @returns {ParamGovernor}
  */
@@ -464,10 +476,17 @@
  * @returns {boolean}
  */
 
+/**
+ * @callback PositionIncluded
+ * @param {Position[]} positions
+ * @param {Position} position
+ * @returns {boolean}
+ */
+
 /**
  * @callback AssertContractGovernance
  *
- * @param {ZoeService} zoe
+ * @param {ERef<ZoeService>} zoe
  * @param {Instance} allegedGoverned
  * @param {Instance} allegedGovernor
  * @returns {GovernancePair}
@@ -477,7 +496,7 @@
  * @callback AssertContractRegistrar - assert that the contract uses the
  *   registrar
  *
- * @param {ZoeService} zoe
+ * @param {ERef<ZoeService>} zoe
  * @param {Instance} allegedGovernor
  * @param {Instance} allegedRegistrar
  */
@@ -489,7 +508,7 @@
  * that the registrar hosts, and that the ballotCounter and other details are
  * consistent.
  *
- * @param {ZoeService} zoe
+ * @param {ERef<ZoeService>} zoe
  * @param {Instance} registrar
  * @param {ParamChangeBallotDetails} details
  * @returns {Promise<*>}
@@ -502,7 +521,7 @@
  * parameter change question that the registrar hosts, and that the
  * ballotCounter and other details are consistent.
  *
- * @param {ZoeService} zoe
+ * @param {ERef<ZoeService>} zoe
  * @param {Instance} registrar
  * @param {Instance} ballotCounter
  * @returns {Promise<*>}

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

@@ -7,9 +7,9 @@ import buildManualTimer from '@agoric/zoe/tools/manualTimer.js';
 import { q } from '@agoric/assert';
 import {
   ChoiceMethod,
-  makeBallotSpec,
   QuorumRule,
   ElectionType,
+  looksLikeBallotSpec,
 } from '../../../src/ballotBuilder.js';
 
 const makeVoterVat = async (log, vats, zoe) => {
@@ -26,15 +26,17 @@ const addQuestion = async (qDetails, closingTime, tools, quorumRule) => {
     deadline: 3n,
   };
 
-  const ballotSpec = makeBallotSpec(
-    ChoiceMethod.CHOOSE_N,
-    question,
-    positions,
-    electionType,
-    1,
-    closingRule,
-    quorumRule,
-    positions[1],
+  const ballotSpec = looksLikeBallotSpec(
+    harden({
+      method: ChoiceMethod.CHOOSE_N,
+      question,
+      positions,
+      electionType,
+      maxChoices: 1,
+      closingRule,
+      quorumRule,
+      tieOutcome: positions[1],
+    }),
   );
 
   const { instance: ballotInstance, publicFacet } = await E(