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

stake-pool-cli: don't send UpdateValidatorListBalance transactions for subslices of validator list that have already been updated #6059

Conversation

billythedummy
Copy link
Contributor

Problem

The update command on the stake-pool-cli always sends update transactions for the entire validator list. This is redundant and increases the odds of failure if a previous update command failed in the middle since you don't have to run the UpdateValidatorListBalance instruction again on the parts of the validator list that were successfully updated. This is especially true for large stake pools - e.g. solblaze has all 2950 validators on their list.

Solution

In the update command, filter out UpdateValidatorListBalance instructions for subslices of the ValidatorList that have already been updated. Keep the current behaviour for the --force flag.

This builds on PR #6058

@mergify mergify bot added the community Community contribution label Jan 4, 2024
Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Nice idea! I left a few comments on CLI ergonomics. Once #6058 moves along we can circle back to this, or we can do this one first and just cherry pick the last commit and move it up, up to you!

stake-pool/cli/src/main.rs Outdated Show resolved Hide resolved
stake-pool/cli/src/main.rs Outdated Show resolved Hide resolved
@oasisMystre

This comment was marked as off-topic.

@billythedummy
Copy link
Contributor Author

Once #6058 moves along we can circle back to this, or we can do this one first and just cherry pick the last commit and move it up, up to you!

yeah i think lets finish #6058 first

@billythedummy billythedummy changed the title stake-pool-cli: don't send UpdateValidatorListBalance transactions for subslices of validator list that have already been updated [WIP] stake-pool-cli: don't send UpdateValidatorListBalance transactions for subslices of validator list that have already been updated Jan 20, 2024
@billythedummy billythedummy changed the title [WIP] stake-pool-cli: don't send UpdateValidatorListBalance transactions for subslices of validator list that have already been updated stake-pool-cli: don't send UpdateValidatorListBalance transactions for subslices of validator list that have already been updated Jan 20, 2024
Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for adding this new functionality. I just had a question about the flag. Curious what you think!

stake-pool/cli/src/main.rs Show resolved Hide resolved
stake-pool/cli/src/main.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

steak pool

@buffalojoec buffalojoec merged commit 0d6832e into solana-labs:master Jan 24, 2024
9 checks passed
billythedummy added a commit to igneous-labs/sanctum-spl-stake-pool that referenced this pull request Feb 9, 2024
…for subslices of validator list that have already been updated (solana-labs#6059)

* remove unnecessary vote acc slice

* nightly fmt

* fix clippy

* update stale validator_list_balance only if not force

* merge

* fmt

* merge conflict

* restore stale ixs, add fresh flag to update

* fmt

* fresh -> stale_only, update --force help
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants