[Aptos Framework][Vesting] Pay out the accumulated rewards relatively to vesting's remaining grant instead of staking contract's principal #6106

movekevin · 2023-01-07T03:57:44Z

Description

Using the staking contract’s principal is indirect and the vesting contract can use it remaining grant amount as the base to get the actual amount of accumulated rewards so far. Here’s an example to make this clearer:

Vesting contract has 100 APT and 1 staking participant (to keep it simple). 100 APT is also the remaining amount. The underlying staking contract has a principal of 100 APT.
The stake pool earns 50 APT.
Before calculating outstanding rewards that belong to the vesting pool, vesting::unlock_rewards first requests commission to be paid out to the operator, which is 5 APT (10% of 50 APT earned). Remaining total active stake is 145 APT, all belonging to the vesting pool.
Since there’s been no tokens vested yet, the current vesting::unlock_rewards would calculate outstanding rewards to be 145 (total active in stake pool) - 100 (remaining_grant) = 45 to be distributed to staking contract participants.

Test Plan

Unit tests

zekun000 · 2023-01-07T09:50:38Z

aptos-move/framework/aptos-framework/sources/vesting.move

+        // Request commission first to make it easier to calculate what's the accumulated rewards that belong to the
+        // vesting pool.
+        let operator = vesting_contract.staking.operator;
+        staking_contract::request_commission(contract_signer, contract_address, operator);


do we need this since unlock_stake already does a request_commission internally? isn't the accumulated_rewards = total_active_stake - remaining_grant - commission_amount?

Great suggestion, thanks! Definitely a simpler and less redundant computation. Updated.

zekun000 · 2023-01-07T09:55:24Z

a completely orthogonal question, after some period of vesting, isn't it possible that the remaining grant drops below the minimal staking amount?

movekevin · 2023-01-07T10:05:43Z

a completely orthogonal question, after some period of vesting, isn't it possible that the remaining grant drops below the minimal staking amount?

Yes this is already a known potential issue in the far future. However, by then, we'll likely have gone through a couple more iterations and add some more features such as adding stake. Another alternative direction is to move the funds to another contract built on top of delegations.

… to vesting's remaining grant instead of staking contract's principal

zekun000 · 2023-01-07T14:08:59Z

aptos-move/framework/aptos-framework/sources/vesting.move

+        let operator = vesting_contract.staking.operator;
+        let (total_active_stake, _, commission_amount) =
+            staking_contract::staking_contract_amounts(contract_address, operator);
+        // Accumulated rewards, excluding unpaid commission, that entirely belongs to the shareholders.


I think the comment could be clearer about the difference between this and the accumulated rewards returned from the staking_contract_amounts call

areshand · 2023-01-12T00:30:57Z

aptos-move/framework/aptos-framework/sources/vesting.move

+        let commission_on_staker_rewards = (with_rewards(staker_rewards) - staker_rewards) / 10;
+        staker_rewards = with_rewards(staker_rewards) - commission_on_staker_rewards;
+        commission = with_rewards(commission) + commission_on_staker_rewards;
+        distribute(contract_address);


what is the expected reward for shareholder here? could we add an assert to make sure the calculation is correct?

github-actions · 2023-01-12T13:35:35Z

✅ Forge suite `compat` success on `testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b` ==> `9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f`

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 7490 TPS, 5202 ms latency, 7800 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f
compatibility::simple-validator-upgrade::single-validator-upgrade : 4575 TPS, 9043 ms latency, 11000 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f
compatibility::simple-validator-upgrade::half-validator-upgrade : 4641 TPS, 9023 ms latency, 11800 ms p99 latency,no expired txns
4. upgrading second batch to new version: 9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f
compatibility::simple-validator-upgrade::rest-validator-upgrade : 6622 TPS, 5949 ms latency, 11400 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f passed
Test Ok

Grafana dashboard
Humio Logs
Test runner output
Test run is land-blocking

github-actions · 2023-01-12T13:39:16Z

✅ Forge suite `land_blocking` success on `9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f`

performance benchmark with full nodes : 6604 TPS, 6008 ms latency, 8700 ms p99 latency,(!) expired 80 out of 2820320 txns
Test Ok

Grafana dashboard
Humio Logs
Test runner output
Test run is land-blocking

movekevin requested review from davidiw and wrwg as code owners January 7, 2023 03:57

movekevin force-pushed the vesting branch from 3a3322f to 5b30390 Compare January 7, 2023 06:04

movekevin requested review from 0xchloe, areshand, lightmark and zekun000 January 7, 2023 06:05

movekevin force-pushed the vesting branch from 5b30390 to dcf19b5 Compare January 7, 2023 06:16

zekun000 reviewed Jan 7, 2023

View reviewed changes

movekevin force-pushed the vesting branch 2 times, most recently from 5d5d273 to e6b537b Compare January 7, 2023 10:04

movekevin requested a review from zekun000 January 7, 2023 10:05

[Aptos Framework][Vesting] Pay out the accumulated rewards relatively…

9d791bc

… to vesting's remaining grant instead of staking contract's principal

movekevin force-pushed the vesting branch from e6b537b to 9d791bc Compare January 7, 2023 10:10

zekun000 approved these changes Jan 7, 2023

View reviewed changes

areshand reviewed Jan 12, 2023

View reviewed changes

davidiw approved these changes Jan 12, 2023

View reviewed changes

movekevin enabled auto-merge (rebase) January 12, 2023 12:51

This comment has been minimized.

Sign in to view

movekevin merged commit 05eabd7 into main Jan 12, 2023

movekevin deleted the vesting branch January 12, 2023 13:39

aptos-labs deleted a comment from Cho202 Jan 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Aptos Framework][Vesting] Pay out the accumulated rewards relatively to vesting's remaining grant instead of staking contract's principal #6106

[Aptos Framework][Vesting] Pay out the accumulated rewards relatively to vesting's remaining grant instead of staking contract's principal #6106

movekevin commented Jan 7, 2023 •

edited

zekun000 Jan 7, 2023

movekevin Jan 7, 2023

zekun000 commented Jan 7, 2023

movekevin commented Jan 7, 2023

zekun000 Jan 7, 2023

areshand Jan 12, 2023

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jan 12, 2023

github-actions bot commented Jan 12, 2023

[Aptos Framework][Vesting] Pay out the accumulated rewards relatively to vesting's remaining grant instead of staking contract's principal #6106

[Aptos Framework][Vesting] Pay out the accumulated rewards relatively to vesting's remaining grant instead of staking contract's principal #6106

Conversation

movekevin commented Jan 7, 2023 • edited

Description

Test Plan

zekun000 Jan 7, 2023

Choose a reason for hiding this comment

movekevin Jan 7, 2023

Choose a reason for hiding this comment

zekun000 commented Jan 7, 2023

movekevin commented Jan 7, 2023

zekun000 Jan 7, 2023

Choose a reason for hiding this comment

areshand Jan 12, 2023

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jan 12, 2023

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f

github-actions bot commented Jan 12, 2023

✅ Forge suite land_blocking success on 9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f

movekevin commented Jan 7, 2023 •

edited

✅ Forge suite `compat` success on `testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b` ==> `9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f`

✅ Forge suite `land_blocking` success on `9d791bc9f7d10c8c10ce0c18d8c03eeda6a70c4f`