Registrar 3188 #3299

Chris-Hibbert · 2021-06-11T16:57:24Z

Add CommitteeRegistrar and test in a swingSet tests to show vats and invitations working.

katelynsills

I think I need more information about why certain choices were made:

Why is checking for a quorum an async call?
What does verifying a ballot mean, and when does a user do that? How easy is it for them?
How is the notifier used? What use it is it to know the latest question?
Why hand out invitations if there is no use of Zoe's escrow? What is the model of trust? Do the users trust this contract? Do users need to convince other users that this contract is legitimate?

katelynsills · 2021-06-17T02:05:26Z

packages/governance/test/swingsetTests/committeeBinary/vat-voter.js

+            methodP,
+            qNameP,
+          ]);
+          log(`Verify ballot from instance: ${qName}, ${position}, ${method}`);


How is this verifying the ballot? does the voter have to request all of this information and compare each? That doesn't seem right. Why not have a WeakSet that contains all of the properly made ballots and validate by sending the ballot in to be checked against the WeakSet?

We will want better support for usability. At this point, I'm trying to demonstrate that a voter can get a ballot and tell that it's connected to a particular BallotCounter and Registrar.

If you'd rather wait until everything is connected, I can stop sending PRs until all the pieces have been written.

katelynsills · 2021-06-17T02:14:31Z

packages/governance/src/committeeRegistrar.js

+      };
+      E(clock).setWakeup(deadline, Far('waker', { wake }));
+
+      updater.updateState(questionDetails.question);


What is the purpose of this updater? As written, the state it publishes is the latest question. What would be a scenario in which this is used?

The voters use this updater to find out when new questions are added. If there are automated voters, they'll need something like this.

If they don't always respond immediately to each, they might use this as a prompt to check on older ones as well, so it will probably be helpful to make it possible to ask for all open questions (and maybe closed questions as well.) I haven't added that yet.

katelynsills · 2021-06-17T02:21:31Z

packages/governance/src/committeeRegistrar.js

+    }
+
+    // This could be parameterized. For now, we check that at least half voted.
+    const quorumChecker = Far('checker', {


Why not have this be a parameter of the ballot counter, rather than an object whose methods must be called asynchronously?

You're right that it should be a legible parameter of the ballot counter, and I've made that change.

For the moment (with only binary ballots), it can be synchronous, but eventually, I expect to want the quorum checking to be pluggable, which means it has to be external to the contract, and hence asynchronous.

I've converted binaryBallots to use a static specification of the quorum threshold, but I expect to refactor this later.

katelynsills · 2021-06-17T02:22:46Z

packages/governance/src/committeeRegistrar.js

+    const handler = voterSeat => {
+      return Far(`voter${index}`, {
+        castBallot: ballot =>
+          E(allQuestions.get(ballot.question)).submitVote(voterSeat, ballot),


Why are we giving out invitations if nothing is escrowed, and this is just a way to submit votes? Why not give out a useFacet with the submitVote method on it? What is the model of who trusts who?

Also, we do not want to encourage users to sell or otherwise pass on their invitation, right?

We use invitations so the recipient has some assurance of what they've received. The invitation allows them to verify which contract instance it came from, and look at the terms, which shows connections to other contract instances.

I don't see any reason to discourage people from passing invitations on. In democratic elections among people subject to a government it makes sense to insist that each ballot be cast by a unique person, but that's not important when the party receiving the ballot might be representing other interests.

And as MarkM sometimes says, it's a mistake to try to forbid what we can't prevent.

katelynsills · 2021-06-17T02:25:05Z

packages/governance/test/swingsetTests/committeeBinary/vat-voter.js

+  return Far('voter', {
+    createVoter: async (name, voteFacet) => {
+      return Far(`Voter ${name}`, {
+        voteBallot: (qName, ballot) => {


If the voters are just casting ballots, why use invitations?

The qName parameter is not used

This PR doesn't include an election manager, which will add some of the crucial connections to support validation.

I've added more legibility and more verification. I wouldn't claim it's done. Some of the legibility comes from the connections to the election manager.

katelynsills · 2021-06-17T02:28:30Z

packages/governance/test/swingsetTests/committeeBinary/bootstrap.js

+  const details2 = await E(zoe).getInvitationDetails(invitations[2]);
+
+  log(
+    `invitaiton details check: ${details2.instance === registrarInstance} ${


Suggested change

`invitaiton details check: ${details2.instance === registrarInstance} ${

`invitation details check: ${details2.instance === registrarInstance} ${

katelynsills · 2021-06-17T02:30:31Z

packages/governance/test/swingsetTests/committeeBinary/bootstrap.js

+  log(
+    `invitaiton details check: ${details2.instance === registrarInstance} ${
+      details2.description
+    }}`,


Suggested change

}}`,

}`,

katelynsills · 2021-06-17T02:31:02Z

packages/governance/test/swingsetTests/committeeBinary/test-committee.js

+const expectedCommitteeBinaryStartLog = [
+  '=> voter and registrar vats are set up',
+  '@@ schedule task for:3, currently: 0 @@',
+  'invitaiton details check: true Voter2}',


Suggested change

'invitaiton details check: true Voter2}',

'invitation details check: true Voter2',

Chris-Hibbert · 2021-06-19T00:49:37Z

I think I need more information about why certain choices were made:
Why is checking for a quorum an async call?

In the binary ballot case, checking quorums is simple enough that we can parameterize it. I've done that for now. I think after we build the more general case, binaryBallots will want to be compatible.

In the general case, we might want to check for a majority of stakeholders at the end of the voting period, so it the quorum checker might need to check with an Oracle. But we can leave this out for now.

I tried to answer your questions here.

What does verifying a ballot mean, and when does a user do that? How easy is it for them?

The first version of all this will make it possible to verify. Making it easier will be a progressive project.

How is the notifier used? What use it is it to know the latest question?

The point isn't that they want to know the most recent one, it's that they want to know when there is a new one. It should be possible for them to get a complete list of open questions (and closed questions as well). I didn't build those first.

Why hand out invitations if there is no use of Zoe's escrow? What is the model of trust? Do the users trust this contract? Do users need to convince other users that this contract is legitimate?

Invitations are the only means we have of assuring someone that they're talking to a particular private interface. I haven't thought of a reason why one person would want to convince someone else, but someone using a contract might want to understand its governance structure even if they aren't a voter.

The code that goes in vats (Registrar, BallotCounter, ElectionManager) is that way so that users and others can verify what code is running. When people get access to private facets, it has to be via an invitation so they can tell what they're getting.

I want the Treasury changes to be on top of the registrar (#3299) which is still in review. Git/GitHub doesn't allow an empty PR to be rebased onto anther PR, so this adds a commit..

katelynsills

Looks good. Just tiny fixes. One design question related to seemingly excessive asynchronous calls in order to vote.

If I understand correctly, one invitation allows you to vote on all further questions. Can we have a test that exercises voting on more than one question. I'd like to see the user flow.

katelynsills · 2021-06-22T18:16:56Z

yarn.lock

@@ -11246,6 +11246,11 @@ [email protected]:
    parseurl "~1.3.3"
    send "0.17.1"

+ses@^0.13.1:


Looks like we are specifying an old version of SES. Let's remove that entry in governance's package.json

katelynsills · 2021-06-22T18:22:14Z

packages/governance/package.json

@@ -31,19 +31,25 @@
  "homepage": "https://github.com/Agoric/agoric-sdk#readme",
  "dependencies": {
    "@agoric/assert": "^0.3.0",
+    "@agoric/bundle-source": "^1.3.9",


This is only used in tests, right? So it's a devDependency

katelynsills · 2021-06-22T18:22:59Z

packages/governance/package.json

    "@agoric/store": "^0.4.15",
+    "@agoric/swingset-vat": "^0.18.0",


Same with this. It's only used in tests, so should be a devDependency

katelynsills · 2021-06-22T18:23:56Z

packages/governance/package.json

    "ava": "^3.12.1",
-    "esm": "agoric-labs/esm#Agoric-built"
+    "esm": "agoric-labs/esm#Agoric-built",
+    "ses": "^0.13.1"


Should be "ses": "^0.13.2"

I bumped it to 0.13.2 even though webstorm tells me that the most recent is 0.13.4. Is there a reason to stick to that older one?

katelynsills · 2021-06-22T18:29:36Z

packages/governance/test/swingsetTests/committeeBinary/test-committee.js

+];
+
+test.serial('zoe - committee binary vote - valid inputs', async t => {
+  // test.serial('zoe - committee binary vote - valid inputs', async t => {


Can we remove this comment?

katelynsills · 2021-06-22T18:46:46Z

packages/governance/test/swingsetTests/committeeBinary/test-committee.js

+  'Verify ballot from instance: Choose, Eeny,Meeny, choose_n',
+  'Emma cast a ballot on Choose for Meeny',
+  'Verify: q: Choose, max: 1, committee: TheCommittee',
+  'Verify: registrar: [Alleged: InstanceHandle], counter: [Alleged: InstanceHandle]',


Hmm, can we log something that is more informative, such as comparing the instanceHandles to known ones?

sure. My intent was to demonstrate that the instances were available to be verified, but I can send in the known instances for checking.

katelynsills · 2021-06-22T18:52:06Z

packages/governance/test/unitTests/test-committee.js

+const counterRoot = `${__dirname}/../../src/binaryBallotCounter`;
+
+async function setupContract() {
+  const zoe = makeZoe(makeFakeVatAdmin(() => {}).admin);


This can just be fakeVatAdmin (imported from fakeVatAdmin directly) if you're not passing a function

thanks. Done

katelynsills · 2021-06-22T18:53:13Z

packages/governance/test/unitTests/test-committee.js

+    zoe.install(registrarBundle),
+    zoe.install(counterBundle),
+  ]);
+  const terms = { committeeName: 'illuminati', committeeSize: 13 };


katelynsills · 2021-06-22T18:57:43Z

packages/governance/test/unitTests/test-committee.js

+    details3,
+  );
+  // We didn't add any votes
+  E(counterPublic)


I think await t.throwsAsync() is the recommended way to test for a thrown error

Yes, it is. But it doesn't work here.

I may be confused, but what I think is happening here is tick() causes the problem, but it doesn't happen before tick() returns, so I can't put throwsAsync() around the tick(). It also doesn't work to put the throwsAsync() around getOutcome(), since the problem doesn't happen until after tick(). So what I'm doing here is putting a catch clause on getOutcome() that will handle the rejected promise that will arise later.

I added a comment repeating this description.

katelynsills · 2021-06-22T19:03:43Z

packages/governance/src/committeeRegistrar.js

+      ...questionDetailsShort,
+      registrar: zcf.getInstance(),
+    };
+    // facets of the ballot counter. Suppress creatorInvitation and adminFaceet.


What does "suppress" mean here?

Not returning them to the caller of addQuestion(). If they need to be held, they should be held somewhere else, I think.

Chris-Hibbert · 2021-06-22T22:55:50Z

If I understand correctly, one invitation allows you to vote on all further questions. Can we have a test that exercises voting on more than one question. I'd like to see the user flow.

Missed this earlier. I'll add a test now.

update testing imports for issue 768 add minimal types better fix for test dependencies change jsconfig.json includes, update packages, change swingset test (#3300) update committeeRegistrar for relocation of voterFacet integrate with changes to ballot PR (3233)

make ballotCounter instance accessible from ballot add more to terms of ballotCounter (closureRule) add more to public facet of Registrar one registrar per contract/vat: details in terms use zcf.getInstance() to supply instances In tests voters look up their own positions voters cast ballots on notification

move some dependencies to devDependencies expect promises for ballots allow voters to cast directly with registrar

add a test for multiple votes by a single electorate

I want the Treasury changes to be on top of the registrar (#3299) which is still in review. Git/GitHub doesn't allow an empty PR to be rebased onto anther PR, so this adds a commit..

Chris-Hibbert self-assigned this Jun 11, 2021

Chris-Hibbert added Beta enhancement New feature or request Governance Governance labels Jun 11, 2021

Chris-Hibbert added this to the Testnet: Stress Test Phase milestone Jun 11, 2021

Chris-Hibbert force-pushed the buildBallot-3188 branch from e0f4f4a to 1e25870 Compare June 11, 2021 17:51

Chris-Hibbert force-pushed the registrar-3188 branch from 3d6d505 to 427cbae Compare June 11, 2021 18:13

Chris-Hibbert requested a review from dtribble June 12, 2021 00:24

Chris-Hibbert assigned dtribble Jun 12, 2021

Chris-Hibbert requested a review from katelynsills June 15, 2021 20:02

Chris-Hibbert force-pushed the buildBallot-3188 branch from 1c9ea98 to a696c43 Compare June 15, 2021 20:42

Chris-Hibbert force-pushed the registrar-3188 branch from 7194fa3 to b048567 Compare June 15, 2021 23:09

katelynsills reviewed Jun 17, 2021

View reviewed changes

katelynsills removed this from the Testnet: Stress Test Phase milestone Jun 18, 2021

Chris-Hibbert force-pushed the buildBallot-3188 branch 3 times, most recently from ee7c381 to e943891 Compare June 18, 2021 22:54

Chris-Hibbert force-pushed the buildBallot-3188 branch 3 times, most recently from c297a97 to b2c0331 Compare June 20, 2021 17:40

Base automatically changed from buildBallot-3188 to master June 20, 2021 21:33

Chris-Hibbert force-pushed the registrar-3188 branch from 3e07196 to 259aa6d Compare June 20, 2021 23:43

Chris-Hibbert changed the base branch from master to closureRule June 21, 2021 16:31

Chris-Hibbert force-pushed the registrar-3188 branch from 259aa6d to 4ea1fc0 Compare June 21, 2021 16:32

Base automatically changed from closureRule to master June 21, 2021 22:14

Chris-Hibbert force-pushed the registrar-3188 branch 2 times, most recently from cd8f597 to a8bb7fb Compare June 21, 2021 23:19

Chris-Hibbert requested a review from katelynsills June 22, 2021 00:14

Chris-Hibbert mentioned this pull request Jun 22, 2021

Committee Binary Vote (Governance) #3188

Closed

katelynsills approved these changes Jun 22, 2021

View reviewed changes

Chris-Hibbert force-pushed the registrar-3188 branch from f46075b to dcd2735 Compare June 22, 2021 23:07

Chris-Hibbert added 7 commits June 23, 2021 10:03

refactor: closureRule renamed to closingRule

d580ae0

refactor: test and repair getOpenQuestions

93e86ab

chore: more cleanup

319fd73

chore: review suggestions

8c87532

move some dependencies to devDependencies expect promises for ballots allow voters to cast directly with registrar

refactor: await all the promises, not just the last in a chain

44e223f

add a test for multiple votes by a single electorate

Chris-Hibbert force-pushed the registrar-3188 branch from 8b92571 to 44e223f Compare June 23, 2021 17:06

Chris-Hibbert merged commit 7547357 into master Jun 23, 2021

Chris-Hibbert deleted the registrar-3188 branch June 23, 2021 17:35

	`invitaiton details check: ${details2.instance === registrarInstance} ${
	`invitation details check: ${details2.instance === registrarInstance} ${

	'invitaiton details check: true Voter2}',
	'invitation details check: true Voter2',

		"@agoric/store": "^0.4.15",
		"@agoric/swingset-vat": "^0.18.0",

Registrar 3188 #3299

Registrar 3188 #3299

Conversation

Chris-Hibbert commented Jun 11, 2021 • edited Loading

katelynsills left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chris-Hibbert commented Jun 19, 2021

katelynsills left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chris-Hibbert commented Jun 22, 2021

Chris-Hibbert commented Jun 11, 2021 •

edited

Loading