Transition to public validators with arbitrary commission

[kkonevets]

We need to be able to add public validators to our pull with arbitrary commission. Now validation fee is 100%, which means all rewards go to validator vote account which Solido has withdraw authority from. Users should see a reasonable operational fee, e.g. 7%, which consists of validator, developer and treasury fees. The main idea is to let Solana pay to a public validator vote account directly and keep the rest of the rewards in stake accounts which are governed by Solido. Solido will have no control over the vote account. Treasury and developer fees are minted per validator as a percentage of a sum over rewards in validator stake accounts. User’s appreciation fee is kept in stake accounts, so it’s automatically compounded.

Main changes:

• add max_validation_fee to Solido state to limit maximum validation rewards
• remove rewards withdraw authority from Solido state
• remove fee_credit and fee_address from Validator, let Solana pay validation fees directly to vote accounts in SOL
• remove ClaimValidatorFee and CollectValidatorFee instructions. Developer and treasury fee minting logic is moved from CollectValidatorFee to WithdrawInactiveStake instruction
• calculate rewards in WithdrawInactiveStake per validator and mint developer and treasury fees for this validator as a portion of rewards in stake accounts. If we do so for all validators we will mint all developer and treasury fees in sum. We mint per validator because Solana has a limit of approximately max 20 accounts referenced in a single transaction, so we can’t reference all vote accounts and their stake accounts in a single transaction. Appreciation fees are kept at stake accounts as is, which means they are automatically compounded.
• add new permissionless instruction CheckMaxCommissionViolation which marks validator as inactive if he sets his commission over max_validation_fee, add maintenance function that periodically triggers CheckMaxCommissionViolation, add CLI entry and python test for this instruction.
• modify all tests and cli tools to take account of the changes

TODO:

• Add an instruction to convert on-chain state from previous version to the new which will arrive as a separate pull request
• Add a multisig instruction to change max_validation_fee
• We need to bypass a CPU and memory threshold on a number of possible validators in the pull, currently 64. For this we will integrate Solana stake-pool BigVec data structure to efficiently serialise/deserialise lists of validators and maintainers.
• We will not know Solido validator fee metrics (e.g. fee_validation_total_lamports), because a public validator can have stake accounts beyond Solido. So we can approximately compute it from amount of Solido stake

[kkonevets]

@ruuda @enriquefynn Please check this PR. I had to add SetMaxValidationFee instruction to this PR, otherwise juggling interdependent PRs would be cumbersome. @Popeye1357

[kkonevets]

There is weird error in CI on tests::fetch_pool_price::test_fail_fetch_pool_price_too_early panicked at 'Could not send transaction to fetch pool price.: IoError(Custom { kind: Other, error: "the request exceeded its deadline" }) On my local machine no errors are observed.

https://github.com/ChorusOne/solido/runs/6903960309?check_suite_focus=true

[kkonevets]

Could it be that GitHub CI is much slower than my local machine and it exceeded time deadline?

[kkonevets]

Yes, the same commit on following push was successful. All bpf tests succeeded. So GitHub CI was a bottleneck. I guess GitHub's test workload is not constant and depends on a time of day. @ruuda I have fixed everything you commented on, please review.

[ruuda]

Could it be that GitHub CI is much slower than my local machine and it exceeded time deadline?

Could be, but I remember seeing a similar error before, that was https://github.com/solana-labs/solana/issues/18201. It has been fixed since, but I wouldn’t be surprised if there are still bugs. But yeah it could also be a capacity issue, I suppose.

[kkonevets]

Thank you for your work, appreciate it. It ain't easy for me too to answer all comments. I fixed all I could, please check it out.

[kkonevets]

fixed

[ruuda]

Merged! The next step now would be the compatibility layer — a way for the new program to read the old state, and upgrade it in place. I also realized that Lido::deserialize_lido does not check the version yet. It should check that the version matches the expected version, and fail otherwise, because it will not be able to read the v0 state.

We will not know Solido validator fee metrics (e.g. fee_validation_total_lamports), because a public validator can have stake accounts beyond Solido. So we can approximately compute it from amount of Solido stake

Ah yes, this would be interesting to add a metric for, the current fee percentages of all enrolled validators.

[kkonevets]

Thanks. Currently I prepare a PR to integrate BigVec. That's a hole lot of changes. Only after that it makes sence to implement UpdateState instruction, because BigVec brings additional changes to the state. Thanks for the hint about Lido::deserialize_lido.

[ruuda]

There are a few places in Solido where we do a linear scan over all validators. In some cases it might be avoided by having the user pass in the index as part of the instruction (similar to how for program-derived addresses, we can pass in the bump seed instead of having to compute it at execution time). But for some instructions (e.g. stake and withdraw), we do really need a linear scan over all validators, to ensure that we stake with the least-staked validator and withdraw from the most-staked validator. From what I can tell about BigVec, it does not solve this problem, it only allows us to selectively deserialize some elements. What I would do instead is to avoid deserialization entirely. We already get the account data as a byte vector, we can work with that directly. Basically the approach that Cap'n Proto takes. The representation of Validator is stable, so we can transmute the bytes directly into a &mut Validator.

[kkonevets]

BigVec does exactly that. It does not deserialize with Borsh, but reinterprets list entries with an unsafe pointer cast. More over it can cheaply select entries with a binary predicate. For example, we can select a validator with a specific pubkey. So that's just a number of memcmp calls, which is not that long, but indexing would be faster for sure.

[kkonevets]

The problem with indexing is that first maintainer can be indexing into the list while the second maintainer is removing from it. So if the second maintainer removes a validator from the beginning of the list indexes will shift by 1 and the first maintainer will have no way to know that. Indexes are not "unique", pubkeys are much better

[ruuda]

So if the second maintainer removes a validator from the beginning of the list indexes will shift by 1 and the first maintainer will have no way to know that. Indexes are not "unique", pubkeys are much better

You would still verify the pubkey of course, but you can skip the linear search. If the pubkey at the user-provided index is not the expected one, simply fail the transaction.