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

Fix several issue with governance and pgf #2459

Merged
merged 7 commits into from
Jan 30, 2024

Conversation

Fraccaman
Copy link
Member

@Fraccaman Fraccaman commented Jan 26, 2024

Describe your changes

A bug was found by @zenodeapp during a public testnet regarding an underflow in governance VP. This PR aims to fix it.

  • Avoid logging and underflow bug
  • Use validator stake instead of total validator when computing vote tally
  • PGFAction for both pgf steward and pgf funding proposal must have unique targets
  • When executing PGF proposals, collect the PGFAction into a sorted collection to avoid inconsistent states due to ordering
  • Enforce a voter to submit all the complete list of validator for which he has a delegation

Indicate on which release or other PRs this topic is based on

Checklist before merging to draft

  • I have added a changelog
  • Git history is in acceptable state

@Fraccaman Fraccaman requested a review from grarco January 26, 2024 16:21
@zenodeapp
Copy link
Contributor

Burn it down 🔥🚒!

...with the right intentions :).

grarco
grarco previously approved these changes Jan 26, 2024
@Fraccaman Fraccaman mentioned this pull request Jan 26, 2024
@Fraccaman Fraccaman marked this pull request as ready for review January 26, 2024 19:54
@Fraccaman Fraccaman force-pushed the fraccaman/fix-governance-underflow branch from ebaf470 to 31788b4 Compare January 28, 2024 18:04
@Fraccaman Fraccaman force-pushed the fraccaman/fix-governance-underflow branch from 96ff8fd to f1d994d Compare January 28, 2024 22:52
.iter()
.map(|steward| match steward {
AddRemove::Add(address) => address,
AddRemove::Remove(address) => address,
Copy link
Member Author

Choose a reason for hiding this comment

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

this is wrong I need to map it to a BTreeSet

@Fraccaman Fraccaman force-pushed the fraccaman/fix-governance-underflow branch from f1d994d to fda71f4 Compare January 29, 2024 10:49
@Fraccaman Fraccaman force-pushed the fraccaman/fix-governance-underflow branch from fda71f4 to dd5b24f Compare January 29, 2024 11:41
@Fraccaman Fraccaman changed the title avoid logging an underflow Fix several issue with governance and pgf Jan 29, 2024
Fraccaman pushed a commit that referenced this pull request Jan 29, 2024
* origin/fraccaman/fix-governance-underflow:
  add changelog
  check for duplicate in pgf action proposals
  restrict delegation field
  validate vote key in governance vp
  avoid duplicate addresses in pgf steward proposal, use btreeset instead of hashset
  use validator voting power instead of total voting power
  avoid logging an underflow, more underflow checks
brentstone added a commit that referenced this pull request Jan 29, 2024
* origin/fraccaman/fix-governance-underflow:
  add changelog
  check for duplicate in pgf action proposals
  restrict delegation field
  validate vote key in governance vp
  avoid duplicate addresses in pgf steward proposal, use btreeset instead of hashset
  use validator voting power instead of total voting power
  avoid logging an underflow, more underflow checks
@brentstone brentstone merged commit 6e41ec9 into main Jan 30, 2024
13 of 15 checks passed
@brentstone brentstone deleted the fraccaman/fix-governance-underflow branch January 30, 2024 01:33
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.

4 participants