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

[sui-system package] Set up a staking threshold of 1 SUI #9825

Closed

Conversation

dmitri-perelman
Copy link
Contributor

The threshold is set at the entry level to the Sui system object: the rewards to the validators might still be smaller than the threshold so we can't set it at the level of the staking pool.

Description

Describe the changes or additions included in this PR.

Test Plan

How did you test the new or updated feature?


If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

  • user-visible impact
  • breaking change for a client SDKs
  • breaking change for FNs (FN binary must upgrade)
  • breaking change for validators or node operators (must upgrade binaries)
  • breaking change for on-chain data layout
  • necessitate either a data wipe or data migration

Release notes

@vercel
Copy link

vercel bot commented Mar 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Ignored Deployments
Name Status Preview Comments Updated
explorer ⬜️ Ignored (Inspect) Mar 24, 2023 at 10:39PM (UTC)
explorer-storybook ⬜️ Ignored (Inspect) Mar 24, 2023 at 10:39PM (UTC)
wallet-adapter ⬜️ Ignored (Inspect) Mar 24, 2023 at 10:39PM (UTC)

// New exchange rate for validator 2: 660/300
// Staker 2 amounts for 0.5 * (660/300 * 100 - 100) + 100 = 160 SUI
// Validator 2 amounts for 660 - 160 = 500 SUI
// TODO: Investigate the discrepancy
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@emmazzz could you please help me with the investigation of this test? I'm not sure but there seems to be a problem with the calculations (my napkin calculations don't match the results)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @dmitri-perelman if we forget about the whole idea of exchange rate, and just think about how much staker 2 and validator 2 should get, we see that the numbers do match:

  • Validator 2 gets her share of the rewards plus half of staker 2's so that's 225 + 105/2 = 277.5 SUI in rewards. And together with the 225 SUI she already has, she gets 225 + 277.5 = 502.5 SUI, which is super close to 502499999999 MIST.
  • Staker 2 therefore gets 105 * 1.5 = 157.5 SUI in total.

Now back to looking at the exchange rate, the issue is with the new exchange rate for validator 2. It's not 660/300. When commissions are distributed, they aren't added as rewards to the pool. Instead they are added as new stakes of validators (this way the commission isn't shared with other stakers) to the pool. Also these commissions include commissions on the validator's self stake as well because, remember, validator is simply another staker.

Therefore we did get 0.5 * 30 = 15 more SUI of stake added to the pool the first time and 0.5 * 330 = 165 SUI added the second time. The first time that translated to 15 * (300 / (300 + 15)) = 14.28 pool tokens and the second time that translated to 165 * (314.28 / (330 + 165)) = 104.76 pool tokens. So the number of pool tokens we have in the pool after the second reward distribution is 300 + 14.28 + 104.76 = 419.04. And plugging in the formula we get:

  • Staker 2 amounts for 100 * (660 / 419.04) = 157.5
  • Validator 2 amounts for 660 - 157.5 = 502.5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow. @emmazzz your math is strong

@dmitri-perelman dmitri-perelman force-pushed the staking_threshold branch 3 times, most recently from be427d4 to e4d9389 Compare March 24, 2023 22:23
The threshold is set at the entry level to the Sui system object: the rewards to the validators might still be smaller than the threshold so we can't set it at the level of the staking pool.
@dmitri-perelman
Copy link
Contributor Author

@Jordan-Mysten given the new 1 SUI staking limit, how should I update the TestToolBox.setup to have the 1 SUI coins for tests in txn-builder, txn-serializer, governance? It's currently failing because the coins are smaller :)

@emmazzz
Copy link
Contributor

emmazzz commented Mar 27, 2023

Edit: looks like this amount was incremented two days ago to more than 1 SUI. So rebasing should fix the failures already.
Incrementing the amount of SUI the local faucet gives out should fix the test failures @dmitri-perelman

.send(Uuid::new_v4(), request_address, &[200_000_000_000; 5])

emmazzz added a commit that referenced this pull request Mar 28, 2023
## Description 

Took over the work @dmitri-perelman started (and almost finished) in
#9825 because I can't push to his fork of the repo.

## Test Plan 

Added some tests.

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [x] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

---------

Co-authored-by: Dmitri Perelman <dmitrip@Dmitris-MacBook-Pro.local>
@bmwill bmwill closed this Mar 31, 2023
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.

3 participants