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 SyncHeaders #414

Closed
colin-axner opened this issue Feb 9, 2021 · 2 comments · Fixed by #416
Closed

Remove SyncHeaders #414

colin-axner opened this issue Feb 9, 2021 · 2 comments · Fixed by #416
Assignees
Labels
T: Refactor TYPE: Refactor

Comments

@colin-axner
Copy link
Contributor

I never understood the exact purpose of SyncHeaders, but I realize now it is just a cache of the latest headers for each chain id ie a cache with a depth of 1 per chain-id. Sync makes sense in this context, but it doesn't make sense until you know what it does, stating the structure to be a cache of headers will make more sense imo

@colin-axner colin-axner added the T: Refactor TYPE: Refactor label Feb 9, 2021
@colin-axner
Copy link
Contributor Author

Alternatively, I'd be in favor of outright removing this structure. We really should just be interacting with the trusted store of the tendermint light client. It doesn't make much sense to keep a cache of one header, I don't even know a location where we benefit from this

@colin-axner colin-axner changed the title Rename SyncHeaders to CachedHeaders Rename SyncHeaders to CachedHeaders or Remove SyncHeaders Feb 9, 2021
@colin-axner colin-axner self-assigned this Feb 9, 2021
@colin-axner colin-axner changed the title Rename SyncHeaders to CachedHeaders or Remove SyncHeaders Remove SyncHeaders Feb 9, 2021
@colin-axner
Copy link
Contributor Author

I'll be removing them, my refactor of the light clients greatly reduces the complexity here. We simply can either update the off chain light client or get a header to update the on chain light client

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

Successfully merging a pull request may close this issue.

1 participant