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

Remove store from Localhost client #6198

Closed
4 tasks
fedekunze opened this issue May 12, 2020 · 1 comment · Fixed by #6202
Closed
4 tasks

Remove store from Localhost client #6198

fedekunze opened this issue May 12, 2020 · 1 comment · Fixed by #6202
Assignees

Comments

@fedekunze
Copy link
Collaborator

fedekunze commented May 12, 2020

Summary

Remove prefix store field from the localhost client and replace it with a cleaner approach.

Problem

The ICS09 localhost client requires to introspect the client store in order to verify state.

Nevertheless, the client needs to be stored in the state as well, so we end up storing the prefix store that is able to introspect itself.

// ClientState requires (read-only) access to keys outside the client prefix.
type ClientState struct {
store sdk.KVStore
ID string `json:"id" yaml:"id"`
ChainID string `json:"chain_id" yaml:"chain_id"`
Height int64 `json:"height" yaml:"height"`
}

This is an antipattern and makes things very confusing.

Solution

Update the ClientState interface to pass an sdk.KVStore to all state verification functions.

cc: @AdityaSripal @cwgoes @colin-axner


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@cwgoes
Copy link
Contributor

cwgoes commented May 12, 2020

Yes, I think this approach is preferable (also see #6189 (comment)).

@fedekunze fedekunze self-assigned this May 12, 2020
alessio pushed a commit that referenced this issue May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants