feat: publish vote results from the voteCounter #6204
Conversation
@turadg, Is there a straightforward change to the test that would allow us to verify the published results?
Chris-Hibbert
added
enhancement
Agree. https://github.com/Agoric/agoric-sdk/blob/master/packages/inter-protocol/README.md#reading-data-off-chain is the closest we have, just the key structure. The data streams are basically documented by the unit tests but whatever gets onto Mainnet should have end-user documentation. Ideally formatted in a way that an storage node explorer tool can include it. |
|
This now has tests and is ready to review. |
Chris-Hibbert
requested review from
erights and
turadg
and removed request for
erights
| question: details.questionHandle, | ||
| outcome: 'No quorum', | ||
| }; | ||
| // @ts-expect-error Expand type to allow non-position? |
There was a problem hiding this comment.
yes, we should. I pushed e93c279
| @@ -123,6 +135,12 @@ const makeBinaryVoteCounter = (questionSpec, threshold, instance) => { | |||
| } else { | |||
| outcomePromise.resolve(questionSpec.tieOutcome); | |||
| } | |||
|
|
|||
| // XXX if we should distinguish ties, publish should be called in if above | |||
There was a problem hiding this comment.
This would be good too but I don't object to leaving this tech debt for later. If you want to do it now, e93c279 shows how.
| @@ -198,7 +216,7 @@ const makeBinaryVoteCounter = (questionSpec, threshold, instance) => { | |||
| /** | |||
| * @type {ContractStartFn<VoteCounterPublicFacet, VoteCounterCreatorFacet, {questionSpec: QuestionSpec, quorumThreshold: bigint}>} | |||
| */ | |||
| const start = zcf => { | |||
| const start = (zcf, { outcomesPublisher }) => { | |||
There was a problem hiding this comment.
Practically every publisher publishes more than once so the "s" plurality for that isn't informative. What is informative is what each thing they publish is. (That helps to distinguish from when each publication is not unary.)
In this case, outcomePublisher.
| @@ -158,13 +158,24 @@ const start = (zcf, privateArgs) => { | |||
| } | |||
| }; | |||
|
|
|||
| const outcomeNode = E(privateArgs.storageNode).makeChildNode( | |||
| 'latestOutcome', | |||
There was a problem hiding this comment.
a little hesitant to have a node for each of these but feels right to error on the side of more nodes so we don't create backwards compatibility problems for any nodes with broader responsibilities.
| const voteOutcome = { | ||
| question: details.questionHandle, | ||
| outcome: 'fail', | ||
| reason: 'No quorum', |
There was a problem hiding this comment.
you mentioned that "quorum" isn't the right term for this. Now may be a good time to improve the message. We can at least provide the threshold value for diagnostics.
e..g. Too few voters for threshold ${threshold}
There was a problem hiding this comment.
I'm resisting scope creep. I filed an issue (#6209) to fix the terminology from "quorum" to "threshold". The term "quorum" appears in too many places to want to fix it at the last minute.
closes: #6198
Description
publish vote results from the voteCounter
Security Considerations
this is already pubic.
Documentation Considerations
We should have description of the structure of the data we publish. Do we, and I'm just unaware?
Testing Considerations
The test doesn't yet verify the published data. I hope to add that before merging.