-
Notifications
You must be signed in to change notification settings - Fork 579
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
make localhost a stateless implementation #5959
Comments
I started looking into this and I found a couple of things that I wanted to discuss around the implementation. 1 Client state verificationAfter making localhost stateless a direct membership proof of the client state To get tests to pass I currently just put a check in to return early with no error if we are trying to verify that. // The localhost client is stateless, so if we need to verify the localhost client state, we can just return nil to verify
if merklePath.KeyPath[1] == host.FullClientStatePath(ModuleName) {
return nil
} Is this the way to do this, or is this actually something we don't need to support and just adjust the tests? 2 Client state queries in 02-clientIn 02-client there is also some consequences for quering client states. For getting a single client state, this is easy enough with a clientID check: if clientID == exported.LocalhostClientID {
return localhost.NewClientState(types.GetSelfHeight(ctx)), true // Or route the call to the light client module directly
} But for the paginated call to get ClientStates we don't get the localhost client state anymore from the store clientStates := types.IdentifiedClientStates{types.NewIdentifiedClientState(exported.LocalhostClientID, localhost.NewClientState(types.GetSelfHeight(ctx)))}
// then do the actually query
pageRes, err := query.FilteredPaginate(store, req.Pagination, func(key, value []byte, accumulate bool) (bool, error) {
... |
Great questions
|
Now that the light client module has the context for all its interface functions, the localhost implementation can be stateless as it should only read off the existing ibc store and not need to store any information under its client store
We should probably add a migration to prune unnecessary state and addition we should remove the additional disc writes that happen on every block to update the height in the localhost client state (set in the client store)
Can be removed:
Originally posted by @colin-axner in #5866 (comment)
The text was updated successfully, but these errors were encountered: