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

Support client, connection, and channel recycling #349

Closed
colin-axner opened this issue Jan 5, 2021 · 3 comments · Fixed by #386
Closed

Support client, connection, and channel recycling #349

colin-axner opened this issue Jan 5, 2021 · 3 comments · Fixed by #386
Assignees
Labels
T: Enhancement TYPE: Enhancement

Comments

@colin-axner
Copy link
Contributor

in client, connection, and channel creating in places marked with

// TODO: Query for existing identifier and fill config, if possible

We should query for existing clients, connections, and channels with the parameters specified in the config. If they exist, we should use those client, connection, and channels since they are public goods. This will prevent normal users from accidentally flooding networks with a ton of new unused clients/connections/channels.

I would start with implementing this for clients. If the client identifier is nil, a query should be made for all existing clients with the same client type. I'm not sure if this is supported on the SDK, but we can possibly add it. Client identifiers are prefixed with their client type on the SDK so a get all clients could be done and then filter based on client identifier prefix. If the client with the same type has the same parameters such as chain id, unbonding period, trusting period, specs etc, then we should use this client.

The same should then be done for connections and channels.

Anyone can feel free to take this on, but should signal that they are working on it. I will start tackling this next week if no one else picks it up

@colin-axner colin-axner added T: Enhancement TYPE: Enhancement R: Major RELEASE: PR contains breaking changes that have to wait till a major release is made to be merged labels Jan 5, 2021
@colin-axner colin-axner self-assigned this Jan 11, 2021
@colin-axner colin-axner removed the R: Major RELEASE: PR contains breaking changes that have to wait till a major release is made to be merged label Jan 12, 2021
@colin-axner
Copy link
Contributor Author

I started work on this in #358 . This feature isn't as useful as I initially thought since the consensus state the client would be created at must exist in a matching client. It is unlikely initially for relayers to fill in all consensus states instead of skipping and only update here and there. If the client identifier cannot be reused, then the subsequent connection/channel cannot be reused since the client identifier will not match.

If it works, then it is better than not having it. I'm unsure of how easy it'll be to test this since the consensus states must match. Maybe integration tests can handle it, but atm they aren't working and docker needs to be debugged

@colin-axner
Copy link
Contributor Author

colin-axner commented Jan 20, 2021

tagging for akash

I've done the work for clients, but connections and channel identifier reuse needs to be implemented as well. It should be fairly straightforward and similar in structure to clients. It would be great if Akash pickup implementing the rest of this issue.

cc @gosuri @akhilkumarpilli @anilcse @boz

@akhilkumarpilli
Copy link
Contributor

We are looking on this.

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

Successfully merging a pull request may close this issue.

2 participants