Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the way SPO votes are counted (Fix) #600

Merged
merged 5 commits into from
Oct 29, 2024

Conversation

williamdemeo
Copy link
Contributor

@williamdemeo williamdemeo commented Oct 25, 2024

Description

This closes issue #578.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • Any semantic changes to the specifications are documented in CHANGELOG.md
  • Code is formatted according to CONTRIBUTING.md
  • Self-reviewed the diff

@williamdemeo williamdemeo linked an issue Oct 25, 2024 that may be closed by this pull request
@williamdemeo williamdemeo self-assigned this Oct 25, 2024
@williamdemeo williamdemeo marked this pull request as ready for review October 25, 2024 21:52
Copy link
Contributor

@Lucsanszky Lucsanszky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conformance tests are passing, so LGTM if Andre approves it too! 👍

Copy link
Collaborator

@WhatisRT WhatisRT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some changes to the alignments and prose necessary

src/Ledger/Ratify.lagda Outdated Show resolved Hide resolved
src/Ledger/Ratify.lagda Show resolved Hide resolved
src/Ledger/Ratify.lagda Outdated Show resolved Hide resolved
Comment on lines 403 to 404
\item \textit{during the bootstrap period}: if an SPO didn't vote, then their vote is counted as \no;
\item \textit{after the bootstrap period}: if an SPO didn't vote, then their vote is counted as \no
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason to split this here, the bootstrap period is covered in an appendix. Also, the bootstrap logic is unchanged, so it should state in the appendix that not voting counts as abstain there.

Additionally, this should also explain that delegating to a DRep is like not delegating at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the first point and will remove the split and bootstrap behavior description.

I don't understand the second point: "Delegating to a DRep is like not delegating at all."?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially (while not delegated) the default is no, and if you delegate to a DRep the default is no as well.

src/Ledger/Ratify.lagda Outdated Show resolved Hide resolved
src/Ledger/Ratify.lagda Outdated Show resolved Hide resolved
src/Ledger/Ratify.lagda Outdated Show resolved Hide resolved
@williamdemeo williamdemeo force-pushed the 578-change-the-way-spo-votes-are-counted-FIX branch from 82c9bc0 to 18b1e0b Compare October 28, 2024 18:17
In particular, make sure it's clear that the credential used for voting is not
the same as the credential which can now be used to set the default voting behavior.
src/Ledger/Ratify.lagda Outdated Show resolved Hide resolved
Copy link
Collaborator

@WhatisRT WhatisRT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some minor stuff I noticed while staring at the PDF

@williamdemeo williamdemeo merged commit 7cc5c06 into master Oct 29, 2024
5 of 6 checks passed
@williamdemeo williamdemeo deleted the 578-change-the-way-spo-votes-are-counted-FIX branch October 29, 2024 15:30
github-actions bot added a commit that referenced this pull request Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change the way SPO votes are counted
3 participants