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

Prohibit governance voting from jailed and inactive validators #3004

Merged
merged 22 commits into from
Apr 12, 2024

Conversation

brentstone
Copy link
Collaborator

@brentstone brentstone commented Apr 3, 2024

Describe your changes

Closes #2796.

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

Based on #2877, which is based on 0.32.0.

diff https://github.com/anoma/namada/pull/3004/files/5c0fbeb6ef60c41e8837dfd56fc91751ccd8bac7..5e490ab9dab3ab4dea64407f31d2deef98a9d25e

Checklist before merging to draft

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

Copy link

codecov bot commented Apr 3, 2024

Codecov Report

Attention: Patch coverage is 89.22040% with 224 lines in your changes are missing coverage. Please review.

Project coverage is 55.14%. Comparing base (5e0b162) to head (5e490ab).
Report is 4 commits behind head on main.

❗ Current head 5e490ab differs from pull request most recent head 28ddef6. Consider uploading reports for the commit 28ddef6 to get more accurate results

Files Patch % Lines
crates/sdk/src/tx.rs 0.00% 103 Missing ⚠️
crates/apps/src/lib/client/rpc.rs 0.00% 32 Missing ⚠️
...rates/apps/src/lib/node/ledger/shell/governance.rs 26.82% 30 Missing ⚠️
crates/governance/src/storage/proposal.rs 0.00% 13 Missing ⚠️
crates/apps/src/lib/cli.rs 0.00% 10 Missing ⚠️
crates/proof_of_stake/src/storage_key.rs 38.46% 8 Missing ⚠️
crates/namada/src/ledger/governance/mod.rs 99.70% 5 Missing ⚠️
crates/sdk/src/lib.rs 0.00% 5 Missing ⚠️
crates/governance/src/storage/mod.rs 0.00% 4 Missing ⚠️
crates/sdk/src/args.rs 0.00% 3 Missing ⚠️
... and 6 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3004      +/-   ##
==========================================
+ Coverage   53.44%   55.14%   +1.69%     
==========================================
  Files         310      309       -1     
  Lines      101574   102816    +1242     
==========================================
+ Hits        54288    56694    +2406     
+ Misses      47286    46122    -1164     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@brentstone brentstone force-pushed the brent/jailed-inactive-vals-no-voting branch from 6e1184c to bdfe46c Compare April 5, 2024 04:05
@brentstone brentstone marked this pull request as ready for review April 5, 2024 05:56
brentstone added a commit that referenced this pull request Apr 6, 2024
* brent/jailed-inactive-vals-no-voting:
  fix proposal_submission e2e test
  fix good validator collection in client
  fix VPs
  changelog: add #3004
  safer state checking
  client: prohibit votes from jailed and inactive validators
  read total active stake (exclude jailed + inactive)
  ignore jailed and inactive validators when computing tally
  active total deltas (voting power) - exclude jailed and inactive validators
tzemanovic
tzemanovic previously approved these changes Apr 8, 2024
Copy link
Member

@tzemanovic tzemanovic left a comment

Choose a reason for hiding this comment

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

LGTM! I'd maybe just rename all the active_voting_power occurrences to total_deltas_sums (as in epoched sums of all validators' total deltas)

tzemanovic added a commit that referenced this pull request Apr 8, 2024
* origin/brent/jailed-inactive-vals-no-voting:
  fix proposal_submission e2e test
  fix good validator collection in client
  fix VPs
  changelog: add #3004
  safer state checking
  client: prohibit votes from jailed and inactive validators
  read total active stake (exclude jailed + inactive)
  ignore jailed and inactive validators when computing tally
  active total deltas (voting power) - exclude jailed and inactive validators
tzemanovic added a commit that referenced this pull request Apr 8, 2024
* origin/brent/jailed-inactive-vals-no-voting:
  rename total_active_voting_power --> total_active_deltas
@brentstone brentstone mentioned this pull request Apr 9, 2024
2 tasks
tzemanovic added a commit that referenced this pull request Apr 10, 2024
* origin/brent/jailed-inactive-vals-no-voting:
  rename total_active_voting_power --> total_active_deltas
  fix proposal_submission e2e test
  fix good validator collection in client
  fix VPs
  changelog: add #3004
  safer state checking
  client: prohibit votes from jailed and inactive validators
  read total active stake (exclude jailed + inactive)
  ignore jailed and inactive validators when computing tally
  active total deltas (voting power) - exclude jailed and inactive validators
@tzemanovic tzemanovic merged commit 2513946 into main Apr 12, 2024
15 of 17 checks passed
@tzemanovic tzemanovic deleted the brent/jailed-inactive-vals-no-voting branch April 12, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not count votes (or delegator votes) for jailed validators
2 participants