Skip to content

Commit

Permalink
chore: cleanups to facilitate reviewing
Browse files Browse the repository at this point in the history
Rename HALF to MAJORITY, and NONE to NO_QUORUM
break up long lines in types.js
Add bullet points to break up paragraph in choiceMethod
use typescript sub-type declarations
fix QuestionTerms
patch around a ts confusion.
  • Loading branch information
Chris-Hibbert committed Aug 20, 2021
1 parent ceaecbb commit 534368f
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 59 deletions.
1 change: 1 addition & 0 deletions packages/governance/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"devDependencies": {
"@agoric/babel-standalone": "^7.14.3",
"@agoric/bundle-source": "^1.4.5",
"@agoric/eslint-plugin": "^0.3.10",
"@agoric/install-ses": "^0.5.21",
"@agoric/swingset-vat": "^0.19.0",
"ava": "^3.12.1",
Expand Down
13 changes: 8 additions & 5 deletions packages/governance/src/ballotBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ const ElectionType = {
};

/** @type {{
* HALF: 'half',
* NONE: 'none',
* MAJORITY: 'majority',
* NO_QUORUM: 'no_quorum',
* ALL: 'all',
* }}
*/
const QuorumRule = {
HALF: 'half',
NONE: 'none',
MAJORITY: 'majority',
NO_QUORUM: 'no_quorum',
// The election isn't valid unless all voters cast a ballot
ALL: 'all',
};

Expand Down Expand Up @@ -95,7 +96,9 @@ const makeBallotSpec = (
);

assert(
[QuorumRule.HALF, QuorumRule.ALL, QuorumRule.NONE].includes(quorumRule),
[QuorumRule.MAJORITY, QuorumRule.ALL, QuorumRule.NO_QUORUM].includes(
quorumRule,
),
X`Illegal QuorumRule ${quorumRule}`,
);

Expand Down
2 changes: 2 additions & 0 deletions packages/governance/src/binaryBallotCounter.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ const makeBinaryBallotCounter = (ballotSpec, threshold, instance) => {

const ballot = buildBallot(ballotSpec, instance);
const details = ballot.getDetails();
// @ts-ignore typescript is confused here. Dunno why. The declarations clearly
// say Ballot.getDetails() returns a ballotDetails, which has a handle.
const { handle } = details;

let isOpen = true;
Expand Down
5 changes: 3 additions & 2 deletions packages/governance/src/committeeRegistrar.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,18 @@ const start = zcf => {
const addQuestion = async (voteCounter, ballotSpec) => {
const quorumThreshold = quorumRule => {
switch (quorumRule) {
case QuorumRule.HALF:
case QuorumRule.MAJORITY:
return ceilDivide(committeeSize, 2);
case QuorumRule.ALL:
return committeeSize;
case QuorumRule.NONE:
case QuorumRule.NO_QUORUM:
return 0;
default:
throw Error(`${quorumRule} is not a recognized quorum rule`);
}
};

/** @type {QuestionTerms} */
const ballotCounterTerms = {
ballotSpec,
registrar: zcf.getInstance(),
Expand Down
6 changes: 3 additions & 3 deletions packages/governance/src/governParam.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ const validateParamChangeBallot = details => {
X`maxChoices must be 1, not ${details.maxChoices}`,
);
assert(
details.quorumRule === QuorumRule.HALF,
X`QuorumRule must be HALF, not ${details.quorumRule}`,
details.quorumRule === QuorumRule.MAJORITY,
X`QuorumRule must be MAJORITY, not ${details.quorumRule}`,
);
assert(
details.tieOutcome.noChange,
Expand Down Expand Up @@ -91,7 +91,7 @@ const setupGovernance = async (
ElectionType.PARAM_CHANGE,
1,
{ timer, deadline },
QuorumRule.HALF,
QuorumRule.MAJORITY,
negative,
);

Expand Down
56 changes: 27 additions & 29 deletions packages/governance/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

/**
* @typedef { 'choose_n' | 'order' | 'weight' } ChoiceMethod
* CHOOSE_N: This is "approval voting". The voter specifies some number of
* * CHOOSE_N: This is "approval voting". The voter specifies some number of
* positions, and is endorsing them equally.
* ORDER: The voter assigns ordinal numbers to some of the positions. The
* * ORDER: The voter assigns ordinal numbers to some of the positions. The
* positions will be treated as an ordered list with no gaps.
* WEIGHT: The voter assigns a distinct integer weight to some number of
* * WEIGHT: The voter assigns a distinct integer weight to some number of
* positions. The weights are not required to be distinct or consecutive.
* High numbers are most preferred.
*/
Expand All @@ -20,7 +20,7 @@
*/

/**
* @typedef { 'half' | 'all' | 'none' } QuorumRule
* @typedef { 'majority' | 'all' | 'no_quorum' } QuorumRule
*/

/**
Expand All @@ -29,24 +29,25 @@
*/

/**
* @typedef { 'amount' | 'brand' | 'installation' | 'instance' | 'nat' | 'ratio' | 'string' | 'unknown' } ParamType
* @typedef { 'amount' | 'brand' | 'installation' | 'instance' | 'nat' |
* 'ratio' | 'string' | 'unknown' } ParamType
*/

/**
* @typedef { Amount | Brand | Installation | Instance | bigint | Ratio | string | unknown } ParamValue
* @typedef { Amount | Brand | Installation | Instance | bigint | Ratio |
* string | unknown } ParamValue
*/

/**
* @typedef { SimpleQuestion | ParamChangeQuestion } Question
*/

/**
* @typedef {Object} QuestionTerms - BallotSpec plus the Registrar Instance
* @property {Question} question
* @property {string[]} positions
* @property {ChoiceMethod} method
* @property {number} maxChoices
* @property {ClosingRule} closingRule
* @typedef {Object} QuestionTerms - BallotSpec plus the Registrar Instance and
* a numerical threshold for the quorum. (The ballotCounter doesn't know the
* size of the electorate, so the Registrar has to say what limit to enforce.)
* @property {BallotSpec} ballotSpec
* @property {number} quorumThreshold
* @property {Instance} registrar
*/

Expand All @@ -56,7 +57,8 @@
*/

/**
* @typedef { textPosition | changeParamPosition | noChangeParamPosition } Position
* @typedef { textPosition | changeParamPosition |
* noChangeParamPosition } Position
*/

/**
Expand All @@ -73,16 +75,8 @@
*/

/**
* @typedef {Object} BallotDetails
* @typedef {BallotSpec} BallotDetails
* complete ballot details: ballotSpec plus counter and handle
* @property {ChoiceMethod} method
* @property {Question} question
* @property {Position[]} positions
* @property {ElectionType} electionType
* @property {number} maxChoices
* @property {ClosingRule} closingRule
* @property {QuorumRule} quorumRule
* @property {Position} tieOutcome
* @property {Instance} counterInstance - instance of the BallotCounter
* @property {Handle<'Ballot'>} handle
*/
Expand Down Expand Up @@ -111,16 +105,16 @@
* @typedef {Object} CompleteWeightedBallot
* @property {Question} question
* @property {Handle<'Ballot'>} handle
* @property {Record<string,bigint>[]} weighted - list of positions with weights.
* BallotCounter may limit weights to a range or require uniqueness.
* @property {Record<string,bigint>[]} weighted - list of positions with
* weights. BallotCounter may limit weights to a range or require uniqueness.
*/

/**
* @typedef {Object} CompleteOrderedBallot
* @property {Question} question
* @property {Handle<'Ballot'>} handle
* @property {string[]} ordered - ordered list of position from most preferred to
* least preferred
* @property {string[]} ordered - ordered list of position from most preferred
* to least preferred
*/

/**
Expand Down Expand Up @@ -187,7 +181,9 @@
*/

/**
* @typedef { CompleteEqualWeightBallot | CompleteOrderedBallot | CompleteWeightedBallot } CompletedBallot
* @typedef { CompleteEqualWeightBallot | CompleteOrderedBallot |
* CompleteWeightedBallot
* } CompletedBallot
*/

/**
Expand Down Expand Up @@ -330,7 +326,8 @@
*/

/**
* @typedef {{ [updater: string]: (arg: ParamValue) => void }} ParamManagerUpdaters
* @typedef {{ [updater: string]: (arg: ParamValue) => void
* }} ParamManagerUpdaters
* @typedef {ParamManagerBase & ParamManagerUpdaters} ParamManagerFull
*/

Expand Down Expand Up @@ -420,7 +417,8 @@
* 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 contract
* @property {() => ERef<Instance>} getInstance - instance of the governed
* contract
*/

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const committeeBinaryStart = async (
details,
3n,
tools,
QuorumRule.HALF,
QuorumRule.MAJORITY,
);

const invitations = await E(registrarFacet).getVoterInvitations();
Expand Down Expand Up @@ -164,7 +164,7 @@ const committeeBinaryTwoQuestions = async (
potato,
3n,
tools,
QuorumRule.HALF,
QuorumRule.MAJORITY,
);

const height = {
Expand All @@ -176,7 +176,7 @@ const committeeBinaryTwoQuestions = async (
height,
4n,
tools,
QuorumRule.HALF,
QuorumRule.MAJORITY,
);

const [alice, bob] = await Promise.all([aliceP, bobP, carolP, daveP, emmaP]);
Expand Down
24 changes: 12 additions & 12 deletions packages/governance/test/unitTests/test-ballotCount.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ test('binary ballot', async t => {
ElectionType.SURVEY,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
BAIT,
);
const { publicFacet, creatorFacet, closeFacet } = makeBinaryBallotCounter(
Expand Down Expand Up @@ -75,7 +75,7 @@ test('binary spoiled', async t => {
ElectionType.ELECTION,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
BAIT,
);
const { publicFacet, creatorFacet } = makeBinaryBallotCounter(
Expand Down Expand Up @@ -111,7 +111,7 @@ test('binary tied', async t => {
ElectionType.PARAM_CHANGE,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
negative,
);
const { publicFacet, creatorFacet, closeFacet } = makeBinaryBallotCounter(
Expand Down Expand Up @@ -140,7 +140,7 @@ test('binary bad vote', async t => {
ElectionType.PARAM_CHANGE,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
negative,
);
const { publicFacet, creatorFacet } = makeBinaryBallotCounter(
Expand Down Expand Up @@ -170,7 +170,7 @@ test('binary counter does not match ballot', async t => {
ElectionType.PARAM_CHANGE,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
negative,
);
const { publicFacet, creatorFacet } = makeBinaryBallotCounter(
Expand Down Expand Up @@ -210,7 +210,7 @@ test('binary no votes', async t => {
ElectionType.PARAM_CHANGE,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
negative,
);
const { publicFacet, closeFacet } = makeBinaryBallotCounter(
Expand All @@ -232,7 +232,7 @@ test('binary varying share weights', async t => {
ElectionType.SURVEY,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
negative,
);
const { publicFacet, creatorFacet, closeFacet } = makeBinaryBallotCounter(
Expand Down Expand Up @@ -265,7 +265,7 @@ test('binary contested', async t => {
ElectionType.ELECTION,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
negative,
);
const { publicFacet, creatorFacet, closeFacet } = makeBinaryBallotCounter(
Expand Down Expand Up @@ -297,7 +297,7 @@ test('binary revote', async t => {
ElectionType.PARAM_CHANGE,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
negative,
);
const { publicFacet, creatorFacet, closeFacet } = makeBinaryBallotCounter(
Expand Down Expand Up @@ -330,7 +330,7 @@ test('binary ballot too many', async t => {
ElectionType.SURVEY,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
BAIT,
);
const { publicFacet, creatorFacet } = makeBinaryBallotCounter(
Expand Down Expand Up @@ -360,7 +360,7 @@ test('binary no quorum', async t => {
ElectionType.ELECTION,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
BAIT,
);
const { publicFacet, creatorFacet, closeFacet } = makeBinaryBallotCounter(
Expand Down Expand Up @@ -392,7 +392,7 @@ test('binary too many positions', async t => {
ElectionType.SURVEY,
1,
FAKE_CLOSING_RULE,
QuorumRule.NONE,
QuorumRule.NO_QUORUM,
BAIT,
);
t.throws(
Expand Down
Loading

0 comments on commit 534368f

Please sign in to comment.