From c297a97fe92ae4b948ec7166757b02c447af653a Mon Sep 17 00:00:00 2001 From: Chris Hibbert Date: Sat, 19 Jun 2021 15:45:53 -0700 Subject: [PATCH] chore: cleanups simplify ballotCounter creating ballots resolve tally promise earlier typos --- packages/governance/package.json | 1 + packages/governance/src/ballotBuilder.js | 4 ++-- .../governance/src/binaryBallotCounter.js | 20 ++++++------------- packages/governance/src/paramManager.js | 2 ++ packages/governance/src/types.js | 8 ++++---- .../governance/test/test-param-manager.js | 7 +++++++ .../test/unitTests/test-ballotCount.js | 2 +- 7 files changed, 23 insertions(+), 21 deletions(-) diff --git a/packages/governance/package.json b/packages/governance/package.json index 6a67bee0107b..df84f11002ea 100644 --- a/packages/governance/package.json +++ b/packages/governance/package.json @@ -35,6 +35,7 @@ "@agoric/ertp": "^0.11.2", "@agoric/eventual-send": "^0.13.16", "@agoric/marshal": "^0.4.13", + "@agoric/nat": "^4.1.0", "@agoric/store": "^0.4.15", "@agoric/promise-kit": "^0.2.13", "@agoric/zoe": "^0.16.0" diff --git a/packages/governance/src/ballotBuilder.js b/packages/governance/src/ballotBuilder.js index 3c6dc6246655..71e16fc1af79 100644 --- a/packages/governance/src/ballotBuilder.js +++ b/packages/governance/src/ballotBuilder.js @@ -24,7 +24,7 @@ const buildEqualWeightBallot = ( method, question, positions, - maxChoices = 0n, + maxChoices = 0, ) => { const choose = chosenPositions => { assert( @@ -57,7 +57,7 @@ const buildEqualWeightBallot = ( }; /** @type {BuildBallot} */ -const buildBallot = (method, question, positions, maxChoices = 0n) => { +const buildBallot = (method, question, positions, maxChoices = 0) => { assert.typeof(question, 'string'); switch (method) { diff --git a/packages/governance/src/binaryBallotCounter.js b/packages/governance/src/binaryBallotCounter.js index 7a69e6244529..22fa36a1069f 100644 --- a/packages/governance/src/binaryBallotCounter.js +++ b/packages/governance/src/binaryBallotCounter.js @@ -9,16 +9,6 @@ import { ChoiceMethod, buildBallot } from './ballotBuilder'; 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, 1n); -}; - const makeQuorumCounter = quorumThreshold => { const check = stats => { const votes = stats.results.reduce( @@ -42,14 +32,17 @@ const makeBinaryBallotCounter = ( positions.length === 2, X`Binary ballots must have exactly two positions. had ${positions.length}: ${positions}`, ); - const [aName, bName] = positions; + assert.typeof(positions[0], 'string'); + assert.typeof(positions[1], 'string'); + assert.typeof(question, 'string'); 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 template = buildBallot(ChoiceMethod.CHOOSE_N, question, positions, 1); const ballotDetails = template.getDetails(); assert( @@ -107,6 +100,7 @@ const makeBinaryBallotCounter = ( { position: positions[1], total: tally[positions[1]] }, ], }; + tallyPromise.resolve(stats); if (!makeQuorumCounter(threshold).check(stats)) { outcomePromise.reject('No quorum'); @@ -120,8 +114,6 @@ const makeBinaryBallotCounter = ( } else { outcomePromise.resolve(tieOutcome); } - - tallyPromise.resolve(stats); }; const sharedFacet = { diff --git a/packages/governance/src/paramManager.js b/packages/governance/src/paramManager.js index c1a9a1a0d1e0..c9b684eb4ffb 100644 --- a/packages/governance/src/paramManager.js +++ b/packages/governance/src/paramManager.js @@ -5,6 +5,7 @@ import { assertIsRatio } from '@agoric/zoe/src/contractSupport'; import { AmountMath, looksLikeBrand } from '@agoric/ertp'; import { Far } from '@agoric/marshal'; import { assertKeywordName } from '@agoric/zoe/src/cleanProposal'; +import { Nat } from '@agoric/nat'; /** * @type {{ @@ -58,6 +59,7 @@ const assertType = (type, value, name) => { break; case ParamType.NAT: assert.typeof(value, 'bigint'); + Nat(value); break; case ParamType.RATIO: assertIsRatio(value); diff --git a/packages/governance/src/types.js b/packages/governance/src/types.js index 11b052093998..3760b1ac2d36 100644 --- a/packages/governance/src/types.js +++ b/packages/governance/src/types.js @@ -65,7 +65,7 @@ */ /** - * @typedef {Object} QurorumCounter + * @typedef {Object} QuorumCounter * @property {(VoteStatistics) => boolean} check */ @@ -74,7 +74,7 @@ * @param {ChoiceMethod} method * @param {string} question * @param {string[]} positions - * @param {bigint} maxChoices + * @param {number} maxChoices * @returns {Ballot} */ @@ -110,8 +110,8 @@ /** * @typedef {Object} CompleteOrderedBallot * @property {string} question - * @property {string[]} ordered - ordered list of position from most prefered to - * least prefered + * @property {string[]} ordered - ordered list of position from most preferred to + * least preferred */ /** diff --git a/packages/governance/test/test-param-manager.js b/packages/governance/test/test-param-manager.js index cbdb95f9fe93..08e138a5444d 100644 --- a/packages/governance/test/test-param-manager.js +++ b/packages/governance/test/test-param-manager.js @@ -100,6 +100,13 @@ test('params one BigInt', async t => { }, 'value should be a bigint', ); + t.throws( + () => updateBigint(-1000n), + { + message: '-1000 is negative', + }, + 'NAT params must be positive', + ); }); test('params one ratio', async t => { diff --git a/packages/governance/test/unitTests/test-ballotCount.js b/packages/governance/test/unitTests/test-ballotCount.js index eef9eb98f523..ae4b0717e322 100644 --- a/packages/governance/test/unitTests/test-ballotCount.js +++ b/packages/governance/test/unitTests/test-ballotCount.js @@ -236,7 +236,7 @@ test('binary ballot too many', async t => { () => E(voterFacet).submitVote(aliceSeat, aliceTemplate.choose(alicePositions)), { - message: 'only "[1n]" position(s) allowed', + message: 'only 1 position(s) allowed', }, ); });