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.

Sign up for GitHub

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

fix: snapshot DAOs are relative majority #72

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix: snapshot DAOs are relative majority #72

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f714d96
fix: snapshot DAOs are relative majority
deep-quality-dev Feb 28, 2023
ce93639
fix: execute criteria for relative majority
deep-quality-dev Feb 28, 2023
5e69947
Merge branch 'main' into fix/majority
vistaproof Mar 1, 2023
c5e6b60
fix: use ethers library functions related with number
deep-quality-dev Mar 6, 2023
6f028a7
chore: use subpackages under @ethersproject
deep-quality-dev Mar 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/SDKInstanceClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class SDKInstanceClient implements SDKInstance {
votingThreshold,
minimumVotingParticipants: 1,
minimumTotalVotingTokens,
isRelativeMajority: false,
isRelativeMajority: true, // All the DAOs in snapshot are relative majority
},
{
delay: space.delay,
Expand Down Expand Up @@ -163,7 +163,7 @@ class SDKInstanceClient implements SDKInstance {
votingThreshold: 0,
minimumVotingParticipants: 1,
minimumTotalVotingTokens: '0',
isRelativeMajority: false,
isRelativeMajority: true,
},
undefined
);
Expand Down Expand Up @@ -205,7 +205,7 @@ class SDKInstanceClient implements SDKInstance {
votingThreshold: 0,
minimumVotingParticipants: 1,
minimumTotalVotingTokens: '0',
isRelativeMajority: false,
isRelativeMajority: true,
},
undefined
);
Expand Down
9 changes: 8 additions & 1 deletion src/client/ProposalClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,14 +260,21 @@ class ProposalClient implements Proposal {
}

canExecute(): boolean {
if (this.zDAO.isRelativeMajority) return false;
if (!this.zDAO.isRelativeMajority) return false;

const totalScore = this.scores.reduce((prev, current) => prev + current, 0);
const totalScoreAsBN = getDecimalAmount(
deep-quality-dev marked this conversation as resolved.
Show resolved Hide resolved
BigNumber.from(totalScore),
this.zDAO.votingToken.decimals
);
if (totalScoreAsBN.gte(this.zDAO.minimumTotalVotingTokens)) {
// Assume score[0] is the score for Approval or Yes
// If Approval > Deny in score, can execute
for (const score of this.scores) {
if (score > this.scores[0]) {
Copy link
Collaborator

@JamesEarle JamesEarle Mar 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure I understand this loop?

We iterate this.scores
Then, we check if the iteration is greater than this.scores[0], which will be the same value of the first iteration, and X > X is always false, and so this loop always hits return true below?

Trying to interpret the above comment, is this.scores[0] the score required for approval? and so when it is false on the first iteration that doesn't matter, and going forward it works? If this is correct please consider putting the required approval score in its own appropriately named variable.

// The minimum score required for approval
const approvalRequired = <some value>;

...
for (const score of this.scores) {
  if (score > approvalScore) {
    return false;
  }
}
...

Copy link
Contributor Author

@deep-quality-dev deep-quality-dev Mar 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I just want to check if the first score is the largest score in this.scores.
So I can skip first score by removing = in if condition and if found any value is greater than first score, then returns false.

const scores = [20, 15, 30, 19];
  for (const score of scores) {
    if (score > scores[0]) {
      return false;
    }
  }

In this example, 20 is less than 30, so returns false, but assume if first score is 30, then that is the largest value, returns true.
Please correct me if wrong.

return false;
}
}
return true;
}
return false;
Expand Down