`solana stakes <vote pubkey>` should cleanly handle a non-vote pubkey

solana-labs / solana

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.

https://solanalabs.com

Apache License 2.0

13.17k stars 4.27k forks source link

`solana stakes <vote pubkey>` should cleanly handle a non-vote pubkey #8441

Open mvines opened 4 years ago

mvines commented 4 years ago

solana stakes <identity pubkey> or solana stakes <does not exist pubkey> all silently return nothing.

Lets:

If possible make solana stakes <identity pubkey> report the stakes from the corresponding vote account
solana stakes <does not exist pubkey> report that the account was not found
solana stakes <some other account pubkey> report that the account is invalid

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

CriesofCarrots commented 3 years ago

This still seems helpful

deven96 commented 3 years ago

@CriesofCarrots I'd be willing to have a go at this. Any helpful information would be appreciated

CriesofCarrots commented 3 years ago

@deven96 , that would be great! I think the changes would likely go in this case: https://github.com/solana-labs/solana/blob/976298b292534155c4c9d54743240011f85840c8/cli/src/cluster_query.rs#L1716

The implementation should probably be something like:

call RpcClient::get_vote_accounts
Check each provided pubkey for existence in node_pubkey or vote_pubkey fields from that response
Build new vote-pubkey array from vote_pubkeys that were either provided or mapped from node_pubkeys
If any provided pubkeys are not present as either a node_pubkey or vote_pubkey, return an error. Open to suggestions as to whether any unknown pubkey should halt the process entirely, or just print the error and continue with valid pubkeys.

I'm also not certain we need to distinguish between account not found and account invalid as the issue description indicates. Happy to hear your thoughts on that.

deven96 commented 3 years ago

@CriesofCarrots printing the error and continuing with valid pubkeys seems fair. Also I'm of the opinion a general error with the account is okay but if we could further make it granular then fine

fee1-dead commented 2 years ago

@deven96 are you still working on this?

deven96 commented 2 years ago

Yes please @fee1-dead

kubanemil commented 6 months ago

@CriesofCarrots I wanted to take this issue, do you know why the last PR solving it was closed? As far as I can see it was a viable solution, wanted to finish it. Is it relevant?

CriesofCarrots commented 6 months ago

@kubanemil , you are very welcome to work on this. As far as I can tell, the previous PR had outstanding suggested changes, but the author disappeared. I have not yet looked at that code closely myself. I did, however, start working on this myself at point. I would say the trickiest piece is gathering the needed account information without adding a bunch of expensive RPC calls.

Anyway, I am happy to answer specific questions if you have them. Otherwise, I will keep an eye out for a PR from you to review.

kubanemil commented 6 months ago

@CriesofCarrots I have opened PR for this issue, but it is closed by chidocibot with following message:

Thank you for the contribution! This repository is no longer in use. Please re-open this pull request in the agave repo: https://github.com/anza-xyz/agave

Is it okay?

CriesofCarrots commented 6 months ago

@kubanemil , please follow the direction to open the pull request in the agave repo.

kubanemil commented 6 months ago

@CriesofCarrots please can you look at my PR?

CriesofCarrots commented 6 months ago

I saw the PR come through over the weekend. Extra pings not necessary.