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.

Sign up for GitHub

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

Jump to bottom

ApplyValidatorSetChanges improvement #722

Closed
AdityaSripal opened this issue Feb 7, 2023 · 1 comment
Closed

ApplyValidatorSetChanges improvement #722

AdityaSripal opened this issue Feb 7, 2023 · 1 comment
Labels
source: audit To indicate an issue found during an audit.

Comments

@AdityaSripal
Copy link
Member

Seems to me like this function is taking the changes, adding them to a list and then returning the new list. This seems unnecessary to me, when we can just use the passed-in changes directly

interchain-security/x/ccv/consumer/keeper/validators.go

Line 16 in 463ec20

func (k Keeper) ApplyCCValidatorChanges(ctx sdk.Context, changes []abci.ValidatorUpdate) []abci.ValidatorUpdate {

@AdityaSripal AdityaSripal added the source: audit To indicate an issue found during an audit. label Feb 7, 2023
@mpoke mpoke mentioned this issue Feb 23, 2023
Closed
1 task
@mpoke
Copy link
Contributor

mpoke commented Feb 23, 2023

@AdityaSripal This approach is needed to deal with an edge case, see

interchain-security/x/ccv/consumer/keeper/validators.go

Line 53 in 463ec20

// edge case: we received an update for 0 power

@mpoke mpoke closed this as completed Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
source: audit To indicate an issue found during an audit.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@AdityaSripal @mpoke