Skip to content

Commit

Permalink
chore: cleanup types, and comments, etc.
Browse files Browse the repository at this point in the history
add an index.js
consistently type looksLikeFoo()
an assertion for the governor's installation in assertContractGovernance()
  • Loading branch information
Chris-Hibbert committed Sep 16, 2021
1 parent 2421429 commit 8ebabda
Show file tree
Hide file tree
Showing 13 changed files with 124 additions and 48 deletions.
1 change: 1 addition & 0 deletions packages/governance/exported.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import './src/types.js';
28 changes: 15 additions & 13 deletions packages/governance/src/committeeRegistrar.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@ import { QuorumRule } from './question.js';

const { ceilDivide } = natSafeMath;

// Each CommitteeRegistrar represents a particular set of voters. The number of
// voters is visible in the terms.
//
// This contract creates an electorate that is not visible to observers. There
// may be uses for such a structure, but it is not appropriate for elections
// where the set of voters needs to be known, unless the contract is used in a
// way that makes the distribution of voter facets visible.

/** @type {ContractStartFn} */
/**
* Each CommitteeRegistrar represents a particular set of voters. The number of
* voters is visible in the terms.
*
* This contract creates an electorate that is not visible to observers. There
* may be uses for such a structure, but it is not appropriate for elections
* where the set of voters needs to be known, unless the contract is used in a
* way that makes the distribution of voter facets visible.
*
* @type {ContractStartFn}
*/
const start = zcf => {
/**
* @typedef {Object} QuestionRecord
Expand All @@ -31,7 +33,6 @@ const start = zcf => {
/** @type {Store<Handle<'Question'>, QuestionRecord>} */
const allQuestions = makeStore('Question');
const { subscription, publication } = makeSubscriptionKit();
const invitations = [];

const getOpenQuestions = async () => {
const isOpenPQuestions = allQuestions.keys().map(key => {
Expand Down Expand Up @@ -69,9 +70,10 @@ const start = zcf => {
};

const { committeeName, committeeSize } = zcf.getTerms();
for (let i = 0; i < committeeSize; i += 1) {
invitations[i] = makeCommitteeVoterInvitation(i);
}

const invitations = harden(
[...Array(committeeSize).keys()].map(makeCommitteeVoterInvitation),
);

/** @type {AddQuestion} */
const addQuestion = async (voteCounter, questionSpec) => {
Expand Down
3 changes: 0 additions & 3 deletions packages/governance/src/contractGovernor.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { E } from '@agoric/eventual-send';
import { Far } from '@agoric/marshal';

import { setupGovernance, validateParamChangeQuestion } from './governParam.js';
import { assertContractGovernance } from './validators.js';

const { details: X } = assert;

Expand All @@ -20,8 +19,6 @@ const validateQuestionDetails = async (zoe, registrar, details) => {
.electionManager;
const governorPublic = E(zoe).getPublicFacet(governorInstance);

await assertContractGovernance(zoe, governedInstance, governorInstance);

return Promise.all([
E(governorPublic).validateVoteCounter(counterInstance),
E(governorPublic).validateRegistrar(registrar),
Expand Down
31 changes: 31 additions & 0 deletions packages/governance/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// @ts-check

export {
ChoiceMethod,
ElectionType,
QuorumRule,
looksLikeQuestionSpec,
positionIncluded,
looksLikeIssueForType,
buildUnrankedQuestion,
} from './question.js';

export {
validateQuestionDetails,
validateQuestionFromCounter,
} from './contractGovernor.js';

export { handleParamGovernance } from './contractHelper.js';

export {
makeParamChangePositions,
validateParamChangeQuestion,
assertBallotConcernsQuestion,
} from './governParam.js';

export { ParamType, assertType } from './paramManager.js';

export {
assertContractGovernance,
assertContractRegistrar,
} from './validators.js';
12 changes: 7 additions & 5 deletions packages/governance/src/question.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,17 @@ const QuorumRule = {
ALL: 'all',
};

/** @param {SimpleIssue} issue */
/** @type {LooksLikeSimpleIssue} */
const looksLikeSimpleIssue = issue => {
assert.typeof(
issue.text,
'string',
assert.typeof(issue, 'object', X`Issue ("${issue}") must be a record`);
assert(
issue && typeof issue.text === 'string',
X`Issue ("${issue}") must be a record with text: aString`,
);
return undefined;
};

/** @param {ParamChangeIssue} issue */
/** @type {LooksLikeParamChangeIssue} */
const looksLikeParamChangeIssue = issue => {
assert.typeof(
issue.paramSpec,
Expand Down Expand Up @@ -101,6 +102,7 @@ const positionIncluded = (positions, p) =>
positions.some(e => sameStructure(e, p));

// QuestionSpec contains the subset of QuestionDetails that can be specified before
/** @type {LooksLikeClosingRule} */
function looksLikeClosingRule(closingRule) {
const { timer, deadline } = closingRule;
Nat(deadline);
Expand Down
48 changes: 33 additions & 15 deletions packages/governance/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@
* available to the Registrar, which should wrap and attenuate it so each
* voter gets only the ability to cast their own vote at a weight specified by
* the registrar.
* attenuated wrapper that doesn't make that available!
* @property {SubmitVote} submitVote
*/

Expand Down Expand Up @@ -205,14 +204,33 @@

/**
* @callback LooksLikeQuestionSpec
* @param {QuestionSpec} allegedQuestionSpec
* @returns {QuestionSpec}
* @param {unknown} allegedQuestionSpec
* @returns { asserts allegedQuestionSpec is QuestionSpec }
*/

/**
* @callback LooksLikeParamChangeIssue
* @param {unknown} issue
* @returns { asserts issue is ParamChangeIssue }
*/

/**
* @callback LooksLikeIssueForType
* @param {ElectionType} electionType
* @param {Issue} issue
* @param {unknown} issue
* @returns { asserts issue is Issue }
*/

/**
* @callback LooksLikeSimpleIssue
* @param {unknown} issue
* @returns { asserts issue is SimpleIssue }
*/

/**
* @callback LooksLikeClosingRule
* @param {unknown} closingRule
* @returns { asserts closingRule is ClosingRule }
*/

/**
Expand All @@ -225,10 +243,10 @@
/**
* @typedef {Object} RegistrarPublic
* @property {() => Subscription<QuestionDetails>} getQuestionSubscription
* @property {() => ERef<Handle<'Question'>[]>} getOpenQuestions,
* @property {() => Promise<Handle<'Question'>[]>} getOpenQuestions,
* @property {() => string} getName
* @property {() => Instance} getInstance
* @property {(h: Handle<'Question'>) => ERef<Question>} getQuestion
* @property {(h: Handle<'Question'>) => Promise<Question>} getQuestion
*/

/**
Expand All @@ -241,16 +259,16 @@
* addQuestion() can be used directly when the creator doesn't need any
* reassurance. When someone needs to connect addQuestion to the Registrar
* instance, getPoserInvitation() lets them get addQuestion with assurance.
* @property {() => ERef<Invitation>} getPoserInvitation
* @property {() => Promise<Invitation>} getPoserInvitation
* @property {AddQuestion} addQuestion
* @property {() => Invitation[]} getVoterInvitations
* @property {() => Promise<Invitation>[]} getVoterInvitations
* @property {() => Subscription<QuestionDetails>} getQuestionSubscription
* @property {() => RegistrarPublic} getPublicFacet
*/

/**
* @typedef {Object} ClosingRule
* @property {Timer} timer
* @property {ERef<Timer>} timer
* @property {Timestamp} deadline
*/

Expand Down Expand Up @@ -391,8 +409,8 @@

/**
* @typedef {Object} GovernorPublic
* @property {() => ERef<Instance>} getRegistrar
* @property {() => ERef<Instance>} getGovernedContract
* @property {() => Promise<Instance>} getRegistrar
* @property {() => Promise<Instance>} getGovernedContract
* @property {(voteCounter: Instance) => Promise<boolean>} validateVoteCounter
* @property {(regP: ERef<Instance>) => Promise<boolean>} validateRegistrar
* @property {(details: QuestionDetails) => boolean} validateTimer
Expand Down Expand Up @@ -427,18 +445,18 @@
* A powerful facet that carries access to both the creatorFacet to be passed
* to the caller and the paramManager, which will be used exclusively by the
* ContractGovenor.
* @property {() => ERef<LimitedCreatorFacet>} getLimitedCreatorFacet
* @property {() => Promise<LimitedCreatorFacet>} getLimitedCreatorFacet
* @property {() => ParamManagerRetriever} getParamMgrRetriever
*/

/**
* @typedef {Object} GovernedContractCreatorFacet
* @property {VoteOnParamChange} voteOnParamChange
* @property {() => ERef<LimitedCreatorFacet>} getCreatorFacet - creator
* @property {() => Promise<LimitedCreatorFacet>} getCreatorFacet - creator
* 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
* @property {() => Promise<Instance>} getInstance - instance of the governed
* contract
*/

Expand Down Expand Up @@ -523,7 +541,7 @@
*
* Validate that the question details correspond to a parameter change question
* that the registrar hosts, and that the voteCounter and other details are
* consistent.
* consistent with it.
*
* @param {ERef<ZoeService>} zoe
* @param {Instance} registrar
Expand Down
12 changes: 11 additions & 1 deletion packages/governance/src/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ const assertContractGovernance = async (
zoe,
allegedGoverned,
allegedGovernor,
contractGovernorInstallation,
) => {
const allegedGovernorPF = E(zoe).getPublicFacet(allegedGovernor);
const realGovernedP = E(allegedGovernorPF).getGovernedContract();
const allegedGovernedTermsP = E(zoe).getTerms(allegedGoverned);

const [
{ electionManager: realGovernorInstance },
realGovernedInstance,
Expand All @@ -35,7 +37,15 @@ const assertContractGovernance = async (
X`The alleged governed did not match the governed contract retrieved from the governor`,
);

// TODO(3344): assert the installation once Zoe validates installations
const governorInstallationFromGoverned = await E(
zoe,
).getInstallationForInstance(realGovernorInstance);

assert(
governorInstallationFromGoverned === contractGovernorInstallation,
X`The governed contract is not governed by an instance of the provided installation.`,
);

return { governor: realGovernorInstance, governed: realGovernedInstance };
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
QuorumRule,
ElectionType,
looksLikeQuestionSpec,
} from '../../../src/question.js';
} from '../../../src/index.js';

const { quote: q } = assert;

Expand All @@ -24,7 +24,7 @@ const createQuestion = async (qDetails, closingTime, tools, quorumRule) => {
const { issue, positions, electionType } = qDetails;
const closingRule = {
timer: tools.timer,
deadline: 3n,
deadline: closingTime,
};

const questionSpec = looksLikeQuestionSpec(
Expand Down Expand Up @@ -66,8 +66,7 @@ const committeeBinaryStart = async (
positions: [harden({ text: 'Eeny' }), harden({ text: 'Meeny' })],
electionType,
};
const eeny = details.positions[0];
const meeny = details.positions[1];
const [eeny, meeny] = details.positions;
const tools = { registrarFacet, installations, timer };
const { counterInstance } = await createQuestion(
details,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const expectedCommitteeBinaryTwoQuestionsLog = [
'Carol voted on {"text":"Choose"} for {"text":"Two Potato"}',
'Dave voted on {"text":"Choose"} for {"text":"One Potato"}',
'Emma voted on {"text":"Choose"} for {"text":"One Potato"}',
'@@ schedule task for:3, currently: 0 @@',
'@@ schedule task for:4, currently: 0 @@',
'Alice voted on {"text":"How high?"} for {"text":"1 foot"}',
'Bob voted on {"text":"How high?"} for {"text":"2 feet"}',
'Carol voted on {"text":"How high?"} for {"text":"1 foot"}',
Expand All @@ -117,8 +117,8 @@ const expectedCommitteeBinaryTwoQuestionsLog = [
'@@ tick:2 @@',
'@@ tick:3 @@',
'&& running a task scheduled for 3. &&',
'&& running a task scheduled for 3. &&',
'@@ tick:4 @@',
'&& running a task scheduled for 4. &&',
'vote outcome: {"text":"One Potato"}',
'vote outcome: {"text":"1 foot"}',
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
import { E } from '@agoric/eventual-send';
import { Far } from '@agoric/marshal';

import { assertContractRegistrar } from '../../../src/validators.js';
import {
assertContractRegistrar,
assertContractGovernance,
} from '../../../src/validators.js';
import {
validateQuestionFromCounter,
validateQuestionDetails,
Expand Down Expand Up @@ -36,6 +39,13 @@ const build = async (log, zoe) => {
counterInstance,
);

const contractGovernanceP = assertContractGovernance(
zoe,
governedInstance,
governorInstance,
installations.contractGovernor,
);

const [
governedParam,
questionDetails,
Expand All @@ -44,6 +54,7 @@ const build = async (log, zoe) => {
governedInstallation,
governorInstallation,
validatedQuestion,
contractGovernance,
] = await Promise.all([
E.get(E(zoe).getTerms(governedInstance)).main,
E(E(zoe).getPublicFacet(counterInstance)).getDetails(),
Expand All @@ -52,6 +63,7 @@ const build = async (log, zoe) => {
E(zoe).getInstallationForInstance(governedInstance),
E(zoe).getInstallationForInstance(governorInstance),
validateQuestionFromCounterP,
contractGovernanceP,
]);

assertBallotConcernsQuestion(governedParam[0].name, questionDetails);
Expand All @@ -71,6 +83,10 @@ const build = async (log, zoe) => {
questionDetails,
);
assert(validatedQuestion, X`governor failed to validate registrar`);
assert(
contractGovernance,
X`governor and governed aren't tightly linked`,
);

log(`Voter ${name} validated all the things`);
},
Expand Down
2 changes: 1 addition & 1 deletion packages/governance/test/unitTests/test-ballotBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
ChoiceMethod,
ElectionType,
QuorumRule,
} from '../../src/question.js';
} from '../../src/index.js';

const issue = harden({ text: 'will it blend?' });
const positions = [harden({ text: 'yes' }), harden({ text: 'no' })];
Expand Down
Loading

0 comments on commit 8ebabda

Please sign in to comment.