Refactor light client handling, remove reliance on histroical info #416
Conversation
Remove SyncHeaders entirely. Reduce light client API. We can update off chain light clients. Then we can get creation or update headers to handle on chain light clients. No other functions necessary
| @@ -1,159 +0,0 @@ | |||
| package relayer | |||
|
This pull request introduces 2 alerts and fixes 2 when merging 970762f into 9f13a93 - view on LGTM.com new alerts:
fixed alerts:
|
|
This pull request introduces 2 alerts and fixes 4 when merging 1fd3e36 into 9f13a93 - view on LGTM.com new alerts:
fixed alerts:
|
|
cc @michaelfig when merged, this should fix any trusted headers errors (since it will now query from the connected light client) |
|
This pull request introduces 2 alerts and fixes 4 when merging 3b551e7 into 9f13a93 - view on LGTM.com new alerts:
fixed alerts:
|
|
fyi, the lgtm alert I believe is something that cannot be fixed and already has a lgtm suppression tag |
| @@ -86,7 +86,8 @@ func (c *Chain) CreateClients(dst *Chain) (modified bool, err error) { | |||
|
|
|||
| } else { | |||
| // Ensure client exists in the event of user inputted identifiers | |||
| _, err := c.QueryClientState(srcH.Header.Height) | |||
| // TODO: check client is not expired | |||
There was a problem hiding this comment.
I know this will error regardless, but maybe we should create an issue
There was a problem hiding this comment.
great minds think alike #417. I made one when I added it
| @@ -15,11 +15,11 @@ var ( | |||
| // Strategy defines | |||
| type Strategy interface { | |||
|
This pull request introduces 2 alerts and fixes 4 when merging 47ef9be into 1e98674 - view on LGTM.com new alerts:
fixed alerts:
|
I set out to redirect trusted header injection queries to use the connected full node and not the on-chain historical info since this was the cause of many problems for users. I quickly ran into many issues due to a poorly defined API with unnecessary complexity.
Specifically functions like
UpdatesWithHeadersupdates the light clients with untrusted headers (simply pulling the validator info from the current block without verifying it against a trusted header). This result in an error when trying to grab the trusted header after updating with that header since the light client never verified it and stored it in the store. Furthermore, I realized SyncHeaders was a depth one cache for headers mapped to chain-id. This added unnecessary layers of complexity with little to no performance benefit.I removed SyncHeaders enitrely. I replaced the existing API with much simpler logic. There is a function
UpdateLightClientwhich belongs to theChainstruct, which will update its connected off-chain light client and return the light block. Then there is a separate function also connected toChainstruct calledGetIBCUpdateHeaderwhich returns the IBC SDK TM header which will update an on-chain client. It is injected with the trusted header retrieved from the trusted store of the light client.I added some other useful functions like
GetIBCCreationHeader(no trusted header injected),MustGetLatestLightHeightlatest height of connected light client.closes: #375
closes: #414
closes: #327
closes: #373