near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.31k stars 618 forks source link

Expose validators as host function #2475

Closed ilblackdragon closed 4 years ago

ilblackdragon commented 4 years ago

For voting contract #2474 it's required to query current stake by validator account id.

TBD the APIs.

evgenykuzyakov commented 4 years ago

It's not simple to provide all validators with their stakes into the wasm because of limited support for serialization. Our current design for WASM was serialization independent, so we didn't settle on Borsh.

I suggest the following API: validator_stake(account_id_len: u64, account_id_ptr: u64, stake_ptr: u64) -> [] where

evgenykuzyakov commented 4 years ago

Another function we need is total_validator_stake(stake_ptr: u64) -> [] to return the total active stake balance. It's needed for denominator when resolving voting majority.

evgenykuzyakov commented 4 years ago

We can also expose the validators the following way: number_of_validators() -> u64 validator_id(validator_index: u64, register_id: u64) -> [] validator_stake(validator_index: u64, stake_ptr: u64) -> []

Which will make it flexible enough so long as we can pass validators in sorted order and the list of validators is not long enough.

bowenwang1996 commented 4 years ago

We also need validator_id_to_account_id(validator_index: u64, account_id_ptr: u64) -> [].

evgenykuzyakov commented 4 years ago

Sorry, I meant validator_id(validator_index: u64, register_id: u64) -> [] is validator_account_id(validator_index: u64, register_id: u64) -> []

register_id contains the account ID

bowenwang1996 commented 4 years ago

Do we actually need the list of all validators? It seems to me that having

validator_stake(account_id_len: u64, account_id_ptr: u64, stake_ptr: u64) -> []
total_validator_stake(stake_ptr: u64) -> []

is better because then if you want to know the stake of a specific validator, you don't need to iterate through all validators, which would be gas-consuming.

evgenykuzyakov commented 4 years ago

Let's create a spec change in NEPs. I think we can go with the following API:

number_of_validators() -> u64
validator_account_id(validator_index: u64, register_id: u64) -> []
validator_stake(validator_index: u64, stake_ptr: u64) -> []
validator_index(account_id_len: u64, account_id_ptr: u64) -> u64
total_validator_stake(stake_ptr: u64) -> []
evgenykuzyakov commented 4 years ago

Voting contract can't iterate through all validators, because they might not be elected in a given epoch. The total amount of validators who tried to stake (but didn't get elected into 100 seats) can be huge. We can't possibly know who was elected in a current epoch, unless we tracked every single stake proposal from the previous epoch, so this is not the case.

It leaves us with the only option for iterating through the list of currently active seats. Ideally they should be sorted in order from the largest stake to the lowest. Iterating through 100 seats is not a lot of gas, because it doesn't touch storage.

bowenwang1996 commented 4 years ago

@evgenykuzyakov I don't understand why the voting contract has to iterate through all validators. From my perspective, it is sufficient to know for each account id, what their stake is (0 if they are not a validator) and the total stake. With this information it is sufficient to determine whether a proposal has the majority of votes.

evgenykuzyakov commented 4 years ago

For this you have to iterate through all possible votes within an epoch. It means if the validator doesn't call the voting contract within this epoch, his vote will not be counted, unless you want to iterate through casted votes. We need to design a contract in such a way that if the epoch is 5 minutes, then we still should be able to complete voting. We shouldn't require validators to re-vote every epoch.

MaksymZavershynskyi commented 4 years ago

This seems to be a duplicate of https://github.com/nearprotocol/nearcore/pull/2504 . Closing it.

bowenwang1996 commented 4 years ago

@nearmax but there is no estimate on #2504, so I suggest keeping this one open until the PR is merged.