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

balance stake for both ton pool contracts #15

Merged
merged 10 commits into from
Jan 16, 2025
Merged

Conversation

mkaczanowski
Copy link
Contributor

@mkaczanowski mkaczanowski commented Jan 15, 2025

TL;DR

Ton Pool has two pools. We want to naively balance the pools so that ideally two pools are always in set.

This PR implements the naive balancing, by taking into account:

  1. minStake - the amount of stake required for a pool to be in the set (taken from the lowest validator stake in past election)
  2. maxStake - the maximum amount of stake allowed per pool (taken from chain config)
  3. poolBalance - the current amount of available stake on the pool contract

The logic (to which u can see unittests) is simple, at first it'll try to fill up at least one if both are below minimum stake, and if both are above min, it'll fill the one with lower stake.

If both are above maximum it will throw error. @welldan97 - Is that okay? or shall we allow people to stake above max?

Test plan

$ npm run staking-cli -- ton tx delegate-pool 2 -c ./config.ton.json -s local -b
https://testnet.tonviewer.com/transaction/94cdd8e783589bb0e96083eb6c480ebded50021d88afde782c9eec5fdafc759f

$ npm run staking-cli -- ton tx unstake-pool 1.8 -c ./config.ton.json -s local -b -I 2
https://testnet.tonviewer.com/transaction/8eac38f46aef4be8a1e88b3202de010b33018c0f3a57294be84657bf4676d191

@mkaczanowski mkaczanowski marked this pull request as ready for review January 15, 2025 12:50
@mikalsande
Copy link

mikalsande commented Jan 15, 2025

  1. maxStake - the maximum amount of stake allowed per pool (taken from chain config)

I think you skip this step :) The current max on main net is 10 million TON staked by a single validator. It is totally OK for our pool contracts to have > 10 million TON. Eg. if our contracts both have 15 million TON we will just spread it across multiple validators 👍 The max stake limit is only for individual validators, not for our pool contracts.

If both are above maximum it will throw error. @welldan97 - Is that okay? or shall we allow people to stake above max?

No need, see answer above. For our pool contracts there is no maximum. We can just add more validators and make sure that each individual validator stakes less than the highest bid from the previous election and less than the configured max in the chain config 👍

Copy link

@mikalsande mikalsande left a comment

Choose a reason for hiding this comment

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

Nice work! The logic for spreading the stake among the contracts is simple and understandable 👍

This looks like it sends all the incoming stake to one of the pools. This is good for now, but later I think we will have to figure something out to send one transaction per pool. Lets get this merged first.

The code is commented just enough to help a TS n00b like me read it quickly 😄

Got a bit confused about variables being named validator when it looks like they are referencing a pool. Checking for the max allowed stake is not necessary, so that part can be removed and hopefully simplifies some part of the code.

packages/staking-cli/src/cmd/ton.ts Outdated Show resolved Hide resolved
packages/staking-cli/src/cmd/ton.ts Outdated Show resolved Hide resolved
packages/ton/src/TonPoolStaker.ts Outdated Show resolved Hide resolved
packages/ton/src/TonPoolStaker.ts Outdated Show resolved Hide resolved
@mkaczanowski
Copy link
Contributor Author

@mikalsande removed maxStake and renamed a few validator* vars. I kept the logic to pull the config from chain, because it may be used in the future (general rule is not to leave the dead code, but in this case I think that functionality is generic enough - and upstream ton client doesn't have it - apart from Ton4 which doesn't work either).

Copy link

@mikalsande mikalsande left a comment

Choose a reason for hiding this comment

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

LGTM! LFG 🚀

Copy link
Collaborator

@welldan97 welldan97 left a comment

Choose a reason for hiding this comment

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

Fantastic work!

packages/staking-cli/src/cmd/ton.ts Show resolved Hide resolved
packages/ton/test/pool-staker.spec.ts Show resolved Hide resolved
packages/ton/src/types.d.ts Outdated Show resolved Hide resolved
// iterate lastElection.frozen and find the lowest validator stake
const lastElection = elections[0]
const values = Array.from(lastElection.frozen.values())
const minStake = values.reduce((min, p) => p.stake < min ? p.stake : min, values[0].stake)
Copy link
Collaborator

Choose a reason for hiding this comment

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

QQ:
Some validators bid but don't get into an active set of validators because they propose too little. Does this min represent the min of the active set?

cc: @mikalsande coming from provider.get('past_elections', [])

Choose a reason for hiding this comment

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

Yes, it should be the active set. It fetches data from the last election, any validator that sends in too little stake should not have been part of that election. Actually, I have not verified whether this is true or not and it is difficult to verify because the elections are not full now :( Still, I think it is reasonable to use this.

If we want to really protect against using the wrong value is to check how long the list is. The chain config defines max validators in config option 16. So as long as the list is shorter or equal than that number (currently 400) it should be 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes this is the lowest of active set. For instance this is the data (using go code, but it's the same data):

 (electorcontract.Election) {
  ID: (*big.Int)(0xc0002a3460)(1736724232),
  UnfreezeAt: (*big.Int)(0xc0002a34a0)(1736822544),
  StakeHeld: (*big.Int)(0xc0002a3500)(32768),
  ValidatorSetHash: (*big.Int)(0xc0002a3560)(84901180556431012479828596589565587603167423808451322956044484867782103172852),
  FrozenDict: (map[string]electorcontract.ValidatorData) (len=386) {
   (string) (len=48) "Ef8EWBJwS-ClUAzpdk7x8acT6u_p7V1RUIdKQ1njUjU8KtTA": (electorcontract.ValidatorData) {
    Address: (*address.Address)(0xc000124500)(Ef8SgHecBgQgcpTvC_hKms7fr925q9jvWqi60d74_0Fmk7rO),
    Weight: (*big.Int)(0xc000160080)(2867459091096508),
    Stake: (*big.Int)(0xc0001600c0)(808364202020000),
    Banned: (bool) false
   },
   .... the rest of 386 vals

And then you look at: https://tonscan.com/validation/1736724232 and see the same data with non-zero wieght for each validator

Copy link
Contributor Author

@mkaczanowski mkaczanowski Jan 16, 2025

Choose a reason for hiding this comment

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

If we want to really protect against using the wrong value is to check how long the list is. The chain config defines max validators in config option 16. So as long as the list is shorter or equal than that number (currently 400) it should be 👍

That could work, but also the validators above 400 won't be in this past elections structure. The reason is simple, this there is a hash for a validator set is present in the structure, meaning that it is calculated (and verified!) using the pubkeys / addresses in the frozen set.

At least that's how 99% of the blockchains would do, but this is TON :)

@mkaczanowski mkaczanowski merged commit a765e98 into main Jan 16, 2025
1 check passed
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