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

[framework] Partial governance voting support for delegation pool #8090

Merged
merged 17 commits into from
Jun 21, 2023

Conversation

xindingw
Copy link
Contributor

@xindingw xindingw commented May 8, 2023

Description

Delegators in a delegation pool can attend Aptos governance.
For a delegator:

  • The delegator could delegate its voting power(the amount equals to its stake) to a voter.
  • By default, a delegator's voter is itself.
  • If a delegator changes its voter, it won't take effect until the current lockup period ends.

For a voter:

  • The voting power of a voter equals to the sum of voting power of all delegators who delegate to the voter.

$$ VotingPower_{voter} = \Sigma_{d \in voter's\ delegators} VotingPower_d$$

  • The voter can use its voting power to vote on a proposal.
  • If the voting power of a voter meets the minimum threshold of creating a proposal, the voter can create a proposal.

Details

Pre-knowledge

  • A stake pool can vote on a proposal only when the lockup period of this stake pool ends after the proposal expiration.
  • Delegation pool's lockup period cannot be reset manually. So if the lockup period of a delegation pool ends, all proposals which this delegation pool voted during the lockup period have already expired.

High-level ideas

  • A delegator's voting power equals to its coins in active shares pool + its coins in pending inactive shares pool.
  • Due to rewards, the amount of coins keeps changing. But the shares of a delegator don't change until the next transaction of this delegator. The amount of coins can be calculated using shares.
  • To track a voter's voting power, its total shares in active shares pool and pending inactive shares pool need to be tracked. Whenever a delegator's voting power changes, its voter's voting power is also updated.
  • Because changing voter takes 1 lockup period to take effects, the total shares of the voter at the next lockup period also need to be tracked.
  • Pending inactive shares will become inactive at the next lockup period. So no need to track pending inactive shares at next lockup.

Let's say delegator D's voter is X at this lockup period and its voter will be changed to Y at the next lockup period. To summarize impacts on voter due to A's operations:

D's change X's active shares at this lockup X's pending_active shares at this lockup Y's active shares at next lockup Z's active shares at next lockup
+ A active shares +A +A
+ A pending inactive shares +A
- A active shares -A -A
- A pending inactive shares -A
Change voter at next lockup from Y to Z -D's shares +D's shares

🤖 Generated by Copilot at 4d58991

This pull request implements partial governance voting, a feature that allows voters to vote with a fraction of their delegated stake, on the Aptos blockchain. It adds and modifies spec functions, transaction payload functions, and entry function variants in the aptos_governance and aptos_framework_sdk_builder modules. It also adds a new module aptos_governance and a new function increase_lockup to the e2e-move-tests folder, and creates a new module vote with several test cases and scripts to test the partial voting feature.

Test Plan

Unit testing
delegation_pool.move: % Move Coverage: 93.49

@xindingw xindingw force-pushed the xinding/partial-voting-delegation-pool branch from cc2247a to 9a17ffd Compare May 8, 2023 18:32
@xindingw xindingw marked this pull request as ready for review May 11, 2023 18:54
@xindingw xindingw requested a review from movekevin May 11, 2023 18:54
@xindingw
Copy link
Contributor Author

xindingw commented May 11, 2023

@movekevin Think more about moving GovernanceRecords out of delegation_pool module. The main problem is that active_shares and inactive_shares are needed when populating default values of DelegatedVotes but the values are stored inside DelegationPool(we cannot call delegation_pool module otherwise there is cyclic depedency). There could be 2 ways for the problem:

  1. When get DelegatedVotes of a voter, always pass its active_shares and inactive_shares as well. Semantically it's weird and ugly.
  2. DelegatedVotes doesn't count the voter itself's voting power. Add the voter power back in delegation_pool module if a delegator's voter is itself. But this makes DelegatedVotes more counterintuitive.

TBH I think moving GovernanceRecords out can reduce the size of delegation_pool.move(it has ~4000 lines now 😞 ) but it won't make the logic cleaner/easier to understand because it's so coupled with delegation_pool.move. Personally I prefer leave GovernanceRecords there and add more documentation to explain the logic. This is pretty tricky so let me know WDYT.

Copy link
Contributor

@movekevin movekevin left a comment

Choose a reason for hiding this comment

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

Looks correct to me high-level wise, just some minor comments and one important high-level comment: Let's create a separate feature flag for enabling delegation voting

@sionescu sionescu requested a review from a team as a code owner May 22, 2023 20:14
@xindingw xindingw force-pushed the xinding/partial-voting-delegation-pool branch from 0741772 to bb950c7 Compare May 31, 2023 18:39
@xindingw xindingw force-pushed the xinding/partial-voting-delegation-pool branch from f386908 to 3f9e51e Compare June 21, 2023 00:05
@xindingw xindingw enabled auto-merge (squash) June 21, 2023 00:06
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58 (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : committed: 9169 txn/s, latency: 3695 ms, (p50: 3700 ms, p90: 5000 ms, p99: 5900 ms), latency samples: 302600
2. Upgrading first Validator to new version: 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 5080 txn/s, latency: 6563 ms, (p50: 7400 ms, p90: 8700 ms, p99: 9100 ms), latency samples: 182880
3. Upgrading rest of first batch to new version: 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 4677 txn/s, latency: 6831 ms, (p50: 7500 ms, p90: 8400 ms, p99: 9100 ms), latency samples: 173080
4. upgrading second batch to new version: 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6724 txn/s, latency: 4921 ms, (p50: 4900 ms, p90: 6700 ms, p99: 8000 ms), latency samples: 235340
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58 passed
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite land_blocking success on 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58

performance benchmark : committed: 5567 txn/s, latency: 7104 ms, (p50: 5700 ms, p90: 11400 ms, p99: 26200 ms), latency samples: 2377400
Max round gap was 1 [limit 4] at version 782952. Max no progress secs was 3.6100159 [limit 10] at version 2528992.
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58

Compatibility test results for aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58 (PR)
Upgrade the nodes to version: 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 4109 txn/s, latency: 7198 ms, (p50: 7200 ms, p90: 9400 ms, p99: 16500 ms), latency samples: 168500
5. check swarm health
Compatibility test for aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 96e3918f13ac8bbb6c4f38d1cbf2d3099ede5f58 passed
Test Ok

@xindingw xindingw merged commit 1b436b2 into main Jun 21, 2023
@xindingw xindingw deleted the xinding/partial-voting-delegation-pool branch June 21, 2023 02:59
banool pushed a commit that referenced this pull request Jul 7, 2023
)

* Support partial voting in aptos-governance

* Support delegation partial governance voting

* Add comments

* Refactor naming.
Fix a edge case when a delegation pool voter votes before enabling partial governance voting on this delegation pool.
Add DelegateVotingPowerEvent.
Fix typos.

* Remove redundant condition checking

* Fix naming
Fix comments
Remove get_latest_delegator_vote_delegation and get_latest_vote_delegation
Wrap logic of updating governance records into functions
Fix a bug of governance records when withdrawing inactive shares
Fix get_remaining_voting_power in aptos_governance when proposals expire

* Add TODOs

* Fix lint

* Fix struct annotation

* Fix a typo

* Fix a bug about voting power
Fix a comment
Add additional feature flag checking in delegation_pool.move

* Fix specs

* Remove unnecessary move prover change

* Fix enable_partial_governance_voting condition in initialize_delegation_pool

* Remove redundant borrow
Remove unnecessary prover spec pragma

* Use smart_table instead of table in aptos_governance

---------

Co-authored-by: Teng Zhang <[email protected]>
xbtmatt pushed a commit to xbtmatt/aptos-core that referenced this pull request Jul 25, 2023
…tos-labs#8090)

* Support partial voting in aptos-governance

* Support delegation partial governance voting

* Add comments

* Refactor naming.
Fix a edge case when a delegation pool voter votes before enabling partial governance voting on this delegation pool.
Add DelegateVotingPowerEvent.
Fix typos.

* Remove redundant condition checking

* Fix naming
Fix comments
Remove get_latest_delegator_vote_delegation and get_latest_vote_delegation
Wrap logic of updating governance records into functions
Fix a bug of governance records when withdrawing inactive shares
Fix get_remaining_voting_power in aptos_governance when proposals expire

* Add TODOs

* Fix lint

* Fix struct annotation

* Fix a typo

* Fix a bug about voting power
Fix a comment
Add additional feature flag checking in delegation_pool.move

* Fix specs

* Remove unnecessary move prover change

* Fix enable_partial_governance_voting condition in initialize_delegation_pool

* Remove redundant borrow
Remove unnecessary prover spec pragma

* Use smart_table instead of table in aptos_governance

---------

Co-authored-by: Teng Zhang <[email protected]>
xbtmatt pushed a commit to xbtmatt/aptos-core that referenced this pull request Jul 25, 2023
…tos-labs#8090)

* Support partial voting in aptos-governance

* Support delegation partial governance voting

* Add comments

* Refactor naming.
Fix a edge case when a delegation pool voter votes before enabling partial governance voting on this delegation pool.
Add DelegateVotingPowerEvent.
Fix typos.

* Remove redundant condition checking

* Fix naming
Fix comments
Remove get_latest_delegator_vote_delegation and get_latest_vote_delegation
Wrap logic of updating governance records into functions
Fix a bug of governance records when withdrawing inactive shares
Fix get_remaining_voting_power in aptos_governance when proposals expire

* Add TODOs

* Fix lint

* Fix struct annotation

* Fix a typo

* Fix a bug about voting power
Fix a comment
Add additional feature flag checking in delegation_pool.move

* Fix specs

* Remove unnecessary move prover change

* Fix enable_partial_governance_voting condition in initialize_delegation_pool

* Remove redundant borrow
Remove unnecessary prover spec pragma

* Use smart_table instead of table in aptos_governance

---------

Co-authored-by: Teng Zhang <[email protected]>
gedigi pushed a commit that referenced this pull request Aug 2, 2023
)

* Support partial voting in aptos-governance

* Support delegation partial governance voting

* Add comments

* Refactor naming.
Fix a edge case when a delegation pool voter votes before enabling partial governance voting on this delegation pool.
Add DelegateVotingPowerEvent.
Fix typos.

* Remove redundant condition checking

* Fix naming
Fix comments
Remove get_latest_delegator_vote_delegation and get_latest_vote_delegation
Wrap logic of updating governance records into functions
Fix a bug of governance records when withdrawing inactive shares
Fix get_remaining_voting_power in aptos_governance when proposals expire

* Add TODOs

* Fix lint

* Fix struct annotation

* Fix a typo

* Fix a bug about voting power
Fix a comment
Add additional feature flag checking in delegation_pool.move

* Fix specs

* Remove unnecessary move prover change

* Fix enable_partial_governance_voting condition in initialize_delegation_pool

* Remove redundant borrow
Remove unnecessary prover spec pragma

* Use smart_table instead of table in aptos_governance

---------

Co-authored-by: Teng Zhang <[email protected]>
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.

5 participants