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

abci's ValidatorUpdates is not compliant with SDK's ProtoMessage #17040

Closed
likhita-809 opened this issue Jul 18, 2023 · 2 comments
Closed

abci's ValidatorUpdates is not compliant with SDK's ProtoMessage #17040

likhita-809 opened this issue Jul 18, 2023 · 2 comments
Assignees

Comments

@likhita-809
Copy link
Contributor

Summary

Problem Definition

With current abci type's ValidatorUpdates, its not possible to migrate it to collections. We'll need a custom codec for it.

Proposal

Create a custom value codec for abci types ValidatorUpdates which is compliant with cosmos sdk's ProtoMessage.

@likhita-809 likhita-809 changed the title ValidatorUpdates is not compliant with SDK's ProtoMessage abci's ValidatorUpdates is not compliant with SDK's ProtoMessage Jul 18, 2023
@tac0turtle
Copy link
Member

hmm could you point me to the code where this is the case

@likhita-809
Copy link
Contributor Author

likhita-809 commented Jul 20, 2023

hmm could you point me to the code where this is the case

Here: https://github.com/cosmos/cosmos-sdk/blob/main/x/staking/keeper/keeper.go#L132
while migrating ValidatorUpdates to collections, the initial idea is to use collections.Item[abci.ValidatorUpdates] for defining ValidatorUpdates field in keeper struct. In this case, those protov2 API's are not compatible to encode using collections value codec.
But we can also use staking types's ValidatorUpdates for it which @testinginprod pointed out. He also made a PR to support protov2 APIs encoding for future use cases

Closing this as it is addressed in #17042

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants