Allowing to unvote without having to switch to a new vote #218

arlai-mk · 2025-01-20T04:28:52Z

Description

Closes: #211

This PR introduces a new Unvote message allowing users to remove their votes from specific tranches by specifying lock IDs. Additionally, it includes a significant refactor of the voting system to optimize storage operations and improve code organization.

Key Changes

Added new Unvote message type for explicit vote removal
Moved voting logic to a new vote.rs module
Refactored storage operations to batch process changes

Implementation Details

New Files

Created vote.rs module containing:
- process_votes: Handles new vote application, once any previous vote has been removed
- process_unvotes: Manages vote removal
- validate_proposals_and_locks_for_voting: Input validation prior to voting

Major Improvements

Optimized Vote Processing
- Combined proposal power changes to reduce storage operations
- Added batch processing for validator shares updates
- Skips unnecessary vote removals when re-voting for same proposal
Enhanced Storage Operations
- New add_many_validator_shares_to_proposal function supporting both positive and negative changes
- Batch updates to SCALED_PROPOSAL_SHARES_MAP and PROPOSAL_TOTAL_MAP
Code Organization
- Split voting logic into separate functions for better maintainability
- Lock validation and skipping logic is done earlier

Minor Changes

While maintaining full functional compatibility, there are minor changes to the Vote message's response attributes: locks re-voting for the same proposal are now included in the locks_skipped attribute rather than the locks_voted attribute.

Testing Notes

Added tests for new unvote functionality
All previous unit tests still pass

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

Targeted the correct branch
Included the necessary unit tests
Added/adjusted the necessary interchain tests
Added a changelog entry in .changelog
Compiled the contracts by using make compile and included content of the artifacts directory into the PR
Regenerated front-end schema by using make schema and included generated files into the PR
Updated the relevant documentation or specification
Reviewed "Files changed" and left comments if necessary
Confirmed all CI checks have passed

…hould fail

…-unvote

p-offtermatt · 2025-01-22T10:20:42Z

contracts/hydro/src/vote.rs

+
+            let lock_entry = &lock_entries[&lock_id];
+
+            // If user didn't yet vote with the given lock in the given round and tranche, check


The comment seems outdated now. We don't have an explicit check anymore for whether the user has voted in the given round/tranche; if they already voted in this round, then implicitly their old VOTING_ALLOWED_ROUND was erased during process_unvotes

p-offtermatt

Looks good to me. Since this is a bit of a bigger change that touches core logic, I want to get a second set of eyes on it, so not approving yet.

Thanks a ton for the contribution!

p-offtermatt · 2025-01-22T10:53:28Z

The merge conflicts with main are quite small (regenerating checksums/artifacts/schema and organizing changelog).

I don't think I have permission to fix them (since the PR is from the moonkitt-lab hydro repo), could you take care of it? The only thing to pay attention to is that the merge automatically tried to put your changelog entry into 3.0.0 (it should stay in unreleased).

Thanks!

…nvote

arlai-mk · 2025-01-22T14:53:23Z

PR is fixed and should resolve conflicts. The changelog entry is kept in unreleased.

dusan-maksimovic

Left a few minor comments, but otherwise looks great to me! Thanks for your contribution!
Will need to be rebased, has some merge conflicts with main.

dusan-maksimovic · 2025-01-30T15:07:23Z

contracts/hydro/src/vote.rs

+            locks_voted.push(lock_id);
+        }
+
+        voted_proposals.push(proposal_id);


If all locks specified to vote for some proposal were skipped (for one reason or another), we would still insert that proposal ID here as voted. This seams to be the issue that existed before your PR. Can we fix this as well, by converting voted_proposals into HashSet<u64> and insert the proposal ID into it each time locks_voted.push(lock_id); is executed? Since it will be a Set there will be no duplicates. And then just convert it into Vec<u64>. WDYT?

dusan-maksimovic · 2025-01-30T15:25:05Z

contracts/hydro/src/score_keeper.rs

+                    update1
+                        .validator_shares
+                        .entry(validator)
+                        .and_modify(|existing| *existing += shares)


Can we have this function filter-out 0 values for some validator from the resulting HashSet? E.g. if we have
updates1: (proposal1, validator1) -500
updates2: (proposal1, validator1) +500

This should result in no entry for (proposal1, validator1).

…-unvote

* Introduce compounder cap (#220) * introduce participant's cap * Introduce snapshot map for total voting power tracking and handle its population * Added round<->height tracking and implemented validation for users locking in extra_cap based on their voting power in previous round. * Introduced USER_LOCKS to be able to calculate users voting power at a given height * - Added test for compunders cap and fixed couple of minor bugs within the new code - validate_denom() modified to be able to execute for past rounds as well * fixing errors after merge from main * added tests for user lock snapshoting and heght<->round tracking * added transaction to remove the constants at a specified timestamps * implementation of migration * CR required changes * added SNAPSHOTS_ACTIVATION_HEIGHT * added migration tests * added changelogs, recompiled contracts and schemas * added util function for ICQ results mock * fixed interchain tests (#222) * borrow input Deps, Env and MessageInfo whenever possible (#224) * Allowing to unvote without having to switch to a new vote (#218) * Refactoring vote and adding unvote feature * Make clippy happy * Remove unnecessary unvoted_proposals in process_unvotes * Adding a test case for unvote * Update wasms * Improving readability by adding / restoring comments, renaming variables * Store vote and voting allowed round directly in process_votes * Adding new test: attempts to unvote non-existing or not-owned locks should fail * Adding changelog * Fix HydroBase.client.ts as generated by make schema * Addressing Dusan's comments * Hydro DAO DAO governance support (#226) * implement hydro DAO DAO governance support * added changelog * CR changes * Prepare release for v3.1.0 (#227) * Prepare v3.1.0 release * Regenerate schema * Regenerate client types * Change migration and version to v3.1.0 * Fix migration mod --------- Co-authored-by: arlai <[email protected]> Co-authored-by: Philip Offtermatt <[email protected]>

arlai-mk added 9 commits January 19, 2025 14:21

Refactoring vote and adding unvote feature

493e7b1

Make clippy happy

b9c01f1

Remove unnecessary unvoted_proposals in process_unvotes

40889a3

Adding a test case for unvote

ee3bdb7

Update wasms

6bb5b12

Improving readability by adding / restoring comments, renaming variables

fee2772

Store vote and voting allowed round directly in process_votes

295e8d6

Adding new test: attempts to unvote non-existing or not-owned locks s…

5226840

…hould fail

Adding changelog

9a3ac9d

arlai-mk force-pushed the arlai/vote-refactor-unvote branch from 4837a83 to 9a3ac9d Compare January 20, 2025 04:39

arlai-mk marked this pull request as ready for review January 20, 2025 05:21

arlai-mk requested a review from a team as a code owner January 20, 2025 05:21

arlai-mk added 2 commits January 20, 2025 08:54

Merge remote-tracking branch 'upstream/main' into arlai/vote-refactor…

d95831f

…-unvote

Fix HydroBase.client.ts as generated by make schema

6a723e9

p-offtermatt reviewed Jan 22, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into arlai/vote-refactor-u…

46c3d2e

…nvote

dusan-maksimovic reviewed Jan 31, 2025

View reviewed changes

arlai-mk added 2 commits February 4, 2025 11:13

Merge remote-tracking branch 'upstream/main' into arlai/vote-refactor…

af76e1f

…-unvote

Addressing Dusan's comments

bdb6860

p-offtermatt approved these changes Feb 4, 2025

View reviewed changes

dusan-maksimovic approved these changes Feb 4, 2025

View reviewed changes

dusan-maksimovic merged commit 561166e into informalsystems:main Feb 4, 2025
4 checks passed

dusan-maksimovic mentioned this pull request Feb 10, 2025

Refactor voting method lockup iteration #164

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allowing to unvote without having to switch to a new vote #218

Allowing to unvote without having to switch to a new vote #218

arlai-mk commented Jan 20, 2025 •

edited

Loading

p-offtermatt Jan 22, 2025

p-offtermatt left a comment

p-offtermatt commented Jan 22, 2025

arlai-mk commented Jan 22, 2025

dusan-maksimovic left a comment

dusan-maksimovic Jan 30, 2025

dusan-maksimovic Jan 30, 2025


		let lock_entry = &lock_entries[&lock_id];

		// If user didn't yet vote with the given lock in the given round and tranche, check

Allowing to unvote without having to switch to a new vote #218

Allowing to unvote without having to switch to a new vote #218

Conversation

arlai-mk commented Jan 20, 2025 • edited Loading

Description

Key Changes

Implementation Details

New Files

Major Improvements

Minor Changes

Testing Notes

Author Checklist

p-offtermatt Jan 22, 2025

Choose a reason for hiding this comment

p-offtermatt left a comment

Choose a reason for hiding this comment

p-offtermatt commented Jan 22, 2025

arlai-mk commented Jan 22, 2025

dusan-maksimovic left a comment

Choose a reason for hiding this comment

dusan-maksimovic Jan 30, 2025

Choose a reason for hiding this comment

dusan-maksimovic Jan 30, 2025

Choose a reason for hiding this comment

arlai-mk commented Jan 20, 2025 •

edited

Loading