From 7645df0e3c4b8ae901ea78e82b34a224ffaf1c2a Mon Sep 17 00:00:00 2001 From: Chris Hibbert Date: Thu, 15 Sep 2022 10:33:36 -0700 Subject: [PATCH] feat: publish vote results from the voteCounter (#6204) * feat: publish vote results from the voteCounter @turadg, Is there a straightforward change to the test that would allow us to verify the published results? * test: test the published outcomes * chore: discriminate fail outcome * chore: drop extra declaration of OutcomeRecord Co-authored-by: Turadg Aleahmad Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- packages/governance/src/binaryVoteCounter.js | 32 +++++- packages/governance/src/committee.js | 19 +++- packages/governance/src/electorateTools.js | 7 +- packages/governance/src/internalTypes.js | 4 +- packages/governance/src/types.js | 8 ++ .../test/unitTests/test-ballotCount.js | 104 +++++++++++++++++- 6 files changed, 161 insertions(+), 13 deletions(-) diff --git a/packages/governance/src/binaryVoteCounter.js b/packages/governance/src/binaryVoteCounter.js index 797addcda4a..f809874f9ef 100644 --- a/packages/governance/src/binaryVoteCounter.js +++ b/packages/governance/src/binaryVoteCounter.js @@ -3,6 +3,7 @@ import { Far } from '@endo/marshal'; import { makePromiseKit } from '@endo/promise-kit'; import { makeHeapFarInstance, keyEQ, makeStore } from '@agoric/store'; +import { E } from '@endo/eventual-send'; import { buildUnrankedQuestion, @@ -53,7 +54,12 @@ const validateBinaryQuestionSpec = questionSpec => { // independently. The standard Zoe start function is at the bottom of this file. /** @type {BuildVoteCounter} */ -const makeBinaryVoteCounter = (questionSpec, threshold, instance) => { +const makeBinaryVoteCounter = ( + questionSpec, + threshold, + instance, + publisher, +) => { validateBinaryQuestionSpec(questionSpec); const question = buildUnrankedQuestion(questionSpec, instance); @@ -113,6 +119,13 @@ const makeBinaryVoteCounter = (questionSpec, threshold, instance) => { if (!makeQuorumCounter(threshold).check(stats)) { outcomePromise.reject('No quorum'); + /** @type {OutcomeRecord} */ + const voteOutcome = { + question: details.questionHandle, + outcome: 'fail', + reason: 'No quorum', + }; + E(publisher).publish(voteOutcome); return; } @@ -123,6 +136,17 @@ const makeBinaryVoteCounter = (questionSpec, threshold, instance) => { } else { outcomePromise.resolve(questionSpec.tieOutcome); } + + // XXX if we should distinguish ties, publish should be called in if above + E.when(outcomePromise.promise, position => { + /** @type {OutcomeRecord} */ + const voteOutcome = { + question: details.questionHandle, + position, + outcome: 'win', + }; + return E(publisher).publish(voteOutcome); + }); }; const closeFacet = makeHeapFarInstance( @@ -196,9 +220,10 @@ const makeBinaryVoteCounter = (questionSpec, threshold, instance) => { // instance in the publicFacet before returning public and creator facets. /** - * @type {ContractStartFn} + * @param {ZCF<{questionSpec: QuestionSpec, quorumThreshold: bigint}>} zcf + * @param {{outcomePublisher: Publisher}} outcomePublisher */ -const start = zcf => { +const start = (zcf, { outcomePublisher }) => { // 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 @@ -210,6 +235,7 @@ const start = zcf => { questionSpec, quorumThreshold, zcf.getInstance(), + outcomePublisher, ); scheduleClose(questionSpec.closingRule, () => closeFacet.closeVoting()); diff --git a/packages/governance/src/committee.js b/packages/governance/src/committee.js index 19a89a22a47..9c9006cd83e 100644 --- a/packages/governance/src/committee.js +++ b/packages/governance/src/committee.js @@ -43,12 +43,12 @@ const start = (zcf, privateArgs) => { const allQuestions = makeStore('Question'); assert(privateArgs?.storageNode, 'Missing storageNode'); assert(privateArgs?.marshaller, 'Missing marshaller'); + const questionNode = E(privateArgs.storageNode).makeChildNode( + 'latestQuestion', + ); /** @type {StoredPublishKit} */ const { subscriber: questionsSubscriber, publisher: questionsPublisher } = - makeStoredPublishKit( - E(privateArgs.storageNode).makeChildNode('latestQuestion'), - privateArgs.marshaller, - ); + makeStoredPublishKit(questionNode, privateArgs.marshaller); const makeCommitteeVoterInvitation = index => { /** @type {OfferHandler} */ @@ -158,6 +158,16 @@ const start = (zcf, privateArgs) => { } }; + const outcomeNode = E(privateArgs.storageNode).makeChildNode( + 'latestOutcome', + ); + + /** @type {StoredPublishKit} */ + const { publisher: outcomePublisher } = makeStoredPublishKit( + outcomeNode, + privateArgs.marshaller, + ); + return startCounter( zcf, questionSpec, @@ -165,6 +175,7 @@ const start = (zcf, privateArgs) => { voteCounter, allQuestions, questionsPublisher, + outcomePublisher, ); }, getVoterInvitations() { diff --git a/packages/governance/src/electorateTools.js b/packages/governance/src/electorateTools.js index 6b7e4cfd49e..6ac020a3711 100644 --- a/packages/governance/src/electorateTools.js +++ b/packages/governance/src/electorateTools.js @@ -14,7 +14,8 @@ const startCounter = async ( quorumThreshold, voteCounter, questionStore, - publisher, + questionsPublisher, + outcomePublisher, ) => { const voteCounterTerms = { questionSpec, @@ -26,10 +27,10 @@ const startCounter = async ( /** @type {{ creatorFacet: VoteCounterCreatorFacet, publicFacet: VoteCounterPublicFacet, instance: Instance }} */ const { creatorFacet, publicFacet, instance } = await E( zcf.getZoeService(), - ).startInstance(voteCounter, {}, voteCounterTerms); + ).startInstance(voteCounter, {}, voteCounterTerms, { outcomePublisher }); const details = await E(publicFacet).getDetails(); const { deadline } = questionSpec.closingRule; - publisher.publish(details); + questionsPublisher.publish(details); const questionHandle = details.questionHandle; const voteCounterFacets = { voteCap: creatorFacet, publicFacet, deadline }; diff --git a/packages/governance/src/internalTypes.js b/packages/governance/src/internalTypes.js index 875e0ffbfb1..08f5ae3e4ae 100644 --- a/packages/governance/src/internalTypes.js +++ b/packages/governance/src/internalTypes.js @@ -1,3 +1,4 @@ +// @ts-check /** * @typedef {object} QuestionRecord * @property {ERef} voteCap @@ -12,6 +13,7 @@ * @param {unknown} quorumThreshold * @param {ERef} voteCounter * @param {Store, QuestionRecord>} questionStore - * @param {Publisher} publisher + * @param {Publisher} questionPublisher + * @param {Publisher} outcomePublisher * @returns {AddQuestionReturn} */ diff --git a/packages/governance/src/types.js b/packages/governance/src/types.js index 3a9bc2b4225..cef771f4f41 100644 --- a/packages/governance/src/types.js +++ b/packages/governance/src/types.js @@ -90,6 +90,13 @@ * OfferFilterPosition | NoChangeOfferFilterPosition | InvokeApiPosition } Position */ +/** + * @typedef {{ question: Handle<'Question'> } & ( + * { outcome: 'win', position: Position } | + * { outcome: 'fail', reason: 'No quorum' } + * )} OutcomeRecord + */ + /** * Specification when requesting creation of a Question * @@ -213,6 +220,7 @@ * @param {bigint} threshold - questionSpec includes quorumRule; the electorate * converts that to a number that the counter can enforce. * @param {Instance} instance + * @param {ERef>} publisher * @returns {VoteCounterFacets} */ diff --git a/packages/governance/test/unitTests/test-ballotCount.js b/packages/governance/test/unitTests/test-ballotCount.js index 1c67475a2ba..2278104198d 100644 --- a/packages/governance/test/unitTests/test-ballotCount.js +++ b/packages/governance/test/unitTests/test-ballotCount.js @@ -6,6 +6,11 @@ import { E } from '@endo/eventual-send'; import buildManualTimer from '@agoric/zoe/tools/manualTimer.js'; import { makeHandle } from '@agoric/zoe/src/makeHandle.js'; import { Far } from '@endo/marshal'; +import { makeStoredPublishKit } from '@agoric/notifier'; +import { + eventLoopIteration, + makeFakeMarshaller, +} from '@agoric/notifier/tools/testSupports.js'; import { makeBinaryVoteCounter, @@ -15,6 +20,7 @@ import { coerceQuestionSpec, makeParamChangePositions, } from '../../src/index.js'; +import { makeMockChainStorageRoot } from '../../../vats/tools/storage-test-utils.js'; const SIMPLE_ISSUE = harden({ text: 'Fish or cut bait?' }); const FISH = harden({ text: 'Fish' }); @@ -40,6 +46,12 @@ const FAKE_CLOSING_RULE = { const FAKE_COUNTER_INSTANCE = makeHandle('Instance'); +function makePublisherFromFakes() { + const storageRoot = makeMockChainStorageRoot(); + const publishKit = makeStoredPublishKit(storageRoot, makeFakeMarshaller()); + return { publisher: publishKit.publisher, storageRoot }; +} + test('binary question', async t => { const questionSpec = coerceQuestionSpec({ method: ChoiceMethod.UNRANKED, @@ -51,10 +63,12 @@ test('binary question', async t => { quorumRule: QuorumRule.NO_QUORUM, tieOutcome: BAIT, }); + const { publisher, storageRoot } = makePublisherFromFakes(); const { publicFacet, creatorFacet, closeFacet } = makeBinaryVoteCounter( questionSpec, 0n, FAKE_COUNTER_INSTANCE, + publisher, ); const aliceTemplate = publicFacet.getQuestion(); const aliceSeat = makeHandle('Voter'); @@ -66,6 +80,12 @@ test('binary question', async t => { closeFacet.closeVoting(); const outcome = await E(publicFacet).getOutcome(); t.deepEqual(outcome, FISH); + + await eventLoopIteration(); + t.like(storageRoot.getBody('mockChainStorageRoot'), { + outcome: 'win', + position: FISH, + }); }); test('binary spoiled', async t => { @@ -79,10 +99,13 @@ test('binary spoiled', async t => { quorumRule: QuorumRule.NO_QUORUM, tieOutcome: BAIT, }); + + const { publisher } = makePublisherFromFakes(); const { publicFacet, creatorFacet } = makeBinaryVoteCounter( questionSpec, 0n, FAKE_COUNTER_INSTANCE, + publisher, ); const aliceTemplate = publicFacet.getQuestion(); const aliceSeat = makeHandle('Voter'); @@ -110,10 +133,13 @@ test('binary tied', async t => { quorumRule: QuorumRule.MAJORITY, tieOutcome: negative, }); + + const { publisher, storageRoot } = makePublisherFromFakes(); const { publicFacet, creatorFacet, closeFacet } = makeBinaryVoteCounter( questionSpec, 2n, FAKE_COUNTER_INSTANCE, + publisher, ); const aliceTemplate = publicFacet.getQuestion(); const aliceSeat = makeHandle('Voter'); @@ -125,6 +151,12 @@ test('binary tied', async t => { closeFacet.closeVoting(); const outcome = await E(publicFacet).getOutcome(); t.deepEqual(outcome, negative); + + await eventLoopIteration(); + t.like(storageRoot.getBody('mockChainStorageRoot'), { + outcome: 'win', + position: negative, + }); }); test('binary bad vote', async t => { @@ -138,16 +170,30 @@ test('binary bad vote', async t => { quorumRule: QuorumRule.MAJORITY, tieOutcome: negative, }); - const { creatorFacet } = makeBinaryVoteCounter( + const { publisher, storageRoot } = makePublisherFromFakes(); + const { creatorFacet, publicFacet, closeFacet } = makeBinaryVoteCounter( questionSpec, 0n, FAKE_COUNTER_INSTANCE, + publisher, ); const aliceSeat = makeHandle('Voter'); await t.throwsAsync(() => E(creatorFacet).submitVote(aliceSeat, [BAIT]), { message: `The specified choice is not a legal position: {"text":"Cut Bait"}.`, }); + + closeFacet.closeVoting(); + const outcome = await E(publicFacet) + .getOutcome() + .catch(e => t.is(e, 'No quorum')); + t.deepEqual(outcome, negative); + + await eventLoopIteration(); + t.like(storageRoot.getBody('mockChainStorageRoot'), { + outcome: 'win', + position: negative, + }); }); test('binary no votes', async t => { @@ -161,15 +207,23 @@ test('binary no votes', async t => { quorumRule: QuorumRule.MAJORITY, tieOutcome: negative, }); + const { publisher, storageRoot } = makePublisherFromFakes(); const { publicFacet, closeFacet } = makeBinaryVoteCounter( questionSpec, 0n, FAKE_COUNTER_INSTANCE, + publisher, ); closeFacet.closeVoting(); const outcome = await E(publicFacet).getOutcome(); t.deepEqual(outcome, negative); + + await eventLoopIteration(); + t.like(storageRoot.getBody('mockChainStorageRoot'), { + outcome: 'win', + position: negative, + }); }); test('binary varying share weights', async t => { @@ -183,10 +237,12 @@ test('binary varying share weights', async t => { quorumRule: QuorumRule.NO_QUORUM, tieOutcome: BAIT, }); + const { publisher, storageRoot } = makePublisherFromFakes(); const { publicFacet, creatorFacet, closeFacet } = makeBinaryVoteCounter( questionSpec, 1n, FAKE_COUNTER_INSTANCE, + publisher, ); const aceSeat = makeHandle('Voter'); const austinSeat = makeHandle('Voter'); @@ -201,6 +257,12 @@ test('binary varying share weights', async t => { closeFacet.closeVoting(); const outcome = await E(publicFacet).getOutcome(); t.deepEqual(outcome, FISH); + + await eventLoopIteration(); + t.like(storageRoot.getBody('mockChainStorageRoot'), { + outcome: 'win', + position: FISH, + }); }); test('binary contested', async t => { @@ -215,10 +277,12 @@ test('binary contested', async t => { quorumRule: QuorumRule.MAJORITY, tieOutcome: negative, }); + const { publisher, storageRoot } = makePublisherFromFakes(); const { publicFacet, creatorFacet, closeFacet } = makeBinaryVoteCounter( questionSpec, 3n, FAKE_COUNTER_INSTANCE, + publisher, ); const template = publicFacet.getQuestion(); const aliceSeat = makeHandle('Voter'); @@ -233,6 +297,12 @@ test('binary contested', async t => { const outcome = await E(publicFacet).getOutcome(); t.deepEqual(outcome, negative); + + await eventLoopIteration(); + t.like(storageRoot.getBody('mockChainStorageRoot'), { + outcome: 'win', + position: negative, + }); }); test('binary revote', async t => { @@ -246,10 +316,12 @@ test('binary revote', async t => { quorumRule: QuorumRule.MAJORITY, tieOutcome: negative, }); + const { publisher, storageRoot } = makePublisherFromFakes(); const { publicFacet, creatorFacet, closeFacet } = makeBinaryVoteCounter( questionSpec, 5n, FAKE_COUNTER_INSTANCE, + publisher, ); const template = publicFacet.getQuestion(); const aliceSeat = makeHandle('Voter'); @@ -265,6 +337,12 @@ test('binary revote', async t => { const outcome = await E(publicFacet).getOutcome(); t.deepEqual(outcome, positive); + + await eventLoopIteration(); + t.like(storageRoot.getBody('mockChainStorageRoot'), { + outcome: 'win', + position: positive, + }); }); test('binary question too many', async t => { @@ -278,10 +356,12 @@ test('binary question too many', async t => { quorumRule: QuorumRule.NO_QUORUM, tieOutcome: BAIT, }); - const { publicFacet, creatorFacet } = makeBinaryVoteCounter( + const { publisher, storageRoot } = makePublisherFromFakes(); + const { publicFacet, creatorFacet, closeFacet } = makeBinaryVoteCounter( questionSpec, 1n, FAKE_COUNTER_INSTANCE, + publisher, ); const aliceTemplate = publicFacet.getQuestion(); const aliceSeat = makeHandle('Voter'); @@ -293,6 +373,18 @@ test('binary question too many', async t => { message: 'only 1 position allowed', }, ); + + closeFacet.closeVoting(); + const outcome = await E(publicFacet) + .getOutcome() + .catch(e => t.is(e, 'No quorum')); + t.deepEqual(outcome, true); + + await eventLoopIteration(); + t.like(storageRoot.getBody('mockChainStorageRoot'), { + outcome: 'fail', + reason: 'No quorum', + }); }); test('binary no quorum', async t => { @@ -306,10 +398,12 @@ test('binary no quorum', async t => { quorumRule: QuorumRule.NO_QUORUM, tieOutcome: BAIT, }); + const { publisher, storageRoot } = makePublisherFromFakes(); const { publicFacet, creatorFacet, closeFacet } = makeBinaryVoteCounter( questionSpec, 2n, FAKE_COUNTER_INSTANCE, + publisher, ); const aliceTemplate = publicFacet.getQuestion(); const aliceSeat = makeHandle('Voter'); @@ -321,6 +415,12 @@ test('binary no quorum', async t => { .getOutcome() .then(o => t.fail(`expected to reject, not ${o}`)) .catch(e => t.deepEqual(e, 'No quorum')); + + await eventLoopIteration(); + t.like(storageRoot.getBody('mockChainStorageRoot'), { + outcome: 'fail', + reason: 'No quorum', + }); }); test('binary too many positions', async t => {