refactor: review prompted: tie handling, types, quorum, ballot types

Add types
flesh out different ballot types a tiny bit
moved quorum counting inside binaryBallotCounter
More assertions
dropped externally visible countVotes(), getQuestionPositions()
dependency updates
Made tie result be a default or undefined

packages/governance/package.json

@@ -31,7 +31,9 @@
   "homepage": "https://github.com/Agoric/agoric-sdk#readme",
   "dependencies": {
     "@agoric/assert": "^0.3.0",
+    "@agoric/captp": "^1.7.15",
     "@agoric/ertp": "^0.11.2",
+    "@agoric/eventual-send": "^0.13.16",
     "@agoric/marshal": "^0.4.13",
     "@agoric/store": "^0.4.15",
     "@agoric/promise-kit": "^0.2.13",

packages/governance/src/ballotBuilder.js

@@ -7,14 +7,26 @@ import { assert, details as X } from '@agoric/assert';
 // WEIGHT: voter lists their choices, each with a numerical weight. High
 //   numbers are most preferred.
 
+/**
+ * @type {{
+ *   CHOOSE_N: 'choose_n',
+ *   ORDER: 'order',
+ *   WEIGHT: 'weight',
+ * }}
+ */
 const ChoiceMethod = {
   CHOOSE_N: 'choose_n',
   ORDER: 'order',
   WEIGHT: 'weight',
 };
 
-const buildBallot = (method, question, positions, maxChoices = 0) => {
-  const choose = (...chosenPositions) => {
+const buildEqualWeightBallot = (
+  method,
+  question,
+  positions,
+  maxChoices = 0n,
+) => {
+  const choose = chosenPositions => {
     assert(
       chosenPositions.length <= maxChoices,
       X`only ${maxChoices} position(s) allowed`,
@@ -26,18 +38,40 @@ const buildBallot = (method, question, positions, maxChoices = 0) => {
         X`Not a valid position: ${position}`,
       );
     }
+    /** @type {CompleteEqualWeightBallot} */
     return { question, chosen: chosenPositions };
   };
 
+  const getDetails = () =>
+    harden({
+      method,
+      question,
+      positions,
+      maxChoices,
+    });
+
   return {
-    getMethod: () => method,
-    getQuestion: () => question,
-    getPositions: () => positions,
-    getMaxChoices: () => maxChoices,
+    getDetails,
     choose,
   };
 };
 
+/** @type {BuildBallot} */
+const buildBallot = (method, question, positions, maxChoices = 0n) => {
+  assert.typeof(question, 'string');
+
+  switch (method) {
+    case ChoiceMethod.CHOOSE_N:
+      return buildEqualWeightBallot(method, question, positions, maxChoices);
+    case ChoiceMethod.ORDER:
+      throw Error(`choice method ${ChoiceMethod.ORDER} is unimplemented`);
+    case ChoiceMethod.WEIGHT:
+      throw Error(`choice method ${ChoiceMethod.WEIGHT} is unimplemented`);
+    default:
+      throw Error(`choice method unrecognized`);
+  }
+};
+
 harden(buildBallot);
 
 export { ChoiceMethod, buildBallot };

packages/governance/src/binaryBallotCounter.js

@@ -5,88 +5,120 @@ import { makeStore } from '@agoric/store';
 import { makePromiseKit } from '@agoric/promise-kit';
 import { Far } from '@agoric/marshal';
 
-import { E } from '@agoric/eventual-send';
 import { ChoiceMethod, buildBallot } from './ballotBuilder';
 
-const TIE_VOTE = "It's a tie!";
-
-const makeWeightedBallot = (ballot, weight) => ({ ballot, weight });
+const makeWeightedBallot = (ballot, shares) => ({ ballot, shares });
 
 const makeBinaryBallot = (question, positionAName, positionBName) => {
   const positions = [];
+  assert.typeof(question, 'string');
   assert.typeof(positionAName, 'string');
   assert.typeof(positionBName, 'string');
   positions.push(positionAName, positionBName);
 
-  return buildBallot(ChoiceMethod.CHOOSE_N, question, positions, 1);
+  return buildBallot(ChoiceMethod.CHOOSE_N, question, positions, 1n);
+};
+
+const makeQuorumCounter = quorumThreshold => {
+  const check = stats => {
+    const votes = stats.results.reduce(
+      (runningTotal, { total }) => runningTotal + total,
+      0n,
+    );
+    return votes >= quorumThreshold;
+  };
+  /** @type {QuorumCounter} */
+  return Far('checker', { check });
 };
 
 // Exported for testing purposes
-const makeBinaryBallotCounter = (question, aName, bName) => {
+const makeBinaryBallotCounter = (
+  question,
+  positions,
+  threshold,
+  tieOutcome = undefined,
+) => {
+  assert(
+    positions.length === 2,
+    X`Binary ballots must have exactly two positions. had ${positions.length}: ${positions}`,
+  );
+  const [aName, bName] = positions;
+  if (tieOutcome) {
+    assert(
+      positions.includes(tieOutcome),
+      X`The default outcome on a tie must be one of the positions, not ${tieOutcome}`,
+    );
+  }
   const template = makeBinaryBallot(question, aName, bName);
+  const ballotDetails = template.getDetails();
 
   assert(
-    template.getMethod() === ChoiceMethod.CHOOSE_N,
+    ballotDetails.method === ChoiceMethod.CHOOSE_N,
     X`Binary ballot counter only works with CHOOSE_N`,
   );
   let isOpen = true;
   const outcomePromise = makePromiseKit();
   const tallyPromise = makePromiseKit();
   const allBallots = makeStore('seat');
 
-  const getQuestionPositions = () => ({
-    question,
-    positionA: aName,
-    positionB: bName,
-  });
-
-  const recordBallot = (seat, filledBallot, weight = 1n) => {
+  const recordBallot = (seat, filledBallot, shares = 1n) => {
+    assert(
+      filledBallot.question === question,
+      X`Ballot not for this question ${filledBallot.question} should have been ${question}`,
+    );
+    assert(
+      positions.includes(filledBallot.chosen[0]),
+      X`The ballot's choice is not a legal position: ${filledBallot.chosen[0]}.`,
+    );
     allBallots.has(seat)
-      ? allBallots.set(seat, makeWeightedBallot(filledBallot, weight))
-      : allBallots.init(seat, makeWeightedBallot(filledBallot, weight));
+      ? allBallots.set(seat, makeWeightedBallot(filledBallot, shares))
+      : allBallots.init(seat, makeWeightedBallot(filledBallot, shares));
   };
 
-  const countVotes = async quorumChecker => {
+  const countVotes = () => {
     assert(!isOpen, X`can't count votes while the election is open`);
 
     // ballot template has position choices; Each ballot in allBallots should
     // match. count the valid ballots and report results.
-    const [positionA, positionB] = template.getPositions();
     let spoiled = 0n;
     const tally = {
-      [positionA]: 0n,
-      [positionB]: 0n,
+      [positions[0]]: 0n,
+      [positions[1]]: 0n,
     };
 
-    allBallots.entries().forEach(([_, { ballot, weight }]) => {
+    allBallots.values().forEach(({ ballot, shares }) => {
+      assert(
+        ballot.chosen.length === 1,
+        X`A binary ballot must contain exactly one choice.`,
+      );
       const choice = ballot.chosen[0];
-      if (!template.getPositions().includes(choice)) {
-        spoiled += weight;
+      if (!ballotDetails.positions.includes(choice)) {
+        spoiled += shares;
       } else {
-        tally[choice] += weight;
+        tally[choice] += shares;
       }
     });
 
     const stats = {
       spoiled,
       votes: allBallots.entries().length,
       results: [
-        { position: positionA, total: tally[positionA] },
-        { position: positionB, total: tally[positionB] },
+        { position: positions[0], total: tally[positions[0]] },
+        { position: positions[1], total: tally[positions[1]] },
       ],
     };
 
-    const quorumCheck = await E(quorumChecker).check(stats);
-    if (!quorumCheck) {
+    if (!makeQuorumCounter(threshold).check(stats)) {
       outcomePromise.reject('No quorum');
+      return;
     }
 
-    if (tally[positionA] > tally[positionB]) {
-      outcomePromise.resolve(positionA);
-    } else if (tally[positionB] > tally[positionA]) {
-      outcomePromise.resolve(positionB);
+    if (tally[positions[0]] > tally[positions[1]]) {
+      outcomePromise.resolve(positions[0]);
+    } else if (tally[positions[1]] > tally[positions[0]]) {
+      outcomePromise.resolve(positions[1]);
     } else {
-      outcomePromise.resolve(TIE_VOTE);
+      outcomePromise.resolve(tieOutcome);
     }
 
     tallyPromise.resolve(stats);
@@ -95,22 +127,24 @@ const makeBinaryBallotCounter = (question, aName, bName) => {
   const sharedFacet = {
     getBallotTemplate: () => template,
     isOpen: () => isOpen,
-    getQuestionPositions,
   };
 
+  /** @type {VoterFacet} */
   const voterFacet = Far('voterFacet', {
     submitVote: recordBallot,
   });
 
+  /** @type {BallotCounterCreatorFacet} */
   const creatorFacet = Far('adminFacet', {
     ...sharedFacet,
     closeVoting: () => {
       isOpen = false;
+      countVotes();
     },
-    countVotes,
     getVoterFacet: () => voterFacet,
   });
 
+  /** @type {BallotCounterPublicFacet} */
   const publicFacet = Far('publicFacet', {
     ...sharedFacet,
     getOutcome: () => outcomePromise.promise,
@@ -120,11 +154,16 @@ const makeBinaryBallotCounter = (question, aName, bName) => {
 };
 
 const start = zcf => {
-  const { question, positions } = zcf.getTerms();
-  return makeBinaryBallotCounter(question, positions[0], positions[1]);
+  // There are a variety of ways of counting quorums. The parameters must be
+  // visible in the terms. We're doing a simple threshold here. If we wanted to
+  // discount abstentions, we could refactor to provide the quorumCounter as a
+  // component.
+  // TODO(hibbert) checking the quorum should be pluggable and legible.
+  const { question, positions, quorumThreshold } = zcf.getTerms();
+  return makeBinaryBallotCounter(question, positions, quorumThreshold);
 };
 
 harden(start);
 harden(makeBinaryBallotCounter);
 
-export { makeBinaryBallotCounter, start, TIE_VOTE };
+export { makeBinaryBallotCounter, start };

packages/governance/src/types.js

@@ -8,6 +8,10 @@
  * @typedef { Amount | Brand | Installation | Instance | bigint | Ratio | string | unknown } ParamValue
  */
 
+/**
+ * @typedef { 'choose_n' | 'order' | 'weight' } ChoiceMethod
+ */
+
 /**
  * @typedef {Object} ParamDescription
  * @property {string} name
@@ -32,3 +36,96 @@
  * @param {ParamDescriptions} paramDesc
  * @returns {ParamManagerFull}
  */
+
+/**
+ * @typedef {Object} BallotDetails
+ * @property {ChoiceMethod} method
+ * @property {string} question
+ * @property {string[]} positions
+ * @property {bigint} maxChoices
+ */
+
+/**
+ * @typedef {Object} Ballot
+ * @property {(positions: string[]) => CompletedBallot} choose
+ * @property {() => BallotDetails} getDetails
+ */
+
+/**
+ * @typedef {Object} PositionCount
+ * @property {string} position
+ * @property {number} tally
+ */
+
+/**
+ * @typedef {Object} VoteStatistics
+ * @property {number} spoiled
+ * @property {number} votes
+ * @property {PositionCount[]} results
+ */
+
+/**
+ * @typedef {Object} QurorumCounter
+ * @property {(VoteStatistics) => boolean} check
+ */
+
+/**
+ * @callback BuildBallot
+ * @param {ChoiceMethod} method
+ * @param {string} question
+ * @param {string[]} positions
+ * @param {bigint} maxChoices
+ * @returns {Ballot}
+ */
+
+/**
+ * @typedef {Object} BallotCounterCreatorFacet
+ * @property {() => boolean} isOpen
+ * @property {() => Ballot} getBallotTemplate
+ * @property {() => void} closeVoting
+ * @property {() => VoterFacet} getVoterFacet
+ */
+
+/**
+ * @typedef {Object} BallotCounterPublicFacet
+ * @property {() => boolean} isOpen
+ * @property {() => Ballot} getBallotTemplate
+ * @property {() => Promise<string>} getOutcome
+ * @property {() => Promise<VoteStatistics>} getStats
+ */
+
+/**
+ * @typedef {Object} CompleteEqualWeightBallot
+ * @property {string} question
+ * @property {string[]} chosen - a list of equal-weight preferred positions
+ */
+
+/**
+ * @typedef {Object} CompleteWeightedBallot
+ * @property {string} question
+ * @property {Record<string,bigint>[]} weighted - list of positions with weights.
+ *   BallotCounter may limit weights to a range or require uniqueness.
+ */
+
+/**
+ * @typedef {Object} CompleteOrderedBallot
+ * @property {string} question
+ * @property {string[]} ordered - ordered list of position from most prefered to
+ *   least prefered
+ */
+
+/**
+ * @typedef { CompleteEqualWeightBallot | CompleteOrderedBallot | CompleteWeightedBallot } CompletedBallot
+ */
+
+/**
+ * @callback SubmitVote
+ * @param {Handle<'Voter'>} seat
+ * @param {CompletedBallot} filledBallot
+ * @param {bigint=} weight
+ */
+
+/**
+ * @typedef {Object} VoterFacet
+ * @property {SubmitVote} submitVote
+ */